This patchset fixes a very long standing bug where we didn't migrate
the values of the Secure banked copy of an AArch32 banked coprocessor
register (https://gitlab.com/qemu-project/qemu/-/work_items/467).
This is something that Tao Ding sent a patch for back in July:
https://lore.kernel.org/qemu-devel/[email protected]/
I didn't think that patch was quite right, but digging into it I found
a slightly wider set of problems than I expected, plus testing on
a bunch of configs was a bit tedious, so it's taken me a while to
get something ready to send out. Sorry for the delay...
The underlying cause of the bug is that we store registers in our
cpreg hashtable with a key with one format (32 bit, QEMU specific),
but store them in the (index, value) lists for migration with an
index of a different format (64 bit, same as KVM uses). Because KVM
never supports EL3 its index format doesn't have a way to say "this
is the Secure banked copy of this cpreg"; only our QEMU specific
index does. The conversion functions kvm_to_cpreg_id() and
cpreg_to_kvm_id() were written to assume that the KVM index was
always for a NonSecure register.
The current code doesn't touch the CP_REG_AA32_NS_MASK bit when
converting from the 32-bit ID format to a KVM ID, and it forces the
NS bit to 1 when converting from a KVM ID to a cpreg. The effect of
this on migration is that the indexes in cpreg_indexes[] have NS = 0
for the Secure banked registers and NS = 1 for the NS banked
registers, but when we read and write the values into the CPUState by
finding the register using kvm_to_cpreg_id(cpreg_indexes[i]) we
always find the NS register, and so the S register values aren't
actually migrated.
This went unnoticed most of the time because (a) most setups don't
enable EL3 at all (b) AArch64 is unaffected (c) most users don't use
migration (d) the case where we direct boot a Linux kernel in NS and
the S regs remain at their reset values happened to work by luck.
But it breaks migration of a guest which is running in S, such as
Linux or u-boot on the imx boards.
Patch summary:
* Patch 1 is the fix for this bug; I included keeping migration
compat from old QEMU here for the benefit of that case (d) that
used to work.
* Patch 2 fixes a corner case for AArch32 v8 CPUs where we wouldn't
migrate some secure banked registers because we incorrectly
assumed an AArch64 regdef would do the job
* Patches 3 and 4 are minor tidyup
* Patches 5, 6, 7 fix some v7R and v8R bugs where we were exposing
cpregs and the ATS insns when we should not (noticed because
this bug caused the debug check in patch 7 to fire)
* patch 8 adds a debug check when QEMU is compiled in debug that
every cpreg that we claimed is an ARM_CP_ALIAS to something
else really does have some other cpreg handling the migration
of its CPU state struct field. It's a bit heuristic but it did
successfully report the missing migration handling for the
bug fixed in patch 1, as well as the other things which
patches 2-7 fix.
thanks
-- PMM
Peter Maydell (8):
target/arm: Keep the S/NS bit in migration data
target/arm: Don't assume secure-banked registers are migrated by
64-bit regs
target/arm: Drop uses of reset_ignore for banked AArch32 registers
target/arm: Don't use arm_cp_reset_ignore when fieldoffset is 0
target/arm: Move ESR_EL1 definition to vmsa_pmsa_cp_reginfo
target/arm: Don't set ARM_FEATURE_VAPA for v7 PMSA
target/arm: Don't register 64-bit TTBR0, TTBR1 for PMSA
target/arm: Add a debug check that all sysregs are migrated
target/arm/cpregs.h | 21 +++--
target/arm/cpu.c | 6 +-
target/arm/cpu.h | 6 ++
target/arm/helper.c | 198 ++++++++++++++++++++++++++++++++++++-------
target/arm/machine.c | 80 +++++++++++++++++
5 files changed, 273 insertions(+), 38 deletions(-)
--
2.43.0
