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 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, &p);
> 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, &p);
> + if (p && *p == ':') {
> + p++;
> + oldp = p;
> + flags = simple_strtoull(p, &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, &start, &size);
> + rc = parse_memmap(str, &start, &size, 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 05/10] x86, efi: Add efi_fake_mem support for EFI_MEMORY_SP
* Dan Williams wrote: > Given that EFI_MEMORY_SP is platform BIOS policy descision for marking > 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. > > 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 A couple of these patches are touching EFI code, but only the first one carries a Reviewed-by from Ard. Ard, are these patches and the whole series fine with you? Thanks, Ingo
[PATCH v5 05/10] x86, efi: Add efi_fake_mem support for EFI_MEMORY_SP
Given that EFI_MEMORY_SP is platform BIOS policy descision for marking
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.
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, &p);
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, &p);
+ if (p && *p == ':') {
+ p++;
+ oldp = p;
+ flags = simple_strtoull(p, &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, &start, &size);
+ rc = parse_memmap(str, &start, &size, 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")) {
- mem_avoid_memmap(val);
+ mem_avoid_memmap(PARSE_MEMMAP, val);
} else if (strstr(param, "hugepages")) {
parse_gb_huge_pages(param, val);
} else if (!strcmp(param, "mem")) {
@@ -285,6 +315,8 @@ static void handle_mem_options(void)
goto out;
m
