diff mbox series

[edk2] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

Message ID 20181127122748.24527-1-ard.biesheuvel@linaro.org
State Superseded
Headers show
Series [edk2] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits | expand

Commit Message

Ard Biesheuvel Nov. 27, 2018, 12:27 p.m. UTC
AArch64 support the use of more than 48 bits for physical and/or
virtual addressing, but only if the page size is set to 64 KB,
which is not supported by UEFI/EDK2. So redefine MAX_ADDRESS to
cover only 48 address bits.

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

---
 MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.19.1

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

Comments

Leif Lindholm Nov. 27, 2018, 1:38 p.m. UTC | #1
On Tue, Nov 27, 2018 at 01:27:48PM +0100, Ard Biesheuvel wrote:
> AArch64 support the use of more than 48 bits for physical and/or


supports

> virtual addressing, but only if the page size is set to 64 KB,

> which is not supported by UEFI/EDK2. So redefine MAX_ADDRESS to


s/EDK2//

With those tweaks:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


> cover only 48 address bits.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

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

> ---

>  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--

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

> 

> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h

> index 968c18f915ae..dad75df1c579 100644

> --- a/MdePkg/Include/AArch64/ProcessorBind.h

> +++ b/MdePkg/Include/AArch64/ProcessorBind.h

> @@ -138,9 +138,9 @@ typedef INT64   INTN;

>  #define MAX_2_BITS  0xC000000000000000ULL

>  

>  ///

> -/// Maximum legal AARCH64  address

> +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)

>  ///

> -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL

> +#define MAX_ADDRESS   0xFFFFFFFFFFFFULL

>  

>  ///

>  /// Maximum legal AArch64 INTN and UINTN values.

> -- 

> 2.19.1

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Nov. 27, 2018, 2:51 p.m. UTC | #2
On 11/27/18 13:27, Ard Biesheuvel wrote:
> AArch64 support the use of more than 48 bits for physical and/or

> virtual addressing, but only if the page size is set to 64 KB,

> which is not supported by UEFI/EDK2. So redefine MAX_ADDRESS to

> cover only 48 address bits.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

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

> ---

>  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--

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

> 

> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h

> index 968c18f915ae..dad75df1c579 100644

> --- a/MdePkg/Include/AArch64/ProcessorBind.h

> +++ b/MdePkg/Include/AArch64/ProcessorBind.h

> @@ -138,9 +138,9 @@ typedef INT64   INTN;

>  #define MAX_2_BITS  0xC000000000000000ULL

>  

>  ///

> -/// Maximum legal AARCH64  address

> +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)

>  ///

> -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL

> +#define MAX_ADDRESS   0xFFFFFFFFFFFFULL

>  

>  ///

>  /// Maximum legal AArch64 INTN and UINTN values.

> 


Hmmmm. I'm worried about this change. I think it could open a can of
worms. I have no clue what *all* the things are that we use MAX_ADDRESS
for. Does it give the maximum value of the canonical address *format*?
Or is it the maximum address that the processor could ever access?

Let's look at the X64 situation... For X64, MAX_ADDRESS is
0xFFFF_FFFF_FFFF_FFFF_ULL (MdePkg/Include/X64/ProcessorBind.h). However,
on X64, even considering the recently introduced 5-level paging
<https://en.wikipedia.org/wiki/Intel_5-level_paging>, the "useful"
number of address bits is up to just 57 -- it used to be 48, with
4-level paging. That is: not 64. Yet we have MAX_UINT64 for MAX_ADDRESS!

Which in turn means that, with X64 5-level paging in mind, the issue
affects X64 as well -- there could be RAM in the system that the 64-bit
DXE phase couldn't access (because edk2 doesn't support 5-level paging,
AIUI), but the OS could.

