diff mbox series

[edk2,1/5] ArmPkg/ArmLib: add support for reading the max physical address space size

Message ID 20181123121431.22353-2-ard.biesheuvel@linaro.org
State New
Headers show
Series ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit | expand

Commit Message

Ard Biesheuvel Nov. 23, 2018, 12:14 p.m. UTC 
Add a helper function that returns the maximum physical address space
size as supported by the current CPU.

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

---
 ArmPkg/Include/Library/ArmLib.h               |  6 ++++++
 ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 16 ++++++++++++++++
 ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S     |  8 ++++++++
 3 files changed, 30 insertions(+)

-- 
2.17.1

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

Comments

Ard Biesheuvel Nov. 23, 2018, 1:20 p.m. UTC | #1 
On Fri, 23 Nov 2018 at 14:16, Andrew Jones <drjones@redhat.com> wrote:
>

> On Fri, Nov 23, 2018 at 01:14:27PM +0100, Ard Biesheuvel wrote:

> > Add a helper function that returns the maximum physical address space

> > size as supported by the current CPU.

> >

> > Contributed-under: TianoCore Contribution Agreement 1.1

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

> > ---

> >  ArmPkg/Include/Library/ArmLib.h               |  6 ++++++

> >  ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 16 ++++++++++++++++

> >  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S     |  8 ++++++++

> >  3 files changed, 30 insertions(+)

> >

> > diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h

> > index ffda50e9d767..9a804c15fdb6 100644

> > --- a/ArmPkg/Include/Library/ArmLib.h

> > +++ b/ArmPkg/Include/Library/ArmLib.h

> > @@ -733,4 +733,10 @@ ArmWriteCntvOff (

> >    UINT64   Val

> >    );

> >

> > +UINTN

> > +EFIAPI

> > +ArmGetPhysicalAddressBits (

> > +  VOID

> > +  );

> > +

> >  #endif // __ARM_LIB__

> > diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S

> > index 1ef2f61f5979..75ab8dade485 100644

> > --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S

> > +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S

> > @@ -196,4 +196,20 @@ ASM_FUNC(ArmWriteSctlr)

> >  3:msr   sctlr_el3, x0

> >  4:ret

> >

> > +ASM_FUNC(ArmGetPhysicalAddressBits)

> > +  mrs   x0, id_aa64mmfr0_el1

> > +  adr   x1, .LPARanges

> > +  and   x0, x0, #7

> > +  ldrb  w0, [x1, x0]

> > +  ret

> > +

> > +//

> > +// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the

> > +// physical address space support on this CPU:

> > +// 0 == 32 bits, 1 == 36 bits, etc etc

> > +// 6 and 7 are reserved

> > +//

> > +.LPARanges:

> > +  .byte 32, 36, 40, 42, 44, 48, -1, -1

>

> Hi Ard,

>

> One of the things I was wondering is how much it matters what the

> firmware's opinion of highest physical address is vs. the guest

> kernel. Do they need to match? This patch series implies they do,

> or at least that 40-bits won't always be sufficient for firmware.


Yes. The size of the GCD space limits how much memory we can report as
present to the OS. So it only matters if there is DRAM there.

> However, guests using 64k pages running on supporting hardware can

> use 52-bits. Considering ArmVirtPkg only uses 4k pages, that's not

> an option for it, and that justifies not defining index 6 == 52 in

> the above array, but will that also restrict the guest?

>


At the moment, yes. UEFI support for 52-bit/64k pages is still under
discussion, and is presently not supported.


> > diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S

> > index f2a517671f0a..f2f3c9a25991 100644

> > --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S

> > +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S

> > @@ -165,4 +165,12 @@ ASM_FUNC(ArmWriteCpuActlr)

> >    isb

> >    bx      lr

> >

> > +ASM_FUNC (ArmGetPhysicalAddressBits)

> > +  mrc     p15, 0, r0, c0, c1, 4   // MMFR0

> > +  and     r0, r0, #0xf            // VMSA [3:0]

> > +  cmp     r0, #5                  // >5 implies LPAE support

