[PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[george pee]

Report as fphp to be consistent with arm64

Signed-off-by: george pee <georgepee@gmail.com>
---
 arch/arm/include/uapi/asm/hwcap.h | 1 +
 arch/arm/kernel/setup.c           | 1 +
 arch/arm/vfp/vfpmodule.c          | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
index 990199d8b7c6..f975845ce5d3 100644
--- a/arch/arm/include/uapi/asm/hwcap.h
+++ b/arch/arm/include/uapi/asm/hwcap.h
@@ -28,6 +28,7 @@
 #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
 #define HWCAP_LPAE	(1 << 20)
 #define HWCAP_EVTSTRM	(1 << 21)
+#define HWCAP_FPHP	(1 << 22)
 
 /*
  * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 1e8a50a97edf..6694ced0552a 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1249,6 +1249,7 @@ static const char *hwcap_str[] = {
 	"vfpd32",
 	"lpae",
 	"evtstrm",
+	"fphp",
 	NULL
 };
 
diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index 2cb355c1b5b7..cef8c64ce8bd 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -831,6 +831,8 @@ static int __init vfp_init(void)
 
 			if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
 				elf_hwcap |= HWCAP_VFPv4;
+			if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
+				elf_hwcap |= HWCAP_FPHP;
 		}
 	/* Extract the architecture version on pre-cpuid scheme */
 	} else {
-- 
2.37.3

[PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[george pee]

Report as fphp to be consistent with arm64

Signed-off-by: george pee <georgepee@gmail.com>
---
 arch/arm/include/uapi/asm/hwcap.h | 1 +
 arch/arm/kernel/setup.c           | 1 +
 arch/arm/vfp/vfpmodule.c          | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
index 990199d8b7c6..f975845ce5d3 100644
--- a/arch/arm/include/uapi/asm/hwcap.h
+++ b/arch/arm/include/uapi/asm/hwcap.h
@@ -28,6 +28,7 @@
 #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
 #define HWCAP_LPAE	(1 << 20)
 #define HWCAP_EVTSTRM	(1 << 21)
+#define HWCAP_FPHP	(1 << 22)
 
 /*
  * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 1e8a50a97edf..6694ced0552a 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1249,6 +1249,7 @@ static const char *hwcap_str[] = {
 	"vfpd32",
 	"lpae",
 	"evtstrm",
+	"fphp",
 	NULL
 };
 
diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index 2cb355c1b5b7..cef8c64ce8bd 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -831,6 +831,8 @@ static int __init vfp_init(void)
 
 			if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
 				elf_hwcap |= HWCAP_VFPv4;
+			if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
+				elf_hwcap |= HWCAP_FPHP;
 		}
 	/* Extract the architecture version on pre-cpuid scheme */
 	} else {
-- 
2.37.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Catalin Marinas]

On Thu, Sep 01, 2022 at 09:13:05AM -0500, george pee wrote:
> Report as fphp to be consistent with arm64

Do you have a use-case as well? It may help deciding what to do with
this.

> diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> index 990199d8b7c6..f975845ce5d3 100644
> --- a/arch/arm/include/uapi/asm/hwcap.h
> +++ b/arch/arm/include/uapi/asm/hwcap.h
> @@ -28,6 +28,7 @@
>  #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
>  #define HWCAP_LPAE	(1 << 20)
>  #define HWCAP_EVTSTRM	(1 << 21)
> +#define HWCAP_FPHP	(1 << 22)

I think with ARMv8 features on the arm32 kernel we tend to add them to
HWCAP2_*. With such change:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

I'll leave the decision to Russell on whether he wants this merged. An
argument may be that people still want to run 32-bit user-space and even
if they use am arm64 kernel, we can't add a COMPAT_HWCAP2_FPHP until we
have the arm32 counterpart. An alternative may be to only add the uapi
definition under arch/arm but without any functionality (so never
exposed to user). The arm64 kernel could expose it to compat tasks.

So, if Russell is ok with any of the options above, please also add the
compat arm64 support ;).

-- 
Catalin

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Catalin Marinas]

On Thu, Sep 01, 2022 at 09:13:05AM -0500, george pee wrote:
> Report as fphp to be consistent with arm64

Do you have a use-case as well? It may help deciding what to do with
this.

> diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> index 990199d8b7c6..f975845ce5d3 100644
> --- a/arch/arm/include/uapi/asm/hwcap.h
> +++ b/arch/arm/include/uapi/asm/hwcap.h
> @@ -28,6 +28,7 @@
>  #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
>  #define HWCAP_LPAE	(1 << 20)
>  #define HWCAP_EVTSTRM	(1 << 21)
> +#define HWCAP_FPHP	(1 << 22)

I think with ARMv8 features on the arm32 kernel we tend to add them to
HWCAP2_*. With such change:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

I'll leave the decision to Russell on whether he wants this merged. An
argument may be that people still want to run 32-bit user-space and even
if they use am arm64 kernel, we can't add a COMPAT_HWCAP2_FPHP until we
have the arm32 counterpart. An alternative may be to only add the uapi
definition under arch/arm but without any functionality (so never
exposed to user). The arm64 kernel could expose it to compat tasks.

So, if Russell is ok with any of the options above, please also add the
compat arm64 support ;).

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[George Pee]

The use case is really being able to tell, from userspace on a 32-bit
kernel, if FPHP is supported. It's really just reporting for
convenience.

It wasn't clear to me why HWCAP2 was used in some cases and not
others.  I can add FPHP to HWCAP2 if that's the right thing to do
here.

