Re: [PATCH v2 7/7] KVM: selftests: Add test for RAZ/WI AArch32 ID registers

2022-09-05 Thread Oliver Upton
Hi Drew,

On Mon, Sep 05, 2022 at 09:31:40AM +0200, Andrew Jones wrote:

[...]

> > +static uint64_t reg_ids[] = {
> > +   KVM_ARM64_SYS_REG(SYS_ID_PFR0_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_PFR1_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_DFR0_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_MMFR0_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_MMFR1_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_MMFR2_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_MMFR3_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_ISAR0_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_ISAR1_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_ISAR2_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_ISAR3_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_ISAR4_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_ISAR5_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_MMFR4_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_ISAR6_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_MVFR0_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_MVFR1_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_MVFR2_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_PFR2_EL1),
> > +   KVM_ARM64_SYS_REG(SYS_ID_MMFR5_EL1),
> 
> Hi Oliver,
> 
> I see all the hidden and unallocated registers have been filtered out of
> the test lists. They should also behave as RAZ, right? Maybe we should
> keep them in the lists here for consistency and to test them as well.

Sure, can do. The reason I only tested these registers is because they
have RAZ/WI behavior with this series, whereas the rest are RAZ +
invariant. Should be easy enough to cover the whole range, though.

--
Thanks,
Oliver
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 7/7] KVM: selftests: Add test for RAZ/WI AArch32 ID registers

2022-09-05 Thread Andrew Jones
On Fri, Sep 02, 2022 at 03:48:03PM +, Oliver Upton wrote:
> Add a test to assert that KVM handles the AArch64 views of the AArch32
> ID registers as RAZ/WI (writable only from userspace).
> 
> Signed-off-by: Oliver Upton 
> ---
>  tools/testing/selftests/kvm/.gitignore|   1 +
>  tools/testing/selftests/kvm/Makefile  |   1 +
>  .../kvm/aarch64/aarch64_only_id_regs.c| 135 ++
>  3 files changed, 137 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/aarch64/aarch64_only_id_regs.c
> 
> diff --git a/tools/testing/selftests/kvm/.gitignore 
> b/tools/testing/selftests/kvm/.gitignore
> index d625a3f83780..4331af62a982 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -1,4 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +/aarch64/aarch64_only_id_regs
>  /aarch64/arch_timer
>  /aarch64/debug-exceptions
>  /aarch64/get-reg-list
> diff --git a/tools/testing/selftests/kvm/Makefile 
> b/tools/testing/selftests/kvm/Makefile
> index 4c122f1b1737..efe155259095 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -144,6 +144,7 @@ TEST_GEN_PROGS_x86_64 += system_counter_offset_test
>  # Compiled outputs used by test targets
>  TEST_GEN_PROGS_EXTENDED_x86_64 += x86_64/nx_huge_pages_test
>  
> +TEST_GEN_PROGS_aarch64 += aarch64/aarch64_only_id_regs
>  TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
>  TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
>  TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
> diff --git a/tools/testing/selftests/kvm/aarch64/aarch64_only_id_regs.c 
> b/tools/testing/selftests/kvm/aarch64/aarch64_only_id_regs.c
> new file mode 100644
> index ..704a3e7524a8
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/aarch64/aarch64_only_id_regs.c
> @@ -0,0 +1,135 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * aarch64_only_id_regs - Test for ID register behavior on AArch64-only 
> systems
> + *
> + * Copyright (c) 2022 Google LLC.
> + *
> + * Test that KVM handles the AArch64 views of the AArch32 ID registers as RAZ
> + * and WI from userspace.
> + */
> +
> +#include 
> +
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "test_util.h"
> +
> +#define BAD_ID_REG_VAL   0x1badc0deul
> +
> +#define GUEST_ASSERT_REG_RAZ(reg)GUEST_ASSERT_EQ(read_sysreg_s(reg), 0)
> +
> +static void guest_main(void)
> +{
> + GUEST_ASSERT_REG_RAZ(SYS_ID_PFR0_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_PFR1_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_DFR0_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR0_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR1_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR2_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR3_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR0_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR1_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR2_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR3_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR4_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR5_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR4_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR6_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_MVFR0_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_MVFR1_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_MVFR2_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_PFR2_EL1);
> + GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR5_EL1);
> +
> + GUEST_DONE();
> +}
> +
> +static void test_guest_raz(struct kvm_vcpu *vcpu)
> +{
> + struct ucall uc;
> +
> + vcpu_run(vcpu);
> +
> + switch (get_ucall(vcpu, )) {
> + case UCALL_ABORT:
> + REPORT_GUEST_ASSERT(uc);
> + break;
> + case UCALL_DONE:
> + break;
> + default:
> + TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
> + }
> +}
> +
> +static uint64_t reg_ids[] = {
> + KVM_ARM64_SYS_REG(SYS_ID_PFR0_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_PFR1_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_DFR0_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_MMFR0_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_MMFR1_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_MMFR2_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_MMFR3_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_ISAR0_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_ISAR1_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_ISAR2_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_ISAR3_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_ISAR4_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_ISAR5_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_MMFR4_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_ISAR6_EL1),
> + KVM_ARM64_SYS_REG(SYS_MVFR0_EL1),
> + KVM_ARM64_SYS_REG(SYS_MVFR1_EL1),
> + KVM_ARM64_SYS_REG(SYS_MVFR2_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_PFR2_EL1),
> + KVM_ARM64_SYS_REG(SYS_ID_MMFR5_EL1),

Hi Oliver,

I see all the hidden and unallocated registers have been filtered out of
the test lists. They should also behave as RAZ, right? Maybe we should
keep them in the lists here for consistency and to test them as well.

Thanks,
drew

> +};
> +
> +static void 

