[Qemu-devel] [PATCH] target/arm: Make FPSCR/FPCR trapped-exception bits RAZ/WI

[Qemu-devel] [PATCH] target/arm: Make FPSCR/FPCR trapped-exception bits RAZ/WI

[Peter Maydell]

The {IOE, DZE, OFE, UFE, IXE, IDE} bits in the FPSCR/FPCR are for
enabling trapped IEEE floating point exceptions (where IEEE exception
conditions cause a CPU exception rather than updating the FPSR status
bits). QEMU doesn't implement this (and nor does the hardware we're
modelling), but for implementations which don't implement trapped
exception handling these control bits are supposed to be RAZ/WI.
This allows guest code to test for whether the feature is present
by trying to write to the bit and checking whether it sticks.

QEMU is incorrectly making these bits read as written. Make them
RAZ/WI as the architecture requires.

In particular this was causing problems for the NetBSD automatic
test suite.

Reported-by: Martin Husemann <martin@netbsd.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Martin: this is a different fix to the one I suggested you test,
because I realized we need to make these bits RAZ/WI in the aarch32
FPSCR as well as the aarch64 FPCR, but it should have the same effect.

General note: the difference between "RAZ/WI" and "RES0" is a bit
subtle (see the Arm ARM glossary), but the main distinction is that
RES0 bits can often be implemented as reads-as-written whilst
RAZ/WI bits never can.
---
 target/arm/cpu.h    | 6 ++++++
 target/arm/helper.c | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index b8161cb6d73..15e1464460f 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1404,6 +1404,12 @@ void vfp_set_fpscr(CPUARMState *env, uint32_t val);
 #define FPSR_MASK 0xf800009f
 #define FPCR_MASK 0x07ff9f00
 
+#define FPCR_IOE    (1 << 8)    /* Invalid Operation exception trap enable */
+#define FPCR_DZE    (1 << 9)    /* Divide by Zero exception trap enable */
+#define FPCR_OFE    (1 << 10)   /* Overflow exception trap enable */
+#define FPCR_UFE    (1 << 11)   /* Underflow exception trap enable */
+#define FPCR_IXE    (1 << 12)   /* Inexact exception trap enable */
+#define FPCR_IDE    (1 << 15)   /* Input Denormal exception trap enable */
 #define FPCR_FZ16   (1 << 19)   /* ARMv8.2+, FP16 flush-to-zero */
 #define FPCR_FZ     (1 << 24)   /* Flush-to-zero enable bit */
 #define FPCR_DN     (1 << 25)   /* Default NaN enable bit */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 66faebea8ec..c5f10ddbe92 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -12508,6 +12508,12 @@ void HELPER(vfp_set_fpscr)(CPUARMState *env, uint32_t val)
         val &= ~FPCR_FZ16;
     }
 
+    /*
+     * We don't implement trapped exception handling, so the
+     * trap enable bits are all RAZ/WI (not RES0!)
+     */
+    val &= ~(FPCR_IDE | FPCR_IXE | FPCR_UFE | FPCR_OFE | FPCR_DZE | FPCR_IOE);
+
     changed = env->vfp.xregs[ARM_VFP_FPSCR];
     env->vfp.xregs[ARM_VFP_FPSCR] = (val & 0xffc8ffff);
     env->vfp.vec_len = (val >> 16) & 7;
-- 
2.20.1

Re: [Qemu-devel] [PATCH] target/arm: Make FPSCR/FPCR trapped-exception bits RAZ/WI

[Martin Husemann]

On Thu, Jan 31, 2019 at 01:07:00PM +0000, Peter Maydell wrote:
> Martin: this is a different fix to the one I suggested you test,
> because I realized we need to make these bits RAZ/WI in the aarch32
> FPSCR as well as the aarch64 FPCR, but it should have the same effect.

This one works fine for me too.

Thanks!

Martin

Re: [Qemu-devel] [PATCH] target/arm: Make FPSCR/FPCR trapped-exception bits RAZ/WI

[Richard Henderson]

On 1/31/19 5:07 AM, Peter Maydell wrote:
> The {IOE, DZE, OFE, UFE, IXE, IDE} bits in the FPSCR/FPCR are for
> enabling trapped IEEE floating point exceptions (where IEEE exception
> conditions cause a CPU exception rather than updating the FPSR status
> bits). QEMU doesn't implement this (and nor does the hardware we're
> modelling), but for implementations which don't implement trapped
> exception handling these control bits are supposed to be RAZ/WI.
> This allows guest code to test for whether the feature is present
> by trying to write to the bit and checking whether it sticks.
> 
> QEMU is incorrectly making these bits read as written. Make them
> RAZ/WI as the architecture requires.
> 
> In particular this was causing problems for the NetBSD automatic
> test suite.
> 
> Reported-by: Martin Husemann <martin@netbsd.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Martin: this is a different fix to the one I suggested you test,
> because I realized we need to make these bits RAZ/WI in the aarch32
> FPSCR as well as the aarch64 FPCR, but it should have the same effect.
> 
> General note: the difference between "RAZ/WI" and "RES0" is a bit
> subtle (see the Arm ARM glossary), but the main distinction is that
> RES0 bits can often be implemented as reads-as-written whilst
> RAZ/WI bits never can.
> ---
>  target/arm/cpu.h    | 6 ++++++
>  target/arm/helper.c | 6 ++++++
>  2 files changed, 12 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~