2021-09-14 22:33:53

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 00/15] KVM: arm64: selftests: Introduce arch_timer selftest

Hello,

The patch series adds a KVM selftest to validate the behavior of
ARM's generic timer (patch-14). The test programs the timer IRQs
periodically, and for each interrupt, it validates the behaviour
against the architecture specifications. The test further provides
a command-line interface to configure the number of vCPUs, the
period of the timer, and the number of iterations that the test
has to run for.

Patch-15 adds an option to randomly migrate the vCPUs to different
physical CPUs across the system. The bug for the fix provided by
Marc with commit 3134cc8beb69d0d ("KVM: arm64: vgic: Resample HW
pending state on deactivation") was discovered using arch_timer
test with vCPU migrations.

Since the test heavily depends on interrupts, patch-13 adds a host
library to setup ARM Generic Interrupt Controller v3 (GICv3). This
includes creating a vGIC device, setting up distributor and
redistributor attributes, and mapping the guest physical addresses.
Symmetrical to this, patch-12 adds a guest library to talk to the vGIC,
which includes initializing the controller, enabling/disabling the
interrupts, and so on.

The following patches are utility patches that the above ones make use
of:
Patch-1 adds readl/writel support for guests to access MMIO space.

Patch-2 imports arch/arm64/include/asm/sysreg.h into
tools/arch/arm64/include/asm/ to make use of the register encodings
and read/write definitions.

Patch-3 is not directly related to the test, but makes
aarch64/debug-exceptions.c use the read/write definitions from the
imported sysreg.h and remove the existing definitions of read_sysreg()
and write_sysreg().

Patch-4 introduces ARM64_SYS_KVM_REG, that helps convert the SYS_*
register encodings in sysreg.h to be acceptable by get_reg() and set_reg().
It further replaces the users of ARM64_SYS_REG to use the new macro.

Patch-5 adds the support for cpu_relax().

Patch-6 adds basic arch_timer framework.

Patch-7 adds udelay() support for the guests to utilize.

Patch-8 adds local_irq_enable() and local_irq_disable() for the guests
to enable/disable interrupts.

Patch-9 is also unrelated to the test. It modifies the prototype of
aarch64_vcpu_setup() to accept vcpuid as uint32_t, to keep it consistent
with the other parts of code.

Patch-10 adds the support to get the vcpuid for the guests. This allows
them to access any cpu-centric private data in the upcoming patches.

Patch-11 adds a light-weight support for spinlocks for the guests to
use.

The patch series, specifically the library support, is derived from the
kvm-unit-tests and the kernel itself.

Regards,
Raghavendra

v6 -> v7:

Addressed comments by Andrew:

- Changed the prototype of aarch64_vcpu_setup() to accept vcpuid as
uint32_t.
- Modified the prototype of guest_get_vcpuid() to return uint32_t.
- Renamed assert messages in lib/aarch64/vgic.c to use "Number".

v5 -> v6:

- Corrected the syntax for write_sysreg_s in gic_v3.c (11/14) so that
the file can be compiled with the unmodified
arch/arm64/include/asm/sysreg.h that's imported into tools/.

v4 -> v5:

Addressed the comments by Andrew, Oliver, and Reiji (Thanks, again):
- Squashed patches 17/18 and 18/18 into 3/18 and 14/18, respectively.
- Dropped patches to keep track kvm_utils of nr_vcpus (12/18) and
vm_get_mode() (13/18) as they were no longer needed.
- Instead of creating the a map, exporting the vcpuid to the guest
is done by using the TPIDR_EL1 register.
- Just to be on the safer side, gic.c's gic_dist_init() explicitly
checks if gic_ops is NULL.
- Move sysreg.h from within selftests to tool/arch/arm64/include/asm/.
- Rename ARM64_SYS_KVM_REG to KVM_ARM64_SYS_REG to improve readability.
- Use the GIC regions' sizes from asm/kvm.h instead of re-defining it
in the vgic host support.
- Get the timer IRQ numbers via timer's device attributes
(KVM_ARM_VCPU_TIMER_IRQ_PTIMER, KVM_ARM_VCPU_TIMER_IRQ_VTIMER) instead
of depending on default numbers to be safe.
- Add check to see if the vCPU migrations are in fact enabled, before
looking for at least two online physical CPUs for the test.
- Add missing blank lines in the arch_timer test.

v3 -> v4:

Addressed the comments by Andrew, Oliver, and Ricardo (Thank you):
- Reimplemented get_vcpuid() by exporting a map of vcpuid:mpidr to the
guest.
- Import sysreg.h from arch/arm64/include/asm/sysreg.h to get the system
register encodings and its read/write support. As a result, delete the
existing definitions in processor.h.
- Introduce ARM64_SYS_KVM_REG that converts SYS_* register definitions
from sysreg.h into the encodings accepted by get_reg() and set_reg().
- Hence, remove the existing encodings of system registers (CPACR_EL1,
TCR_EL1, and friends) and replace all the its consumers throughout
the selftests with ARM64_SYS_KVM_REG.
- Keep track of number of vCPUs in 'struct kvm_vm'.
- Add a helper method to get the KVM VM's mode.
- Modify the vGIC host function vgic_v3_setup to make use of the above
two helper methods, which prevents it from accepting nr_vcpus as
an argument.
- Move the definition of REDIST_REGION_ATTR_ADDR from lib/aarch64/vgic.c
to include/aarch64/vgic.h.
- Make the selftest, vgic_init.c, use the definition of REDIST_REGION_ATTR_ADDR
from include/aarch64/vgic.h.
- Turn ON vCPU migration by default (-m 2).
- Add pr_debug() to log vCPU migrations. Helpful for diagnosis.
- Change TEST_ASSERT(false,...) to TEST_FAIL() in the base arch_timer
test.
- Include linux/types.h for __force definitions.
- Change the type of 'val' to 'int' in spin_lock() to match the lock
value type.
- Fix typos in code files and comments.

v2 -> v3:

- Addressed the comments from Ricardo regarding moving the vGIC host
support for selftests to its own library.
- Added an option (-m) to migrate the guest vCPUs to physical CPUs
in the system.

v1 -> v2:

Addressed comments from Zenghui in include/aarch64/arch_timer.h:
- Correct the header description
- Remove unnecessary inclusion of linux/sizes.h
- Re-arrange CTL_ defines in ascending order
- Remove inappropriate 'return' from timer_set_* functions, which
returns 'void'.

v1: https://lore.kernel.org/kvmarm/[email protected]/
v2: https://lore.kernel.org/kvmarm/[email protected]/
v3: https://lore.kernel.org/kvmarm/[email protected]/
v4: https://lore.kernel.org/kvmarm/[email protected]/
v5: https://lore.kernel.org/kvmarm/[email protected]/
v6: https://lore.kernel.org/kvmarm/[email protected]/

Raghavendra Rao Ananta (15):
KVM: arm64: selftests: Add MMIO readl/writel support
tools: arm64: Import sysreg.h
KVM: arm64: selftests: Use read/write definitions from sysreg.h
KVM: arm64: selftests: Introduce ARM64_SYS_KVM_REG
KVM: arm64: selftests: Add support for cpu_relax
KVM: arm64: selftests: Add basic support for arch_timers
KVM: arm64: selftests: Add basic support to generate delays
KVM: arm64: selftests: Add support to disable and enable local IRQs
KVM: arm64: selftests: Maintain consistency for vcpuid type
KVM: arm64: selftests: Add guest support to get the vcpuid
KVM: arm64: selftests: Add light-weight spinlock support
KVM: arm64: selftests: Add basic GICv3 support
KVM: arm64: selftests: Add host support for vGIC
KVM: arm64: selftests: Add arch_timer test
KVM: arm64: selftests: arch_timer: Support vCPU migration

tools/arch/arm64/include/asm/sysreg.h | 1296 +++++++++++++++++
tools/testing/selftests/kvm/.gitignore | 1 +
tools/testing/selftests/kvm/Makefile | 3 +-
.../selftests/kvm/aarch64/arch_timer.c | 479 ++++++
.../selftests/kvm/aarch64/debug-exceptions.c | 30 +-
.../selftests/kvm/aarch64/psci_cpu_on_test.c | 2 +-
.../testing/selftests/kvm/aarch64/vgic_init.c | 3 +-
.../kvm/include/aarch64/arch_timer.h | 142 ++
.../selftests/kvm/include/aarch64/delay.h | 25 +
.../selftests/kvm/include/aarch64/gic.h | 21 +
.../selftests/kvm/include/aarch64/processor.h | 90 +-
.../selftests/kvm/include/aarch64/spinlock.h | 13 +
.../selftests/kvm/include/aarch64/vgic.h | 20 +
.../testing/selftests/kvm/include/kvm_util.h | 2 +
tools/testing/selftests/kvm/lib/aarch64/gic.c | 95 ++
.../selftests/kvm/lib/aarch64/gic_private.h | 21 +
.../selftests/kvm/lib/aarch64/gic_v3.c | 240 +++
.../selftests/kvm/lib/aarch64/gic_v3.h | 70 +
.../selftests/kvm/lib/aarch64/processor.c | 24 +-
.../selftests/kvm/lib/aarch64/spinlock.c | 27 +
.../testing/selftests/kvm/lib/aarch64/vgic.c | 70 +
21 files changed, 2626 insertions(+), 48 deletions(-)
create mode 100644 tools/arch/arm64/include/asm/sysreg.h
create mode 100644 tools/testing/selftests/kvm/aarch64/arch_timer.c
create mode 100644 tools/testing/selftests/kvm/include/aarch64/arch_timer.h
create mode 100644 tools/testing/selftests/kvm/include/aarch64/delay.h
create mode 100644 tools/testing/selftests/kvm/include/aarch64/gic.h
create mode 100644 tools/testing/selftests/kvm/include/aarch64/spinlock.h
create mode 100644 tools/testing/selftests/kvm/include/aarch64/vgic.h
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic.c
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic_private.h
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic_v3.c
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic_v3.h
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/spinlock.c
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/vgic.c

--
2.33.0.309.g3052b89438-goog


2021-09-14 22:33:58

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 05/15] KVM: arm64: selftests: Add support for cpu_relax

Implement the guest helper routine, cpu_relax(), to yield
the processor to other tasks.

The function was derived from
arch/arm64/include/asm/vdso/processor.h.

Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Oliver Upton <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
tools/testing/selftests/kvm/include/aarch64/processor.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
index 93797783abad..265054c24481 100644
--- a/tools/testing/selftests/kvm/include/aarch64/processor.h
+++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
@@ -122,6 +122,11 @@ void vm_install_exception_handler(struct kvm_vm *vm,
void vm_install_sync_handler(struct kvm_vm *vm,
int vector, int ec, handler_fn handler);

+static inline void cpu_relax(void)
+{
+ asm volatile("yield" ::: "memory");
+}
+
#define isb() asm volatile("isb" : : : "memory")
#define dsb(opt) asm volatile("dsb " #opt : : : "memory")
#define dmb(opt) asm volatile("dmb " #opt : : : "memory")
--
2.33.0.309.g3052b89438-goog

2021-09-14 22:34:11

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 06/15] KVM: arm64: selftests: Add basic support for arch_timers

Add a minimalistic library support to access the virtual timers,
that can be used for simple timing functionalities, such as
introducing delays in the guest.

Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
.../kvm/include/aarch64/arch_timer.h | 142 ++++++++++++++++++
1 file changed, 142 insertions(+)
create mode 100644 tools/testing/selftests/kvm/include/aarch64/arch_timer.h