That officially turns the question into a multi-architectural one: how
should the UEFI memmap describe the highest RAM range, such that it be
exposed to the OS, but not exposed to the firmware itself? (Because, the
firmware doesn't support the necessary paging mode, or processor mode.)

Liming, can you share what Intel plans, in edk2, for supporting 5-level
paging?

And, on such physical X64 systems today that support 57-bit paging, how
does the UEFI memmap describe the [2^48 .. 2^57) RAM?

And how does the firmware allocate and use memory from that area?

Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Gao, Liming Nov. 29, 2018, 2:59 p.m. UTC | #3
Laszlo:
  I add my comments. 

Thanks
Liming
> -----Original Message-----

> From: Laszlo Ersek [mailto:lersek@redhat.com]

> Sent: Tuesday, November 27, 2018 10:52 PM

> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org

> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; leif.lindholm@linaro.org;

> philmd@redhat.com

> Subject: Re: [PATCH] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

> 

> On 11/27/18 13:27, Ard Biesheuvel wrote:

> > AArch64 support the use of more than 48 bits for physical and/or

> > virtual addressing, but only if the page size is set to 64 KB,

> > which is not supported by UEFI/EDK2. So redefine MAX_ADDRESS to

> > cover only 48 address bits.

> >

> > Contributed-under: TianoCore Contribution Agreement 1.1

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

> > ---

> >  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--

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

> >

> > diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h

> > index 968c18f915ae..dad75df1c579 100644

> > --- a/MdePkg/Include/AArch64/ProcessorBind.h

> > +++ b/MdePkg/Include/AArch64/ProcessorBind.h

> > @@ -138,9 +138,9 @@ typedef INT64   INTN;

> >  #define MAX_2_BITS  0xC000000000000000ULL

> >

> >  ///

> > -/// Maximum legal AARCH64  address

> > +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)

> >  ///

> > -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL

> > +#define MAX_ADDRESS   0xFFFFFFFFFFFFULL

> >

> >  ///

> >  /// Maximum legal AArch64 INTN and UINTN values.

> >

> 

> Hmmmm. I'm worried about this change. I think it could open a can of

> worms. I have no clue what *all* the things are that we use MAX_ADDRESS

> for. Does it give the maximum value of the canonical address *format*?

> Or is it the maximum address that the processor could ever access?

> 

> Let's look at the X64 situation... For X64, MAX_ADDRESS is

> 0xFFFF_FFFF_FFFF_FFFF_ULL (MdePkg/Include/X64/ProcessorBind.h). However,

> on X64, even considering the recently introduced 5-level paging

> <https://en.wikipedia.org/wiki/Intel_5-level_paging>, the "useful"

> number of address bits is up to just 57 -- it used to be 48, with

> 4-level paging. That is: not 64. Yet we have MAX_UINT64 for MAX_ADDRESS!

> 

> Which in turn means that, with X64 5-level paging in mind, the issue

> affects X64 as well -- there could be RAM in the system that the 64-bit

> DXE phase couldn't access (because edk2 doesn't support 5-level paging,

> AIUI), but the OS could.

> 

> That officially turns the question into a multi-architectural one: how

> should the UEFI memmap describe the highest RAM range, such that it be