On Fri, Sep 9, 2022 at 6:39 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Thu, Sep 01, 2022 at 09:13:05AM -0500, george pee wrote:
> > Report as fphp to be consistent with arm64
>
> Do you have a use-case as well? It may help deciding what to do with
> this.
>
> > diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> > index 990199d8b7c6..f975845ce5d3 100644
> > --- a/arch/arm/include/uapi/asm/hwcap.h
> > +++ b/arch/arm/include/uapi/asm/hwcap.h
> > @@ -28,6 +28,7 @@
> >  #define HWCAP_IDIV   (HWCAP_IDIVA | HWCAP_IDIVT)
> >  #define HWCAP_LPAE   (1 << 20)
> >  #define HWCAP_EVTSTRM        (1 << 21)
> > +#define HWCAP_FPHP   (1 << 22)
>
> I think with ARMv8 features on the arm32 kernel we tend to add them to
> HWCAP2_*. With such change:
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>
> I'll leave the decision to Russell on whether he wants this merged. An
> argument may be that people still want to run 32-bit user-space and even
> if they use am arm64 kernel, we can't add a COMPAT_HWCAP2_FPHP until we
> have the arm32 counterpart. An alternative may be to only add the uapi
> definition under arch/arm but without any functionality (so never
> exposed to user). The arm64 kernel could expose it to compat tasks.
>
> So, if Russell is ok with any of the options above, please also add the
> compat arm64 support ;).
>
> --
> Catalin

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[George Pee]

The use case is really being able to tell, from userspace on a 32-bit
kernel, if FPHP is supported. It's really just reporting for
convenience.

It wasn't clear to me why HWCAP2 was used in some cases and not
others.  I can add FPHP to HWCAP2 if that's the right thing to do
here.

On Fri, Sep 9, 2022 at 6:39 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Thu, Sep 01, 2022 at 09:13:05AM -0500, george pee wrote:
> > Report as fphp to be consistent with arm64
>
> Do you have a use-case as well? It may help deciding what to do with
> this.
>
> > diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> > index 990199d8b7c6..f975845ce5d3 100644
> > --- a/arch/arm/include/uapi/asm/hwcap.h
> > +++ b/arch/arm/include/uapi/asm/hwcap.h
> > @@ -28,6 +28,7 @@
> >  #define HWCAP_IDIV   (HWCAP_IDIVA | HWCAP_IDIVT)
> >  #define HWCAP_LPAE   (1 << 20)
> >  #define HWCAP_EVTSTRM        (1 << 21)
> > +#define HWCAP_FPHP   (1 << 22)
>
> I think with ARMv8 features on the arm32 kernel we tend to add them to
> HWCAP2_*. With such change:
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>
> I'll leave the decision to Russell on whether he wants this merged. An
> argument may be that people still want to run 32-bit user-space and even
> if they use am arm64 kernel, we can't add a COMPAT_HWCAP2_FPHP until we
> have the arm32 counterpart. An alternative may be to only add the uapi
> definition under arch/arm but without any functionality (so never
> exposed to user). The arm64 kernel could expose it to compat tasks.
>
> So, if Russell is ok with any of the options above, please also add the
> compat arm64 support ;).
>
> --
> Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Robin Murphy]

On 2022-09-01 15:13, george pee wrote:
> Report as fphp to be consistent with arm64

Wasn't the original problem that the VFP support code doesn't understand 
the new FP16 instruction encodings, so in practice they don't actually 
work reliably? Exposing a hwcap to say they're functional doesn't 
inherently make them functional - if there is already another patch 
somewhere for that, it should be made clear that this depends on it.

Robin.

> Signed-off-by: george pee <georgepee@gmail.com>
> ---
>   arch/arm/include/uapi/asm/hwcap.h | 1 +
>   arch/arm/kernel/setup.c           | 1 +
>   arch/arm/vfp/vfpmodule.c          | 2 ++
>   3 files changed, 4 insertions(+)
> 
> diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> index 990199d8b7c6..f975845ce5d3 100644
> --- a/arch/arm/include/uapi/asm/hwcap.h
> +++ b/arch/arm/include/uapi/asm/hwcap.h
> @@ -28,6 +28,7 @@
>   #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
>   #define HWCAP_LPAE	(1 << 20)
>   #define HWCAP_EVTSTRM	(1 << 21)
> +#define HWCAP_FPHP	(1 << 22)
>   
>   /*
>    * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 1e8a50a97edf..6694ced0552a 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -1249,6 +1249,7 @@ static const char *hwcap_str[] = {
>   	"vfpd32",
>   	"lpae",
>   	"evtstrm",
> +	"fphp",
>   	NULL
>   };
>   
> diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
> index 2cb355c1b5b7..cef8c64ce8bd 100644
> --- a/arch/arm/vfp/vfpmodule.c
> +++ b/arch/arm/vfp/vfpmodule.c
> @@ -831,6 +831,8 @@ static int __init vfp_init(void)
>   
>   			if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
>   				elf_hwcap |= HWCAP_VFPv4;
> +			if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
> +				elf_hwcap |= HWCAP_FPHP;
>   		}
>   	/* Extract the architecture version on pre-cpuid scheme */
>   	} else {

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Robin Murphy]

On 2022-09-01 15:13, george pee wrote:
> Report as fphp to be consistent with arm64

Wasn't the original problem that the VFP support code doesn't understand 
the new FP16 instruction encodings, so in practice they don't actually 
work reliably? Exposing a hwcap to say they're functional doesn't 
inherently make them functional - if there is already another patch 
somewhere for that, it should be made clear that this depends on it.

Robin.

