Re: [PATCH v5 05/10] x86, efi: Add efi_fake_mem support for EFI_MEMORY_SP
On Fri, Sep 13, 2019 at 12:49 PM Ard Biesheuvel wrote: > > On Fri, 30 Aug 2019 at 03:07, Dan Williams wrote: > > > ... > > diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile > > index 4ac2de4dfa72..d7a6db03ea79 100644 > > --- a/drivers/firmware/efi/Makefile > > +++ b/drivers/firmware/efi/Makefile > > @@ -20,13 +20,16 @@ obj-$(CONFIG_UEFI_CPER) += cper.o > > obj-$(CONFIG_EFI_RUNTIME_MAP) += runtime-map.o > > obj-$(CONFIG_EFI_RUNTIME_WRAPPERS) += runtime-wrappers.o > > obj-$(CONFIG_EFI_STUB) += libstub/ > > -obj-$(CONFIG_EFI_FAKE_MEMMAP) += fake_mem.o > > +obj-$(CONFIG_EFI_FAKE_MEMMAP) += fake_map.o > > obj-$(CONFIG_EFI_BOOTLOADER_CONTROL) += efibc.o > > obj-$(CONFIG_EFI_TEST) += test/ > > obj-$(CONFIG_EFI_DEV_PATH_PARSER) += dev-path-parser.o > > obj-$(CONFIG_APPLE_PROPERTIES) += apple-properties.o > > obj-$(CONFIG_EFI_RCI2_TABLE) += rci2-table.o > > > > +fake_map-y += fake_mem.o > > +fake_map-$(CONFIG_X86) += x86-fake_mem.o > > + > > Please use > > fake-mem-$(CONFIG_X86) := x86-fake_mem.o > obj-$(CONFIG_EFI_FAKE_MEMMAP) += fake_mem.o $(fake-mem-y) Ok, looks good. > > instead, and please use either - or _ in filenames, not both. Fair enough.
Re: [PATCH v5 05/10] x86, efi: Add efi_fake_mem support for EFI_MEMORY_SP
On Fri, 30 Aug 2019 at 03:07, Dan Williams wrote: > ... > diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile > index 4ac2de4dfa72..d7a6db03ea79 100644 > --- a/drivers/firmware/efi/Makefile > +++ b/drivers/firmware/efi/Makefile > @@ -20,13 +20,16 @@ obj-$(CONFIG_UEFI_CPER) += cper.o > obj-$(CONFIG_EFI_RUNTIME_MAP) += runtime-map.o > obj-$(CONFIG_EFI_RUNTIME_WRAPPERS) += runtime-wrappers.o > obj-$(CONFIG_EFI_STUB) += libstub/ > -obj-$(CONFIG_EFI_FAKE_MEMMAP) += fake_mem.o > +obj-$(CONFIG_EFI_FAKE_MEMMAP) += fake_map.o > obj-$(CONFIG_EFI_BOOTLOADER_CONTROL) += efibc.o > obj-$(CONFIG_EFI_TEST) += test/ > obj-$(CONFIG_EFI_DEV_PATH_PARSER) += dev-path-parser.o > obj-$(CONFIG_APPLE_PROPERTIES) += apple-properties.o > obj-$(CONFIG_EFI_RCI2_TABLE) += rci2-table.o > > +fake_map-y += fake_mem.o > +fake_map-$(CONFIG_X86) += x86-fake_mem.o > + Please use fake-mem-$(CONFIG_X86) := x86-fake_mem.o obj-$(CONFIG_EFI_FAKE_MEMMAP) += fake_mem.o $(fake-mem-y) instead, and please use either - or _ in filenames, not both.
Re: [PATCH v5 04/10] x86, efi: Reserve UEFI 2.8 Specific Purpose Memory for dax
On Fri, 13 Sep 2019 at 18:39, Ard Biesheuvel wrote:
>
> On Fri, 13 Sep 2019 at 17:39, Dan Williams wrote:
> >
> > On Fri, Sep 13, 2019 at 9:29 AM Ard Biesheuvel
> > wrote:
> > >
> > > On Fri, 13 Sep 2019 at 17:22, Dan Williams
> > > wrote:
> > > >
> > > > On Fri, Sep 13, 2019 at 6:00 AM Ard Biesheuvel
> > > > wrote:
> ...
> > > > > > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> > > > > > index 363bb9d00fa5..6d54d5c74347 100644
> > > > > > --- a/drivers/firmware/efi/efi.c
> > > > > > +++ b/drivers/firmware/efi/efi.c
> > > > > > @@ -52,6 +52,9 @@ struct efi __read_mostly efi = {
> > > > > > .tpm_log= EFI_INVALID_TABLE_ADDR,
> > > > > > .tpm_final_log = EFI_INVALID_TABLE_ADDR,
> > > > > > .mem_reserve= EFI_INVALID_TABLE_ADDR,
> > > > > > +#ifdef CONFIG_EFI_SOFT_RESERVE
> > > > > > + .flags = 1UL << EFI_MEM_SOFT_RESERVE,
> > > > > > +#endif
> > > > > > };
> > > > > > EXPORT_SYMBOL(efi);
> > > > > >
> > > > >
> > > > > I'd prefer it if we could call this EFI_MEM_NO_SOFT_RESERVE instead,
> > > > > and invert the meaning of the bit.
> > > >
> > > > ...but that would mean repeat occurrences of
> > > > "!efi_enabled(EFI_MEM_NO_SOFT_RESERVE)", doesn't the double negative
> > > > seem less readable to you?
> > > >
> > >
> > > One the one hand, yes. On the other hand, it is the only flag whose
> > > default is 'enabled' which is also less than ideal.
> >
> > Ok, I can get on board with "default 0" being the non exception state
> > of the flags.
> >
>
> In fact, let's just add something like
>
> static inline bool efi_soft_reserve_enabled(void)
> {
> return IS_ENABLED(CONFIG_EFI_SOFT_RESERVE) &&
>!efi_enabled(EFI_MEM_NO_SOFT_RESERVE);
> }
>
> to linux/efi.h and use that in the code?
Or even better, add just the declaration to linux.efi,h
bool __pure efi_soft_reserve_enabled(void);
and put one implementation in efi-stub-helper.c:
bool __pure efi_soft_reserve_enabled(void)
{
return IS_ENABLED(CONFIG_EFI_SOFT_RESERVE) &&
!efi_nosoftreserve;
}
and the one above in drivers/firmware/efi/efi.c
Re: [PATCH v5 04/10] x86, efi: Reserve UEFI 2.8 Specific Purpose Memory for dax
On Fri, 13 Sep 2019 at 17:39, Dan Williams wrote:
>
> On Fri, Sep 13, 2019 at 9:29 AM Ard Biesheuvel
> wrote:
> >
> > On Fri, 13 Sep 2019 at 17:22, Dan Williams wrote:
> > >
> > > On Fri, Sep 13, 2019 at 6:00 AM Ard Biesheuvel
> > > wrote:
...
> > > > > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> > > > > index 363bb9d00fa5..6d54d5c74347 100644
> > > > > --- a/drivers/firmware/efi/efi.c
> > > > > +++ b/drivers/firmware/efi/efi.c
> > > > > @@ -52,6 +52,9 @@ struct efi __read_mostly efi = {
> > > > > .tpm_log= EFI_INVALID_TABLE_ADDR,
> > > > > .tpm_final_log = EFI_INVALID_TABLE_ADDR,
> > > > > .mem_reserve= EFI_INVALID_TABLE_ADDR,
> > > > > +#ifdef CONFIG_EFI_SOFT_RESERVE
> > > > > + .flags = 1UL << EFI_MEM_SOFT_RESERVE,
> > > > > +#endif
> > > > > };
> > > > > EXPORT_SYMBOL(efi);
> > > > >
> > > >
> > > > I'd prefer it if we could call this EFI_MEM_NO_SOFT_RESERVE instead,
> > > > and invert the meaning of the bit.
> > >
> > > ...but that would mean repeat occurrences of
> > > "!efi_enabled(EFI_MEM_NO_SOFT_RESERVE)", doesn't the double negative
> > > seem less readable to you?
> > >
> >
> > One the one hand, yes. On the other hand, it is the only flag whose
> > default is 'enabled' which is also less than ideal.
>
> Ok, I can get on board with "default 0" being the non exception state
> of the flags.
>
In fact, let's just add something like
static inline bool efi_soft_reserve_enabled(void)
{
return IS_ENABLED(CONFIG_EFI_SOFT_RESERVE) &&
!efi_enabled(EFI_MEM_NO_SOFT_RESERVE);
}
to linux/efi.h and use that in the code?
Re: [PATCH v5 04/10] x86, efi: Reserve UEFI 2.8 Specific Purpose Memory for dax
On Fri, Sep 13, 2019 at 9:29 AM Ard Biesheuvel wrote: > > On Fri, 13 Sep 2019 at 17:22, Dan Williams wrote: > > > > On Fri, Sep 13, 2019 at 6:00 AM Ard Biesheuvel > > wrote: > > > > > > On Fri, 30 Aug 2019 at 03:06, Dan Williams > > > wrote: > > > > > > > > UEFI 2.8 defines an EFI_MEMORY_SP attribute bit to augment the > > > > interpretation of the EFI Memory Types as "reserved for a specific > > > > purpose". > > > > > > > > The proposed Linux behavior for specific purpose memory is that it is > > > > reserved for direct-access (device-dax) by default and not available for > > > > any kernel usage, not even as an OOM fallback. Later, through udev > > > > scripts or another init mechanism, these device-dax claimed ranges can > > > > be reconfigured and hot-added to the available System-RAM with a unique > > > > node identifier. This device-dax management scheme implements "soft" in > > > > the "soft reserved" designation by allowing some or all of the > > > > reservation to be recovered as typical memory. This policy can be > > > > disabled at compile-time with CONFIG_EFI_SOFT_RESERVE=n, or runtime with > > > > efi=nosoftreserve. > > > > > > > > This patch introduces 2 new concepts at once given the entanglement > > > > between early boot enumeration relative to memory that can optionally be > > > > reserved from the kernel page allocator by default. The new concepts > > > > are: > > > > > > > > - E820_TYPE_SOFT_RESERVED: Upon detecting the EFI_MEMORY_SP > > > > attribute on EFI_CONVENTIONAL memory, update the E820 map with this > > > > new type. Only perform this classification if the > > > > CONFIG_EFI_SOFT_RESERVE=y policy is enabled, otherwise treat it as > > > > typical ram. > > > > > > > > - IORES_DESC_SOFT_RESERVED: Add a new I/O resource descriptor for > > > > a device driver to search iomem resources for application specific > > > > memory. Teach the iomem code to identify such ranges as "Soft > > > > Reserved". > > > > > > > > A follow-on change integrates parsing of the ACPI HMAT to identify the > > > > node and sub-range boundaries of EFI_MEMORY_SP designated memory. For > > > > now, just identify and reserve memory of this type. > > > > > > > > The translation of EFI_CONVENTIONAL_MEMORY + EFI_MEMORY_SP to "soft > > > > reserved" is x86/E820-only, but other archs could choose to publish > > > > IORES_DESC_SOFT_RESERVED resources from their platform-firmware memory > > > > map handlers. Other EFI-capable platforms would need to go audit their > > > > local usages of EFI_CONVENTIONAL_MEMORY to consider the soft reserved > > > > case. > > > > > > > > Cc: > > > > Cc: Borislav Petkov > > > > Cc: Ingo Molnar > > > > Cc: "H. Peter Anvin" > > > > Cc: Darren Hart > > > > Cc: Andy Shevchenko > > > > Cc: Andy Lutomirski > > > > Cc: Peter Zijlstra > > > > Cc: Thomas Gleixner > > > > Cc: Ard Biesheuvel > > > > Reported-by: kbuild test robot > > > > Reviewed-by: Dave Hansen > > > > Signed-off-by: Dan Williams > > > > > > Hi Dan, > > > > > > I understand that non-x86 may be out of scope for you, but this patch > > > makes changes to x86 and generic code at the same time without regard > > > for other architectures. > > > > Yes, that did give me pause. > > > > > I'd prefer it if we could cover ARM cleanly as well right at the start. > > > > Let's do it. > > > > > > > > The first step would be to split out the EFI stub changes (i.e., to > > > avoid allocating memory from EFI_MEMORY_SP regions) and the EFI core > > > changes from the other changes. Then, I would like to ask for your > > > help to get the arm64 part implemented where EFI_MEMORY_SP memory gets > > > registered/reserved in a way that allows the HMAT code (which should > > > be arch agnostic) to operate in the same way as it does on x86. Would > > > it be enough to simply memblock_reserve() it and insert the iomem > > > resource with the soft_reserved attribute? > > > > > > Some more comments below. > > > > > > > --- > > > > Documentation/admin-guide/kernel-parameters.txt | 19 +++-- > > > > arch/x86/Kconfig| 21 + > > > > arch/x86/boot/compressed/eboot.c|7 +++ > > > > arch/x86/boot/compressed/kaslr.c|4 ++ > > > > arch/x86/include/asm/e820/types.h |8 > > > > arch/x86/include/asm/efi-stub.h | 11 + > > > > arch/x86/kernel/e820.c | 12 + > > > > arch/x86/platform/efi/efi.c | 51 > > > > +-- > > > > drivers/firmware/efi/efi.c |3 + > > > > drivers/firmware/efi/libstub/efi-stub-helper.c | 12 + > > > > include/linux/efi.h |1 > > > > include/linux/ioport.h |1 > > > > 12 files changed, 139 insertions(+), 11 deletions(-) > > > > create mode 100644 arch/x86/include/asm/efi-stub.h > > > > > > > > diff --git
Re: [PATCH v5 04/10] x86, efi: Reserve UEFI 2.8 Specific Purpose Memory for dax
On Fri, 13 Sep 2019 at 17:22, Dan Williams wrote: > > On Fri, Sep 13, 2019 at 6:00 AM Ard Biesheuvel > wrote: > > > > On Fri, 30 Aug 2019 at 03:06, Dan Williams wrote: > > > > > > UEFI 2.8 defines an EFI_MEMORY_SP attribute bit to augment the > > > interpretation of the EFI Memory Types as "reserved for a specific > > > purpose". > > > > > > The proposed Linux behavior for specific purpose memory is that it is > > > reserved for direct-access (device-dax) by default and not available for > > > any kernel usage, not even as an OOM fallback. Later, through udev > > > scripts or another init mechanism, these device-dax claimed ranges can > > > be reconfigured and hot-added to the available System-RAM with a unique > > > node identifier. This device-dax management scheme implements "soft" in > > > the "soft reserved" designation by allowing some or all of the > > > reservation to be recovered as typical memory. This policy can be > > > disabled at compile-time with CONFIG_EFI_SOFT_RESERVE=n, or runtime with > > > efi=nosoftreserve. > > > > > > This patch introduces 2 new concepts at once given the entanglement > > > between early boot enumeration relative to memory that can optionally be > > > reserved from the kernel page allocator by default. The new concepts > > > are: > > > > > > - E820_TYPE_SOFT_RESERVED: Upon detecting the EFI_MEMORY_SP > > > attribute on EFI_CONVENTIONAL memory, update the E820 map with this > > > new type. Only perform this classification if the > > > CONFIG_EFI_SOFT_RESERVE=y policy is enabled, otherwise treat it as > > > typical ram. > > > > > > - IORES_DESC_SOFT_RESERVED: Add a new I/O resource descriptor for > > > a device driver to search iomem resources for application specific > > > memory. Teach the iomem code to identify such ranges as "Soft Reserved". > > > > > > A follow-on change integrates parsing of the ACPI HMAT to identify the > > > node and sub-range boundaries of EFI_MEMORY_SP designated memory. For > > > now, just identify and reserve memory of this type. > > > > > > The translation of EFI_CONVENTIONAL_MEMORY + EFI_MEMORY_SP to "soft > > > reserved" is x86/E820-only, but other archs could choose to publish > > > IORES_DESC_SOFT_RESERVED resources from their platform-firmware memory > > > map handlers. Other EFI-capable platforms would need to go audit their > > > local usages of EFI_CONVENTIONAL_MEMORY to consider the soft reserved > > > case. > > > > > > Cc: > > > Cc: Borislav Petkov > > > Cc: Ingo Molnar > > > Cc: "H. Peter Anvin" > > > Cc: Darren Hart > > > Cc: Andy Shevchenko > > > Cc: Andy Lutomirski > > > Cc: Peter Zijlstra > > > Cc: Thomas Gleixner > > > Cc: Ard Biesheuvel > > > Reported-by: kbuild test robot > > > Reviewed-by: Dave Hansen > > > Signed-off-by: Dan Williams > > > > Hi Dan, > > > > I understand that non-x86 may be out of scope for you, but this patch > > makes changes to x86 and generic code at the same time without regard > > for other architectures. > > Yes, that did give me pause. > > > I'd prefer it if we could cover ARM cleanly as well right at the start. > > Let's do it. > > > > > The first step would be to split out the EFI stub changes (i.e., to > > avoid allocating memory from EFI_MEMORY_SP regions) and the EFI core > > changes from the other changes. Then, I would like to ask for your > > help to get the arm64 part implemented where EFI_MEMORY_SP memory gets > > registered/reserved in a way that allows the HMAT code (which should > > be arch agnostic) to operate in the same way as it does on x86. Would > > it be enough to simply memblock_reserve() it and insert the iomem > > resource with the soft_reserved attribute? > > > > Some more comments below. > > > > > --- > > > Documentation/admin-guide/kernel-parameters.txt | 19 +++-- > > > arch/x86/Kconfig| 21 + > > > arch/x86/boot/compressed/eboot.c|7 +++ > > > arch/x86/boot/compressed/kaslr.c|4 ++ > > > arch/x86/include/asm/e820/types.h |8 > > > arch/x86/include/asm/efi-stub.h | 11 + > > > arch/x86/kernel/e820.c | 12 + > > > arch/x86/platform/efi/efi.c | 51 > > > +-- > > > drivers/firmware/efi/efi.c |3 + > > > drivers/firmware/efi/libstub/efi-stub-helper.c | 12 + > > > include/linux/efi.h |1 > > > include/linux/ioport.h |1 > > > 12 files changed, 139 insertions(+), 11 deletions(-) > > > create mode 100644 arch/x86/include/asm/efi-stub.h > > > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt > > > b/Documentation/admin-guide/kernel-parameters.txt > > > index 1c67acd1df65..dd28f0726309 100644 > > > --- a/Documentation/admin-guide/kernel-parameters.txt > > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > > @@ -1152,7 +1152,8
Re: [PATCH v5 04/10] x86, efi: Reserve UEFI 2.8 Specific Purpose Memory for dax
On Fri, Sep 13, 2019 at 6:00 AM Ard Biesheuvel
wrote:
>
> On Fri, 30 Aug 2019 at 03:06, Dan Williams wrote:
> >
> > UEFI 2.8 defines an EFI_MEMORY_SP attribute bit to augment the
> > interpretation of the EFI Memory Types as "reserved for a specific
> > purpose".
> >
> > The proposed Linux behavior for specific purpose memory is that it is
> > reserved for direct-access (device-dax) by default and not available for
> > any kernel usage, not even as an OOM fallback. Later, through udev
> > scripts or another init mechanism, these device-dax claimed ranges can
> > be reconfigured and hot-added to the available System-RAM with a unique
> > node identifier. This device-dax management scheme implements "soft" in
> > the "soft reserved" designation by allowing some or all of the
> > reservation to be recovered as typical memory. This policy can be
> > disabled at compile-time with CONFIG_EFI_SOFT_RESERVE=n, or runtime with
> > efi=nosoftreserve.
> >
> > This patch introduces 2 new concepts at once given the entanglement
> > between early boot enumeration relative to memory that can optionally be
> > reserved from the kernel page allocator by default. The new concepts
> > are:
> >
> > - E820_TYPE_SOFT_RESERVED: Upon detecting the EFI_MEMORY_SP
> > attribute on EFI_CONVENTIONAL memory, update the E820 map with this
> > new type. Only perform this classification if the
> > CONFIG_EFI_SOFT_RESERVE=y policy is enabled, otherwise treat it as
> > typical ram.
> >
> > - IORES_DESC_SOFT_RESERVED: Add a new I/O resource descriptor for
> > a device driver to search iomem resources for application specific
> > memory. Teach the iomem code to identify such ranges as "Soft Reserved".
> >
> > A follow-on change integrates parsing of the ACPI HMAT to identify the
> > node and sub-range boundaries of EFI_MEMORY_SP designated memory. For
> > now, just identify and reserve memory of this type.
> >
> > The translation of EFI_CONVENTIONAL_MEMORY + EFI_MEMORY_SP to "soft
> > reserved" is x86/E820-only, but other archs could choose to publish
> > IORES_DESC_SOFT_RESERVED resources from their platform-firmware memory
> > map handlers. Other EFI-capable platforms would need to go audit their
> > local usages of EFI_CONVENTIONAL_MEMORY to consider the soft reserved
> > case.
> >
> > Cc:
> > Cc: Borislav Petkov
> > Cc: Ingo Molnar
> > Cc: "H. Peter Anvin"
> > Cc: Darren Hart
> > Cc: Andy Shevchenko
> > Cc: Andy Lutomirski
> > Cc: Peter Zijlstra
> > Cc: Thomas Gleixner
> > Cc: Ard Biesheuvel
> > Reported-by: kbuild test robot
> > Reviewed-by: Dave Hansen
> > Signed-off-by: Dan Williams
>
> Hi Dan,
>
> I understand that non-x86 may be out of scope for you, but this patch
> makes changes to x86 and generic code at the same time without regard
> for other architectures.
Yes, that did give me pause.
> I'd prefer it if we could cover ARM cleanly as well right at the start.
Let's do it.
>
> The first step would be to split out the EFI stub changes (i.e., to
> avoid allocating memory from EFI_MEMORY_SP regions) and the EFI core
> changes from the other changes. Then, I would like to ask for your
> help to get the arm64 part implemented where EFI_MEMORY_SP memory gets
> registered/reserved in a way that allows the HMAT code (which should
> be arch agnostic) to operate in the same way as it does on x86. Would
> it be enough to simply memblock_reserve() it and insert the iomem
> resource with the soft_reserved attribute?
>
> Some more comments below.
>
> > ---
> > Documentation/admin-guide/kernel-parameters.txt | 19 +++--
> > arch/x86/Kconfig| 21 +
> > arch/x86/boot/compressed/eboot.c|7 +++
> > arch/x86/boot/compressed/kaslr.c|4 ++
> > arch/x86/include/asm/e820/types.h |8
> > arch/x86/include/asm/efi-stub.h | 11 +
> > arch/x86/kernel/e820.c | 12 +
> > arch/x86/platform/efi/efi.c | 51
> > +--
> > drivers/firmware/efi/efi.c |3 +
> > drivers/firmware/efi/libstub/efi-stub-helper.c | 12 +
> > include/linux/efi.h |1
> > include/linux/ioport.h |1
> > 12 files changed, 139 insertions(+), 11 deletions(-)
> > create mode 100644 arch/x86/include/asm/efi-stub.h
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt
> > b/Documentation/admin-guide/kernel-parameters.txt
> > index 1c67acd1df65..dd28f0726309 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -1152,7 +1152,8 @@
> > Format: {"off" | "on" | "skip[mbr]"}
> >
> > efi=[EFI]
> > - Format: { "old_map", "nochunk", "noruntime",
> > "debug" }
> > + Format: { "old_map", "nochunk",
Re: [PATCH v5 05/10] x86, efi: Add efi_fake_mem support for EFI_MEMORY_SP
On Fri, Sep 13, 2019 at 6:02 AM Ard Biesheuvel wrote: > > On Fri, 30 Aug 2019 at 03:07, Dan Williams wrote: > > > > Given that EFI_MEMORY_SP is platform BIOS policy descision for marking > > decision Fixed. > > > memory ranges as "reserved for a specific purpose" there will inevitably > > be scenarios where the BIOS omits the attribute in situations where it > > is desired. Unlike other attributes if the OS wants to reserve this > > memory from the kernel the reservation needs to happen early in init. So > > early, in fact, that it needs to happen before e820__memblock_setup() > > which is a pre-requisite for efi_fake_memmap() that wants to allocate > > memory for the updated table. > > > > Introduce an x86 specific efi_fake_memmap_early() that can search for > > attempts to set EFI_MEMORY_SP via efi_fake_mem and update the e820 table > > accordingly. > > > > Is this early enough? The EFI stub runs before this, and allocates > memory as well. Unless I'm missing something the stub only allocates where the kernel will land. That should be handled by the new mem_avoid_memmap() extensions to consider "efi_fake_mem" in its exclusions.
Re: [PATCH v5 05/10] x86, efi: Add efi_fake_mem support for EFI_MEMORY_SP
On Fri, 30 Aug 2019 at 03:07, Dan Williams wrote:
>
> Given that EFI_MEMORY_SP is platform BIOS policy descision for marking
decision
> memory ranges as "reserved for a specific purpose" there will inevitably
> be scenarios where the BIOS omits the attribute in situations where it
> is desired. Unlike other attributes if the OS wants to reserve this
> memory from the kernel the reservation needs to happen early in init. So
> early, in fact, that it needs to happen before e820__memblock_setup()
> which is a pre-requisite for efi_fake_memmap() that wants to allocate
> memory for the updated table.
>
> Introduce an x86 specific efi_fake_memmap_early() that can search for
> attempts to set EFI_MEMORY_SP via efi_fake_mem and update the e820 table
> accordingly.
>
Is this early enough? The EFI stub runs before this, and allocates
memory as well.
> The KASLR code that scans the command line looking for user-directed
> memory reservations also needs to be updated to consider
> "efi_fake_mem=nn@ss:0x4" requests.
>
> Cc:
> Cc: Borislav Petkov
> Cc: Ingo Molnar
> Cc: "H. Peter Anvin"
> Cc: Thomas Gleixner
> Cc: Ard Biesheuvel
> Reviewed-by: Dave Hansen
> Signed-off-by: Dan Williams
> ---
> arch/x86/boot/compressed/kaslr.c| 46 ---
> arch/x86/include/asm/efi.h |8
> arch/x86/platform/efi/efi.c |2 +
> drivers/firmware/efi/Makefile |5 ++-
> drivers/firmware/efi/fake_mem.c | 24 ++--
> drivers/firmware/efi/fake_mem.h | 10 +
> drivers/firmware/efi/x86-fake_mem.c | 69
> +++
> 7 files changed, 143 insertions(+), 21 deletions(-)
> create mode 100644 drivers/firmware/efi/fake_mem.h
> create mode 100644 drivers/firmware/efi/x86-fake_mem.c
>
> diff --git a/arch/x86/boot/compressed/kaslr.c
> b/arch/x86/boot/compressed/kaslr.c
> index 093e84e28b7a..53ed3991f9a8 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -133,8 +133,14 @@ char *skip_spaces(const char *str)
> #include "../../../../lib/ctype.c"
> #include "../../../../lib/cmdline.c"
>
> +enum parse_mode {
> + PARSE_MEMMAP,
> + PARSE_EFI,
> +};
> +
> static int
> -parse_memmap(char *p, unsigned long long *start, unsigned long long *size)
> +parse_memmap(char *p, unsigned long long *start, unsigned long long *size,
> + enum parse_mode mode)
> {
> char *oldp;
>
> @@ -157,8 +163,33 @@ parse_memmap(char *p, unsigned long long *start,
> unsigned long long *size)
> *start = memparse(p + 1, );
> return 0;
> case '@':
> - /* memmap=nn@ss specifies usable region, should be skipped */
> - *size = 0;
> + if (mode == PARSE_MEMMAP) {
> + /*
> +* memmap=nn@ss specifies usable region, should
> +* be skipped
> +*/
> + *size = 0;
> + } else {
> + unsigned long long flags;
> +
> + /*
> +* efi_fake_mem=nn@ss:attr the attr specifies
> +* flags that might imply a soft-reservation.
> +*/
> + *start = memparse(p + 1, );
> + if (p && *p == ':') {
> + p++;
> + oldp = p;
> + flags = simple_strtoull(p, , 0);
> + if (p == oldp)
> + *size = 0;
> + else if (flags & EFI_MEMORY_SP)
> + return 0;
> + else
> + *size = 0;
> + } else
> + *size = 0;
> + }
> /* Fall through */
> default:
> /*
> @@ -173,7 +204,7 @@ parse_memmap(char *p, unsigned long long *start, unsigned
> long long *size)
> return -EINVAL;
> }
>
> -static void mem_avoid_memmap(char *str)
> +static void mem_avoid_memmap(enum parse_mode mode, char *str)
> {
> static int i;
>
> @@ -188,7 +219,7 @@ static void mem_avoid_memmap(char *str)
> if (k)
> *k++ = 0;
>
> - rc = parse_memmap(str, , );
> + rc = parse_memmap(str, , , mode);
> if (rc < 0)
> break;
> str = k;
> @@ -239,7 +270,6 @@ static void parse_gb_huge_pages(char *param, char *val)
> }
> }
>
> -
> static void handle_mem_options(void)
> {
> char *args = (char *)get_cmd_line_ptr();
> @@ -272,7 +302,7 @@ static void handle_mem_options(void)
> }
>
> if (!strcmp(param, "memmap")) {
> -
Re: [PATCH v5 04/10] x86, efi: Reserve UEFI 2.8 Specific Purpose Memory for dax
On Fri, 30 Aug 2019 at 03:06, Dan Williams wrote:
>
> UEFI 2.8 defines an EFI_MEMORY_SP attribute bit to augment the
> interpretation of the EFI Memory Types as "reserved for a specific
> purpose".
>
> The proposed Linux behavior for specific purpose memory is that it is
> reserved for direct-access (device-dax) by default and not available for
> any kernel usage, not even as an OOM fallback. Later, through udev
> scripts or another init mechanism, these device-dax claimed ranges can
> be reconfigured and hot-added to the available System-RAM with a unique
> node identifier. This device-dax management scheme implements "soft" in
> the "soft reserved" designation by allowing some or all of the
> reservation to be recovered as typical memory. This policy can be
> disabled at compile-time with CONFIG_EFI_SOFT_RESERVE=n, or runtime with
> efi=nosoftreserve.
>
> This patch introduces 2 new concepts at once given the entanglement
> between early boot enumeration relative to memory that can optionally be
> reserved from the kernel page allocator by default. The new concepts
> are:
>
> - E820_TYPE_SOFT_RESERVED: Upon detecting the EFI_MEMORY_SP
> attribute on EFI_CONVENTIONAL memory, update the E820 map with this
> new type. Only perform this classification if the
> CONFIG_EFI_SOFT_RESERVE=y policy is enabled, otherwise treat it as
> typical ram.
>
> - IORES_DESC_SOFT_RESERVED: Add a new I/O resource descriptor for
> a device driver to search iomem resources for application specific
> memory. Teach the iomem code to identify such ranges as "Soft Reserved".
>
> A follow-on change integrates parsing of the ACPI HMAT to identify the
> node and sub-range boundaries of EFI_MEMORY_SP designated memory. For
> now, just identify and reserve memory of this type.
>
> The translation of EFI_CONVENTIONAL_MEMORY + EFI_MEMORY_SP to "soft
> reserved" is x86/E820-only, but other archs could choose to publish
> IORES_DESC_SOFT_RESERVED resources from their platform-firmware memory
> map handlers. Other EFI-capable platforms would need to go audit their
> local usages of EFI_CONVENTIONAL_MEMORY to consider the soft reserved
> case.
>
> Cc:
> Cc: Borislav Petkov
> Cc: Ingo Molnar
> Cc: "H. Peter Anvin"
> Cc: Darren Hart
> Cc: Andy Shevchenko
> Cc: Andy Lutomirski
> Cc: Peter Zijlstra
> Cc: Thomas Gleixner
> Cc: Ard Biesheuvel
> Reported-by: kbuild test robot
> Reviewed-by: Dave Hansen
> Signed-off-by: Dan Williams
Hi Dan,
I understand that non-x86 may be out of scope for you, but this patch
makes changes to x86 and generic code at the same time without regard
for other architectures.
I'd prefer it if we could cover ARM cleanly as well right at the start.
The first step would be to split out the EFI stub changes (i.e., to
avoid allocating memory from EFI_MEMORY_SP regions) and the EFI core
changes from the other changes. Then, I would like to ask for your
help to get the arm64 part implemented where EFI_MEMORY_SP memory gets
registered/reserved in a way that allows the HMAT code (which should
be arch agnostic) to operate in the same way as it does on x86. Would
it be enough to simply memblock_reserve() it and insert the iomem
resource with the soft_reserved attribute?
Some more comments below.
> ---
> Documentation/admin-guide/kernel-parameters.txt | 19 +++--
> arch/x86/Kconfig| 21 +
> arch/x86/boot/compressed/eboot.c|7 +++
> arch/x86/boot/compressed/kaslr.c|4 ++
> arch/x86/include/asm/e820/types.h |8
> arch/x86/include/asm/efi-stub.h | 11 +
> arch/x86/kernel/e820.c | 12 +
> arch/x86/platform/efi/efi.c | 51
> +--
> drivers/firmware/efi/efi.c |3 +
> drivers/firmware/efi/libstub/efi-stub-helper.c | 12 +
> include/linux/efi.h |1
> include/linux/ioport.h |1
> 12 files changed, 139 insertions(+), 11 deletions(-)
> create mode 100644 arch/x86/include/asm/efi-stub.h
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt
> b/Documentation/admin-guide/kernel-parameters.txt
> index 1c67acd1df65..dd28f0726309 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1152,7 +1152,8 @@
> Format: {"off" | "on" | "skip[mbr]"}
>
> efi=[EFI]
> - Format: { "old_map", "nochunk", "noruntime", "debug" }
> + Format: { "old_map", "nochunk", "noruntime", "debug",
> + "nosoftreserve" }
> old_map [X86-64]: switch to the old ioremap-based EFI
> runtime services mapping. 32-bit still uses this one
> by
> default.
> @@ -1161,6 +1162,12 @@
>
Re: [PATCH v5 03/10] x86, efi: Push EFI_MEMMAP check into leaf routines
On Fri, Sep 13, 2019 at 2:06 AM Ard Biesheuvel wrote: > > On Fri, 30 Aug 2019 at 03:06, Dan Williams wrote: > > > > In preparation for adding another EFI_MEMMAP dependent call that needs > > to occur before e820__memblock_setup() fixup the existing efi calls to > > check for EFI_MEMMAP internally. This ends up being cleaner than the > > alternative of checking EFI_MEMMAP multiple times in setup_arch(). > > > > Cc: > > Cc: Ingo Molnar > > Cc: "H. Peter Anvin" > > Cc: Andy Lutomirski > > Cc: Thomas Gleixner > > Cc: Peter Zijlstra > > Reviewed-by: Dave Hansen > > Signed-off-by: Dan Williams > > I'd prefer it if the spurious whitespace changes could be dropped, but > otherwise, this looks fine to me, so I am not going to obsess about > it. Fair point, I'll drop those when I resubmit after -rc1. > Reviewed-by: Ard Biesheuvel Thanks!
Re: [PATCH v5 03/10] x86, efi: Push EFI_MEMMAP check into leaf routines
On Fri, 30 Aug 2019 at 03:06, Dan Williams wrote:
>
> In preparation for adding another EFI_MEMMAP dependent call that needs
> to occur before e820__memblock_setup() fixup the existing efi calls to
> check for EFI_MEMMAP internally. This ends up being cleaner than the
> alternative of checking EFI_MEMMAP multiple times in setup_arch().
>
> Cc:
> Cc: Ingo Molnar
> Cc: "H. Peter Anvin"
> Cc: Andy Lutomirski
> Cc: Thomas Gleixner
> Cc: Peter Zijlstra
> Reviewed-by: Dave Hansen
> Signed-off-by: Dan Williams
I'd prefer it if the spurious whitespace changes could be dropped, but
otherwise, this looks fine to me, so I am not going to obsess about
it.
Reviewed-by: Ard Biesheuvel
> ---
> arch/x86/include/asm/efi.h |9 -
> arch/x86/kernel/setup.c | 19 +--
> arch/x86/platform/efi/efi.c |3 +++
> arch/x86/platform/efi/quirks.c |3 +++
> drivers/firmware/efi/esrt.c |3 +++
> drivers/firmware/efi/fake_mem.c |2 +-
> include/linux/efi.h |2 --
> 7 files changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index 43a82e59c59d..45f853bce869 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -140,7 +140,6 @@ extern void efi_delete_dummy_variable(void);
> extern void efi_switch_mm(struct mm_struct *mm);
> extern void efi_recover_from_page_fault(unsigned long phys_addr);
> extern void efi_free_boot_services(void);
> -extern void efi_reserve_boot_services(void);
>
> struct efi_setup_data {
> u64 fw_vendor;
> @@ -244,6 +243,8 @@ static inline bool efi_is_64bit(void)
> extern bool efi_reboot_required(void);
> extern bool efi_is_table_address(unsigned long phys_addr);
>
> +extern void efi_find_mirror(void);
> +extern void efi_reserve_boot_services(void);
> #else
> static inline void parse_efi_setup(u64 phys_addr, u32 data_len) {}
> static inline bool efi_reboot_required(void)
> @@ -254,6 +255,12 @@ static inline bool efi_is_table_address(unsigned long
> phys_addr)
> {
> return false;
> }
> +static inline void efi_find_mirror(void)
> +{
> +}
> +static inline void efi_reserve_boot_services(void)
> +{
> +}
> #endif /* CONFIG_EFI */
>
> #endif /* _ASM_X86_EFI_H */
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index bbe35bf879f5..9bfecb542440 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1118,21 +1118,20 @@ void __init setup_arch(char **cmdline_p)
> cleanup_highmap();
>
> memblock_set_current_limit(ISA_END_ADDRESS);
> +
> e820__memblock_setup();
>
> reserve_bios_regions();
>
> - if (efi_enabled(EFI_MEMMAP)) {
> - efi_fake_memmap();
> - efi_find_mirror();
> - efi_esrt_init();
> + efi_fake_memmap();
> + efi_find_mirror();
> + efi_esrt_init();
>
> - /*
> -* The EFI specification says that boot service code won't be
> -* called after ExitBootServices(). This is, in fact, a lie.
> -*/
> - efi_reserve_boot_services();
> - }
> + /*
> +* The EFI specification says that boot service code won't be
> +* called after ExitBootServices(). This is, in fact, a lie.
> +*/
> + efi_reserve_boot_services();
>
> /* preallocate 4k for mptable mpc */
> e820__memblock_alloc_reserved_mpc_new();
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index c202e1b07e29..0bb58eb33ca0 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -128,6 +128,9 @@ void __init efi_find_mirror(void)
> efi_memory_desc_t *md;
> u64 mirror_size = 0, total_size = 0;
>
> + if (!efi_enabled(EFI_MEMMAP))
> + return;
> +
> for_each_efi_memory_desc(md) {
> unsigned long long start = md->phys_addr;
> unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index 3b9fd679cea9..7675cf754d90 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -320,6 +320,9 @@ void __init efi_reserve_boot_services(void)
> {
> efi_memory_desc_t *md;
>
> + if (!efi_enabled(EFI_MEMMAP))
> + return;
> +
> for_each_efi_memory_desc(md) {
> u64 start = md->phys_addr;
> u64 size = md->num_pages << EFI_PAGE_SHIFT;
> diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
> index d6dd5f503fa2..2762e0662bf4 100644
> --- a/drivers/firmware/efi/esrt.c
> +++ b/drivers/firmware/efi/esrt.c
> @@ -246,6 +246,9 @@ void __init efi_esrt_init(void)
> int rc;
> phys_addr_t end;
>
> + if (!efi_enabled(EFI_MEMMAP))
> + return;
> +
> pr_debug("esrt-init:
