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

George Pee
2022-09-09T14:55:23Z

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 {