[PATCH v2 7/7] KVM: selftests: Add test for RAZ/WI AArch32 ID registers

2022-09-02 Thread Oliver Upton
Add a test to assert that KVM handles the AArch64 views of the AArch32
ID registers as RAZ/WI (writable only from userspace).

Signed-off-by: Oliver Upton 
---
 tools/testing/selftests/kvm/.gitignore|   1 +
 tools/testing/selftests/kvm/Makefile  |   1 +
 .../kvm/aarch64/aarch64_only_id_regs.c| 135 ++
 3 files changed, 137 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/aarch64/aarch64_only_id_regs.c

diff --git a/tools/testing/selftests/kvm/.gitignore 
b/tools/testing/selftests/kvm/.gitignore
index d625a3f83780..4331af62a982 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
+/aarch64/aarch64_only_id_regs
 /aarch64/arch_timer
 /aarch64/debug-exceptions
 /aarch64/get-reg-list
diff --git a/tools/testing/selftests/kvm/Makefile 
b/tools/testing/selftests/kvm/Makefile
index 4c122f1b1737..efe155259095 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -144,6 +144,7 @@ TEST_GEN_PROGS_x86_64 += system_counter_offset_test
 # Compiled outputs used by test targets
 TEST_GEN_PROGS_EXTENDED_x86_64 += x86_64/nx_huge_pages_test
 
+TEST_GEN_PROGS_aarch64 += aarch64/aarch64_only_id_regs
 TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
 TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
 TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
diff --git a/tools/testing/selftests/kvm/aarch64/aarch64_only_id_regs.c 
b/tools/testing/selftests/kvm/aarch64/aarch64_only_id_regs.c
new file mode 100644
index ..704a3e7524a8
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/aarch64_only_id_regs.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * aarch64_only_id_regs - Test for ID register behavior on AArch64-only systems
+ *
+ * Copyright (c) 2022 Google LLC.
+ *
+ * Test that KVM handles the AArch64 views of the AArch32 ID registers as RAZ
+ * and WI from userspace.
+ */
+
+#include 
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "test_util.h"
+
+#define BAD_ID_REG_VAL 0x1badc0deul
+
+#define GUEST_ASSERT_REG_RAZ(reg)  GUEST_ASSERT_EQ(read_sysreg_s(reg), 0)
+
+static void guest_main(void)
+{
+   GUEST_ASSERT_REG_RAZ(SYS_ID_PFR0_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_PFR1_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_DFR0_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR0_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR1_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR2_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR3_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR0_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR1_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR2_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR3_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR4_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR5_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR4_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_ISAR6_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_MVFR0_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_MVFR1_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_MVFR2_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_PFR2_EL1);
+   GUEST_ASSERT_REG_RAZ(SYS_ID_MMFR5_EL1);
+
+   GUEST_DONE();
+}
+
+static void test_guest_raz(struct kvm_vcpu *vcpu)
+{
+   struct ucall uc;
+
+   vcpu_run(vcpu);
+
+   switch (get_ucall(vcpu, )) {
+   case UCALL_ABORT:
+   REPORT_GUEST_ASSERT(uc);
+   break;
+   case UCALL_DONE:
+   break;
+   default:
+   TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
+   }
+}
+
+static uint64_t reg_ids[] = {
+   KVM_ARM64_SYS_REG(SYS_ID_PFR0_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_PFR1_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_DFR0_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_MMFR0_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_MMFR1_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_MMFR2_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_MMFR3_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_ISAR0_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_ISAR1_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_ISAR2_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_ISAR3_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_ISAR4_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_ISAR5_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_MMFR4_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_ISAR6_EL1),
+   KVM_ARM64_SYS_REG(SYS_MVFR0_EL1),
+   KVM_ARM64_SYS_REG(SYS_MVFR1_EL1),
+   KVM_ARM64_SYS_REG(SYS_MVFR2_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_PFR2_EL1),
+   KVM_ARM64_SYS_REG(SYS_ID_MMFR5_EL1),
+};
+
+static void test_user_raz_wi(struct kvm_vcpu *vcpu)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(reg_ids); i++) {
+   uint64_t reg_id = reg_ids[i];
+   uint64_t val;
+
+   vcpu_get_reg(vcpu, reg_id, );
+   ASSERT_EQ(val, 0);
+
+   /*
+* Expect the ioctl to succeed with no effect on the register
+* value.
+*/
+   vcpu_set_reg(vcpu, reg_id,