> Signed-off-by: george pee <georgepee@gmail.com>
> ---
>   arch/arm/include/uapi/asm/hwcap.h | 1 +
>   arch/arm/kernel/setup.c           | 1 +
>   arch/arm/vfp/vfpmodule.c          | 2 ++
>   3 files changed, 4 insertions(+)
> 
> diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> index 990199d8b7c6..f975845ce5d3 100644
> --- a/arch/arm/include/uapi/asm/hwcap.h
> +++ b/arch/arm/include/uapi/asm/hwcap.h
> @@ -28,6 +28,7 @@
>   #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
>   #define HWCAP_LPAE	(1 << 20)
>   #define HWCAP_EVTSTRM	(1 << 21)
> +#define HWCAP_FPHP	(1 << 22)
>   
>   /*
>    * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 1e8a50a97edf..6694ced0552a 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -1249,6 +1249,7 @@ static const char *hwcap_str[] = {
>   	"vfpd32",
>   	"lpae",
>   	"evtstrm",
> +	"fphp",
>   	NULL
>   };
>   
> diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
> index 2cb355c1b5b7..cef8c64ce8bd 100644
> --- a/arch/arm/vfp/vfpmodule.c
> +++ b/arch/arm/vfp/vfpmodule.c
> @@ -831,6 +831,8 @@ static int __init vfp_init(void)
>   
>   			if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
>   				elf_hwcap |= HWCAP_VFPv4;
> +			if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
> +				elf_hwcap |= HWCAP_FPHP;
>   		}
>   	/* Extract the architecture version on pre-cpuid scheme */
>   	} else {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[George Pee]

Adding the hwcap was part of the diagnosis process-- I added it just
to make sure that the cpu in question supported the optional
extension.
It seems like it could be useful to be able to check for support in
/proc/cpuinfo.

On Fri, Sep 9, 2022 at 7:46 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2022-09-01 15:13, george pee wrote:
> > Report as fphp to be consistent with arm64
>
> Wasn't the original problem that the VFP support code doesn't understand
> the new FP16 instruction encodings, so in practice they don't actually
> work reliably? Exposing a hwcap to say they're functional doesn't
> inherently make them functional - if there is already another patch
> somewhere for that, it should be made clear that this depends on it.
>
> Robin.
>
> > Signed-off-by: george pee <georgepee@gmail.com>
> > ---
> >   arch/arm/include/uapi/asm/hwcap.h | 1 +
> >   arch/arm/kernel/setup.c           | 1 +
> >   arch/arm/vfp/vfpmodule.c          | 2 ++
> >   3 files changed, 4 insertions(+)
> >
> > diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> > index 990199d8b7c6..f975845ce5d3 100644
> > --- a/arch/arm/include/uapi/asm/hwcap.h
> > +++ b/arch/arm/include/uapi/asm/hwcap.h
> > @@ -28,6 +28,7 @@
> >   #define HWCAP_IDIV  (HWCAP_IDIVA | HWCAP_IDIVT)
> >   #define HWCAP_LPAE  (1 << 20)
> >   #define HWCAP_EVTSTRM       (1 << 21)
> > +#define HWCAP_FPHP   (1 << 22)
> >
> >   /*
> >    * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2
> > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > index 1e8a50a97edf..6694ced0552a 100644
> > --- a/arch/arm/kernel/setup.c
> > +++ b/arch/arm/kernel/setup.c
> > @@ -1249,6 +1249,7 @@ static const char *hwcap_str[] = {
> >       "vfpd32",
> >       "lpae",
> >       "evtstrm",
> > +     "fphp",
> >       NULL
> >   };
> >
> > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
> > index 2cb355c1b5b7..cef8c64ce8bd 100644
> > --- a/arch/arm/vfp/vfpmodule.c
> > +++ b/arch/arm/vfp/vfpmodule.c
> > @@ -831,6 +831,8 @@ static int __init vfp_init(void)
> >
> >                       if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
> >                               elf_hwcap |= HWCAP_VFPv4;
> > +                     if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
> > +                             elf_hwcap |= HWCAP_FPHP;
> >               }
> >       /* Extract the architecture version on pre-cpuid scheme */
> >       } else {

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[George Pee]

Adding the hwcap was part of the diagnosis process-- I added it just
to make sure that the cpu in question supported the optional
extension.
It seems like it could be useful to be able to check for support in
/proc/cpuinfo.

On Fri, Sep 9, 2022 at 7:46 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2022-09-01 15:13, george pee wrote:
> > Report as fphp to be consistent with arm64
>
> Wasn't the original problem that the VFP support code doesn't understand
> the new FP16 instruction encodings, so in practice they don't actually
> work reliably? Exposing a hwcap to say they're functional doesn't
> inherently make them functional - if there is already another patch
> somewhere for that, it should be made clear that this depends on it.
>
> Robin.
>
> > Signed-off-by: george pee <georgepee@gmail.com>
> > ---
> >   arch/arm/include/uapi/asm/hwcap.h | 1 +
> >   arch/arm/kernel/setup.c           | 1 +
> >   arch/arm/vfp/vfpmodule.c          | 2 ++
> >   3 files changed, 4 insertions(+)
> >
> > diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> > index 990199d8b7c6..f975845ce5d3 100644
> > --- a/arch/arm/include/uapi/asm/hwcap.h
> > +++ b/arch/arm/include/uapi/asm/hwcap.h
> > @@ -28,6 +28,7 @@
> >   #define HWCAP_IDIV  (HWCAP_IDIVA | HWCAP_IDIVT)
> >   #define HWCAP_LPAE  (1 << 20)
> >   #define HWCAP_EVTSTRM       (1 << 21)
> > +#define HWCAP_FPHP   (1 << 22)
> >
> >   /*
> >    * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2
> > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > index 1e8a50a97edf..6694ced0552a 100644
> > --- a/arch/arm/kernel/setup.c
> > +++ b/arch/arm/kernel/setup.c
> > @@ -1249,6 +1249,7 @@ static const char *hwcap_str[] = {
> >       "vfpd32",
> >       "lpae",
> >       "evtstrm",
> > +     "fphp",
> >       NULL
> >   };
> >
> > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
> > index 2cb355c1b5b7..cef8c64ce8bd 100644
> > --- a/arch/arm/vfp/vfpmodule.c
> > +++ b/arch/arm/vfp/vfpmodule.c
> > @@ -831,6 +831,8 @@ static int __init vfp_init(void)
> >
> >                       if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
> >                               elf_hwcap |= HWCAP_VFPv4;
> > +                     if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
> > +                             elf_hwcap |= HWCAP_FPHP;
> >               }
> >       /* Extract the architecture version on pre-cpuid scheme */
> >       } else {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Catalin Marinas]

