[PATCH] ARM64: kernel: implement ACPI parking protocol

[lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)]

On Fri, Aug 28, 2015 at 04:56:36PM +0100, Mark Salter wrote:
> On Fri, 2015-08-28 at 16:32 +0100, Lorenzo Pieralisi wrote:
> > [Cc'ing Leif and Ard]
> > 
> > On Fri, Aug 28, 2015 at 03:29:46PM +0100, Mark Salter wrote:
> > > On Fri, 2015-08-28 at 11:23 +0100, Lorenzo Pieralisi wrote:
> > > > Mark,
> > > > 
> > > > On Thu, Jul 16, 2015 at 06:40:49PM +0100, Mark Salter wrote:
> > > > 
> > > > [...]
> > > > 
> > > > > On Thu, 2015-07-16 at 18:12 +0100, Lorenzo Pieralisi wrote:
> > > > > > > The kernel will only add cached memory regions to linear 
> > > > > > > mapping 
> > > > > > > and
> > > > > > > presumably, the FW will mark the mailboxes as uncached. 
> > > > > > > Otherwise, 
> > > > > > > it
> > > > > > > is a FW bug. But I suppose we could run into problems with 
> > > > > > > kernels
> > > > > > > using 64K pagesize since firmware assumes 4k.
> > > > > > 
> > > > > > Nope, ioremap takes care of that, everything should be fine.
> > > > > 
> > > > > The mailbox is 4K. If it is next to a cached UEFI region, the 
> > > > > kernel 
> > > > > may
> > > > > have to overlap the mailbox with a cached 64K mapping in order to 
> > > > > include
> > > > > the adjoining UEFI region in the linear map. Then the ioremap would 
> > > > > fail
> > > > > because the mailbox is included in the linear mapping.
> > > > 
> > > > So that I understand: are you referring to memrange_efi_to_native()
> > > > in arch/arm64/kernel/efi.c ? Is it safe to round up (and add it to
> > > > the memblock layer) the memory region size to PAGE_SIZE without 
> > > > checking
> > > > attributes of overlapping (within PAGE_SIZE) UEFI regions ?
> > > 
> > > The problem is that nothing in the UEFI spec or parking protocol spec
> > > prevents firmware from placing a 4K parking protocol mailbox area in
> > > the same 64K page as normal cached memory which winds up mapped in
> > > the kernel linear mapping. The kernel might be able to work around
> > > that by not putting the 64K page in the linear map. There's nothing
> > > the kernel could do if the mailbox is in same 64K page with UEFI 
> > > runtime
> > > memory which would use a cached mapping.
> > 
> > Ok, I failed to explain myself. Let's imagine that we have a, say, 64k
> > aligned memory region (EFI_MEMORY_WB - size 16k) that lives in the same
> > 64K memory frame as a device's registers memory space.
> > The code I pointed at above creates a 64K memory frame out of the 16K
> > region and adds it to the memblock so that it ends up in the kernel 
> > linear
> > mapping which ends up mapping the device registers too with cacheable
> > attributes and that's not correct.
> > 
> > I am not sure what's the best way to solve that, probably we should
> > amend the UEFI specs to enforce uniform 64K memory region attributes,
> > comments welcome.
> 
> OIC. Well it is a potential problem. Hardware designers do some whacky
> things, but I'd like to think I/O regions and cached mem regions bordering
> on such a small power of two boundary is not one of them.

Yes, I agree with you as far as I/O is concerned, I just wanted
to understand if the issue you mentioned is a consequence of how
the code above performs the rounding (it could discard memory regions
having mixed attributes within PAGE_SIZE instead, if we enforce it in
the specs).

Unfortunately for the mailboxes, since it is RAM, it might well happen.
If we change the UEFI specs to enforce uniform attributes on 64K regions
the problem you raised is solved at UEFI spec level instead of changing
the ACPI parking protocol specs.

I will raise the point, apart from that I will send a v2 soon with
the issue you raised addressed in the specs.

Thanks,
Lorenzo