Re: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices()

[Jeffrey Hugo]

On 2/1/2017 2:08 PM, Jeffrey Hugo wrote:
> On 2/1/2017 10:45 AM, Ard Biesheuvel wrote:
> > Some AArch64 UEFI implementations disable the MMU in ExitBootServices(),
> > after which unaligned accesses to RAM are no longer supported.
> >
> > Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
> > kernel") fixed an issue in the memory map handling of the stub FDT code,
> > but inadvertently created an issue with such firmwares, by moving some
> > of the FDT manipulation to after the invocation of ExitBootServices().
> > Given that the stub's libfdt implementation uses the ordinary,
> > accelerated
> > string functions, which rely on hardware handling of unaligned accesses,
> > manipulating the FDT with the MMU off may result in alignment faults.
> >
> > So fix the situation by moving the update_fdt_memmap() call into the
> > callback function invoked by efi_exit_boot_services() right before it
> > calls the ExitBootServices() UEFI service (which is arguably a better
> > place for it anyway)
> >
> > Note that disabling the MMU in ExitBootServices() is not compliant with
> > the UEFI spec, and carries great risk due to the fact that switching from
> > cached to uncached memory accesses halfway through compiler generated
> > code
> > (i.e., involving a stack) can never be done in a way that is
> > architecturally
> > safe.
> >
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
> > kernel")
> > Tested-by: Riku Voipio <riku.voipio@xxxxxxxxxx>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>
> NACK, please.  This causes a regression on my platform, in the form of
> an assert in UEFI once ExitBootServices() is called, per initial
> testing.  I'll do more testing to determine why.

Sorry, false alarm.  The assert appears to have been the result of a bad 
tree and a bad target configuration, and was not reproduced on another 
setup, nor did the assert make sense in context with this change.

I withdraw my NACK.

--
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.