> > +  movlt   r0, #32                 // 32 bits if no LPAE

> > +  movge   r0, #40                 // 40 bits if LPAE

> > +  bx      lr

> > +

> >  ASM_FUNCTION_REMOVE_IF_UNREFERENCED

> > --

> > 2.17.1

> >

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Nov. 23, 2018, 1:23 p.m. UTC | #2 
On Fri, 23 Nov 2018 at 14:20, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>

> On Fri, 23 Nov 2018 at 14:16, Andrew Jones <drjones@redhat.com> wrote:

> >

> > On Fri, Nov 23, 2018 at 01:14:27PM +0100, Ard Biesheuvel wrote:

> > > Add a helper function that returns the maximum physical address space

> > > size as supported by the current CPU.

> > >

> > > Contributed-under: TianoCore Contribution Agreement 1.1

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

> > > ---

> > >  ArmPkg/Include/Library/ArmLib.h               |  6 ++++++

> > >  ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 16 ++++++++++++++++

> > >  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S     |  8 ++++++++

> > >  3 files changed, 30 insertions(+)

> > >

> > > diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h

> > > index ffda50e9d767..9a804c15fdb6 100644

> > > --- a/ArmPkg/Include/Library/ArmLib.h

> > > +++ b/ArmPkg/Include/Library/ArmLib.h

> > > @@ -733,4 +733,10 @@ ArmWriteCntvOff (

> > >    UINT64   Val

> > >    );

> > >

> > > +UINTN

> > > +EFIAPI

> > > +ArmGetPhysicalAddressBits (

> > > +  VOID

> > > +  );

> > > +

> > >  #endif // __ARM_LIB__

> > > diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S

> > > index 1ef2f61f5979..75ab8dade485 100644

> > > --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S

> > > +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S

> > > @@ -196,4 +196,20 @@ ASM_FUNC(ArmWriteSctlr)

> > >  3:msr   sctlr_el3, x0

> > >  4:ret

> > >

> > > +ASM_FUNC(ArmGetPhysicalAddressBits)

> > > +  mrs   x0, id_aa64mmfr0_el1

> > > +  adr   x1, .LPARanges

> > > +  and   x0, x0, #7

> > > +  ldrb  w0, [x1, x0]

> > > +  ret

> > > +

> > > +//

> > > +// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the

> > > +// physical address space support on this CPU:

> > > +// 0 == 32 bits, 1 == 36 bits, etc etc

> > > +// 6 and 7 are reserved

> > > +//

> > > +.LPARanges:

> > > +  .byte 32, 36, 40, 42, 44, 48, -1, -1

> >

> > Hi Ard,

> >

> > One of the things I was wondering is how much it matters what the

> > firmware's opinion of highest physical address is vs. the guest

> > kernel. Do they need to match? This patch series implies they do,

> > or at least that 40-bits won't always be sufficient for firmware.

>

> Yes. The size of the GCD space limits how much memory we can report as

> present to the OS. So it only matters if there is DRAM there.

>

> > However, guests using 64k pages running on supporting hardware can

> > use 52-bits. Considering ArmVirtPkg only uses 4k pages, that's not

> > an option for it, and that justifies not defining index 6 == 52 in

> > the above array, but will that also restrict the guest?

> >

>

> At the moment, yes. UEFI support for 52-bit/64k pages is still under

> discussion, and is presently not supported.

>


... which btw doesn't mean we can't report that much memory in the GCD
memory map, we just can't map it in UEFI.

There were some discussions about how to proceed here, since there
appear to be some SoC vendors that want to use bit 51 to distinguish
between DRAM and MMIO regions, which implies that we have to support
it to be able to boot such systems (and the architecture does not
forbid or discourage the practice)
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Nov. 25, 2018, 5:21 p.m. UTC | #3 
meta:

On 11/23/18 14:20, Ard Biesheuvel wrote:
> On Fri, 23 Nov 2018 at 14:16, Andrew Jones <drjones@redhat.com> wrote:

>>

>> [...]


Drew, Eric, please subscribe to edk2-devel at

  https://lists.01.org/mailman/listinfo/edk2-devel