diff --git a/tools/testing/selftests/kvm/include/aarch64/arch_timer.h b/tools/testing/selftests/kvm/include/aarch64/arch_timer.h
new file mode 100644
index 000000000000..cb7c03de3a21
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/aarch64/arch_timer.h
@@ -0,0 +1,142 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * ARM Generic Timer specific interface
+ */
+
+#ifndef SELFTEST_KVM_ARCH_TIMER_H
+#define SELFTEST_KVM_ARCH_TIMER_H
+
+#include "processor.h"
+
+enum arch_timer {
+ VIRTUAL,
+ PHYSICAL,
+};
+
+#define CTL_ENABLE (1 << 0)
+#define CTL_IMASK (1 << 1)
+#define CTL_ISTATUS (1 << 2)
+
+#define msec_to_cycles(msec) \
+ (timer_get_cntfrq() * (uint64_t)(msec) / 1000)
+
+#define usec_to_cycles(usec) \
+ (timer_get_cntfrq() * (uint64_t)(usec) / 1000000)
+
+#define cycles_to_usec(cycles) \
+ ((uint64_t)(cycles) * 1000000 / timer_get_cntfrq())
+
+static inline uint32_t timer_get_cntfrq(void)
+{
+ return read_sysreg(cntfrq_el0);
+}
+
+static inline uint64_t timer_get_cntct(enum arch_timer timer)
+{
+ isb();
+
+ switch (timer) {
+ case VIRTUAL:
+ return read_sysreg(cntvct_el0);
+ case PHYSICAL:
+ return read_sysreg(cntpct_el0);
+ default:
+ GUEST_ASSERT_1(0, timer);
+ }
+
+ /* We should not reach here */
+ return 0;
+}
+
+static inline void timer_set_cval(enum arch_timer timer, uint64_t cval)
+{
+ switch (timer) {
+ case VIRTUAL:
+ write_sysreg(cval, cntv_cval_el0);
+ break;
+ case PHYSICAL:
+ write_sysreg(cval, cntp_cval_el0);
+ break;
+ default:
+ GUEST_ASSERT_1(0, timer);
+ }
+
+ isb();
+}
+
+static inline uint64_t timer_get_cval(enum arch_timer timer)
+{
+ switch (timer) {
+ case VIRTUAL:
+ return read_sysreg(cntv_cval_el0);
+ case PHYSICAL:
+ return read_sysreg(cntp_cval_el0);
+ default:
+ GUEST_ASSERT_1(0, timer);
+ }
+
+ /* We should not reach here */
+ return 0;
+}
+
+static inline void timer_set_tval(enum arch_timer timer, uint32_t tval)
+{
+ switch (timer) {
+ case VIRTUAL:
+ write_sysreg(tval, cntv_tval_el0);
+ break;
+ case PHYSICAL:
+ write_sysreg(tval, cntp_tval_el0);
+ break;
+ default:
+ GUEST_ASSERT_1(0, timer);
+ }
+
+ isb();
+}
+
+static inline void timer_set_ctl(enum arch_timer timer, uint32_t ctl)
+{
+ switch (timer) {
+ case VIRTUAL:
+ write_sysreg(ctl, cntv_ctl_el0);
+ break;
+ case PHYSICAL:
+ write_sysreg(ctl, cntp_ctl_el0);
+ break;
+ default:
+ GUEST_ASSERT_1(0, timer);
+ }
+
+ isb();
+}
+
+static inline uint32_t timer_get_ctl(enum arch_timer timer)
+{
+ switch (timer) {
+ case VIRTUAL:
+ return read_sysreg(cntv_ctl_el0);
+ case PHYSICAL:
+ return read_sysreg(cntp_ctl_el0);
+ default:
+ GUEST_ASSERT_1(0, timer);
+ }
+
+ /* We should not reach here */
+ return 0;
+}
+
+static inline void timer_set_next_cval_ms(enum arch_timer timer, uint32_t msec)
+{
+ uint64_t now_ct = timer_get_cntct(timer);
+ uint64_t next_ct = now_ct + msec_to_cycles(msec);
+
+ timer_set_cval(timer, next_ct);
+}
+
+static inline void timer_set_next_tval_ms(enum arch_timer timer, uint32_t msec)
+{
+ timer_set_tval(timer, msec_to_cycles(msec));
+}
+
+#endif /* SELFTEST_KVM_ARCH_TIMER_H */
--
2.33.0.309.g3052b89438-goog

2021-09-14 22:34:17

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 07/15] KVM: arm64: selftests: Add basic support to generate delays

Add udelay() support to generate a delay in the guest.

The routines are derived and simplified from kernel's
arch/arm64/lib/delay.c.

Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
Reviewed-by: Oliver Upton <[email protected]>
---
.../selftests/kvm/include/aarch64/delay.h | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 tools/testing/selftests/kvm/include/aarch64/delay.h

diff --git a/tools/testing/selftests/kvm/include/aarch64/delay.h b/tools/testing/selftests/kvm/include/aarch64/delay.h
new file mode 100644
index 000000000000..329e4f5079ea
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/aarch64/delay.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * ARM simple delay routines
+ */
+
+#ifndef SELFTEST_KVM_ARM_DELAY_H
+#define SELFTEST_KVM_ARM_DELAY_H
+
+#include "arch_timer.h"
+
+static inline void __delay(uint64_t cycles)
+{
+ enum arch_timer timer = VIRTUAL;
+ uint64_t start = timer_get_cntct(timer);
+
+ while ((timer_get_cntct(timer) - start) < cycles)
+ cpu_relax();
+}
+
+static inline void udelay(unsigned long usec)
+{
+ __delay(usec_to_cycles(usec));
+}
+
+#endif /* SELFTEST_KVM_ARM_DELAY_H */
--
2.33.0.309.g3052b89438-goog

2021-09-14 22:34:20

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 09/15] KVM: arm64: selftests: Maintain consistency for vcpuid type

The prototype of aarch64_vcpu_setup() accepts vcpuid as
'int', while the rest of the aarch64 (and struct vcpu)
carries it as 'uint32_t'. Hence, change the prototype
to make it consistent throughout the board.

Signed-off-by: Raghavendra Rao Ananta <[email protected]>
---
tools/testing/selftests/kvm/include/aarch64/processor.h | 2 +-
tools/testing/selftests/kvm/lib/aarch64/processor.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
index 515d04a3c27d..27d8e1bb5b36 100644
--- a/tools/testing/selftests/kvm/include/aarch64/processor.h
+++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
@@ -63,7 +63,7 @@ static inline void set_reg(struct kvm_vm *vm, uint32_t vcpuid, uint64_t id, uint
vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, &reg);
}

-void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *init);
+void aarch64_vcpu_setup(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_vcpu_init *init);
void aarch64_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_vcpu_init *init, void *guest_code);

diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index db64ee206064..34f6bd47661f 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -212,7 +212,7 @@ void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
}
}

-void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *init)
+void aarch64_vcpu_setup(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_vcpu_init *init)
{
struct kvm_vcpu_init default_init = { .target = -1, };
uint64_t sctlr_el1, tcr_el1;
--
2.33.0.309.g3052b89438-goog

2021-09-14 22:35:24

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 08/15] KVM: arm64: selftests: Add support to disable and enable local IRQs

Add functions local_irq_enable() and local_irq_disable() to
enable and disable the IRQs from the guest, respectively.

Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Oliver Upton <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
.../testing/selftests/kvm/include/aarch64/processor.h | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
index 265054c24481..515d04a3c27d 100644
--- a/tools/testing/selftests/kvm/include/aarch64/processor.h
+++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
@@ -172,4 +172,14 @@ static __always_inline u32 __raw_readl(const volatile void *addr)
#define writel(v,c) ({ __iowmb(); writel_relaxed((v),(c));})
#define readl(c) ({ u32 __v = readl_relaxed(c); __iormb(__v); __v; })

+static inline void local_irq_enable(void)
+{
+ asm volatile("msr daifclr, #3" : : : "memory");
+}
+
+static inline void local_irq_disable(void)
+{
+ asm volatile("msr daifset, #3" : : : "memory");
+}
+
#endif /* SELFTEST_KVM_PROCESSOR_H */
--
2.33.0.309.g3052b89438-goog

2021-09-14 22:35:26

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 04/15] KVM: arm64: selftests: Introduce ARM64_SYS_KVM_REG

With the inclusion of sysreg.h, that brings in system register
encodings, it would be redundant to re-define register encodings
again in processor.h to use it with ARM64_SYS_REG for the KVM
functions such as set_reg() or get_reg(). Hence, add helper macro,
ARM64_SYS_KVM_REG, that converts SYS_* definitions in sysreg.h
into ARM64_SYS_REG definitions.

Also replace all the users of ARM64_SYS_REG, relying on
the encodings created in processor.h, with ARM64_SYS_KVM_REG and
remove the definitions.

Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Ricardo Koller <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
.../selftests/kvm/aarch64/debug-exceptions.c | 2 +-
.../selftests/kvm/aarch64/psci_cpu_on_test.c | 2 +-
.../selftests/kvm/include/aarch64/processor.h | 20 ++++++++++---------
.../selftests/kvm/lib/aarch64/processor.c | 16 +++++++--------
4 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
index 11fd23e21cb4..ea189d83abf7 100644
--- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
+++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
@@ -190,7 +190,7 @@ static int debug_version(struct kvm_vm *vm)
{
uint64_t id_aa64dfr0;

- get_reg(vm, VCPU_ID, ARM64_SYS_REG(ID_AA64DFR0_EL1), &id_aa64dfr0);
+ get_reg(vm, VCPU_ID, KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), &id_aa64dfr0);
return id_aa64dfr0 & 0xf;
}

diff --git a/tools/testing/selftests/kvm/aarch64/psci_cpu_on_test.c b/tools/testing/selftests/kvm/aarch64/psci_cpu_on_test.c
index 018c269990e1..4c5f6814030f 100644
--- a/tools/testing/selftests/kvm/aarch64/psci_cpu_on_test.c
+++ b/tools/testing/selftests/kvm/aarch64/psci_cpu_on_test.c
@@ -91,7 +91,7 @@ int main(void)
init.features[0] |= (1 << KVM_ARM_VCPU_POWER_OFF);
aarch64_vcpu_add_default(vm, VCPU_ID_TARGET, &init, guest_main);

- get_reg(vm, VCPU_ID_TARGET, ARM64_SYS_REG(MPIDR_EL1), &target_mpidr);
+ get_reg(vm, VCPU_ID_TARGET, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1), &target_mpidr);
vcpu_args_set(vm, VCPU_ID_SOURCE, 1, target_mpidr & MPIDR_HWID_BITMASK);
vcpu_run(vm, VCPU_ID_SOURCE);

diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
index 7989e832cafb..93797783abad 100644
--- a/tools/testing/selftests/kvm/include/aarch64/processor.h
+++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
@@ -16,15 +16,17 @@
#define ARM64_CORE_REG(x) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))

-#define CPACR_EL1 3, 0, 1, 0, 2
-#define TCR_EL1 3, 0, 2, 0, 2
-#define MAIR_EL1 3, 0, 10, 2, 0
-#define MPIDR_EL1 3, 0, 0, 0, 5
-#define TTBR0_EL1 3, 0, 2, 0, 0
-#define SCTLR_EL1 3, 0, 1, 0, 0
-#define VBAR_EL1 3, 0, 12, 0, 0
-
-#define ID_AA64DFR0_EL1 3, 0, 0, 5, 0
+/*
+ * KVM_ARM64_SYS_REG(sys_reg_id): Helper macro to convert
+ * SYS_* register definitions in asm/sysreg.h to use in KVM
+ * calls such as get_reg() and set_reg().
+ */
+#define KVM_ARM64_SYS_REG(sys_reg_id) \
+ ARM64_SYS_REG(sys_reg_Op0(sys_reg_id), \
+ sys_reg_Op1(sys_reg_id), \
+ sys_reg_CRn(sys_reg_id), \
+ sys_reg_CRm(sys_reg_id), \
+ sys_reg_Op2(sys_reg_id))

/*
* Default MAIR
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index 632b74d6b3ca..db64ee206064 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -232,10 +232,10 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *ini
* Enable FP/ASIMD to avoid trapping when accessing Q0-Q15
* registers, which the variable argument list macros do.
*/
- set_reg(vm, vcpuid, ARM64_SYS_REG(CPACR_EL1), 3 << 20);
+ set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_CPACR_EL1), 3 << 20);

- get_reg(vm, vcpuid, ARM64_SYS_REG(SCTLR_EL1), &sctlr_el1);
- get_reg(vm, vcpuid, ARM64_SYS_REG(TCR_EL1), &tcr_el1);
+ get_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_SCTLR_EL1), &sctlr_el1);
+ get_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_TCR_EL1), &tcr_el1);