On Fri, Sep 09, 2022 at 08:34:26AM -0500, George Pee wrote:
> Adding the hwcap was part of the diagnosis process-- I added it just
> to make sure that the cpu in question supported the optional
> extension.
> It seems like it could be useful to be able to check for support in
> /proc/cpuinfo.

Ah, I wasn't aware that the feature doesn't work on arm32. I don't think
it makes sense to expose a hwcap bit to user in this case.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Catalin Marinas]

On Fri, Sep 09, 2022 at 08:34:26AM -0500, George Pee wrote:
> Adding the hwcap was part of the diagnosis process-- I added it just
> to make sure that the cpu in question supported the optional
> extension.
> It seems like it could be useful to be able to check for support in
> /proc/cpuinfo.

Ah, I wasn't aware that the feature doesn't work on arm32. I don't think
it makes sense to expose a hwcap bit to user in this case.

-- 
Catalin

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[George Pee]

The details are here.  I originally thought it was a compiler bug
because it first showed up after a toolchain update.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106763

Since FP16 is an optional extension, wouldn't it be beneficial to a
user who compiled some userspace float16 code using gcc
-mcpu=cortex-a55 which ran on a cortex-a55 with FP16 extensions but
SIGILL'd on a cortex-a55 w/o FP16?

On Fri, Sep 9, 2022 at 9:07 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Fri, Sep 09, 2022 at 08:34:26AM -0500, George Pee wrote:
> > Adding the hwcap was part of the diagnosis process-- I added it just
> > to make sure that the cpu in question supported the optional
> > extension.
> > It seems like it could be useful to be able to check for support in
> > /proc/cpuinfo.
>
> Ah, I wasn't aware that the feature doesn't work on arm32. I don't think
> it makes sense to expose a hwcap bit to user in this case.
>
> --
> Catalin

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[George Pee]

The details are here.  I originally thought it was a compiler bug
because it first showed up after a toolchain update.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106763

Since FP16 is an optional extension, wouldn't it be beneficial to a
user who compiled some userspace float16 code using gcc
-mcpu=cortex-a55 which ran on a cortex-a55 with FP16 extensions but
SIGILL'd on a cortex-a55 w/o FP16?

On Fri, Sep 9, 2022 at 9:07 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Fri, Sep 09, 2022 at 08:34:26AM -0500, George Pee wrote:
> > Adding the hwcap was part of the diagnosis process-- I added it just
> > to make sure that the cpu in question supported the optional
> > extension.
> > It seems like it could be useful to be able to check for support in
> > /proc/cpuinfo.
>
> Ah, I wasn't aware that the feature doesn't work on arm32. I don't think
> it makes sense to expose a hwcap bit to user in this case.
>
> --
> Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Catalin Marinas]

On Fri, Sep 09, 2022 at 09:57:39AM -0500, George Pee wrote:
> On Fri, Sep 9, 2022 at 9:07 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Fri, Sep 09, 2022 at 08:34:26AM -0500, George Pee wrote:
> > > Adding the hwcap was part of the diagnosis process-- I added it just
> > > to make sure that the cpu in question supported the optional
> > > extension.
> > > It seems like it could be useful to be able to check for support in
> > > /proc/cpuinfo.
> >
> > Ah, I wasn't aware that the feature doesn't work on arm32. I don't think
> > it makes sense to expose a hwcap bit to user in this case.
> 
> The details are here.  I originally thought it was a compiler bug
> because it first showed up after a toolchain update.
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106763
> 
> Since FP16 is an optional extension, wouldn't it be beneficial to a
> user who compiled some userspace float16 code using gcc
> -mcpu=cortex-a55 which ran on a cortex-a55 with FP16 extensions but
> SIGILL'd on a cortex-a55 w/o FP16?

(please don't top-post)

My point is that if the kernel doesn't have full support for FP16, it
shouldn't advertise it to user even if the hardware supports it. If you
fix the kernel to properly handle FP16 on supporting hardware, then the
HWCAP part is fine by me.

-- 
Catalin

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Catalin Marinas]

On Fri, Sep 09, 2022 at 09:57:39AM -0500, George Pee wrote:
> On Fri, Sep 9, 2022 at 9:07 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Fri, Sep 09, 2022 at 08:34:26AM -0500, George Pee wrote:
> > > Adding the hwcap was part of the diagnosis process-- I added it just
> > > to make sure that the cpu in question supported the optional
> > > extension.
> > > It seems like it could be useful to be able to check for support in
> > > /proc/cpuinfo.
> >
> > Ah, I wasn't aware that the feature doesn't work on arm32. I don't think
> > it makes sense to expose a hwcap bit to user in this case.
> 
> The details are here.  I originally thought it was a compiler bug
> because it first showed up after a toolchain update.
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106763
> 
> Since FP16 is an optional extension, wouldn't it be beneficial to a
> user who compiled some userspace float16 code using gcc
> -mcpu=cortex-a55 which ran on a cortex-a55 with FP16 extensions but
> SIGILL'd on a cortex-a55 w/o FP16?

(please don't top-post)

My point is that if the kernel doesn't have full support for FP16, it
shouldn't advertise it to user even if the hardware supports it. If you
fix the kernel to properly handle FP16 on supporting hardware, then the
HWCAP part is fine by me.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Russell King (Oracle)]