otherwise, the list will drop your messages.

This is a completely unacceptable requirement for posting; I apologize
for it. For a while it looked like we had replaced it with manual
white-listing / moderation
<https://bugzilla.tianocore.org/show_bug.cgi?id=451>, but then the
policy was reverted to the original (broken) one, as a (misguided)
response to a spam attack. (And to how Red Hat's mail infrastructure
reacted, when those spam moderation requests were delivered to my mailbox.)

So, for now, please subscribe. If you don't intend to receive edk2-devel
messages, you can still stay subscribed: after your subscription
completes, go to the same URL as above, click "Unsubscribe or edit
options", and then disable mail delivery.

Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Nov. 26, 2018, 11:46 a.m. UTC | #4 
On Fri, 23 Nov 2018 at 13:14, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>

> Add a helper function that returns the maximum physical address space

> size as supported by the current CPU.

>

> Contributed-under: TianoCore Contribution Agreement 1.1

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

> ---

>  ArmPkg/Include/Library/ArmLib.h               |  6 ++++++

>  ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 16 ++++++++++++++++

>  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S     |  8 ++++++++

>  3 files changed, 30 insertions(+)

>

> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h

> index ffda50e9d767..9a804c15fdb6 100644

> --- a/ArmPkg/Include/Library/ArmLib.h

> +++ b/ArmPkg/Include/Library/ArmLib.h

> @@ -733,4 +733,10 @@ ArmWriteCntvOff (

>    UINT64   Val

>    );

>

> +UINTN

> +EFIAPI

> +ArmGetPhysicalAddressBits (

> +  VOID

> +  );

> +

>  #endif // __ARM_LIB__

> diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S

> index 1ef2f61f5979..75ab8dade485 100644

> --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S

> +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S

> @@ -196,4 +196,20 @@ ASM_FUNC(ArmWriteSctlr)

>  3:msr   sctlr_el3, x0

>  4:ret

>

> +ASM_FUNC(ArmGetPhysicalAddressBits)

> +  mrs   x0, id_aa64mmfr0_el1

> +  adr   x1, .LPARanges

> +  and   x0, x0, #7

> +  ldrb  w0, [x1, x0]

> +  ret

> +

> +//

> +// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the

> +// physical address space support on this CPU:

> +// 0 == 32 bits, 1 == 36 bits, etc etc

> +// 6 and 7 are reserved

> +//

> +.LPARanges:

> +  .byte 32, 36, 40, 42, 44, 48, -1, -1

> +


Note: as Drew pointed out, we want 52 bits included as well in this
enumeration. I will fix that up when applying (unless anyone objects)

>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED

> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S

> index f2a517671f0a..f2f3c9a25991 100644

> --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S

> +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S

> @@ -165,4 +165,12 @@ ASM_FUNC(ArmWriteCpuActlr)

>    isb

>    bx      lr

>

> +ASM_FUNC (ArmGetPhysicalAddressBits)

> +  mrc     p15, 0, r0, c0, c1, 4   // MMFR0

> +  and     r0, r0, #0xf            // VMSA [3:0]

> +  cmp     r0, #5                  // >5 implies LPAE support

> +  movlt   r0, #32                 // 32 bits if no LPAE

> +  movge   r0, #40                 // 40 bits if LPAE

> +  bx      lr

> +

>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED

> --

> 2.17.1