switch (vm->mode) {
case VM_MODE_P52V48_4K:
@@ -273,10 +273,10 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *ini
tcr_el1 |= (1 << 8) | (1 << 10) | (3 << 12);
tcr_el1 |= (64 - vm->va_bits) /* T0SZ */;

- set_reg(vm, vcpuid, ARM64_SYS_REG(SCTLR_EL1), sctlr_el1);
- set_reg(vm, vcpuid, ARM64_SYS_REG(TCR_EL1), tcr_el1);
- set_reg(vm, vcpuid, ARM64_SYS_REG(MAIR_EL1), DEFAULT_MAIR_EL1);
- set_reg(vm, vcpuid, ARM64_SYS_REG(TTBR0_EL1), vm->pgd);
+ set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_SCTLR_EL1), sctlr_el1);
+ set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_TCR_EL1), tcr_el1);
+ set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_MAIR_EL1), DEFAULT_MAIR_EL1);
+ set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_TTBR0_EL1), vm->pgd);
}

void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
@@ -362,7 +362,7 @@ void vcpu_init_descriptor_tables(struct kvm_vm *vm, uint32_t vcpuid)
{
extern char vectors;

- set_reg(vm, vcpuid, ARM64_SYS_REG(VBAR_EL1), (uint64_t)&vectors);
+ set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_VBAR_EL1), (uint64_t)&vectors);
}

void route_exception(struct ex_regs *regs, int vector)
--
2.33.0.309.g3052b89438-goog

2021-09-14 22:35:42

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 10/15] KVM: arm64: selftests: Add guest support to get the vcpuid

At times, such as when in the interrupt handler, the guest wants
to get the vcpuid that it's running on to pull the per-cpu private
data. As a result, introduce guest_get_vcpuid() that returns the
vcpuid of the calling vcpu. The interface is architecture
independent, but defined only for arm64 as of now.

Suggested-by: Reiji Watanabe <[email protected]>
Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Ricardo Koller <[email protected]>
Reviewed-by: Reiji Watanabe <[email protected]>
---
tools/testing/selftests/kvm/include/kvm_util.h | 2 ++
tools/testing/selftests/kvm/lib/aarch64/processor.c | 6 ++++++
2 files changed, 8 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 010b59b13917..bcf05f5381ed 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -400,4 +400,6 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc);
int vm_get_stats_fd(struct kvm_vm *vm);
int vcpu_get_stats_fd(struct kvm_vm *vm, uint32_t vcpuid);

+uint32_t guest_get_vcpuid(void);
+
#endif /* SELFTEST_KVM_UTIL_H */
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index 34f6bd47661f..b4eeeafd2a70 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -277,6 +277,7 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_vcpu_init
set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_TCR_EL1), tcr_el1);
set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_MAIR_EL1), DEFAULT_MAIR_EL1);
set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_TTBR0_EL1), vm->pgd);
+ set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_TPIDR_EL1), vcpuid);
}

void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
@@ -426,3 +427,8 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
assert(vector < VECTOR_NUM);
handlers->exception_handlers[vector][0] = handler;
}
+
+uint32_t guest_get_vcpuid(void)
+{
+ return read_sysreg(tpidr_el1);
+}
--
2.33.0.309.g3052b89438-goog

2021-09-14 22:35:47

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 14/15] KVM: arm64: selftests: Add arch_timer test

Add a KVM selftest to validate the arch_timer functionality.
Primarily, the test sets up periodic timer interrupts and
validates the basic architectural expectations upon its receipt.

The test provides command-line options to configure the period
of the timer, number of iterations, and number of vCPUs.

Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
tools/testing/selftests/kvm/.gitignore | 1 +
tools/testing/selftests/kvm/Makefile | 1 +
.../selftests/kvm/aarch64/arch_timer.c | 366 ++++++++++++++++++
3 files changed, 368 insertions(+)
create mode 100644 tools/testing/selftests/kvm/aarch64/arch_timer.c

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 98053d3afbda..c6058df0cd18 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/arch_timer
/aarch64/debug-exceptions
/aarch64/get-reg-list
/aarch64/psci_cpu_on_test
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 8342f65c1d96..46d43e706b20 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -84,6 +84,7 @@ TEST_GEN_PROGS_x86_64 += set_memory_region_test
TEST_GEN_PROGS_x86_64 += steal_time
TEST_GEN_PROGS_x86_64 += kvm_binary_stats_test

+TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
TEST_GEN_PROGS_aarch64 += aarch64/psci_cpu_on_test
diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c
new file mode 100644
index 000000000000..3b6ea6a462f4
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c
@@ -0,0 +1,366 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * arch_timer.c - Tests the aarch64 timer IRQ functionality
+ *
+ * The test validates both the virtual and physical timer IRQs using
+ * CVAL and TVAL registers. This consitutes the four stages in the test.
+ * The guest's main thread configures the timer interrupt for a stage
+ * and waits for it to fire, with a timeout equal to the timer period.
+ * It asserts that the timeout doesn't exceed the timer period.
+ *
+ * On the other hand, upon receipt of an interrupt, the guest's interrupt
+ * handler validates the interrupt by checking if the architectural state
+ * is in compliance with the specifications.
+ *
+ * The test provides command-line options to configure the timer's
+ * period (-p), number of vCPUs (-n), and iterations per stage (-i).
+ *
+ * Copyright (c) 2021, Google LLC.
+ */
+
+#define _GNU_SOURCE
+
+#include <stdlib.h>
+#include <pthread.h>
+#include <linux/kvm.h>
+#include <linux/sizes.h>
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "delay.h"
+#include "arch_timer.h"
+#include "gic.h"
+#include "vgic.h"
+
+#define NR_VCPUS_DEF 4
+#define NR_TEST_ITERS_DEF 5
+#define TIMER_TEST_PERIOD_MS_DEF 10
+#define TIMER_TEST_ERR_MARGIN_US 100
+
+struct test_args {
+ int nr_vcpus;
+ int nr_iter;
+ int timer_period_ms;
+};
+
+static struct test_args test_args = {
+ .nr_vcpus = NR_VCPUS_DEF,
+ .nr_iter = NR_TEST_ITERS_DEF,
+ .timer_period_ms = TIMER_TEST_PERIOD_MS_DEF,
+};
+
+#define msecs_to_usecs(msec) ((msec) * 1000LL)
+
+#define GICD_BASE_GPA 0x8000000ULL
+#define GICR_BASE_GPA 0x80A0000ULL
+
+enum guest_stage {
+ GUEST_STAGE_VTIMER_CVAL = 1,
+ GUEST_STAGE_VTIMER_TVAL,
+ GUEST_STAGE_PTIMER_CVAL,
+ GUEST_STAGE_PTIMER_TVAL,
+ GUEST_STAGE_MAX,
+};
+
+/* Shared variables between host and guest */
+struct test_vcpu_shared_data {
+ int nr_iter;
+ enum guest_stage guest_stage;
+ uint64_t xcnt;
+};
+
+struct test_vcpu {
+ uint32_t vcpuid;
+ pthread_t pt_vcpu_run;
+ struct kvm_vm *vm;
+};
+
+static struct test_vcpu test_vcpu[KVM_MAX_VCPUS];
+static struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS];
+
+static int vtimer_irq, ptimer_irq;
+
+static void
+guest_configure_timer_action(struct test_vcpu_shared_data *shared_data)
+{
+ switch (shared_data->guest_stage) {
+ case GUEST_STAGE_VTIMER_CVAL:
+ timer_set_next_cval_ms(VIRTUAL, test_args.timer_period_ms);
+ shared_data->xcnt = timer_get_cntct(VIRTUAL);
+ timer_set_ctl(VIRTUAL, CTL_ENABLE);
+ break;
+ case GUEST_STAGE_VTIMER_TVAL:
+ timer_set_next_tval_ms(VIRTUAL, test_args.timer_period_ms);
+ shared_data->xcnt = timer_get_cntct(VIRTUAL);
+ timer_set_ctl(VIRTUAL, CTL_ENABLE);
+ break;
+ case GUEST_STAGE_PTIMER_CVAL:
+ timer_set_next_cval_ms(PHYSICAL, test_args.timer_period_ms);
+ shared_data->xcnt = timer_get_cntct(PHYSICAL);
+ timer_set_ctl(PHYSICAL, CTL_ENABLE);
+ break;
+ case GUEST_STAGE_PTIMER_TVAL:
+ timer_set_next_tval_ms(PHYSICAL, test_args.timer_period_ms);
+ shared_data->xcnt = timer_get_cntct(PHYSICAL);
+ timer_set_ctl(PHYSICAL, CTL_ENABLE);
+ break;
+ default:
+ GUEST_ASSERT(0);
+ }
+}
+
+static void guest_validate_irq(unsigned int intid,
+ struct test_vcpu_shared_data *shared_data)
+{
+ enum guest_stage stage = shared_data->guest_stage;
+ uint64_t xcnt = 0, xcnt_diff_us, cval = 0;
+ unsigned long xctl = 0;
+ unsigned int timer_irq = 0;
+
+ if (stage == GUEST_STAGE_VTIMER_CVAL ||
+ stage == GUEST_STAGE_VTIMER_TVAL) {
+ xctl = timer_get_ctl(VIRTUAL);
+ timer_set_ctl(VIRTUAL, CTL_IMASK);
+ xcnt = timer_get_cntct(VIRTUAL);
+ cval = timer_get_cval(VIRTUAL);
+ timer_irq = vtimer_irq;
+ } else if (stage == GUEST_STAGE_PTIMER_CVAL ||
+ stage == GUEST_STAGE_PTIMER_TVAL) {
+ xctl = timer_get_ctl(PHYSICAL);
+ timer_set_ctl(PHYSICAL, CTL_IMASK);
+ xcnt = timer_get_cntct(PHYSICAL);
+ cval = timer_get_cval(PHYSICAL);
+ timer_irq = ptimer_irq;
+ } else {
+ GUEST_ASSERT(0);
+ }
+
+ xcnt_diff_us = cycles_to_usec(xcnt - shared_data->xcnt);
+
+ /* Make sure we are dealing with the correct timer IRQ */
+ GUEST_ASSERT_2(intid == timer_irq, intid, timer_irq);
+
+ /* Basic 'timer condition met' check */
+ GUEST_ASSERT_3(xcnt >= cval, xcnt, cval, xcnt_diff_us);
+ GUEST_ASSERT_1(xctl & CTL_ISTATUS, xctl);
+}
+
+static void guest_irq_handler(struct ex_regs *regs)
+{
+ unsigned int intid = gic_get_and_ack_irq();
+ uint32_t cpu = guest_get_vcpuid();
+ struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[cpu];
+
+ guest_validate_irq(intid, shared_data);
+
+ WRITE_ONCE(shared_data->nr_iter, shared_data->nr_iter + 1);
+
+ gic_set_eoi(intid);
+}
+
+static void guest_run_stage(struct test_vcpu_shared_data *shared_data,
+ enum guest_stage stage)
+{
+ uint32_t irq_iter, config_iter;
+
+ shared_data->guest_stage = stage;
+ shared_data->nr_iter = 0;
+
+ for (config_iter = 0; config_iter < test_args.nr_iter; config_iter++) {
+ /* Setup the next interrupt */
+ guest_configure_timer_action(shared_data);
+
+ /* Setup a timeout for the interrupt to arrive */
+ udelay(msecs_to_usecs(test_args.timer_period_ms) +
+ TIMER_TEST_ERR_MARGIN_US);
+
+ irq_iter = READ_ONCE(shared_data->nr_iter);
+ GUEST_ASSERT_2(config_iter + 1 == irq_iter,
+ config_iter + 1, irq_iter);
+ }
+}
+
+static void guest_code(void)
+{
+ uint32_t cpu = guest_get_vcpuid();
+ struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[cpu];
+
+ local_irq_disable();
+
+ gic_init(GIC_V3, test_args.nr_vcpus,
+ (void *)GICD_BASE_GPA, (void *)GICR_BASE_GPA);
+
+ timer_set_ctl(VIRTUAL, CTL_IMASK);
+ timer_set_ctl(PHYSICAL, CTL_IMASK);
+
+ gic_irq_enable(vtimer_irq);
+ gic_irq_enable(ptimer_irq);
+ local_irq_enable();
+
+ guest_run_stage(shared_data, GUEST_STAGE_VTIMER_CVAL);
+ guest_run_stage(shared_data, GUEST_STAGE_VTIMER_TVAL);
+ guest_run_stage(shared_data, GUEST_STAGE_PTIMER_CVAL);
+ guest_run_stage(shared_data, GUEST_STAGE_PTIMER_TVAL);
+
+ GUEST_DONE();
+}
+
+static void *test_vcpu_run(void *arg)
+{
+ struct ucall uc;
+ struct test_vcpu *vcpu = arg;
+ struct kvm_vm *vm = vcpu->vm;
+ uint32_t vcpuid = vcpu->vcpuid;
+ struct test_vcpu_shared_data *shared_data = &vcpu_shared_data[vcpuid];
+
+ vcpu_run(vm, vcpuid);
+
+ switch (get_ucall(vm, vcpuid, &uc)) {
+ case UCALL_SYNC:
+ case UCALL_DONE:
+ break;
+ case UCALL_ABORT:
+ sync_global_from_guest(vm, *shared_data);
+ TEST_FAIL("%s at %s:%ld\n\tvalues: %lu, %lu; %lu, vcpu: %u; stage: %u; iter: %u",
+ (const char *)uc.args[0], __FILE__, uc.args[1],
+ uc.args[2], uc.args[3], uc.args[4], vcpuid,
+ shared_data->guest_stage, shared_data->nr_iter);
+ break;
+ default:
+ TEST_FAIL("Unexpected guest exit\n");
+ }
+
+ return NULL;
+}
+
+static void test_run(struct kvm_vm *vm)
+{
+ int i, ret;
+
+ for (i = 0; i < test_args.nr_vcpus; i++) {
+ ret = pthread_create(&test_vcpu[i].pt_vcpu_run, NULL,
+ test_vcpu_run, &test_vcpu[i]);
+ TEST_ASSERT(!ret, "Failed to create vCPU-%d pthread\n", i);
+ }
+
+ for (i = 0; i < test_args.nr_vcpus; i++)
+ pthread_join(test_vcpu[i].pt_vcpu_run, NULL);
+}
+
+static void test_init_timer_irq(struct kvm_vm *vm)
+{
+ /* Timer initid should be same for all the vCPUs, so query only vCPU-0 */
+ int vcpu0_fd = vcpu_get_fd(vm, 0);
+
+ kvm_device_access(vcpu0_fd, KVM_ARM_VCPU_TIMER_CTRL,
+ KVM_ARM_VCPU_TIMER_IRQ_PTIMER, &ptimer_irq, false);
+ kvm_device_access(vcpu0_fd, KVM_ARM_VCPU_TIMER_CTRL,
+ KVM_ARM_VCPU_TIMER_IRQ_VTIMER, &vtimer_irq, false);
+
+ sync_global_to_guest(vm, ptimer_irq);
+ sync_global_to_guest(vm, vtimer_irq);
+
+ pr_debug("ptimer_irq: %d; vtimer_irq: %d\n", ptimer_irq, vtimer_irq);
+}
+
+static struct kvm_vm *test_vm_create(void)
+{
+ struct kvm_vm *vm;
+ unsigned int i;
+ int nr_vcpus = test_args.nr_vcpus;
+
+ vm = vm_create_default_with_vcpus(nr_vcpus, 0, 0, guest_code, NULL);
+
+ vm_init_descriptor_tables(vm);
+ vm_install_exception_handler(vm, VECTOR_IRQ_CURRENT, guest_irq_handler);
+
+ for (i = 0; i < nr_vcpus; i++) {
+ vcpu_init_descriptor_tables(vm, i);
+
+ test_vcpu[i].vcpuid = i;
+ test_vcpu[i].vm = vm;
+ }
+
+ ucall_init(vm, NULL);
+ test_init_timer_irq(vm);
+ vgic_v3_setup(vm, nr_vcpus, GICD_BASE_GPA, GICR_BASE_GPA);
+
+ /* Make all the test's cmdline args visible to the guest */
+ sync_global_to_guest(vm, test_args);
+
+ return vm;
+}
+
+static void test_print_help(char *name)
+{
+ pr_info("Usage: %s [-h] [-n nr_vcpus] [-i iterations] [-p timer_period_ms]\n",
+ name);
+ pr_info("\t-n: Number of vCPUs to configure (default: %u; max: %u)\n",
+ NR_VCPUS_DEF, KVM_MAX_VCPUS);
+ pr_info("\t-i: Number of iterations per stage (default: %u)\n",
+ NR_TEST_ITERS_DEF);
+ pr_info("\t-p: Periodicity (in ms) of the guest timer (default: %u)\n",
+ TIMER_TEST_PERIOD_MS_DEF);
+ pr_info("\t-h: print this help screen\n");
+}
+
+static bool parse_args(int argc, char *argv[])
+{
+ int opt;
+
+ while ((opt = getopt(argc, argv, "hn:i:p:")) != -1) {
+ switch (opt) {
+ case 'n':
+ test_args.nr_vcpus = atoi(optarg);
+ if (test_args.nr_vcpus <= 0) {
+ pr_info("Positive value needed for -n\n");
+ goto err;
+ } else if (test_args.nr_vcpus > KVM_MAX_VCPUS) {
+ pr_info("Max allowed vCPUs: %u\n",
+ KVM_MAX_VCPUS);
+ goto err;
+ }
+ break;
+ case 'i':
+ test_args.nr_iter = atoi(optarg);
+ if (test_args.nr_iter <= 0) {
+ pr_info("Positive value needed for -i\n");
+ goto err;
+ }
+ break;
+ case 'p':
+ test_args.timer_period_ms = atoi(optarg);
+ if (test_args.timer_period_ms <= 0) {
+ pr_info("Positive value needed for -p\n");
+ goto err;
+ }
+ break;
+ case 'h':
+ default:
+ goto err;
+ }
+ }
+
+ return true;
+
+err:
+ test_print_help(argv[0]);
+ return false;
+}
+
+int main(int argc, char *argv[])
+{
+ struct kvm_vm *vm;
+
+ /* Tell stdout not to buffer its content */
+ setbuf(stdout, NULL);
+
+ if (!parse_args(argc, argv))
+ exit(KSFT_SKIP);
+
+ vm = test_vm_create();
+ test_run(vm);
+ kvm_vm_free(vm);
+
+ return 0;
+}
--
2.33.0.309.g3052b89438-goog

