We mark some sysregs with ARM_CP_ALIAS (either manually or automatically) to indicate that they are accessing underlying guest CPU state whose migration and reset is handled by some other register. (For example, typically we make the 32-bit view of a register an alias and handle migration for the 64-bit view.)
Add a debug check that we actually do handle the migration elsewhere: for every sysreg in the hashtable (i.e. which might be visible to the guest), check that some register in the migration list has a field offset that matches. This isn't perfect but it can catch some bugs. In particular it did fire for the various secure banked registers that we were not migrating before the fixes in the previous patches. Because the check is more of a debugging aid than something I'm confident will never produce a false-positive report for some niche combination of CPU and machine options, we only do it for debug builds, and make it a warning printed to the user rather than an assertion failure. Signed-off-by: Peter Maydell <[email protected]> --- target/arm/helper.c | 121 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/target/arm/helper.c b/target/arm/helper.c index b41e07e84b4..82e7a20bdb6 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -230,6 +230,125 @@ bool write_list_to_cpustate(ARMCPU *cpu) return ok; } +#ifdef CONFIG_DEBUG_TCG +/* Data to pass through to check_cpreg() */ +typedef struct CheckCPRegData { + ARMCPU *cpu; + bool warned; +} CheckCPRegData; + +static void check_cpreg(gpointer key, gpointer value, gpointer opaque) +{ + CheckCPRegData *data = opaque; + ARMCPU *cpu = data->cpu; + CPUARMState *env = &cpu->env; + bool match = false; + const ARMCPRegInfo *ri = value; + + if ((ri->type & (ARM_CP_NO_RAW | ARM_CP_CONST)) || + ri->fieldoffset == 0) { + return; + } + + /* + * If we don't have EL3 then the guest can't get at S-only + * registers even if they're in the hashtable. This avoids + * false-positive complaints about e.g. TTBCR_S and DFSR_S: they + * are migrated via ESR_EL3 and TCR_EL3, but we only register those + * _EL3 regs if ARM_FEATURE_EL3. + */ + if (ri->secure == ARM_CP_SECSTATE_S && !arm_feature(env, ARM_FEATURE_EL3)) { + return; + } + + /* + * Some special cases for sysregs that are backed by fields that we + * migrate in the main vmstate, not the cpregs array. + */ + if (ri->fieldoffset >= offsetof(CPUARMState, banked_spsr[BANK_USRSYS]) && + ri->fieldoffset <= offsetof(CPUARMState, banked_spsr[BANK_MON])) { + return; + } + if (ri->fieldoffset >= offsetof(CPUARMState, elr_el[0]) && + ri->fieldoffset <= offsetof(CPUARMState, elr_el[3])) { + return; + } + if (ri->fieldoffset >= offsetof(CPUARMState, sp_el[0]) && + ri->fieldoffset <= offsetof(CPUARMState, sp_el[3])) { + return; + } + if (ri->fieldoffset == offsetof(CPUARMState, vfp.xregs[ARM_VFP_FPEXC])) { + return; + } + + for (int i = 0; i < cpu->cpreg_array_len; i++) { + uint32_t other_regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]); + const ARMCPRegInfo *other_ri = get_arm_cp_reginfo(cpu->cp_regs, + other_regidx); + + if (!other_ri || (other_ri->type & (ARM_CP_NO_RAW | ARM_CP_CONST)) || + other_ri->fieldoffset == 0) { + continue; + } + + /* + * If the field offsets match exactly, or this 32-bit cpreg + * is in the second half of a 64-bit field, consider it to + * be handled. + */ + if (ri->fieldoffset == other_ri->fieldoffset || + (cpreg_field_type(ri) == MO_32 && + cpreg_field_type(other_ri) == MO_64 && + ri->fieldoffset == other_ri->fieldoffset + 4)) { + match = true; + break; + } + } + if (!match) { + warn_report("check_cpreg: no migration entry found for %s", ri->name); + data->warned = true; + } +} +#endif /* CONFIG_DEBUG_TCG */ + +static void arm_check_cpreg_coverage(ARMCPU *cpu) +{ + /* + * Try to catch bugs where we don't actually migrate a cpreg. + * Specifically, here we check that every cpreg in the hash table + * (and thus potentially visible to the guest) which specifies + * a fieldoffset has some entry in the cpreg_indexes[] array that + * handles that same fieldoffset. + * The typical bug that will be flagged up here is if a register + * is marked as an alias of something else, but there isn't actually + * anything else that handles the field. + * + * Note that the checks are more of a best-effort, are somewhat + * expensive at O(n^2) in the number of sysregs, and need some + * cases to be excluded where the sysreg data isn't migrated via + * the cpreg arrays. So we only do them in --enable-debug builds, + * to avoid potentially causing problems for users. + */ +#ifdef CONFIG_DEBUG_TCG + CheckCPRegData data = { + .cpu = cpu, + .warned = false, + }; + + /* We'll only use the cpregs for migration with TCG */ + if (!tcg_enabled()) { + return; + } + g_hash_table_foreach(cpu->cp_regs, check_cpreg, &data); + + if (data.warned) { + error_printf("check_cpreg errors are a QEMU bug that may cause " + "migration to fail. Please report this with the full " + "QEMU command line and version.\n"); + } +#endif +} + static void add_cpreg_to_list(gpointer key, gpointer value, gpointer opaque) { ARMCPU *cpu = opaque; @@ -281,6 +400,8 @@ void arm_init_cpreg_list(ARMCPU *cpu) if (arraylen) { qsort(cpu->cpreg_indexes, arraylen, sizeof(uint64_t), compare_u64); } + + arm_check_cpreg_coverage(cpu); } bool arm_pan_enabled(CPUARMState *env) -- 2.43.0