>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Philippe Mathieu-Daudé Nov. 26, 2018, 6:17 p.m. UTC | #5 
On 26/11/18 12:46, Ard Biesheuvel wrote:
> On Fri, 23 Nov 2018 at 13:14, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>
>> Add a helper function that returns the maximum physical address space
>> size as supported by the current CPU.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmPkg/Include/Library/ArmLib.h               |  6 ++++++
>>  ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 16 ++++++++++++++++
>>  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S     |  8 ++++++++
>>  3 files changed, 30 insertions(+)
>>
>> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
>> index ffda50e9d767..9a804c15fdb6 100644
>> --- a/ArmPkg/Include/Library/ArmLib.h
>> +++ b/ArmPkg/Include/Library/ArmLib.h
>> @@ -733,4 +733,10 @@ ArmWriteCntvOff (
>>    UINT64   Val
>>    );
>>
>> +UINTN
>> +EFIAPI
>> +ArmGetPhysicalAddressBits (
>> +  VOID
>> +  );
>> +
>>  #endif // __ARM_LIB__
>> diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
>> index 1ef2f61f5979..75ab8dade485 100644
>> --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
>> +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
>> @@ -196,4 +196,20 @@ ASM_FUNC(ArmWriteSctlr)
>>  3:msr   sctlr_el3, x0
>>  4:ret
>>
>> +ASM_FUNC(ArmGetPhysicalAddressBits)
>> +  mrs   x0, id_aa64mmfr0_el1
>> +  adr   x1, .LPARanges
>> +  and   x0, x0, #7
>> +  ldrb  w0, [x1, x0]
>> +  ret
>> +
>> +//
>> +// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
>> +// physical address space support on this CPU:
>> +// 0 == 32 bits, 1 == 36 bits, etc etc
>> +// 6 and 7 are reserved
>> +//
>> +.LPARanges:
>> +  .byte 32, 36, 40, 42, 44, 48, -1, -1
>> +
> 
> Note: as Drew pointed out, we want 52 bits included as well in this
> enumeration. I will fix that up when applying (unless anyone objects)

Using index 6 == 52 and updating the comment:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
>>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
>> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
>> index f2a517671f0a..f2f3c9a25991 100644
>> --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
>> +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
>> @@ -165,4 +165,12 @@ ASM_FUNC(ArmWriteCpuActlr)
>>    isb
>>    bx      lr
>>
>> +ASM_FUNC (ArmGetPhysicalAddressBits)
>> +  mrc     p15, 0, r0, c0, c1, 4   // MMFR0
>> +  and     r0, r0, #0xf            // VMSA [3:0]
>> +  cmp     r0, #5                  // >5 implies LPAE support
>> +  movlt   r0, #32                 // 32 bits if no LPAE
>> +  movge   r0, #40                 // 40 bits if LPAE
>> +  bx      lr
>> +
>>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
>> --
>> 2.17.1
>>
diff mbox series

Patch

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index ffda50e9d767..9a804c15fdb6 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -733,4 +733,10 @@  ArmWriteCntvOff (
   UINT64   Val
   );
 
+UINTN
+EFIAPI
+ArmGetPhysicalAddressBits (
+  VOID
+  );
+
 #endif // __ARM_LIB__
diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
index 1ef2f61f5979..75ab8dade485 100644
--- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
@@ -196,4 +196,20 @@  ASM_FUNC(ArmWriteSctlr)
 3:msr   sctlr_el3, x0
 4:ret
 
+ASM_FUNC(ArmGetPhysicalAddressBits)
+  mrs   x0, id_aa64mmfr0_el1
+  adr   x1, .LPARanges
+  and   x0, x0, #7
+  ldrb  w0, [x1, x0]
+  ret
+
+//
+// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
+// physical address space support on this CPU:
+// 0 == 32 bits, 1 == 36 bits, etc etc
+// 6 and 7 are reserved
+//
+.LPARanges:
+  .byte 32, 36, 40, 42, 44, 48, -1, -1
+
 ASM_FUNCTION_REMOVE_IF_UNREFERENCED
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
index f2a517671f0a..f2f3c9a25991 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
@@ -165,4 +165,12 @@  ASM_FUNC(ArmWriteCpuActlr)
   isb
   bx      lr
 
+ASM_FUNC (ArmGetPhysicalAddressBits)
+  mrc     p15, 0, r0, c0, c1, 4   // MMFR0
+  and     r0, r0, #0xf            // VMSA [3:0]
+  cmp     r0, #5                  // >5 implies LPAE support
+  movlt   r0, #32                 // 32 bits if no LPAE
+  movge   r0, #40                 // 40 bits if LPAE
+  bx      lr
+
 ASM_FUNCTION_REMOVE_IF_UNREFERENCED