2021-09-14 22:35:54

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 13/15] KVM: arm64: selftests: Add host support for vGIC

Implement a simple library to perform vGIC-v3 setup
from a host point of view. This includes creating a
vGIC device, setting up distributor and redistributor
attributes, and mapping the guest physical addresses.

The definition of REDIST_REGION_ATTR_ADDR is taken from
aarch64/vgic_init test. Hence, replace the definition
by including vgic.h in the test file.

Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Ricardo Koller <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
tools/testing/selftests/kvm/Makefile | 2 +-
.../testing/selftests/kvm/aarch64/vgic_init.c | 3 +-
.../selftests/kvm/include/aarch64/vgic.h | 20 ++++++
.../testing/selftests/kvm/lib/aarch64/vgic.c | 70 +++++++++++++++++++
4 files changed, 92 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/kvm/include/aarch64/vgic.h
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/vgic.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 5476a8ddef60..8342f65c1d96 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -35,7 +35,7 @@ endif

LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/rbtree.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c lib/perf_test_util.c
LIBKVM_x86_64 = lib/x86_64/apic.c lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
-LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c lib/aarch64/handlers.S lib/aarch64/spinlock.c lib/aarch64/gic.c lib/aarch64/gic_v3.c
+LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c lib/aarch64/handlers.S lib/aarch64/spinlock.c lib/aarch64/gic.c lib/aarch64/gic_v3.c lib/aarch64/vgic.c
LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c

TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c b/tools/testing/selftests/kvm/aarch64/vgic_init.c
index 623f31a14326..157fc24f39c5 100644
--- a/tools/testing/selftests/kvm/aarch64/vgic_init.c
+++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c
@@ -13,11 +13,10 @@
#include "test_util.h"
#include "kvm_util.h"
#include "processor.h"
+#include "vgic.h"

#define NR_VCPUS 4

-#define REDIST_REGION_ATTR_ADDR(count, base, flags, index) (((uint64_t)(count) << 52) | \
- ((uint64_t)((base) >> 16) << 16) | ((uint64_t)(flags) << 12) | index)
#define REG_OFFSET(vcpu, offset) (((uint64_t)vcpu << 32) | offset)

#define GICR_TYPER 0x8
diff --git a/tools/testing/selftests/kvm/include/aarch64/vgic.h b/tools/testing/selftests/kvm/include/aarch64/vgic.h
new file mode 100644
index 000000000000..0ecfb253893c
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/aarch64/vgic.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * ARM Generic Interrupt Controller (GIC) host specific defines
+ */
+
+#ifndef SELFTEST_KVM_VGIC_H
+#define SELFTEST_KVM_VGIC_H
+
+#include <linux/kvm.h>
+
+#define REDIST_REGION_ATTR_ADDR(count, base, flags, index) \
+ (((uint64_t)(count) << 52) | \
+ ((uint64_t)((base) >> 16) << 16) | \
+ ((uint64_t)(flags) << 12) | \
+ index)
+
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+ uint64_t gicd_base_gpa, uint64_t gicr_base_gpa);
+
+#endif /* SELFTEST_KVM_VGIC_H */
diff --git a/tools/testing/selftests/kvm/lib/aarch64/vgic.c b/tools/testing/selftests/kvm/lib/aarch64/vgic.c
new file mode 100644
index 000000000000..b9b271ff520d
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/aarch64/vgic.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM Generic Interrupt Controller (GIC) v3 host support
+ */
+
+#include <linux/kvm.h>
+#include <linux/sizes.h>
+#include <asm/kvm.h>
+
+#include "kvm_util.h"
+#include "../kvm_util_internal.h"
+#include "vgic.h"
+
+/*
+ * vGIC-v3 default host setup
+ *
+ * Input args:
+ * vm - KVM VM
+ * nr_vcpus - Number of vCPUs supported by this VM
+ * gicd_base_gpa - Guest Physical Address of the Distributor region
+ * gicr_base_gpa - Guest Physical Address of the Redistributor region
+ *
+ * Output args: None
+ *
+ * Return: GIC file-descriptor or negative error code upon failure
+ *
+ * The function creates a vGIC-v3 device and maps the distributor and
+ * redistributor regions of the guest. Since it depends on the number of
+ * vCPUs for the VM, it must be called after all the vCPUs have been created.
+ */
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+ uint64_t gicd_base_gpa, uint64_t gicr_base_gpa)
+{
+ int gic_fd;
+ uint64_t redist_attr;
+ struct list_head *iter;
+ unsigned int nr_gic_pages, nr_vcpus_created = 0;
+
+ TEST_ASSERT(nr_vcpus, "Number of vCPUs cannot be empty\n");
+
+ /*
+ * Make sure that the caller is infact calling this
+ * function after all the vCPUs are added.
+ */
+ list_for_each(iter, &vm->vcpus)
+ nr_vcpus_created++;
+ TEST_ASSERT(nr_vcpus == nr_vcpus_created,
+ "Number of vCPUs requested (%u) doesn't match with the ones created for the VM (%u)\n",
+ nr_vcpus, nr_vcpus_created);
+
+ /* Distributor setup */
+ gic_fd = kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V3, false);
+ kvm_device_access(gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
+ KVM_VGIC_V3_ADDR_TYPE_DIST, &gicd_base_gpa, true);
+ nr_gic_pages = vm_calc_num_guest_pages(vm->mode, KVM_VGIC_V3_DIST_SIZE);
+ virt_map(vm, gicd_base_gpa, gicd_base_gpa, nr_gic_pages);
+
+ /* Redistributor setup */
+ redist_attr = REDIST_REGION_ATTR_ADDR(nr_vcpus, gicr_base_gpa, 0, 0);
+ kvm_device_access(gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
+ KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &redist_attr, true);
+ nr_gic_pages = vm_calc_num_guest_pages(vm->mode,
+ KVM_VGIC_V3_REDIST_SIZE * nr_vcpus);
+ virt_map(vm, gicr_base_gpa, gicr_base_gpa, nr_gic_pages);
+
+ kvm_device_access(gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+ KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
+
+ return gic_fd;
+}
--
2.33.0.309.g3052b89438-goog

2021-09-14 22:36:27

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 11/15] KVM: arm64: selftests: Add light-weight spinlock support

