Skip to contentSkip to conversation

KVM: arm64: selftests: Introduce arch_timer selftest

PATCH Superseded Raghavendra Rao Ananta 4 participants 53 messages Sep 1, 2021 – Sep 7, 2021

Subsystems (3)

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

v3 cover 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-11). 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-12 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-10 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-9 adds a guest library to talk to the vGIC, which includes initializing the controller, enabling/disabling the interrupts, and so on.

Furthermore, additional processor utilities such as accessing the MMIO (via readl/writel), read/write to assembler unsupported registers, basic delay generation, enable/disable local IRQs, and so on, are also introduced that the test/GICv3 takes advantage of (patches 1 through 8).

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

Regards,
Raghavendra

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'.

Raghavendra Rao Ananta (12):
KVM: arm64: selftests: Add MMIO readl/writel support
KVM: arm64: selftests: Add write_sysreg_s and read_sysreg_s
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: Add support to get the vcpuid from MPIDR_EL1
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/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   3 +-
 .../selftests/kvm/aarch64/arch_timer.c        | 457 ++++++++++++++++++
 .../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 | 140 +++++-
 .../selftests/kvm/include/aarch64/spinlock.h  |  13 +
 .../selftests/kvm/include/aarch64/vgic.h      |  14 +
 tools/testing/selftests/kvm/lib/aarch64/gic.c |  93 ++++
 .../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/spinlock.c      |  27 ++
 .../testing/selftests/kvm/lib/aarch64/vgic.c  |  67 +++
 15 files changed, 1332 insertions(+), 2 deletions(-)
 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
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
#1 lore raw

PATCH v3 1/12 KVM: arm64: selftests: Add MMIO readl/writel support

Define the readl() and writel() functions for the guests to access (4-byte) the MMIO region.

The routines, and their dependents, are inspired from the kernel's arch/arm64/include/asm/io.h and arch/arm64/include/asm/barrier.h.

---
 .../selftests/kvm/include/aarch64/processor.h | 45 ++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/include/aarch64/processor.h +44 -1
index c0273aefa63d..3cbaf5c1e26b 100644
@@ -130,6 +130,49 @@ void vm_install_sync_handler(struct kvm_vm *vm,
 	val;								  \
 })
 
-#define isb()	asm volatile("isb" : : : "memory")
+#define isb()		asm volatile("isb" : : : "memory")
+#define dsb(opt)	asm volatile("dsb " #opt : : : "memory")
+#define dmb(opt)	asm volatile("dmb " #opt : : : "memory")
+
+#define dma_wmb()	dmb(oshst)
+#define __iowmb()	dma_wmb()
+
+#define dma_rmb()	dmb(oshld)
+
+#define __iormb(v)							\
+({									\
+	unsigned long tmp;						\
+									\
+	dma_rmb();							\
+									\
+	/*								\
+	 * Courtesy of arch/arm64/include/asm/io.h:			\
+	 * Create a dummy control dependency from the IO read to any	\
+	 * later instructions. This ensures that a subsequent call	\
+	 * to udelay() will be ordered due to the ISB in __delay().	\
+	 */								\
+	asm volatile("eor	%0, %1, %1\n"				\
+		     "cbnz	%0, ."					\
+		     : "=r" (tmp) : "r" ((unsigned long)(v))		\
+		     : "memory");					\
+})
+
+static __always_inline void __raw_writel(u32 val, volatile void *addr)
+{
+	asm volatile("str %w0, [%1]" : : "rZ" (val), "r" (addr));
+}
+
+static __always_inline u32 __raw_readl(const volatile void *addr)
+{
+	u32 val;
+	asm volatile("ldr %w0, [%1]" : "=r" (val) : "r" (addr));
+	return val;
+}
+
+#define writel_relaxed(v,c)	((void)__raw_writel((__force u32)cpu_to_le32(v),(c)))
+#define readl_relaxed(c)	({ u32 __r = le32_to_cpu((__force __le32)__raw_readl(c)); __r; })
+
+#define writel(v,c)		({ __iowmb(); writel_relaxed((v),(c));})
+#define readl(c)		({ u32 __v = readl_relaxed(c); __iormb(__v); __v; })
 
 #endif /* SELFTEST_KVM_PROCESSOR_H */
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
Reviewed-by on v3 1/12
#2 lore raw

Re: [PATCH v3 01/12] KVM: arm64: selftests: Add MMIO readl/writel support

Quoted 16 lines · Raghavendra Rao Ananta

Define the readl() and writel() functions for the guests to access (4-byte) the MMIO region.

The routines, and their dependents, are inspired from the kernel's arch/arm64/include/asm/io.h and arch/arm64/include/asm/barrier.h.

---
 .../selftests/kvm/include/aarch64/processor.h | 45 ++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/include/aarch64/processor.h +0 -0
index c0273aefa63d..3cbaf5c1e26b 100644

tools/testing/selftests/kvm/include/aarch64/processor.h void vm_install_sync_handler(struct kvm_vm *vm, · line 133

Quoted 5 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +1 -1
@@ -130,6 +130,49 @@ void vm_install_sync_handler(struct kvm_vm *vm,
 	val;								  \
 })
 
-#define isb()	asm volatile("isb" : : : "memory")
+#define isb()		asm volatile("isb" : : : "memory")

Is this a stray diff?

Otherwise:

#3 lore raw

Re: [PATCH v3 01/12] KVM: arm64: selftests: Add MMIO readl/writel support

Quoted 21 lines · Oliver Upton
Quoted 19 lines · Raghavendra Rao Ananta

Define the readl() and writel() functions for the guests to access (4-byte) the MMIO region.

The routines, and their dependents, are inspired from the kernel's arch/arm64/include/asm/io.h and arch/arm64/include/asm/barrier.h.

---
 .../selftests/kvm/include/aarch64/processor.h | 45 ++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/include/aarch64/processor.h +0 -0
index c0273aefa63d..3cbaf5c1e26b 100644
@@ -130,6 +130,49 @@ void vm_install_sync_handler(struct kvm_vm *vm,
      val;                                                              \
 })
 
Quoted 5 lines

tools/testing/selftests/kvm/include/aarch64/processor.h void vm_install_sync_handler(struct kvm_vm *vm, · line 133

Quoted 2 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +1 -1
-#define isb()        asm volatile("isb" : : : "memory")
+#define isb()                asm volatile("isb" : : : "memory")

Is this a stray diff?

Oh no, that's intentional. Just trying to align with others below.

Regards,
Raghavendra

Quoted 3 lines

Otherwise:

#4 lore raw

Re: [PATCH v3 01/12] KVM: arm64: selftests: Add MMIO readl/writel support

Quoted 23 lines · Raghavendra Rao Ananta
Quoted 21 lines · Oliver Upton
Quoted 19 lines · Raghavendra Rao Ananta

Define the readl() and writel() functions for the guests to access (4-byte) the MMIO region.

The routines, and their dependents, are inspired from the kernel's arch/arm64/include/asm/io.h and arch/arm64/include/asm/barrier.h.

---
 .../selftests/kvm/include/aarch64/processor.h | 45 ++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/include/aarch64/processor.h +0 -0
index c0273aefa63d..3cbaf5c1e26b 100644
@@ -130,6 +130,49 @@ void vm_install_sync_handler(struct kvm_vm *vm,
      val;                                                              \
 })
 
Quoted 6 lines
Quoted 5 lines

tools/testing/selftests/kvm/include/aarch64/processor.h void vm_install_sync_handler(struct kvm_vm *vm, · line 133

Quoted 2 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +1 -1
-#define isb()        asm volatile("isb" : : : "memory")
+#define isb()                asm volatile("isb" : : : "memory")

Is this a stray diff?

Oh no, that's intentional. Just trying to align with others below.

You are of course right, I read the diff wrong and didn't think it was correctly aligned.

Thanks,
Oliver
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reviewed-by on v3 1/12
#5 lore raw

Re: [PATCH v3 01/12] KVM: arm64: selftests: Add MMIO readl/writel support

In reply to Raghavendra Rao Ananta
Quoted 30 lines · Raghavendra Rao Ananta

Define the readl() and writel() functions for the guests to access (4-byte) the MMIO region.

The routines, and their dependents, are inspired from the kernel's arch/arm64/include/asm/io.h and arch/arm64/include/asm/barrier.h.

---
 .../selftests/kvm/include/aarch64/processor.h | 45 ++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/include/aarch64/processor.h +10 -1
index c0273aefa63d..3cbaf5c1e26b 100644
@@ -130,6 +130,49 @@ void vm_install_sync_handler(struct kvm_vm *vm,
 	val;								  \
 })
 
-#define isb()	asm volatile("isb" : : : "memory")
+#define isb()		asm volatile("isb" : : : "memory")
+#define dsb(opt)	asm volatile("dsb " #opt : : : "memory")
+#define dmb(opt)	asm volatile("dmb " #opt : : : "memory")
+
+#define dma_wmb()	dmb(oshst)
+#define __iowmb()	dma_wmb()
+
+#define dma_rmb()	dmb(oshld)
+
+#define __iormb(v)							\