> exposed to the OS, but not exposed to the firmware itself? (Because, the

> firmware doesn't support the necessary paging mode, or processor mode.)

> 

> Liming, can you share what Intel plans, in edk2, for supporting 5-level

> paging?

So far, I have no more to be shared. I don't know whether it is necessary to support 5-level paging with the max memory. 
The firmware can report [2^48 .. 2^57) RAM with the allocated status. So, those region memory can't be allocated in firmware. 
They will be visible to OS. If OS enables 5-level paging, it can access them. 
> 

> And, on such physical X64 systems today that support 57-bit paging, how

> does the UEFI memmap describe the [2^48 .. 2^57) RAM?

> 

> And how does the firmware allocate and use memory from that area?

> 

> Thanks,

> Laszlo

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Nov. 29, 2018, 6:02 p.m. UTC | #4
On 11/29/18 15:59, Gao, Liming wrote:
> Laszlo:

>   I add my comments. 

> 

> Thanks

> Liming

>> -----Original Message-----

>> From: Laszlo Ersek [mailto:lersek@redhat.com]

>> Sent: Tuesday, November 27, 2018 10:52 PM

>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org

>> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; leif.lindholm@linaro.org;

>> philmd@redhat.com

>> Subject: Re: [PATCH] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

>>

>> On 11/27/18 13:27, Ard Biesheuvel wrote:

>>> AArch64 support the use of more than 48 bits for physical and/or

>>> virtual addressing, but only if the page size is set to 64 KB,

>>> which is not supported by UEFI/EDK2. So redefine MAX_ADDRESS to

>>> cover only 48 address bits.

>>>

>>> Contributed-under: TianoCore Contribution Agreement 1.1

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

>>> ---

>>>  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--

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

>>>

>>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h

>>> index 968c18f915ae..dad75df1c579 100644

>>> --- a/MdePkg/Include/AArch64/ProcessorBind.h

>>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h

>>> @@ -138,9 +138,9 @@ typedef INT64   INTN;

>>>  #define MAX_2_BITS  0xC000000000000000ULL

>>>

>>>  ///

>>> -/// Maximum legal AARCH64  address

>>> +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)

>>>  ///

>>> -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL

>>> +#define MAX_ADDRESS   0xFFFFFFFFFFFFULL

>>>

>>>  ///

>>>  /// Maximum legal AArch64 INTN and UINTN values.

>>>

>>

>> Hmmmm. I'm worried about this change. I think it could open a can of

>> worms. I have no clue what *all* the things are that we use MAX_ADDRESS

>> for. Does it give the maximum value of the canonical address *format*?

>> Or is it the maximum address that the processor could ever access?

>>

>> Let's look at the X64 situation... For X64, MAX_ADDRESS is

>> 0xFFFF_FFFF_FFFF_FFFF_ULL (MdePkg/Include/X64/ProcessorBind.h). However,

>> on X64, even considering the recently introduced 5-level paging

>> <https://en.wikipedia.org/wiki/Intel_5-level_paging>, the "useful"

>> number of address bits is up to just 57 -- it used to be 48, with

>> 4-level paging. That is: not 64. Yet we have MAX_UINT64 for MAX_ADDRESS!

>>

>> Which in turn means that, with X64 5-level paging in mind, the issue

>> affects X64 as well -- there could be RAM in the system that the 64-bit

>> DXE phase couldn't access (because edk2 doesn't support 5-level paging,

>> AIUI), but the OS could.

>>

>> That officially turns the question into a multi-architectural one: how

>> should the UEFI memmap describe the highest RAM range, such that it be

>> exposed to the OS, but not exposed to the firmware itself? (Because, the

>> firmware doesn't support the necessary paging mode, or processor mode.)

>>

>> Liming, can you share what Intel plans, in edk2, for supporting 5-level

>> paging?

> So far, I have no more to be shared. I don't know whether it is necessary to support 5-level paging with the max memory. 

> The firmware can report [2^48 .. 2^57) RAM with the allocated status. So, those region memory can't be allocated in firmware. 

> They will be visible to OS. If OS enables 5-level paging, it can access them. 


Great, thank you!
Laszlo

>>

>> And, on such physical X64 systems today that support 57-bit paging, how

>> does the UEFI memmap describe the [2^48 .. 2^57) RAM?

>>

>> And how does the firmware allocate and use memory from that area?

>>

>> Thanks,

>> Laszlo


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

Patch

diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
index 968c18f915ae..dad75df1c579 100644
--- a/MdePkg/Include/AArch64/ProcessorBind.h
+++ b/MdePkg/Include/AArch64/ProcessorBind.h
@@ -138,9 +138,9 @@  typedef INT64   INTN;
 #define MAX_2_BITS  0xC000000000000000ULL
 
 ///
-/// Maximum legal AARCH64  address
+/// Maximum legal AARCH64  address (48 bits for 4 KB page size)
 ///
-#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL
+#define MAX_ADDRESS   0xFFFFFFFFFFFFULL
 
 ///
 /// Maximum legal AArch64 INTN and UINTN values.