Re: [PATCH v2 3/3] x86, efi: Add "efi_fake_mem_mirror" boot option
On 12 September 2015 at 12:41, Matt Fleming wrote: > On Wed, 09 Sep, at 04:16:09PM, Ard Biesheuvel wrote: >> >> Hello Taku, >> >> To be honest, I think that the naming of this feature is poorly >> chosen. The UEFI spec gets it right by using 'MORE_RELIABLE'. Since >> one way to implement more reliable memory ranges is mirroring, the >> implementation detail of that has leaked into the generic naming, >> which is confusing. Not your fault though, just something I wanted to >> highlight. > > Care to suggest an alternative option? efi_fake_mem_more_reliable ? > No, that does not make sense either. I don't like the name that was chosen when the feature was added to memblock, and now I suppose we just have to live with it. > Maybe we should go further than this current design and generalise > things to allow an EFI_MEMORY_ATTRIBUTE value to be specified for > these memory ranges that supplements the ones actually provided by the > firmware? > > Something like, > > efi_fake_mem=2G@4G:0x1,2G@0x10a000:0x1 > > Where 0x1 is the EFI_MEMORY_MORE_RELIABLE attribute bit. > Yes, I like that. Should we use a mask/xor pair for flexibility? > That would seem incredibly useful for testing the kernel side of the > EFI_PROPERTIES_TABLE changes, i.e. you wouldn't need support in the > firmware and could just "mock-up" an EFI memory map with EFI_MEMORY_XP > for the data regions (code regions and EFI_MEMORY_RO are a little > trickier as I understand it, because they may also contain data). > ... hence the need for the memprotect feature in the first place. PE/COFF images are normally covered in their entirety (.text + .data) by a single BScode/RTcode region. But indeed, the ability to manipulate the memory map like that could be useful, although it would need to be ported to the stub to be useful on ARM, I think. >> So first of all, could you please update the example so that it only >> shows a single more reliable region (or two but of different sizes)? >> It took me a while to figure out that those 2 GB regions are not >> mirrors of each other in any way, they are simply two separate regions >> that are marked as more reliable than the remaining memory. >> >> I do wonder if this functionality belongs in the kernel, though. I see >> how it could be useful, and you can keep it as a local hack, but >> generally, the firmware (OVMF?) is a better way to play around with >> code like this, I think? > > I (partially) disagree. Using real life memory maps has its > advantages, since different layouts exercise the code in different > ways, and I'd really like to see this used on beefy machines with > multiple GB/TB or RAM. It also allows performance measurements to be > taken with bare metal accuracy. Plus there's precedent in the kernel > for creating fake memory/topology objects, e.g. see numa=fake. > OK, perhaps I just don't understand the use case too well. > Not everyone who touches the EFI memory mirror code is going to want > (or be able) to run firmware with EFI_MEMORY_MORE_RELIABLE support. > > Having said that, I'd love to also see EFI_MEMORY_MORE_RELIABLE > support in OVMF! I think both options make sense for different > reasons. > Good point. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] x86, efi: Add "efi_fake_mem_mirror" boot option
On Wed, 09 Sep, at 04:16:09PM, Ard Biesheuvel wrote: > > Hello Taku, > > To be honest, I think that the naming of this feature is poorly > chosen. The UEFI spec gets it right by using 'MORE_RELIABLE'. Since > one way to implement more reliable memory ranges is mirroring, the > implementation detail of that has leaked into the generic naming, > which is confusing. Not your fault though, just something I wanted to > highlight. Care to suggest an alternative option? efi_fake_mem_more_reliable ? Maybe we should go further than this current design and generalise things to allow an EFI_MEMORY_ATTRIBUTE value to be specified for these memory ranges that supplements the ones actually provided by the firmware? Something like, efi_fake_mem=2G@4G:0x1,2G@0x10a000:0x1 Where 0x1 is the EFI_MEMORY_MORE_RELIABLE attribute bit. That would seem incredibly useful for testing the kernel side of the EFI_PROPERTIES_TABLE changes, i.e. you wouldn't need support in the firmware and could just "mock-up" an EFI memory map with EFI_MEMORY_XP for the data regions (code regions and EFI_MEMORY_RO are a little trickier as I understand it, because they may also contain data). > So first of all, could you please update the example so that it only > shows a single more reliable region (or two but of different sizes)? > It took me a while to figure out that those 2 GB regions are not > mirrors of each other in any way, they are simply two separate regions > that are marked as more reliable than the remaining memory. > > I do wonder if this functionality belongs in the kernel, though. I see > how it could be useful, and you can keep it as a local hack, but > generally, the firmware (OVMF?) is a better way to play around with > code like this, I think? I (partially) disagree. Using real life memory maps has its advantages, since different layouts exercise the code in different ways, and I'd really like to see this used on beefy machines with multiple GB/TB or RAM. It also allows performance measurements to be taken with bare metal accuracy. Plus there's precedent in the kernel for creating fake memory/topology objects, e.g. see numa=fake. Not everyone who touches the EFI memory mirror code is going to want (or be able) to run firmware with EFI_MEMORY_MORE_RELIABLE support. Having said that, I'd love to also see EFI_MEMORY_MORE_RELIABLE support in OVMF! I think both options make sense for different reasons. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] x86, efi: Add "efi_fake_mem_mirror" boot option
On 26 August 2015 at 19:11, Taku Izumi wrote: > This patch introduces new boot option named "efi_fake_mem_mirror". > By specifying this parameter, you can mark specific memory as > mirrored memory. This is useful for debugging of Address Range > Mirroring feature. > > For example, if you specify "efi_fake_mem_mirror=2G@4G,2G@0x10a000", > the original (firmware provided) EFI memmap will be updated so that > the specified memory regions have EFI_MEMORY_MORE_RELIABLE attribute: > > >efi: mem00: [Boot Data | | | | | | |WB|WT|WC|UC] > range=[0x-0x1000) (0MB) >efi: mem01: [Loader Data| | | | | | |WB|WT|WC|UC] > range=[0x1000-0x2000) (0MB) >... >efi: mem35: [Boot Data | | | | | | |WB|WT|WC|UC] > range=[0x47ee6000-0x48014000) (1MB) >efi: mem36: [Conventional Memory| | | | | | |WB|WT|WC|UC] > range=[0x0001-0x0020a000) (129536MB) >efi: mem37: [Reserved |RT| | | | | | | | |UC] > range=[0x6000-0x9000) (768MB) > > >efi: mem00: [Boot Data | | | | | | |WB|WT|WC|UC] > range=[0x-0x1000) (0MB) >efi: mem01: [Loader Data| | | | | | |WB|WT|WC|UC] > range=[0x1000-0x2000) (0MB) >... >efi: mem35: [Boot Data | | | | | | |WB|WT|WC|UC] > range=[0x47ee6000-0x48014000) (1MB) >efi: mem36: [Conventional Memory| |MR| | | | |WB|WT|WC|UC] > range=[0x0001-0x00018000) (2048MB) >efi: mem37: [Conventional Memory| | | | | | |WB|WT|WC|UC] > range=[0x00018000-0x0010a000) (61952MB) >efi: mem38: [Conventional Memory| |MR| | | | |WB|WT|WC|UC] > range=[0x0010a000-0x00112000) (2048MB) >efi: mem39: [Conventional Memory| | | | | | |WB|WT|WC|UC] > range=[0x00112000-0x0020a000) (63488MB) >efi: mem40: [Reserved |RT| | | | | | | | |UC] > range=[0x6000-0x9000) (768MB) > > And you will find that the following message is output: > >efi: Memory: 4096M/131455M mirrored memory > > Signed-off-by: Taku Izumi Hello Taku, To be honest, I think that the naming of this feature is poorly chosen. The UEFI spec gets it right by using 'MORE_RELIABLE'. Since one way to implement more reliable memory ranges is mirroring, the implementation detail of that has leaked into the generic naming, which is confusing. Not your fault though, just something I wanted to highlight. So first of all, could you please update the example so that it only shows a single more reliable region (or two but of different sizes)? It took me a while to figure out that those 2 GB regions are not mirrors of each other in any way, they are simply two separate regions that are marked as more reliable than the remaining memory. I do wonder if this functionality belongs in the kernel, though. I see how it could be useful, and you can keep it as a local hack, but generally, the firmware (OVMF?) is a better way to play around with code like this, I think? -- Ard. > --- > Documentation/kernel-parameters.txt | 8 ++ > arch/x86/include/asm/efi.h | 1 + > arch/x86/kernel/setup.c | 4 +- > arch/x86/platform/efi/efi.c | 2 +- > drivers/firmware/efi/Kconfig| 12 +++ > drivers/firmware/efi/Makefile | 1 + > drivers/firmware/efi/fake_mem.c | 204 > > include/linux/efi.h | 6 ++ > 8 files changed, 236 insertions(+), 2 deletions(-) > create mode 100644 drivers/firmware/efi/fake_mem.c > > diff --git a/Documentation/kernel-parameters.txt > b/Documentation/kernel-parameters.txt > index 1d6f045..0efded6 100644 > --- a/Documentation/kernel-parameters.txt > +++ b/Documentation/kernel-parameters.txt > @@ -1092,6 +1092,14 @@ bytes respectively. Such letter suffixes can also be > entirely omitted. > you are really sure that your UEFI does sane gc and > fulfills the spec otherwise your board may brick. > > + efi_fake_mem_mirror=nn[KMG]@ss[KMG][,nn[KMG]@ss[KMG],..] [EFI; X86] > + Mark specific memory as mirrored memory and update > + EFI memory map. > + Region of memory to be marked is from ss to ss+nn. > + Using this parameter you can do debugging of Address > + Range Mirroring feature even if your box doesn't > support > + it. > + > eisa_irq_edge= [PARISC,HW] > See header of drivers/parisc/eisa.c. > > diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h > index 155162e..479fd51 100644 > --- a/arch/x86/include/asm/efi.h > +++ b/arch/x86/include/asm/efi.h > @@ -9
Re: [PATCH v2 3/3] x86, efi: Add "efi_fake_mem_mirror" boot option
On Thu, 27 Aug, at 02:11:37AM, Taku Izumi wrote:
> This patch introduces new boot option named "efi_fake_mem_mirror".
> By specifying this parameter, you can mark specific memory as
> mirrored memory. This is useful for debugging of Address Range
> Mirroring feature.
>
> For example, if you specify "efi_fake_mem_mirror=2G@4G,2G@0x10a000",
> the original (firmware provided) EFI memmap will be updated so that
> the specified memory regions have EFI_MEMORY_MORE_RELIABLE attribute:
>
>
>efi: mem00: [Boot Data | | | | | | |WB|WT|WC|UC]
> range=[0x-0x1000) (0MB)
>efi: mem01: [Loader Data| | | | | | |WB|WT|WC|UC]
> range=[0x1000-0x2000) (0MB)
>...
>efi: mem35: [Boot Data | | | | | | |WB|WT|WC|UC]
> range=[0x47ee6000-0x48014000) (1MB)
>efi: mem36: [Conventional Memory| | | | | | |WB|WT|WC|UC]
> range=[0x0001-0x0020a000) (129536MB)
>efi: mem37: [Reserved |RT| | | | | | | | |UC]
> range=[0x6000-0x9000) (768MB)
>
>
>efi: mem00: [Boot Data | | | | | | |WB|WT|WC|UC]
> range=[0x-0x1000) (0MB)
>efi: mem01: [Loader Data| | | | | | |WB|WT|WC|UC]
> range=[0x1000-0x2000) (0MB)
>...
>efi: mem35: [Boot Data | | | | | | |WB|WT|WC|UC]
> range=[0x47ee6000-0x48014000) (1MB)
>efi: mem36: [Conventional Memory| |MR| | | | |WB|WT|WC|UC]
> range=[0x0001-0x00018000) (2048MB)
>efi: mem37: [Conventional Memory| | | | | | |WB|WT|WC|UC]
> range=[0x00018000-0x0010a000) (61952MB)
>efi: mem38: [Conventional Memory| |MR| | | | |WB|WT|WC|UC]
> range=[0x0010a000-0x00112000) (2048MB)
>efi: mem39: [Conventional Memory| | | | | | |WB|WT|WC|UC]
> range=[0x00112000-0x0020a000) (63488MB)
>efi: mem40: [Reserved |RT| | | | | | | | |UC]
> range=[0x6000-0x9000) (768MB)
>
> And you will find that the following message is output:
>
>efi: Memory: 4096M/131455M mirrored memory
>
> Signed-off-by: Taku Izumi
> ---
> Documentation/kernel-parameters.txt | 8 ++
> arch/x86/include/asm/efi.h | 1 +
> arch/x86/kernel/setup.c | 4 +-
> arch/x86/platform/efi/efi.c | 2 +-
> drivers/firmware/efi/Kconfig| 12 +++
> drivers/firmware/efi/Makefile | 1 +
> drivers/firmware/efi/fake_mem.c | 204
>
> include/linux/efi.h | 6 ++
> 8 files changed, 236 insertions(+), 2 deletions(-)
> create mode 100644 drivers/firmware/efi/fake_mem.c
[...]
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index e4308fe..eee8068 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -222,7 +222,7 @@ int __init efi_memblock_x86_reserve_range(void)
> return 0;
> }
>
> -static void __init print_efi_memmap(void)
> +void __init print_efi_memmap(void)
> {
> #ifdef EFI_DEBUG
> efi_memory_desc_t *md;
If we're going to make this function global we should stick to the
existing naming convention and rename it efi_print_memmap() in a
separate patch.
> diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
> new file mode 100644
> index 000..2645d4a
> --- /dev/null
> +++ b/drivers/firmware/efi/fake_mem.c
> @@ -0,0 +1,204 @@
> +/*
> + * fake_mem.c
> + *
> + * Copyright (C) 2015 FUJITSU LIMITED
> + * Author: Taku Izumi
> + *
> + * This code introduces new boot option named "efi_fake_mem_mirror"
> + * By specifying this parameter, you can mark specific memory as
> + * mirrored memory by updating original (firmware provided) EFI
> + * memmap.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program; if not, see
[PATCH v2 3/3] x86, efi: Add "efi_fake_mem_mirror" boot option
This patch introduces new boot option named "efi_fake_mem_mirror".
By specifying this parameter, you can mark specific memory as
mirrored memory. This is useful for debugging of Address Range
Mirroring feature.
For example, if you specify "efi_fake_mem_mirror=2G@4G,2G@0x10a000",
the original (firmware provided) EFI memmap will be updated so that
the specified memory regions have EFI_MEMORY_MORE_RELIABLE attribute:
efi: mem00: [Boot Data | | | | | | |WB|WT|WC|UC]
range=[0x-0x1000) (0MB)
efi: mem01: [Loader Data| | | | | | |WB|WT|WC|UC]
range=[0x1000-0x2000) (0MB)
...
efi: mem35: [Boot Data | | | | | | |WB|WT|WC|UC]
range=[0x47ee6000-0x48014000) (1MB)
efi: mem36: [Conventional Memory| | | | | | |WB|WT|WC|UC]
range=[0x0001-0x0020a000) (129536MB)
efi: mem37: [Reserved |RT| | | | | | | | |UC]
range=[0x6000-0x9000) (768MB)
efi: mem00: [Boot Data | | | | | | |WB|WT|WC|UC]
range=[0x-0x1000) (0MB)
efi: mem01: [Loader Data| | | | | | |WB|WT|WC|UC]
range=[0x1000-0x2000) (0MB)
...
efi: mem35: [Boot Data | | | | | | |WB|WT|WC|UC]
range=[0x47ee6000-0x48014000) (1MB)
efi: mem36: [Conventional Memory| |MR| | | | |WB|WT|WC|UC]
range=[0x0001-0x00018000) (2048MB)
efi: mem37: [Conventional Memory| | | | | | |WB|WT|WC|UC]
range=[0x00018000-0x0010a000) (61952MB)
efi: mem38: [Conventional Memory| |MR| | | | |WB|WT|WC|UC]
range=[0x0010a000-0x00112000) (2048MB)
efi: mem39: [Conventional Memory| | | | | | |WB|WT|WC|UC]
range=[0x00112000-0x0020a000) (63488MB)
efi: mem40: [Reserved |RT| | | | | | | | |UC]
range=[0x6000-0x9000) (768MB)
And you will find that the following message is output:
efi: Memory: 4096M/131455M mirrored memory
Signed-off-by: Taku Izumi
---
Documentation/kernel-parameters.txt | 8 ++
arch/x86/include/asm/efi.h | 1 +
arch/x86/kernel/setup.c | 4 +-
arch/x86/platform/efi/efi.c | 2 +-
drivers/firmware/efi/Kconfig| 12 +++
drivers/firmware/efi/Makefile | 1 +
drivers/firmware/efi/fake_mem.c | 204
include/linux/efi.h | 6 ++
8 files changed, 236 insertions(+), 2 deletions(-)
create mode 100644 drivers/firmware/efi/fake_mem.c
diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index 1d6f045..0efded6 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1092,6 +1092,14 @@ bytes respectively. Such letter suffixes can also be
entirely omitted.
you are really sure that your UEFI does sane gc and
fulfills the spec otherwise your board may brick.
+ efi_fake_mem_mirror=nn[KMG]@ss[KMG][,nn[KMG]@ss[KMG],..] [EFI; X86]
+ Mark specific memory as mirrored memory and update
+ EFI memory map.
+ Region of memory to be marked is from ss to ss+nn.
+ Using this parameter you can do debugging of Address
+ Range Mirroring feature even if your box doesn't support
+ it.
+
eisa_irq_edge= [PARISC,HW]
See header of drivers/parisc/eisa.c.
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 155162e..479fd51 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -93,6 +93,7 @@ extern void __init efi_set_executable(efi_memory_desc_t *md,
bool executable);
extern int __init efi_memblock_x86_reserve_range(void);
extern pgd_t * __init efi_call_phys_prolog(void);
extern void __init efi_call_phys_epilog(pgd_t *save_pgd);
+extern void __init print_efi_memmap(void);
extern void __init efi_unmap_memmap(void);
extern void __init efi_memory_uc(u64 addr, unsigned long size);
extern void __init efi_map_region(efi_memory_desc_t *md);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 80f874b..e3ed628 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1104,8 +1104,10 @@ void __init setup_arch(char **cmdline_p)
memblock_set_current_limit(ISA_END_ADDRESS);
memblock_x86_fill();
- if (efi_enabled(EFI_BOOT))
+ if (efi_enabled(EFI_BOOT)) {
+ efi_fake_memmap();
efi_find_mirror();
+ }
/*
* The EFI specification says that boot service code won't be called
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index e4308fe..eee8068 100644
--- a/arch/x86/platform/efi/ef