tools/testing/selftests/kvm/include/aarch64/processor.h void vm_install_sync_handler(struct kvm_vm *vm, · line 152

Quoted 10 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +10 -0
+({									\
+	unsigned long tmp;						\
+									\
+	dma_rmb();							\
+									\
+	/*								\
+	 * Courtesy of arch/arm64/include/asm/io.h:			\
+	 * Create a dummy control dependency from the IO read to any	\
+	 * later instructions. This ensures that a subsequent call	\
+	 * to udelay() will be ordered due to the ISB in __delay().	\

We don't have udelay or __delay yet, but I assume they're coming soon.

Quoted 11 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +11 -0
+	 */								\
+	asm volatile("eor	%0, %1, %1\n"				\
+		     "cbnz	%0, ."					\
+		     : "=r" (tmp) : "r" ((unsigned long)(v))		\
+		     : "memory");					\
+})
+
+static __always_inline void __raw_writel(u32 val, volatile void *addr)
+{
+	asm volatile("str %w0, [%1]" : : "rZ" (val), "r" (addr));
+}

tools/testing/selftests/kvm/include/aarch64/processor.h void vm_install_sync_handler(struct kvm_vm *vm, · line 173

Quoted 10 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +10 -0
+
+static __always_inline u32 __raw_readl(const volatile void *addr)
+{
+	u32 val;
+	asm volatile("ldr %w0, [%1]" : "=r" (val) : "r" (addr));
+	return val;
+}
+
+#define writel_relaxed(v,c)	((void)__raw_writel((__force u32)cpu_to_le32(v),(c)))
+#define readl_relaxed(c)	({ u32 __r = le32_to_cpu((__force __le32)__raw_readl(c)); __r; })

Might want to explicitly include linux/types.h for these __force symbols.

Quoted 7 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +3 -0
+
+#define writel(v,c)		({ __iowmb(); writel_relaxed((v),(c));})
+#define readl(c)		({ u32 __v = readl_relaxed(c); __iormb(__v); __v; })
 
 #endif /* SELFTEST_KVM_PROCESSOR_H */
Signature · 1 line
2.33.0.153.gba50c8fa24-goog

Thanks,
drew

Quoted 6 lines

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#6 lore raw

PATCH v3 2/12 KVM: arm64: selftests: Add write_sysreg_s and read_sysreg_s

In reply to Raghavendra Rao Ananta

For register names that are unsupported by the assembler or the ones without architectural names, add the macros write_sysreg_s and read_sysreg_s to support them.

The functionality is derived from kvm-unit-tests and kernel's arch/arm64/include/asm/sysreg.h.

---
 .../selftests/kvm/include/aarch64/processor.h | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +61 -0
index 3cbaf5c1e26b..082cc97ad8d3 100644
@@ -118,6 +118,67 @@ 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);
 
+/*
+ * ARMv8 ARM reserves the following encoding for system registers:
+ * (Ref: ARMv8 ARM, Section: "System instruction class encoding overview",
+ *  C5.2, version:ARM DDI 0487A.f)
+ *	[20-19] : Op0
+ *	[18-16] : Op1
+ *	[15-12] : CRn
+ *	[11-8]  : CRm
+ *	[7-5]   : Op2
+ */
+#define Op0_shift	19
+#define Op0_mask	0x3
+#define Op1_shift	16
+#define Op1_mask	0x7
+#define CRn_shift	12
+#define CRn_mask	0xf
+#define CRm_shift	8
+#define CRm_mask	0xf
+#define Op2_shift	5
+#define Op2_mask	0x7
+
+/*
+ * When accessed from guests, the ARM64_SYS_REG() doesn't work since it
+ * generates a different encoding for additional KVM processing, and is
+ * only suitable for userspace to access the register via ioctls.
+ * Hence, define a 'pure' sys_reg() here to generate the encodings as per spec.
+ */
+#define sys_reg(op0, op1, crn, crm, op2) \
+	(((op0) << Op0_shift) | ((op1) << Op1_shift) | \
+	 ((crn) << CRn_shift) | ((crm) << CRm_shift) | \
+	 ((op2) << Op2_shift))
+
+asm(
+"	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n"
+"	.equ	.L__reg_num_x\\num, \\num\n"
+"	.endr\n"
+"	.equ	.L__reg_num_xzr, 31\n"
+"\n"
+"	.macro	mrs_s, rt, sreg\n"
+"	.inst	0xd5200000|(\\sreg)|(.L__reg_num_\\rt)\n"
+"	.endm\n"
+"\n"
+"	.macro	msr_s, sreg, rt\n"
+"	.inst	0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n"
+"	.endm\n"
+);
+
+/*
+ * read_sysreg_s() and write_sysreg_s()'s 'reg' has to be encoded via sys_reg()
+ */
+#define read_sysreg_s(reg) ({						\
+	u64 __val;							\
+	asm volatile("mrs_s %0, "__stringify(reg) : "=r" (__val));	\
+	__val;								\
+})
+
+#define write_sysreg_s(reg, val) do {					\
+	u64 __val = (u64)val;						\
+	asm volatile("msr_s "__stringify(reg) ", %x0" : : "rZ" (__val));\
+} while (0)
+
 #define write_sysreg(reg, val)						  \
 ({									  \
 	u64 __val = (u64)(val);						  \
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
#7 lore raw

Re: [PATCH v3 02/12] KVM: arm64: selftests: Add write_sysreg_s and read_sysreg_s

Quoted 8 lines · Raghavendra Rao Ananta

For register names that are unsupported by the assembler or the ones without architectural names, add the macros write_sysreg_s and read_sysreg_s to support them.

The functionality is derived from kvm-unit-tests and kernel's arch/arm64/include/asm/sysreg.h.

Would it be possible to just include <asm/sysreg.h>? See tools/arch/arm64/include/asm/sysreg.h

Quoted 79 lines
---
 .../selftests/kvm/include/aarch64/processor.h | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +61 -0
index 3cbaf5c1e26b..082cc97ad8d3 100644
@@ -118,6 +118,67 @@ 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);
 
+/*
+ * ARMv8 ARM reserves the following encoding for system registers:
+ * (Ref: ARMv8 ARM, Section: "System instruction class encoding overview",
+ *  C5.2, version:ARM DDI 0487A.f)
+ *	[20-19] : Op0
+ *	[18-16] : Op1
+ *	[15-12] : CRn
+ *	[11-8]  : CRm
+ *	[7-5]   : Op2
+ */
+#define Op0_shift	19
+#define Op0_mask	0x3
+#define Op1_shift	16
+#define Op1_mask	0x7
+#define CRn_shift	12
+#define CRn_mask	0xf
+#define CRm_shift	8
+#define CRm_mask	0xf
+#define Op2_shift	5
+#define Op2_mask	0x7
+
+/*
+ * When accessed from guests, the ARM64_SYS_REG() doesn't work since it
+ * generates a different encoding for additional KVM processing, and is
+ * only suitable for userspace to access the register via ioctls.
+ * Hence, define a 'pure' sys_reg() here to generate the encodings as per spec.
+ */
+#define sys_reg(op0, op1, crn, crm, op2) \
+	(((op0) << Op0_shift) | ((op1) << Op1_shift) | \
+	 ((crn) << CRn_shift) | ((crm) << CRm_shift) | \
+	 ((op2) << Op2_shift))
+
+asm(
+"	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n"
+"	.equ	.L__reg_num_x\\num, \\num\n"
+"	.endr\n"
+"	.equ	.L__reg_num_xzr, 31\n"
+"\n"
+"	.macro	mrs_s, rt, sreg\n"
+"	.inst	0xd5200000|(\\sreg)|(.L__reg_num_\\rt)\n"
+"	.endm\n"
+"\n"
+"	.macro	msr_s, sreg, rt\n"
+"	.inst	0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n"
+"	.endm\n"
+);
+
+/*
+ * read_sysreg_s() and write_sysreg_s()'s 'reg' has to be encoded via sys_reg()
+ */
+#define read_sysreg_s(reg) ({						\
+	u64 __val;							\
+	asm volatile("mrs_s %0, "__stringify(reg) : "=r" (__val));	\
+	__val;								\
+})
+
+#define write_sysreg_s(reg, val) do {					\
+	u64 __val = (u64)val;						\
+	asm volatile("msr_s "__stringify(reg) ", %x0" : : "rZ" (__val));\
+} while (0)
+
 #define write_sysreg(reg, val)						  \
 ({									  \
 	u64 __val = (u64)(val);						  \
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
#8 lore raw

Re: [PATCH v3 02/12] KVM: arm64: selftests: Add write_sysreg_s and read_sysreg_s

Quoted 5 lines · Oliver Upton
Quoted 4 lines · Raghavendra Rao Ananta

For register names that are unsupported by the assembler or the ones without architectural names, add the macros write_sysreg_s and read_sysreg_s to support them.

Quoted 7 lines
Quoted 4 lines

The functionality is derived from kvm-unit-tests and kernel's arch/arm64/include/asm/sysreg.h.

Would it be possible to just include <asm/sysreg.h>? See tools/arch/arm64/include/asm/sysreg.h

Geez, sorry for the noise. I mistakenly searched from the root of my repository, not the tools/ directory.

In any case, you could perhaps just drop the kernel header there just to use the exact same source for kernel and selftest.

Thanks,
Oliver

Quoted 79 lines
Quoted 79 lines
---
 .../selftests/kvm/include/aarch64/processor.h | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +61 -0
index 3cbaf5c1e26b..082cc97ad8d3 100644
@@ -118,6 +118,67 @@ 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);
 
+/*
+ * ARMv8 ARM reserves the following encoding for system registers:
+ * (Ref: ARMv8 ARM, Section: "System instruction class encoding overview",
+ *  C5.2, version:ARM DDI 0487A.f)
+ *	[20-19] : Op0
+ *	[18-16] : Op1
+ *	[15-12] : CRn
+ *	[11-8]  : CRm
+ *	[7-5]   : Op2
+ */
+#define Op0_shift	19
+#define Op0_mask	0x3
+#define Op1_shift	16
+#define Op1_mask	0x7
+#define CRn_shift	12
+#define CRn_mask	0xf
+#define CRm_shift	8
+#define CRm_mask	0xf
+#define Op2_shift	5
+#define Op2_mask	0x7
+
+/*
+ * When accessed from guests, the ARM64_SYS_REG() doesn't work since it
+ * generates a different encoding for additional KVM processing, and is
+ * only suitable for userspace to access the register via ioctls.
+ * Hence, define a 'pure' sys_reg() here to generate the encodings as per spec.
+ */
+#define sys_reg(op0, op1, crn, crm, op2) \
+	(((op0) << Op0_shift) | ((op1) << Op1_shift) | \
+	 ((crn) << CRn_shift) | ((crm) << CRm_shift) | \
+	 ((op2) << Op2_shift))
+
+asm(
+"	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n"
+"	.equ	.L__reg_num_x\\num, \\num\n"
+"	.endr\n"
+"	.equ	.L__reg_num_xzr, 31\n"
+"\n"
+"	.macro	mrs_s, rt, sreg\n"
+"	.inst	0xd5200000|(\\sreg)|(.L__reg_num_\\rt)\n"
+"	.endm\n"
+"\n"
+"	.macro	msr_s, sreg, rt\n"
+"	.inst	0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n"
+"	.endm\n"
+);
+
+/*
+ * read_sysreg_s() and write_sysreg_s()'s 'reg' has to be encoded via sys_reg()
+ */
+#define read_sysreg_s(reg) ({						\
+	u64 __val;							\
+	asm volatile("mrs_s %0, "__stringify(reg) : "=r" (__val));	\
+	__val;								\
+})
+
+#define write_sysreg_s(reg, val) do {					\
+	u64 __val = (u64)val;						\
+	asm volatile("msr_s "__stringify(reg) ", %x0" : : "rZ" (__val));\
+} while (0)
+
 #define write_sysreg(reg, val)						  \
 ({									  \
 	u64 __val = (u64)(val);						  \
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
#9 lore raw

Re: [PATCH v3 02/12] KVM: arm64: selftests: Add write_sysreg_s and read_sysreg_s

Quoted 10 lines · Oliver Upton
Quoted 8 lines · Oliver Upton
Quoted 7 lines · Raghavendra Rao Ananta

For register names that are unsupported by the assembler or the ones without architectural names, add the macros write_sysreg_s and read_sysreg_s to support them.

The functionality is derived from kvm-unit-tests and kernel's arch/arm64/include/asm/sysreg.h.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Would it be possible to just include <asm/sysreg.h>? See tools/arch/arm64/include/asm/sysreg.h

Geez, sorry for the noise. I mistakenly searched from the root of my repository, not the tools/ directory.

No worries :)

Quoted 3 lines

In any case, you could perhaps just drop the kernel header there just to use the exact same source for kernel and selftest.

You mean just copy/paste the entire header? There's a lot of stuff in there which we
don't need it (yet).

Regards,
Raghavendra

Quoted 74 lines

Thanks,
Oliver

Quoted 71 lines
Quoted 71 lines
---
 .../selftests/kvm/include/aarch64/processor.h | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +59 -0
index 3cbaf5c1e26b..082cc97ad8d3 100644
@@ -118,6 +118,67 @@ 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);
 
+/*
+ * ARMv8 ARM reserves the following encoding for system registers:
+ * (Ref: ARMv8 ARM, Section: "System instruction class encoding overview",
+ *  C5.2, version:ARM DDI 0487A.f)
+ * [20-19] : Op0
+ * [18-16] : Op1
+ * [15-12] : CRn
+ * [11-8]  : CRm
+ * [7-5]   : Op2
+ */
+#define Op0_shift  19
+#define Op0_mask   0x3
+#define Op1_shift  16
+#define Op1_mask   0x7
+#define CRn_shift  12
+#define CRn_mask   0xf
+#define CRm_shift  8
+#define CRm_mask   0xf
+#define Op2_shift  5
+#define Op2_mask   0x7
+
+/*
+ * When accessed from guests, the ARM64_SYS_REG() doesn't work since it
+ * generates a different encoding for additional KVM processing, and is
+ * only suitable for userspace to access the register via ioctls.
+ * Hence, define a 'pure' sys_reg() here to generate the encodings as per spec.
+ */
+#define sys_reg(op0, op1, crn, crm, op2) \
+   (((op0) << Op0_shift) | ((op1) << Op1_shift) | \
+    ((crn) << CRn_shift) | ((crm) << CRm_shift) | \
+    ((op2) << Op2_shift))
+
+asm(
+"  .irp    num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n"
+"  .equ    .L__reg_num_x\\num, \\num\n"
+"  .endr\n"
+"  .equ    .L__reg_num_xzr, 31\n"
+"\n"
+"  .macro  mrs_s, rt, sreg\n"
+"  .inst   0xd5200000|(\\sreg)|(.L__reg_num_\\rt)\n"
+"  .endm\n"
+"\n"
+"  .macro  msr_s, sreg, rt\n"
+"  .inst   0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n"
+"  .endm\n"
+);
+
+/*
+ * read_sysreg_s() and write_sysreg_s()'s 'reg' has to be encoded via sys_reg()
+ */
+#define read_sysreg_s(reg) ({                                              \
+   u64 __val;                                                      \
+   asm volatile("mrs_s %0, "__stringify(reg) : "=r" (__val));      \
+   __val;                                                          \
+})
+
+#define write_sysreg_s(reg, val) do {                                      \
+   u64 __val = (u64)val;                                           \
+   asm volatile("msr_s "__stringify(reg) ", %x0" : : "rZ" (__val));\
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +2 -1
+} while (0)
+
 #define write_sysreg(reg, val)                                               \
 ({                                                                   \
    u64 __val = (u64)(val);                                           \
--

2.33.0.153.gba50c8fa24-goog

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#10 lore raw

Re: [PATCH v3 02/12] KVM: arm64: selftests: Add write_sysreg_s and read_sysreg_s

Quoted 19 lines · Raghavendra Rao Ananta
Quoted 10 lines · Oliver Upton
Quoted 8 lines · Oliver Upton
Quoted 7 lines · Raghavendra Rao Ananta

For register names that are unsupported by the assembler or the ones without architectural names, add the macros write_sysreg_s and read_sysreg_s to support them.

The functionality is derived from kvm-unit-tests and kernel's arch/arm64/include/asm/sysreg.h.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Would it be possible to just include <asm/sysreg.h>? See tools/arch/arm64/include/asm/sysreg.h

Geez, sorry for the noise. I mistakenly searched from the root of my repository, not the tools/ directory.

Quoted 8 lines

No worries :)

Quoted 3 lines

In any case, you could perhaps just drop the kernel header there just to use the exact same source for kernel and selftest.

You mean just copy/paste the entire header? There's a lot of stuff in there which we
don't need it (yet).

Right. It's mostly register definitions, which I don't think is too high of an overhead. Don't know where others stand, but I would prefer a header that is equivalent between kernel & selftests over a concise header.

--
Thanks,
Oliver

Quoted 82 lines
Quoted 82 lines

Thanks,
Oliver

Quoted 79 lines
Quoted 79 lines
---
 .../selftests/kvm/include/aarch64/processor.h | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +61 -1
index 3cbaf5c1e26b..082cc97ad8d3 100644
@@ -118,6 +118,67 @@ 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);
 
+/*
+ * ARMv8 ARM reserves the following encoding for system registers:
+ * (Ref: ARMv8 ARM, Section: "System instruction class encoding overview",
+ *  C5.2, version:ARM DDI 0487A.f)
+ * [20-19] : Op0
+ * [18-16] : Op1
+ * [15-12] : CRn
+ * [11-8]  : CRm
+ * [7-5]   : Op2
+ */
+#define Op0_shift  19
+#define Op0_mask   0x3
+#define Op1_shift  16
+#define Op1_mask   0x7
+#define CRn_shift  12
+#define CRn_mask   0xf
+#define CRm_shift  8
+#define CRm_mask   0xf
+#define Op2_shift  5
+#define Op2_mask   0x7
+
+/*
+ * When accessed from guests, the ARM64_SYS_REG() doesn't work since it
+ * generates a different encoding for additional KVM processing, and is
+ * only suitable for userspace to access the register via ioctls.
+ * Hence, define a 'pure' sys_reg() here to generate the encodings as per spec.
+ */
+#define sys_reg(op0, op1, crn, crm, op2) \
+   (((op0) << Op0_shift) | ((op1) << Op1_shift) | \
+    ((crn) << CRn_shift) | ((crm) << CRm_shift) | \
+    ((op2) << Op2_shift))
+
+asm(
+"  .irp    num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n"
+"  .equ    .L__reg_num_x\\num, \\num\n"
+"  .endr\n"
+"  .equ    .L__reg_num_xzr, 31\n"
+"\n"
+"  .macro  mrs_s, rt, sreg\n"
+"  .inst   0xd5200000|(\\sreg)|(.L__reg_num_\\rt)\n"
+"  .endm\n"
+"\n"
+"  .macro  msr_s, sreg, rt\n"
+"  .inst   0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n"
+"  .endm\n"
+);
+
+/*
+ * read_sysreg_s() and write_sysreg_s()'s 'reg' has to be encoded via sys_reg()
+ */
+#define read_sysreg_s(reg) ({                                              \
+   u64 __val;                                                      \
+   asm volatile("mrs_s %0, "__stringify(reg) : "=r" (__val));      \
+   __val;                                                          \
+})
+
+#define write_sysreg_s(reg, val) do {                                      \
+   u64 __val = (u64)val;                                           \
+   asm volatile("msr_s "__stringify(reg) ", %x0" : : "rZ" (__val));\
+} while (0)
+
 #define write_sysreg(reg, val)                                               \
 ({                                                                   \
    u64 __val = (u64)(val);                                           \
--

2.33.0.153.gba50c8fa24-goog

#11 lore raw

Re: [PATCH v3 02/12] KVM: arm64: selftests: Add write_sysreg_s and read_sysreg_s

Quoted 25 lines · Oliver Upton
Quoted 24 lines · Raghavendra Rao Ananta
Quoted 10 lines · Oliver Upton
Quoted 8 lines · Oliver Upton
Quoted 7 lines · Raghavendra Rao Ananta

For register names that are unsupported by the assembler or the ones without architectural names, add the macros write_sysreg_s and read_sysreg_s to support them.

The functionality is derived from kvm-unit-tests and kernel's arch/arm64/include/asm/sysreg.h.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Would it be possible to just include <asm/sysreg.h>? See tools/arch/arm64/include/asm/sysreg.h

Geez, sorry for the noise. I mistakenly searched from the root of my repository, not the tools/ directory.

No worries :)

Quoted 3 lines

In any case, you could perhaps just drop the kernel header there just to use the exact same source for kernel and selftest.

Quoted 9 lines
Quoted 3 lines

You mean just copy/paste the entire header? There's a lot of stuff in there which we
don't need it (yet).

Right. It's mostly register definitions, which I don't think is too high of an overhead. Don't know where others stand, but I would prefer a header that is equivalent between kernel & selftests over a concise header.

Until now we haven't needed the sys_reg(...) type of definitions for sysregs in selftests. In case we did, we defined the registers we needed for get/set_one_reg by their parts, e.g.

#define ID_AA64DFR0_EL1 3, 0, 0, 5, 0

allowing us to choose how we use them, ARM64_SYS_REG(...) vs. sys_reg(...).

Bringing over sysreg.h is probably a good idea though. If we do, then I'd suggest we define a new macro that allows us to convert a SYS_* register definition from sysreg.h into an ARM64_SYS_REG definition for get/set_one_reg in order to avoid redundant definitions.

Thanks,
drew

#12 lore raw

Re: [PATCH v3 02/12] KVM: arm64: selftests: Add write_sysreg_s and read_sysreg_s

Quoted 43 lines · Andrew Jones
Quoted 25 lines · Oliver Upton
Quoted 24 lines · Raghavendra Rao Ananta
Quoted 10 lines · Oliver Upton
Quoted 8 lines · Oliver Upton
Quoted 7 lines · Raghavendra Rao Ananta

For register names that are unsupported by the assembler or the ones without architectural names, add the macros write_sysreg_s and read_sysreg_s to support them.

The functionality is derived from kvm-unit-tests and kernel's arch/arm64/include/asm/sysreg.h.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Would it be possible to just include <asm/sysreg.h>? See tools/arch/arm64/include/asm/sysreg.h

Geez, sorry for the noise. I mistakenly searched from the root of my repository, not the tools/ directory.

No worries :)

Quoted 3 lines

In any case, you could perhaps just drop the kernel header there just to use the exact same source for kernel and selftest.

Quoted 9 lines
Quoted 3 lines

You mean just copy/paste the entire header? There's a lot of stuff in there which we
don't need it (yet).

Right. It's mostly register definitions, which I don't think is too high of an overhead. Don't know where others stand, but I would prefer a header that is equivalent between kernel & selftests over a concise header.

Until now we haven't needed the sys_reg(...) type of definitions for sysregs in selftests. In case we did, we defined the registers we needed for get/set_one_reg by their parts, e.g.

#define ID_AA64DFR0_EL1 3, 0, 0, 5, 0

Quoted 8 lines

allowing us to choose how we use them, ARM64_SYS_REG(...) vs. sys_reg(...).

Bringing over sysreg.h is probably a good idea though. If we do, then I'd suggest we define a new macro that allows us to convert a SYS_* register definition from sysreg.h into an ARM64_SYS_REG definition for get/set_one_reg in order to avoid redundant definitions.

I agree. Will look into it, and plan to pull the original sysreg.h into selftests.

Regards,
Raghavendra

Quoted 3 lines

Thanks,
drew

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reviewed-by on v3 2/12
#13 lore raw

Re: [PATCH v3 02/12] KVM: arm64: selftests: Add write_sysreg_s and read_sysreg_s

In reply to Raghavendra Rao Ananta
Quoted 11 lines · Raghavendra Rao Ananta

For register names that are unsupported by the assembler or the ones without architectural names, add the macros write_sysreg_s and read_sysreg_s to support them.

The functionality is derived from kvm-unit-tests and kernel's arch/arm64/include/asm/sysreg.h.

---
 .../selftests/kvm/include/aarch64/processor.h | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)

If we don't replace with an import of arch/arm64/include/asm/sysreg.h

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#14 lore raw

PATCH v3 3/12 KVM: arm64: selftests: Add support for cpu_relax

In reply to Raghavendra Rao Ananta

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.

---
 tools/testing/selftests/kvm/include/aarch64/processor.h | 5 +++++
 1 file changed, 5 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +5 -0
index 082cc97ad8d3..78df059dc974 100644
@@ -191,6 +191,11 @@ asm(
 	val;								  \
 })
 
+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")
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
Reviewed-by on v3 3/12
#15 lore raw

Re: [PATCH v3 03/12] KVM: arm64: selftests: Add support for cpu_relax

Quoted 7 lines · Raghavendra Rao Ananta

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.

Quoted 23 lines
---
 tools/testing/selftests/kvm/include/aarch64/processor.h | 5 +++++
 1 file changed, 5 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +5 -0
index 082cc97ad8d3..78df059dc974 100644
@@ -191,6 +191,11 @@ asm(
 	val;								  \
 })
 
+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")
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
Reviewed-by on v3 3/12
#16 lore raw

Re: [PATCH v3 03/12] KVM: arm64: selftests: Add support for cpu_relax

In reply to Raghavendra Rao Ananta
Quoted 7 lines · Raghavendra Rao Ananta

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.

Quoted 23 lines
---
 tools/testing/selftests/kvm/include/aarch64/processor.h | 5 +++++
 1 file changed, 5 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +5 -0
index 082cc97ad8d3..78df059dc974 100644
@@ -191,6 +191,11 @@ asm(
 	val;								  \
 })
 
+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")
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
Reviewed-by on v3 3/12
#17 lore raw

Re: [PATCH v3 03/12] KVM: arm64: selftests: Add support for cpu_relax

In reply to Raghavendra Rao Ananta
Quoted 11 lines · Raghavendra Rao Ananta

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.

---
 tools/testing/selftests/kvm/include/aarch64/processor.h | 5 +++++
 1 file changed, 5 insertions(+)
#18 lore raw

PATCH v3 4/12 KVM: arm64: selftests: Add basic support for arch_timers

In reply to Raghavendra Rao Ananta

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.

---
 .../kvm/include/aarch64/arch_timer.h          | 142 ++++++++++++++++++
 1 file changed, 142 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/arch_timer.h
tools/testing/selftests/kvm/include/aarch64/arch_timer.h new file +142 -0
new file mode 100644
index 000000000000..9df5b63abc47
@@ -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(cntv_cval_el0, cval);
+		break;
+	case PHYSICAL:
+		write_sysreg(cntp_cval_el0, cval);
+		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(cntv_tval_el0, tval);
+		break;
+	case PHYSICAL:
+		write_sysreg(cntp_tval_el0, tval);
+		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(cntv_ctl_el0, ctl);
+		break;
+	case PHYSICAL:
+		write_sysreg(cntp_ctl_el0, ctl);
+		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 */
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
#19 lore raw

Re: [PATCH v3 04/12] KVM: arm64: selftests: Add basic support for arch_timers

Quoted 64 lines · Raghavendra Rao Ananta

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.

---
 .../kvm/include/aarch64/arch_timer.h          | 142 ++++++++++++++++++
 1 file changed, 142 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/arch_timer.h
tools/testing/selftests/kvm/include/aarch64/arch_timer.h new file +48 -0
new file mode 100644
index 000000000000..9df5b63abc47
@@ -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;

tools/testing/selftests/kvm/include/aarch64/arch_timer.h line 58

Quoted 10 lines
tools/testing/selftests/kvm/include/aarch64/arch_timer.h new file +10 -0
+}
+
+static inline void timer_set_cval(enum arch_timer timer, uint64_t cval)
+{
+	switch (timer) {
+	case VIRTUAL:
+		write_sysreg(cntv_cval_el0, cval);
+		break;
+	case PHYSICAL:
+		write_sysreg(cntp_cval_el0, cval);

Huh, looks like we managed to merge a backwards write_sysreg into kvm selftests. write_sysreg in the kernel and kvm-unit-tests is (value, reg). We should post a patch fixing that before adding more calls to it.

Quoted 59 lines
tools/testing/selftests/kvm/include/aarch64/arch_timer.h new file +59 -0
+		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(cntv_tval_el0, tval);
+		break;
+	case PHYSICAL:
+		write_sysreg(cntp_tval_el0, tval);
+		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(cntv_ctl_el0, ctl);
+		break;
+	case PHYSICAL:
+		write_sysreg(cntp_ctl_el0, ctl);
+		break;
+	default:
+		GUEST_ASSERT_1(0, timer);
+	}
+
+	isb();
+}
+
+static inline uint32_t timer_get_ctl(enum arch_timer timer)
+{
+	switch (timer) {
+	case VIRTUAL:

tools/testing/selftests/kvm/include/aarch64/arch_timer.h line 127

Quoted 10 lines
tools/testing/selftests/kvm/include/aarch64/arch_timer.h new file +10 -0
+		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;
+}

I'll have to look at the test code that uses the above functions, but I wonder if it wouldn't be better to define two test functions, one for vtimer and one for ptimer where the sysreg accesses are direct, rather than all these switched wrappers.

tools/testing/selftests/kvm/include/aarch64/arch_timer.h line 140

Quoted 13 lines
tools/testing/selftests/kvm/include/aarch64/arch_timer.h new file +13 -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));
+}

I'll also look at how these wrappers are used, since open coding them may be OK.

Thanks,
drew

Quoted 10 lines
tools/testing/selftests/kvm/include/aarch64/arch_timer.h new file +2 -0
+
+#endif /* SELFTEST_KVM_ARCH_TIMER_H */
Signature · 6 lines
2.33.0.153.gba50c8fa24-goog

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#20 lore raw

PATCH v3 5/12 KVM: arm64: selftests: Add basic support to generate delays

In reply to Raghavendra Rao Ananta

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

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

---
 .../selftests/kvm/include/aarch64/delay.h     | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/delay.h
tools/testing/selftests/kvm/include/aarch64/delay.h new file +25 -0
new file mode 100644
index 000000000000..329e4f5079ea
@@ -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 */
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
Reviewed-by on v3 5/12
#21 lore raw

Re: [PATCH v3 05/12] KVM: arm64: selftests: Add basic support to generate delays

Quoted 11 lines · Raghavendra Rao Ananta

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

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

---
 .../selftests/kvm/include/aarch64/delay.h     | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/delay.h
Reviewed-by on v3 5/12
#22 lore raw

Re: [PATCH v3 05/12] KVM: arm64: selftests: Add basic support to generate delays

In reply to Raghavendra Rao Ananta
Quoted 12 lines · Raghavendra Rao Ananta

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

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

---
 .../selftests/kvm/include/aarch64/delay.h     | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/delay.h

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#23 lore raw

PATCH v3 6/12 KVM: arm64: selftests: Add support to disable and enable local IRQs

In reply to Raghavendra Rao Ananta

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

---
 .../testing/selftests/kvm/include/aarch64/processor.h  | 10 ++++++++++
 1 file changed, 10 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +10 -0
index 78df059dc974..c35bb7b8e870 100644
@@ -241,4 +241,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 */
Signature · 6 lines
2.33.0.153.gba50c8fa24-goog

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Reviewed-by on v3 6/12
#24 lore raw

Re: [PATCH v3 06/12] KVM: arm64: selftests: Add support to disable and enable local IRQs

Quoted 4 lines · Raghavendra Rao Ananta

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

Quoted 26 lines
---
 .../testing/selftests/kvm/include/aarch64/processor.h  | 10 ++++++++++
 1 file changed, 10 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +10 -0
index 78df059dc974..c35bb7b8e870 100644
@@ -241,4 +241,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 */
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
Reviewed-by on v3 6/12
#25 lore raw

Re: [PATCH v3 06/12] KVM: arm64: selftests: Add support to disable and enable local IRQs

In reply to Raghavendra Rao Ananta
Quoted 8 lines · Raghavendra Rao Ananta

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

---
 .../testing/selftests/kvm/include/aarch64/processor.h  | 10 ++++++++++
 1 file changed, 10 insertions(+)
#26 lore raw

PATCH v3 7/12 KVM: arm64: selftests: Add support to get the vcpuid from MPIDR_EL1

In reply to Raghavendra Rao Ananta

At times, such as when in the interrupt handler, the guest wants to get the vCPU-id that it's running on. As a result, introduce get_vcpuid() that parses the MPIDR_EL1 and returns the vcpuid to the requested caller.

---
 .../selftests/kvm/include/aarch64/processor.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +19 -0
index c35bb7b8e870..8b372cd427da 100644
@@ -251,4 +251,23 @@ static inline void local_irq_disable(void)
 	asm volatile("msr daifset, #3" : : : "memory");
 }
 
+#define MPIDR_LEVEL_BITS 8
+#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
+#define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
+#define MPIDR_AFFINITY_LEVEL(mpidr, level) \
+	((mpidr >> MPIDR_LEVEL_SHIFT(level)) & MPIDR_LEVEL_MASK)
+
+static inline uint32_t get_vcpuid(void)
+{
+	uint32_t vcpuid = 0;
+	uint64_t mpidr = read_sysreg(mpidr_el1);
+
+	/* KVM limits only 16 vCPUs at level 0 */
+	vcpuid = mpidr & 0x0f;
+	vcpuid |= MPIDR_AFFINITY_LEVEL(mpidr, 1) << 4;
+	vcpuid |= MPIDR_AFFINITY_LEVEL(mpidr, 2) << 12;
+
+	return vcpuid;
+}
+
 #endif /* SELFTEST_KVM_PROCESSOR_H */
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
#27 lore raw

Re: [PATCH v3 07/12] KVM: arm64: selftests: Add support to get the vcpuid from MPIDR_EL1

Quoted 26 lines · Raghavendra Rao Ananta

At times, such as when in the interrupt handler, the guest wants to get the vCPU-id that it's running on. As a result, introduce get_vcpuid() that parses the MPIDR_EL1 and returns the vcpuid to the requested caller.

---
 .../selftests/kvm/include/aarch64/processor.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +8 -0
index c35bb7b8e870..8b372cd427da 100644
@@ -251,4 +251,23 @@ static inline void local_irq_disable(void)
 	asm volatile("msr daifset, #3" : : : "memory");
 }
 
+#define MPIDR_LEVEL_BITS 8
+#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
+#define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
+#define MPIDR_AFFINITY_LEVEL(mpidr, level) \
+	((mpidr >> MPIDR_LEVEL_SHIFT(level)) & MPIDR_LEVEL_MASK)
+
+static inline uint32_t get_vcpuid(void)
+{

tools/testing/selftests/kvm/include/aarch64/processor.h static inline void local_irq_disable(void) · line 271

Quoted 10 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +10 -0
+	uint32_t vcpuid = 0;
+	uint64_t mpidr = read_sysreg(mpidr_el1);
+
+	/* KVM limits only 16 vCPUs at level 0 */
+	vcpuid = mpidr & 0x0f;
+	vcpuid |= MPIDR_AFFINITY_LEVEL(mpidr, 1) << 4;
+	vcpuid |= MPIDR_AFFINITY_LEVEL(mpidr, 2) << 12;
+
+	return vcpuid;
+}

Are we guaranteed that KVM will always compose vCPU IDs the same way? I do not believe this is guaranteed ABI.

For the base case, you could pass the vCPU ID as an arg to the guest function.

I do agree that finding the vCPU ID is a bit more challenging in an interrupt context. Maybe use a ucall to ask userspace? But of course, every test implements its own run loop, so its yet another case that tests need to handle.

Or, you could allocate an array at runtime of length KVM_CAP_MAX_VCPUS (use the KVM_CHECK_EXTENSION ioctl to get the value). Once all vCPUs are instantiated, iterate over them from userspace to populate the {MPIDR, VCPU_ID} map. You'd need to guarantee that callers initialize the vGIC *after* adding vCPUs to the guest.

--
Thanks,
Oliver

Quoted 4 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +0 -0
 #endif /* SELFTEST_KVM_PROCESSOR_H */
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
#28 lore raw

Re: [PATCH v3 07/12] KVM: arm64: selftests: Add support to get the vcpuid from MPIDR_EL1

Quoted 32 lines · Oliver Upton
Quoted 31 lines · Raghavendra Rao Ananta

At times, such as when in the interrupt handler, the guest wants to get the vCPU-id that it's running on. As a result, introduce get_vcpuid() that parses the MPIDR_EL1 and returns the vcpuid to the requested caller.

---
 .../selftests/kvm/include/aarch64/processor.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +13 -0
index c35bb7b8e870..8b372cd427da 100644
@@ -251,4 +251,23 @@ static inline void local_irq_disable(void)
 	asm volatile("msr daifset, #3" : : : "memory");
 }
 
+#define MPIDR_LEVEL_BITS 8
+#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
+#define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
+#define MPIDR_AFFINITY_LEVEL(mpidr, level) \
+	((mpidr >> MPIDR_LEVEL_SHIFT(level)) & MPIDR_LEVEL_MASK)
+
+static inline uint32_t get_vcpuid(void)
+{
+	uint32_t vcpuid = 0;
+	uint64_t mpidr = read_sysreg(mpidr_el1);
+
+	/* KVM limits only 16 vCPUs at level 0 */
+	vcpuid = mpidr & 0x0f;
Quoted 8 lines

tools/testing/selftests/kvm/include/aarch64/processor.h static inline void local_irq_disable(void) · line 271

Quoted 5 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +5 -0
+	vcpuid |= MPIDR_AFFINITY_LEVEL(mpidr, 1) << 4;
+	vcpuid |= MPIDR_AFFINITY_LEVEL(mpidr, 2) << 12;
+
+	return vcpuid;
+}

Are we guaranteed that KVM will always compose vCPU IDs the same way? I do not believe this is guaranteed ABI.

I don't believe we are. At least in QEMU we take pains to avoid that assumption.

Quoted 9 lines

For the base case, you could pass the vCPU ID as an arg to the guest function.

I do agree that finding the vCPU ID is a bit more challenging in an interrupt context. Maybe use a ucall to ask userspace? But of course, every test implements its own run loop, so its yet another case that tests need to handle.

Quoted 5 lines

Or, you could allocate an array at runtime of length KVM_CAP_MAX_VCPUS (use the KVM_CHECK_EXTENSION ioctl to get the value). Once all vCPUs are instantiated, iterate over them from userspace to populate the {MPIDR, VCPU_ID} map. You'd need to guarantee that callers initialize the vGIC *after* adding vCPUs to the guest.

I agree with this approach. It may even make sense to create a common function that returns a {cpu_id,vcpu_index} map for other tests to use.

Thanks,
drew

Quoted 14 lines

--
Thanks,
Oliver

Quoted 4 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +0 -0
 #endif /* SELFTEST_KVM_PROCESSOR_H */
Signature · 1 line
2.33.0.153.gba50c8fa24-goog

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#29 lore raw

Re: [PATCH v3 07/12] KVM: arm64: selftests: Add support to get the vcpuid from MPIDR_EL1

Quoted 55 lines · Andrew Jones
Quoted 32 lines · Oliver Upton
Quoted 31 lines · Raghavendra Rao Ananta

At times, such as when in the interrupt handler, the guest wants to get the vCPU-id that it's running on. As a result, introduce get_vcpuid() that parses the MPIDR_EL1 and returns the vcpuid to the requested caller.

---
 .../selftests/kvm/include/aarch64/processor.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
tools/testing/selftests/kvm/include/aarch64/processor.h +13 -0
index c35bb7b8e870..8b372cd427da 100644
@@ -251,4 +251,23 @@ static inline void local_irq_disable(void)
    asm volatile("msr daifset, #3" : : : "memory");
 }
 
+#define MPIDR_LEVEL_BITS 8
+#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
+#define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
+#define MPIDR_AFFINITY_LEVEL(mpidr, level) \
+   ((mpidr >> MPIDR_LEVEL_SHIFT(level)) & MPIDR_LEVEL_MASK)
+
+static inline uint32_t get_vcpuid(void)
+{
+   uint32_t vcpuid = 0;
+   uint64_t mpidr = read_sysreg(mpidr_el1);
+
+   /* KVM limits only 16 vCPUs at level 0 */
+   vcpuid = mpidr & 0x0f;
Quoted 8 lines

tools/testing/selftests/kvm/include/aarch64/processor.h static inline void local_irq_disable(void) · line 271

Quoted 5 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +5 -0
+   vcpuid |= MPIDR_AFFINITY_LEVEL(mpidr, 1) << 4;
+   vcpuid |= MPIDR_AFFINITY_LEVEL(mpidr, 2) << 12;
+
+   return vcpuid;
+}

Are we guaranteed that KVM will always compose vCPU IDs the same way? I do not believe this is guaranteed ABI.

I don't believe we are. At least in QEMU we take pains to avoid that assumption.

Quoted 9 lines

For the base case, you could pass the vCPU ID as an arg to the guest function.

I do agree that finding the vCPU ID is a bit more challenging in an interrupt context. Maybe use a ucall to ask userspace? But of course, every test implements its own run loop, so its yet another case that tests need to handle.

Quoted 9 lines
Quoted 5 lines

Or, you could allocate an array at runtime of length KVM_CAP_MAX_VCPUS (use the KVM_CHECK_EXTENSION ioctl to get the value). Once all vCPUs are instantiated, iterate over them from userspace to populate the {MPIDR, VCPU_ID} map. You'd need to guarantee that callers initialize the vGIC *after* adding vCPUs to the guest.

I agree with this approach. It may even make sense to create a common function that returns a {cpu_id,vcpu_index} map for other tests to use.

Interesting idea. I'll look into this.

Regards,
Raghavendra

Quoted 18 lines

Thanks,
drew

Quoted 14 lines

--
Thanks,
Oliver

Quoted 4 lines
tools/testing/selftests/kvm/include/aarch64/processor.h +0 -1
 #endif /* SELFTEST_KVM_PROCESSOR_H */
--

2.33.0.153.gba50c8fa24-goog

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#30 lore raw

PATCH v3 8/12 KVM: arm64: selftests: Add light-weight spinlock support