On Fri, Sep 09, 2022 at 04:05:53PM +0100, Catalin Marinas wrote:
> On Fri, Sep 09, 2022 at 09:57:39AM -0500, George Pee wrote:
> > The details are here.  I originally thought it was a compiler bug
> > because it first showed up after a toolchain update.
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106763
> > 
> > Since FP16 is an optional extension, wouldn't it be beneficial to a
> > user who compiled some userspace float16 code using gcc
> > -mcpu=cortex-a55 which ran on a cortex-a55 with FP16 extensions but
> > SIGILL'd on a cortex-a55 w/o FP16?
> 
> (please don't top-post)
> 
> My point is that if the kernel doesn't have full support for FP16, it
> shouldn't advertise it to user even if the hardware supports it. If you
> fix the kernel to properly handle FP16 on supporting hardware, then the
> HWCAP part is fine by me.

Presumably, the only CPUs that are going to support FP16 will have
non-trapping floating point, so the support code shouldn't be entered
at any time to emulate a half-precision instruction, but only to
handle the lazy restore of the thread's floating point registers?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Russell King (Oracle)]

On Fri, Sep 09, 2022 at 04:05:53PM +0100, Catalin Marinas wrote:
> On Fri, Sep 09, 2022 at 09:57:39AM -0500, George Pee wrote:
> > The details are here.  I originally thought it was a compiler bug
> > because it first showed up after a toolchain update.
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106763
> > 
> > Since FP16 is an optional extension, wouldn't it be beneficial to a
> > user who compiled some userspace float16 code using gcc
> > -mcpu=cortex-a55 which ran on a cortex-a55 with FP16 extensions but
> > SIGILL'd on a cortex-a55 w/o FP16?
> 
> (please don't top-post)
> 
> My point is that if the kernel doesn't have full support for FP16, it
> shouldn't advertise it to user even if the hardware supports it. If you
> fix the kernel to properly handle FP16 on supporting hardware, then the
> HWCAP part is fine by me.

Presumably, the only CPUs that are going to support FP16 will have
non-trapping floating point, so the support code shouldn't be entered
at any time to emulate a half-precision instruction, but only to
handle the lazy restore of the thread's floating point registers?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[George Pee]

On Mon, Sep 12, 2022 at 8:05 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Fri, Sep 09, 2022 at 04:05:53PM +0100, Catalin Marinas wrote:
> > On Fri, Sep 09, 2022 at 09:57:39AM -0500, George Pee wrote:
> > > The details are here.  I originally thought it was a compiler bug
> > > because it first showed up after a toolchain update.
> > >
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106763
> > >
> > > Since FP16 is an optional extension, wouldn't it be beneficial to a
> > > user who compiled some userspace float16 code using gcc
> > > -mcpu=cortex-a55 which ran on a cortex-a55 with FP16 extensions but
> > > SIGILL'd on a cortex-a55 w/o FP16?
> >
> > (please don't top-post)
> >
> > My point is that if the kernel doesn't have full support for FP16, it
> > shouldn't advertise it to user even if the hardware supports it. If you
> > fix the kernel to properly handle FP16 on supporting hardware, then the
> > HWCAP part is fine by me.
>
> Presumably, the only CPUs that are going to support FP16 will have
> non-trapping floating point, so the support code shouldn't be entered
> at any time to emulate a half-precision instruction, but only to
> handle the lazy restore of the thread's floating point registers?
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

I didn't see this until after I submitted v2 of the patch.  Let me
take a look at the fp emulation code path.
I had assumed that CP9 handling would work just like CP10/CP11 does in
entry-armv.S and wouldn't need any special handling.

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[George Pee]

On Mon, Sep 12, 2022 at 8:05 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Fri, Sep 09, 2022 at 04:05:53PM +0100, Catalin Marinas wrote:
> > On Fri, Sep 09, 2022 at 09:57:39AM -0500, George Pee wrote:
> > > The details are here.  I originally thought it was a compiler bug
> > > because it first showed up after a toolchain update.
> > >
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106763
> > >
> > > Since FP16 is an optional extension, wouldn't it be beneficial to a
> > > user who compiled some userspace float16 code using gcc
> > > -mcpu=cortex-a55 which ran on a cortex-a55 with FP16 extensions but
> > > SIGILL'd on a cortex-a55 w/o FP16?
> >
> > (please don't top-post)
> >
> > My point is that if the kernel doesn't have full support for FP16, it
> > shouldn't advertise it to user even if the hardware supports it. If you
> > fix the kernel to properly handle FP16 on supporting hardware, then the
> > HWCAP part is fine by me.
>
> Presumably, the only CPUs that are going to support FP16 will have
> non-trapping floating point, so the support code shouldn't be entered
> at any time to emulate a half-precision instruction, but only to
> handle the lazy restore of the thread's floating point registers?
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

I didn't see this until after I submitted v2 of the patch.  Let me
take a look at the fp emulation code path.
I had assumed that CP9 handling would work just like CP10/CP11 does in
entry-armv.S and wouldn't need any special handling.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Robin Murphy]

On 2022-09-09 14:34, George Pee wrote:
> Adding the hwcap was part of the diagnosis process-- I added it just
> to make sure that the cpu in question supported the optional
> extension.
> It seems like it could be useful to be able to check for support in
> /proc/cpuinfo.

Sure, but "support" is about more than just what happens to be present 
in hardware. Observe that you can build the 32-bit kernel with 
CONFIG_VFP=n, and it then does not report and VFP or NEON hwcaps, 
because those features will not be usable in that configuration, even if 
you know the hardware implements them.

Note that this looks different on arm64, since there we always expect to 
have FPSIMD hardware available, so support in the kernel is 
unconditional, plus that kernel support is also a lot simpler since we 
don't have a soft-float ABI with all the corresponding trapping stuff 
either.

It might just be the case here that the call_fpe logic needs a bit of 
tweaking to provide proper support, but I'm not sufficiently familiar 
with the ARM VFP code in general to be sure.

Thanks,
Robin.

> On Fri, Sep 9, 2022 at 7:46 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2022-09-01 15:13, george pee wrote:
>>> Report as fphp to be consistent with arm64
>>
>> Wasn't the original problem that the VFP support code doesn't understand
>> the new FP16 instruction encodings, so in practice they don't actually
>> work reliably? Exposing a hwcap to say they're functional doesn't
>> inherently make them functional - if there is already another patch
>> somewhere for that, it should be made clear that this depends on it.
>>
>> Robin.
>>
>>> Signed-off-by: george pee <georgepee@gmail.com>
>>> ---
>>>    arch/arm/include/uapi/asm/hwcap.h | 1 +
>>>    arch/arm/kernel/setup.c           | 1 +
>>>    arch/arm/vfp/vfpmodule.c          | 2 ++
>>>    3 files changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
>>> index 990199d8b7c6..f975845ce5d3 100644
>>> --- a/arch/arm/include/uapi/asm/hwcap.h
>>> +++ b/arch/arm/include/uapi/asm/hwcap.h
>>> @@ -28,6 +28,7 @@
>>>    #define HWCAP_IDIV  (HWCAP_IDIVA | HWCAP_IDIVT)
>>>    #define HWCAP_LPAE  (1 << 20)
>>>    #define HWCAP_EVTSTRM       (1 << 21)
>>> +#define HWCAP_FPHP   (1 << 22)
>>>
>>>    /*
>>>     * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2
>>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
>>> index 1e8a50a97edf..6694ced0552a 100644
>>> --- a/arch/arm/kernel/setup.c
>>> +++ b/arch/arm/kernel/setup.c
>>> @@ -1249,6 +1249,7 @@ static const char *hwcap_str[] = {
>>>        "vfpd32",
>>>        "lpae",
>>>        "evtstrm",
>>> +     "fphp",
>>>        NULL
>>>    };
>>>
>>> diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
>>> index 2cb355c1b5b7..cef8c64ce8bd 100644
>>> --- a/arch/arm/vfp/vfpmodule.c
>>> +++ b/arch/arm/vfp/vfpmodule.c
>>> @@ -831,6 +831,8 @@ static int __init vfp_init(void)
>>>
>>>                        if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
>>>                                elf_hwcap |= HWCAP_VFPv4;
>>> +                     if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
>>> +                             elf_hwcap |= HWCAP_FPHP;
>>>                }
>>>        /* Extract the architecture version on pre-cpuid scheme */
>>>        } else {

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[Robin Murphy]

On 2022-09-09 14:34, George Pee wrote:
> Adding the hwcap was part of the diagnosis process-- I added it just
> to make sure that the cpu in question supported the optional
> extension.
> It seems like it could be useful to be able to check for support in
> /proc/cpuinfo.

Sure, but "support" is about more than just what happens to be present 
in hardware. Observe that you can build the 32-bit kernel with 
CONFIG_VFP=n, and it then does not report and VFP or NEON hwcaps, 
because those features will not be usable in that configuration, even if 
you know the hardware implements them.

Note that this looks different on arm64, since there we always expect to 
have FPSIMD hardware available, so support in the kernel is 
unconditional, plus that kernel support is also a lot simpler since we 
don't have a soft-float ABI with all the corresponding trapping stuff 
either.

It might just be the case here that the call_fpe logic needs a bit of 
tweaking to provide proper support, but I'm not sufficiently familiar 
with the ARM VFP code in general to be sure.

Thanks,
Robin.

> On Fri, Sep 9, 2022 at 7:46 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2022-09-01 15:13, george pee wrote:
>>> Report as fphp to be consistent with arm64
>>
>> Wasn't the original problem that the VFP support code doesn't understand
>> the new FP16 instruction encodings, so in practice they don't actually
>> work reliably? Exposing a hwcap to say they're functional doesn't
>> inherently make them functional - if there is already another patch
>> somewhere for that, it should be made clear that this depends on it.
>>
>> Robin.
>>
>>> Signed-off-by: george pee <georgepee@gmail.com>
>>> ---
>>>    arch/arm/include/uapi/asm/hwcap.h | 1 +
>>>    arch/arm/kernel/setup.c           | 1 +
>>>    arch/arm/vfp/vfpmodule.c          | 2 ++
>>>    3 files changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
>>> index 990199d8b7c6..f975845ce5d3 100644
>>> --- a/arch/arm/include/uapi/asm/hwcap.h
>>> +++ b/arch/arm/include/uapi/asm/hwcap.h
>>> @@ -28,6 +28,7 @@
>>>    #define HWCAP_IDIV  (HWCAP_IDIVA | HWCAP_IDIVT)
>>>    #define HWCAP_LPAE  (1 << 20)
>>>    #define HWCAP_EVTSTRM       (1 << 21)
>>> +#define HWCAP_FPHP   (1 << 22)
>>>
>>>    /*
>>>     * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2
>>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
>>> index 1e8a50a97edf..6694ced0552a 100644
>>> --- a/arch/arm/kernel/setup.c
>>> +++ b/arch/arm/kernel/setup.c
>>> @@ -1249,6 +1249,7 @@ static const char *hwcap_str[] = {
>>>        "vfpd32",
>>>        "lpae",
>>>        "evtstrm",
>>> +     "fphp",
>>>        NULL
>>>    };
>>>
>>> diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
>>> index 2cb355c1b5b7..cef8c64ce8bd 100644
>>> --- a/arch/arm/vfp/vfpmodule.c
>>> +++ b/arch/arm/vfp/vfpmodule.c
>>> @@ -831,6 +831,8 @@ static int __init vfp_init(void)
>>>
>>>                        if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
>>>                                elf_hwcap |= HWCAP_VFPv4;
>>> +                     if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
>>> +                             elf_hwcap |= HWCAP_FPHP;
>>>                }
>>>        /* Extract the architecture version on pre-cpuid scheme */
>>>        } else {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[George Pee]

That makes a lot of sense.  How's this?  Flipping the HWCAP2_FPHP bit
is already in a CONFIG_VFPv3 check.

diff --git a/arch/arm/include/uapi/asm/hwcap.h
b/arch/arm/include/uapi/asm/hwcap.h
index 990199d8b7c6..5d635dce8853 100644
--- a/arch/arm/include/uapi/asm/hwcap.h
+++ b/arch/arm/include/uapi/asm/hwcap.h
@@ -37,5 +37,6 @@
 #define HWCAP2_SHA1    (1 << 2)
 #define HWCAP2_SHA2    (1 << 3)
 #define HWCAP2_CRC32    (1 << 4)
+#define HWCAP2_FPHP    (1 << 5)

 #endif /* _UAPI__ASMARM_HWCAP_H */
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index c39303e5c234..161f8df852e1 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -625,11 +625,12 @@ call_fpe:
     ret.w    lr                @ CP#6
     ret.w    lr                @ CP#7
     ret.w    lr                @ CP#8
-    ret.w    lr                @ CP#9
 #ifdef CONFIG_VFP
+    W(b)    do_vfp                @ CP#9  (VFP/FP16)
     W(b)    do_vfp                @ CP#10 (VFP)
     W(b)    do_vfp                @ CP#11 (VFP)
 #else
+    ret.w    lr                @ CP#9
     ret.w    lr                @ CP#10 (VFP)
     ret.w    lr                @ CP#11 (VFP)
 #endif
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 1e8a50a97edf..8887d0f447d6 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1258,6 +1258,7 @@ static const char *hwcap2_str[] = {
     "sha1",
     "sha2",
     "crc32",
+    "fphp",
     NULL
 };

diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index 2cb355c1b5b7..fb774fd5c614 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -831,6 +831,8 @@ static int __init vfp_init(void)

             if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
                 elf_hwcap |= HWCAP_VFPv4;
+            if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
+                elf_hwcap2 |= HWCAP2_FPHP;
         }
     /* Extract the architecture version on pre-cpuid scheme */
     } else {

On Fri, Sep 9, 2022 at 9:17 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2022-09-09 14:34, George Pee wrote:
> > Adding the hwcap was part of the diagnosis process-- I added it just
> > to make sure that the cpu in question supported the optional
> > extension.
> > It seems like it could be useful to be able to check for support in
> > /proc/cpuinfo.
>
> Sure, but "support" is about more than just what happens to be present
> in hardware. Observe that you can build the 32-bit kernel with
> CONFIG_VFP=n, and it then does not report and VFP or NEON hwcaps,
> because those features will not be usable in that configuration, even if
> you know the hardware implements them.
>
> Note that this looks different on arm64, since there we always expect to
> have FPSIMD hardware available, so support in the kernel is
> unconditional, plus that kernel support is also a lot simpler since we
> don't have a soft-float ABI with all the corresponding trapping stuff
> either.
>
> It might just be the case here that the call_fpe logic needs a bit of
> tweaking to provide proper support, but I'm not sufficiently familiar
> with the ARM VFP code in general to be sure.
>
> Thanks,
> Robin.
>
> > On Fri, Sep 9, 2022 at 7:46 AM Robin Murphy <robin.murphy@arm.com> wrote:
> >>
> >> On 2022-09-01 15:13, george pee wrote:
> >>> Report as fphp to be consistent with arm64
> >>
> >> Wasn't the original problem that the VFP support code doesn't understand
> >> the new FP16 instruction encodings, so in practice they don't actually
> >> work reliably? Exposing a hwcap to say they're functional doesn't
> >> inherently make them functional - if there is already another patch
> >> somewhere for that, it should be made clear that this depends on it.
> >>
> >> Robin.
> >>
> >>> Signed-off-by: george pee <georgepee@gmail.com>
> >>> ---
> >>>    arch/arm/include/uapi/asm/hwcap.h | 1 +
> >>>    arch/arm/kernel/setup.c           | 1 +
> >>>    arch/arm/vfp/vfpmodule.c          | 2 ++
> >>>    3 files changed, 4 insertions(+)
> >>>
> >>> diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> >>> index 990199d8b7c6..f975845ce5d3 100644
> >>> --- a/arch/arm/include/uapi/asm/hwcap.h
> >>> +++ b/arch/arm/include/uapi/asm/hwcap.h
> >>> @@ -28,6 +28,7 @@
> >>>    #define HWCAP_IDIV  (HWCAP_IDIVA | HWCAP_IDIVT)
> >>>    #define HWCAP_LPAE  (1 << 20)
> >>>    #define HWCAP_EVTSTRM       (1 << 21)
> >>> +#define HWCAP_FPHP   (1 << 22)
> >>>
> >>>    /*
> >>>     * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2
> >>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> >>> index 1e8a50a97edf..6694ced0552a 100644
> >>> --- a/arch/arm/kernel/setup.c
> >>> +++ b/arch/arm/kernel/setup.c
> >>> @@ -1249,6 +1249,7 @@ static const char *hwcap_str[] = {
> >>>        "vfpd32",
> >>>        "lpae",
> >>>        "evtstrm",
> >>> +     "fphp",
> >>>        NULL
> >>>    };
> >>>
> >>> diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
> >>> index 2cb355c1b5b7..cef8c64ce8bd 100644
> >>> --- a/arch/arm/vfp/vfpmodule.c
> >>> +++ b/arch/arm/vfp/vfpmodule.c
> >>> @@ -831,6 +831,8 @@ static int __init vfp_init(void)
> >>>
> >>>                        if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
> >>>                                elf_hwcap |= HWCAP_VFPv4;
> >>> +                     if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
> >>> +                             elf_hwcap |= HWCAP_FPHP;
> >>>                }
> >>>        /* Extract the architecture version on pre-cpuid scheme */
> >>>        } else {

Re: [PATCH] Report support for optional ARMv8.2 half-precision floating point extension

[George Pee]

That makes a lot of sense.  How's this?  Flipping the HWCAP2_FPHP bit
is already in a CONFIG_VFPv3 check.

diff --git a/arch/arm/include/uapi/asm/hwcap.h
b/arch/arm/include/uapi/asm/hwcap.h
index 990199d8b7c6..5d635dce8853 100644
--- a/arch/arm/include/uapi/asm/hwcap.h
+++ b/arch/arm/include/uapi/asm/hwcap.h
@@ -37,5 +37,6 @@
 #define HWCAP2_SHA1    (1 << 2)
 #define HWCAP2_SHA2    (1 << 3)
 #define HWCAP2_CRC32    (1 << 4)
+#define HWCAP2_FPHP    (1 << 5)

 #endif /* _UAPI__ASMARM_HWCAP_H */
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index c39303e5c234..161f8df852e1 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -625,11 +625,12 @@ call_fpe:
     ret.w    lr                @ CP#6
     ret.w    lr                @ CP#7
     ret.w    lr                @ CP#8
-    ret.w    lr                @ CP#9
 #ifdef CONFIG_VFP
+    W(b)    do_vfp                @ CP#9  (VFP/FP16)
     W(b)    do_vfp                @ CP#10 (VFP)
     W(b)    do_vfp                @ CP#11 (VFP)
 #else
+    ret.w    lr                @ CP#9
     ret.w    lr                @ CP#10 (VFP)
     ret.w    lr                @ CP#11 (VFP)
 #endif
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 1e8a50a97edf..8887d0f447d6 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1258,6 +1258,7 @@ static const char *hwcap2_str[] = {
     "sha1",
     "sha2",
     "crc32",
+    "fphp",
     NULL
 };

diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index 2cb355c1b5b7..fb774fd5c614 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -831,6 +831,8 @@ static int __init vfp_init(void)

             if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
                 elf_hwcap |= HWCAP_VFPv4;
+            if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
+                elf_hwcap2 |= HWCAP2_FPHP;
         }
     /* Extract the architecture version on pre-cpuid scheme */
     } else {

On Fri, Sep 9, 2022 at 9:17 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2022-09-09 14:34, George Pee wrote:
> > Adding the hwcap was part of the diagnosis process-- I added it just
> > to make sure that the cpu in question supported the optional
> > extension.
> > It seems like it could be useful to be able to check for support in
> > /proc/cpuinfo.
>
> Sure, but "support" is about more than just what happens to be present
> in hardware. Observe that you can build the 32-bit kernel with
> CONFIG_VFP=n, and it then does not report and VFP or NEON hwcaps,
> because those features will not be usable in that configuration, even if
> you know the hardware implements them.
>
> Note that this looks different on arm64, since there we always expect to
> have FPSIMD hardware available, so support in the kernel is
> unconditional, plus that kernel support is also a lot simpler since we
> don't have a soft-float ABI with all the corresponding trapping stuff
> either.
>
> It might just be the case here that the call_fpe logic needs a bit of
> tweaking to provide proper support, but I'm not sufficiently familiar
> with the ARM VFP code in general to be sure.
>
> Thanks,
> Robin.
>
> > On Fri, Sep 9, 2022 at 7:46 AM Robin Murphy <robin.murphy@arm.com> wrote:
> >>
> >> On 2022-09-01 15:13, george pee wrote:
> >>> Report as fphp to be consistent with arm64
> >>
> >> Wasn't the original problem that the VFP support code doesn't understand
> >> the new FP16 instruction encodings, so in practice they don't actually
> >> work reliably? Exposing a hwcap to say they're functional doesn't
> >> inherently make them functional - if there is already another patch
> >> somewhere for that, it should be made clear that this depends on it.
> >>
> >> Robin.
> >>
> >>> Signed-off-by: george pee <georgepee@gmail.com>
> >>> ---
> >>>    arch/arm/include/uapi/asm/hwcap.h | 1 +
> >>>    arch/arm/kernel/setup.c           | 1 +
> >>>    arch/arm/vfp/vfpmodule.c          | 2 ++
> >>>    3 files changed, 4 insertions(+)
> >>>
> >>> diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> >>> index 990199d8b7c6..f975845ce5d3 100644
> >>> --- a/arch/arm/include/uapi/asm/hwcap.h
> >>> +++ b/arch/arm/include/uapi/asm/hwcap.h
> >>> @@ -28,6 +28,7 @@
> >>>    #define HWCAP_IDIV  (HWCAP_IDIVA | HWCAP_IDIVT)
> >>>    #define HWCAP_LPAE  (1 << 20)
> >>>    #define HWCAP_EVTSTRM       (1 << 21)
> >>> +#define HWCAP_FPHP   (1 << 22)
> >>>
> >>>    /*
> >>>     * HWCAP2 flags - for elf_hwcap2 (in kernel) and AT_HWCAP2
> >>> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> >>> index 1e8a50a97edf..6694ced0552a 100644
> >>> --- a/arch/arm/kernel/setup.c
> >>> +++ b/arch/arm/kernel/setup.c
> >>> @@ -1249,6 +1249,7 @@ static const char *hwcap_str[] = {
> >>>        "vfpd32",
> >>>        "lpae",
> >>>        "evtstrm",
> >>> +     "fphp",
> >>>        NULL
> >>>    };
> >>>
> >>> diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
> >>> index 2cb355c1b5b7..cef8c64ce8bd 100644
> >>> --- a/arch/arm/vfp/vfpmodule.c
> >>> +++ b/arch/arm/vfp/vfpmodule.c
> >>> @@ -831,6 +831,8 @@ static int __init vfp_init(void)
> >>>
> >>>                        if ((fmrx(MVFR1) & 0xf0000000) == 0x10000000)
> >>>                                elf_hwcap |= HWCAP_VFPv4;
> >>> +                     if ((fmrx(MVFR1) & 0x0f000000) == 0x03000000)
> >>> +                             elf_hwcap |= HWCAP_FPHP;
> >>>                }
> >>>        /* Extract the architecture version on pre-cpuid scheme */
> >>>        } else {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel