Message ID | 20240308101230.2595220-1-patrice.chotard@foss.st.com |
---|---|
State | Superseded |
Delegated to: | Patrick Delaunay |
Headers | show |
Series | stm32mp: Reserve OPTEE area in EFI memory map | expand |
Hi, On 3/8/24 11:12, Patrice Chotard wrote: > Since commit 7b78d6438a2b3 ("efi_loader: Reserve unaccessible memory") > memory region above ram_top is tagged in EFI memory map as > EFI_BOOT_SERVICES_DATA. > In case of STM32MP1 platform, above ram_top, there is one reserved-memory > region tagged "no-map" dedicated to OP-TEE (addr=de000000 size=2000000). > > Before booting kernel, EFI memory map is first built, the OPTEE region is > tagged as EFI_BOOT_SERVICES_DATA and when reserving LMB, the tag LMB_NONE > is used. > > Then after, the LMB are completed by boot_fdt_add_mem_rsv_regions() > which try to add again the same OPTEE region (addr=de000000 size=2000000) > but now with LMB_NOMAP tag which produces the following error message : > > "ERROR: reserving fdt memory region failed (addr=de000000 size=2000000 flags=4)" > > To avoid this, OPTEE area shouldn't be used by EFI, so we need to mark > it as reserved. > > Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> > --- > > arch/arm/mach-stm32mp/dram_init.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm/mach-stm32mp/dram_init.c b/arch/arm/mach-stm32mp/dram_init.c > index fb1208fc5d5..f67f54f2ae0 100644 > --- a/arch/arm/mach-stm32mp/dram_init.c > +++ b/arch/arm/mach-stm32mp/dram_init.c > @@ -7,6 +7,7 @@ > > #include <common.h> > #include <dm.h> > +#include <efi_loader.h> > #include <image.h> > #include <init.h> > #include <lmb.h> > @@ -75,3 +76,14 @@ phys_addr_t board_get_usable_ram_top(phys_size_t total_size) > > return reg + size; > } > + > +void efi_add_known_memory(void) > +{ > + if (IS_ENABLED(CONFIG_EFI_LOADER)) > + /* > + * Memory over ram_top is reserved to OPTEE. > + * Declare to EFI only memory area below ram_top > + */ > + efi_add_memory_map(gd->ram_base, gd->ram_top - gd->ram_base, > + EFI_CONVENTIONAL_MEMORY); > +} Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com> Thanks Patrick
On 17.04.24 09:25, Patrick DELAUNAY wrote: > Hi, > > On 3/8/24 11:12, Patrice Chotard wrote: >> Since commit 7b78d6438a2b3 ("efi_loader: Reserve unaccessible memory") >> memory region above ram_top is tagged in EFI memory map as >> EFI_BOOT_SERVICES_DATA. >> In case of STM32MP1 platform, above ram_top, there is one reserved-memory >> region tagged "no-map" dedicated to OP-TEE (addr=de000000 size=2000000). >> >> Before booting kernel, EFI memory map is first built, the OPTEE region is >> tagged as EFI_BOOT_SERVICES_DATA and when reserving LMB, the tag LMB_NONE >> is used. >> >> Then after, the LMB are completed by boot_fdt_add_mem_rsv_regions() >> which try to add again the same OPTEE region (addr=de000000 size=2000000) >> but now with LMB_NOMAP tag which produces the following error message : >> >> "ERROR: reserving fdt memory region failed (addr=de000000 size=2000000 >> flags=4)" >> >> To avoid this, OPTEE area shouldn't be used by EFI, so we need to mark >> it as reserved. >> >> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> >> --- >> >> arch/arm/mach-stm32mp/dram_init.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/arch/arm/mach-stm32mp/dram_init.c >> b/arch/arm/mach-stm32mp/dram_init.c >> index fb1208fc5d5..f67f54f2ae0 100644 >> --- a/arch/arm/mach-stm32mp/dram_init.c >> +++ b/arch/arm/mach-stm32mp/dram_init.c >> @@ -7,6 +7,7 @@ >> #include <common.h> >> #include <dm.h> >> +#include <efi_loader.h> >> #include <image.h> >> #include <init.h> >> #include <lmb.h> >> @@ -75,3 +76,14 @@ phys_addr_t board_get_usable_ram_top(phys_size_t >> total_size) >> return reg + size; >> } >> + >> +void efi_add_known_memory(void) >> +{ >> + if (IS_ENABLED(CONFIG_EFI_LOADER)) >> + /* >> + * Memory over ram_top is reserved to OPTEE. >> + * Declare to EFI only memory area below ram_top >> + */ >> + efi_add_memory_map(gd->ram_base, gd->ram_top - gd->ram_base, >> + EFI_CONVENTIONAL_MEMORY); With this change the EFI memory map passed to the operating system will not contain any memory above gd->ram_top. Is this really your intent? Don't you have any memory above 0xe0000000? Best regards Heinrich >> +} > > > > Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com> > > Thanks > Patrick >
On 4/17/24 09:45, Heinrich Schuchardt wrote: > On 17.04.24 09:25, Patrick DELAUNAY wrote: >> Hi, >> >> On 3/8/24 11:12, Patrice Chotard wrote: >>> Since commit 7b78d6438a2b3 ("efi_loader: Reserve unaccessible memory") >>> memory region above ram_top is tagged in EFI memory map as >>> EFI_BOOT_SERVICES_DATA. >>> In case of STM32MP1 platform, above ram_top, there is one reserved-memory >>> region tagged "no-map" dedicated to OP-TEE (addr=de000000 size=2000000). >>> >>> Before booting kernel, EFI memory map is first built, the OPTEE region is >>> tagged as EFI_BOOT_SERVICES_DATA and when reserving LMB, the tag LMB_NONE >>> is used. >>> >>> Then after, the LMB are completed by boot_fdt_add_mem_rsv_regions() >>> which try to add again the same OPTEE region (addr=de000000 size=2000000) >>> but now with LMB_NOMAP tag which produces the following error message : >>> >>> "ERROR: reserving fdt memory region failed (addr=de000000 size=2000000 >>> flags=4)" >>> >>> To avoid this, OPTEE area shouldn't be used by EFI, so we need to mark >>> it as reserved. >>> >>> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> >>> --- >>> >>> arch/arm/mach-stm32mp/dram_init.c | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/arch/arm/mach-stm32mp/dram_init.c >>> b/arch/arm/mach-stm32mp/dram_init.c >>> index fb1208fc5d5..f67f54f2ae0 100644 >>> --- a/arch/arm/mach-stm32mp/dram_init.c >>> +++ b/arch/arm/mach-stm32mp/dram_init.c >>> @@ -7,6 +7,7 @@ >>> #include <common.h> >>> #include <dm.h> >>> +#include <efi_loader.h> >>> #include <image.h> >>> #include <init.h> >>> #include <lmb.h> >>> @@ -75,3 +76,14 @@ phys_addr_t board_get_usable_ram_top(phys_size_t >>> total_size) >>> return reg + size; >>> } >>> + >>> +void efi_add_known_memory(void) >>> +{ >>> + if (IS_ENABLED(CONFIG_EFI_LOADER)) >>> + /* >>> + * Memory over ram_top is reserved to OPTEE. >>> + * Declare to EFI only memory area below ram_top >>> + */ >>> + efi_add_memory_map(gd->ram_base, gd->ram_top - gd->ram_base, >>> + EFI_CONVENTIONAL_MEMORY); > > With this change the EFI memory map passed to the operating system will > not contain any memory above gd->ram_top. Is this really your intent? > Don't you have any memory above 0xe0000000? Hi Heinrich On stm32mp157x-dk and stm32mp135x-dk boards, there is no memory above 0xe0000000. But, on stm32mp157x-ed1 or stm32mp157x-ev1, you are right, we got memory above 0xe0000000. I will rework this patch to take into account memory that could be present above OPTEE area. Thanks for pointing this Patrice > > Best regards > > Heinrich > >>> +} >> >> >> >> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com> >> >> Thanks >> Patrick >> >
On 4/19/24 09:44, Patrice CHOTARD wrote: > > > On 4/17/24 09:45, Heinrich Schuchardt wrote: >> On 17.04.24 09:25, Patrick DELAUNAY wrote: >>> Hi, >>> >>> On 3/8/24 11:12, Patrice Chotard wrote: >>>> Since commit 7b78d6438a2b3 ("efi_loader: Reserve unaccessible memory") >>>> memory region above ram_top is tagged in EFI memory map as >>>> EFI_BOOT_SERVICES_DATA. >>>> In case of STM32MP1 platform, above ram_top, there is one reserved-memory >>>> region tagged "no-map" dedicated to OP-TEE (addr=de000000 size=2000000). >>>> >>>> Before booting kernel, EFI memory map is first built, the OPTEE region is >>>> tagged as EFI_BOOT_SERVICES_DATA and when reserving LMB, the tag LMB_NONE >>>> is used. >>>> >>>> Then after, the LMB are completed by boot_fdt_add_mem_rsv_regions() >>>> which try to add again the same OPTEE region (addr=de000000 size=2000000) >>>> but now with LMB_NOMAP tag which produces the following error message : >>>> >>>> "ERROR: reserving fdt memory region failed (addr=de000000 size=2000000 >>>> flags=4)" >>>> >>>> To avoid this, OPTEE area shouldn't be used by EFI, so we need to mark >>>> it as reserved. >>>> >>>> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> >>>> --- >>>> >>>> arch/arm/mach-stm32mp/dram_init.c | 12 ++++++++++++ >>>> 1 file changed, 12 insertions(+) >>>> >>>> diff --git a/arch/arm/mach-stm32mp/dram_init.c >>>> b/arch/arm/mach-stm32mp/dram_init.c >>>> index fb1208fc5d5..f67f54f2ae0 100644 >>>> --- a/arch/arm/mach-stm32mp/dram_init.c >>>> +++ b/arch/arm/mach-stm32mp/dram_init.c >>>> @@ -7,6 +7,7 @@ >>>> #include <common.h> >>>> #include <dm.h> >>>> +#include <efi_loader.h> >>>> #include <image.h> >>>> #include <init.h> >>>> #include <lmb.h> >>>> @@ -75,3 +76,14 @@ phys_addr_t board_get_usable_ram_top(phys_size_t >>>> total_size) >>>> return reg + size; >>>> } >>>> + >>>> +void efi_add_known_memory(void) >>>> +{ >>>> + if (IS_ENABLED(CONFIG_EFI_LOADER)) >>>> + /* >>>> + * Memory over ram_top is reserved to OPTEE. >>>> + * Declare to EFI only memory area below ram_top >>>> + */ >>>> + efi_add_memory_map(gd->ram_base, gd->ram_top - gd->ram_base, >>>> + EFI_CONVENTIONAL_MEMORY); >> >> With this change the EFI memory map passed to the operating system will >> not contain any memory above gd->ram_top. Is this really your intent? >> Don't you have any memory above 0xe0000000? > > Hi Heinrich > > On stm32mp157x-dk and stm32mp135x-dk boards, there is no memory above 0xe0000000. > But, on stm32mp157x-ed1 or stm32mp157x-ev1, you are right, we got memory above 0xe0000000. > > I will rework this patch to take into account memory that could be present above OPTEE area. After double checking, this patch is correct but i need to update the commit message. In fact for stm32mp157x-dk and stm32mp135x-dk boards, there is 512MB of memory (0xc0000000 0x20000000), OPTEE is located at the end of memory : 0xde000000 0x02000000 But for stm32mp157c-ev1, there is 1GB of memory (0xC0000000 0x40000000), and identically, OPTEE is located at the end of memory : 0xfe000000 0x2000000 So in both cases, above OPTEE reserved-memory area, there is no more free memory. Thanks Patrice > > Thanks for pointing this > Patrice > >> >> Best regards >> >> Heinrich >> >>>> +} >>> >>> >>> >>> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com> >>> >>> Thanks >>> Patrick >>> >> > _______________________________________________ > Uboot-stm32 mailing list > Uboot-stm32@st-md-mailman.stormreply.com > https://st-md-mailman.stormreply.com/mailman/listinfo/uboot-stm32
diff --git a/arch/arm/mach-stm32mp/dram_init.c b/arch/arm/mach-stm32mp/dram_init.c index fb1208fc5d5..f67f54f2ae0 100644 --- a/arch/arm/mach-stm32mp/dram_init.c +++ b/arch/arm/mach-stm32mp/dram_init.c @@ -7,6 +7,7 @@ #include <common.h> #include <dm.h> +#include <efi_loader.h> #include <image.h> #include <init.h> #include <lmb.h> @@ -75,3 +76,14 @@ phys_addr_t board_get_usable_ram_top(phys_size_t total_size) return reg + size; } + +void efi_add_known_memory(void) +{ + if (IS_ENABLED(CONFIG_EFI_LOADER)) + /* + * Memory over ram_top is reserved to OPTEE. + * Declare to EFI only memory area below ram_top + */ + efi_add_memory_map(gd->ram_base, gd->ram_top - gd->ram_base, + EFI_CONVENTIONAL_MEMORY); +}
Since commit 7b78d6438a2b3 ("efi_loader: Reserve unaccessible memory") memory region above ram_top is tagged in EFI memory map as EFI_BOOT_SERVICES_DATA. In case of STM32MP1 platform, above ram_top, there is one reserved-memory region tagged "no-map" dedicated to OP-TEE (addr=de000000 size=2000000). Before booting kernel, EFI memory map is first built, the OPTEE region is tagged as EFI_BOOT_SERVICES_DATA and when reserving LMB, the tag LMB_NONE is used. Then after, the LMB are completed by boot_fdt_add_mem_rsv_regions() which try to add again the same OPTEE region (addr=de000000 size=2000000) but now with LMB_NOMAP tag which produces the following error message : "ERROR: reserving fdt memory region failed (addr=de000000 size=2000000 flags=4)" To avoid this, OPTEE area shouldn't be used by EFI, so we need to mark it as reserved. Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com> --- arch/arm/mach-stm32mp/dram_init.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)