In reply to Raghavendra Rao Ananta

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.

---
 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
tools/testing/selftests/kvm/Makefile +1 -1
index 5d05801ab816..61f0d376af99 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/spinlock.h new file +13 -0
new file mode 100644
index 000000000000..cf0984106d14
@@ -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 */
tools/testing/selftests/kvm/lib/aarch64/spinlock.c new file +27 -0
new file mode 100644
index 000000000000..6d66a3dac237
@@ -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)
+{
+	uint32_t 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");
+}
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
Reviewed-by on v3 8/12
#31 lore raw

Re: [PATCH v3 08/12] KVM: arm64: selftests: Add light-weight spinlock support

Quoted 55 lines · Raghavendra Rao Ananta

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.

---
 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
tools/testing/selftests/kvm/Makefile +1 -1
index 5d05801ab816..61f0d376af99 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/spinlock.h new file +13 -0
new file mode 100644
index 000000000000..cf0984106d14
@@ -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 */
tools/testing/selftests/kvm/lib/aarch64/spinlock.c new file +1 -0
new file mode 100644
index 000000000000..6d66a3dac237
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0

tools/testing/selftests/kvm/lib/aarch64/spinlock.c line 11

Quoted 10 lines
tools/testing/selftests/kvm/lib/aarch64/spinlock.c new file +10 -0
+/*
+ * ARM64 Spinlock support
+ */
+#include <stdint.h>
+
+#include "spinlock.h"
+
+void spin_lock(struct spinlock *lock)
+{
+       uint32_t val, res;

nit: use 'int' to match the lock value type.

Quoted 17 lines
tools/testing/selftests/kvm/lib/aarch64/spinlock.c new file +16 -1
+
+       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");
+}
--
Quoted 2 lines

2.33.0.153.gba50c8fa24-goog

Otherwise, LGTM.

Reviewed-by on v3 8/12
#32 lore raw

Re: [PATCH v3 08/12] KVM: arm64: selftests: Add light-weight spinlock support

In reply to Raghavendra Rao Ananta
Quoted 71 lines · Raghavendra Rao Ananta

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.

---
 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
tools/testing/selftests/kvm/Makefile +1 -1
index 5d05801ab816..61f0d376af99 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/spinlock.h new file +13 -0
new file mode 100644
index 000000000000..cf0984106d14
@@ -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 */
tools/testing/selftests/kvm/lib/aarch64/spinlock.c new file +17 -0
new file mode 100644
index 000000000000..6d66a3dac237
@@ -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)
+{
+	uint32_t val, res;
+
+	asm volatile(
+	"1:	ldaxr	%w0, [%2]\n"
+	"	cbnz	%w0, 1b\n"
+	"	mov	%w0, #1\n"
+	"	stxr	%w1, %w0, [%2]\n"

tools/testing/selftests/kvm/lib/aarch64/spinlock.c line 27

Quoted 10 lines
tools/testing/selftests/kvm/lib/aarch64/spinlock.c new file +10 -0
+	"	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");
+}

It makes sense that the explicit barriers in kvm-unit-tests weren't also inherited, because we already have the implicit barriers with these ld/st instruction variants. (I suppose we could improve the kvm-unit-tests implementation at some point.)

Thanks,
drew

#33 lore raw

PATCH v3 9/12 KVM: arm64: selftests: Add basic GICv3 support

In reply to Raghavendra Rao Ananta

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).

---
 tools/testing/selftests/kvm/Makefile          |   2 +-
 .../selftests/kvm/include/aarch64/gic.h       |  21 ++
 tools/testing/selftests/kvm/lib/aarch64/gic.c |  93 +++++++
 .../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, 446 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
tools/testing/selftests/kvm/Makefile +1 -1
index 61f0d376af99..5476a8ddef60 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/gic.h new file +21 -0
new file mode 100644
index 000000000000..85dd1e53048e
@@ -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 */
tools/testing/selftests/kvm/lib/aarch64/gic.c new file +93 -0

93 lines not on this page · raw message

tools/testing/selftests/kvm/lib/aarch64/gic_private.h new file +21 -0
new file mode 100644
index 000000000000..d81d739433dc
@@ -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 */
tools/testing/selftests/kvm/lib/aarch64/gic_v3.c new file +240 -0

240 lines not on this page · raw message

tools/testing/selftests/kvm/lib/aarch64/gic_v3.h new file +70 -0

70 lines not on this page · raw message

Signature · 6 lines
2.33.0.153.gba50c8fa24-goog

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Reviewed-by on v3 9/12
#34 lore raw

Re: [PATCH v3 09/12] KVM: arm64: selftests: Add basic GICv3 support

Quoted 69 lines · Raghavendra Rao Ananta

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).

---
 tools/testing/selftests/kvm/Makefile          |   2 +-
 .../selftests/kvm/include/aarch64/gic.h       |  21 ++
 tools/testing/selftests/kvm/lib/aarch64/gic.c |  93 +++++++
 .../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, 446 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
tools/testing/selftests/kvm/Makefile +1 -1
index 61f0d376af99..5476a8ddef60 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/gic.h new file +21 -0
new file mode 100644
index 000000000000..85dd1e53048e
@@ -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 */
tools/testing/selftests/kvm/lib/aarch64/gic.c new file +2 -0
new file mode 100644
index 000000000000..b0b67f5aeaa6
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0
+/*

tools/testing/selftests/kvm/lib/aarch64/gic.c line 12

Quoted 10 lines
tools/testing/selftests/kvm/lib/aarch64/gic.c new file +10 -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>

"gic.h"

Quoted 433 lines
tools/testing/selftests/kvm/lib/aarch64/gic.c new file +81 -0
+#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;
+
+	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;
+
+	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 = 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);
+}
tools/testing/selftests/kvm/lib/aarch64/gic_private.h new file +21 -0
new file mode 100644
index 000000000000..d81d739433dc
@@ -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 */
tools/testing/selftests/kvm/lib/aarch64/gic_v3.c new file +240 -0
new file mode 100644
index 000000000000..4b635ca6a8cb
@@ -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(SYS_ICC_EOIR1_EL1, irq);
+	isb();
+}
+
+static void
+gicv3_config_irq(unsigned int intid, unsigned int offset)
+{
+	uint32_t cpu = 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(SYS_ICC_SRE_EL1,
+			read_sysreg_s(SYS_ICC_SRE_EL1) | ICC_SRE_EL1_SRE);
+
+	/* Set a default priority threshold */
+	write_sysreg_s(SYS_ICC_PMR_EL1, ICC_PMR_DEF_PRIO);
+
+	/* Enable non-secure Group-1 interrupts */
+	write_sysreg_s(SYS_ICC_GRPEN1_EL1, ICC_IGRPEN1_EL1_ENABLE);
+
+	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,
+};
tools/testing/selftests/kvm/lib/aarch64/gic_v3.h new file +70 -0
new file mode 100644
index 000000000000..d41195e347b3
@@ -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 "processor.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 */
Signature · 1 line
2.33.0.153.gba50c8fa24-goog

Looks good to me. I also see some nice stuff to bring over to kvm-unit-tests in order to make some improvements there.

Thanks,
drew

#35 lore raw

PATCH v3 10/12 KVM: arm64: selftests: Add host support for vGIC

In reply to Raghavendra Rao Ananta

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

---
 tools/testing/selftests/kvm/Makefile          |  2 +-
 .../selftests/kvm/include/aarch64/vgic.h      | 14 ++++
 .../testing/selftests/kvm/lib/aarch64/vgic.c  | 67 +++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/vgic.h
 create mode 100644 tools/testing/selftests/kvm/lib/aarch64/vgic.c
tools/testing/selftests/kvm/Makefile +1 -1
index 5476a8ddef60..8342f65c1d96 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/vgic.h new file +14 -0
new file mode 100644
index 000000000000..45bbf238147a
@@ -0,0 +1,14 @@
+/* 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>
+
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+		uint64_t gicd_base_gpa, uint64_t gicr_base_gpa, uint32_t slot);
+
+#endif /* SELFTEST_KVM_VGIC_H */
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +67 -0

67 lines not on this page · raw message

Signature · 6 lines
2.33.0.153.gba50c8fa24-goog

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
#36 lore raw

Re: [PATCH v3 10/12] KVM: arm64: selftests: Add host support for vGIC

Quoted 96 lines · Raghavendra Rao Ananta

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

---
 tools/testing/selftests/kvm/Makefile          |  2 +-
 .../selftests/kvm/include/aarch64/vgic.h      | 14 ++++
 .../testing/selftests/kvm/lib/aarch64/vgic.c  | 67 +++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/vgic.h
 create mode 100644 tools/testing/selftests/kvm/lib/aarch64/vgic.c
tools/testing/selftests/kvm/Makefile +1 -1
index 5476a8ddef60..8342f65c1d96 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/vgic.h new file +14 -0
new file mode 100644
index 000000000000..45bbf238147a
@@ -0,0 +1,14 @@
+/* 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>
+
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+		uint64_t gicd_base_gpa, uint64_t gicr_base_gpa, uint32_t slot);
+
+#endif /* SELFTEST_KVM_VGIC_H */
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +42 -0
new file mode 100644
index 000000000000..a0e4b986d335
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM Generic Interrupt Controller (GIC) v3 host support
+ */
+
+#include <linux/kvm.h>
+#include <linux/sizes.h>
+
+#include "kvm_util.h"
+
+#define VGIC_V3_GICD_SZ		(SZ_64K)
+#define VGIC_V3_GICR_SZ		(2 * SZ_64K)
+
+#define REDIST_REGION_ATTR_ADDR(count, base, flags, index) \
+	(((uint64_t)(count) << 52) | \
+	((uint64_t)((base) >> 16) << 16) | \
+	((uint64_t)(flags) << 12) | \
+	index)
+
+static void vgic_v3_map(struct kvm_vm *vm, uint64_t addr, unsigned int size)
+{
+	unsigned int n_pages = DIV_ROUND_UP(size, vm_get_page_size(vm));
+
+	virt_map(vm, addr, addr, n_pages);
+}
+
+/*
+ * vGIC-v3 default host setup
+ *
+ * Input args:
+ *	vm - KVM VM
+ *	nr_vcpus - Number of vCPUs for 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.
+ */

tools/testing/selftests/kvm/lib/aarch64/vgic.c line 52

Quoted 10 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +10 -0
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+		uint64_t gicd_base_gpa, uint64_t gicr_base_gpa)
+{
+	uint64_t redist_attr;
+	int gic_fd;
+
+	TEST_ASSERT(nr_vcpus <= KVM_MAX_VCPUS,
+			"Invalid number of CPUs: %u\n", nr_vcpus);
+
+	gic_fd = kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V3, false);

Nit: you can return early if gic_fd is bad.

tools/testing/selftests/kvm/lib/aarch64/vgic.c line 56

Quoted 4 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +4 -0
+
+	kvm_device_access(gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
+			KVM_VGIC_V3_ADDR_TYPE_DIST, &gicd_base_gpa, true);
+	vgic_v3_map(vm, gicd_base_gpa, VGIC_V3_GICD_SZ);

vgic_v3_map() implies that it's doing something vgic specific, when it's just converting bytes to pages. What about something like the following?

	virt_map(vm, addr, addr, VM_BYTES_TO_PAGES(vm, VGIC_V3_GICD_SZ));

and you add a VM_BYTES_TO_PAGES macro to include/kvm_util.h? I think this macro can be useful to others.

Quoted 14 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +11 -0
+
+	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);
+	vgic_v3_map(vm, gicr_base_gpa, VGIC_V3_GICR_SZ * nr_vcpus);
+
+	kvm_device_access(gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+				KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
+
+	return gic_fd;
+}
Signature · 1 line
2.33.0.153.gba50c8fa24-goog
#37 lore raw

Re: [PATCH v3 10/12] KVM: arm64: selftests: Add host support for vGIC

Quoted 102 lines · Ricardo Koller
Quoted 100 lines · Raghavendra Rao Ananta

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

---
 tools/testing/selftests/kvm/Makefile          |  2 +-
 .../selftests/kvm/include/aarch64/vgic.h      | 14 ++++
 .../testing/selftests/kvm/lib/aarch64/vgic.c  | 67 +++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/vgic.h
 create mode 100644 tools/testing/selftests/kvm/lib/aarch64/vgic.c