Add a simpler version of spinlock support for ARM64 for
the guests to use.

The implementation is loosely based on the spinlock
implementation in kvm-unit-tests.

Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Oliver Upton <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
tools/testing/selftests/kvm/Makefile | 2 +-
.../selftests/kvm/include/aarch64/spinlock.h | 13 +++++++++
.../selftests/kvm/lib/aarch64/spinlock.c | 27 +++++++++++++++++++
3 files changed, 41 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/kvm/include/aarch64/spinlock.h
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/spinlock.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 5d05801ab816..61f0d376af99 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -35,7 +35,7 @@ endif

LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/rbtree.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c lib/perf_test_util.c
LIBKVM_x86_64 = lib/x86_64/apic.c lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
-LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c lib/aarch64/handlers.S
+LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c lib/aarch64/handlers.S lib/aarch64/spinlock.c
LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c

TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
diff --git a/tools/testing/selftests/kvm/include/aarch64/spinlock.h b/tools/testing/selftests/kvm/include/aarch64/spinlock.h
new file mode 100644
index 000000000000..cf0984106d14
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/aarch64/spinlock.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef SELFTEST_KVM_ARM64_SPINLOCK_H
+#define SELFTEST_KVM_ARM64_SPINLOCK_H
+
+struct spinlock {
+ int v;
+};
+
+extern void spin_lock(struct spinlock *lock);
+extern void spin_unlock(struct spinlock *lock);
+
+#endif /* SELFTEST_KVM_ARM64_SPINLOCK_H */
diff --git a/tools/testing/selftests/kvm/lib/aarch64/spinlock.c b/tools/testing/selftests/kvm/lib/aarch64/spinlock.c
new file mode 100644
index 000000000000..a076e780be5d
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/aarch64/spinlock.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM64 Spinlock support
+ */
+#include <stdint.h>
+
+#include "spinlock.h"
+
+void spin_lock(struct spinlock *lock)
+{
+ int val, res;
+
+ asm volatile(
+ "1: ldaxr %w0, [%2]\n"
+ " cbnz %w0, 1b\n"
+ " mov %w0, #1\n"
+ " stxr %w1, %w0, [%2]\n"
+ " cbnz %w1, 1b\n"
+ : "=&r" (val), "=&r" (res)
+ : "r" (&lock->v)
+ : "memory");
+}
+
+void spin_unlock(struct spinlock *lock)
+{
+ asm volatile("stlr wzr, [%0]\n" : : "r" (&lock->v) : "memory");
+}
--
2.33.0.309.g3052b89438-goog

2021-09-14 22:36:27

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 12/15] KVM: arm64: selftests: Add basic GICv3 support

Add basic support for ARM Generic Interrupt Controller v3.
The support provides guests to setup interrupts.

The work is inspired from kvm-unit-tests and the kernel's
GIC driver (drivers/irqchip/irq-gic-v3.c).

Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
Reviewed-by: Ricardo Koller <[email protected]>
---
tools/testing/selftests/kvm/Makefile | 2 +-
.../selftests/kvm/include/aarch64/gic.h | 21 ++
tools/testing/selftests/kvm/lib/aarch64/gic.c | 95 +++++++
.../selftests/kvm/lib/aarch64/gic_private.h | 21 ++
.../selftests/kvm/lib/aarch64/gic_v3.c | 240 ++++++++++++++++++
.../selftests/kvm/lib/aarch64/gic_v3.h | 70 +++++
6 files changed, 448 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/kvm/include/aarch64/gic.h
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic.c
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic_private.h
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic_v3.c
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic_v3.h

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 61f0d376af99..5476a8ddef60 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -35,7 +35,7 @@ endif

LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/rbtree.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c lib/perf_test_util.c
LIBKVM_x86_64 = lib/x86_64/apic.c lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
-LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c lib/aarch64/handlers.S lib/aarch64/spinlock.c
+LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c lib/aarch64/handlers.S lib/aarch64/spinlock.c lib/aarch64/gic.c lib/aarch64/gic_v3.c
LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c

TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
diff --git a/tools/testing/selftests/kvm/include/aarch64/gic.h b/tools/testing/selftests/kvm/include/aarch64/gic.h
new file mode 100644
index 000000000000..85dd1e53048e
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/aarch64/gic.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * ARM Generic Interrupt Controller (GIC) specific defines
+ */
+
+#ifndef SELFTEST_KVM_GIC_H
+#define SELFTEST_KVM_GIC_H
+
+enum gic_type {
+ GIC_V3,
+ GIC_TYPE_MAX,
+};
+
+void gic_init(enum gic_type type, unsigned int nr_cpus,
+ void *dist_base, void *redist_base);
+void gic_irq_enable(unsigned int intid);
+void gic_irq_disable(unsigned int intid);
+unsigned int gic_get_and_ack_irq(void);
+void gic_set_eoi(unsigned int intid);
+
+#endif /* SELFTEST_KVM_GIC_H */
diff --git a/tools/testing/selftests/kvm/lib/aarch64/gic.c b/tools/testing/selftests/kvm/lib/aarch64/gic.c
new file mode 100644
index 000000000000..fff4fc27504d
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/aarch64/gic.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM Generic Interrupt Controller (GIC) support
+ */
+
+#include <errno.h>
+#include <linux/bits.h>
+#include <linux/sizes.h>
+
+#include "kvm_util.h"
+
+#include <gic.h>
+#include "gic_private.h"
+#include "processor.h"
+#include "spinlock.h"
+
+static const struct gic_common_ops *gic_common_ops;
+static struct spinlock gic_lock;
+
+static void gic_cpu_init(unsigned int cpu, void *redist_base)
+{
+ gic_common_ops->gic_cpu_init(cpu, redist_base);
+}
+
+static void
+gic_dist_init(enum gic_type type, unsigned int nr_cpus, void *dist_base)
+{
+ const struct gic_common_ops *gic_ops = NULL;
+
+ spin_lock(&gic_lock);
+
+ /* Distributor initialization is needed only once per VM */
+ if (gic_common_ops) {
+ spin_unlock(&gic_lock);
+ return;
+ }
+
+ if (type == GIC_V3)
+ gic_ops = &gicv3_ops;
+
+ GUEST_ASSERT(gic_ops);
+
+ gic_ops->gic_init(nr_cpus, dist_base);
+ gic_common_ops = gic_ops;
+
+ /* Make sure that the initialized data is visible to all the vCPUs */
+ dsb(sy);
+
+ spin_unlock(&gic_lock);
+}
+
+void gic_init(enum gic_type type, unsigned int nr_cpus,
+ void *dist_base, void *redist_base)
+{
+ uint32_t cpu = guest_get_vcpuid();
+
+ GUEST_ASSERT(type < GIC_TYPE_MAX);
+ GUEST_ASSERT(dist_base);
+ GUEST_ASSERT(redist_base);
+ GUEST_ASSERT(nr_cpus);
+
+ gic_dist_init(type, nr_cpus, dist_base);
+ gic_cpu_init(cpu, redist_base);
+}
+
+void gic_irq_enable(unsigned int intid)
+{
+ GUEST_ASSERT(gic_common_ops);
+ gic_common_ops->gic_irq_enable(intid);
+}
+
+void gic_irq_disable(unsigned int intid)
+{
+ GUEST_ASSERT(gic_common_ops);
+ gic_common_ops->gic_irq_disable(intid);
+}
+
+unsigned int gic_get_and_ack_irq(void)
+{
+ uint64_t irqstat;
+ unsigned int intid;
+
+ GUEST_ASSERT(gic_common_ops);
+
+ irqstat = gic_common_ops->gic_read_iar();
+ intid = irqstat & GENMASK(23, 0);
+
+ return intid;
+}
+
+void gic_set_eoi(unsigned int intid)
+{
+ GUEST_ASSERT(gic_common_ops);
+ gic_common_ops->gic_write_eoir(intid);
+}
diff --git a/tools/testing/selftests/kvm/lib/aarch64/gic_private.h b/tools/testing/selftests/kvm/lib/aarch64/gic_private.h
new file mode 100644
index 000000000000..d81d739433dc
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/aarch64/gic_private.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * ARM Generic Interrupt Controller (GIC) private defines that's only
+ * shared among the GIC library code.
+ */
+
+#ifndef SELFTEST_KVM_GIC_PRIVATE_H
+#define SELFTEST_KVM_GIC_PRIVATE_H
+
+struct gic_common_ops {
+ void (*gic_init)(unsigned int nr_cpus, void *dist_base);
+ void (*gic_cpu_init)(unsigned int cpu, void *redist_base);
+ void (*gic_irq_enable)(unsigned int intid);
+ void (*gic_irq_disable)(unsigned int intid);
+ uint64_t (*gic_read_iar)(void);
+ void (*gic_write_eoir)(uint32_t irq);
+};
+
+extern const struct gic_common_ops gicv3_ops;
+
+#endif /* SELFTEST_KVM_GIC_PRIVATE_H */
diff --git a/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c
new file mode 100644
index 000000000000..6a335c16d1fe
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM Generic Interrupt Controller (GIC) v3 support
+ */
+
+#include <linux/sizes.h>
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "delay.h"
+
+#include "gic_v3.h"
+#include "gic_private.h"
+
+struct gicv3_data {
+ void *dist_base;
+ void *redist_base[GICV3_MAX_CPUS];
+ unsigned int nr_cpus;
+ unsigned int nr_spis;
+};
+
+#define sgi_base_from_redist(redist_base) (redist_base + SZ_64K)
+
+enum gicv3_intid_range {
+ SGI_RANGE,
+ PPI_RANGE,
+ SPI_RANGE,
+ INVALID_RANGE,
+};
+
+static struct gicv3_data gicv3_data;
+
+static void gicv3_gicd_wait_for_rwp(void)
+{
+ unsigned int count = 100000; /* 1s */
+
+ while (readl(gicv3_data.dist_base + GICD_CTLR) & GICD_CTLR_RWP) {
+ GUEST_ASSERT(count--);
+ udelay(10);
+ }
+}
+
+static void gicv3_gicr_wait_for_rwp(void *redist_base)
+{
+ unsigned int count = 100000; /* 1s */
+
+ while (readl(redist_base + GICR_CTLR) & GICR_CTLR_RWP) {
+ GUEST_ASSERT(count--);
+ udelay(10);
+ }
+}
+
+static enum gicv3_intid_range get_intid_range(unsigned int intid)
+{
+ switch (intid) {
+ case 0 ... 15:
+ return SGI_RANGE;
+ case 16 ... 31:
+ return PPI_RANGE;
+ case 32 ... 1019:
+ return SPI_RANGE;
+ }
+
+ /* We should not be reaching here */
+ GUEST_ASSERT(0);
+
+ return INVALID_RANGE;
+}
+
+static uint64_t gicv3_read_iar(void)
+{
+ uint64_t irqstat = read_sysreg_s(SYS_ICC_IAR1_EL1);
+
+ dsb(sy);
+ return irqstat;
+}
+
+static void gicv3_write_eoir(uint32_t irq)
+{
+ write_sysreg_s(irq, SYS_ICC_EOIR1_EL1);
+ isb();
+}
+
+static void
+gicv3_config_irq(unsigned int intid, unsigned int offset)
+{
+ uint32_t cpu = guest_get_vcpuid();
+ uint32_t mask = 1 << (intid % 32);
+ enum gicv3_intid_range intid_range = get_intid_range(intid);
+ void *reg;
+
+ /* We care about 'cpu' only for SGIs or PPIs */
+ if (intid_range == SGI_RANGE || intid_range == PPI_RANGE) {
+ GUEST_ASSERT(cpu < gicv3_data.nr_cpus);
+
+ reg = sgi_base_from_redist(gicv3_data.redist_base[cpu]) +
+ offset;
+ writel(mask, reg);
+ gicv3_gicr_wait_for_rwp(gicv3_data.redist_base[cpu]);
+ } else if (intid_range == SPI_RANGE) {
+ reg = gicv3_data.dist_base + offset + (intid / 32) * 4;
+ writel(mask, reg);
+ gicv3_gicd_wait_for_rwp();
+ } else {
+ GUEST_ASSERT(0);
+ }
+}
+
+static void gicv3_irq_enable(unsigned int intid)
+{
+ gicv3_config_irq(intid, GICD_ISENABLER);
+}
+
+static void gicv3_irq_disable(unsigned int intid)
+{
+ gicv3_config_irq(intid, GICD_ICENABLER);
+}
+
+static void gicv3_enable_redist(void *redist_base)
+{
+ uint32_t val = readl(redist_base + GICR_WAKER);
+ unsigned int count = 100000; /* 1s */
+
+ val &= ~GICR_WAKER_ProcessorSleep;
+ writel(val, redist_base + GICR_WAKER);
+
+ /* Wait until the processor is 'active' */
+ while (readl(redist_base + GICR_WAKER) & GICR_WAKER_ChildrenAsleep) {
+ GUEST_ASSERT(count--);
+ udelay(10);
+ }
+}
+
+static inline void *gicr_base_gpa_cpu(void *redist_base, uint32_t cpu)
+{
+ /* Align all the redistributors sequentially */
+ return redist_base + cpu * SZ_64K * 2;
+}
+
+static void gicv3_cpu_init(unsigned int cpu, void *redist_base)
+{
+ void *sgi_base;
+ unsigned int i;
+ void *redist_base_cpu;
+
+ GUEST_ASSERT(cpu < gicv3_data.nr_cpus);
+
+ redist_base_cpu = gicr_base_gpa_cpu(redist_base, cpu);
+ sgi_base = sgi_base_from_redist(redist_base_cpu);
+
+ gicv3_enable_redist(redist_base_cpu);
+
+ /*
+ * Mark all the SGI and PPI interrupts as non-secure Group-1.
+ * Also, deactivate and disable them.
+ */
+ writel(~0, sgi_base + GICR_IGROUPR0);
+ writel(~0, sgi_base + GICR_ICACTIVER0);
+ writel(~0, sgi_base + GICR_ICENABLER0);
+
+ /* Set a default priority for all the SGIs and PPIs */
+ for (i = 0; i < 32; i += 4)
+ writel(GICD_INT_DEF_PRI_X4,
+ sgi_base + GICR_IPRIORITYR0 + i);
+
+ gicv3_gicr_wait_for_rwp(redist_base_cpu);
+
+ /* Enable the GIC system register (ICC_*) access */
+ write_sysreg_s(read_sysreg_s(SYS_ICC_SRE_EL1) | ICC_SRE_EL1_SRE,
+ SYS_ICC_SRE_EL1);
+
+ /* Set a default priority threshold */
+ write_sysreg_s(ICC_PMR_DEF_PRIO, SYS_ICC_PMR_EL1);
+
+ /* Enable non-secure Group-1 interrupts */
+ write_sysreg_s(ICC_IGRPEN1_EL1_ENABLE, SYS_ICC_GRPEN1_EL1);
+
+ gicv3_data.redist_base[cpu] = redist_base_cpu;
+}
+
+static void gicv3_dist_init(void)
+{
+ void *dist_base = gicv3_data.dist_base;
+ unsigned int i;
+
+ /* Disable the distributor until we set things up */
+ writel(0, dist_base + GICD_CTLR);
+ gicv3_gicd_wait_for_rwp();
+
+ /*
+ * Mark all the SPI interrupts as non-secure Group-1.
+ * Also, deactivate and disable them.
+ */
+ for (i = 32; i < gicv3_data.nr_spis; i += 32) {
+ writel(~0, dist_base + GICD_IGROUPR + i / 8);
+ writel(~0, dist_base + GICD_ICACTIVER + i / 8);
+ writel(~0, dist_base + GICD_ICENABLER + i / 8);
+ }
+
+ /* Set a default priority for all the SPIs */
+ for (i = 32; i < gicv3_data.nr_spis; i += 4)
+ writel(GICD_INT_DEF_PRI_X4,
+ dist_base + GICD_IPRIORITYR + i);
+
+ /* Wait for the settings to sync-in */
+ gicv3_gicd_wait_for_rwp();
+
+ /* Finally, enable the distributor globally with ARE */
+ writel(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A |
+ GICD_CTLR_ENABLE_G1, dist_base + GICD_CTLR);
+ gicv3_gicd_wait_for_rwp();
+}
+
+static void gicv3_init(unsigned int nr_cpus, void *dist_base)
+{
+ GUEST_ASSERT(nr_cpus <= GICV3_MAX_CPUS);
+
+ gicv3_data.nr_cpus = nr_cpus;
+ gicv3_data.dist_base = dist_base;
+ gicv3_data.nr_spis = GICD_TYPER_SPIS(
+ readl(gicv3_data.dist_base + GICD_TYPER));
+ if (gicv3_data.nr_spis > 1020)
+ gicv3_data.nr_spis = 1020;
+
+ /*
+ * Initialize only the distributor for now.
+ * The redistributor and CPU interfaces are initialized
+ * later for every PE.
+ */
+ gicv3_dist_init();
+}
+
+const struct gic_common_ops gicv3_ops = {
+ .gic_init = gicv3_init,
+ .gic_cpu_init = gicv3_cpu_init,
+ .gic_irq_enable = gicv3_irq_enable,
+ .gic_irq_disable = gicv3_irq_disable,
+ .gic_read_iar = gicv3_read_iar,
+ .gic_write_eoir = gicv3_write_eoir,
+};
diff --git a/tools/testing/selftests/kvm/lib/aarch64/gic_v3.h b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.h
new file mode 100644
index 000000000000..b51536d469a6
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/aarch64/gic_v3.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * ARM Generic Interrupt Controller (GIC) v3 specific defines
+ */
+
+#ifndef SELFTEST_KVM_GICV3_H
+#define SELFTEST_KVM_GICV3_H
+
+#include <asm/sysreg.h>
+
+/*
+ * Distributor registers
+ */
+#define GICD_CTLR 0x0000
+#define GICD_TYPER 0x0004
+#define GICD_IGROUPR 0x0080
+#define GICD_ISENABLER 0x0100
+#define GICD_ICENABLER 0x0180
+#define GICD_ICACTIVER 0x0380
+#define GICD_IPRIORITYR 0x0400
+
+/*
+ * The assumption is that the guest runs in a non-secure mode.
+ * The following bits of GICD_CTLR are defined accordingly.
+ */
+#define GICD_CTLR_RWP (1U << 31)
+#define GICD_CTLR_nASSGIreq (1U << 8)
+#define GICD_CTLR_ARE_NS (1U << 4)
+#define GICD_CTLR_ENABLE_G1A (1U << 1)
+#define GICD_CTLR_ENABLE_G1 (1U << 0)
+
+#define GICD_TYPER_SPIS(typer) ((((typer) & 0x1f) + 1) * 32)
+#define GICD_INT_DEF_PRI_X4 0xa0a0a0a0
+
+/*
+ * Redistributor registers
+ */
+#define GICR_CTLR 0x000
+#define GICR_WAKER 0x014
+
+#define GICR_CTLR_RWP (1U << 3)
+
+#define GICR_WAKER_ProcessorSleep (1U << 1)
+#define GICR_WAKER_ChildrenAsleep (1U << 2)
+
+/*
+ * Redistributor registers, offsets from SGI base
+ */
+#define GICR_IGROUPR0 GICD_IGROUPR
+#define GICR_ISENABLER0 GICD_ISENABLER
+#define GICR_ICENABLER0 GICD_ICENABLER
+#define GICR_ICACTIVER0 GICD_ICACTIVER
+#define GICR_IPRIORITYR0 GICD_IPRIORITYR
+
+/* CPU interface registers */
+#define SYS_ICC_PMR_EL1 sys_reg(3, 0, 4, 6, 0)
+#define SYS_ICC_IAR1_EL1 sys_reg(3, 0, 12, 12, 0)
+#define SYS_ICC_EOIR1_EL1 sys_reg(3, 0, 12, 12, 1)
+#define SYS_ICC_SRE_EL1 sys_reg(3, 0, 12, 12, 5)
+#define SYS_ICC_GRPEN1_EL1 sys_reg(3, 0, 12, 12, 7)
+
+#define ICC_PMR_DEF_PRIO 0xf0
+
+#define ICC_SRE_EL1_SRE (1U << 0)
+
+#define ICC_IGRPEN1_EL1_ENABLE (1U << 0)
+
+#define GICV3_MAX_CPUS 512
+
+#endif /* SELFTEST_KVM_GICV3_H */
--
2.33.0.309.g3052b89438-goog

