[PATCH] ArmPkg: ARM v8.2 updates for detecting FP

[PATCH] ArmPkg: ARM v8.2 updates for detecting FP

[evan.lloyd]

From: Sami Mujawar <samimujawar>

The ARMv8.2-FP16 extension introduces support for half precision
floating point and the processor ID registers have been updated to
enable detection of the implementation.

The possible values for the FP bits in ID_AA64PFR0_EL1[19:16] are:
  - 0000 : Floating-point is implemented.
  - 0001 : Floating-point including Half-precision support is
           implemented.
  - 1111 : Floating-point is not implemented.
  - All other values are reserved.

Previously ArmEnableVFP() compared the FP bits with 0000b to see if
the FP was implemented, before enabling FP. Modified this check to
enable the FP if the FP bits 19:16 are not 1111b.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
---
 ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
index dde6a756528f3abf1bd5a142448e42122a9bd8fa..2d136d242b943fe2f365bc824953b7fe10c944b7 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
@@ -1,7 +1,7 @@
 #------------------------------------------------------------------------------
 #
 # Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
-# Copyright (c) 2011 - 2014, ARM Limited. All rights reserved.
+# Copyright (c) 2011 - 2017, ARM Limited. All rights reserved.
 # Copyright (c) 2016, Linaro Limited. All rights reserved.
 #
 # This program and the accompanying materials
@@ -403,9 +403,11 @@ ASM_FUNC(ArmEnableVFP)
   mov   x1, x30                 // Save LR
   bl    ArmReadIdPfr0           // Read EL1 Processor Feature Register (PFR0)
   mov   x30, x1                 // Restore LR
-  ands  x0, x0, #AARCH64_PFR0_FP// Extract bits indicating VFP implementation
-  cmp   x0, #0                  // VFP is implemented if '0'.
-  b.ne  4f                      // Exit if VFP not implemented.
+  ubfx  x0, x0, #16, #4         // Extract the FP bits 16:19
+  cmp   x0, #0xF                // Check if FP bits are '1111b',
+                                // i.e. Floating Point not implemented
+  b.eq  4f                      // Exit when VFP is not implemented.
+
   // FVP is implemented.
   // Make sure VFP exceptions are not trapped (to any exception level).
   mrs   x0, cpacr_el1           // Read EL1 Coprocessor Access Control Register (CPACR)
-- 
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")

Re: [PATCH] ArmPkg: ARM v8.2 updates for detecting FP

[Leif Lindholm]

On Thu, Sep 14, 2017 at 05:11:21PM +0100, evan.lloyd@arm.com wrote:
> From: Sami Mujawar <samimujawar>

I have fixed Sami's email address up before pushing.

> The ARMv8.2-FP16 extension introduces support for half precision
> floating point and the processor ID registers have been updated to
> enable detection of the implementation.
> 
> The possible values for the FP bits in ID_AA64PFR0_EL1[19:16] are:
>   - 0000 : Floating-point is implemented.
>   - 0001 : Floating-point including Half-precision support is
>            implemented.
>   - 1111 : Floating-point is not implemented.
>   - All other values are reserved.
> 
> Previously ArmEnableVFP() compared the FP bits with 0000b to see if
> the FP was implemented, before enabling FP. Modified this check to
> enable the FP if the FP bits 19:16 are not 1111b.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>

So ... I think longer-term we should try to move a lot of this type of
stuff to c instead, but this is a clear fix - thanks!

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed as 2f16993c25

> ---
>  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> index dde6a756528f3abf1bd5a142448e42122a9bd8fa..2d136d242b943fe2f365bc824953b7fe10c944b7 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> @@ -1,7 +1,7 @@
>  #------------------------------------------------------------------------------
>  #
>  # Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
> -# Copyright (c) 2011 - 2014, ARM Limited. All rights reserved.
> +# Copyright (c) 2011 - 2017, ARM Limited. All rights reserved.
>  # Copyright (c) 2016, Linaro Limited. All rights reserved.
>  #
>  # This program and the accompanying materials
> @@ -403,9 +403,11 @@ ASM_FUNC(ArmEnableVFP)
>    mov   x1, x30                 // Save LR
>    bl    ArmReadIdPfr0           // Read EL1 Processor Feature Register (PFR0)
>    mov   x30, x1                 // Restore LR
> -  ands  x0, x0, #AARCH64_PFR0_FP// Extract bits indicating VFP implementation
> -  cmp   x0, #0                  // VFP is implemented if '0'.
> -  b.ne  4f                      // Exit if VFP not implemented.
> +  ubfx  x0, x0, #16, #4         // Extract the FP bits 16:19
> +  cmp   x0, #0xF                // Check if FP bits are '1111b',
> +                                // i.e. Floating Point not implemented
> +  b.eq  4f                      // Exit when VFP is not implemented.
> +
>    // FVP is implemented.
>    // Make sure VFP exceptions are not trapped (to any exception level).
>    mrs   x0, cpacr_el1           // Read EL1 Coprocessor Access Control Register (CPACR)
> -- 
> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> 

