Re: [PATCH v3 4/4] ARM: vfp: Reimplement VFP exception entry in C code

[Ard Biesheuvel <ardb@kernel.org>]

On Thu, 16 Mar 2023 at 09:59, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Mar 16, 2023 at 9:20 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> > En/disabling softirqs from asm code turned out to be trickier than
> > expected, so vfp_support_entry now returns by tail calling
> > __local_enable_bh_ip() and passing the same arguments that a C call to
> > local_bh_enable() would pass. However, this is slightly hacky, as we
> > don't want to carry our own implementation of local_bh_enable().
> >
> > So let's bite the bullet, and get rid of the asm logic in
> > vfp_support_entry that reasons about whether or not to save and/or
> > reload the VFP state, and about whether or not an FP exception is
> > pending, and only keep the VFP loading logic as a function that is
> > callable from C.
> >
> > Replicate the removed logic in vfp_entry(), and use the exact same
> > reasoning as in the asm code. To emphasize the correspondance, retain
> > some of the asm comments in the C version as well.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> I tried to model what the assembly is doing in my head to review
> if the new code in C does the same thing, but after following some of
> the semantics (which look correct) I realized it will mostly be down to
> testing anyway.
>

Indeed. The only hardware I have that actually generates VFP
exceptions is Raspberry Pi 1, so I gave it a spin there.

After adding a perf software event like below, and running John
Hauser's testfloat tool [0], I get the exact same mix of passes and
failures, i.e.,

$ perf_4.9 stat -e emulation-faults ./testfloat -all2 >before 2>&1
$ diff -u before after
--- before 2023-03-18 16:51:17.842814747 +0100
+++ after 2023-03-18 16:49:11.783973769 +0100
@@ -399,5 +399,5 @@

            259,277      emulation-faults:u

-       5.810963659 seconds time elapsed
+       5.748333166 seconds time elapsed

So as far as ARM 1176 is concerned, I think we are ok. However, it
appears all generated emulation exceptions are synchronous ones, so
the async code path is not testable this way.


[0] http://www.jhauser.us/arithmetic/TestFloat.html

diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index b56fad06cef0dfad..f46eb681f2f95448 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -18,6 +18,7 @@
 #include <linux/uaccess.h>
 #include <linux/user.h>
 #include <linux/export.h>
+#include <linux/perf_event.h>

 #include <asm/cp15.h>
 #include <asm/cputype.h>
@@ -313,6 +314,7 @@ static u32 vfp_emulate_instruction(u32 inst, u32
fpscr, struct pt_regs *regs)
                 * emulate it.
                 */
        }
+       perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->ARM_pc);
        return exceptions & ~VFP_NAN_FLAG;
 }



> It's definately better to have it like this so FWIW:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>

Thanks

> Apparently a bunch of this code was written by Catalin for VFPv3 support,
> he may or may not have the memory and time to go back and look at it, but
> let's page him in anyway :)
>
> Yours,
> Linus Walleij

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