Hi Ard, On 20/11/2018 17:35, Ard Biesheuvel wrote: > Mapping the MEMRESERVE EFI configuration table from an early initcall > is too late: the GICv3 ITS code that creates persistent reservations > for the boot CPU's LPI tables is invoked from init_IRQ(), which runs > much earlier than the handling of the initcalls. > > So instead, move the initialization performed by the initcall into > efi_mem_reserve_persistent() itself. > > Signed-off-by: Ard Biesheuvel <[email protected]>
I've just given it a go on one of my TX2s, and it boots just fine. So for that: Tested-by: Marc Zyngier <[email protected]> A comment below though: > --- > drivers/firmware/efi/efi.c | 26 ++++++++++---------------- > 1 file changed, 10 insertions(+), 16 deletions(-) > > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c > index fad7c62cfc0e..40de2f6734cc 100644 > --- a/drivers/firmware/efi/efi.c > +++ b/drivers/firmware/efi/efi.c > @@ -967,15 +967,23 @@ bool efi_is_table_address(unsigned long phys_addr) > } > > static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock); > -static struct linux_efi_memreserve *efi_memreserve_root __ro_after_init; > +static struct linux_efi_memreserve *efi_memreserve_root; > > int efi_mem_reserve_persistent(phys_addr_t addr, u64 size) > { > struct linux_efi_memreserve *rsv; > > - if (!efi_memreserve_root) > + if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR) > return -ENODEV; > > + if (!efi_memreserve_root) { > + efi_memreserve_root = memremap(efi.mem_reserve, > + sizeof(*efi_memreserve_root), > + MEMREMAP_WB); > + if (WARN_ON_ONCE(!efi_memreserve_root)) > + return -ENOMEM; This is now a bit racy if there is more than a single online CPU. > + } > + > rsv = kmalloc(sizeof(*rsv), GFP_ATOMIC); > if (!rsv) > return -ENOMEM; > @@ -991,20 +999,6 @@ int efi_mem_reserve_persistent(phys_addr_t addr, u64 > size) > return 0; > } > > -static int __init efi_memreserve_root_init(void) > -{ > - if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR) > - return -ENODEV; > - > - efi_memreserve_root = memremap(efi.mem_reserve, > - sizeof(*efi_memreserve_root), > - MEMREMAP_WB); > - if (!efi_memreserve_root) > - return -ENOMEM; > - return 0; > -} > -early_initcall(efi_memreserve_root_init); But if we keep this (+ a check that the root is indeed NULL), we should be able to make sure efi_memreserve_root is set before we enable a secondary CPU. Still fragile though. Thoughts? M. -- Jazz is not dead. It just smells funny...
