Currently get_phys_addr() and its various subfunctions return a
hard-coded fault status register value for translation failures. This
is awkward because FSR values these days may be either long-descriptor
format or short-descriptor format. Worse, the right FSR type to use
doesn't depend only on the translation table being walked -- some
cases, like fault info reported to AArch32 EL2 for some kinds of ATS
operation, must be in long-descriptor format even if the translation
table being walked was short format. We can't get those cases right
with our current approach. (We put in a bodge fix so we could at
least run Xen, in commit 50cd71b0d347c7.)
This series does a refactoring of get_phys_addr() so that instead
of returning FSR values it puts information in the existing
ARMMMUFaultInfo structure that is sufficient to construct an
FSR value, and the callers then do that using the right condition
for whatever they're doing. I've included Edgar's patch from back
in June that fixes the handling of ATS instructions, which is most
of the point of the refactoring.
Rather than doing it all in one massive unreviewable patch, the
series is structured as:
* patch 1: add fields to ARMMMUFaultInfo, and the utility functions
that return a long or short format FSR given a fault info struct
* patch 2: stop passing an fsr argument to arm_ld*_ptw(), since
nothing actually looks at the result. (This breaks a cycle
arm_ldq_ptw()->S1_ptw_translate()->get_phys_addr_lpae()->arm_ldq_ptw()
which would otherwise make a stepwise refactoring awkward.)
* patches 3 to 8: for each of the different subfunctions of
get_phys_addr(), make them fill in the fault info fields instead
of an fsr value. Temporarily we add calls to arm_fi_to_sfsc()
and arm_fi_to_lfsc() at the callsites in get_phys_addr(), since
the callers of get_phys_addr() still need the fsr values. These
will go away again in a later patch.
* patches 9 and 10: change the callers of get_phys_addr() to use
the info in the fault info struct and ignore the fsr value.
* patch 11: remove the now unused fsr argument (and the temporary
calls to arm_fi_to_sfsc()/arm_fi_to_lfsc())
* patch 12: Edgar's patch for ATS PAR format
Arguably it would be good to push the "create the FSR value"
logic further, into the point where the exception is taken.
(This would let us correct the oddity where for M profile we
get an A/R profile FSR value in arm_v7m_cpu_do_interrupt()
which we then have to convert into the appropriate M profile
fault status bits.) However, migration backcompat makes this
tricky, because at the moment we migrate env->exception.fsr,
and so we'd need to have code just to handle inbound migrations
from old QEMU with an FSR and reconstitute a fault info struct.
So I've stopped here, at least for now.
This whole series sits on top of my v8M TT patchset (which also
had to touch some of the get_phys_addr() code). I've pushed it
to this git branch if that's more convenient:
https://git.linaro.org/people/peter.maydell/qemu-arm.git fsr-in-faultinfo
I have tested a bit (including a Linux KVM EL2 setup), but more
testing would definitely be useful. Stefano: in particular it would
be good to check this hasn't broken Xen again :-)
Based-on: 1512153879-5291-1-git-send-email-peter.mayd...@linaro.org
([PATCH 0/7] armv8m: Implement TT, and other bugfixes)
thanks
-- PMM
Edgar E. Iglesias (1):
target/arm: Extend PAR format determination
Peter Maydell (11):
target/arm: Provide fault type enum and FSR conversion functions
target/arm: Remove fsr argument from arm_ld*_ptw()
target/arm: Convert get_phys_addr_v5() to not return FSC values
target/arm: Convert get_phys_addr_v6() to not return FSC values
target/arm: Convert get_phys_addr_lpae() to not return FSC values
target/arm: Convert get_phys_addr_pmsav5() to not return FSC values
target/arm: Convert get_phys_addr_pmsav7() to not return FSC values
target/arm: Convert get_phys_addr_pmsav8() to not return FSC values
target/arm: Use ARMMMUFaultInfo in deliver_fault()
target/arm: Ignore fsr from get_phys_addr() in do_ats_write()
target/arm: Remove fsr argument from get_phys_addr() and
arm_tlb_fill()
target/arm/internals.h | 187 +++++++++++++++++++++++++++++++++++++-
target/arm/helper.c | 238 +++++++++++++++++++++++++++----------------------
target/arm/op_helper.c | 82 +++++------------
3 files changed, 342 insertions(+), 165 deletions(-)
--
2.7.4