Re: [PATCH] ArmPkg: ARM v8.2 updates for detecting FP

[Evan Lloyd]

> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> Sent: 15 September 2017 17:06
> To: Evan Lloyd <Evan.Lloyd@arm.com>
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>;
> Matteo Carlini <Matteo.Carlini@arm.com>; Sami Mujawar
> <Sami.Mujawar@arm.com>
> Subject: Re: [PATCH] ArmPkg: ARM v8.2 updates for detecting FP
>
> On Thu, Sep 14, 2017 at 05:11:21PM +0100, evan.lloyd@arm.com wrote:
> > From: Sami Mujawar <samimujawar>
>
> I have fixed Sami's email address up before pushing.

 [[Evan Lloyd]]
Thanks, Leif.  That was a problem caused by an out of date config in the
original repo where the change was created.
Sami has fixed any others in our internal repo, so it shouldn't happen again.

>
> > The ARMv8.2-FP16 extension introduces support for half precision
> > floating point and the processor ID registers have been updated to
> > enable detection of the implementation.
> >
> > The possible values for the FP bits in ID_AA64PFR0_EL1[19:16] are:
> >   - 0000 : Floating-point is implemented.
> >   - 0001 : Floating-point including Half-precision support is
> >            implemented.
> >   - 1111 : Floating-point is not implemented.
> >   - All other values are reserved.
> >
> > Previously ArmEnableVFP() compared the FP bits with 0000b to see if
> > the FP was implemented, before enabling FP. Modified this check to
> > enable the FP if the FP bits 19:16 are not 1111b.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> > Signed-off-by: Evan Lloyd <evan.lloyd@arm.com>
>
> So ... I think longer-term we should try to move a lot of this type of stuff to c
> instead, but this is a clear fix - thanks!

 [[Evan Lloyd]] You are most welcome.  We agree about the move to c, as assembler
code doesn't buy anything here. However, if you are going to change that
you probably have to do all of it - not a task I'd want to take on.

>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> Pushed as
> 2f16993c25
>
> > ---
> >  ArmPkg/Library/ArmLib/AArch64/AArch64Support.S | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > index
> >
> dde6a756528f3abf1bd5a142448e42122a9bd8fa..2d136d242b943fe2f365bc82
> 4953
> > b7fe10c944b7 100644
> > --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S
> > @@ -1,7 +1,7 @@
> >
> > #---------------------------------------------------------------------
> > ---------
> >  #
> >  # Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR> -#
> > Copyright (c) 2011 - 2014, ARM Limited. All rights reserved.
> > +# Copyright (c) 2011 - 2017, ARM Limited. All rights reserved.
> >  # Copyright (c) 2016, Linaro Limited. All rights reserved.
> >  #
> >  # This program and the accompanying materials @@ -403,9 +403,11 @@
> > ASM_FUNC(ArmEnableVFP)
> >    mov   x1, x30                 // Save LR
> >    bl    ArmReadIdPfr0           // Read EL1 Processor Feature Register (PFR0)
> >    mov   x30, x1                 // Restore LR
> > -  ands  x0, x0, #AARCH64_PFR0_FP// Extract bits indicating VFP
> implementation
> > -  cmp   x0, #0                  // VFP is implemented if '0'.
> > -  b.ne  4f                      // Exit if VFP not implemented.
> > +  ubfx  x0, x0, #16, #4         // Extract the FP bits 16:19
> > +  cmp   x0, #0xF                // Check if FP bits are '1111b',
> > +                                // i.e. Floating Point not implemented
> > +  b.eq  4f                      // Exit when VFP is not implemented.
> > +
> >    // FVP is implemented.
> >    // Make sure VFP exceptions are not trapped (to any exception level).
> >    mrs   x0, cpacr_el1           // Read EL1 Coprocessor Access Control Register
> (CPACR)
> > --
> > Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
> >
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.