2021-09-14 22:37:34

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 15/15] KVM: arm64: selftests: arch_timer: Support vCPU migration

Since the timer stack (hardware and KVM) is per-CPU, there
are potential chances for races to occur when the scheduler
decides to migrate a vCPU thread to a different physical CPU.
Hence, include an option to stress-test this part as well by
forcing the vCPUs to migrate across physical CPUs in the
system at a particular rate.

Originally, the bug for the fix with commit 3134cc8beb69d0d
("KVM: arm64: vgic: Resample HW pending state on deactivation")
was discovered using arch_timer test with vCPU migrations and
can be easily reproduced.

Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
.../selftests/kvm/aarch64/arch_timer.c | 115 +++++++++++++++++-
1 file changed, 114 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c
index 3b6ea6a462f4..228e7ed5531c 100644
--- a/tools/testing/selftests/kvm/aarch64/arch_timer.c
+++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c
@@ -14,6 +14,8 @@
*
* The test provides command-line options to configure the timer's
* period (-p), number of vCPUs (-n), and iterations per stage (-i).
+ * To stress-test the timer stack even more, an option to migrate the
+ * vCPUs across pCPUs (-m), at a particular rate, is also provided.
*
* Copyright (c) 2021, Google LLC.
*/
@@ -24,6 +26,8 @@
#include <pthread.h>
#include <linux/kvm.h>
#include <linux/sizes.h>
+#include <linux/bitmap.h>
+#include <sys/sysinfo.h>

#include "kvm_util.h"
#include "processor.h"
@@ -36,17 +40,20 @@
#define NR_TEST_ITERS_DEF 5
#define TIMER_TEST_PERIOD_MS_DEF 10
#define TIMER_TEST_ERR_MARGIN_US 100
+#define TIMER_TEST_MIGRATION_FREQ_MS 2

struct test_args {
int nr_vcpus;
int nr_iter;
int timer_period_ms;
+ int migration_freq_ms;
};

static struct test_args test_args = {
.nr_vcpus = NR_VCPUS_DEF,
.nr_iter = NR_TEST_ITERS_DEF,
.timer_period_ms = TIMER_TEST_PERIOD_MS_DEF,
+ .migration_freq_ms = TIMER_TEST_MIGRATION_FREQ_MS,
};

#define msecs_to_usecs(msec) ((msec) * 1000LL)
@@ -80,6 +87,9 @@ static struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS];

static int vtimer_irq, ptimer_irq;