tools/testing/selftests/kvm/Makefile +1 -1
index 5476a8ddef60..8342f65c1d96 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/vgic.h new file +14 -0
new file mode 100644
index 000000000000..45bbf238147a
@@ -0,0 +1,14 @@
+/* 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>
+
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+             uint64_t gicd_base_gpa, uint64_t gicr_base_gpa, uint32_t slot);
+
+#endif /* SELFTEST_KVM_VGIC_H */
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +46 -0
new file mode 100644
index 000000000000..a0e4b986d335
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM Generic Interrupt Controller (GIC) v3 host support
+ */
+
+#include <linux/kvm.h>
+#include <linux/sizes.h>
+
+#include "kvm_util.h"
+
+#define VGIC_V3_GICD_SZ              (SZ_64K)
+#define VGIC_V3_GICR_SZ              (2 * SZ_64K)
+
+#define REDIST_REGION_ATTR_ADDR(count, base, flags, index) \
+     (((uint64_t)(count) << 52) | \
+     ((uint64_t)((base) >> 16) << 16) | \
+     ((uint64_t)(flags) << 12) | \
+     index)
+
+static void vgic_v3_map(struct kvm_vm *vm, uint64_t addr, unsigned int size)
+{
+     unsigned int n_pages = DIV_ROUND_UP(size, vm_get_page_size(vm));
+
+     virt_map(vm, addr, addr, n_pages);
+}
+
+/*
+ * vGIC-v3 default host setup
+ *
+ * Input args:
+ *   vm - KVM VM
+ *   nr_vcpus - Number of vCPUs for 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.
+ */
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+             uint64_t gicd_base_gpa, uint64_t gicr_base_gpa)
+{
+     uint64_t redist_attr;
Quoted 9 lines

tools/testing/selftests/kvm/lib/aarch64/vgic.c line 52

Quoted 6 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +6 -0
+     int gic_fd;
+
+     TEST_ASSERT(nr_vcpus <= KVM_MAX_VCPUS,
+                     "Invalid number of CPUs: %u\n", nr_vcpus);
+
+     gic_fd = kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V3, false);

Nit: you can return early if gic_fd is bad.

kvm_create_device() already takes care of this and the test would simply fail if gic_fd was bad.
So, I think we can positively move on :)

Quoted 5 lines

tools/testing/selftests/kvm/lib/aarch64/vgic.c line 56

Quoted 4 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +4 -0
+
+     kvm_device_access(gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
+                     KVM_VGIC_V3_ADDR_TYPE_DIST, &gicd_base_gpa, true);
+     vgic_v3_map(vm, gicd_base_gpa, VGIC_V3_GICD_SZ);
Quoted 8 lines

vgic_v3_map() implies that it's doing something vgic specific, when it's just converting bytes to pages. What about something like the following?

        virt_map(vm, addr, addr, VM_BYTES_TO_PAGES(vm, VGIC_V3_GICD_SZ));

and you add a VM_BYTES_TO_PAGES macro to include/kvm_util.h? I think this macro can be useful to others.

Yeah, good idea. Will do.

Regards,
Raghavendra

Quoted 14 lines
Quoted 14 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +11 -1
+
+     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);
+     vgic_v3_map(vm, gicr_base_gpa, VGIC_V3_GICR_SZ * nr_vcpus);
+
+     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.153.gba50c8fa24-goog

Reviewed-by on v3 10/12
#38 lore raw

Re: [PATCH v3 10/12] KVM: arm64: selftests: Add host support for vGIC

In reply to Raghavendra Rao Ananta
Quoted 1 line · Raghavendra Rao Ananta

Implement a simple library to do perform vGIC-v3

s/do//

Quoted 1 line

setup from a host of view.

^ point

Quoted 61 lines

This includes creating
a vGIC device, setting up distributor and redistributor attributes, and mapping the guest physical addresses.

---
 tools/testing/selftests/kvm/Makefile          |  2 +-
 .../selftests/kvm/include/aarch64/vgic.h      | 14 ++++
 .../testing/selftests/kvm/lib/aarch64/vgic.c  | 67 +++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/vgic.h
 create mode 100644 tools/testing/selftests/kvm/lib/aarch64/vgic.c
tools/testing/selftests/kvm/Makefile +1 -1
index 5476a8ddef60..8342f65c1d96 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/vgic.h new file +14 -0
new file mode 100644
index 000000000000..45bbf238147a
@@ -0,0 +1,14 @@
+/* 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>
+
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+		uint64_t gicd_base_gpa, uint64_t gicr_base_gpa, uint32_t slot);
+
+#endif /* SELFTEST_KVM_VGIC_H */
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +8 -0
new file mode 100644
index 000000000000..a0e4b986d335
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM Generic Interrupt Controller (GIC) v3 host support
+ */
+
+#include <linux/kvm.h>
+#include <linux/sizes.h>
+

tools/testing/selftests/kvm/lib/aarch64/vgic.c line 18

Quoted 10 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +10 -0
+#include "kvm_util.h"
+
+#define VGIC_V3_GICD_SZ		(SZ_64K)
+#define VGIC_V3_GICR_SZ		(2 * SZ_64K)
+
+#define REDIST_REGION_ATTR_ADDR(count, base, flags, index) \
+	(((uint64_t)(count) << 52) | \
+	((uint64_t)((base) >> 16) << 16) | \
+	((uint64_t)(flags) << 12) | \
+	index)

This could go in vgic.h allowing us to share it in aarch64/vgic_init.c, where we already have the same definition.

Quoted 22 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +22 -0
+
+static void vgic_v3_map(struct kvm_vm *vm, uint64_t addr, unsigned int size)
+{
+	unsigned int n_pages = DIV_ROUND_UP(size, vm_get_page_size(vm));
+
+	virt_map(vm, addr, addr, n_pages);
+}
+
+/*
+ * vGIC-v3 default host setup
+ *
+ * Input args:
+ *	vm - KVM VM
+ *	nr_vcpus - Number of vCPUs for 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

tools/testing/selftests/kvm/lib/aarch64/vgic.c line 50

Quoted 10 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +10 -0
+ * redistributor regions of the guest.
+ */
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+		uint64_t gicd_base_gpa, uint64_t gicr_base_gpa)
+{
+	uint64_t redist_attr;
+	int gic_fd;
+
+	TEST_ASSERT(nr_vcpus <= KVM_MAX_VCPUS,
+			"Invalid number of CPUs: %u\n", nr_vcpus);

TEST_ASSERT(!list_empty(&vm->vcpus), ...) to ensure we've created vcpus first. To be really paranoid we could even confirm the number of vcpus in the list matches nr_vcpus.

Quoted 19 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +17 -0
+
+	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);
+	vgic_v3_map(vm, gicd_base_gpa, VGIC_V3_GICD_SZ);
+
+	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);
+	vgic_v3_map(vm, gicr_base_gpa, VGIC_V3_GICR_SZ * nr_vcpus);
+
+	kvm_device_access(gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+				KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
+
+	return gic_fd;
+}
Signature · 1 line
2.33.0.153.gba50c8fa24-goog

Otherwise

Thanks,
drew

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#39 lore raw

Re: [PATCH v3 10/12] KVM: arm64: selftests: Add host support for vGIC

Quoted 75 lines · Andrew Jones
Quoted 1 line · Raghavendra Rao Ananta

Implement a simple library to do perform vGIC-v3

s/do//

Quoted 1 line

setup from a host of view.

^ point

Quoted 66 lines

This includes creating
a vGIC device, setting up distributor and redistributor attributes, and mapping the guest physical addresses.

---
 tools/testing/selftests/kvm/Makefile          |  2 +-
 .../selftests/kvm/include/aarch64/vgic.h      | 14 ++++
 .../testing/selftests/kvm/lib/aarch64/vgic.c  | 67 +++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/vgic.h
 create mode 100644 tools/testing/selftests/kvm/lib/aarch64/vgic.c
tools/testing/selftests/kvm/Makefile +1 -1
index 5476a8ddef60..8342f65c1d96 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/vgic.h new file +14 -0
new file mode 100644
index 000000000000..45bbf238147a
@@ -0,0 +1,14 @@
+/* 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>
+
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+             uint64_t gicd_base_gpa, uint64_t gicr_base_gpa, uint32_t slot);
+
+#endif /* SELFTEST_KVM_VGIC_H */
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +13 -0
new file mode 100644
index 000000000000..a0e4b986d335
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM Generic Interrupt Controller (GIC) v3 host support
+ */
+
+#include <linux/kvm.h>
+#include <linux/sizes.h>
+
+#include "kvm_util.h"
+
+#define VGIC_V3_GICD_SZ              (SZ_64K)
+#define VGIC_V3_GICR_SZ              (2 * SZ_64K)
+
Quoted 9 lines

tools/testing/selftests/kvm/lib/aarch64/vgic.c line 18

Quoted 5 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +5 -0
+#define REDIST_REGION_ATTR_ADDR(count, base, flags, index) \
+     (((uint64_t)(count) << 52) | \
+     ((uint64_t)((base) >> 16) << 16) | \
+     ((uint64_t)(flags) << 12) | \
+     index)

This could go in vgic.h allowing us to share it in aarch64/vgic_init.c, where we already have the same definition.

Sure, I will move this.

Quoted 28 lines
Quoted 28 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +28 -0
+
+static void vgic_v3_map(struct kvm_vm *vm, uint64_t addr, unsigned int size)
+{
+     unsigned int n_pages = DIV_ROUND_UP(size, vm_get_page_size(vm));
+
+     virt_map(vm, addr, addr, n_pages);
+}
+
+/*
+ * vGIC-v3 default host setup
+ *
+ * Input args:
+ *   vm - KVM VM
+ *   nr_vcpus - Number of vCPUs for 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.
+ */
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+             uint64_t gicd_base_gpa, uint64_t gicr_base_gpa)
+{
+     uint64_t redist_attr;
Quoted 9 lines

tools/testing/selftests/kvm/lib/aarch64/vgic.c line 50

Quoted 4 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +4 -0
+     int gic_fd;
+
+     TEST_ASSERT(nr_vcpus <= KVM_MAX_VCPUS,
+                     "Invalid number of CPUs: %u\n", nr_vcpus);

TEST_ASSERT(!list_empty(&vm->vcpus), ...) to ensure we've created vcpus first. To be really paranoid we could even confirm the number of vcpus in the list matches nr_vcpus.

Great idea! I'll add these checks too.

Regards
Raghavendra

Quoted 27 lines
Quoted 18 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +17 -1
+
+     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);
+     vgic_v3_map(vm, gicd_base_gpa, VGIC_V3_GICD_SZ);
+
+     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);
+     vgic_v3_map(vm, gicr_base_gpa, VGIC_V3_GICR_SZ * nr_vcpus);
+
+     kvm_device_access(gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+                             KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
+
+     return gic_fd;
+}
--
Quoted 1 line

2.33.0.153.gba50c8fa24-goog

Otherwise

Thanks,
drew

#40 lore raw

Re: [PATCH v3 10/12] KVM: arm64: selftests: Add host support for vGIC

In reply to Raghavendra Rao Ananta
Quoted 36 lines · Raghavendra Rao Ananta

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

---
 tools/testing/selftests/kvm/Makefile          |  2 +-
 .../selftests/kvm/include/aarch64/vgic.h      | 14 ++++
 .../testing/selftests/kvm/lib/aarch64/vgic.c  | 67 +++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/vgic.h
 create mode 100644 tools/testing/selftests/kvm/lib/aarch64/vgic.c
tools/testing/selftests/kvm/Makefile +1 -1
index 5476a8ddef60..8342f65c1d96 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/vgic.h new file +2 -0
new file mode 100644
index 000000000000..45bbf238147a
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*

tools/testing/selftests/kvm/include/aarch64/vgic.h line 12

Quoted 10 lines
tools/testing/selftests/kvm/include/aarch64/vgic.h new file +10 -0
+ * ARM Generic Interrupt Controller (GIC) host specific defines
+ */
+
+#ifndef SELFTEST_KVM_VGIC_H
+#define SELFTEST_KVM_VGIC_H
+
+#include <linux/kvm.h>
+
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+		uint64_t gicd_base_gpa, uint64_t gicr_base_gpa, uint32_t slot);

Having seen 5 arguments used in the vgic_v3_setup call in the next patch I came back here to refresh my memory on that parameter. 'slot' is the one I forgot, but...

Quoted 42 lines
tools/testing/selftests/kvm/include/aarch64/vgic.h new file +2 -0
+
+#endif /* SELFTEST_KVM_VGIC_H */
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +34 -0
new file mode 100644
index 000000000000..a0e4b986d335
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM Generic Interrupt Controller (GIC) v3 host support
+ */
+
+#include <linux/kvm.h>
+#include <linux/sizes.h>
+
+#include "kvm_util.h"
+
+#define VGIC_V3_GICD_SZ		(SZ_64K)
+#define VGIC_V3_GICR_SZ		(2 * SZ_64K)
+
+#define REDIST_REGION_ATTR_ADDR(count, base, flags, index) \
+	(((uint64_t)(count) << 52) | \
+	((uint64_t)((base) >> 16) << 16) | \
+	((uint64_t)(flags) << 12) | \
+	index)
+
+static void vgic_v3_map(struct kvm_vm *vm, uint64_t addr, unsigned int size)
+{
+	unsigned int n_pages = DIV_ROUND_UP(size, vm_get_page_size(vm));
+
+	virt_map(vm, addr, addr, n_pages);
+}
+
+/*
+ * vGIC-v3 default host setup
+ *
+ * Input args:
+ *	vm - KVM VM
+ *	nr_vcpus - Number of vCPUs for this VM
+ *	gicd_base_gpa - Guest Physical Address of the Distributor region
+ *	gicr_base_gpa - Guest Physical Address of the Redistributor region

tools/testing/selftests/kvm/lib/aarch64/vgic.c line 44

Quoted 10 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +10 -0
+ *
+ * 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.
+ */
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+		uint64_t gicd_base_gpa, uint64_t gicr_base_gpa)

