| Message ID | 20230801152007.337272-11-jingzhangos@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable writable for idregs DFR0,PFR0, MMFR{0,1,2,3} | expand |
Hi Jing, On Tue, Aug 01, 2023 at 08:20:06AM -0700, Jing Zhang wrote: > Add tests to verify setting ID registers from userapce is handled > correctly by KVM. Also add a test case to use ioctl > KVM_ARM_GET_FEATURE_ID_WRITABLE_MASKS to get writable masks. > > Signed-off-by: Jing Zhang <jingzhangos@google.com> > --- > tools/arch/arm64/include/uapi/asm/kvm.h | 25 +++ > tools/include/uapi/linux/kvm.h | 2 + Why is this diff needed? I thought we wound up using the latest headers from the kernel. > tools/testing/selftests/kvm/Makefile | 1 + Need to add your file to .gitignore too. > .../selftests/kvm/aarch64/set_id_regs.c | 191 ++++++++++++++++++ > 4 files changed, 219 insertions(+) > create mode 100644 tools/testing/selftests/kvm/aarch64/set_id_regs.c [...] > diff --git a/tools/testing/selftests/kvm/aarch64/set_id_regs.c b/tools/testing/selftests/kvm/aarch64/set_id_regs.c > new file mode 100644 > index 000000000000..9c8f439ac7b3 > --- /dev/null > +++ b/tools/testing/selftests/kvm/aarch64/set_id_regs.c > @@ -0,0 +1,191 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * set_id_regs - Test for setting ID register from usersapce. > + * > + * Copyright (c) 2023 Google LLC. > + * > + * > + * Test that KVM supports setting ID registers from userspace and handles the > + * feature set correctly. > + */ > + > +#include <stdint.h> > +#include "kvm_util.h" > +#include "processor.h" > +#include "test_util.h" > +#include <linux/bitfield.h> > + > +#define field_get(_mask, _reg) (((_reg) & (_mask)) >> (ffsl(_mask) - 1)) > +#define field_prep(_mask, _val) (((_val) << (ffsl(_mask) - 1)) & (_mask)) > + Shadowing the naming of the kernel's own FIELD_{GET,PREP}() is a bit awkward. I'm guessing that you're working around @_mask not being a compile-time constant? > +struct reg_feature { > + uint64_t reg; > + uint64_t ftr_mask; > +}; > + > +static void guest_code(void) > +{ > + for (;;) > + GUEST_SYNC(0); > +} The test should check that the written values are visible both from the guest as well as userspace. > +static struct reg_feature lower_safe_reg_ftrs[] = { > + { KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), ARM64_FEATURE_MASK(ID_AA64DFR0_WRPS) }, > + { KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), ARM64_FEATURE_MASK(ID_AA64PFR0_EL3) }, > + { KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR0_EL1), ARM64_FEATURE_MASK(ID_AA64MMFR0_FGT) }, > + { KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR1_EL1), ARM64_FEATURE_MASK(ID_AA64MMFR1_PAN) }, > + { KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR2_EL1), ARM64_FEATURE_MASK(ID_AA64MMFR2_FWB) }, > +}; My preference would be to organize the field descriptors by register rather than the policy. This matches what the kernel does in cpufeature.c quite closely and allows us to easily reason about which fields are/aren't tested. > +static void test_user_set_lower_safe(struct kvm_vcpu *vcpu) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(lower_safe_reg_ftrs); i++) { > + struct reg_feature *reg_ftr = lower_safe_reg_ftrs + i; > + uint64_t val, new_val, ftr; > + > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > + ftr = field_get(reg_ftr->ftr_mask, val); > + > + /* Set a safe value for the feature */ > + if (ftr > 0) > + ftr--; > + > + val &= ~reg_ftr->ftr_mask; > + val |= field_prep(reg_ftr->ftr_mask, ftr); > + > + vcpu_set_reg(vcpu, reg_ftr->reg, val); > + vcpu_get_reg(vcpu, reg_ftr->reg, &new_val); > + ASSERT_EQ(new_val, val); > + } > +} > + > +static void test_user_set_fail(struct kvm_vcpu *vcpu) > +{ > + int i, r; > + > + for (i = 0; i < ARRAY_SIZE(lower_safe_reg_ftrs); i++) { > + struct reg_feature *reg_ftr = lower_safe_reg_ftrs + i; > + uint64_t val, old_val, ftr; > + > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > + ftr = field_get(reg_ftr->ftr_mask, val); > + > + /* Set a invalid value (too big) for the feature */ > + if (ftr >= GENMASK_ULL(ARM64_FEATURE_FIELD_BITS - 1, 0)) > + continue; This assumes that the fields in the register are unsigned, but there are several which are not. > + ftr++; > + > + old_val = val; > + val &= ~reg_ftr->ftr_mask; > + val |= field_prep(reg_ftr->ftr_mask, ftr); > + > + r = __vcpu_set_reg(vcpu, reg_ftr->reg, val); > + TEST_ASSERT(r < 0 && errno == EINVAL, > + "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno); > + > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > + ASSERT_EQ(val, old_val); > + } > +} > + > +static struct reg_feature exact_reg_ftrs[] = { > + /* Items will be added when there is appropriate field of type > + * FTR_EXACT enabled writing from userspace later. > + */ > +}; > + > +static void test_user_set_exact(struct kvm_vcpu *vcpu) > +{ > + int i, r; > + > + for (i = 0; i < ARRAY_SIZE(exact_reg_ftrs); i++) { > + struct reg_feature *reg_ftr = exact_reg_ftrs + i; > + uint64_t val, old_val, ftr; > + > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > + ftr = field_get(reg_ftr->ftr_mask, val); > + old_val = val; > + > + /* Exact match */ > + vcpu_set_reg(vcpu, reg_ftr->reg, val); > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > + ASSERT_EQ(val, old_val); > + > + /* Smaller value */ > + if (ftr > 0) { > + ftr--; > + val &= ~reg_ftr->ftr_mask; > + val |= field_prep(reg_ftr->ftr_mask, ftr); > + r = __vcpu_set_reg(vcpu, reg_ftr->reg, val); > + TEST_ASSERT(r < 0 && errno == EINVAL, > + "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno); > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > + ASSERT_EQ(val, old_val); > + ftr++; > + } > + > + /* Bigger value */ > + ftr++; > + val &= ~reg_ftr->ftr_mask; > + val |= field_prep(reg_ftr->ftr_mask, ftr); > + r = __vcpu_set_reg(vcpu, reg_ftr->reg, val); > + TEST_ASSERT(r < 0 && errno == EINVAL, > + "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno); > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > + ASSERT_EQ(val, old_val); > + } > +} Don't add dead code, this can be added when we actually test FTR_EXACT fields. Are there not any in the registers exposed by this series? > +static uint32_t writable_regs[] = { > + SYS_ID_DFR0_EL1, > + SYS_ID_AA64DFR0_EL1, > + SYS_ID_AA64PFR0_EL1, > + SYS_ID_AA64MMFR0_EL1, > + SYS_ID_AA64MMFR1_EL1, > + SYS_ID_AA64MMFR2_EL1, > +}; > + > +void test_user_get_writable_masks(struct kvm_vm *vm) > +{ > + struct feature_id_writable_masks masks; > + > + vm_ioctl(vm, KVM_ARM_GET_FEATURE_ID_WRITABLE_MASKS, &masks); > + > + for (int i = 0; i < ARRAY_SIZE(writable_regs); i++) { > + uint32_t reg = writable_regs[i]; > + int idx = ARM64_FEATURE_ID_SPACE_IDX(sys_reg_Op0(reg), > + sys_reg_Op1(reg), sys_reg_CRn(reg), > + sys_reg_CRm(reg), sys_reg_Op2(reg)); > + > + ASSERT_EQ(masks.mask[idx], GENMASK_ULL(63, 0)); > + } > +} The more robust test would be to check that every field this test knows is writable is actually advertised as such in the ioctl. So you could fetch this array at the start of the entire test and pass it through to the routines that do granular checks against the fields of every register. It'd also be good to see basic sanity tests on the ioctl (i.e. call fails if ::rsvd is nonzero), since KVM has screwed that up on several occasions in past. > +int main(void) > +{ > + struct kvm_vcpu *vcpu; > + struct kvm_vm *vm; > + > + vm = vm_create_with_one_vcpu(&vcpu, guest_code); > + > + ksft_print_header(); > + ksft_set_plan(4); > + > + test_user_get_writable_masks(vm); > + ksft_test_result_pass("test_user_get_writable_masks\n"); > + > + test_user_set_exact(vcpu); > + ksft_test_result_pass("test_user_set_exact\n"); > + > + test_user_set_fail(vcpu); > + ksft_test_result_pass("test_user_set_fail\n"); > + > + test_user_set_lower_safe(vcpu); > + ksft_test_result_pass("test_user_set_lower_safe\n"); > + > + kvm_vm_free(vm); > + > + ksft_finished(); > +} > -- > 2.41.0.585.gd2178a4bd4-goog >
Hi Oliver, On Wed, Aug 2, 2023 at 10:28 AM Oliver Upton <oliver.upton@linux.dev> wrote: > > Hi Jing, > > On Tue, Aug 01, 2023 at 08:20:06AM -0700, Jing Zhang wrote: > > Add tests to verify setting ID registers from userapce is handled > > correctly by KVM. Also add a test case to use ioctl > > KVM_ARM_GET_FEATURE_ID_WRITABLE_MASKS to get writable masks. > > > > Signed-off-by: Jing Zhang <jingzhangos@google.com> > > --- > > tools/arch/arm64/include/uapi/asm/kvm.h | 25 +++ > > tools/include/uapi/linux/kvm.h | 2 + > > Why is this diff needed? I thought we wound up using the latest headers > from the kernel. You are right. These changes are not needed. Will drop them. > > > tools/testing/selftests/kvm/Makefile | 1 + > > Need to add your file to .gitignore too. Will do. > > > .../selftests/kvm/aarch64/set_id_regs.c | 191 ++++++++++++++++++ > > 4 files changed, 219 insertions(+) > > create mode 100644 tools/testing/selftests/kvm/aarch64/set_id_regs.c > > [...] > > > diff --git a/tools/testing/selftests/kvm/aarch64/set_id_regs.c b/tools/testing/selftests/kvm/aarch64/set_id_regs.c > > new file mode 100644 > > index 000000000000..9c8f439ac7b3 > > --- /dev/null > > +++ b/tools/testing/selftests/kvm/aarch64/set_id_regs.c > > @@ -0,0 +1,191 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * set_id_regs - Test for setting ID register from usersapce. > > + * > > + * Copyright (c) 2023 Google LLC. > > + * > > + * > > + * Test that KVM supports setting ID registers from userspace and handles the > > + * feature set correctly. > > + */ > > + > > +#include <stdint.h> > > +#include "kvm_util.h" > > +#include "processor.h" > > +#include "test_util.h" > > +#include <linux/bitfield.h> > > + > > +#define field_get(_mask, _reg) (((_reg) & (_mask)) >> (ffsl(_mask) - 1)) > > +#define field_prep(_mask, _val) (((_val) << (ffsl(_mask) - 1)) & (_mask)) > > + > > Shadowing the naming of the kernel's own FIELD_{GET,PREP}() is a bit > awkward. I'm guessing that you're working around @_mask not being a > compile-time constant? Yes, they are used to work around @_mask not being a compile-time constant. As we discussed offline, I'll port automatic system registers definition generation in selftests and use those definition instead. > > > +struct reg_feature { > > + uint64_t reg; > > + uint64_t ftr_mask; > > +}; > > + > > +static void guest_code(void) > > +{ > > + for (;;) > > + GUEST_SYNC(0); > > +} > > The test should check that the written values are visible both from the > guest as well as userspace. Sure. Will add checks in guest too. > > > +static struct reg_feature lower_safe_reg_ftrs[] = { > > + { KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), ARM64_FEATURE_MASK(ID_AA64DFR0_WRPS) }, > > + { KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), ARM64_FEATURE_MASK(ID_AA64PFR0_EL3) }, > > + { KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR0_EL1), ARM64_FEATURE_MASK(ID_AA64MMFR0_FGT) }, > > + { KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR1_EL1), ARM64_FEATURE_MASK(ID_AA64MMFR1_PAN) }, > > + { KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR2_EL1), ARM64_FEATURE_MASK(ID_AA64MMFR2_FWB) }, > > +}; > > My preference would be to organize the field descriptors by register > rather than the policy. This matches what the kernel does in cpufeature.c > quite closely and allows us to easily reason about which fields are/aren't > tested. Sure. Will do. > > > +static void test_user_set_lower_safe(struct kvm_vcpu *vcpu) > > +{ > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(lower_safe_reg_ftrs); i++) { > > + struct reg_feature *reg_ftr = lower_safe_reg_ftrs + i; > > + uint64_t val, new_val, ftr; > > + > > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > > + ftr = field_get(reg_ftr->ftr_mask, val); > > + > > + /* Set a safe value for the feature */ > > + if (ftr > 0) > > + ftr--; > > + > > + val &= ~reg_ftr->ftr_mask; > > + val |= field_prep(reg_ftr->ftr_mask, ftr); > > + > > + vcpu_set_reg(vcpu, reg_ftr->reg, val); > > + vcpu_get_reg(vcpu, reg_ftr->reg, &new_val); > > + ASSERT_EQ(new_val, val); > > + } > > +} > > + > > +static void test_user_set_fail(struct kvm_vcpu *vcpu) > > +{ > > + int i, r; > > + > > + for (i = 0; i < ARRAY_SIZE(lower_safe_reg_ftrs); i++) { > > + struct reg_feature *reg_ftr = lower_safe_reg_ftrs + i; > > + uint64_t val, old_val, ftr; > > + > > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > > + ftr = field_get(reg_ftr->ftr_mask, val); > > + > > + /* Set a invalid value (too big) for the feature */ > > + if (ftr >= GENMASK_ULL(ARM64_FEATURE_FIELD_BITS - 1, 0)) > > + continue; > > This assumes that the fields in the register are unsigned, but there are > several which are not. This is based on the reg/fields which are defined previously. Those are carefully chosen without unsigned ones. > > > + ftr++; > > + > > + old_val = val; > > + val &= ~reg_ftr->ftr_mask; > > + val |= field_prep(reg_ftr->ftr_mask, ftr); > > + > > + r = __vcpu_set_reg(vcpu, reg_ftr->reg, val); > > + TEST_ASSERT(r < 0 && errno == EINVAL, > > + "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno); > > + > > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > > + ASSERT_EQ(val, old_val); > > + } > > +} > > + > > +static struct reg_feature exact_reg_ftrs[] = { > > + /* Items will be added when there is appropriate field of type > > + * FTR_EXACT enabled writing from userspace later. > > + */ > > +}; > > + > > +static void test_user_set_exact(struct kvm_vcpu *vcpu) > > +{ > > + int i, r; > > + > > + for (i = 0; i < ARRAY_SIZE(exact_reg_ftrs); i++) { > > + struct reg_feature *reg_ftr = exact_reg_ftrs + i; > > + uint64_t val, old_val, ftr; > > + > > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > > + ftr = field_get(reg_ftr->ftr_mask, val); > > + old_val = val; > > + > > + /* Exact match */ > > + vcpu_set_reg(vcpu, reg_ftr->reg, val); > > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > > + ASSERT_EQ(val, old_val); > > + > > + /* Smaller value */ > > + if (ftr > 0) { > > + ftr--; > > + val &= ~reg_ftr->ftr_mask; > > + val |= field_prep(reg_ftr->ftr_mask, ftr); > > + r = __vcpu_set_reg(vcpu, reg_ftr->reg, val); > > + TEST_ASSERT(r < 0 && errno == EINVAL, > > + "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno); > > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > > + ASSERT_EQ(val, old_val); > > + ftr++; > > + } > > + > > + /* Bigger value */ > > + ftr++; > > + val &= ~reg_ftr->ftr_mask; > > + val |= field_prep(reg_ftr->ftr_mask, ftr); > > + r = __vcpu_set_reg(vcpu, reg_ftr->reg, val); > > + TEST_ASSERT(r < 0 && errno == EINVAL, > > + "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno); > > + vcpu_get_reg(vcpu, reg_ftr->reg, &val); > > + ASSERT_EQ(val, old_val); > > + } > > +} > > Don't add dead code, this can be added when we actually test FTR_EXACT > fields. Are there not any in the registers exposed by this series? Nope, there is no in the registers exposed by this series. Anyway, I'll drop this code in this series. > > > +static uint32_t writable_regs[] = { > > + SYS_ID_DFR0_EL1, > > + SYS_ID_AA64DFR0_EL1, > > + SYS_ID_AA64PFR0_EL1, > > + SYS_ID_AA64MMFR0_EL1, > > + SYS_ID_AA64MMFR1_EL1, > > + SYS_ID_AA64MMFR2_EL1, > > +}; > > + > > +void test_user_get_writable_masks(struct kvm_vm *vm) > > +{ > > + struct feature_id_writable_masks masks; > > + > > + vm_ioctl(vm, KVM_ARM_GET_FEATURE_ID_WRITABLE_MASKS, &masks); > > + > > + for (int i = 0; i < ARRAY_SIZE(writable_regs); i++) { > > + uint32_t reg = writable_regs[i]; > > + int idx = ARM64_FEATURE_ID_SPACE_IDX(sys_reg_Op0(reg), > > + sys_reg_Op1(reg), sys_reg_CRn(reg), > > + sys_reg_CRm(reg), sys_reg_Op2(reg)); > > + > > + ASSERT_EQ(masks.mask[idx], GENMASK_ULL(63, 0)); > > + } > > +} > > The more robust test would be to check that every field this test knows > is writable is actually advertised as such in the ioctl. So you could > fetch this array at the start of the entire test and pass it through to > the routines that do granular checks against the fields of every > register. Makes sense. Will do. > > It'd also be good to see basic sanity tests on the ioctl (i.e. call > fails if ::rsvd is nonzero), since KVM has screwed that up on several > occasions in past. Sure. Will add some basic sanity tests on the ioctl. > > > +int main(void) > > +{ > > + struct kvm_vcpu *vcpu; > > + struct kvm_vm *vm; > > + > > + vm = vm_create_with_one_vcpu(&vcpu, guest_code); > > + > > + ksft_print_header(); > > + ksft_set_plan(4); > > + > > + test_user_get_writable_masks(vm); > > + ksft_test_result_pass("test_user_get_writable_masks\n"); > > + > > + test_user_set_exact(vcpu); > > + ksft_test_result_pass("test_user_set_exact\n"); > > + > > + test_user_set_fail(vcpu); > > + ksft_test_result_pass("test_user_set_fail\n"); > > + > > + test_user_set_lower_safe(vcpu); > > + ksft_test_result_pass("test_user_set_lower_safe\n"); > > + > > + kvm_vm_free(vm); > > + > > + ksft_finished(); > > +} > > -- > > 2.41.0.585.gd2178a4bd4-goog > > > > -- > Thanks, > Oliver Thanks, Jing
diff --git a/tools/arch/arm64/include/uapi/asm/kvm.h b/tools/arch/arm64/include/uapi/asm/kvm.h index f7ddd73a8c0f..2970c0d792ee 100644 --- a/tools/arch/arm64/include/uapi/asm/kvm.h +++ b/tools/arch/arm64/include/uapi/asm/kvm.h @@ -505,6 +505,31 @@ struct kvm_smccc_filter { #define KVM_HYPERCALL_EXIT_SMC (1U << 0) #define KVM_HYPERCALL_EXIT_16BIT (1U << 1) +/* Get feature ID registers userspace writable mask. */ +/* + * From DDI0487J.a, D19.2.66 ("ID_AA64MMFR2_EL1, AArch64 Memory Model + * Feature Register 2"): + * + * "The Feature ID space is defined as the System register space in + * AArch64 with op0==3, op1=={0, 1, 3}, CRn==0, CRm=={0-7}, + * op2=={0-7}." + * + * This covers all R/O registers that indicate anything useful feature + * wise, including the ID registers. + */ +#define ARM64_FEATURE_ID_SPACE_IDX(op0, op1, crn, crm, op2) \ + ({ \ + __u64 __op1 = (op1) & 3; \ + __op1 -= (__op1 == 3); \ + (__op1 << 6 | ((crm) & 7) << 3 | (op2)); \ + }) + +#define ARM64_FEATURE_ID_SPACE_SIZE (3 * 8 * 8) + +struct feature_id_writable_masks { + __u64 mask[ARM64_FEATURE_ID_SPACE_SIZE]; +}; + #endif #endif /* __ARM_KVM_H__ */ diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h index f089ab290978..9abe69cf4001 100644 --- a/tools/include/uapi/linux/kvm.h +++ b/tools/include/uapi/linux/kvm.h @@ -1555,6 +1555,8 @@ struct kvm_s390_ucas_mapping { #define KVM_ARM_MTE_COPY_TAGS _IOR(KVMIO, 0xb4, struct kvm_arm_copy_mte_tags) /* Available with KVM_CAP_COUNTER_OFFSET */ #define KVM_ARM_SET_COUNTER_OFFSET _IOW(KVMIO, 0xb5, struct kvm_arm_counter_offset) +#define KVM_ARM_GET_FEATURE_ID_WRITABLE_MASKS \ + _IOR(KVMIO, 0xb6, struct kvm_arm_feature_id_masks) /* ioctl for vm fd */ #define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device) diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index c692cc86e7da..87ceadc1292a 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -144,6 +144,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list TEST_GEN_PROGS_aarch64 += aarch64/hypercalls TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test TEST_GEN_PROGS_aarch64 += aarch64/psci_test +TEST_GEN_PROGS_aarch64 += aarch64/set_id_regs TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config TEST_GEN_PROGS_aarch64 += aarch64/vgic_init diff --git a/tools/testing/selftests/kvm/aarch64/set_id_regs.c b/tools/testing/selftests/kvm/aarch64/set_id_regs.c new file mode 100644 index 000000000000..9c8f439ac7b3 --- /dev/null +++ b/tools/testing/selftests/kvm/aarch64/set_id_regs.c @@ -0,0 +1,191 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * set_id_regs - Test for setting ID register from usersapce. + * + * Copyright (c) 2023 Google LLC. + * + * + * Test that KVM supports setting ID registers from userspace and handles the + * feature set correctly. + */ + +#include <stdint.h> +#include "kvm_util.h" +#include "processor.h" +#include "test_util.h" +#include <linux/bitfield.h> + +#define field_get(_mask, _reg) (((_reg) & (_mask)) >> (ffsl(_mask) - 1)) +#define field_prep(_mask, _val) (((_val) << (ffsl(_mask) - 1)) & (_mask)) + +struct reg_feature { + uint64_t reg; + uint64_t ftr_mask; +}; + +static void guest_code(void) +{ + for (;;) + GUEST_SYNC(0); +} + +static struct reg_feature lower_safe_reg_ftrs[] = { + { KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), ARM64_FEATURE_MASK(ID_AA64DFR0_WRPS) }, + { KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1), ARM64_FEATURE_MASK(ID_AA64PFR0_EL3) }, + { KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR0_EL1), ARM64_FEATURE_MASK(ID_AA64MMFR0_FGT) }, + { KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR1_EL1), ARM64_FEATURE_MASK(ID_AA64MMFR1_PAN) }, + { KVM_ARM64_SYS_REG(SYS_ID_AA64MMFR2_EL1), ARM64_FEATURE_MASK(ID_AA64MMFR2_FWB) }, +}; + +static void test_user_set_lower_safe(struct kvm_vcpu *vcpu) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(lower_safe_reg_ftrs); i++) { + struct reg_feature *reg_ftr = lower_safe_reg_ftrs + i; + uint64_t val, new_val, ftr; + + vcpu_get_reg(vcpu, reg_ftr->reg, &val); + ftr = field_get(reg_ftr->ftr_mask, val); + + /* Set a safe value for the feature */ + if (ftr > 0) + ftr--; + + val &= ~reg_ftr->ftr_mask; + val |= field_prep(reg_ftr->ftr_mask, ftr); + + vcpu_set_reg(vcpu, reg_ftr->reg, val); + vcpu_get_reg(vcpu, reg_ftr->reg, &new_val); + ASSERT_EQ(new_val, val); + } +} + +static void test_user_set_fail(struct kvm_vcpu *vcpu) +{ + int i, r; + + for (i = 0; i < ARRAY_SIZE(lower_safe_reg_ftrs); i++) { + struct reg_feature *reg_ftr = lower_safe_reg_ftrs + i; + uint64_t val, old_val, ftr; + + vcpu_get_reg(vcpu, reg_ftr->reg, &val); + ftr = field_get(reg_ftr->ftr_mask, val); + + /* Set a invalid value (too big) for the feature */ + if (ftr >= GENMASK_ULL(ARM64_FEATURE_FIELD_BITS - 1, 0)) + continue; + ftr++; + + old_val = val; + val &= ~reg_ftr->ftr_mask; + val |= field_prep(reg_ftr->ftr_mask, ftr); + + r = __vcpu_set_reg(vcpu, reg_ftr->reg, val); + TEST_ASSERT(r < 0 && errno == EINVAL, + "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno); + + vcpu_get_reg(vcpu, reg_ftr->reg, &val); + ASSERT_EQ(val, old_val); + } +} + +static struct reg_feature exact_reg_ftrs[] = { + /* Items will be added when there is appropriate field of type + * FTR_EXACT enabled writing from userspace later. + */ +}; + +static void test_user_set_exact(struct kvm_vcpu *vcpu) +{ + int i, r; + + for (i = 0; i < ARRAY_SIZE(exact_reg_ftrs); i++) { + struct reg_feature *reg_ftr = exact_reg_ftrs + i; + uint64_t val, old_val, ftr; + + vcpu_get_reg(vcpu, reg_ftr->reg, &val); + ftr = field_get(reg_ftr->ftr_mask, val); + old_val = val; + + /* Exact match */ + vcpu_set_reg(vcpu, reg_ftr->reg, val); + vcpu_get_reg(vcpu, reg_ftr->reg, &val); + ASSERT_EQ(val, old_val); + + /* Smaller value */ + if (ftr > 0) { + ftr--; + val &= ~reg_ftr->ftr_mask; + val |= field_prep(reg_ftr->ftr_mask, ftr); + r = __vcpu_set_reg(vcpu, reg_ftr->reg, val); + TEST_ASSERT(r < 0 && errno == EINVAL, + "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno); + vcpu_get_reg(vcpu, reg_ftr->reg, &val); + ASSERT_EQ(val, old_val); + ftr++; + } + + /* Bigger value */ + ftr++; + val &= ~reg_ftr->ftr_mask; + val |= field_prep(reg_ftr->ftr_mask, ftr); + r = __vcpu_set_reg(vcpu, reg_ftr->reg, val); + TEST_ASSERT(r < 0 && errno == EINVAL, + "Unexpected KVM_SET_ONE_REG error: r=%d, errno=%d", r, errno); + vcpu_get_reg(vcpu, reg_ftr->reg, &val); + ASSERT_EQ(val, old_val); + } +} + +static uint32_t writable_regs[] = { + SYS_ID_DFR0_EL1, + SYS_ID_AA64DFR0_EL1, + SYS_ID_AA64PFR0_EL1, + SYS_ID_AA64MMFR0_EL1, + SYS_ID_AA64MMFR1_EL1, + SYS_ID_AA64MMFR2_EL1, +}; + +void test_user_get_writable_masks(struct kvm_vm *vm) +{ + struct feature_id_writable_masks masks; + + vm_ioctl(vm, KVM_ARM_GET_FEATURE_ID_WRITABLE_MASKS, &masks); + + for (int i = 0; i < ARRAY_SIZE(writable_regs); i++) { + uint32_t reg = writable_regs[i]; + int idx = ARM64_FEATURE_ID_SPACE_IDX(sys_reg_Op0(reg), + sys_reg_Op1(reg), sys_reg_CRn(reg), + sys_reg_CRm(reg), sys_reg_Op2(reg)); + + ASSERT_EQ(masks.mask[idx], GENMASK_ULL(63, 0)); + } +} + +int main(void) +{ + struct kvm_vcpu *vcpu; + struct kvm_vm *vm; + + vm = vm_create_with_one_vcpu(&vcpu, guest_code); + + ksft_print_header(); + ksft_set_plan(4); + + test_user_get_writable_masks(vm); + ksft_test_result_pass("test_user_get_writable_masks\n"); + + test_user_set_exact(vcpu); + ksft_test_result_pass("test_user_set_exact\n"); + + test_user_set_fail(vcpu); + ksft_test_result_pass("test_user_set_fail\n"); + + test_user_set_lower_safe(vcpu); + ksft_test_result_pass("test_user_set_lower_safe\n"); + + kvm_vm_free(vm); + + ksft_finished(); +}
Add tests to verify setting ID registers from userapce is handled correctly by KVM. Also add a test case to use ioctl KVM_ARM_GET_FEATURE_ID_WRITABLE_MASKS to get writable masks. Signed-off-by: Jing Zhang <jingzhangos@google.com> --- tools/arch/arm64/include/uapi/asm/kvm.h | 25 +++ tools/include/uapi/linux/kvm.h | 2 + tools/testing/selftests/kvm/Makefile | 1 + .../selftests/kvm/aarch64/set_id_regs.c | 191 ++++++++++++++++++ 4 files changed, 219 insertions(+) create mode 100644 tools/testing/selftests/kvm/aarch64/set_id_regs.c