+static unsigned long *vcpu_done_map;
+static pthread_mutex_t vcpu_done_map_lock;
+
static void
guest_configure_timer_action(struct test_vcpu_shared_data *shared_data)
{
@@ -215,6 +225,11 @@ static void *test_vcpu_run(void *arg)

vcpu_run(vm, vcpuid);

+ /* Currently, any exit from guest is an indication of completion */
+ pthread_mutex_lock(&vcpu_done_map_lock);
+ set_bit(vcpuid, vcpu_done_map);
+ pthread_mutex_unlock(&vcpu_done_map_lock);
+
switch (get_ucall(vm, vcpuid, &uc)) {
case UCALL_SYNC:
case UCALL_DONE:
@@ -233,9 +248,78 @@ static void *test_vcpu_run(void *arg)
return NULL;
}

+static uint32_t test_get_pcpu(void)
+{
+ uint32_t pcpu;
+ unsigned int nproc_conf;
+ cpu_set_t online_cpuset;
+
+ nproc_conf = get_nprocs_conf();
+ sched_getaffinity(0, sizeof(cpu_set_t), &online_cpuset);
+
+ /* Randomly find an available pCPU to place a vCPU on */
+ do {
+ pcpu = rand() % nproc_conf;
+ } while (!CPU_ISSET(pcpu, &online_cpuset));
+
+ return pcpu;
+}
+
+static int test_migrate_vcpu(struct test_vcpu *vcpu)
+{
+ int ret;
+ cpu_set_t cpuset;
+ uint32_t new_pcpu = test_get_pcpu();
+
+ CPU_ZERO(&cpuset);
+ CPU_SET(new_pcpu, &cpuset);
+
+ pr_debug("Migrating vCPU: %u to pCPU: %u\n", vcpu->vcpuid, new_pcpu);
+
+ ret = pthread_setaffinity_np(vcpu->pt_vcpu_run,
+ sizeof(cpuset), &cpuset);
+
+ /* Allow the error where the vCPU thread is already finished */
+ TEST_ASSERT(ret == 0 || ret == ESRCH,
+ "Failed to migrate the vCPU:%u to pCPU: %u; ret: %d\n",
+ vcpu->vcpuid, new_pcpu, ret);
+
+ return ret;
+}
+
+static void *test_vcpu_migration(void *arg)
+{
+ unsigned int i, n_done;
+ bool vcpu_done;
+
+ do {
+ usleep(msecs_to_usecs(test_args.migration_freq_ms));
+
+ for (n_done = 0, i = 0; i < test_args.nr_vcpus; i++) {
+ pthread_mutex_lock(&vcpu_done_map_lock);
+ vcpu_done = test_bit(i, vcpu_done_map);
+ pthread_mutex_unlock(&vcpu_done_map_lock);
+
+ if (vcpu_done) {
+ n_done++;
+ continue;
+ }
+
+ test_migrate_vcpu(&test_vcpu[i]);
+ }
+ } while (test_args.nr_vcpus != n_done);
+
+ return NULL;
+}
+
static void test_run(struct kvm_vm *vm)
{
int i, ret;
+ pthread_t pt_vcpu_migration;
+
+ pthread_mutex_init(&vcpu_done_map_lock, NULL);
+ vcpu_done_map = bitmap_alloc(test_args.nr_vcpus);
+ TEST_ASSERT(vcpu_done_map, "Failed to allocate vcpu done bitmap\n");

for (i = 0; i < test_args.nr_vcpus; i++) {
ret = pthread_create(&test_vcpu[i].pt_vcpu_run, NULL,
@@ -243,8 +327,23 @@ static void test_run(struct kvm_vm *vm)
TEST_ASSERT(!ret, "Failed to create vCPU-%d pthread\n", i);
}

+ /* Spawn a thread to control the vCPU migrations */
+ if (test_args.migration_freq_ms) {
+ srand(time(NULL));
+
+ ret = pthread_create(&pt_vcpu_migration, NULL,
+ test_vcpu_migration, NULL);
+ TEST_ASSERT(!ret, "Failed to create the migration pthread\n");
+ }
+
+
for (i = 0; i < test_args.nr_vcpus; i++)
pthread_join(test_vcpu[i].pt_vcpu_run, NULL);
+
+ if (test_args.migration_freq_ms)
+ pthread_join(pt_vcpu_migration, NULL);
+
+ bitmap_free(vcpu_done_map);
}

static void test_init_timer_irq(struct kvm_vm *vm)
@@ -301,6 +400,8 @@ static void test_print_help(char *name)
NR_TEST_ITERS_DEF);
pr_info("\t-p: Periodicity (in ms) of the guest timer (default: %u)\n",
TIMER_TEST_PERIOD_MS_DEF);
+ pr_info("\t-m: Frequency (in ms) of vCPUs to migrate to different pCPU. 0 to turn off (default: %u)\n",
+ TIMER_TEST_MIGRATION_FREQ_MS);
pr_info("\t-h: print this help screen\n");
}

@@ -308,7 +409,7 @@ static bool parse_args(int argc, char *argv[])
{
int opt;

- while ((opt = getopt(argc, argv, "hn:i:p:")) != -1) {
+ while ((opt = getopt(argc, argv, "hn:i:p:m:")) != -1) {
switch (opt) {
case 'n':
test_args.nr_vcpus = atoi(optarg);
@@ -335,6 +436,13 @@ static bool parse_args(int argc, char *argv[])
goto err;
}
break;
+ case 'm':
+ test_args.migration_freq_ms = atoi(optarg);
+ if (test_args.migration_freq_ms < 0) {
+ pr_info("0 or positive value needed for -m\n");
+ goto err;
+ }
+ break;
case 'h':
default:
goto err;
@@ -358,6 +466,11 @@ int main(int argc, char *argv[])
if (!parse_args(argc, argv))
exit(KSFT_SKIP);

+ if (test_args.migration_freq_ms && get_nprocs() < 2) {
+ print_skip("At least two physical CPUs needed for vCPU migration");
+ exit(KSFT_SKIP);
+ }
+
vm = test_vm_create();
test_run(vm);
kvm_vm_free(vm);
--
2.33.0.309.g3052b89438-goog

2021-09-15 05:56:16

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v7 09/15] KVM: arm64: selftests: Maintain consistency for vcpuid type

On Tue, Sep 14, 2021 at 10:31:08PM +0000, Raghavendra Rao Ananta wrote:
> The prototype of aarch64_vcpu_setup() accepts vcpuid as
> 'int', while the rest of the aarch64 (and struct vcpu)
> carries it as 'uint32_t'. Hence, change the prototype
> to make it consistent throughout the board.
>
> Signed-off-by: Raghavendra Rao Ananta <[email protected]>
> ---
> tools/testing/selftests/kvm/include/aarch64/processor.h | 2 +-
> tools/testing/selftests/kvm/lib/aarch64/processor.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
> index 515d04a3c27d..27d8e1bb5b36 100644
> --- a/tools/testing/selftests/kvm/include/aarch64/processor.h
> +++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
> @@ -63,7 +63,7 @@ static inline void set_reg(struct kvm_vm *vm, uint32_t vcpuid, uint64_t id, uint
> vcpu_ioctl(vm, vcpuid, KVM_SET_ONE_REG, &reg);
> }
>
> -void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *init);
> +void aarch64_vcpu_setup(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_vcpu_init *init);
> void aarch64_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid,
> struct kvm_vcpu_init *init, void *guest_code);
>
> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> index db64ee206064..34f6bd47661f 100644
> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> @@ -212,7 +212,7 @@ void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
> }
> }
>
> -void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *init)
> +void aarch64_vcpu_setup(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_vcpu_init *init)
> {
> struct kvm_vcpu_init default_init = { .target = -1, };
> uint64_t sctlr_el1, tcr_el1;
> --
> 2.33.0.309.g3052b89438-goog
>

Reviewed-by: Andrew Jones <[email protected]>

2021-09-15 05:56:24

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v7 10/15] KVM: arm64: selftests: Add guest support to get the vcpuid

On Tue, Sep 14, 2021 at 10:31:09PM +0000, Raghavendra Rao Ananta wrote:
> At times, such as when in the interrupt handler, the guest wants
> to get the vcpuid that it's running on to pull the per-cpu private
> data. As a result, introduce guest_get_vcpuid() that returns the
> vcpuid of the calling vcpu. The interface is architecture
> independent, but defined only for arm64 as of now.
>
> Suggested-by: Reiji Watanabe <[email protected]>
> Signed-off-by: Raghavendra Rao Ananta <[email protected]>
> Reviewed-by: Ricardo Koller <[email protected]>
> Reviewed-by: Reiji Watanabe <[email protected]>
> ---
> tools/testing/selftests/kvm/include/kvm_util.h | 2 ++
> tools/testing/selftests/kvm/lib/aarch64/processor.c | 6 ++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 010b59b13917..bcf05f5381ed 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -400,4 +400,6 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc);
> int vm_get_stats_fd(struct kvm_vm *vm);
> int vcpu_get_stats_fd(struct kvm_vm *vm, uint32_t vcpuid);
>
> +uint32_t guest_get_vcpuid(void);
> +
> #endif /* SELFTEST_KVM_UTIL_H */
> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> index 34f6bd47661f..b4eeeafd2a70 100644
> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> @@ -277,6 +277,7 @@ void aarch64_vcpu_setup(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_vcpu_init
> set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_TCR_EL1), tcr_el1);
> set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_MAIR_EL1), DEFAULT_MAIR_EL1);
> set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_TTBR0_EL1), vm->pgd);
> + set_reg(vm, vcpuid, KVM_ARM64_SYS_REG(SYS_TPIDR_EL1), vcpuid);
> }
>
> void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
> @@ -426,3 +427,8 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
> assert(vector < VECTOR_NUM);
> handlers->exception_handlers[vector][0] = handler;
> }
> +
> +uint32_t guest_get_vcpuid(void)
> +{
> + return read_sysreg(tpidr_el1);
> +}
> --
> 2.33.0.309.g3052b89438-goog
>

Reviewed-by: Andrew Jones <[email protected]>

2021-09-28 17:07:22

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: Re: [PATCH v7 00/15] KVM: arm64: selftests: Introduce arch_timer selftest

Hello all,

I think all the patches in the series have 'Reviewed-by' now. Please
let me know if there's anything to do from my end for it move forward.

Regards,
Raghavendra