...looks like it was forgotten here too.

Thanks,
drew

Quoted 31 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +23 -0
+{
+	uint64_t redist_attr;
+	int gic_fd;
+
+	TEST_ASSERT(nr_vcpus <= KVM_MAX_VCPUS,
+			"Invalid number of CPUs: %u\n", nr_vcpus);
+
+	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);
+	vgic_v3_map(vm, gicd_base_gpa, VGIC_V3_GICD_SZ);
+
+	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);
+	vgic_v3_map(vm, gicr_base_gpa, VGIC_V3_GICR_SZ * nr_vcpus);
+
+	kvm_device_access(gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
+				KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
+
+	return gic_fd;
+}
Signature · 6 lines
2.33.0.153.gba50c8fa24-goog

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
#41 lore raw

Re: [PATCH v3 10/12] KVM: arm64: selftests: Add host support for vGIC

Quoted 99 lines · Andrew Jones
Quoted 36 lines · Raghavendra Rao Ananta

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

---
 tools/testing/selftests/kvm/Makefile          |  2 +-
 .../selftests/kvm/include/aarch64/vgic.h      | 14 ++++
 .../testing/selftests/kvm/lib/aarch64/vgic.c  | 67 +++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/kvm/include/aarch64/vgic.h
 create mode 100644 tools/testing/selftests/kvm/lib/aarch64/vgic.c
tools/testing/selftests/kvm/Makefile +1 -1
index 5476a8ddef60..8342f65c1d96 100644
@@ -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
tools/testing/selftests/kvm/include/aarch64/vgic.h new file +2 -0
new file mode 100644
index 000000000000..45bbf238147a
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*

tools/testing/selftests/kvm/include/aarch64/vgic.h line 12

Quoted 10 lines
tools/testing/selftests/kvm/include/aarch64/vgic.h new file +10 -0
+ * ARM Generic Interrupt Controller (GIC) host specific defines
+ */
+
+#ifndef SELFTEST_KVM_VGIC_H
+#define SELFTEST_KVM_VGIC_H
+
+#include <linux/kvm.h>
+
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+             uint64_t gicd_base_gpa, uint64_t gicr_base_gpa, uint32_t slot);

Having seen 5 arguments used in the vgic_v3_setup call in the next patch I came back here to refresh my memory on that parameter. 'slot' is the one I forgot, but...

Quoted 46 lines
tools/testing/selftests/kvm/include/aarch64/vgic.h new file +2 -0
+
+#endif /* SELFTEST_KVM_VGIC_H */
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +38 -0
new file mode 100644
index 000000000000..a0e4b986d335
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM Generic Interrupt Controller (GIC) v3 host support
+ */
+
+#include <linux/kvm.h>
+#include <linux/sizes.h>
+
+#include "kvm_util.h"
+
+#define VGIC_V3_GICD_SZ              (SZ_64K)
+#define VGIC_V3_GICR_SZ              (2 * SZ_64K)
+
+#define REDIST_REGION_ATTR_ADDR(count, base, flags, index) \
+     (((uint64_t)(count) << 52) | \
+     ((uint64_t)((base) >> 16) << 16) | \
+     ((uint64_t)(flags) << 12) | \
+     index)
+
+static void vgic_v3_map(struct kvm_vm *vm, uint64_t addr, unsigned int size)
+{
+     unsigned int n_pages = DIV_ROUND_UP(size, vm_get_page_size(vm));
+
+     virt_map(vm, addr, addr, n_pages);
+}
+
+/*
+ * vGIC-v3 default host setup
+ *
+ * Input args:
+ *   vm - KVM VM
+ *   nr_vcpus - Number of vCPUs for 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
Quoted 9 lines

tools/testing/selftests/kvm/lib/aarch64/vgic.c line 44

Quoted 6 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +6 -0
+ *
+ * The function creates a vGIC-v3 device and maps the distributor and
+ * redistributor regions of the guest.
+ */
+int vgic_v3_setup(struct kvm_vm *vm, unsigned int nr_vcpus,
+             uint64_t gicd_base_gpa, uint64_t gicr_base_gpa)

...looks like it was forgotten here too.

You are right. I had 'slot' to try other mechanisms, but I guess I forgot to clean it up.
Thanks for catching this.

Regards,
Raghavendra

Quoted 35 lines

Thanks,
drew

Quoted 31 lines
tools/testing/selftests/kvm/lib/aarch64/vgic.c new file +23 -1
+{
+     uint64_t redist_attr;
+     int gic_fd;
+
+     TEST_ASSERT(nr_vcpus <= KVM_MAX_VCPUS,
+                     "Invalid number of CPUs: %u\n", nr_vcpus);
+
+     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);
+     vgic_v3_map(vm, gicd_base_gpa, VGIC_V3_GICD_SZ);
+
+     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);
+     vgic_v3_map(vm, gicr_base_gpa, VGIC_V3_GICR_SZ * nr_vcpus);
+
+     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.153.gba50c8fa24-goog

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#42 lore raw

PATCH v3 11/12 KVM: arm64: selftests: Add arch_timer test

In reply to Raghavendra Rao Ananta

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.

---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/arch_timer.c        | 351 ++++++++++++++++++
 3 files changed, 353 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/aarch64/arch_timer.c
tools/testing/selftests/kvm/.gitignore +1 -0
index 98053d3afbda..c6058df0cd18 100644
@@ -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
tools/testing/selftests/kvm/Makefile +1 -0
index 8342f65c1d96..46d43e706b20 100644
@@ -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
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +351 -0

351 lines not on this page · raw message

Signature · 6 lines
2.33.0.153.gba50c8fa24-goog

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Reviewed-by on v3 11/12
#43 lore raw

Re: [PATCH v3 11/12] KVM: arm64: selftests: Add arch_timer test

Quoted 101 lines · Raghavendra Rao Ananta

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.

---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/arch_timer.c        | 351 ++++++++++++++++++
 3 files changed, 353 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/aarch64/arch_timer.c
tools/testing/selftests/kvm/.gitignore +1 -0
index 98053d3afbda..c6058df0cd18 100644
@@ -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
tools/testing/selftests/kvm/Makefile +1 -0
index 8342f65c1d96..46d43e706b20 100644
@@ -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
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +58 -0
new file mode 100644
index 000000000000..1383f33850e9
@@ -0,0 +1,351 @@
+// 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 VTIMER_IRQ			27
+#define PTIMER_IRQ			30
+
+#define GICD_BASE_GPA			0x8000000ULL
+#define GICR_BASE_GPA			0x80A0000ULL

tools/testing/selftests/kvm/aarch64/arch_timer.c line 68

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +10 -0
+
+enum guest_stage {
+	GUEST_STAGE_VTIMER_CVAL = 1,
+	GUEST_STAGE_VTIMER_TVAL,
+	GUEST_STAGE_PTIMER_CVAL,
+	GUEST_STAGE_PTIMER_TVAL,
+	GUEST_STAGE_MAX,
+};
+
+/* Sahred variables between host and guest */

Shared

Quoted 29 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +29 -0
+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 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;

tools/testing/selftests/kvm/aarch64/arch_timer.c line 107

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +10 -0
+	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;

Since we divide the stages up for vtimer and ptimer, then I'm not sure we need the wrapper fuctions for timer register get/set with the vtimer and ptimer switches too.

Quoted 27 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +27 -0
+	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;

tools/testing/selftests/kvm/aarch64/arch_timer.c line 144

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +10 -0
+	} 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 codition met' check */

condition

Quoted 27 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +27 -0
+	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 = 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 */

tools/testing/selftests/kvm/aarch64/arch_timer.c line 181

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +10 -0
+		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);
+	};

extra ;

Quoted 34 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +34 -0
+}
+
+static void guest_code(void)
+{
+	uint32_t cpu = 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];

tools/testing/selftests/kvm/aarch64/arch_timer.c line 225

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +10 -0
+
+	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_ASSERT(false,

TEST_FAIL(fmt, ...) can be used.

Quoted 128 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +126 -0
+			"%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 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);
+	vgic_v3_setup(vm, nr_vcpus, GICD_BASE_GPA, GICR_BASE_GPA, 1);
+
+	/* 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;
+}
Signature · 1 line
2.33.0.153.gba50c8fa24-goog

Besides the nits,

Thanks,
drew

#44 lore raw

Re: [PATCH v3 11/12] KVM: arm64: selftests: Add arch_timer test

Quoted 151 lines · Andrew Jones
Quoted 101 lines · Raghavendra Rao Ananta

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.

---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/arch_timer.c        | 351 ++++++++++++++++++
 3 files changed, 353 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/aarch64/arch_timer.c
tools/testing/selftests/kvm/.gitignore +1 -0
index 98053d3afbda..c6058df0cd18 100644
@@ -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
tools/testing/selftests/kvm/Makefile +1 -0
index 8342f65c1d96..46d43e706b20 100644
@@ -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
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +58 -0
new file mode 100644
index 000000000000..1383f33850e9
@@ -0,0 +1,351 @@
+// 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 VTIMER_IRQ                   27
+#define PTIMER_IRQ                   30
+
+#define GICD_BASE_GPA                        0x8000000ULL
+#define GICR_BASE_GPA                        0x80A0000ULL

tools/testing/selftests/kvm/aarch64/arch_timer.c line 68

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +10 -0
+
+enum guest_stage {
+     GUEST_STAGE_VTIMER_CVAL = 1,
+     GUEST_STAGE_VTIMER_TVAL,
+     GUEST_STAGE_PTIMER_CVAL,
+     GUEST_STAGE_PTIMER_TVAL,
+     GUEST_STAGE_MAX,
+};
+
+/* Sahred variables between host and guest */

Shared

Quoted 35 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +35 -0
+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 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:
Quoted 9 lines

tools/testing/selftests/kvm/aarch64/arch_timer.c line 107

Quoted 4 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +4 -0
+             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;

Since we divide the stages up for vtimer and ptimer, then I'm not sure we need the wrapper fuctions for timer register get/set with the vtimer and ptimer switches too.

I understand it's a little redundant. But like you saw in the arch_timer framework patch,
the helper functions to program the cval, or tval, or any others to follow, can be made very slim.

Thanks for catching the typos. Will fix them.

Regards,
Raghavendra

Quoted 263 lines
Quoted 27 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +27 -0
+     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;

tools/testing/selftests/kvm/aarch64/arch_timer.c line 144

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +10 -0
+     } 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 codition met' check */

condition

Quoted 27 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +27 -0
+     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 = 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 */

tools/testing/selftests/kvm/aarch64/arch_timer.c line 181

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +10 -0
+             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);
+     };

extra ;

Quoted 34 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +34 -0
+}
+
+static void guest_code(void)
+{
+     uint32_t cpu = 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];

tools/testing/selftests/kvm/aarch64/arch_timer.c line 225

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +10 -0
+
+     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_ASSERT(false,

TEST_FAIL(fmt, ...) can be used.

Quoted 127 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c new file +126 -1
+                     "%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 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);
+     vgic_v3_setup(vm, nr_vcpus, GICD_BASE_GPA, GICR_BASE_GPA, 1);
+
+     /* 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;
+}
--
Quoted 1 line

2.33.0.153.gba50c8fa24-goog

Besides the nits,

Thanks,
drew

#45 lore raw

PATCH v3 12/12 KVM: arm64: selftests: arch_timer: Support vCPU migration

In reply to Raghavendra Rao Ananta

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.

---
 .../selftests/kvm/aarch64/arch_timer.c        | 108 +++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/aarch64/arch_timer.c +107 -1
index 1383f33850e9..de246c7afab2 100644
@@ -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"
@@ -41,12 +45,14 @@ 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 = 0,		/* Turn off migrations by default */
 };
 
 #define msecs_to_usecs(msec)		((msec) * 1000LL)
@@ -81,6 +87,9 @@ struct test_vcpu {
 static struct test_vcpu test_vcpu[KVM_MAX_VCPUS];
 static struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS];
 
+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)
 {
@@ -216,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:
@@ -235,9 +249,73 @@ 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);
+	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,
@@ -245,8 +323,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 struct kvm_vm *test_vm_create(void)
@@ -286,6 +379,7 @@ 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: 0)\n");
 	pr_info("\t-h: print this help screen\n");
 }
 
