On 10/26/21 9:38 AM, Alexander Graf wrote:
On 10/26/21 12:12 AM, Alexander Graf wrote:
+        if (cm) {
+            /* We don't cache MMIO regions */
+            advance_pc = true;
+            break;
+        }
+
          assert(isv);

The assert should come first.  If the "iss valid" bit is not set, then nothing 
else in the word is defined.

Otherwise,
Reviewed-by: Richard Henderson <richard.hender...@linaro.org>

Yes, but isv=0 for cm=1. And even in other isv=0 situations most other fields 
are valid (post-idx provides the correct va/pa too for example).

Does cm=1 really give you isv=1 on other hardware?

Ah hah.  From 0487G.a, page D13-3191:

# For other faults reported in ESR_EL2, ISV is 0 except
# for the following stage 2 aborts...

(which incidentally sounds like documenting around a historic chip bug, since both EL1 and EL3 do get ISV set). And of course HVF would be passing along the EL2 version, being the hypervisor.

I guess the assert can stand where it is, because everything below should be one of those "following stage 2 aborts". The only thing that could be improved is a bit of commentary there at the assert.

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


r~

Reply via email to