[edk2,RFC,5/7] ArmPlatformPkg/MemoryInitPeim: take MAX_ALLOC_ADDRESS into account

Message ID 20181207112304.19765-6-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • introduce MAX_ALLOC_ADDRESS to limit boot time allocations
Related show

Commit Message

Ard Biesheuvel Dec. 7, 2018, 11:23 a.m.
Limit the PEI memory region so it will not extend beyond what we can
address architecturally when running with 4 KB pages.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.19.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek Dec. 7, 2018, 12:46 p.m. | #1
On 12/07/18 12:23, Ard Biesheuvel wrote:
> Limit the PEI memory region so it will not extend beyond what we can

> address architecturally when running with 4 KB pages.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c

> index 389a2e6f1abd..25db87fb2374 100644

> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c

> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c

> @@ -109,8 +109,8 @@ InitializeMemory (

>  

>    SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);

>    SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);

> -  if (SystemMemoryTop - 1 > MAX_ADDRESS) {

> -    SystemMemoryTop = (UINT64)MAX_ADDRESS + 1;

> +  if (SystemMemoryTop - 1 > MAX_ALLOC_ADDRESS) {

> +    SystemMemoryTop = (UINT64)MAX_ALLOC_ADDRESS + 1;

>    }

>    FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);

>    FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);

> 


Shouldn't you also update

  ASSERT (PcdGet64 (PcdSystemMemoryBase) < (UINT64)MAX_ADDRESS);

just above the context visible here?

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Dec. 7, 2018, 12:47 p.m. | #2
On Fri, 7 Dec 2018 at 13:46, Laszlo Ersek <lersek@redhat.com> wrote:
>

> On 12/07/18 12:23, Ard Biesheuvel wrote:

> > Limit the PEI memory region so it will not extend beyond what we can

> > address architecturally when running with 4 KB pages.

> >

> > Contributed-under: TianoCore Contribution Agreement 1.1

> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> > ---

> >  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 4 ++--

> >  1 file changed, 2 insertions(+), 2 deletions(-)

> >

> > diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c

> > index 389a2e6f1abd..25db87fb2374 100644

> > --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c

> > +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c

> > @@ -109,8 +109,8 @@ InitializeMemory (

> >

> >    SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);

> >    SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);

> > -  if (SystemMemoryTop - 1 > MAX_ADDRESS) {

> > -    SystemMemoryTop = (UINT64)MAX_ADDRESS + 1;

> > +  if (SystemMemoryTop - 1 > MAX_ALLOC_ADDRESS) {

> > +    SystemMemoryTop = (UINT64)MAX_ALLOC_ADDRESS + 1;

> >    }

> >    FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);

> >    FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);

> >

>

> Shouldn't you also update

>

>   ASSERT (PcdGet64 (PcdSystemMemoryBase) < (UINT64)MAX_ADDRESS);

>

> just above the context visible here?

>


No, it's fine for PcdSystemMemoryBase/PcdSystemMemorySize to describe
a region we cannot access in its entirety, as long as we don't use the
top part (See the next patch as well)
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Dec. 7, 2018, 12:48 p.m. | #3
On Fri, 7 Dec 2018 at 13:47, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>

> On Fri, 7 Dec 2018 at 13:46, Laszlo Ersek <lersek@redhat.com> wrote:

> >

> > On 12/07/18 12:23, Ard Biesheuvel wrote:

> > > Limit the PEI memory region so it will not extend beyond what we can

> > > address architecturally when running with 4 KB pages.

> > >

> > > Contributed-under: TianoCore Contribution Agreement 1.1

> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> > > ---

> > >  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c | 4 ++--

> > >  1 file changed, 2 insertions(+), 2 deletions(-)

> > >

> > > diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c

> > > index 389a2e6f1abd..25db87fb2374 100644

> > > --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c

> > > +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c

> > > @@ -109,8 +109,8 @@ InitializeMemory (

> > >

> > >    SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);

> > >    SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);

> > > -  if (SystemMemoryTop - 1 > MAX_ADDRESS) {

> > > -    SystemMemoryTop = (UINT64)MAX_ADDRESS + 1;

> > > +  if (SystemMemoryTop - 1 > MAX_ALLOC_ADDRESS) {

> > > +    SystemMemoryTop = (UINT64)MAX_ALLOC_ADDRESS + 1;

> > >    }

> > >    FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);

> > >    FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);

> > >

> >

> > Shouldn't you also update

> >

> >   ASSERT (PcdGet64 (PcdSystemMemoryBase) < (UINT64)MAX_ADDRESS);

> >

> > just above the context visible here?

> >

>

> No, it's fine for PcdSystemMemoryBase/PcdSystemMemorySize to describe

> a region we cannot access in its entirety, as long as we don't use the

> top part (See the next patch as well)


Whoops, no you are right. That is Base not Base+Size, so indeed, Base
should be below MAX_ALLOC_ADDRESS
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patch

diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
index 389a2e6f1abd..25db87fb2374 100644
--- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
+++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c
@@ -109,8 +109,8 @@  InitializeMemory (
 
   SystemMemoryBase = (UINTN)PcdGet64 (PcdSystemMemoryBase);
   SystemMemoryTop = SystemMemoryBase + PcdGet64 (PcdSystemMemorySize);
-  if (SystemMemoryTop - 1 > MAX_ADDRESS) {
-    SystemMemoryTop = (UINT64)MAX_ADDRESS + 1;
+  if (SystemMemoryTop - 1 > MAX_ALLOC_ADDRESS) {
+    SystemMemoryTop = (UINT64)MAX_ALLOC_ADDRESS + 1;
   }
   FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
   FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);