@@ -293,7 +387,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);
@@ -320,6 +414,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;
@@ -343,6 +444,11 @@ int main(int argc, char *argv[])
 	if (!parse_args(argc, argv))
 		exit(KSFT_SKIP);
 
+	if (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);
Signature · 6 lines
2.33.0.153.gba50c8fa24-goog

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
#46 lore raw

Re: [PATCH v3 12/12] KVM: arm64: selftests: arch_timer: Support vCPU migration

Quoted 41 lines · Raghavendra Rao Ananta

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.

---
 .../selftests/kvm/aarch64/arch_timer.c        | 108 +++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/aarch64/arch_timer.c +4 -0
index 1383f33850e9..de246c7afab2 100644
@@ -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"
@@ -41,12 +45,14 @@ struct test_args {
 	int nr_vcpus;

tools/testing/selftests/kvm/aarch64/arch_timer.c struct test_args { · line 55

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +2 -0
 	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 = 0,		/* Turn off migrations by default */

I'd rather we enable good tests like these by default.

Quoted 50 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +29 -0
 };
 
 #define msecs_to_usecs(msec)		((msec) * 1000LL)
@@ -81,6 +87,9 @@ struct test_vcpu {
 static struct test_vcpu test_vcpu[KVM_MAX_VCPUS];
 static struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS];
 
+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)
 {
@@ -216,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:
@@ -235,9 +249,73 @@ 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();

tools/testing/selftests/kvm/aarch64/arch_timer.c static void *test_vcpu_run(void *arg) · line 282

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +10 -0
+
+	CPU_ZERO(&cpuset);
+	CPU_SET(new_pcpu, &cpuset);
+	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);

It'd be good to collect stats for the two cases so we know how many vcpus we actually migrated with a successful setaffinity and how many just completed too early. If our stats don't look good, then we can adjust our timeouts and frequencies.

Quoted 95 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +57 -1
+
+	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,
@@ -245,8 +323,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 struct kvm_vm *test_vm_create(void)
@@ -286,6 +379,7 @@ 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: 0)\n");
 	pr_info("\t-h: print this help screen\n");
 }
 
@@ -293,7 +387,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);
@@ -320,6 +414,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;

tools/testing/selftests/kvm/aarch64/arch_timer.c int main(int argc, char *argv[]) · line 447

Quoted 4 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +1 -0
@@ -343,6 +444,11 @@ int main(int argc, char *argv[])
 	if (!parse_args(argc, argv))
 		exit(KSFT_SKIP);
 
+	if (get_nprocs() < 2) {

if (test_args.migration_freq_ms && get_nprocs() < 2)

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +4 -0
+		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);
Signature · 1 line
2.33.0.153.gba50c8fa24-goog

Thanks,
drew

#47 lore raw

Re: [PATCH v3 12/12] KVM: arm64: selftests: arch_timer: Support vCPU migration

Quoted 48 lines · Andrew Jones
Quoted 46 lines · Raghavendra Rao Ananta

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.

---
 .../selftests/kvm/aarch64/arch_timer.c        | 108 +++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/aarch64/arch_timer.c +5 -0
index 1383f33850e9..de246c7afab2 100644
@@ -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"
@@ -41,12 +45,14 @@ struct test_args {
      int nr_vcpus;
      int nr_iter;
      int timer_period_ms;
+     int migration_freq_ms;
 };
 
Quoted 8 lines

tools/testing/selftests/kvm/aarch64/arch_timer.c struct test_args { · line 55

Quoted 5 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +1 -0
 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 = 0,         /* Turn off migrations by default */

I'd rather we enable good tests like these by default.

Well, that was my original idea, but I was concerned about the ease for diagnosing
things since it'll become too noisy. And so I let it as a personal preference. But I can
include it back and see how it goes.

Quoted 57 lines
Quoted 57 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +36 -0
 };
 
 #define msecs_to_usecs(msec)         ((msec) * 1000LL)
@@ -81,6 +87,9 @@ struct test_vcpu {
 static struct test_vcpu test_vcpu[KVM_MAX_VCPUS];
 static struct test_vcpu_shared_data vcpu_shared_data[KVM_MAX_VCPUS];
 
+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)
 {
@@ -216,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:
@@ -235,9 +249,73 @@ 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);
+     ret = pthread_setaffinity_np(vcpu->pt_vcpu_run,
+                                     sizeof(cpuset), &cpuset);
+
+     /* Allow the error where the vCPU thread is already finished */
Quoted 9 lines

tools/testing/selftests/kvm/aarch64/arch_timer.c static void *test_vcpu_run(void *arg) · line 282

Quoted 3 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +3 -0
+     TEST_ASSERT(ret == 0 || ret == ESRCH,
+                     "Failed to migrate the vCPU:%u to pCPU: %u; ret: %d\n",
+                     vcpu->vcpuid, new_pcpu, ret);

It'd be good to collect stats for the two cases so we know how many vcpus we actually migrated with a successful setaffinity and how many just completed too early. If our stats don't look good, then we can adjust our timeouts and frequencies.

I can do that, but since we don't attempt to migrate if the migration thread learns
that the vCPU is already done, I'm guessing we may not hit ESRCH as much.

Quoted 113 lines
Quoted 95 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +57 -1
+
+     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,
@@ -245,8 +323,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 struct kvm_vm *test_vm_create(void)
@@ -286,6 +379,7 @@ 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: 0)\n");
      pr_info("\t-h: print this help screen\n");
 }
 
@@ -293,7 +387,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);
@@ -320,6 +414,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;

tools/testing/selftests/kvm/aarch64/arch_timer.c int main(int argc, char *argv[]) · line 447

Quoted 4 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +1 -0
@@ -343,6 +444,11 @@ int main(int argc, char *argv[])
      if (!parse_args(argc, argv))
              exit(KSFT_SKIP);
 
+     if (get_nprocs() < 2) {

if (test_args.migration_freq_ms && get_nprocs() < 2)

Quoted 10 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +4 -1
+             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.153.gba50c8fa24-goog

Quoted 3 lines

Thanks,
drew

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

#48 lore raw

Re: [PATCH v3 12/12] KVM: arm64: selftests: arch_timer: Support vCPU migration

Quoted 57 lines · Raghavendra Rao Ananta
Quoted 48 lines · Andrew Jones
Quoted 46 lines · Raghavendra Rao Ananta

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.

---
 .../selftests/kvm/aarch64/arch_timer.c        | 108 +++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/aarch64/arch_timer.c +5 -0
index 1383f33850e9..de246c7afab2 100644
@@ -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"
@@ -41,12 +45,14 @@ struct test_args {
      int nr_vcpus;
      int nr_iter;
      int timer_period_ms;
+     int migration_freq_ms;
 };
 
Quoted 8 lines

tools/testing/selftests/kvm/aarch64/arch_timer.c struct test_args { · line 55

Quoted 5 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +1 -0
 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 = 0,         /* Turn off migrations by default */

I'd rather we enable good tests like these by default.

Quoted 5 lines

Well, that was my original idea, but I was concerned about the ease for diagnosing
things since it'll become too noisy. And so I let it as a personal preference. But I can
include it back and see how it goes.

Break the tests into two? One run without migration and one with. If neither work, then we can diagnose the one without first, possibly avoiding the need to diagnose the one with.

Thanks,
drew

#49 lore raw

Re: [PATCH v3 12/12] KVM: arm64: selftests: arch_timer: Support vCPU migration

Quoted 65 lines · Andrew Jones
Quoted 57 lines · Raghavendra Rao Ananta
Quoted 48 lines · Andrew Jones
Quoted 46 lines · Raghavendra Rao Ananta

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.

---
 .../selftests/kvm/aarch64/arch_timer.c        | 108 +++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/aarch64/arch_timer.c +5 -0
index 1383f33850e9..de246c7afab2 100644
@@ -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"
@@ -41,12 +45,14 @@ struct test_args {
      int nr_vcpus;
      int nr_iter;
      int timer_period_ms;
+     int migration_freq_ms;
 };
 
Quoted 8 lines

tools/testing/selftests/kvm/aarch64/arch_timer.c struct test_args { · line 55

Quoted 5 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +1 -0
 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 = 0,         /* Turn off migrations by default */

I'd rather we enable good tests like these by default.

Quoted 5 lines

Well, that was my original idea, but I was concerned about the ease for diagnosing
things since it'll become too noisy. And so I let it as a personal preference. But I can
include it back and see how it goes.

Quoted 4 lines

Break the tests into two? One run without migration and one with. If neither work, then we can diagnose the one without first, possibly avoiding the need to diagnose the one with.

Right. I guess that's where the test's migration switch can come in handy. Let's turn migration ON by default. If we see a failure, we can turn OFF migration
and run the tests. I'll try to include some verbose printing for diagnosability.

Regards,
Raghavendra

Quoted 3 lines

Thanks,
drew

#50 lore raw

Re: [PATCH v3 12/12] KVM: arm64: selftests: arch_timer: Support vCPU migration

Quoted 66 lines · Raghavendra Rao Ananta
Quoted 65 lines · Andrew Jones
Quoted 62 lines · Raghavendra Rao Ananta
Quoted 48 lines · Andrew Jones
Quoted 46 lines · Raghavendra Rao Ananta

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.

---
 .../selftests/kvm/aarch64/arch_timer.c        | 108 +++++++++++++++++-
 1 file changed, 107 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/aarch64/arch_timer.c +5 -0
index 1383f33850e9..de246c7afab2 100644
@@ -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"
@@ -41,12 +45,14 @@ struct test_args {
      int nr_vcpus;
      int nr_iter;
      int timer_period_ms;
+     int migration_freq_ms;
 };
 
Quoted 8 lines

tools/testing/selftests/kvm/aarch64/arch_timer.c struct test_args { · line 55

Quoted 5 lines
tools/testing/selftests/kvm/aarch64/arch_timer.c +1 -0
 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 = 0,         /* Turn off migrations by default */

I'd rather we enable good tests like these by default.

Well, that was my original idea, but I was concerned about the ease for diagnosing
things since it'll become too noisy. And so I let it as a personal preference. But I can
include it back and see how it goes.

Quoted 9 lines
Quoted 4 lines

Break the tests into two? One run without migration and one with. If neither work, then we can diagnose the one without first, possibly avoiding the need to diagnose the one with.

Right. I guess that's where the test's migration switch can come in handy. Let's turn migration ON by default. If we see a failure, we can turn OFF migration
and run the tests. I'll try to include some verbose printing for diagnosability.

Sounds good. You can also consider using pr_debug if you feel the need to balance verbosity with diagnostics.

Thanks,
drew

#51 lore raw
In reply to Raghavendra Rao Ananta
Quoted 29 lines · Raghavendra Rao Ananta

Hello,

The patch series adds a KVM selftest to validate the behavior of ARM's generic timer (patch-11). 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-12 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-10 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-9 adds a guest library to talk to the vGIC, which includes initializing the controller, enabling/disabling the interrupts, and so on.

Furthermore, additional processor utilities such as accessing the MMIO (via readl/writel), read/write to assembler unsupported registers, basic delay generation, enable/disable local IRQs, and so on, are also introduced that the test/GICv3 takes advantage of (patches 1 through 8).

Quoted 5 lines

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

Regards,
Raghavendra

For later submissions, can you include a lore.kernel.org link to your older revisions of the series? NBD now, its easy to find in my inbox but just for future reference.

--
Best,
Oliver

#52 lore raw
In reply to Raghavendra Rao Ananta

+cc Andrew Jones

Quoted 97 lines · Raghavendra Rao Ananta

Hello,

The patch series adds a KVM selftest to validate the behavior of ARM's generic timer (patch-11). 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-12 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-10 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-9 adds a guest library to talk to the vGIC, which includes initializing the controller, enabling/disabling the interrupts, and so on.

Furthermore, additional processor utilities such as accessing the MMIO (via readl/writel), read/write to assembler unsupported registers, basic delay generation, enable/disable local IRQs, and so on, are also introduced that the test/GICv3 takes advantage of (patches 1 through 8).

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

Regards,
Raghavendra

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'.

Raghavendra Rao Ananta (12):
KVM: arm64: selftests: Add MMIO readl/writel support
KVM: arm64: selftests: Add write_sysreg_s and read_sysreg_s
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: Add support to get the vcpuid from MPIDR_EL1
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/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   3 +-
 .../selftests/kvm/aarch64/arch_timer.c        | 457 ++++++++++++++++++
 .../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 | 140 +++++-
 .../selftests/kvm/include/aarch64/spinlock.h  |  13 +
 .../selftests/kvm/include/aarch64/vgic.h      |  14 +
 tools/testing/selftests/kvm/lib/aarch64/gic.c |  93 ++++
 .../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/spinlock.c      |  27 ++
 .../testing/selftests/kvm/lib/aarch64/vgic.c  |  67 +++
 15 files changed, 1332 insertions(+), 2 deletions(-)
 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
Signature · 1 line
2.33.0.153.gba50c8fa24-goog