Some fields should not be written back to KVM for various reasons: - ID_AA64PFR0_EL1.GIC / ID_PFR1_EL1.GIC are fabricated by KVM based on the gic-version property. - KVM does not allow writing 0 to ID_DFR0_EL1.CopDbg, which breaks booting an AArch64-only guest on a host that also supports AArch32. - ID_DFR0_EL1.PerfMon is populated by KVM even for AArch64-only guests but is not writable there. - MPIDR_EL1 is managed by KVM based on vCPU index. - Hold off writing back CLIDR_EL1 until we properly support exposing cache topology to a KVM guest.
Some registers such as DCZID_EL0 are not exposed through the KVM cpreg list at all, but guest still sees the host value, we verify that vCPU's ID regs values match the host value. To do this, refactor kvm_arm_writable_idregs_to_cpreg_list to kvm_arm_write_idregs_to_cpregs_list, which operates per field and now returns an error if a non-writable field mismatches with the host value. Signed-off-by: Khushit Shah <[email protected]> --- target/arm/cpu-idregs.c | 19 +++++++ target/arm/cpu-idregs.h | 4 ++ target/arm/kvm.c | 116 ++++++++++++++++++++++++++++++++++------ target/arm/trace-events | 2 +- 4 files changed, 125 insertions(+), 16 deletions(-) diff --git a/target/arm/cpu-idregs.c b/target/arm/cpu-idregs.c index c41d0ab0c4..6fa02af0f3 100644 --- a/target/arm/cpu-idregs.c +++ b/target/arm/cpu-idregs.c @@ -7,6 +7,7 @@ * SPDX-License-Identifier: GPL-2.0-or-later */ #include "qemu/osdep.h" +#include "qemu/bitops.h" #include "qemu/error-report.h" #include "qapi/error.h" #include "cpu.h" @@ -94,3 +95,21 @@ ARM64SysReg arm64_id_regs[NUM_ID_IDX] = { #include "cpu-idregs.h.inc" }; +uint64_t arm_get_field_mask(const ARM64SysRegField *field) +{ + return MAKE_64BIT_MASK(field->shift, field->length); +} + +bool arm_field_is_writable(const ARM64SysRegField *field) +{ + uint64_t field_mask = arm_get_field_mask(field); + + return (arm64_id_regs[field->index].writable_mask & field_mask) + == field_mask; +} + +bool field_matches(const ARM64SysRegField *field, ARMIDRegisterIdx index, + const char *name) +{ + return field->index == index && !strcmp(field->name, name); +} diff --git a/target/arm/cpu-idregs.h b/target/arm/cpu-idregs.h index 245f1c8103..d866bd9e0a 100644 --- a/target/arm/cpu-idregs.h +++ b/target/arm/cpu-idregs.h @@ -38,4 +38,8 @@ typedef struct ARM64SysReg { */ extern ARM64SysReg arm64_id_regs[NUM_ID_IDX]; +uint64_t arm_get_field_mask(const ARM64SysRegField *field); +bool arm_field_is_writable(const ARM64SysRegField *field); +bool field_matches(const ARM64SysRegField *field, ARMIDRegisterIdx index, + const char *name); #endif diff --git a/target/arm/kvm.c b/target/arm/kvm.c index 6974e5c551..c38b99cfce 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -951,6 +951,16 @@ static uint64_t *kvm_arm_get_cpreg_ptr(ARMCPU *cpu, uint64_t regidx) return &cpu->cpreg_values[res - cpu->cpreg_indexes]; } +/* Like kvm_arm_get_cpreg_ptr, but returns NULL if the register is not found. */ +static uint64_t *kvm_arm_find_cpreg_ptr(ARMCPU *cpu, uint64_t regidx) +{ + uint64_t *res; + res = bsearch(®idx, cpu->cpreg_indexes, cpu->cpreg_array_len, + sizeof(uint64_t), compare_u64); + + return res ? &cpu->cpreg_values[res - cpu->cpreg_indexes] : NULL; +} + /** * kvm_arm_reg_syncs_via_cpreg_list: * @regidx: KVM register index @@ -1252,32 +1262,105 @@ bool kvm_arm_cpu_post_load(ARMCPU *cpu) return true; } +static bool arm_field_skip_writeback_always(const ARM64SysRegField *field) +{ + /* + * GIC is controlled by the gic-version property and fabricated by KVM + * when the vGIC device is created, so a named CPU model must not touch + * it. KVM also rejects writing 0 to ID_DFR0_EL1.CopDbg, which breaks + * booting a model that lacks AArch32 support on a host that supports + * it. MPIDR_EL1 is managed by KVM based on number of vCPUs, skip + * writing it back. Similarly, skip writing back CLIDR_EL1 till we + * properly support exposing cache for KVM guest. + */ + return field_matches(field, ID_AA64PFR0_EL1_IDX, "GIC") + || field_matches(field, ID_PFR1_EL1_IDX, "GIC") + || field_matches(field, ID_DFR0_EL1_IDX, "CopDbg") + || field->index == MPIDR_EL1_IDX + || field->index == CLIDR_EL1_IDX; +} + +static bool arm_field_skip_writeback_if_not_writable(const ARM64SysRegField *field) +{ + /* + * KVM populates ID_DFR0_EL1.PerfMon even for AArch64-only guests but + * does not expose it as writable there, so skip it when it is not + * writable. + */ + return field_matches(field, ID_DFR0_EL1_IDX, "PerfMon"); +} + /* - * Copy writable ID regs from isar.idregs[] to cpreg_list - * in case their value differs from the original init cpreg value + * Copy writable ID reg fields from isar.idregs[] into the KVM cpreg list, + * so the subsequent write_list_to_kvmstate() pushes them to KVM. + * Only writable fields are copied; fields that must not be written back + * (see arm_field_skip_writeback_*) are skipped. + * Returns -1 if any vCPU's ID reg value differs from the host value and the + * field is not writable. */ -static void kvm_arm_writable_idregs_to_cpreg_list(ARMCPU *cpu) +static int kvm_arm_write_idregs_to_cpreg_list(ARMCPU *cpu) { for (int i = 0; i < NUM_ID_IDX; i++) { ARM64SysReg *sysregdesc = &arm64_id_regs[i]; ARMSysRegs sysreg = id_register_sysreg[i]; - uint64_t previous, new; + uint64_t writable_mask = sysregdesc->writable_mask; + uint64_t desired = cpu->isar.idregs[i]; + uint64_t previous, updated; uint64_t *cpreg; - if (!sysregdesc->writable_mask) { + cpreg = kvm_arm_find_cpreg_ptr(cpu, idregs_sysreg_to_kvm_reg(sysreg)); + /* + * Registers such as DCZID_EL0 are not exposed through the cpreg + * list and are therefore not writable. Use the host value + * snapshotted at probe time as the reference to check the vCPU ID + * regs have the same value. + */ + previous = cpreg ? *cpreg : arm_host_cpu_features.isar.idregs[i]; + + if (previous == desired) { continue; } - cpreg = kvm_arm_get_cpreg_ptr(cpu, idregs_sysreg_to_kvm_reg(sysreg)); - previous = *cpreg; - new = cpu->isar.idregs[i]; + for (int j = 0; j < sysregdesc->fields_count; j++) { + const ARM64SysRegField *field = &sysregdesc->fields[j]; + uint64_t field_mask = arm_get_field_mask(field); + uint64_t prev_val = previous & field_mask; + uint64_t new_val = desired & field_mask; + + if (prev_val == new_val) { + continue; + } + + if (arm_field_skip_writeback_always(field) || + (!arm_field_is_writable(field) && + arm_field_skip_writeback_if_not_writable(field))) { + /* Never write this field back; keep KVM's value. */ + writable_mask &= ~field_mask; + continue; + } + + if (!arm_field_is_writable(field)) { + error_report("%s.%s is not writable: host=0x%" PRIx64 + ", requested=0x%" PRIx64, + sysregdesc->name, field->name, + prev_val >> field->shift, new_val >> field->shift); + return -1; + } + } + + if (!cpreg || !writable_mask) { + continue; + } - if (previous != new) { - *cpreg = new; - trace_kvm_arm_writable_idregs_to_cpreg_list(sysregdesc->name, - previous, new); - } + updated = (previous & ~writable_mask) | (desired & writable_mask); + if (updated != previous) { + *cpreg = updated; + trace_kvm_arm_write_idregs_to_cpreg_list(sysregdesc->name, + previous, updated); + } } + + return 0; } void kvm_arm_reset_vcpu(ARMCPU *cpu) @@ -2235,8 +2318,11 @@ int kvm_arch_init_vcpu(CPUState *cs) if (ret) { return ret; } - /* overwrite writable ID regs with their updated property values */ - kvm_arm_writable_idregs_to_cpreg_list(cpu); + /* overwrite ID reg fields with their updated property values */ + ret = kvm_arm_write_idregs_to_cpreg_list(cpu); + if (ret) { + return ret; + } ret = write_list_to_kvmstate(cpu, KVM_PUT_FULL_STATE); if (!ret) { return -1; diff --git a/target/arm/trace-events b/target/arm/trace-events index f33b0d821d..f042ab59b8 100644 --- a/target/arm/trace-events +++ b/target/arm/trace-events @@ -14,7 +14,7 @@ arm_gt_update_irq(int timer, int irqstate) "gt_update_irq: timer %d irqstate %d" # kvm.c kvm_arm_fixup_msi_route(uint64_t iova, uint64_t gpa) "MSI iova = 0x%"PRIx64" is translated into 0x%"PRIx64 get_host_cpu_idregs(const char *name, uint64_t value) "scratch vcpu host value for %s is 0x%"PRIx64 -kvm_arm_writable_idregs_to_cpreg_list(const char *name, uint64_t previous, uint64_t new) "%s overwrite default 0x%"PRIx64" with 0x%"PRIx64 +kvm_arm_write_idregs_to_cpreg_list(const char *name, uint64_t previous, uint64_t new) "%s overwrite default 0x%"PRIx64" with 0x%"PRIx64 # cpu64.c get_sysreg_prop(const char *name, uint64_t value) "%s 0x%"PRIx64 -- 2.52.0