On Tue, Sep 14, 2021 at 3:31 PM Raghavendra Rao Ananta
<[email protected]> wrote:
>
> Hello,
>
> The patch series adds a KVM selftest to validate the behavior of
> ARM's generic timer (patch-14). The test programs the timer IRQs
> periodically, and for each interrupt, it validates the behaviour
> against the architecture specifications. The test further provides
> a command-line interface to configure the number of vCPUs, the
> period of the timer, and the number of iterations that the test
> has to run for.
>
> Patch-15 adds an option to randomly migrate the vCPUs to different
> physical CPUs across the system. The bug for the fix provided by
> Marc with commit 3134cc8beb69d0d ("KVM: arm64: vgic: Resample HW
> pending state on deactivation") was discovered using arch_timer
> test with vCPU migrations.
>
> Since the test heavily depends on interrupts, patch-13 adds a host
> library to setup ARM Generic Interrupt Controller v3 (GICv3). This
> includes creating a vGIC device, setting up distributor and
> redistributor attributes, and mapping the guest physical addresses.
> Symmetrical to this, patch-12 adds a guest library to talk to the vGIC,
> which includes initializing the controller, enabling/disabling the
> interrupts, and so on.
>
> The following patches are utility patches that the above ones make use
> of:
> Patch-1 adds readl/writel support for guests to access MMIO space.
>
> Patch-2 imports arch/arm64/include/asm/sysreg.h into
> tools/arch/arm64/include/asm/ to make use of the register encodings
> and read/write definitions.
>
> Patch-3 is not directly related to the test, but makes
> aarch64/debug-exceptions.c use the read/write definitions from the
> imported sysreg.h and remove the existing definitions of read_sysreg()
> and write_sysreg().
>
> Patch-4 introduces ARM64_SYS_KVM_REG, that helps convert the SYS_*
> register encodings in sysreg.h to be acceptable by get_reg() and set_reg().
> It further replaces the users of ARM64_SYS_REG to use the new macro.
>
> Patch-5 adds the support for cpu_relax().
>
> Patch-6 adds basic arch_timer framework.
>
> Patch-7 adds udelay() support for the guests to utilize.
>
> Patch-8 adds local_irq_enable() and local_irq_disable() for the guests
> to enable/disable interrupts.
>
> Patch-9 is also unrelated to the test. It modifies the prototype of
> aarch64_vcpu_setup() to accept vcpuid as uint32_t, to keep it consistent
> with the other parts of code.
>
> Patch-10 adds the support to get the vcpuid for the guests. This allows
> them to access any cpu-centric private data in the upcoming patches.
>
> Patch-11 adds a light-weight support for spinlocks for the guests to
> use.
>
> The patch series, specifically the library support, is derived from the
> kvm-unit-tests and the kernel itself.
>
> Regards,
> Raghavendra
>
> v6 -> v7:
>
> Addressed comments by Andrew:
>
> - Changed the prototype of aarch64_vcpu_setup() to accept vcpuid as
> uint32_t.
> - Modified the prototype of guest_get_vcpuid() to return uint32_t.
> - Renamed assert messages in lib/aarch64/vgic.c to use "Number".
>
> v5 -> v6:
>
> - Corrected the syntax for write_sysreg_s in gic_v3.c (11/14) so that
> the file can be compiled with the unmodified
> arch/arm64/include/asm/sysreg.h that's imported into tools/.
>
> v4 -> v5:
>
> Addressed the comments by Andrew, Oliver, and Reiji (Thanks, again):
> - Squashed patches 17/18 and 18/18 into 3/18 and 14/18, respectively.
> - Dropped patches to keep track kvm_utils of nr_vcpus (12/18) and
> vm_get_mode() (13/18) as they were no longer needed.
> - Instead of creating the a map, exporting the vcpuid to the guest
> is done by using the TPIDR_EL1 register.
> - Just to be on the safer side, gic.c's gic_dist_init() explicitly
> checks if gic_ops is NULL.
> - Move sysreg.h from within selftests to tool/arch/arm64/include/asm/.
> - Rename ARM64_SYS_KVM_REG to KVM_ARM64_SYS_REG to improve readability.
> - Use the GIC regions' sizes from asm/kvm.h instead of re-defining it
> in the vgic host support.
> - Get the timer IRQ numbers via timer's device attributes
> (KVM_ARM_VCPU_TIMER_IRQ_PTIMER, KVM_ARM_VCPU_TIMER_IRQ_VTIMER) instead
> of depending on default numbers to be safe.
> - Add check to see if the vCPU migrations are in fact enabled, before
> looking for at least two online physical CPUs for the test.
> - Add missing blank lines in the arch_timer test.
>
> v3 -> v4:
>
> Addressed the comments by Andrew, Oliver, and Ricardo (Thank you):
> - Reimplemented get_vcpuid() by exporting a map of vcpuid:mpidr to the
> guest.
> - Import sysreg.h from arch/arm64/include/asm/sysreg.h to get the system
> register encodings and its read/write support. As a result, delete the
> existing definitions in processor.h.
> - Introduce ARM64_SYS_KVM_REG that converts SYS_* register definitions
> from sysreg.h into the encodings accepted by get_reg() and set_reg().
> - Hence, remove the existing encodings of system registers (CPACR_EL1,
> TCR_EL1, and friends) and replace all the its consumers throughout
> the selftests with ARM64_SYS_KVM_REG.
> - Keep track of number of vCPUs in 'struct kvm_vm'.
> - Add a helper method to get the KVM VM's mode.
> - Modify the vGIC host function vgic_v3_setup to make use of the above
> two helper methods, which prevents it from accepting nr_vcpus as
> an argument.
> - Move the definition of REDIST_REGION_ATTR_ADDR from lib/aarch64/vgic.c
> to include/aarch64/vgic.h.
> - Make the selftest, vgic_init.c, use the definition of REDIST_REGION_ATTR_ADDR
> from include/aarch64/vgic.h.
> - Turn ON vCPU migration by default (-m 2).
> - Add pr_debug() to log vCPU migrations. Helpful for diagnosis.
> - Change TEST_ASSERT(false,...) to TEST_FAIL() in the base arch_timer
> test.
> - Include linux/types.h for __force definitions.
> - Change the type of 'val' to 'int' in spin_lock() to match the lock
> value type.
> - Fix typos in code files and comments.
>
> v2 -> v3:
>
> - Addressed the comments from Ricardo regarding moving the vGIC host
> support for selftests to its own library.
> - Added an option (-m) to migrate the guest vCPUs to physical CPUs
> in the system.
>
> v1 -> v2:
>
> Addressed comments from Zenghui in include/aarch64/arch_timer.h:
> - Correct the header description
> - Remove unnecessary inclusion of linux/sizes.h
> - Re-arrange CTL_ defines in ascending order
> - Remove inappropriate 'return' from timer_set_* functions, which
> returns 'void'.
>
> v1: https://lore.kernel.org/kvmarm/[email protected]/
> v2: https://lore.kernel.org/kvmarm/[email protected]/
> v3: https://lore.kernel.org/kvmarm/[email protected]/
> v4: https://lore.kernel.org/kvmarm/[email protected]/
> v5: https://lore.kernel.org/kvmarm/[email protected]/
> v6: https://lore.kernel.org/kvmarm/[email protected]/
>
> Raghavendra Rao Ananta (15):
> KVM: arm64: selftests: Add MMIO readl/writel support
> tools: arm64: Import sysreg.h
> KVM: arm64: selftests: Use read/write definitions from sysreg.h
> KVM: arm64: selftests: Introduce ARM64_SYS_KVM_REG
> KVM: arm64: selftests: Add support for cpu_relax
> KVM: arm64: selftests: Add basic support for arch_timers
> KVM: arm64: selftests: Add basic support to generate delays
> KVM: arm64: selftests: Add support to disable and enable local IRQs
> KVM: arm64: selftests: Maintain consistency for vcpuid type
> KVM: arm64: selftests: Add guest support to get the vcpuid
> KVM: arm64: selftests: Add light-weight spinlock support
> KVM: arm64: selftests: Add basic GICv3 support
> KVM: arm64: selftests: Add host support for vGIC
> KVM: arm64: selftests: Add arch_timer test
> KVM: arm64: selftests: arch_timer: Support vCPU migration
>
> tools/arch/arm64/include/asm/sysreg.h | 1296 +++++++++++++++++
> tools/testing/selftests/kvm/.gitignore | 1 +
> tools/testing/selftests/kvm/Makefile | 3 +-
> .../selftests/kvm/aarch64/arch_timer.c | 479 ++++++
> .../selftests/kvm/aarch64/debug-exceptions.c | 30 +-
> .../selftests/kvm/aarch64/psci_cpu_on_test.c | 2 +-
> .../testing/selftests/kvm/aarch64/vgic_init.c | 3 +-
> .../kvm/include/aarch64/arch_timer.h | 142 ++
> .../selftests/kvm/include/aarch64/delay.h | 25 +
> .../selftests/kvm/include/aarch64/gic.h | 21 +
> .../selftests/kvm/include/aarch64/processor.h | 90 +-
> .../selftests/kvm/include/aarch64/spinlock.h | 13 +
> .../selftests/kvm/include/aarch64/vgic.h | 20 +
> .../testing/selftests/kvm/include/kvm_util.h | 2 +
> tools/testing/selftests/kvm/lib/aarch64/gic.c | 95 ++
> .../selftests/kvm/lib/aarch64/gic_private.h | 21 +
> .../selftests/kvm/lib/aarch64/gic_v3.c | 240 +++
> .../selftests/kvm/lib/aarch64/gic_v3.h | 70 +
> .../selftests/kvm/lib/aarch64/processor.c | 24 +-
> .../selftests/kvm/lib/aarch64/spinlock.c | 27 +
> .../testing/selftests/kvm/lib/aarch64/vgic.c | 70 +
> 21 files changed, 2626 insertions(+), 48 deletions(-)
> create mode 100644 tools/arch/arm64/include/asm/sysreg.h
> create mode 100644 tools/testing/selftests/kvm/aarch64/arch_timer.c
> create mode 100644 tools/testing/selftests/kvm/include/aarch64/arch_timer.h
> create mode 100644 tools/testing/selftests/kvm/include/aarch64/delay.h
> create mode 100644 tools/testing/selftests/kvm/include/aarch64/gic.h
> create mode 100644 tools/testing/selftests/kvm/include/aarch64/spinlock.h
> create mode 100644 tools/testing/selftests/kvm/include/aarch64/vgic.h
> create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic.c
> create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic_private.h
> create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic_v3.c
> create mode 100644 tools/testing/selftests/kvm/lib/aarch64/gic_v3.h
> create mode 100644 tools/testing/selftests/kvm/lib/aarch64/spinlock.c
> create mode 100644 tools/testing/selftests/kvm/lib/aarch64/vgic.c
>
> --
> 2.33.0.309.g3052b89438-goog
>

2021-09-30 08:09:06

by Zenghui Yu

[permalink] [raw]
Subject: Re: [PATCH v7 12/15] KVM: arm64: selftests: Add basic GICv3 support

On 2021/9/15 6:31, Raghavendra Rao Ananta wrote:
> +static inline void *gicr_base_gpa_cpu(void *redist_base, uint32_t cpu)
> +{
> + /* Align all the redistributors sequentially */
> + return redist_base + cpu * SZ_64K * 2;
> +}
> +
> +static void gicv3_cpu_init(unsigned int cpu, void *redist_base)
> +{
> + void *sgi_base;
> + unsigned int i;
> + void *redist_base_cpu;
> +
> + GUEST_ASSERT(cpu < gicv3_data.nr_cpus);
> +
> + redist_base_cpu = gicr_base_gpa_cpu(redist_base, cpu);

This is not 'gpa' and I'd rather open-code it directly as there's
just a single caller.

Zenghui

2021-09-30 09:44:00

by Zenghui Yu

[permalink] [raw]
Subject: Re: [PATCH v7 15/15] KVM: arm64: selftests: arch_timer: Support vCPU migration

On 2021/9/15 6:31, Raghavendra Rao Ananta wrote:
> static void test_run(struct kvm_vm *vm)
> {
> int i, ret;
> + pthread_t pt_vcpu_migration;
> +
> + pthread_mutex_init(&vcpu_done_map_lock, NULL);
> + vcpu_done_map = bitmap_alloc(test_args.nr_vcpus);

This one fails to build.

aarch64/arch_timer.c: In function 'test_run':
aarch64/arch_timer.c:321:18: warning: implicit declaration of function
'bitmap_alloc'; did you mean 'bitmap_zalloc'?
[-Wimplicit-function-declaration]
vcpu_done_map = bitmap_alloc(test_args.nr_vcpus);
^~~~~~~~~~~~
bitmap_zalloc
aarch64/arch_timer.c:321:16: warning: assignment makes pointer from
integer without a cast [-Wint-conversion]
vcpu_done_map = bitmap_alloc(test_args.nr_vcpus);
^

2021-10-04 19:09:58

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: Re: [PATCH v7 15/15] KVM: arm64: selftests: arch_timer: Support vCPU migration

On Thu, Sep 30, 2021 at 2:41 AM Zenghui Yu <[email protected]> wrote:
>
> On 2021/9/15 6:31, Raghavendra Rao Ananta wrote:
> > static void test_run(struct kvm_vm *vm)
> > {
> > int i, ret;
> > + pthread_t pt_vcpu_migration;
> > +
> > + pthread_mutex_init(&vcpu_done_map_lock, NULL);
> > + vcpu_done_map = bitmap_alloc(test_args.nr_vcpus);
>
> This one fails to build.
>
> aarch64/arch_timer.c: In function 'test_run':
> aarch64/arch_timer.c:321:18: warning: implicit declaration of function
> 'bitmap_alloc'; did you mean 'bitmap_zalloc'?
> [-Wimplicit-function-declaration]
> vcpu_done_map = bitmap_alloc(test_args.nr_vcpus);
> ^~~~~~~~~~~~
> bitmap_zalloc
> aarch64/arch_timer.c:321:16: warning: assignment makes pointer from
> integer without a cast [-Wint-conversion]
> vcpu_done_map = bitmap_alloc(test_args.nr_vcpus);
> ^

bitmap_alloc() is defined as an inline function in
tools/include/linux/bitmap.h (since 4.8), which is included as a part
of this patch. Do you mind double-checking if you have it in the
header file?
Also, the function is referenced by other tests such as
dirty_log_[perf_]]test.c. Do you see the same warnings when you
compile them too?

Regards,
Raghavendra

2021-10-04 19:10:35

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: Re: [PATCH v7 12/15] KVM: arm64: selftests: Add basic GICv3 support

On Thu, Sep 30, 2021 at 1:06 AM Zenghui Yu <[email protected]> wrote:
>
> On 2021/9/15 6:31, Raghavendra Rao Ananta wrote:
> > +static inline void *gicr_base_gpa_cpu(void *redist_base, uint32_t cpu)
> > +{
> > + /* Align all the redistributors sequentially */
> > + return redist_base + cpu * SZ_64K * 2;
> > +}
> > +
> > +static void gicv3_cpu_init(unsigned int cpu, void *redist_base)
> > +{
> > + void *sgi_base;
> > + unsigned int i;
> > + void *redist_base_cpu;
> > +
> > + GUEST_ASSERT(cpu < gicv3_data.nr_cpus);
> > +
> > + redist_base_cpu = gicr_base_gpa_cpu(redist_base, cpu);
>
> This is not 'gpa' and I'd rather open-code it directly as there's
> just a single caller.
>
> Zenghui

Thanks for catching this. I agree that mentioning 'gpa' oddballs this
function, since it's called from a guest's point of view. If there are
any other major changes in the series, I'll try to change this as
well. Else, I will try to raise a separate patch to resolve this.

Regards,
Raghavendra