diff mbox series

[2/3] target/arm: Don't set ISV when reporting stage 1 faults in ESR_EL2

Message ID 20230331145045.2584941-3-peter.maydell@linaro.org
State Superseded
Headers show
Series target/arm: Fix ESR_EL2 buglet, implement FEAT_PAN3 | expand

Commit Message

Peter Maydell March 31, 2023, 2:50 p.m. UTC 
The syndrome value reported to ESR_EL2 should only contain the
detailed instruction syndrome information when the fault has been
caused by a stage 2 abort, not when the fault was a stage 1 abort
(i.e.  caused by execution at EL2).  We were getting this wrong and
reporting the detailed ISV information all the time.

Fix the bug by checking fi->stage2.  Add a TODO comment noting the
cases where we'll have to come back and revisit this when we
implement FEAT_LS64 and friends.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/tlb_helper.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Richard Henderson March 31, 2023, 3:52 p.m. UTC | #1 
On 3/31/23 07:50, Peter Maydell wrote:
> The syndrome value reported to ESR_EL2 should only contain the
> detailed instruction syndrome information when the fault has been
> caused by a stage 2 abort, not when the fault was a stage 1 abort
> (i.e.  caused by execution at EL2).  We were getting this wrong and
> reporting the detailed ISV information all the time.
> 
> Fix the bug by checking fi->stage2.  Add a TODO comment noting the
> cases where we'll have to come back and revisit this when we
> implement FEAT_LS64 and friends.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/tcg/tlb_helper.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)

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

r~
diff mbox series

Patch

diff --git a/target/arm/tcg/tlb_helper.c b/target/arm/tcg/tlb_helper.c
index 1a61adb8a68..d5a89bc5141 100644
--- a/target/arm/tcg/tlb_helper.c
+++ b/target/arm/tcg/tlb_helper.c
@@ -32,8 +32,9 @@  static inline uint32_t merge_syn_data_abort(uint32_t template_syn,
     uint32_t syn;
 
     /*
-     * ISV is only set for data aborts routed to EL2 and
-     * never for stage-1 page table walks faulting on stage 2.
+     * ISV is only set for stage-2 data aborts routed to EL2 and
+     * never for stage-1 page table walks faulting on stage 2
+     * or for stage-1 faults.
      *
      * Furthermore, ISV is only set for certain kinds of load/stores.
      * If the template syndrome does not have ISV set, we should leave
@@ -42,8 +43,14 @@  static inline uint32_t merge_syn_data_abort(uint32_t template_syn,
      * See ARMv8 specs, D7-1974:
      * ISS encoding for an exception from a Data Abort, the
      * ISV field.
+     *
+     * TODO: FEAT_LS64/FEAT_LS64_V/FEAT_SL64_ACCDATA: Translation,
+     * Access Flag, and Permission faults caused by LD64B, ST64B,
+     * ST64BV, or ST64BV0 insns report syndrome info even for stage-1
+     * faults and regardless of the target EL.
      */
-    if (!(template_syn & ARM_EL_ISV) || target_el != 2 || fi->s1ptw) {
+    if (!(template_syn & ARM_EL_ISV) || target_el != 2
+        || fi->s1ptw || !fi->stage2) {
         syn = syn_data_abort_no_iss(same_el, 0,
                                     fi->ea, 0, fi->s1ptw, is_write, fsc);
     } else {