Hej folks,
I'm looking into supporting migration between 2 Ampere Altra (Max)
machines (using Neoverse-N1). They are almost identical regarding
their feature id register state except for CTR_EL0.DIC which is set
on one machine but not the other.
CTR_EL0 is currently marked as invariant and migrating a VM between
those 2 machines using qemu fails.
Changes RFC [0] -> V1 [1]:
* store the emulated value per VM and not per VCPU
* allow to change more values than just the DIC bit
* only trap guest access to that reg when needed
* make sure to not present the guest with an inconsistent register set
Changes V1 -> V2 [2]:
* implemented Marc's suggestion for keeping registers consistent while
not breaking userspace ABI / expectations (I hope correctly this time)
* keep the shadowed value valid at all time
* unify the code to setup traps
Changes V2 -> V3 [3]:
* rebased to kvm-arm-next (to include Olivers idreg fixes)
* fixed VM ops trapping for non-FWB CPUs
* fixed writable mask for CLIDR_EL1
* re-added manual ctr validation (using arm64_check_features() had a
side effect with the way .reset is working for these registers)
* added a testcase
Changes V3 -> V4:
* incorporated feedback from Shaoqin and Eric
Thanks,
Sebastian
[0]: https://lore.kernel.org/all/[email protected]/T/
[1]: https://lore.kernel.org/lkml/[email protected]/T/
[2]: https://lore.kernel.org/lkml/[email protected]/T/
[3]: https://lore.kernel.org/lkml/[email protected]/T/
Sebastian Ott (6):
KVM: arm64: unify code to prepare traps
KVM: arm64: maintain per VM value for CTR_EL0
KVM: arm64: add emulation for CTR_EL0 register
KVM: arm64: show writable masks for feature registers
KVM: arm64: rename functions for invariant sys regs
KVM: selftests: arm64: Test writes to CTR_EL0
arch/arm64/include/asm/kvm_emulate.h | 40 +---
arch/arm64/include/asm/kvm_host.h | 4 +-
arch/arm64/kvm/arm.c | 2 +-
arch/arm64/kvm/sys_regs.c | 214 ++++++++++++++----
.../selftests/kvm/aarch64/set_id_regs.c | 16 ++
5 files changed, 201 insertions(+), 75 deletions(-)
--
2.42.0
In preparation for CTR_EL0 emulation maintain a per VM value for this
register and use it where appropriate.
Signed-off-by: Sebastian Ott <[email protected]>
Reviewed-by: Shaoqin Huang <[email protected]>
---
arch/arm64/include/asm/kvm_host.h | 2 ++
arch/arm64/kvm/sys_regs.c | 21 ++++++++++++++-------
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 212ae77eefaf..1259be5e2f3e 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -331,6 +331,8 @@ struct kvm_arch {
#define KVM_ARM_ID_REG_NUM (IDREG_IDX(sys_reg(3, 0, 0, 7, 7)) + 1)
u64 id_regs[KVM_ARM_ID_REG_NUM];
+ u64 ctr_el0;
+
/* Masks for VNCR-baked sysregs */
struct kvm_sysreg_masks *sysreg_masks;
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 41741bf4d2b2..0213c96f73f2 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -219,9 +219,9 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
* Returns the minimum line size for the selected cache, expressed as
* Log2(bytes).
*/
-static u8 get_min_cache_line_size(bool icache)
+static u8 get_min_cache_line_size(struct kvm *kvm, bool icache)
{
- u64 ctr = read_sanitised_ftr_reg(SYS_CTR_EL0);
+ u64 ctr = kvm->arch.ctr_el0;
u8 field;
if (icache)
@@ -248,7 +248,7 @@ static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr)
if (vcpu->arch.ccsidr)
return vcpu->arch.ccsidr[csselr];
- line_size = get_min_cache_line_size(csselr & CSSELR_EL1_InD);
+ line_size = get_min_cache_line_size(vcpu->kvm, csselr & CSSELR_EL1_InD);
/*
* Fabricate a CCSIDR value as the overriding value does not exist.
@@ -283,7 +283,7 @@ static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
u32 i;
if ((val & CCSIDR_EL1_RES0) ||
- line_size < get_min_cache_line_size(csselr & CSSELR_EL1_InD))
+ line_size < get_min_cache_line_size(vcpu->kvm, csselr & CSSELR_EL1_InD))
return -EINVAL;
if (!ccsidr) {
@@ -1886,7 +1886,7 @@ static bool access_ctr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
if (p->is_write)
return write_to_read_only(vcpu, p, r);
- p->regval = read_sanitised_ftr_reg(SYS_CTR_EL0);
+ p->regval = vcpu->kvm->arch.ctr_el0;
return true;
}
@@ -1906,7 +1906,7 @@ static bool access_clidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
*/
static u64 reset_clidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
{
- u64 ctr_el0 = read_sanitised_ftr_reg(SYS_CTR_EL0);
+ u64 ctr_el0 = vcpu->kvm->arch.ctr_el0;
u64 clidr;
u8 loc;
@@ -1959,8 +1959,8 @@ static u64 reset_clidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
static int set_clidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
u64 val)
{
- u64 ctr_el0 = read_sanitised_ftr_reg(SYS_CTR_EL0);
u64 idc = !CLIDR_LOC(val) || (!CLIDR_LOUIS(val) && !CLIDR_LOUU(val));
+ u64 ctr_el0 = vcpu->kvm->arch.ctr_el0;
if ((val & CLIDR_EL1_RES0) || (!(ctr_el0 & CTR_EL0_IDC) && idc))
return -EINVAL;
@@ -3557,6 +3557,13 @@ void kvm_reset_sys_regs(struct kvm_vcpu *vcpu)
struct kvm *kvm = vcpu->kvm;
unsigned long i;
+ if (!kvm_vcpu_initialized(vcpu))
+ /*
+ * Make sure CTR_EL0 is initialized before registers
+ * that depend on it are reset.
+ */
+ kvm->arch.ctr_el0 = read_sanitised_ftr_reg(SYS_CTR_EL0);
+
for (i = 0; i < ARRAY_SIZE(sys_reg_descs); i++) {
const struct sys_reg_desc *r = &sys_reg_descs[i];
--
2.42.0
Test that CTR_EL0 is modifiable from userspace, that changes are
visible to guests, and that they are preserved across a vCPU reset.
Signed-off-by: Sebastian Ott <[email protected]>
Reviewed-by: Eric Auger <[email protected]>
---
.../testing/selftests/kvm/aarch64/set_id_regs.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tools/testing/selftests/kvm/aarch64/set_id_regs.c b/tools/testing/selftests/kvm/aarch64/set_id_regs.c
index a7de39fa2a0a..9583c04f1228 100644
--- a/tools/testing/selftests/kvm/aarch64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/aarch64/set_id_regs.c
@@ -219,6 +219,7 @@ static void guest_code(void)
GUEST_REG_SYNC(SYS_ID_AA64MMFR1_EL1);
GUEST_REG_SYNC(SYS_ID_AA64MMFR2_EL1);
GUEST_REG_SYNC(SYS_ID_AA64ZFR0_EL1);
+ GUEST_REG_SYNC(SYS_CTR_EL0);
GUEST_DONE();
}
@@ -490,11 +491,25 @@ static void test_clidr(struct kvm_vcpu *vcpu)
test_reg_vals[encoding_to_range_idx(SYS_CLIDR_EL1)] = clidr;
}
+static void test_ctr(struct kvm_vcpu *vcpu)
+{
+ u64 ctr;
+
+ vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CTR_EL0), &ctr);
+ ctr &= ~CTR_EL0_DIC_MASK;
+ if (ctr & CTR_EL0_IminLine_MASK)
+ ctr--;
+
+ vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CTR_EL0), ctr);
+ test_reg_vals[encoding_to_range_idx(SYS_CTR_EL0)] = ctr;
+}
+
static void test_vcpu_ftr_id_regs(struct kvm_vcpu *vcpu)
{
u64 val;
test_clidr(vcpu);
+ test_ctr(vcpu);
vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1), &val);
val++;
@@ -525,6 +540,7 @@ static void test_reset_preserves_id_regs(struct kvm_vcpu *vcpu)
test_assert_id_reg_unchanged(vcpu, test_regs[i].reg);
test_assert_id_reg_unchanged(vcpu, SYS_CLIDR_EL1);
+ test_assert_id_reg_unchanged(vcpu, SYS_CTR_EL0);
ksft_test_result_pass("%s\n", __func__);
}
--
2.42.0
Hi Marc, Oliver
anything else I should change here?
Sebastian