Re: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup

2015-01-10 Thread Bryan O'Donoghue
I'm wondering if there is a need for this to be a platform driver at all. Could we, instead, tear down any unlocked IMRs at boot as part of the IMR driver itself, as well as lock the kernel .text. Firmware has the option of making an IMR mandatory by locking it. If it isn't locked, what use is i

Re: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup

2015-01-09 Thread Darren Hart
On Fri, Jan 09, 2015 at 11:17:43AM +, Bryan O'Donoghue wrote: > On 09/01/15 04:46, Darren Hart wrote: > >>+ /* Try overlap - IMR_ALIGN */ > >>+ base = base + size - IMR_ALIGN; > >>+ if (imr_add(base, size, IMR_NON_SMM, IMR_NON_SMM, true) == 0) > >>+ pr_err(SANITY "overlapped IMR

RE: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup

2015-01-09 Thread Ong, Boon Leong
> >On 09/01/15 11:17, Bryan O'Donoghue wrote: >>> So this will load on any Quark device, Galileo or not, that doesn't >>> provide a system_id. Is there any reason we need to support 0.8.0 and >>> earlier firmware? >> >> Every Galileo Gen1 device ships with firmware version 0.7.5. You can >> do an E

Re: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup

2015-01-09 Thread Bryan O'Donoghue
On 09/01/15 11:17, Bryan O'Donoghue wrote: So this will load on any Quark device, Galileo or not, that doesn't provide a system_id. Is there any reason we need to support 0.8.0 and earlier firmware? Every Galileo Gen1 device ships with firmware version 0.7.5. You can do an EFI capsule update to

Re: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup

2015-01-09 Thread Bryan O'Donoghue
On 09/01/15 04:46, Darren Hart wrote: + /* Try overlap - IMR_ALIGN */ + base = base + size - IMR_ALIGN; + if (imr_add(base, size, IMR_NON_SMM, IMR_NON_SMM, true) == 0) + pr_err(SANITY "overlapped IMR @ (0x%08lx - 0x%08lx)\n", + base, base + siz

Re: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup

2015-01-08 Thread Darren Hart
On Mon, Dec 29, 2014 at 05:23:03PM +, Bryan O'Donoghue wrote: Hi Bryan, Good discussion from Andy and Boon Leong, I'll try not to duplicate their review here. > Intel Galileo Gen1 and Gen2 boot to Linux from EFI and grub with IMR > registers enabled around the compressed kernel image and boo

Re: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup

2015-01-08 Thread Bryan O'Donoghue
On 09/01/15 01:00, Ong, Boon Leong wrote: +static void __init +intel_galileo_imr_sanity(unsigned long base, unsigned long size) { + /* Test zero zero */ + if (imr_add(0, 0, 0, 0, true) == 0) + pr_err(SANITY "zero sized IMR @ 0x\n"); A side-discussion on imr_ad

RE: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup

2015-01-08 Thread Ong, Boon Leong
>Subject: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup > [snippet removed] >Since the Quark EFI bringup code configures the system to reset on an IMR Typo: bring-up >violation, this means that common operations such as mouting an SD based root Typo: mounti

Re: [PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup

2014-12-31 Thread Andy Shevchenko
On Mon, Dec 29, 2014 at 7:23 PM, Bryan O'Donoghue wrote: > Intel Galileo Gen1 and Gen2 boot to Linux from EFI and grub with IMR > registers enabled around the compressed kernel image and boot params data > structure. > > The purpose of the IMRs around the compressed kernel and boot params data > s

[PATCH 2/2] platform/x86: Add Intel Galileo platform specific setup

2014-12-29 Thread Bryan O'Donoghue
Intel Galileo Gen1 and Gen2 boot to Linux from EFI and grub with IMR registers enabled around the compressed kernel image and boot params data structure. The purpose of the IMRs around the compressed kernel and boot params data structure is to ensure that no spurious data writes from any agent wit