Skip to contentSkip to conversation

KVM GICv3 emulation

PATCH Superseded Andre Przywara 5 participants 46 messages Dec 8, 2014 – Jan 6, 2015

Subsystems (7)
Documentation Arm (32-bit) KVM arm64 +3 more subsystems

v5 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v5 cover KVM GICv3 emulation

This is version 5 of the GICv3 guest emulation series (not for 3.19).

As the changes this time were much smaller, I updated to tree to 3.18.0, as it includes some bug fixes in the VGIC.

I addressed the remaining comments from Christoffer and Eric, thanks for the review! The changes this times were much smaller, most of them cosmetic or rewordings of commit messages and comments.
I updated the kvm-gicv3/v4 branch in my repo[1] to carry all the delta patches. Those patches are just for reference to see what has changed between v4 and v5. For review and all other purposes please use the v5 branch.

For a changelog summary see below, also each patch carries a changelog now.
Only patches 05, 08, 12, 15, 17, 18 and 19 have been changed compared to v4. I dropped Christoffer's Reviewed-by: tag from 05/19 because of the newly added function, but added the respective tags to the other commit messages.

I quickly tested this version with a GICv3 capable fast model in all endianness modes (LE guest on LE host, BE on LE, LE on BE, BE on BE). Both a GICv2 and a GICv3 guest were booted in all four combinations.

A git repo hosting all these patches lives in the kvm-gicv3/v5 branch of:
http://www.linux-arm.org/git?p=linux-ap.git
git://linux-arm.org/linux-ap.git
-----

GICv3 is the ARM generic interrupt controller designed to overcome some limits of the prevalent GICv2. Most notably it lifts the 8-CPU limit. Though with Linux-3.17 Marc introduced support for hosts to use a GICv3, the CPU limitation still applies to KVM guests, since the current code emulates a GICv2 only.
Also, GICv2 backward compatibility being optional in GICv3, a number of systems won't be able to run GICv2 guests.

This patch series provides code to emulate a GICv3 distributor and redistributor for any KVM guest. It requires a GICv3 in the host to work. With those patches one can run guests efficiently on any GICv3 host. It has the following features:
- Affinity routing (support for up to 255 VCPUs, more possible)
- System registers (as opposed to MMIO access)
- No ITS
- No priority support (as the GICv2 emulation)
- No save / restore support so far (will be added soon)
- Only Group1 interrupts support

The first patches actually refactor the current VGIC code to make room for a different VGIC model to be dropped in with Patch 16. The remaining patches connect the new model to the kernel backend and the userland facing code.

The series goes on top of v3.18.0.
The necessary patches for kvmtool to enable the guest's GICv3 have been posted here before [2], an updated version will follow soon.

There was some testing on the fast model with some I/O and interrupt affinity shuffling in a Linux guest with a varying number of VCPUs as well as some testing on a Juno board (GICv2 only, to spot regressions).

Please review and test.
I would be grateful for people to test for GICv2 regressions also (so on a GICv2 host with current kvmtool/qemu), as there is quite some refactoring on that front.

Much of the code was inspired by MarcZ, also kudos to him for doing the rather painful rebase on top of v3.17-rc1.

Cheers,
Andre.

[1] http://www.linux-arm.org/git?p=linux-ap.git
[2] https://lists.cs.columbia.edu/pipermail/kvmarm/2014-June/010086.html

Changes v4 ... v5:
* fix size of GICD_IROUTER region
* fix compilation on ARM with VGIC not configured
* properly fix the remaining FIXME at the GICv3 memory allocation
* minor style changes, comments and commit messages fixes

Changes v3 ... v4:
* bug-fix in handling GICv3 redistributor CFG register
* move set/get_lr from gic_vm_ops back to vgic_ops (get rid of v3 06/19)
* getting rid of init_emul() at all
* rework guest GIC model initialization
* use non-atomic bit-set and bit-clear functions
* split up handle_mmio_misc* into multiple functions
* refine handling of some reserved registers
* use symbolic names for ICC_SGI1R_EL1 register fields (new patch 16/19)
* move private parameter from MMIO accessors to struct kvm_mmio_exit
* added documentation of new GICv3 guest device
* added lots of comments
* some renaming of identifiers
* minor changes in style and code flow of various functions

Changes v2 ... v3:
* rebase to v3.18-rc2
* adapt to new kvm_register_device() function
* split up vm_ops patch and the GICv2 split-off patch to ease review
* various smaller changes due to Christoffer's review
* fix compilation for arm
* remove support for trapping SGI sysreg accesses on arm hosts

Changes v1 ... v2:
* rebase to v3.17-rc1, caused quite some changes to the init code
* new 9/15 patch to make 10/15 smaller
* fix wrongly ordered cp15 register trap entry (MarcZ)
* fix SGI broadcast (thanks to wanghaibin for spotting)
* fix broken bailout path in kvm_vgic_create (wanghaibin)
* check return value of init_emulation_ops() (wanghaibin)
* fix return value check in vgic_[sg]et_attr()
* add header inclusion guards
* remove double definition of VCPU_NOT_ALLOCATED
* some code move-around
* whitespace fixes

Andre Przywara (19):
arm/arm64: KVM: rework MPIDR assignment and add accessors arm/arm64: KVM: pass down user space provided GIC type into vGIC code arm/arm64: KVM: refactor vgic_handle_mmio() function arm/arm64: KVM: wrap 64 bit MMIO accesses with two 32 bit ones arm/arm64: KVM: introduce per-VM ops
arm/arm64: KVM: move kvm_register_device_ops() into vGIC probing arm/arm64: KVM: dont rely on a valid GICH base address arm/arm64: KVM: make the maximum number of vCPUs a per-VM value arm/arm64: KVM: make the value of ICC_SRE_EL1 a per-VM variable arm/arm64: KVM: refactor MMIO accessors
arm/arm64: KVM: refactor/wrap vgic_set/get_attr() arm/arm64: KVM: add vgic.h header file
arm/arm64: KVM: split GICv2 specific emulation code from vgic.c arm/arm64: KVM: add opaque private pointer to MMIO data arm/arm64: KVM: add virtual GICv3 distributor emulation
arm64: GICv3: introduce symbolic names for GICv3 ICC_SGI1R_EL1 fields
arm64: KVM: add SGI generation register emulation arm/arm64: KVM: enable kernel side of GICv3 emulation arm/arm64: KVM: allow userland to request a virtual GICv3

 Documentation/virtual/kvm/devices/arm-vgic.txt |   22 +-
 arch/arm/include/asm/kvm_emulate.h             |    5 +-
 arch/arm/include/asm/kvm_host.h                |    3 +
 arch/arm/include/asm/kvm_mmio.h                |    1 +
 arch/arm/kvm/Makefile                          |    1 +
 arch/arm/kvm/arm.c                             |   23 +-
 arch/arm/kvm/psci.c                            |   17 +-
 arch/arm64/include/asm/kvm_emulate.h           |    5 +-
 arch/arm64/include/asm/kvm_host.h              |    5 +
 arch/arm64/include/asm/kvm_mmio.h              |    1 +
 arch/arm64/include/uapi/asm/kvm.h              |    7 +
 arch/arm64/kernel/asm-offsets.c                |    1 +
 arch/arm64/kvm/Makefile                        |    2 +
 arch/arm64/kvm/sys_regs.c                      |   39 +-
 arch/arm64/kvm/vgic-v3-switch.S                |   14 +-
 drivers/irqchip/irq-gic-v3.c                   |   14 +-
 include/kvm/arm_vgic.h                         |   41 +-
 include/linux/irqchip/arm-gic-v3.h             |   44 +
 include/linux/kvm_host.h                       |    2 +
 include/uapi/linux/kvm.h                       |    2 +
 virt/kvm/arm/vgic-v2-emul.c                    |  811 ++++++++++++++++++
 virt/kvm/arm/vgic-v2.c                         |    3 +
 virt/kvm/arm/vgic-v3-emul.c                    | 1013 ++++++++++++++++++++++
 virt/kvm/arm/vgic-v3.c                         |   90 +-
 virt/kvm/arm/vgic.c                            | 1070 ++++++------------------
 virt/kvm/arm/vgic.h                            |  122 +++
 26 files changed, 2485 insertions(+), 873 deletions(-)
 create mode 100644 virt/kvm/arm/vgic-v2-emul.c
 create mode 100644 virt/kvm/arm/vgic-v3-emul.c
 create mode 100644 virt/kvm/arm/vgic.h
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH v5 1/19 arm/arm64: KVM: rework MPIDR assignment and add accessors

The virtual MPIDR registers (containing topology information) for the guest are currently mapped linearily to the vcpu_id. Improve this mapping for arm64 by using three levels to not artificially limit the number of vCPUs.
To help this, change and rename the kvm_vcpu_get_mpidr() function to mask off the non-affinity bits in the MPIDR register.
Also add an accessor to later allow easier access to a vCPU with a given MPIDR. Use this new accessor in the PSCI emulation.

---
Changelog v4...v5:
(add Reviewed-by:)

Changelog v3...v4:
- rename kvm_vcpu_get_mpidr() to kvm_vcpu_get_mpidr_aff()
- simplify kvm_mpidr_to_vcpu()
- fixup comment

 arch/arm/include/asm/kvm_emulate.h   |    5 +++--
 arch/arm/include/asm/kvm_host.h      |    2 ++
 arch/arm/kvm/arm.c                   |   13 +++++++++++++
 arch/arm/kvm/psci.c                  |   17 +++++------------
 arch/arm64/include/asm/kvm_emulate.h |    5 +++--
 arch/arm64/include/asm/kvm_host.h    |    2 ++
 arch/arm64/kvm/sys_regs.c            |   13 +++++++++++--
 7 files changed, 39 insertions(+), 18 deletions(-)
arch/arm/include/asm/kvm_emulate.h +3 -2
index b9db269..3ae88ac 100644
@@ -23,6 +23,7 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_mmio.h>
 #include <asm/kvm_arm.h>
+#include <asm/cputype.h>
 
 unsigned long *vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num);
 unsigned long *vcpu_spsr(struct kvm_vcpu *vcpu);
@@ -162,9 +163,9 @@ static inline u32 kvm_vcpu_hvc_get_imm(struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & HSR_HVC_IMM_MASK;
 }
 
-static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
+static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
 {
-	return vcpu->arch.cp15[c0_MPIDR];
+	return vcpu->arch.cp15[c0_MPIDR] & MPIDR_HWID_BITMASK;
 }
 
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
arch/arm/include/asm/kvm_host.h +2 -0
index 53036e2..b443dfe 100644
@@ -236,6 +236,8 @@ static inline void vgic_arch_setup(const struct vgic_params *vgic)
 int kvm_perf_init(void);
 int kvm_perf_teardown(void);
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
+
 static inline void kvm_arch_hardware_disable(void) {}
 static inline void kvm_arch_hardware_unsetup(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
arch/arm/kvm/arm.c +13 -0
index 9e193c8..c2a5c69 100644
@@ -977,6 +977,19 @@ static void check_kvm_target_cpu(void *ret)
 	*(int *)ret = kvm_target_cpu();
 }
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr)
+{
+	struct kvm_vcpu *vcpu;
+	int i;
+
+	mpidr &= MPIDR_HWID_BITMASK;
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		if (mpidr == kvm_vcpu_get_mpidr_aff(vcpu))
+			return vcpu;
+	}
+	return NULL;
+}
+
 /**
  * Initialize Hyp-mode and memory mappings on all CPUs.
  */
arch/arm/kvm/psci.c +5 -12
index 09cf377..84121b2 100644
@@ -21,6 +21,7 @@
 #include <asm/cputype.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_psci.h>
+#include <asm/kvm_host.h>
 
 /*
  * This is an implementation of the Power State Coordination Interface
@@ -65,25 +66,17 @@ static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
 static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 {
 	struct kvm *kvm = source_vcpu->kvm;
-	struct kvm_vcpu *vcpu = NULL, *tmp;
+	struct kvm_vcpu *vcpu = NULL;
 	wait_queue_head_t *wq;
 	unsigned long cpu_id;
 	unsigned long context_id;
-	unsigned long mpidr;
 	phys_addr_t target_pc;
-	int i;
 
-	cpu_id = *vcpu_reg(source_vcpu, 1);
+	cpu_id = *vcpu_reg(source_vcpu, 1) & MPIDR_HWID_BITMASK;
 	if (vcpu_mode_is_32bit(source_vcpu))
 		cpu_id &= ~((u32) 0);
 
-	kvm_for_each_vcpu(i, tmp, kvm) {
-		mpidr = kvm_vcpu_get_mpidr(tmp);
-		if ((mpidr & MPIDR_HWID_BITMASK) == (cpu_id & MPIDR_HWID_BITMASK)) {
-			vcpu = tmp;
-			break;
-		}
-	}
+	vcpu = kvm_mpidr_to_vcpu(kvm, cpu_id);
 
 	/*
 	 * Make sure the caller requested a valid CPU and that the CPU is
@@ -154,7 +147,7 @@ static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
 	 * then ON else OFF
 	 */
 	kvm_for_each_vcpu(i, tmp, kvm) {
-		mpidr = kvm_vcpu_get_mpidr(tmp);
+		mpidr = kvm_vcpu_get_mpidr_aff(tmp);
 		if (((mpidr & target_affinity_mask) == target_affinity) &&
 		    !tmp->arch.pause) {
 			return PSCI_0_2_AFFINITY_LEVEL_ON;
arch/arm64/include/asm/kvm_emulate.h +3 -2
index 5674a55..d4daaa5 100644
@@ -27,6 +27,7 @@
 #include <asm/kvm_arm.h>
 #include <asm/kvm_mmio.h>
 #include <asm/ptrace.h>
+#include <asm/cputype.h>
 
 unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
 unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu);
@@ -182,9 +183,9 @@ static inline u8 kvm_vcpu_trap_get_fault_type(const struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & ESR_EL2_FSC_TYPE;
 }
 
-static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
+static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
 {
-	return vcpu_sys_reg(vcpu, MPIDR_EL1);
+	return vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;
 }
 
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
arch/arm64/include/asm/kvm_host.h +2 -0
index 2012c4b..286bb61 100644
@@ -207,6 +207,8 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
 int kvm_perf_init(void);
 int kvm_perf_teardown(void);
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
+
 static inline void __cpu_init_hyp_mode(phys_addr_t boot_pgd_ptr,
 				       phys_addr_t pgd_ptr,
 				       unsigned long hyp_stack_ptr,
arch/arm64/kvm/sys_regs.c +11 -2
index 3d7c2df..136e679 100644
@@ -252,10 +252,19 @@ static void reset_amair_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 
 static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 {
+	u64 mpidr;
+
 	/*
-	 * Simply map the vcpu_id into the Aff0 field of the MPIDR.
+	 * Map the vcpu_id into the first three affinity level fields of
+	 * the MPIDR. We limit the number of VCPUs in level 0 due to a
+	 * limitation to 16 CPUs in that level in the ICC_SGIxR registers
+	 * of the GICv3 to be able to address each CPU directly when
+	 * sending IPIs.
 	 */
-	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1UL << 31) | (vcpu->vcpu_id & 0xff);
+	mpidr = (vcpu->vcpu_id & 0x0f) << MPIDR_LEVEL_SHIFT(0);
+	mpidr |= ((vcpu->vcpu_id >> 4) & 0xff) << MPIDR_LEVEL_SHIFT(1);
+	mpidr |= ((vcpu->vcpu_id >> 12) & 0xff) << MPIDR_LEVEL_SHIFT(2);
+	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
 }
 
 /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
Signature · 1 line
1.7.9.5
reviewer
#2 lore raw

PATCH v5 1/19 arm/arm64: KVM: rework MPIDR assignment and add accessors

Hi Andre,

Quoted 4 lines · Andre Przywara

The virtual MPIDR registers (containing topology information) for the guest are currently mapped linearily to the vcpu_id. Improve this mapping for arm64 by using three levels to not artificially limit the number of vCPUs.

For AArch64, there are four levels in the MPIDR_EL1, Aff{3,2,1,0}, and all four are covered by MPIDR_HWID_BITMASK.

The code below only seems to handle Aff{2,1,0}. Is the vCPU limited at creation time such that Aff3 is always zero?

Thanks,
Mark.

Quoted 208 lines

To help this, change and rename the kvm_vcpu_get_mpidr() function to mask off the non-affinity bits in the MPIDR register. Also add an accessor to later allow easier access to a vCPU with a given MPIDR. Use this new accessor in the PSCI emulation.

---
Changelog v4...v5:
(add Reviewed-by:)

Changelog v3...v4:
- rename kvm_vcpu_get_mpidr() to kvm_vcpu_get_mpidr_aff()
- simplify kvm_mpidr_to_vcpu()
- fixup comment

 arch/arm/include/asm/kvm_emulate.h   |    5 +++--
 arch/arm/include/asm/kvm_host.h      |    2 ++
 arch/arm/kvm/arm.c                   |   13 +++++++++++++
 arch/arm/kvm/psci.c                  |   17 +++++------------
 arch/arm64/include/asm/kvm_emulate.h |    5 +++--
 arch/arm64/include/asm/kvm_host.h    |    2 ++
 arch/arm64/kvm/sys_regs.c            |   13 +++++++++++--
 7 files changed, 39 insertions(+), 18 deletions(-)
arch/arm/include/asm/kvm_emulate.h +3 -2
index b9db269..3ae88ac 100644
@@ -23,6 +23,7 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_mmio.h>
 #include <asm/kvm_arm.h>
+#include <asm/cputype.h>
 
 unsigned long *vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num);
 unsigned long *vcpu_spsr(struct kvm_vcpu *vcpu);
@@ -162,9 +163,9 @@ static inline u32 kvm_vcpu_hvc_get_imm(struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & HSR_HVC_IMM_MASK;
 }
 
-static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
+static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
 {
-	return vcpu->arch.cp15[c0_MPIDR];
+	return vcpu->arch.cp15[c0_MPIDR] & MPIDR_HWID_BITMASK;
 }
 
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
arch/arm/include/asm/kvm_host.h +2 -0
index 53036e2..b443dfe 100644
@@ -236,6 +236,8 @@ static inline void vgic_arch_setup(const struct vgic_params *vgic)
 int kvm_perf_init(void);
 int kvm_perf_teardown(void);
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
+
 static inline void kvm_arch_hardware_disable(void) {}
 static inline void kvm_arch_hardware_unsetup(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
arch/arm/kvm/arm.c +13 -0
index 9e193c8..c2a5c69 100644
@@ -977,6 +977,19 @@ static void check_kvm_target_cpu(void *ret)
 	*(int *)ret = kvm_target_cpu();
 }
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr)
+{
+	struct kvm_vcpu *vcpu;
+	int i;
+
+	mpidr &= MPIDR_HWID_BITMASK;
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		if (mpidr == kvm_vcpu_get_mpidr_aff(vcpu))
+			return vcpu;
+	}
+	return NULL;
+}
+
 /**
  * Initialize Hyp-mode and memory mappings on all CPUs.
  */
arch/arm/kvm/psci.c +5 -12
index 09cf377..84121b2 100644
@@ -21,6 +21,7 @@
 #include <asm/cputype.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_psci.h>
+#include <asm/kvm_host.h>
 
 /*
  * This is an implementation of the Power State Coordination Interface
@@ -65,25 +66,17 @@ static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
 static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 {
 	struct kvm *kvm = source_vcpu->kvm;
-	struct kvm_vcpu *vcpu = NULL, *tmp;
+	struct kvm_vcpu *vcpu = NULL;
 	wait_queue_head_t *wq;
 	unsigned long cpu_id;
 	unsigned long context_id;
-	unsigned long mpidr;
 	phys_addr_t target_pc;
-	int i;
 
-	cpu_id = *vcpu_reg(source_vcpu, 1);
+	cpu_id = *vcpu_reg(source_vcpu, 1) & MPIDR_HWID_BITMASK;
 	if (vcpu_mode_is_32bit(source_vcpu))
 		cpu_id &= ~((u32) 0);
 
-	kvm_for_each_vcpu(i, tmp, kvm) {
-		mpidr = kvm_vcpu_get_mpidr(tmp);
-		if ((mpidr & MPIDR_HWID_BITMASK) == (cpu_id & MPIDR_HWID_BITMASK)) {
-			vcpu = tmp;
-			break;
-		}
-	}
+	vcpu = kvm_mpidr_to_vcpu(kvm, cpu_id);
 
 	/*
 	 * Make sure the caller requested a valid CPU and that the CPU is
@@ -154,7 +147,7 @@ static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
 	 * then ON else OFF
 	 */
 	kvm_for_each_vcpu(i, tmp, kvm) {
-		mpidr = kvm_vcpu_get_mpidr(tmp);
+		mpidr = kvm_vcpu_get_mpidr_aff(tmp);
 		if (((mpidr & target_affinity_mask) == target_affinity) &&
 		    !tmp->arch.pause) {
 			return PSCI_0_2_AFFINITY_LEVEL_ON;
arch/arm64/include/asm/kvm_emulate.h +3 -2
index 5674a55..d4daaa5 100644
@@ -27,6 +27,7 @@
 #include <asm/kvm_arm.h>
 #include <asm/kvm_mmio.h>
 #include <asm/ptrace.h>
+#include <asm/cputype.h>
 
 unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
 unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu);
@@ -182,9 +183,9 @@ static inline u8 kvm_vcpu_trap_get_fault_type(const struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & ESR_EL2_FSC_TYPE;
 }
 
-static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
+static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
 {
-	return vcpu_sys_reg(vcpu, MPIDR_EL1);
+	return vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;
 }
 
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
arch/arm64/include/asm/kvm_host.h +2 -0
index 2012c4b..286bb61 100644
@@ -207,6 +207,8 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
 int kvm_perf_init(void);
 int kvm_perf_teardown(void);
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
+
 static inline void __cpu_init_hyp_mode(phys_addr_t boot_pgd_ptr,
 				       phys_addr_t pgd_ptr,
 				       unsigned long hyp_stack_ptr,
arch/arm64/kvm/sys_regs.c +11 -2
index 3d7c2df..136e679 100644
@@ -252,10 +252,19 @@ static void reset_amair_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 
 static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 {
+	u64 mpidr;
+
 	/*
-	 * Simply map the vcpu_id into the Aff0 field of the MPIDR.
+	 * Map the vcpu_id into the first three affinity level fields of
+	 * the MPIDR. We limit the number of VCPUs in level 0 due to a
+	 * limitation to 16 CPUs in that level in the ICC_SGIxR registers
+	 * of the GICv3 to be able to address each CPU directly when
+	 * sending IPIs.
 	 */
-	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1UL << 31) | (vcpu->vcpu_id & 0xff);
+	mpidr = (vcpu->vcpu_id & 0x0f) << MPIDR_LEVEL_SHIFT(0);
+	mpidr |= ((vcpu->vcpu_id >> 4) & 0xff) << MPIDR_LEVEL_SHIFT(1);
+	mpidr |= ((vcpu->vcpu_id >> 12) & 0xff) << MPIDR_LEVEL_SHIFT(2);
+	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
 }
 
 /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
Signature · 7 lines
1.7.9.5


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#3 lore raw

PATCH v5 1/19 arm/arm64: KVM: rework MPIDR assignment and add accessors

Hi Mark,

Quoted 8 lines · Mark Rutland

Hi Andre,

Quoted 4 lines · Andre Przywara

The virtual MPIDR registers (containing topology information) for the guest are currently mapped linearily to the vcpu_id. Improve this mapping for arm64 by using three levels to not artificially limit the number of vCPUs.

Quoted 5 lines

For AArch64, there are four levels in the MPIDR_EL1, Aff{3,2,1,0}, and all four are covered by MPIDR_HWID_BITMASK.

The code below only seems to handle Aff{2,1,0}. Is the vCPU limited at creation time such that Aff3 is always zero?

Yes. The GICv3 has a bit to optionally drop Aff3 support from it's own CPU addressing scheme, which we use to avoid "real" 64-bit register accesses [1].
So by definition we don't use Aff3 for the VCPUs, which limits our number of VCPUs to 2**20 (8+8+4), which isn't really a limit right now. Other parts of the code only allow 255 VCPUs at the moment.

Cheers,
Andre.

[1]
http://www.linux-arm.org/git?p=linux-ap.git;a=blob;f=virt/kvm/arm/vgic-v3-emul.c;h=18af188e8c64da5ed21291f8b7da02cd7aed07c4;hb=8bcfd318e0edabbf9aafa8512af0b56507495661#l261

Quoted 208 lines
Quoted 208 lines

To help this, change and rename the kvm_vcpu_get_mpidr() function to mask off the non-affinity bits in the MPIDR register. Also add an accessor to later allow easier access to a vCPU with a given MPIDR. Use this new accessor in the PSCI emulation.

---
Changelog v4...v5:
(add Reviewed-by:)

Changelog v3...v4:
- rename kvm_vcpu_get_mpidr() to kvm_vcpu_get_mpidr_aff()
- simplify kvm_mpidr_to_vcpu()
- fixup comment

 arch/arm/include/asm/kvm_emulate.h   |    5 +++--
 arch/arm/include/asm/kvm_host.h      |    2 ++
 arch/arm/kvm/arm.c                   |   13 +++++++++++++
 arch/arm/kvm/psci.c                  |   17 +++++------------
 arch/arm64/include/asm/kvm_emulate.h |    5 +++--
 arch/arm64/include/asm/kvm_host.h    |    2 ++
 arch/arm64/kvm/sys_regs.c            |   13 +++++++++++--
 7 files changed, 39 insertions(+), 18 deletions(-)
arch/arm/include/asm/kvm_emulate.h +3 -2
index b9db269..3ae88ac 100644
@@ -23,6 +23,7 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_mmio.h>
 #include <asm/kvm_arm.h>
+#include <asm/cputype.h>
 
 unsigned long *vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num);
 unsigned long *vcpu_spsr(struct kvm_vcpu *vcpu);
@@ -162,9 +163,9 @@ static inline u32 kvm_vcpu_hvc_get_imm(struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & HSR_HVC_IMM_MASK;
 }
 
-static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
+static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
 {
-	return vcpu->arch.cp15[c0_MPIDR];
+	return vcpu->arch.cp15[c0_MPIDR] & MPIDR_HWID_BITMASK;
 }
 
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
arch/arm/include/asm/kvm_host.h +2 -0
index 53036e2..b443dfe 100644
@@ -236,6 +236,8 @@ static inline void vgic_arch_setup(const struct vgic_params *vgic)
 int kvm_perf_init(void);
 int kvm_perf_teardown(void);
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
+
 static inline void kvm_arch_hardware_disable(void) {}
 static inline void kvm_arch_hardware_unsetup(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
arch/arm/kvm/arm.c +13 -0
index 9e193c8..c2a5c69 100644
@@ -977,6 +977,19 @@ static void check_kvm_target_cpu(void *ret)
 	*(int *)ret = kvm_target_cpu();
 }
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr)
+{
+	struct kvm_vcpu *vcpu;
+	int i;
+
+	mpidr &= MPIDR_HWID_BITMASK;
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		if (mpidr == kvm_vcpu_get_mpidr_aff(vcpu))
+			return vcpu;
+	}
+	return NULL;
+}
+
 /**
  * Initialize Hyp-mode and memory mappings on all CPUs.
  */
arch/arm/kvm/psci.c +5 -12
index 09cf377..84121b2 100644
@@ -21,6 +21,7 @@
 #include <asm/cputype.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_psci.h>
+#include <asm/kvm_host.h>
 
 /*
  * This is an implementation of the Power State Coordination Interface
@@ -65,25 +66,17 @@ static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
 static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 {
 	struct kvm *kvm = source_vcpu->kvm;
-	struct kvm_vcpu *vcpu = NULL, *tmp;
+	struct kvm_vcpu *vcpu = NULL;
 	wait_queue_head_t *wq;
 	unsigned long cpu_id;
 	unsigned long context_id;
-	unsigned long mpidr;
 	phys_addr_t target_pc;
-	int i;
 
-	cpu_id = *vcpu_reg(source_vcpu, 1);
+	cpu_id = *vcpu_reg(source_vcpu, 1) & MPIDR_HWID_BITMASK;
 	if (vcpu_mode_is_32bit(source_vcpu))
 		cpu_id &= ~((u32) 0);
 
-	kvm_for_each_vcpu(i, tmp, kvm) {
-		mpidr = kvm_vcpu_get_mpidr(tmp);
-		if ((mpidr & MPIDR_HWID_BITMASK) == (cpu_id & MPIDR_HWID_BITMASK)) {
-			vcpu = tmp;
-			break;
-		}
-	}
+	vcpu = kvm_mpidr_to_vcpu(kvm, cpu_id);
 
 	/*
 	 * Make sure the caller requested a valid CPU and that the CPU is
@@ -154,7 +147,7 @@ static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
 	 * then ON else OFF
 	 */
 	kvm_for_each_vcpu(i, tmp, kvm) {
-		mpidr = kvm_vcpu_get_mpidr(tmp);
+		mpidr = kvm_vcpu_get_mpidr_aff(tmp);
 		if (((mpidr & target_affinity_mask) == target_affinity) &&
 		    !tmp->arch.pause) {
 			return PSCI_0_2_AFFINITY_LEVEL_ON;
arch/arm64/include/asm/kvm_emulate.h +3 -2
index 5674a55..d4daaa5 100644
@@ -27,6 +27,7 @@
 #include <asm/kvm_arm.h>
 #include <asm/kvm_mmio.h>
 #include <asm/ptrace.h>
+#include <asm/cputype.h>
 
 unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
 unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu);
@@ -182,9 +183,9 @@ static inline u8 kvm_vcpu_trap_get_fault_type(const struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & ESR_EL2_FSC_TYPE;
 }
 
-static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
+static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
 {
-	return vcpu_sys_reg(vcpu, MPIDR_EL1);
+	return vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;
 }
 
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
arch/arm64/include/asm/kvm_host.h +2 -0
index 2012c4b..286bb61 100644
@@ -207,6 +207,8 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
 int kvm_perf_init(void);
 int kvm_perf_teardown(void);
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
+
 static inline void __cpu_init_hyp_mode(phys_addr_t boot_pgd_ptr,
 				       phys_addr_t pgd_ptr,
 				       unsigned long hyp_stack_ptr,
arch/arm64/kvm/sys_regs.c +11 -2
index 3d7c2df..136e679 100644
@@ -252,10 +252,19 @@ static void reset_amair_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 
 static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 {
+	u64 mpidr;
+
 	/*
-	 * Simply map the vcpu_id into the Aff0 field of the MPIDR.
+	 * Map the vcpu_id into the first three affinity level fields of
+	 * the MPIDR. We limit the number of VCPUs in level 0 due to a
+	 * limitation to 16 CPUs in that level in the ICC_SGIxR registers
+	 * of the GICv3 to be able to address each CPU directly when
+	 * sending IPIs.
 	 */
-	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1UL << 31) | (vcpu->vcpu_id & 0xff);
+	mpidr = (vcpu->vcpu_id & 0x0f) << MPIDR_LEVEL_SHIFT(0);
+	mpidr |= ((vcpu->vcpu_id >> 4) & 0xff) << MPIDR_LEVEL_SHIFT(1);
+	mpidr |= ((vcpu->vcpu_id >> 12) & 0xff) << MPIDR_LEVEL_SHIFT(2);
+	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
 }
 
 /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
Signature · 7 lines
1.7.9.5


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#4 lore raw

PATCH v5 1/19 arm/arm64: KVM: rework MPIDR assignment and add accessors

In reply to Andre Przywara
Quoted 11 lines · Andre Przywara

The virtual MPIDR registers (containing topology information) for the guest are currently mapped linearily to the vcpu_id. Improve this mapping for arm64 by using three levels to not artificially limit the number of vCPUs.
To help this, change and rename the kvm_vcpu_get_mpidr() function to mask off the non-affinity bits in the MPIDR register.
Also add an accessor to later allow easier access to a vCPU with a given MPIDR. Use this new accessor in the PSCI emulation.

Quoted 192 lines

---
Changelog v4...v5:
(add Reviewed-by:)

Changelog v3...v4:
- rename kvm_vcpu_get_mpidr() to kvm_vcpu_get_mpidr_aff()
- simplify kvm_mpidr_to_vcpu()
- fixup comment

 arch/arm/include/asm/kvm_emulate.h   |    5 +++--
 arch/arm/include/asm/kvm_host.h      |    2 ++
 arch/arm/kvm/arm.c                   |   13 +++++++++++++
 arch/arm/kvm/psci.c                  |   17 +++++------------
 arch/arm64/include/asm/kvm_emulate.h |    5 +++--
 arch/arm64/include/asm/kvm_host.h    |    2 ++
 arch/arm64/kvm/sys_regs.c            |   13 +++++++++++--
 7 files changed, 39 insertions(+), 18 deletions(-)
arch/arm/include/asm/kvm_emulate.h +3 -2
index b9db269..3ae88ac 100644
@@ -23,6 +23,7 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_mmio.h>
 #include <asm/kvm_arm.h>
+#include <asm/cputype.h>
 
 unsigned long *vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num);
 unsigned long *vcpu_spsr(struct kvm_vcpu *vcpu);
@@ -162,9 +163,9 @@ static inline u32 kvm_vcpu_hvc_get_imm(struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & HSR_HVC_IMM_MASK;
 }
 
-static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
+static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
 {
-	return vcpu->arch.cp15[c0_MPIDR];
+	return vcpu->arch.cp15[c0_MPIDR] & MPIDR_HWID_BITMASK;
 }
 
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
arch/arm/include/asm/kvm_host.h +2 -0
index 53036e2..b443dfe 100644
@@ -236,6 +236,8 @@ static inline void vgic_arch_setup(const struct vgic_params *vgic)
 int kvm_perf_init(void);
 int kvm_perf_teardown(void);
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
+
 static inline void kvm_arch_hardware_disable(void) {}
 static inline void kvm_arch_hardware_unsetup(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
arch/arm/kvm/arm.c +13 -0
index 9e193c8..c2a5c69 100644
@@ -977,6 +977,19 @@ static void check_kvm_target_cpu(void *ret)
 	*(int *)ret = kvm_target_cpu();
 }
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr)
+{
+	struct kvm_vcpu *vcpu;
+	int i;
+
+	mpidr &= MPIDR_HWID_BITMASK;
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		if (mpidr == kvm_vcpu_get_mpidr_aff(vcpu))
+			return vcpu;
+	}
+	return NULL;
+}
+
 /**
  * Initialize Hyp-mode and memory mappings on all CPUs.
  */
arch/arm/kvm/psci.c +5 -12
index 09cf377..84121b2 100644
@@ -21,6 +21,7 @@
 #include <asm/cputype.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_psci.h>
+#include <asm/kvm_host.h>
 
 /*
  * This is an implementation of the Power State Coordination Interface
@@ -65,25 +66,17 @@ static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
 static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
 {
 	struct kvm *kvm = source_vcpu->kvm;
-	struct kvm_vcpu *vcpu = NULL, *tmp;
+	struct kvm_vcpu *vcpu = NULL;
 	wait_queue_head_t *wq;
 	unsigned long cpu_id;
 	unsigned long context_id;
-	unsigned long mpidr;
 	phys_addr_t target_pc;
-	int i;
 
-	cpu_id = *vcpu_reg(source_vcpu, 1);
+	cpu_id = *vcpu_reg(source_vcpu, 1) & MPIDR_HWID_BITMASK;
 	if (vcpu_mode_is_32bit(source_vcpu))
 		cpu_id &= ~((u32) 0);
 
-	kvm_for_each_vcpu(i, tmp, kvm) {
-		mpidr = kvm_vcpu_get_mpidr(tmp);
-		if ((mpidr & MPIDR_HWID_BITMASK) == (cpu_id & MPIDR_HWID_BITMASK)) {
-			vcpu = tmp;
-			break;
-		}
-	}
+	vcpu = kvm_mpidr_to_vcpu(kvm, cpu_id);
 
 	/*
 	 * Make sure the caller requested a valid CPU and that the CPU is
@@ -154,7 +147,7 @@ static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
 	 * then ON else OFF
 	 */
 	kvm_for_each_vcpu(i, tmp, kvm) {
-		mpidr = kvm_vcpu_get_mpidr(tmp);
+		mpidr = kvm_vcpu_get_mpidr_aff(tmp);
 		if (((mpidr & target_affinity_mask) == target_affinity) &&
 		    !tmp->arch.pause) {
 			return PSCI_0_2_AFFINITY_LEVEL_ON;
arch/arm64/include/asm/kvm_emulate.h +3 -2
index 5674a55..d4daaa5 100644
@@ -27,6 +27,7 @@
 #include <asm/kvm_arm.h>
 #include <asm/kvm_mmio.h>
 #include <asm/ptrace.h>
+#include <asm/cputype.h>
 
 unsigned long *vcpu_reg32(const struct kvm_vcpu *vcpu, u8 reg_num);
 unsigned long *vcpu_spsr32(const struct kvm_vcpu *vcpu);
@@ -182,9 +183,9 @@ static inline u8 kvm_vcpu_trap_get_fault_type(const struct kvm_vcpu *vcpu)
 	return kvm_vcpu_get_hsr(vcpu) & ESR_EL2_FSC_TYPE;
 }
 
-static inline unsigned long kvm_vcpu_get_mpidr(struct kvm_vcpu *vcpu)
+static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu)
 {
-	return vcpu_sys_reg(vcpu, MPIDR_EL1);
+	return vcpu_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK;
 }
 
 static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu)
arch/arm64/include/asm/kvm_host.h +2 -0
index 2012c4b..286bb61 100644
@@ -207,6 +207,8 @@ int handle_exit(struct kvm_vcpu *vcpu, struct kvm_run *run,
 int kvm_perf_init(void);
 int kvm_perf_teardown(void);
 
+struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
+
 static inline void __cpu_init_hyp_mode(phys_addr_t boot_pgd_ptr,
 				       phys_addr_t pgd_ptr,
 				       unsigned long hyp_stack_ptr,
arch/arm64/kvm/sys_regs.c +11 -2
index 3d7c2df..136e679 100644
@@ -252,10 +252,19 @@ static void reset_amair_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 
 static void reset_mpidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 {
+	u64 mpidr;
+
 	/*
-	 * Simply map the vcpu_id into the Aff0 field of the MPIDR.
+	 * Map the vcpu_id into the first three affinity level fields of
+	 * the MPIDR. We limit the number of VCPUs in level 0 due to a
+	 * limitation to 16 CPUs in that level in the ICC_SGIxR registers
+	 * of the GICv3 to be able to address each CPU directly when
+	 * sending IPIs.
 	 */
-	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1UL << 31) | (vcpu->vcpu_id & 0xff);
+	mpidr = (vcpu->vcpu_id & 0x0f) << MPIDR_LEVEL_SHIFT(0);
+	mpidr |= ((vcpu->vcpu_id >> 4) & 0xff) << MPIDR_LEVEL_SHIFT(1);
+	mpidr |= ((vcpu->vcpu_id >> 12) & 0xff) << MPIDR_LEVEL_SHIFT(2);
+	vcpu_sys_reg(vcpu, MPIDR_EL1) = (1ULL << 31) | mpidr;
 }
 
 /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
Signature · 1 line
Jazz is not dead. It just smells funny.
#5 lore raw

PATCH v5 2/19 arm/arm64: KVM: pass down user space provided GIC type into vGIC code

In reply to Andre Przywara

With the introduction of a second emulated GIC model we need to let userspace specify the GIC model to use for each VM. Pass the userspace provided value down into the vGIC code and store it there to differentiate later.

---
Changelog v4...v5:
(none)

Changelog v3...v4:
- added Acked-by

 arch/arm/kvm/arm.c     |    2 +-
 include/kvm/arm_vgic.h |    7 +++++--
 virt/kvm/arm/vgic.c    |    5 +++--
 3 files changed, 9 insertions(+), 5 deletions(-)
arch/arm/kvm/arm.c +1 -1
index c2a5c69..8817fbd 100644
@@ -753,7 +753,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	switch (ioctl) {
 	case KVM_CREATE_IRQCHIP: {
 		if (vgic_present)
-			return kvm_vgic_create(kvm);
+			return kvm_vgic_create(kvm, KVM_DEV_TYPE_ARM_VGIC_V2);
 		else
 			return -ENXIO;
 	}
include/kvm/arm_vgic.h +5 -2
index 206dcc3..dde5a00 100644
@@ -140,6 +140,9 @@ struct vgic_dist {
 	bool			in_kernel;
 	bool			ready;
 
+	/* vGIC model the kernel emulates for the guest (GICv2 or GICv3) */
+	u32			vgic_model;
+
 	int			nr_cpus;
 	int			nr_irqs;
 
@@ -275,7 +278,7 @@ struct kvm_exit_mmio;
 int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write);
 int kvm_vgic_hyp_init(void);
 int kvm_vgic_init(struct kvm *kvm);
-int kvm_vgic_create(struct kvm *kvm);
+int kvm_vgic_create(struct kvm *kvm, u32 type);
 void kvm_vgic_destroy(struct kvm *kvm);
 void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu);
 void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu);
@@ -326,7 +329,7 @@ static inline int kvm_vgic_init(struct kvm *kvm)
 	return 0;
 }
 
-static inline int kvm_vgic_create(struct kvm *kvm)
+static inline int kvm_vgic_create(struct kvm *kvm, u32 type)
 {
 	return 0;
 }
virt/kvm/arm/vgic.c +3 -2
index aacdb59..c2e0795 100644
@@ -1931,7 +1931,7 @@ out:
 	return ret;
 }
 
-int kvm_vgic_create(struct kvm *kvm)
+int kvm_vgic_create(struct kvm *kvm, u32 type)
 {
 	int i, vcpu_lock_idx = -1, ret;
 	struct kvm_vcpu *vcpu;
@@ -1963,6 +1963,7 @@ int kvm_vgic_create(struct kvm *kvm)
 
 	spin_lock_init(&kvm->arch.vgic.lock);
 	kvm->arch.vgic.in_kernel = true;
+	kvm->arch.vgic.vgic_model = type;
 	kvm->arch.vgic.vctrl_base = vgic->vctrl_base;
 	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
 	kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
@@ -2388,7 +2389,7 @@ static void vgic_destroy(struct kvm_device *dev)
 
 static int vgic_create(struct kvm_device *dev, u32 type)
 {
-	return kvm_vgic_create(dev->kvm);
+	return kvm_vgic_create(dev->kvm, type);
 }
 
 static struct kvm_device_ops kvm_arm_vgic_v2_ops = {
Signature · 1 line
1.7.9.5
#6 lore raw

PATCH v5 2/19 arm/arm64: KVM: pass down user space provided GIC type into vGIC code

Quoted 7 lines · Andre Przywara

With the introduction of a second emulated GIC model we need to let userspace specify the GIC model to use for each VM. Pass the userspace provided value down into the vGIC code and store it there to differentiate later.

Quoted 87 lines

---
Changelog v4...v5:
(none)

Changelog v3...v4:
- added Acked-by

 arch/arm/kvm/arm.c     |    2 +-
 include/kvm/arm_vgic.h |    7 +++++--
 virt/kvm/arm/vgic.c    |    5 +++--
 3 files changed, 9 insertions(+), 5 deletions(-)
arch/arm/kvm/arm.c +1 -1
index c2a5c69..8817fbd 100644
@@ -753,7 +753,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	switch (ioctl) {
 	case KVM_CREATE_IRQCHIP: {
 		if (vgic_present)
-			return kvm_vgic_create(kvm);
+			return kvm_vgic_create(kvm, KVM_DEV_TYPE_ARM_VGIC_V2);
 		else
 			return -ENXIO;
 	}
include/kvm/arm_vgic.h +5 -2
index 206dcc3..dde5a00 100644
@@ -140,6 +140,9 @@ struct vgic_dist {
 	bool			in_kernel;
 	bool			ready;
 
+	/* vGIC model the kernel emulates for the guest (GICv2 or GICv3) */
+	u32			vgic_model;
+
 	int			nr_cpus;
 	int			nr_irqs;
 
@@ -275,7 +278,7 @@ struct kvm_exit_mmio;
 int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write);
 int kvm_vgic_hyp_init(void);
 int kvm_vgic_init(struct kvm *kvm);
-int kvm_vgic_create(struct kvm *kvm);
+int kvm_vgic_create(struct kvm *kvm, u32 type);
 void kvm_vgic_destroy(struct kvm *kvm);
 void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu);
 void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu);
@@ -326,7 +329,7 @@ static inline int kvm_vgic_init(struct kvm *kvm)
 	return 0;
 }
 
-static inline int kvm_vgic_create(struct kvm *kvm)
+static inline int kvm_vgic_create(struct kvm *kvm, u32 type)
 {
 	return 0;
 }
virt/kvm/arm/vgic.c +3 -2
index aacdb59..c2e0795 100644
@@ -1931,7 +1931,7 @@ out:
 	return ret;
 }
 
-int kvm_vgic_create(struct kvm *kvm)
+int kvm_vgic_create(struct kvm *kvm, u32 type)
 {
 	int i, vcpu_lock_idx = -1, ret;
 	struct kvm_vcpu *vcpu;
@@ -1963,6 +1963,7 @@ int kvm_vgic_create(struct kvm *kvm)
 
 	spin_lock_init(&kvm->arch.vgic.lock);
 	kvm->arch.vgic.in_kernel = true;
+	kvm->arch.vgic.vgic_model = type;
 	kvm->arch.vgic.vctrl_base = vgic->vctrl_base;
 	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
 	kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
@@ -2388,7 +2389,7 @@ static void vgic_destroy(struct kvm_device *dev)
 
 static int vgic_create(struct kvm_device *dev, u32 type)
 {
-	return kvm_vgic_create(dev->kvm);
+	return kvm_vgic_create(dev->kvm, type);
 }
 
 static struct kvm_device_ops kvm_arm_vgic_v2_ops = {
Signature · 1 line
Jazz is not dead. It just smells funny.
#7 lore raw

PATCH v5 3/19 arm/arm64: KVM: refactor vgic_handle_mmio() function

In reply to Andre Przywara

Currently we only need to deal with one MMIO region for the GIC emulation (the GICv2 distributor), but we soon need to extend this. Refactor the existing code to allow easier addition of different ranges without code duplication.

---
Changelog v4...v5:
(none)

Changelog v3...v4:
- simplify is_in_range()
- added Reviewed-by:

 virt/kvm/arm/vgic.c |   73 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 53 insertions(+), 20 deletions(-)
virt/kvm/arm/vgic.c +53 -20
index c2e0795..9822735 100644
@@ -1033,36 +1033,27 @@ static bool vgic_validate_access(const struct vgic_dist *dist,
 }
 
 /**
- * vgic_handle_mmio - handle an in-kernel MMIO access
+ * vgic_handle_mmio_range - handle an in-kernel MMIO access
  * @vcpu:	pointer to the vcpu performing the access
  * @run:	pointer to the kvm_run structure
  * @mmio:	pointer to the data describing the access
+ * @ranges:	array of MMIO ranges in a given region
+ * @mmio_base:	base address of that region
  *
- * returns true if the MMIO access has been performed in kernel space,
- * and false if it needs to be emulated in user space.
+ * returns true if the MMIO access could be performed
  */
-bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
-		      struct kvm_exit_mmio *mmio)
+static bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
+			    struct kvm_exit_mmio *mmio,
+			    const struct mmio_range *ranges,
+			    unsigned long mmio_base)
 {
 	const struct mmio_range *range;
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
-	unsigned long base = dist->vgic_dist_base;
 	bool updated_state;
 	unsigned long offset;
 
-	if (!irqchip_in_kernel(vcpu->kvm) ||
-	    mmio->phys_addr < base ||
-	    (mmio->phys_addr + mmio->len) > (base + KVM_VGIC_V2_DIST_SIZE))
-		return false;
-
-	/* We don't support ldrd / strd or ldm / stm to the emulated vgic */
-	if (mmio->len > 4) {
-		kvm_inject_dabt(vcpu, mmio->phys_addr);
-		return true;
-	}
-
-	offset = mmio->phys_addr - base;
-	range = find_matching_range(vgic_dist_ranges, mmio, offset);
+	offset = mmio->phys_addr - mmio_base;
+	range = find_matching_range(ranges, mmio, offset);
 	if (unlikely(!range || !range->handle_mmio)) {
 		pr_warn("Unhandled access %d %08llx %d\n",
 			mmio->is_write, mmio->phys_addr, mmio->len);
@@ -1070,7 +1061,7 @@ bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	}
 
 	spin_lock(&vcpu->kvm->arch.vgic.lock);
-	offset = mmio->phys_addr - range->base - base;
+	offset -= range->base;
 	if (vgic_validate_access(dist, range, offset)) {
 		updated_state = range->handle_mmio(vcpu, mmio, offset);
 	} else {
@@ -1088,6 +1079,48 @@ bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	return true;
 }
 
+static inline bool is_in_range(phys_addr_t addr, unsigned long len,
+			       phys_addr_t baseaddr, unsigned long size)
+{
+	return (addr >= baseaddr) && (addr + len <= baseaddr + size);
+}
+
+static bool vgic_v2_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
+				struct kvm_exit_mmio *mmio)
+{
+	unsigned long base = vcpu->kvm->arch.vgic.vgic_dist_base;
+
+	if (!is_in_range(mmio->phys_addr, mmio->len, base,
+			 KVM_VGIC_V2_DIST_SIZE))
+		return false;
+
+	/* GICv2 does not support accesses wider than 32 bits */
+	if (mmio->len > 4) {
+		kvm_inject_dabt(vcpu, mmio->phys_addr);
+		return true;
+	}
+
+	return vgic_handle_mmio_range(vcpu, run, mmio, vgic_dist_ranges, base);
+}
+
+/**
+ * vgic_handle_mmio - handle an in-kernel MMIO access for the GIC emulation
+ * @vcpu:      pointer to the vcpu performing the access
+ * @run:       pointer to the kvm_run structure
+ * @mmio:      pointer to the data describing the access
+ *
+ * returns true if the MMIO access has been performed in kernel space,
+ * and false if it needs to be emulated in user space.
+ */
+bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
+		      struct kvm_exit_mmio *mmio)
+{
+	if (!irqchip_in_kernel(vcpu->kvm))
+		return false;
+
+	return vgic_v2_handle_mmio(vcpu, run, mmio);
+}
+
 static u8 *vgic_get_sgi_sources(struct vgic_dist *dist, int vcpu_id, int sgi)
 {
 	return dist->irq_sgi_sources + vcpu_id * VGIC_NR_SGIS + sgi;
Signature · 1 line
1.7.9.5
#8 lore raw

PATCH v5 3/19 arm/arm64: KVM: refactor vgic_handle_mmio() function

Quoted 7 lines · Andre Przywara

Currently we only need to deal with one MMIO region for the GIC emulation (the GICv2 distributor), but we soon need to extend this. Refactor the existing code to allow easier addition of different ranges without code duplication.

Quoted 120 lines

---
Changelog v4...v5:
(none)

Changelog v3...v4:
- simplify is_in_range()
- added Reviewed-by:

 virt/kvm/arm/vgic.c |   73 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 53 insertions(+), 20 deletions(-)
virt/kvm/arm/vgic.c +53 -20
index c2e0795..9822735 100644
@@ -1033,36 +1033,27 @@ static bool vgic_validate_access(const struct vgic_dist *dist,
 }
 
 /**
- * vgic_handle_mmio - handle an in-kernel MMIO access
+ * vgic_handle_mmio_range - handle an in-kernel MMIO access
  * @vcpu:	pointer to the vcpu performing the access
  * @run:	pointer to the kvm_run structure
  * @mmio:	pointer to the data describing the access
+ * @ranges:	array of MMIO ranges in a given region
+ * @mmio_base:	base address of that region
  *
- * returns true if the MMIO access has been performed in kernel space,
- * and false if it needs to be emulated in user space.
+ * returns true if the MMIO access could be performed
  */
-bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
-		      struct kvm_exit_mmio *mmio)
+static bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
+			    struct kvm_exit_mmio *mmio,
+			    const struct mmio_range *ranges,
+			    unsigned long mmio_base)
 {
 	const struct mmio_range *range;
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
-	unsigned long base = dist->vgic_dist_base;
 	bool updated_state;
 	unsigned long offset;
 
-	if (!irqchip_in_kernel(vcpu->kvm) ||
-	    mmio->phys_addr < base ||
-	    (mmio->phys_addr + mmio->len) > (base + KVM_VGIC_V2_DIST_SIZE))
-		return false;
-
-	/* We don't support ldrd / strd or ldm / stm to the emulated vgic */
-	if (mmio->len > 4) {
-		kvm_inject_dabt(vcpu, mmio->phys_addr);
-		return true;
-	}
-
-	offset = mmio->phys_addr - base;
-	range = find_matching_range(vgic_dist_ranges, mmio, offset);
+	offset = mmio->phys_addr - mmio_base;
+	range = find_matching_range(ranges, mmio, offset);
 	if (unlikely(!range || !range->handle_mmio)) {
 		pr_warn("Unhandled access %d %08llx %d\n",
 			mmio->is_write, mmio->phys_addr, mmio->len);
@@ -1070,7 +1061,7 @@ bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	}
 
 	spin_lock(&vcpu->kvm->arch.vgic.lock);
-	offset = mmio->phys_addr - range->base - base;
+	offset -= range->base;
 	if (vgic_validate_access(dist, range, offset)) {
 		updated_state = range->handle_mmio(vcpu, mmio, offset);
 	} else {
@@ -1088,6 +1079,48 @@ bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	return true;
 }
 
+static inline bool is_in_range(phys_addr_t addr, unsigned long len,
+			       phys_addr_t baseaddr, unsigned long size)
+{
+	return (addr >= baseaddr) && (addr + len <= baseaddr + size);
+}
+
+static bool vgic_v2_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
+				struct kvm_exit_mmio *mmio)
+{
+	unsigned long base = vcpu->kvm->arch.vgic.vgic_dist_base;
+
+	if (!is_in_range(mmio->phys_addr, mmio->len, base,
+			 KVM_VGIC_V2_DIST_SIZE))
+		return false;
+
+	/* GICv2 does not support accesses wider than 32 bits */
+	if (mmio->len > 4) {
+		kvm_inject_dabt(vcpu, mmio->phys_addr);
+		return true;
+	}
+
+	return vgic_handle_mmio_range(vcpu, run, mmio, vgic_dist_ranges, base);
+}
+
+/**
+ * vgic_handle_mmio - handle an in-kernel MMIO access for the GIC emulation
+ * @vcpu:      pointer to the vcpu performing the access
+ * @run:       pointer to the kvm_run structure
+ * @mmio:      pointer to the data describing the access
+ *
+ * returns true if the MMIO access has been performed in kernel space,
+ * and false if it needs to be emulated in user space.
+ */
+bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
+		      struct kvm_exit_mmio *mmio)
+{
+	if (!irqchip_in_kernel(vcpu->kvm))
+		return false;
+
+	return vgic_v2_handle_mmio(vcpu, run, mmio);
+}
+
 static u8 *vgic_get_sgi_sources(struct vgic_dist *dist, int vcpu_id, int sgi)
 {
 	return dist->irq_sgi_sources + vcpu_id * VGIC_NR_SGIS + sgi;
Signature · 1 line
Jazz is not dead. It just smells funny.
#9 lore raw

PATCH v5 4/19 arm/arm64: KVM: wrap 64 bit MMIO accesses with two 32 bit ones

In reply to Andre Przywara

Some GICv3 registers can and will be accessed as 64 bit registers. Currently the register handling code can only deal with 32 bit accesses, so we do two consecutive calls to cover this.

---
Changelog v4...v5:
(add Reviewed-by:)

Changelog v3...v4:
- add comment explaining little endian handling

 virt/kvm/arm/vgic.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 3 deletions(-)
virt/kvm/arm/vgic.c +50 -3
index 9822735..bc2546e 100644
@@ -1032,6 +1032,53 @@ static bool vgic_validate_access(const struct vgic_dist *dist,
 	return true;
 }
 
+/*
+ * Call the respective handler function for the given range.
+ * We split up any 64 bit accesses into two consecutive 32 bit
+ * handler calls and merge the result afterwards.
+ * We do this in a little endian fashion regardless of the host's
+ * or guest's endianness, because the GIC is always LE and the rest of
+ * the code (vgic_reg_access) also puts it in a LE fashion already.
+ * At this point we have already identified the handle function, so
+ * range points to that one entry and offset is relative to this.
+ */
+static bool call_range_handler(struct kvm_vcpu *vcpu,
+			       struct kvm_exit_mmio *mmio,
+			       unsigned long offset,
+			       const struct mmio_range *range)
+{
+	u32 *data32 = (void *)mmio->data;
+	struct kvm_exit_mmio mmio32;
+	bool ret;
+
+	if (likely(mmio->len <= 4))
+		return range->handle_mmio(vcpu, mmio, offset);
+
+	/*
+	 * Any access bigger than 4 bytes (that we currently handle in KVM)
+	 * is actually 8 bytes long, caused by a 64-bit access
+	 */
+
+	mmio32.len = 4;
+	mmio32.is_write = mmio->is_write;
+
+	mmio32.phys_addr = mmio->phys_addr + 4;
+	if (mmio->is_write)
+		*(u32 *)mmio32.data = data32[1];
+	ret = range->handle_mmio(vcpu, &mmio32, offset + 4);
+	if (!mmio->is_write)
+		data32[1] = *(u32 *)mmio32.data;
+
+	mmio32.phys_addr = mmio->phys_addr;
+	if (mmio->is_write)
+		*(u32 *)mmio32.data = data32[0];
+	ret |= range->handle_mmio(vcpu, &mmio32, offset);
+	if (!mmio->is_write)
+		data32[0] = *(u32 *)mmio32.data;
+
+	return ret;
+}
+
 /**
  * vgic_handle_mmio_range - handle an in-kernel MMIO access
  * @vcpu:	pointer to the vcpu performing the access
@@ -1063,10 +1110,10 @@ static bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	spin_lock(&vcpu->kvm->arch.vgic.lock);
 	offset -= range->base;
 	if (vgic_validate_access(dist, range, offset)) {
-		updated_state = range->handle_mmio(vcpu, mmio, offset);
+		updated_state = call_range_handler(vcpu, mmio, offset, range);
 	} else {
-		vgic_reg_access(mmio, NULL, offset,
-				ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+		if (!mmio->is_write)
+			memset(mmio->data, 0, mmio->len);
 		updated_state = false;
 	}
 	spin_unlock(&vcpu->kvm->arch.vgic.lock);
Signature · 1 line
1.7.9.5
#10 lore raw

PATCH v5 4/19 arm/arm64: KVM: wrap 64 bit MMIO accesses with two 32 bit ones

Quoted 6 lines · Andre Przywara

Some GICv3 registers can and will be accessed as 64 bit registers. Currently the register handling code can only deal with 32 bit accesses, so we do two consecutive calls to cover this.

Quoted 82 lines

---
Changelog v4...v5:
(add Reviewed-by:)

Changelog v3...v4:
- add comment explaining little endian handling

 virt/kvm/arm/vgic.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 3 deletions(-)
virt/kvm/arm/vgic.c +50 -3
index 9822735..bc2546e 100644
@@ -1032,6 +1032,53 @@ static bool vgic_validate_access(const struct vgic_dist *dist,
 	return true;
 }
 
+/*
+ * Call the respective handler function for the given range.
+ * We split up any 64 bit accesses into two consecutive 32 bit
+ * handler calls and merge the result afterwards.
+ * We do this in a little endian fashion regardless of the host's
+ * or guest's endianness, because the GIC is always LE and the rest of
+ * the code (vgic_reg_access) also puts it in a LE fashion already.
+ * At this point we have already identified the handle function, so
+ * range points to that one entry and offset is relative to this.
+ */
+static bool call_range_handler(struct kvm_vcpu *vcpu,
+			       struct kvm_exit_mmio *mmio,
+			       unsigned long offset,
+			       const struct mmio_range *range)
+{
+	u32 *data32 = (void *)mmio->data;
+	struct kvm_exit_mmio mmio32;
+	bool ret;
+
+	if (likely(mmio->len <= 4))
+		return range->handle_mmio(vcpu, mmio, offset);
+
+	/*
+	 * Any access bigger than 4 bytes (that we currently handle in KVM)
+	 * is actually 8 bytes long, caused by a 64-bit access
+	 */
+
+	mmio32.len = 4;
+	mmio32.is_write = mmio->is_write;
+
+	mmio32.phys_addr = mmio->phys_addr + 4;
+	if (mmio->is_write)
+		*(u32 *)mmio32.data = data32[1];
+	ret = range->handle_mmio(vcpu, &mmio32, offset + 4);
+	if (!mmio->is_write)
+		data32[1] = *(u32 *)mmio32.data;
+
+	mmio32.phys_addr = mmio->phys_addr;
+	if (mmio->is_write)
+		*(u32 *)mmio32.data = data32[0];
+	ret |= range->handle_mmio(vcpu, &mmio32, offset);
+	if (!mmio->is_write)
+		data32[0] = *(u32 *)mmio32.data;
+
+	return ret;
+}
+
 /**
  * vgic_handle_mmio_range - handle an in-kernel MMIO access
  * @vcpu:	pointer to the vcpu performing the access
@@ -1063,10 +1110,10 @@ static bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	spin_lock(&vcpu->kvm->arch.vgic.lock);
 	offset -= range->base;
 	if (vgic_validate_access(dist, range, offset)) {
-		updated_state = range->handle_mmio(vcpu, mmio, offset);
+		updated_state = call_range_handler(vcpu, mmio, offset, range);
 	} else {
-		vgic_reg_access(mmio, NULL, offset,
-				ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+		if (!mmio->is_write)
+			memset(mmio->data, 0, mmio->len);
 		updated_state = false;
 	}
 	spin_unlock(&vcpu->kvm->arch.vgic.lock);
Signature · 1 line
Jazz is not dead. It just smells funny.
#11 lore raw

PATCH v5 5/19 arm/arm64: KVM: introduce per-VM ops

In reply to Andre Przywara

Currently we only have one virtual GIC model supported, so all guests use the same emulation code. With the addition of another model we end up with different guests using potentially different vGIC models, so we have to split up some functions to be per VM. Introduce a vgic_vm_ops struct to hold function pointers for those functions that are different and provide the necessary code to initialize them.
Also split up the kvm_vgic_init() function to separate out VGIC model specific functionality into a separate function, which will later be different for a GICv3 model.

---
Changelog v4...v5:
- add vgic_init_maps() function pointer

Changelog v3...v4:
- add accessor functions for vm_ops members
- introduce init_vgic_model() to differentiate between guest GIC models
- simplify vgic_v2_init_emulation()
- help debugging by hinting on handle_mmio codeflow in comment

 include/kvm/arm_vgic.h |   11 +++++
 virt/kvm/arm/vgic.c    |  127 +++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 115 insertions(+), 23 deletions(-)
include/kvm/arm_vgic.h +11 -0
index dde5a00..e691932 100644
@@ -134,6 +134,15 @@ struct vgic_params {
 	void __iomem	*vctrl_base;
 };
 
+struct vgic_vm_ops {
+	bool	(*handle_mmio)(struct kvm_vcpu *, struct kvm_run *,
+			       struct kvm_exit_mmio *);
+	bool	(*queue_sgi)(struct kvm_vcpu *, int irq);
+	void	(*add_sgi_source)(struct kvm_vcpu *, int irq, int source);
+	int	(*vgic_init)(struct kvm *, const struct vgic_params *);
+	int	(*vgic_init_maps)(struct kvm *);
+};
+
 struct vgic_dist {
 #ifdef CONFIG_KVM_ARM_VGIC
 	spinlock_t		lock;
@@ -215,6 +224,8 @@ struct vgic_dist {
 
 	/* Bitmap indicating which CPU has something pending */
 	unsigned long		*irq_pending_on_cpu;
+
+	struct vgic_vm_ops	vm_ops;
 #endif
 };
 
virt/kvm/arm/vgic.c +104 -23

219 lines not on this page · raw message

Signature · 1 line
1.7.9.5
#12 lore raw

PATCH v5 5/19 arm/arm64: KVM: introduce per-VM ops

Quoted 11 lines · Andre Przywara

Currently we only have one virtual GIC model supported, so all guests use the same emulation code. With the addition of another model we end up with different guests using potentially different vGIC models, so we have to split up some functions to be per VM. Introduce a vgic_vm_ops struct to hold function pointers for those functions that are different and provide the necessary code to initialize them.
Also split up the kvm_vgic_init() function to separate out VGIC model specific functionality into a separate function, which will later be different for a GICv3 model.

Quoted 1 line
#13 lore raw

PATCH v5 5/19 arm/arm64: KVM: introduce per-VM ops

In reply to Andre Przywara
Quoted 11 lines · Andre Przywara

Currently we only have one virtual GIC model supported, so all guests use the same emulation code. With the addition of another model we end up with different guests using potentially different vGIC models, so we have to split up some functions to be per VM. Introduce a vgic_vm_ops struct to hold function pointers for those functions that are different and provide the necessary code to initialize them.
Also split up the kvm_vgic_init() function to separate out VGIC model specific functionality into a separate function, which will later be different for a GICv3 model.

Quoted 1 line
	M.
Signature · 1 line
Jazz is not dead. It just smells funny.
#14 lore raw

PATCH v5 6/19 arm/arm64: KVM: move kvm_register_device_ops() into vGIC probing

In reply to Andre Przywara

Currently we unconditionally register the GICv2 emulation device during the host's KVM initialization. Since with GICv3 support we may end up with only v2 or only v3 or both supported, we move the registration into the GIC probing function, where we will later know which combination is valid.

---
Changelog v4...v5:
(none)

Changelog v3...v4:
- add Acked-by

 include/linux/kvm_host.h |    1 +
 virt/kvm/arm/vgic-v2.c   |    2 ++
 virt/kvm/arm/vgic-v3.c   |    1 +
 virt/kvm/arm/vgic.c      |    5 ++---
 4 files changed, 6 insertions(+), 3 deletions(-)
include/linux/kvm_host.h +1 -0
index a6059bd..e674693 100644
@@ -1084,6 +1084,7 @@ void kvm_unregister_device_ops(u32 type);
 
 extern struct kvm_device_ops kvm_mpic_ops;
 extern struct kvm_device_ops kvm_xics_ops;
+extern struct kvm_device_ops kvm_arm_vgic_v2_ops;
 
 #ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
 
virt/kvm/arm/vgic-v2.c +2 -0
index 2935405..e1cd3cb 100644
@@ -229,6 +229,8 @@ int vgic_v2_probe(struct device_node *vgic_node,
 		goto out_unmap;
 	}
 
+	kvm_register_device_ops(&kvm_arm_vgic_v2_ops, KVM_DEV_TYPE_ARM_VGIC_V2);
+
 	vgic->vcpu_base = vcpu_res.start;
 
 	kvm_info("%s@%llx IRQ%d\n", vgic_node->name,
virt/kvm/arm/vgic-v3.c +1 -0
index 1c2c8ee..d14c75f 100644
@@ -230,6 +230,7 @@ int vgic_v3_probe(struct device_node *vgic_node,
 		ret = -ENXIO;
 		goto out;
 	}
+	kvm_register_device_ops(&kvm_arm_vgic_v2_ops, KVM_DEV_TYPE_ARM_VGIC_V2);
 
 	vgic->vcpu_base = vcpu_res.start;
 	vgic->vctrl_base = NULL;
virt/kvm/arm/vgic.c +2 -3
index 8c55fb8..38f48ca 100644
@@ -2553,7 +2553,7 @@ static int vgic_create(struct kvm_device *dev, u32 type)
 	return kvm_vgic_create(dev->kvm, type);
 }
 
-static struct kvm_device_ops kvm_arm_vgic_v2_ops = {
+struct kvm_device_ops kvm_arm_vgic_v2_ops = {
 	.name = "kvm-arm-vgic",
 	.create = vgic_create,
 	.destroy = vgic_destroy,
@@ -2632,8 +2632,7 @@ int kvm_vgic_hyp_init(void)
 
 	on_each_cpu(vgic_init_maintenance_interrupt, NULL, 1);
 
-	return kvm_register_device_ops(&kvm_arm_vgic_v2_ops,
-				       KVM_DEV_TYPE_ARM_VGIC_V2);
+	return 0;
 
 out_free_irq:
 	free_percpu_irq(vgic->maint_irq, kvm_get_running_vcpus());
Signature · 1 line
1.7.9.5
#15 lore raw

PATCH v5 6/19 arm/arm64: KVM: move kvm_register_device_ops() into vGIC probing

Quoted 8 lines · Andre Przywara

Currently we unconditionally register the GICv2 emulation device during the host's KVM initialization. Since with GICv3 support we may end up with only v2 or only v3 or both supported, we move the registration into the GIC probing function, where we will later know which combination is valid.

	M.
Signature · 1 line
Jazz is not dead. It just smells funny.
#16 lore raw

PATCH v5 7/19 arm/arm64: KVM: dont rely on a valid GICH base address

In reply to Andre Przywara

To check whether the vGIC was already initialized, we currently check the GICH base address for not being NULL. Since with GICv3 we may get along without this address, lets use the irqchip_in_kernel() function to detect an already initialized vGIC.

---
Changelog v4...v5:
(none)

Changelog v3...v4:
- add Acked-by

 virt/kvm/arm/vgic.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
virt/kvm/arm/vgic.c +1 -1
index 38f48ca..c481362 100644
@@ -2095,7 +2095,7 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
 
 	mutex_lock(&kvm->lock);
 
-	if (kvm->arch.vgic.vctrl_base) {
+	if (irqchip_in_kernel(kvm)) {
 		ret = -EEXIST;
 		goto out;
 	}
Signature · 1 line
1.7.9.5
#17 lore raw

PATCH v5 7/19 arm/arm64: KVM: dont rely on a valid GICH base address

Quoted 7 lines · Andre Przywara

To check whether the vGIC was already initialized, we currently check the GICH base address for not being NULL. Since with GICv3 we may get along without this address, lets use the irqchip_in_kernel() function to detect an already initialized vGIC.

	M.
Signature · 1 line
Jazz is not dead. It just smells funny.
#18 lore raw

PATCH v5 8/19 arm/arm64: KVM: make the maximum number of vCPUs a per-VM value

In reply to Andre Przywara

Currently the maximum number of vCPUs supported is a global value limited by the used GIC model. GICv3 will lift this limit, but we still need to observe it for guests using GICv2.
So the maximum number of vCPUs is per-VM value, depending on the GIC model the guest uses.
Store and check the value in struct kvm_arch, but keep it down to 8 for now.

---
Changelog v4...v5:
- add define for GIC_V2_MAX_CPUS
- rename max_hw_vcpus
- add prototype for ARM non-VGIC configs

Changelog v3...v4:
- initialize max_vcpus with limit based on host GIC
- remove *_init_emul_* from VGIC backend
- refine VCPU limit on VGIC creation
- print warning when userland tries to create more VCPUs than supported

 arch/arm/include/asm/kvm_host.h   |    1 +
 arch/arm/kvm/arm.c                |    8 ++++++++
 arch/arm64/include/asm/kvm_host.h |    3 +++
 include/kvm/arm_vgic.h            |    8 ++++++++
 virt/kvm/arm/vgic-v2.c            |    1 +
 virt/kvm/arm/vgic-v3.c            |    1 +
 virt/kvm/arm/vgic.c               |   22 ++++++++++++++++++++++
 7 files changed, 44 insertions(+)
arch/arm/include/asm/kvm_host.h +1 -0
index b443dfe..7969e6e 100644
@@ -68,6 +68,7 @@ struct kvm_arch {
 
 	/* Interrupt controller */
 	struct vgic_dist	vgic;
+	int max_vcpus;
 };
 
 #define KVM_NR_MEM_OBJS     40
arch/arm/kvm/arm.c +8 -0
index 8817fbd..c3d0fbd 100644
@@ -132,6 +132,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	/* Mark the initial VMID generation invalid */
 	kvm->arch.vmid_gen = 0;
 
+	/* The maximum number of VCPUs is limited by the host's GIC model */
+	kvm->arch.max_vcpus = kvm_vgic_get_max_vcpus();
+
 	return ret;
 out_free_stage2_pgd:
 	kvm_free_stage2_pgd(kvm);
@@ -213,6 +216,11 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 	int err;
 	struct kvm_vcpu *vcpu;
 
+	if (id >= kvm->arch.max_vcpus) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
 	if (!vcpu) {
 		err = -ENOMEM;
arch/arm64/include/asm/kvm_host.h +3 -0
index 286bb61..f9e130d 100644
@@ -59,6 +59,9 @@ struct kvm_arch {
 	/* VTTBR value associated with above pgd and vmid */
 	u64    vttbr;
 
+	/* The maximum number of vCPUs depends on the used GIC model */
+	int max_vcpus;
+
 	/* Interrupt controller */
 	struct vgic_dist	vgic;
 
include/kvm/arm_vgic.h +8 -0
index e691932..72a9fef 100644
@@ -33,6 +33,7 @@
 #define VGIC_V2_MAX_LRS		(1 << 6)
 #define VGIC_V3_MAX_LRS		16
 #define VGIC_MAX_IRQS		1024
+#define VGIC_V2_MAX_CPUS	8
 
 /* Sanity checks... */
 #if (KVM_MAX_VCPUS > 8)
@@ -132,6 +133,7 @@ struct vgic_params {
 	unsigned int	maint_irq;
 	/* Virtual control interface base address */
 	void __iomem	*vctrl_base;
+	int		max_gic_vcpus;
 };
 
 struct vgic_vm_ops {
@@ -288,6 +290,7 @@ struct kvm_exit_mmio;
 #ifdef CONFIG_KVM_ARM_VGIC
 int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write);
 int kvm_vgic_hyp_init(void);
+int kvm_vgic_get_max_vcpus(void);
 int kvm_vgic_init(struct kvm *kvm);
 int kvm_vgic_create(struct kvm *kvm, u32 type);
 void kvm_vgic_destroy(struct kvm *kvm);
@@ -387,6 +390,11 @@ static inline bool vgic_initialized(struct kvm *kvm)
 {
 	return true;
 }
+
+static inline int kvm_vgic_get_max_vcpus(void)
+{
+	return KVM_MAX_VCPUS;
+}
 #endif
 
 #endif
virt/kvm/arm/vgic-v2.c +1 -0
index e1cd3cb..e8b82b2 100644
@@ -237,6 +237,7 @@ int vgic_v2_probe(struct device_node *vgic_node,
 		 vctrl_res.start, vgic->maint_irq);
 
 	vgic->type = VGIC_V2;
+	vgic->max_gic_vcpus = VGIC_V2_MAX_CPUS;
 	*ops = &vgic_v2_ops;
 	*params = vgic;
 	goto out;
virt/kvm/arm/vgic-v3.c +1 -0
index d14c75f..ea39bad 100644
@@ -235,6 +235,7 @@ int vgic_v3_probe(struct device_node *vgic_node,
 	vgic->vcpu_base = vcpu_res.start;
 	vgic->vctrl_base = NULL;
 	vgic->type = VGIC_V3;
+	vgic->max_gic_vcpus = KVM_MAX_VCPUS;
 
 	kvm_info("%s@%llx IRQ%d\n", vgic_node->name,
 		 vcpu_res.start, vgic->maint_irq);
virt/kvm/arm/vgic.c +22 -0
index c481362..dcdbae5 100644
@@ -1848,6 +1848,17 @@ static int vgic_vcpu_init_maps(struct kvm_vcpu *vcpu, int nr_irqs)
 }
 
 /**
+ * kvm_vgic_get_max_vcpus - Get the maximum number of VCPUs allowed by HW
+ *
+ * The host's GIC naturally limits the maximum amount of VCPUs a guest
+ * can use.
+ */
+int kvm_vgic_get_max_vcpus(void)
+{
+	return vgic->max_gic_vcpus;
+}
+
+/**
  * kvm_vgic_vcpu_init - Initialize per-vcpu VGIC state
  * @vcpu: pointer to the vcpu struct
  *
@@ -2069,6 +2080,8 @@ static int vgic_v2_init_emulation(struct kvm *kvm)
 	dist->vm_ops.vgic_init = vgic_v2_init;
 	dist->vm_ops.vgic_init_maps = vgic_v2_init_maps;
 
+	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
+
 	return 0;
 }
 
@@ -2085,6 +2098,15 @@ static int init_vgic_model(struct kvm *kvm, int type)
 		break;
 	}
 
+	if (ret)
+		return ret;
+
+	if (atomic_read(&kvm->online_vcpus) > kvm->arch.max_vcpus) {
+		pr_warn_ratelimited("VGIC model only supports up to %d vCPUs\n",
+			kvm->arch.max_vcpus);
+		ret = -EINVAL;
+	}
+
 	return ret;
 }
 
Signature · 1 line
1.7.9.5
#19 lore raw

PATCH v5 8/19 arm/arm64: KVM: make the maximum number of vCPUs a per-VM value

Quoted 9 lines · Andre Przywara

Currently the maximum number of vCPUs supported is a global value limited by the used GIC model. GICv3 will lift this limit, but we still need to observe it for guests using GICv2.
So the maximum number of vCPUs is per-VM value, depending on the GIC model the guest uses.
Store and check the value in struct kvm_arch, but keep it down to 8 for now.

#20 lore raw

PATCH v5 8/19 arm/arm64: KVM: make the maximum number of vCPUs a per-VM value

In reply to Andre Przywara
Quoted 178 lines · Andre Przywara

Currently the maximum number of vCPUs supported is a global value limited by the used GIC model. GICv3 will lift this limit, but we still need to observe it for guests using GICv2.
So the maximum number of vCPUs is per-VM value, depending on the GIC model the guest uses.
Store and check the value in struct kvm_arch, but keep it down to 8 for now.

---
Changelog v4...v5:
- add define for GIC_V2_MAX_CPUS
- rename max_hw_vcpus
- add prototype for ARM non-VGIC configs

Changelog v3...v4:
- initialize max_vcpus with limit based on host GIC
- remove *_init_emul_* from VGIC backend
- refine VCPU limit on VGIC creation
- print warning when userland tries to create more VCPUs than supported

 arch/arm/include/asm/kvm_host.h   |    1 +
 arch/arm/kvm/arm.c                |    8 ++++++++
 arch/arm64/include/asm/kvm_host.h |    3 +++
 include/kvm/arm_vgic.h            |    8 ++++++++
 virt/kvm/arm/vgic-v2.c            |    1 +
 virt/kvm/arm/vgic-v3.c            |    1 +
 virt/kvm/arm/vgic.c               |   22 ++++++++++++++++++++++
 7 files changed, 44 insertions(+)
arch/arm/include/asm/kvm_host.h +1 -0
index b443dfe..7969e6e 100644
@@ -68,6 +68,7 @@ struct kvm_arch {
 
 	/* Interrupt controller */
 	struct vgic_dist	vgic;
+	int max_vcpus;
 };
 
 #define KVM_NR_MEM_OBJS     40
arch/arm/kvm/arm.c +8 -0
index 8817fbd..c3d0fbd 100644
@@ -132,6 +132,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	/* Mark the initial VMID generation invalid */
 	kvm->arch.vmid_gen = 0;
 
+	/* The maximum number of VCPUs is limited by the host's GIC model */
+	kvm->arch.max_vcpus = kvm_vgic_get_max_vcpus();
+
 	return ret;
 out_free_stage2_pgd:
 	kvm_free_stage2_pgd(kvm);
@@ -213,6 +216,11 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 	int err;
 	struct kvm_vcpu *vcpu;
 
+	if (id >= kvm->arch.max_vcpus) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
 	if (!vcpu) {
 		err = -ENOMEM;
arch/arm64/include/asm/kvm_host.h +3 -0
index 286bb61..f9e130d 100644
@@ -59,6 +59,9 @@ struct kvm_arch {
 	/* VTTBR value associated with above pgd and vmid */
 	u64    vttbr;
 
+	/* The maximum number of vCPUs depends on the used GIC model */
+	int max_vcpus;
+
 	/* Interrupt controller */
 	struct vgic_dist	vgic;
 
include/kvm/arm_vgic.h +8 -0
index e691932..72a9fef 100644
@@ -33,6 +33,7 @@
 #define VGIC_V2_MAX_LRS		(1 << 6)
 #define VGIC_V3_MAX_LRS		16
 #define VGIC_MAX_IRQS		1024
+#define VGIC_V2_MAX_CPUS	8
 
 /* Sanity checks... */
 #if (KVM_MAX_VCPUS > 8)
@@ -132,6 +133,7 @@ struct vgic_params {
 	unsigned int	maint_irq;
 	/* Virtual control interface base address */
 	void __iomem	*vctrl_base;
+	int		max_gic_vcpus;
 };
 
 struct vgic_vm_ops {
@@ -288,6 +290,7 @@ struct kvm_exit_mmio;
 #ifdef CONFIG_KVM_ARM_VGIC
 int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write);
 int kvm_vgic_hyp_init(void);
+int kvm_vgic_get_max_vcpus(void);
 int kvm_vgic_init(struct kvm *kvm);
 int kvm_vgic_create(struct kvm *kvm, u32 type);
 void kvm_vgic_destroy(struct kvm *kvm);
@@ -387,6 +390,11 @@ static inline bool vgic_initialized(struct kvm *kvm)
 {
 	return true;
 }
+
+static inline int kvm_vgic_get_max_vcpus(void)
+{
+	return KVM_MAX_VCPUS;
+}
 #endif
 
 #endif
virt/kvm/arm/vgic-v2.c +1 -0
index e1cd3cb..e8b82b2 100644
@@ -237,6 +237,7 @@ int vgic_v2_probe(struct device_node *vgic_node,
 		 vctrl_res.start, vgic->maint_irq);
 
 	vgic->type = VGIC_V2;
+	vgic->max_gic_vcpus = VGIC_V2_MAX_CPUS;
 	*ops = &vgic_v2_ops;
 	*params = vgic;
 	goto out;
virt/kvm/arm/vgic-v3.c +1 -0
index d14c75f..ea39bad 100644
@@ -235,6 +235,7 @@ int vgic_v3_probe(struct device_node *vgic_node,
 	vgic->vcpu_base = vcpu_res.start;
 	vgic->vctrl_base = NULL;
 	vgic->type = VGIC_V3;
+	vgic->max_gic_vcpus = KVM_MAX_VCPUS;
 
 	kvm_info("%s@%llx IRQ%d\n", vgic_node->name,
 		 vcpu_res.start, vgic->maint_irq);
virt/kvm/arm/vgic.c +13 -0
index c481362..dcdbae5 100644
@@ -1848,6 +1848,17 @@ static int vgic_vcpu_init_maps(struct kvm_vcpu *vcpu, int nr_irqs)
 }
 
 /**
+ * kvm_vgic_get_max_vcpus - Get the maximum number of VCPUs allowed by HW
+ *
+ * The host's GIC naturally limits the maximum amount of VCPUs a guest
+ * can use.
+ */
+int kvm_vgic_get_max_vcpus(void)
+{
+	return vgic->max_gic_vcpus;
+}
+
+/**
  * kvm_vgic_vcpu_init - Initialize per-vcpu VGIC state
  * @vcpu: pointer to the vcpu struct
  *
@@ -2069,6 +2080,8 @@ static int vgic_v2_init_emulation(struct kvm *kvm)
 	dist->vm_ops.vgic_init = vgic_v2_init;
 	dist->vm_ops.vgic_init_maps = vgic_v2_init_maps;
 
+	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
+
 	return 0;
 }
 

virt/kvm/arm/vgic.c static int init_vgic_model(struct kvm *kvm, int type) · line 2106

Quoted 9 lines
virt/kvm/arm/vgic.c +6 -0
@@ -2085,6 +2098,15 @@ static int init_vgic_model(struct kvm *kvm, int type)
 		break;
 	}
 
+	if (ret)
+		return ret;
+
+	if (atomic_read(&kvm->online_vcpus) > kvm->arch.max_vcpus) {
+		pr_warn_ratelimited("VGIC model only supports up to %d vCPUs\n",
+			kvm->arch.max_vcpus);

I'm not overly keen on the kernel spitting out messages based on userspace errors. Returning an error should be enough.

virt/kvm/arm/vgic.c static int init_vgic_model(struct kvm *kvm, int type) · line 2111

Quoted 5 lines
virt/kvm/arm/vgic.c +3 -0
+		ret = -EINVAL;
+	}
+
 	return ret;
 }

I'm also slightly confused as to how it works when I create a VM without an in-kernel GIC. How many vcpus am I allowed?

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny.
#21 lore raw

PATCH v5 8/19 arm/arm64: KVM: make the maximum number of vCPUs a per-VM value

Hi Marc,

Quoted 182 lines · Marc Zyngier
Quoted 181 lines · Andre Przywara

Currently the maximum number of vCPUs supported is a global value limited by the used GIC model. GICv3 will lift this limit, but we still need to observe it for guests using GICv2.
So the maximum number of vCPUs is per-VM value, depending on the GIC model the guest uses.
Store and check the value in struct kvm_arch, but keep it down to 8 for now.

---
Changelog v4...v5:
- add define for GIC_V2_MAX_CPUS
- rename max_hw_vcpus
- add prototype for ARM non-VGIC configs

Changelog v3...v4:
- initialize max_vcpus with limit based on host GIC
- remove *_init_emul_* from VGIC backend
- refine VCPU limit on VGIC creation
- print warning when userland tries to create more VCPUs than supported

 arch/arm/include/asm/kvm_host.h   |    1 +
 arch/arm/kvm/arm.c                |    8 ++++++++
 arch/arm64/include/asm/kvm_host.h |    3 +++
 include/kvm/arm_vgic.h            |    8 ++++++++
 virt/kvm/arm/vgic-v2.c            |    1 +
 virt/kvm/arm/vgic-v3.c            |    1 +
 virt/kvm/arm/vgic.c               |   22 ++++++++++++++++++++++
 7 files changed, 44 insertions(+)
arch/arm/include/asm/kvm_host.h +1 -0
index b443dfe..7969e6e 100644
@@ -68,6 +68,7 @@ struct kvm_arch {
 
 	/* Interrupt controller */
 	struct vgic_dist	vgic;
+	int max_vcpus;
 };
 
 #define KVM_NR_MEM_OBJS     40
arch/arm/kvm/arm.c +8 -0
index 8817fbd..c3d0fbd 100644
@@ -132,6 +132,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	/* Mark the initial VMID generation invalid */
 	kvm->arch.vmid_gen = 0;
 
+	/* The maximum number of VCPUs is limited by the host's GIC model */
+	kvm->arch.max_vcpus = kvm_vgic_get_max_vcpus();
+
 	return ret;
 out_free_stage2_pgd:
 	kvm_free_stage2_pgd(kvm);
@@ -213,6 +216,11 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 	int err;
 	struct kvm_vcpu *vcpu;
 
+	if (id >= kvm->arch.max_vcpus) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
 	if (!vcpu) {
 		err = -ENOMEM;
arch/arm64/include/asm/kvm_host.h +3 -0
index 286bb61..f9e130d 100644
@@ -59,6 +59,9 @@ struct kvm_arch {
 	/* VTTBR value associated with above pgd and vmid */
 	u64    vttbr;
 
+	/* The maximum number of vCPUs depends on the used GIC model */
+	int max_vcpus;
+
 	/* Interrupt controller */
 	struct vgic_dist	vgic;
 
include/kvm/arm_vgic.h +8 -0
index e691932..72a9fef 100644
@@ -33,6 +33,7 @@
 #define VGIC_V2_MAX_LRS		(1 << 6)
 #define VGIC_V3_MAX_LRS		16
 #define VGIC_MAX_IRQS		1024
+#define VGIC_V2_MAX_CPUS	8
 
 /* Sanity checks... */
 #if (KVM_MAX_VCPUS > 8)
@@ -132,6 +133,7 @@ struct vgic_params {
 	unsigned int	maint_irq;
 	/* Virtual control interface base address */
 	void __iomem	*vctrl_base;
+	int		max_gic_vcpus;
 };
 
 struct vgic_vm_ops {
@@ -288,6 +290,7 @@ struct kvm_exit_mmio;
 #ifdef CONFIG_KVM_ARM_VGIC
 int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write);
 int kvm_vgic_hyp_init(void);
+int kvm_vgic_get_max_vcpus(void);
 int kvm_vgic_init(struct kvm *kvm);
 int kvm_vgic_create(struct kvm *kvm, u32 type);
 void kvm_vgic_destroy(struct kvm *kvm);
@@ -387,6 +390,11 @@ static inline bool vgic_initialized(struct kvm *kvm)
 {
 	return true;
 }
+
+static inline int kvm_vgic_get_max_vcpus(void)
+{
+	return KVM_MAX_VCPUS;
+}
 #endif
 
 #endif
virt/kvm/arm/vgic-v2.c +1 -0
index e1cd3cb..e8b82b2 100644
@@ -237,6 +237,7 @@ int vgic_v2_probe(struct device_node *vgic_node,
 		 vctrl_res.start, vgic->maint_irq);
 
 	vgic->type = VGIC_V2;
+	vgic->max_gic_vcpus = VGIC_V2_MAX_CPUS;
 	*ops = &vgic_v2_ops;
 	*params = vgic;
 	goto out;
virt/kvm/arm/vgic-v3.c +1 -0
index d14c75f..ea39bad 100644
@@ -235,6 +235,7 @@ int vgic_v3_probe(struct device_node *vgic_node,
 	vgic->vcpu_base = vcpu_res.start;
 	vgic->vctrl_base = NULL;
 	vgic->type = VGIC_V3;
+	vgic->max_gic_vcpus = KVM_MAX_VCPUS;
 
 	kvm_info("%s@%llx IRQ%d\n", vgic_node->name,
 		 vcpu_res.start, vgic->maint_irq);
virt/kvm/arm/vgic.c +13 -0
index c481362..dcdbae5 100644
@@ -1848,6 +1848,17 @@ static int vgic_vcpu_init_maps(struct kvm_vcpu *vcpu, int nr_irqs)
 }
 
 /**
+ * kvm_vgic_get_max_vcpus - Get the maximum number of VCPUs allowed by HW
+ *
+ * The host's GIC naturally limits the maximum amount of VCPUs a guest
+ * can use.
+ */
+int kvm_vgic_get_max_vcpus(void)
+{
+	return vgic->max_gic_vcpus;
+}
+
+/**
  * kvm_vgic_vcpu_init - Initialize per-vcpu VGIC state
  * @vcpu: pointer to the vcpu struct
  *
@@ -2069,6 +2080,8 @@ static int vgic_v2_init_emulation(struct kvm *kvm)
 	dist->vm_ops.vgic_init = vgic_v2_init;
 	dist->vm_ops.vgic_init_maps = vgic_v2_init_maps;
 
+	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
+
 	return 0;
 }
 
@@ -2085,6 +2098,15 @@ static int init_vgic_model(struct kvm *kvm, int type)
 		break;
 	}
 
Quoted 9 lines

virt/kvm/arm/vgic.c static int init_vgic_model(struct kvm *kvm, int type) · line 2106

Quoted 6 lines
virt/kvm/arm/vgic.c +6 -0
+	if (ret)
+		return ret;
+
+	if (atomic_read(&kvm->online_vcpus) > kvm->arch.max_vcpus) {
+		pr_warn_ratelimited("VGIC model only supports up to %d vCPUs\n",
+			kvm->arch.max_vcpus);

I'm not overly keen on the kernel spitting out messages based on userspace errors. Returning an error should be enough.

I added this message after a comment from Christoffer:

Quoted 9 lines · Christoffer Dall

virt/kvm/arm/vgic.c static int init_vgic_model(struct kvm *kvm, int type) · line 2111

Quoted 5 lines
virt/kvm/arm/vgic.c +4 -0
 	case KVM_DEV_TYPE_ARM_VGIC_V2:
+		nr_vcpus = atomic_read(&kvm->online_vcpus);
+		if (nr_vcpus > 8)
+			return false;
+

I have a feeling we could be seeing this error a bit, could we dedicate an error code for the purpose or print a ratelimited warning or something?

Christoffer, can you state whether it's OK to do away with the warning message and just use the -EINVAL?

Quoted 9 lines

virt/kvm/arm/vgic.c static int init_vgic_model(struct kvm *kvm, int type) · line 2111

Quoted 6 lines
virt/kvm/arm/vgic.c +3 -0
+		ret = -EINVAL;
+	}
+
 	return ret;
 }

I'm also slightly confused as to how it works when I create a VM without an in-kernel GIC. How many vcpus am I allowed?

Mmmh, fair point. I guess we should just allow KVM_MAX_VCPUS in this case, right? Has anyone tested userland GIC emulation recently? I will see if I find some method of testing this with QEMU.

Cheers,
Andre.

#22 lore raw

PATCH v5 8/19 arm/arm64: KVM: make the maximum number of vCPUs a per-VM value

Quoted 207 lines · Andre Przywara

Hi Marc,

Quoted 182 lines · Marc Zyngier
Quoted 181 lines · Andre Przywara

Currently the maximum number of vCPUs supported is a global value limited by the used GIC model. GICv3 will lift this limit, but we still need to observe it for guests using GICv2.
So the maximum number of vCPUs is per-VM value, depending on the GIC model the guest uses.
Store and check the value in struct kvm_arch, but keep it down to 8 for now.

---
Changelog v4...v5:
- add define for GIC_V2_MAX_CPUS
- rename max_hw_vcpus
- add prototype for ARM non-VGIC configs

Changelog v3...v4:
- initialize max_vcpus with limit based on host GIC
- remove *_init_emul_* from VGIC backend
- refine VCPU limit on VGIC creation
- print warning when userland tries to create more VCPUs than supported

 arch/arm/include/asm/kvm_host.h   |    1 +
 arch/arm/kvm/arm.c                |    8 ++++++++
 arch/arm64/include/asm/kvm_host.h |    3 +++
 include/kvm/arm_vgic.h            |    8 ++++++++
 virt/kvm/arm/vgic-v2.c            |    1 +
 virt/kvm/arm/vgic-v3.c            |    1 +
 virt/kvm/arm/vgic.c               |   22 ++++++++++++++++++++++
 7 files changed, 44 insertions(+)
arch/arm/include/asm/kvm_host.h +1 -0
index b443dfe..7969e6e 100644
@@ -68,6 +68,7 @@ struct kvm_arch {
 
 	/* Interrupt controller */
 	struct vgic_dist	vgic;
+	int max_vcpus;
 };
 
 #define KVM_NR_MEM_OBJS     40
arch/arm/kvm/arm.c +8 -0
index 8817fbd..c3d0fbd 100644
@@ -132,6 +132,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	/* Mark the initial VMID generation invalid */
 	kvm->arch.vmid_gen = 0;
 
+	/* The maximum number of VCPUs is limited by the host's GIC model */
+	kvm->arch.max_vcpus = kvm_vgic_get_max_vcpus();
+
 	return ret;
 out_free_stage2_pgd:
 	kvm_free_stage2_pgd(kvm);
@@ -213,6 +216,11 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 	int err;
 	struct kvm_vcpu *vcpu;
 
+	if (id >= kvm->arch.max_vcpus) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
 	if (!vcpu) {
 		err = -ENOMEM;
arch/arm64/include/asm/kvm_host.h +3 -0
index 286bb61..f9e130d 100644
@@ -59,6 +59,9 @@ struct kvm_arch {
 	/* VTTBR value associated with above pgd and vmid */
 	u64    vttbr;
 
+	/* The maximum number of vCPUs depends on the used GIC model */
+	int max_vcpus;
+
 	/* Interrupt controller */
 	struct vgic_dist	vgic;
 
include/kvm/arm_vgic.h +8 -0
index e691932..72a9fef 100644
@@ -33,6 +33,7 @@
 #define VGIC_V2_MAX_LRS		(1 << 6)
 #define VGIC_V3_MAX_LRS		16
 #define VGIC_MAX_IRQS		1024
+#define VGIC_V2_MAX_CPUS	8
 
 /* Sanity checks... */
 #if (KVM_MAX_VCPUS > 8)
@@ -132,6 +133,7 @@ struct vgic_params {
 	unsigned int	maint_irq;
 	/* Virtual control interface base address */
 	void __iomem	*vctrl_base;
+	int		max_gic_vcpus;
 };
 
 struct vgic_vm_ops {
@@ -288,6 +290,7 @@ struct kvm_exit_mmio;
 #ifdef CONFIG_KVM_ARM_VGIC
 int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write);
 int kvm_vgic_hyp_init(void);
+int kvm_vgic_get_max_vcpus(void);
 int kvm_vgic_init(struct kvm *kvm);
 int kvm_vgic_create(struct kvm *kvm, u32 type);
 void kvm_vgic_destroy(struct kvm *kvm);
@@ -387,6 +390,11 @@ static inline bool vgic_initialized(struct kvm *kvm)
 {
 	return true;
 }
+
+static inline int kvm_vgic_get_max_vcpus(void)
+{
+	return KVM_MAX_VCPUS;
+}
 #endif
 
 #endif
virt/kvm/arm/vgic-v2.c +1 -0
index e1cd3cb..e8b82b2 100644
@@ -237,6 +237,7 @@ int vgic_v2_probe(struct device_node *vgic_node,
 		 vctrl_res.start, vgic->maint_irq);
 
 	vgic->type = VGIC_V2;
+	vgic->max_gic_vcpus = VGIC_V2_MAX_CPUS;
 	*ops = &vgic_v2_ops;
 	*params = vgic;
 	goto out;
virt/kvm/arm/vgic-v3.c +1 -0
index d14c75f..ea39bad 100644
@@ -235,6 +235,7 @@ int vgic_v3_probe(struct device_node *vgic_node,
 	vgic->vcpu_base = vcpu_res.start;
 	vgic->vctrl_base = NULL;
 	vgic->type = VGIC_V3;
+	vgic->max_gic_vcpus = KVM_MAX_VCPUS;
 
 	kvm_info("%s@%llx IRQ%d\n", vgic_node->name,
 		 vcpu_res.start, vgic->maint_irq);
virt/kvm/arm/vgic.c +13 -0
index c481362..dcdbae5 100644
@@ -1848,6 +1848,17 @@ static int vgic_vcpu_init_maps(struct kvm_vcpu *vcpu, int nr_irqs)
 }
 
 /**
+ * kvm_vgic_get_max_vcpus - Get the maximum number of VCPUs allowed by HW
+ *
+ * The host's GIC naturally limits the maximum amount of VCPUs a guest
+ * can use.
+ */
+int kvm_vgic_get_max_vcpus(void)
+{
+	return vgic->max_gic_vcpus;
+}
+
+/**
  * kvm_vgic_vcpu_init - Initialize per-vcpu VGIC state
  * @vcpu: pointer to the vcpu struct
  *
@@ -2069,6 +2080,8 @@ static int vgic_v2_init_emulation(struct kvm *kvm)
 	dist->vm_ops.vgic_init = vgic_v2_init;
 	dist->vm_ops.vgic_init_maps = vgic_v2_init_maps;
 
+	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
+
 	return 0;
 }
 
@@ -2085,6 +2098,15 @@ static int init_vgic_model(struct kvm *kvm, int type)
 		break;
 	}
 
Quoted 9 lines

virt/kvm/arm/vgic.c static int init_vgic_model(struct kvm *kvm, int type) · line 2106

Quoted 6 lines
virt/kvm/arm/vgic.c +6 -0
+	if (ret)
+		return ret;
+
+	if (atomic_read(&kvm->online_vcpus) > kvm->arch.max_vcpus) {
+		pr_warn_ratelimited("VGIC model only supports up to %d vCPUs\n",
+			kvm->arch.max_vcpus);

I'm not overly keen on the kernel spitting out messages based on userspace errors. Returning an error should be enough.

I added this message after a comment from Christoffer:

Quoted 9 lines · Christoffer Dall

virt/kvm/arm/vgic.c static int init_vgic_model(struct kvm *kvm, int type) · line 2111

Quoted 5 lines
virt/kvm/arm/vgic.c +4 -0
 	case KVM_DEV_TYPE_ARM_VGIC_V2:
+		nr_vcpus = atomic_read(&kvm->online_vcpus);
+		if (nr_vcpus > 8)
+			return false;
+

I have a feeling we could be seeing this error a bit, could we dedicate an error code for the purpose or print a ratelimited warning or something?

Quoted 3 lines

Christoffer, can you state whether it's OK to do away with the warning message and just use the -EINVAL?

we saw these errors from users a lot in the past, hence my comment.

can you use a different error code?

otherwise turn it into a kvm_debug().

Quoted 10 lines
Quoted 9 lines

virt/kvm/arm/vgic.c static int init_vgic_model(struct kvm *kvm, int type) · line 2111

Quoted 6 lines
virt/kvm/arm/vgic.c +3 -0
+		ret = -EINVAL;
+	}
+
 	return ret;
 }

I'm also slightly confused as to how it works when I create a VM without an in-kernel GIC. How many vcpus am I allowed?

Quoted 4 lines

Mmmh, fair point. I guess we should just allow KVM_MAX_VCPUS in this case, right? Has anyone tested userland GIC emulation recently? I will see if I find some method of testing this with QEMU.

It's been a while (late 2013 if my memory serves me right).

Allowing KVM_MAX_VCPUS does sound reasonable then, yes. But if you're testing on arm64 it probably won't work because you can't use architected timers with a userland gic and I don't think we emulate an external timer for the virt board on arm64.

-Christoffer

#23 lore raw

PATCH v5 8/19 arm/arm64: KVM: make the maximum number of vCPUs a per-VM value

Hi Christoffer,

Quoted 233 lines · Christoffer Dall
Quoted 207 lines · Andre Przywara

Hi Marc,

Quoted 182 lines · Marc Zyngier
Quoted 181 lines · Andre Przywara

Currently the maximum number of vCPUs supported is a global value limited by the used GIC model. GICv3 will lift this limit, but we still need to observe it for guests using GICv2.
So the maximum number of vCPUs is per-VM value, depending on the GIC model the guest uses.
Store and check the value in struct kvm_arch, but keep it down to 8 for now.

---
Changelog v4...v5:
- add define for GIC_V2_MAX_CPUS
- rename max_hw_vcpus
- add prototype for ARM non-VGIC configs

Changelog v3...v4:
- initialize max_vcpus with limit based on host GIC
- remove *_init_emul_* from VGIC backend
- refine VCPU limit on VGIC creation
- print warning when userland tries to create more VCPUs than supported

 arch/arm/include/asm/kvm_host.h   |    1 +
 arch/arm/kvm/arm.c                |    8 ++++++++
 arch/arm64/include/asm/kvm_host.h |    3 +++
 include/kvm/arm_vgic.h            |    8 ++++++++
 virt/kvm/arm/vgic-v2.c            |    1 +
 virt/kvm/arm/vgic-v3.c            |    1 +
 virt/kvm/arm/vgic.c               |   22 ++++++++++++++++++++++
 7 files changed, 44 insertions(+)
arch/arm/include/asm/kvm_host.h +1 -0
index b443dfe..7969e6e 100644
@@ -68,6 +68,7 @@ struct kvm_arch {
 
 	/* Interrupt controller */
 	struct vgic_dist	vgic;
+	int max_vcpus;
 };
 
 #define KVM_NR_MEM_OBJS     40
arch/arm/kvm/arm.c +8 -0
index 8817fbd..c3d0fbd 100644
@@ -132,6 +132,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	/* Mark the initial VMID generation invalid */
 	kvm->arch.vmid_gen = 0;
 
+	/* The maximum number of VCPUs is limited by the host's GIC model */
+	kvm->arch.max_vcpus = kvm_vgic_get_max_vcpus();
+
 	return ret;
 out_free_stage2_pgd:
 	kvm_free_stage2_pgd(kvm);
@@ -213,6 +216,11 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 	int err;
 	struct kvm_vcpu *vcpu;
 
+	if (id >= kvm->arch.max_vcpus) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
 	if (!vcpu) {
 		err = -ENOMEM;
arch/arm64/include/asm/kvm_host.h +3 -0
index 286bb61..f9e130d 100644
@@ -59,6 +59,9 @@ struct kvm_arch {
 	/* VTTBR value associated with above pgd and vmid */
 	u64    vttbr;
 
+	/* The maximum number of vCPUs depends on the used GIC model */
+	int max_vcpus;
+
 	/* Interrupt controller */
 	struct vgic_dist	vgic;
 
include/kvm/arm_vgic.h +8 -0
index e691932..72a9fef 100644
@@ -33,6 +33,7 @@
 #define VGIC_V2_MAX_LRS		(1 << 6)
 #define VGIC_V3_MAX_LRS		16
 #define VGIC_MAX_IRQS		1024
+#define VGIC_V2_MAX_CPUS	8
 
 /* Sanity checks... */
 #if (KVM_MAX_VCPUS > 8)
@@ -132,6 +133,7 @@ struct vgic_params {
 	unsigned int	maint_irq;
 	/* Virtual control interface base address */
 	void __iomem	*vctrl_base;
+	int		max_gic_vcpus;
 };
 
 struct vgic_vm_ops {
@@ -288,6 +290,7 @@ struct kvm_exit_mmio;
 #ifdef CONFIG_KVM_ARM_VGIC
 int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write);
 int kvm_vgic_hyp_init(void);
+int kvm_vgic_get_max_vcpus(void);
 int kvm_vgic_init(struct kvm *kvm);
 int kvm_vgic_create(struct kvm *kvm, u32 type);
 void kvm_vgic_destroy(struct kvm *kvm);
@@ -387,6 +390,11 @@ static inline bool vgic_initialized(struct kvm *kvm)
 {
 	return true;
 }
+
+static inline int kvm_vgic_get_max_vcpus(void)
+{
+	return KVM_MAX_VCPUS;
+}
 #endif
 
 #endif
virt/kvm/arm/vgic-v2.c +1 -0
index e1cd3cb..e8b82b2 100644
@@ -237,6 +237,7 @@ int vgic_v2_probe(struct device_node *vgic_node,
 		 vctrl_res.start, vgic->maint_irq);
 
 	vgic->type = VGIC_V2;
+	vgic->max_gic_vcpus = VGIC_V2_MAX_CPUS;
 	*ops = &vgic_v2_ops;
 	*params = vgic;
 	goto out;
virt/kvm/arm/vgic-v3.c +1 -0
index d14c75f..ea39bad 100644
@@ -235,6 +235,7 @@ int vgic_v3_probe(struct device_node *vgic_node,
 	vgic->vcpu_base = vcpu_res.start;
 	vgic->vctrl_base = NULL;
 	vgic->type = VGIC_V3;
+	vgic->max_gic_vcpus = KVM_MAX_VCPUS;
 
 	kvm_info("%s@%llx IRQ%d\n", vgic_node->name,
 		 vcpu_res.start, vgic->maint_irq);
virt/kvm/arm/vgic.c +13 -0
index c481362..dcdbae5 100644
@@ -1848,6 +1848,17 @@ static int vgic_vcpu_init_maps(struct kvm_vcpu *vcpu, int nr_irqs)
 }
 
 /**
+ * kvm_vgic_get_max_vcpus - Get the maximum number of VCPUs allowed by HW
+ *
+ * The host's GIC naturally limits the maximum amount of VCPUs a guest
+ * can use.
+ */
+int kvm_vgic_get_max_vcpus(void)
+{
+	return vgic->max_gic_vcpus;
+}
+
+/**
  * kvm_vgic_vcpu_init - Initialize per-vcpu VGIC state
  * @vcpu: pointer to the vcpu struct
  *
@@ -2069,6 +2080,8 @@ static int vgic_v2_init_emulation(struct kvm *kvm)
 	dist->vm_ops.vgic_init = vgic_v2_init;
 	dist->vm_ops.vgic_init_maps = vgic_v2_init_maps;
 
+	kvm->arch.max_vcpus = VGIC_V2_MAX_CPUS;
+
 	return 0;
 }
 
@@ -2085,6 +2098,15 @@ static int init_vgic_model(struct kvm *kvm, int type)
 		break;
 	}
 
Quoted 9 lines

virt/kvm/arm/vgic.c static int init_vgic_model(struct kvm *kvm, int type) · line 2106

Quoted 6 lines
virt/kvm/arm/vgic.c +6 -0
+	if (ret)
+		return ret;
+
+	if (atomic_read(&kvm->online_vcpus) > kvm->arch.max_vcpus) {
+		pr_warn_ratelimited("VGIC model only supports up to %d vCPUs\n",
+			kvm->arch.max_vcpus);

I'm not overly keen on the kernel spitting out messages based on userspace errors. Returning an error should be enough.

I added this message after a comment from Christoffer:

Quoted 9 lines · Christoffer Dall

virt/kvm/arm/vgic.c static int init_vgic_model(struct kvm *kvm, int type) · line 2111

Quoted 5 lines
virt/kvm/arm/vgic.c +4 -0
 	case KVM_DEV_TYPE_ARM_VGIC_V2:
+		nr_vcpus = atomic_read(&kvm->online_vcpus);
+		if (nr_vcpus > 8)
+			return false;
+

I have a feeling we could be seeing this error a bit, could we dedicate an error code for the purpose or print a ratelimited warning or something?

Quoted 3 lines

Christoffer, can you state whether it's OK to do away with the warning message and just use the -EINVAL?

we saw these errors from users a lot in the past, hence my comment.

can you use a different error code?

otherwise turn it into a kvm_debug().

Quoted 10 lines
Quoted 9 lines

virt/kvm/arm/vgic.c static int init_vgic_model(struct kvm *kvm, int type) · line 2111

Quoted 6 lines
virt/kvm/arm/vgic.c +3 -0
+		ret = -EINVAL;
+	}
+
 	return ret;
 }

I'm also slightly confused as to how it works when I create a VM without an in-kernel GIC. How many vcpus am I allowed?

Quoted 4 lines

Mmmh, fair point. I guess we should just allow KVM_MAX_VCPUS in this case, right? Has anyone tested userland GIC emulation recently? I will see if I find some method of testing this with QEMU.

Quoted 6 lines

It's been a while (late 2013 if my memory serves me right).

Allowing KVM_MAX_VCPUS does sound reasonable then, yes. But if you're testing on arm64 it probably won't work because you can't use architected timers with a userland gic and I don't think we emulate an external timer for the virt board on arm64.

Right, I just learned today that KVM does not even initialize on arm64 if there is no VGIC support available.
In fact I spent this morning to find a good solution for the issue that with the above changes in the VGICv3-only case KVM_CREATE_DEVICE(GICV2) is rightfully rejected, but KVM_CREATE_IRQCHIP still succeeds (though the guest hangs later of course). Keep you posted on that progress...

Cheers,
Andre.

#24 lore raw

PATCH v5 8/19 arm/arm64: KVM: make the maximum number of vCPUs a per-VM value

In reply to Andre Przywara
Quoted 3 lines · Andre Przywara

Mmmh, fair point. I guess we should just allow KVM_MAX_VCPUS in this case, right? Has anyone tested userland GIC emulation recently? I will see if I find some method of testing this with QEMU.

I did actually use this when I was trying to track down the bugs that turned out to be vgic-init-ordering last month. If you add -machine kernel_irqchip=off to your QEMU command line it will try to use an emulated userspace GIC. But as Christoffer says, there is a catch: there's no way to get the vcpu's generic timers to tell userspace that there's been a timer interrupt that needs to be fed into the userspace GIC. So you need to fiddle with the device tree to tell the guest the generic timers don't exist, and provide a different timer, at least in theory [though I didn't actually end up having to do that, I think it was just fluke that it worked for my testing].

-- PMM

#25 lore raw

PATCH v5 9/19 arm/arm64: KVM: make the value of ICC_SRE_EL1 a per-VM variable

In reply to Andre Przywara

ICC_SRE_EL1 is a system register allowing msr/mrs accesses to the GIC CPU interface for EL1 (guests). Currently we force it to 0, but for proper GICv3 support we have to allow guests to use it (depending on their selected virtual GIC model).
So add ICC_SRE_EL1 to the list of saved/restored registers on a world switch, but actually disallow a guest to change it by only restoring a fixed, once-initialized value.
This value depends on the GIC model userland has chosen for a guest.

---
Changelog v4...v5:
(none)

Changelog v3...v4:
- initialize variable on declaration

 arch/arm64/kernel/asm-offsets.c |    1 +
 arch/arm64/kvm/vgic-v3-switch.S |   14 +++++++++-----
 include/kvm/arm_vgic.h          |    1 +
 virt/kvm/arm/vgic-v3.c          |    8 ++++++--
 4 files changed, 17 insertions(+), 7 deletions(-)
arch/arm64/kernel/asm-offsets.c +1 -0
index 9a9fce0..9d34486 100644
@@ -140,6 +140,7 @@ int main(void)
   DEFINE(VGIC_V2_CPU_ELRSR,	offsetof(struct vgic_cpu, vgic_v2.vgic_elrsr));
   DEFINE(VGIC_V2_CPU_APR,	offsetof(struct vgic_cpu, vgic_v2.vgic_apr));
   DEFINE(VGIC_V2_CPU_LR,	offsetof(struct vgic_cpu, vgic_v2.vgic_lr));
+  DEFINE(VGIC_V3_CPU_SRE,	offsetof(struct vgic_cpu, vgic_v3.vgic_sre));
   DEFINE(VGIC_V3_CPU_HCR,	offsetof(struct vgic_cpu, vgic_v3.vgic_hcr));
   DEFINE(VGIC_V3_CPU_VMCR,	offsetof(struct vgic_cpu, vgic_v3.vgic_vmcr));
   DEFINE(VGIC_V3_CPU_MISR,	offsetof(struct vgic_cpu, vgic_v3.vgic_misr));
arch/arm64/kvm/vgic-v3-switch.S +9 -5
index d160469..617a012 100644
@@ -148,17 +148,18 @@
  * x0: Register pointing to VCPU struct
  */
 .macro	restore_vgic_v3_state
-	// Disable SRE_EL1 access. Necessary, otherwise
-	// ICH_VMCR_EL2.VFIQEn becomes one, and FIQ happens...
-	msr_s	ICC_SRE_EL1, xzr
-	isb
-
 	// Compute the address of struct vgic_cpu
 	add	x3, x0, #VCPU_VGIC_CPU
 
 	// Restore all interesting registers
 	ldr	w4, [x3, #VGIC_V3_CPU_HCR]
 	ldr	w5, [x3, #VGIC_V3_CPU_VMCR]
+	ldr	w25, [x3, #VGIC_V3_CPU_SRE]
+
+	msr_s	ICC_SRE_EL1, x25
+
+	// make sure SRE is valid before writing the other registers
+	isb
 
 	msr_s	ICH_HCR_EL2, x4
 	msr_s	ICH_VMCR_EL2, x5
@@ -244,9 +245,12 @@
 	dsb	sy
 
 	// Prevent the guest from touching the GIC system registers
+	// if SRE isn't enabled for GICv3 emulation
+	cbnz	x25, 1f
 	mrs_s	x5, ICC_SRE_EL2
 	and	x5, x5, #~ICC_SRE_EL2_ENABLE
 	msr_s	ICC_SRE_EL2, x5
+1:
 .endm
 
 ENTRY(__save_vgic_v3_state)
include/kvm/arm_vgic.h +1 -0
index 72a9fef..f7570f83 100644
@@ -245,6 +245,7 @@ struct vgic_v3_cpu_if {
 #ifdef CONFIG_ARM_GIC_V3
 	u32		vgic_hcr;
 	u32		vgic_vmcr;
+	u32		vgic_sre;	/* Restored only, change ignored */
 	u32		vgic_misr;	/* Saved only */
 	u32		vgic_eisr;	/* Saved only */
 	u32		vgic_elrsr;	/* Saved only */
virt/kvm/arm/vgic-v3.c +6 -2
index ea39bad..5249048 100644
@@ -145,15 +145,19 @@ static void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp)
 
 static void vgic_v3_enable(struct kvm_vcpu *vcpu)
 {
+	struct vgic_v3_cpu_if *vgic_v3 = &vcpu->arch.vgic_cpu.vgic_v3;
+
 	/*
 	 * By forcing VMCR to zero, the GIC will restore the binary
 	 * points to their reset values. Anything else resets to zero
 	 * anyway.
 	 */
-	vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr = 0;
+	vgic_v3->vgic_vmcr = 0;
+
+	vgic_v3->vgic_sre = 0;
 
 	/* Get the show on the road... */
-	vcpu->arch.vgic_cpu.vgic_v3.vgic_hcr = ICH_HCR_EN;
+	vgic_v3->vgic_hcr = ICH_HCR_EN;
 }
 
 static const struct vgic_ops vgic_v3_ops = {
Signature · 1 line
1.7.9.5
#26 lore raw

PATCH v5 9/19 arm/arm64: KVM: make the value of ICC_SRE_EL1 a per-VM variable

Quoted 11 lines · Andre Przywara

ICC_SRE_EL1 is a system register allowing msr/mrs accesses to the GIC CPU interface for EL1 (guests). Currently we force it to 0, but for proper GICv3 support we have to allow guests to use it (depending on their selected virtual GIC model).
So add ICC_SRE_EL1 to the list of saved/restored registers on a world switch, but actually disallow a guest to change it by only restoring a fixed, once-initialized value.
This value depends on the GIC model userland has chosen for a guest.

	M.
Signature · 1 line
Jazz is not dead. It just smells funny.
#27 lore raw

PATCH v5 10/19 arm/arm64: KVM: refactor MMIO accessors

In reply to Andre Przywara

The MMIO accessors for GICD_I[CS]ENABLER, GICD_I[CS]PENDR and GICD_ICFGR behave very similar for GICv2 and GICv3, although the way the affected VCPU is determined differs.
Since we need them to access the registers from three different places in the future, we factor out a generic, backend-facing implementation and use small wrappers in the current GICv2 emulation. This will ease adding GICv3 accessors later.

---
Changelog v4...v5:
(none)

Changelog v3...v4:
- improve commit message
- add Reviewed-by

 virt/kvm/arm/vgic.c |  126 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 74 insertions(+), 52 deletions(-)
virt/kvm/arm/vgic.c +74 -52
index dcdbae5..9b0cc49 100644
@@ -496,64 +496,66 @@ static bool handle_mmio_raz_wi(struct kvm_vcpu *vcpu,
 	return false;
 }
 
-static bool handle_mmio_set_enable_reg(struct kvm_vcpu *vcpu,
-				       struct kvm_exit_mmio *mmio,
-				       phys_addr_t offset)
+static bool vgic_handle_enable_reg(struct kvm *kvm, struct kvm_exit_mmio *mmio,
+				   phys_addr_t offset, int vcpu_id, int access)
 {
-	u32 *reg = vgic_bitmap_get_reg(&vcpu->kvm->arch.vgic.irq_enabled,
-				       vcpu->vcpu_id, offset);
-	vgic_reg_access(mmio, reg, offset,
-			ACCESS_READ_VALUE | ACCESS_WRITE_SETBIT);
+	u32 *reg;
+	int mode = ACCESS_READ_VALUE | access;
+	struct kvm_vcpu *target_vcpu = kvm_get_vcpu(kvm, vcpu_id);
+
+	reg = vgic_bitmap_get_reg(&kvm->arch.vgic.irq_enabled, vcpu_id, offset);
+	vgic_reg_access(mmio, reg, offset, mode);
 	if (mmio->is_write) {
-		vgic_update_state(vcpu->kvm);
+		if (access & ACCESS_WRITE_CLEARBIT) {
+			if (offset < 4) /* Force SGI enabled */
+				*reg |= 0xffff;
+			vgic_retire_disabled_irqs(target_vcpu);
+		}
+		vgic_update_state(kvm);
 		return true;
 	}
 
 	return false;
 }
 
+static bool handle_mmio_set_enable_reg(struct kvm_vcpu *vcpu,
+				       struct kvm_exit_mmio *mmio,
+				       phys_addr_t offset)
+{
+	return vgic_handle_enable_reg(vcpu->kvm, mmio, offset,
+				      vcpu->vcpu_id, ACCESS_WRITE_SETBIT);
+}
+
 static bool handle_mmio_clear_enable_reg(struct kvm_vcpu *vcpu,
 					 struct kvm_exit_mmio *mmio,
 					 phys_addr_t offset)
 {
-	u32 *reg = vgic_bitmap_get_reg(&vcpu->kvm->arch.vgic.irq_enabled,
-				       vcpu->vcpu_id, offset);
-	vgic_reg_access(mmio, reg, offset,
-			ACCESS_READ_VALUE | ACCESS_WRITE_CLEARBIT);
-	if (mmio->is_write) {
-		if (offset < 4) /* Force SGI enabled */
-			*reg |= 0xffff;
-		vgic_retire_disabled_irqs(vcpu);
-		vgic_update_state(vcpu->kvm);
-		return true;
-	}
-
-	return false;
+	return vgic_handle_enable_reg(vcpu->kvm, mmio, offset,
+				      vcpu->vcpu_id, ACCESS_WRITE_CLEARBIT);
 }
 
-static bool handle_mmio_set_pending_reg(struct kvm_vcpu *vcpu,
+static bool vgic_handle_set_pending_reg(struct kvm *kvm,
 					struct kvm_exit_mmio *mmio,
-					phys_addr_t offset)
+					phys_addr_t offset, int vcpu_id)
 {
 	u32 *reg, orig;
 	u32 level_mask;
-	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+	int mode = ACCESS_READ_VALUE | ACCESS_WRITE_SETBIT;
+	struct vgic_dist *dist = &kvm->arch.vgic;
 
-	reg = vgic_bitmap_get_reg(&dist->irq_cfg, vcpu->vcpu_id, offset);
+	reg = vgic_bitmap_get_reg(&dist->irq_cfg, vcpu_id, offset);
 	level_mask = (~(*reg));
 
 	/* Mark both level and edge triggered irqs as pending */
-	reg = vgic_bitmap_get_reg(&dist->irq_pending, vcpu->vcpu_id, offset);
+	reg = vgic_bitmap_get_reg(&dist->irq_pending, vcpu_id, offset);
 	orig = *reg;
-	vgic_reg_access(mmio, reg, offset,
-			ACCESS_READ_VALUE | ACCESS_WRITE_SETBIT);
+	vgic_reg_access(mmio, reg, offset, mode);
 
 	if (mmio->is_write) {
 		/* Set the soft-pending flag only for level-triggered irqs */
 		reg = vgic_bitmap_get_reg(&dist->irq_soft_pend,
-					  vcpu->vcpu_id, offset);
-		vgic_reg_access(mmio, reg, offset,
-				ACCESS_READ_VALUE | ACCESS_WRITE_SETBIT);
+					  vcpu_id, offset);
+		vgic_reg_access(mmio, reg, offset, mode);
 		*reg &= level_mask;
 
 		/* Ignore writes to SGIs */
@@ -562,31 +564,30 @@ static bool handle_mmio_set_pending_reg(struct kvm_vcpu *vcpu,
 			*reg |= orig & 0xffff;
 		}
 
-		vgic_update_state(vcpu->kvm);
+		vgic_update_state(kvm);
 		return true;
 	}
 
 	return false;
 }
 
-static bool handle_mmio_clear_pending_reg(struct kvm_vcpu *vcpu,
+static bool vgic_handle_clear_pending_reg(struct kvm *kvm,
 					  struct kvm_exit_mmio *mmio,
-					  phys_addr_t offset)
+					  phys_addr_t offset, int vcpu_id)
 {
 	u32 *level_active;
 	u32 *reg, orig;
-	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+	int mode = ACCESS_READ_VALUE | ACCESS_WRITE_CLEARBIT;
+	struct vgic_dist *dist = &kvm->arch.vgic;
 
-	reg = vgic_bitmap_get_reg(&dist->irq_pending, vcpu->vcpu_id, offset);
+	reg = vgic_bitmap_get_reg(&dist->irq_pending, vcpu_id, offset);
 	orig = *reg;
-	vgic_reg_access(mmio, reg, offset,
-			ACCESS_READ_VALUE | ACCESS_WRITE_CLEARBIT);
+	vgic_reg_access(mmio, reg, offset, mode);
 	if (mmio->is_write) {
 		/* Re-set level triggered level-active interrupts */
 		level_active = vgic_bitmap_get_reg(&dist->irq_level,
-					  vcpu->vcpu_id, offset);
-		reg = vgic_bitmap_get_reg(&dist->irq_pending,
-					  vcpu->vcpu_id, offset);
+					  vcpu_id, offset);
+		reg = vgic_bitmap_get_reg(&dist->irq_pending, vcpu_id, offset);
 		*reg |= *level_active;
 
 		/* Ignore writes to SGIs */
@@ -597,17 +598,31 @@ static bool handle_mmio_clear_pending_reg(struct kvm_vcpu *vcpu,
 
 		/* Clear soft-pending flags */
 		reg = vgic_bitmap_get_reg(&dist->irq_soft_pend,
-					  vcpu->vcpu_id, offset);
-		vgic_reg_access(mmio, reg, offset,
-				ACCESS_READ_VALUE | ACCESS_WRITE_CLEARBIT);
+					  vcpu_id, offset);
+		vgic_reg_access(mmio, reg, offset, mode);
 
-		vgic_update_state(vcpu->kvm);
+		vgic_update_state(kvm);
 		return true;
 	}
-
 	return false;
 }
 
+static bool handle_mmio_set_pending_reg(struct kvm_vcpu *vcpu,
+					struct kvm_exit_mmio *mmio,
+					phys_addr_t offset)
+{
+	return vgic_handle_set_pending_reg(vcpu->kvm, mmio, offset,
+					   vcpu->vcpu_id);
+}
+
+static bool handle_mmio_clear_pending_reg(struct kvm_vcpu *vcpu,
+					  struct kvm_exit_mmio *mmio,
+					  phys_addr_t offset)
+{
+	return vgic_handle_clear_pending_reg(vcpu->kvm, mmio, offset,
+					     vcpu->vcpu_id);
+}
+
 static bool handle_mmio_priority_reg(struct kvm_vcpu *vcpu,
 				     struct kvm_exit_mmio *mmio,
 				     phys_addr_t offset)
@@ -730,14 +745,10 @@ static u16 vgic_cfg_compress(u32 val)
  * LSB is always 0. As such, we only keep the upper bit, and use the
  * two above functions to compress/expand the bits
  */
-static bool handle_mmio_cfg_reg(struct kvm_vcpu *vcpu,
-				struct kvm_exit_mmio *mmio, phys_addr_t offset)
+static bool vgic_handle_cfg_reg(u32 *reg, struct kvm_exit_mmio *mmio,
+				phys_addr_t offset)
 {
 	u32 val;
-	u32 *reg;
-
-	reg = vgic_bitmap_get_reg(&vcpu->kvm->arch.vgic.irq_cfg,
-				  vcpu->vcpu_id, offset >> 1);
 
 	if (offset & 4)
 		val = *reg >> 16;
@@ -766,6 +777,17 @@ static bool handle_mmio_cfg_reg(struct kvm_vcpu *vcpu,
 	return false;
 }
 
+static bool handle_mmio_cfg_reg(struct kvm_vcpu *vcpu,
+				struct kvm_exit_mmio *mmio, phys_addr_t offset)
+{
+	u32 *reg;
+
+	reg = vgic_bitmap_get_reg(&vcpu->kvm->arch.vgic.irq_cfg,
+				  vcpu->vcpu_id, offset >> 1);
+
+	return vgic_handle_cfg_reg(reg, mmio, offset);
+}
+
 static bool handle_mmio_sgi_reg(struct kvm_vcpu *vcpu,
 				struct kvm_exit_mmio *mmio, phys_addr_t offset)
 {
Signature · 1 line
1.7.9.5
#28 lore raw

PATCH v5 11/19 arm/arm64: KVM: refactor/wrap vgic_set/get_attr()

In reply to Andre Przywara

vgic_set_attr() and vgic_get_attr() contain both code specific for the emulated GIC as well as code for the userland facing, generic part of the GIC.
Split the guest GIC facing code of from the generic part to allow easier splitting later.

---
Changelog v4...v5:
(add Reviewed-by:)

Changelog v3...v4:
- (none)

 virt/kvm/arm/vgic.c |   78 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 54 insertions(+), 24 deletions(-)
virt/kvm/arm/vgic.c +54 -24
index 9b0cc49..004892c 100644
@@ -2447,7 +2447,8 @@ out:
 	return ret;
 }
 
-static int vgic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
+static int vgic_set_common_attr(struct kvm_device *dev,
+				struct kvm_device_attr *attr)
 {
 	int r;
 
@@ -2463,17 +2464,6 @@ static int vgic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 		r = kvm_vgic_addr(dev->kvm, type, &addr, true);
 		return (r == -ENODEV) ? -ENXIO : r;
 	}
-
-	case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
-	case KVM_DEV_ARM_VGIC_GRP_CPU_REGS: {
-		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
-		u32 reg;
-
-		if (get_user(reg, uaddr))
-			return -EFAULT;
-
-		return vgic_attr_regs_access(dev, attr, &reg, true);
-	}
 	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS: {
 		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
 		u32 val;
@@ -2510,7 +2500,33 @@ static int vgic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 	return -ENXIO;
 }
 
-static int vgic_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
+static int vgic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
+{
+	int ret;
+
+	ret = vgic_set_common_attr(dev, attr);
+	if (ret != -ENXIO)
+		return ret;
+
+	switch (attr->group) {
+	case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
+	case KVM_DEV_ARM_VGIC_GRP_CPU_REGS: {
+		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
+		u32 reg;
+
+		if (get_user(reg, uaddr))
+			return -EFAULT;
+
+		return vgic_attr_regs_access(dev, attr, &reg, true);
+	}
+
+	}
+
+	return -ENXIO;
+}
+
+static int vgic_get_common_attr(struct kvm_device *dev,
+				struct kvm_device_attr *attr)
 {
 	int r = -ENXIO;
 
@@ -2528,27 +2544,41 @@ static int vgic_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 			return -EFAULT;
 		break;
 	}
+	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS: {
+		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
+
+		r = put_user(dev->kvm->arch.vgic.nr_irqs, uaddr);
+		break;
+	}
+
+	}
+
+	return r;
+}
+
+static int vgic_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
+{
+	int ret;
+
+	ret = vgic_get_common_attr(dev, attr);
+	if (ret != -ENXIO)
+		return ret;
 
+	switch (attr->group) {
 	case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
 	case KVM_DEV_ARM_VGIC_GRP_CPU_REGS: {
 		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
 		u32 reg = 0;
 
-		r = vgic_attr_regs_access(dev, attr, &reg, false);
-		if (r)
-			return r;
-		r = put_user(reg, uaddr);
-		break;
-	}
-	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS: {
-		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
-		r = put_user(dev->kvm->arch.vgic.nr_irqs, uaddr);
-		break;
+		ret = vgic_attr_regs_access(dev, attr, &reg, false);
+		if (ret)
+			return ret;
+		return put_user(reg, uaddr);
 	}
 
 	}
 
-	return r;
+	return -ENXIO;
 }
 
 static int vgic_has_attr_regs(const struct mmio_range *ranges,
Signature · 1 line
1.7.9.5
#29 lore raw

PATCH v5 12/19 arm/arm64: KVM: add vgic.h header file

In reply to Andre Przywara

vgic.c is currently a mixture of generic vGIC emulation code and functions specific to emulating a GICv2. To ease the addition of GICv3 later, we create new header file vgic.h, which holds constants and prototypes of commonly used functions.
Rename some identifiers to avoid name space clutter.
I removed the long-standing comment about using the kvm_io_bus API to tackle the GIC register ranges, as it wouldn't be a win for us anymore.

-------
As the diff isn't always obvious here (and to aid eventual rebases), here is a list of high-level changes done to the code:
* moved definitions and prototypes from vgic.c to vgic.h:
- VGIC_ADDR_UNDEF
- ACCESS_{READ,WRITE}_*
- vgic_update_state()
- vgic_kick_vcpus()
- vgic_get_vmcr()
- vgic_set_vmcr()
- struct mmio_range {} (renamed to struct kvm_mmio_range)
* removed static keyword and exported prototype in vgic.h:
- vgic_bitmap_get_reg()
- vgic_bitmap_set_irq_val()
- vgic_bitmap_get_shared_map()
- vgic_bytemap_get_reg()
- vgic_dist_irq_set()
- vgic_dist_irq_clear()
- vgic_cpu_irq_clear()
- vgic_reg_access()
- handle_mmio_raz_wi()
- vgic_handle_enable_reg()
- vgic_handle_pending_reg()
- vgic_handle_cfg_reg()
- vgic_unqueue_irqs()
- find_matching_range() (renamed to vgic_find_range)
- vgic_handle_mmio_range()
- vgic_update_state()
- vgic_get_vmcr()
- vgic_set_vmcr()
- vgic_queue_irq()
- vgic_kick_vcpus()
- vgic_init_common_maps()
- vgic_has_attr_regs()
- vgic_set_common_attr()
- vgic_get_common_attr()
* moved functions to vgic.h (static inline):
- mmio_data_read()
- mmio_data_write()
- is_in_range()
---
Changelog v4...v5:
- add vgic_init_common_maps (from changes in previous patches) (add Acked-by:)

Changelog v3...v4:
- rename struct mmio_range to struct kvm_mmio_range
- rename find_matching_range() to vgic_find_range()
- remove vgic_create() and vgic_destroy() from header

 virt/kvm/arm/vgic.c |  150 +++++++++++++++++----------------------------------
 virt/kvm/arm/vgic.h |  119 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 169 insertions(+), 100 deletions(-)
 create mode 100644 virt/kvm/arm/vgic.h
virt/kvm/arm/vgic.c +50 -100
index 004892c..4a17afd 100644
@@ -75,32 +75,16 @@
  *   inactive as long as the external input line is held high.
  */
 
-#define VGIC_ADDR_UNDEF		(-1)
-#define IS_VGIC_ADDR_UNDEF(_x)  ((_x) == VGIC_ADDR_UNDEF)
+#include "vgic.h"
 
-#define PRODUCT_ID_KVM		0x4b	/* ASCII code K */
-#define IMPLEMENTER_ARM		0x43b
 #define GICC_ARCH_VERSION_V2	0x2
 
-#define ACCESS_READ_VALUE	(1 << 0)
-#define ACCESS_READ_RAZ		(0 << 0)
-#define ACCESS_READ_MASK(x)	((x) & (1 << 0))
-#define ACCESS_WRITE_IGNORED	(0 << 1)
-#define ACCESS_WRITE_SETBIT	(1 << 1)
-#define ACCESS_WRITE_CLEARBIT	(2 << 1)
-#define ACCESS_WRITE_VALUE	(3 << 1)
-#define ACCESS_WRITE_MASK(x)	((x) & (3 << 1))
-
 static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
 static void vgic_retire_lr(int lr_nr, int irq, struct kvm_vcpu *vcpu);
-static void vgic_update_state(struct kvm *kvm);
-static void vgic_kick_vcpus(struct kvm *kvm);
 static u8 *vgic_get_sgi_sources(struct vgic_dist *dist, int vcpu_id, int sgi);
 static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg);
 static struct vgic_lr vgic_get_lr(const struct kvm_vcpu *vcpu, int lr);
 static void vgic_set_lr(struct kvm_vcpu *vcpu, int lr, struct vgic_lr lr_desc);
-static void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
-static void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
 
 static const struct vgic_ops *vgic_ops;
 static const struct vgic_params *vgic;
@@ -179,8 +163,7 @@ static unsigned long *u64_to_bitmask(u64 *val)
 	return (unsigned long *)val;
 }
 
-static u32 *vgic_bitmap_get_reg(struct vgic_bitmap *x,
-				int cpuid, u32 offset)
+u32 *vgic_bitmap_get_reg(struct vgic_bitmap *x, int cpuid, u32 offset)
 {
 	offset >>= 2;
 	if (!offset)
@@ -198,8 +181,8 @@ static int vgic_bitmap_get_irq_val(struct vgic_bitmap *x,
 	return test_bit(irq - VGIC_NR_PRIVATE_IRQS, x->shared);
 }
 
-static void vgic_bitmap_set_irq_val(struct vgic_bitmap *x, int cpuid,
-				    int irq, int val)
+void vgic_bitmap_set_irq_val(struct vgic_bitmap *x, int cpuid,
+			     int irq, int val)
 {
 	unsigned long *reg;
 
@@ -221,7 +204,7 @@ static unsigned long *vgic_bitmap_get_cpu_map(struct vgic_bitmap *x, int cpuid)
 	return x->private + cpuid;
 }
 
-static unsigned long *vgic_bitmap_get_shared_map(struct vgic_bitmap *x)
+unsigned long *vgic_bitmap_get_shared_map(struct vgic_bitmap *x)
 {
 	return x->shared;
 }
@@ -248,7 +231,7 @@ static void vgic_free_bytemap(struct vgic_bytemap *b)
 	b->shared = NULL;
 }
 
-static u32 *vgic_bytemap_get_reg(struct vgic_bytemap *x, int cpuid, u32 offset)
+u32 *vgic_bytemap_get_reg(struct vgic_bytemap *x, int cpuid, u32 offset)
 {
 	u32 *reg;
 
@@ -345,14 +328,14 @@ static int vgic_dist_irq_is_pending(struct kvm_vcpu *vcpu, int irq)
 	return vgic_bitmap_get_irq_val(&dist->irq_pending, vcpu->vcpu_id, irq);
 }
 
-static void vgic_dist_irq_set_pending(struct kvm_vcpu *vcpu, int irq)
+void vgic_dist_irq_set_pending(struct kvm_vcpu *vcpu, int irq)
 {
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
 
 	vgic_bitmap_set_irq_val(&dist->irq_pending, vcpu->vcpu_id, irq, 1);
 }
 
-static void vgic_dist_irq_clear_pending(struct kvm_vcpu *vcpu, int irq)
+void vgic_dist_irq_clear_pending(struct kvm_vcpu *vcpu, int irq)
 {
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
 
@@ -368,7 +351,7 @@ static void vgic_cpu_irq_set(struct kvm_vcpu *vcpu, int irq)
 			vcpu->arch.vgic_cpu.pending_shared);
 }
 
-static void vgic_cpu_irq_clear(struct kvm_vcpu *vcpu, int irq)
+void vgic_cpu_irq_clear(struct kvm_vcpu *vcpu, int irq)
 {
 	if (irq < VGIC_NR_PRIVATE_IRQS)
 		clear_bit(irq, vcpu->arch.vgic_cpu.pending_percpu);
@@ -382,16 +365,6 @@ static bool vgic_can_sample_irq(struct kvm_vcpu *vcpu, int irq)
 	return vgic_irq_is_edge(vcpu, irq) || !vgic_irq_is_queued(vcpu, irq);
 }
 
-static u32 mmio_data_read(struct kvm_exit_mmio *mmio, u32 mask)
-{
-	return le32_to_cpu(*((u32 *)mmio->data)) & mask;
-}
-
-static void mmio_data_write(struct kvm_exit_mmio *mmio, u32 mask, u32 value)
-{
-	*((u32 *)mmio->data) = cpu_to_le32(value) & mask;
-}
-
 /**
  * vgic_reg_access - access vgic register
  * @mmio:   pointer to the data describing the mmio access
@@ -403,8 +376,8 @@ static void mmio_data_write(struct kvm_exit_mmio *mmio, u32 mask, u32 value)
  * modes defined for vgic register access
  * (read,raz,write-ignored,setbit,clearbit,write)
  */
-static void vgic_reg_access(struct kvm_exit_mmio *mmio, u32 *reg,
-			    phys_addr_t offset, int mode)
+void vgic_reg_access(struct kvm_exit_mmio *mmio, u32 *reg,
+		     phys_addr_t offset, int mode)
 {
 	int word_offset = (offset & 3) * 8;
 	u32 mask = (1UL << (mmio->len * 8)) - 1;
@@ -488,16 +461,16 @@ static bool handle_mmio_misc(struct kvm_vcpu *vcpu,
 	return false;
 }
 
-static bool handle_mmio_raz_wi(struct kvm_vcpu *vcpu,
-			       struct kvm_exit_mmio *mmio, phys_addr_t offset)
+bool handle_mmio_raz_wi(struct kvm_vcpu *vcpu, struct kvm_exit_mmio *mmio,
+			phys_addr_t offset)
 {
 	vgic_reg_access(mmio, NULL, offset,
 			ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
 	return false;
 }
 
-static bool vgic_handle_enable_reg(struct kvm *kvm, struct kvm_exit_mmio *mmio,
-				   phys_addr_t offset, int vcpu_id, int access)
+bool vgic_handle_enable_reg(struct kvm *kvm, struct kvm_exit_mmio *mmio,
+			    phys_addr_t offset, int vcpu_id, int access)
 {
 	u32 *reg;
 	int mode = ACCESS_READ_VALUE | access;
@@ -534,9 +507,9 @@ static bool handle_mmio_clear_enable_reg(struct kvm_vcpu *vcpu,
 				      vcpu->vcpu_id, ACCESS_WRITE_CLEARBIT);
 }
 
-static bool vgic_handle_set_pending_reg(struct kvm *kvm,
-					struct kvm_exit_mmio *mmio,
-					phys_addr_t offset, int vcpu_id)
+bool vgic_handle_set_pending_reg(struct kvm *kvm,
+				 struct kvm_exit_mmio *mmio,
+				 phys_addr_t offset, int vcpu_id)
 {
 	u32 *reg, orig;
 	u32 level_mask;
@@ -571,9 +544,9 @@ static bool vgic_handle_set_pending_reg(struct kvm *kvm,
 	return false;
 }
 
-static bool vgic_handle_clear_pending_reg(struct kvm *kvm,
-					  struct kvm_exit_mmio *mmio,
-					  phys_addr_t offset, int vcpu_id)
+bool vgic_handle_clear_pending_reg(struct kvm *kvm,
+				   struct kvm_exit_mmio *mmio,
+				   phys_addr_t offset, int vcpu_id)
 {
 	u32 *level_active;
 	u32 *reg, orig;
@@ -745,8 +718,8 @@ static u16 vgic_cfg_compress(u32 val)
  * LSB is always 0. As such, we only keep the upper bit, and use the
  * two above functions to compress/expand the bits
  */
-static bool vgic_handle_cfg_reg(u32 *reg, struct kvm_exit_mmio *mmio,
-				phys_addr_t offset)
+bool vgic_handle_cfg_reg(u32 *reg, struct kvm_exit_mmio *mmio,
+			 phys_addr_t offset)
 {
 	u32 val;
 
@@ -822,7 +795,7 @@ static void vgic_v2_add_sgi_source(struct kvm_vcpu *vcpu, int irq, int source)
  * to the distributor but the active state stays in the LRs, because we don't
  * track the active state on the distributor side.
  */
-static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
+void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	int i;
@@ -947,21 +920,7 @@ static bool handle_mmio_sgi_clear(struct kvm_vcpu *vcpu,
 		return write_set_clear_sgi_pend_reg(vcpu, mmio, offset, false);
 }
 
-/*
- * I would have liked to use the kvm_bus_io_*() API instead, but it
- * cannot cope with banked registers (only the VM pointer is passed
- * around, and we need the vcpu). One of these days, someone please
- * fix it!
- */
-struct mmio_range {
-	phys_addr_t base;
-	unsigned long len;
-	int bits_per_irq;
-	bool (*handle_mmio)(struct kvm_vcpu *vcpu, struct kvm_exit_mmio *mmio,
-			    phys_addr_t offset);
-};
-
-static const struct mmio_range vgic_dist_ranges[] = {
+static const struct kvm_mmio_range vgic_dist_ranges[] = {
 	{
 		.base		= GIC_DIST_CTRL,
 		.len		= 12,
@@ -1046,12 +1005,12 @@ static const struct mmio_range vgic_dist_ranges[] = {
 	{}
 };
 
-static const
-struct mmio_range *find_matching_range(const struct mmio_range *ranges,
+const
+struct kvm_mmio_range *vgic_find_range(const struct kvm_mmio_range *ranges,
 				       struct kvm_exit_mmio *mmio,
 				       phys_addr_t offset)
 {
-	const struct mmio_range *r = ranges;
+	const struct kvm_mmio_range *r = ranges;
 
 	while (r->len) {
 		if (offset >= r->base &&
@@ -1064,7 +1023,7 @@ struct mmio_range *find_matching_range(const struct mmio_range *ranges,
 }
 
 static bool vgic_validate_access(const struct vgic_dist *dist,
-				 const struct mmio_range *range,
+				 const struct kvm_mmio_range *range,
 				 unsigned long offset)
 {
 	int irq;
@@ -1092,7 +1051,7 @@ static bool vgic_validate_access(const struct vgic_dist *dist,
 static bool call_range_handler(struct kvm_vcpu *vcpu,
 			       struct kvm_exit_mmio *mmio,
 			       unsigned long offset,
-			       const struct mmio_range *range)
+			       const struct kvm_mmio_range *range)
 {
 	u32 *data32 = (void *)mmio->data;
 	struct kvm_exit_mmio mmio32;
@@ -1136,18 +1095,18 @@ static bool call_range_handler(struct kvm_vcpu *vcpu,
  *
  * returns true if the MMIO access could be performed
  */
-static bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
+bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
 			    struct kvm_exit_mmio *mmio,
-			    const struct mmio_range *ranges,
+			    const struct kvm_mmio_range *ranges,
 			    unsigned long mmio_base)
 {
-	const struct mmio_range *range;
+	const struct kvm_mmio_range *range;
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
 	bool updated_state;
 	unsigned long offset;
 
 	offset = mmio->phys_addr - mmio_base;
-	range = find_matching_range(ranges, mmio, offset);
+	range = vgic_find_range(ranges, mmio, offset);
 	if (unlikely(!range || !range->handle_mmio)) {
 		pr_warn("Unhandled access %d %08llx %d\n",
 			mmio->is_write, mmio->phys_addr, mmio->len);
@@ -1173,12 +1132,6 @@ static bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	return true;
 }
 
-static inline bool is_in_range(phys_addr_t addr, unsigned long len,
-			       phys_addr_t baseaddr, unsigned long size)
-{
-	return (addr >= baseaddr) && (addr + len <= baseaddr + size);
-}
-
 static bool vgic_v2_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 				struct kvm_exit_mmio *mmio)
 {
@@ -1305,7 +1258,7 @@ static int compute_pending_for_cpu(struct kvm_vcpu *vcpu)
  * Update the interrupt state and determine which CPUs have pending
  * interrupts. Must be called with distributor lock held.
  */
-static void vgic_update_state(struct kvm *kvm)
+void vgic_update_state(struct kvm *kvm)
 {
 	struct vgic_dist *dist = &kvm->arch.vgic;
 	struct kvm_vcpu *vcpu;
@@ -1366,12 +1319,12 @@ static inline void vgic_disable_underflow(struct kvm_vcpu *vcpu)
 	vgic_ops->disable_underflow(vcpu);
 }
 
-static inline void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr)
+void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr)
 {
 	vgic_ops->get_vmcr(vcpu, vmcr);
 }
 
-static void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr)
+void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr)
 {
 	vgic_ops->set_vmcr(vcpu, vmcr);
 }
@@ -1421,7 +1374,7 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)
  * Queue an interrupt to a CPU virtual interface. Return true on success,
  * or false if it wasn't possible to queue it.
  */
-static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
+bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
@@ -1707,7 +1660,7 @@ int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu)
 	return test_bit(vcpu->vcpu_id, dist->irq_pending_on_cpu);
 }
 
-static void vgic_kick_vcpus(struct kvm *kvm)
+void vgic_kick_vcpus(struct kvm *kvm)
 {
 	struct kvm_vcpu *vcpu;
 	int c;
@@ -1948,7 +1901,7 @@ void kvm_vgic_destroy(struct kvm *kvm)
  * Allocate and initialize the various data structures. Must be called
  * with kvm->lock held!
  */
-static int vgic_init_common_maps(struct kvm *kvm)
+int vgic_init_common_maps(struct kvm *kvm)
 {
 	struct vgic_dist *dist = &kvm->arch.vgic;
 	struct kvm_vcpu *vcpu;
@@ -2092,7 +2045,7 @@ out:
 	return ret;
 }
 
-static int vgic_v2_init_emulation(struct kvm *kvm)
+int vgic_v2_init_emulation(struct kvm *kvm)
 {
 	struct vgic_dist *dist = &kvm->arch.vgic;
 
@@ -2333,7 +2286,7 @@ static bool handle_cpu_mmio_ident(struct kvm_vcpu *vcpu,
  * CPU Interface Register accesses - these are not accessed by the VM, but by
  * user space for saving and restoring VGIC state.
  */
-static const struct mmio_range vgic_cpu_ranges[] = {
+static const struct kvm_mmio_range vgic_cpu_ranges[] = {
 	{
 		.base		= GIC_CPU_CTRL,
 		.len		= 12,
@@ -2360,7 +2313,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 				 struct kvm_device_attr *attr,
 				 u32 *reg, bool is_write)
 {
-	const struct mmio_range *r = NULL, *ranges;
+	const struct kvm_mmio_range *r = NULL, *ranges;
 	phys_addr_t offset;
 	int ret, cpuid, c;
 	struct kvm_vcpu *vcpu, *tmp_vcpu;
@@ -2401,7 +2354,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 	default:
 		BUG();
 	}
-	r = find_matching_range(ranges, &mmio, offset);
+	r = vgic_find_range(ranges, &mmio, offset);
 
 	if (unlikely(!r || !r->handle_mmio)) {
 		ret = -ENXIO;
@@ -2447,8 +2400,7 @@ out:
 	return ret;
 }
 
-static int vgic_set_common_attr(struct kvm_device *dev,
-				struct kvm_device_attr *attr)
+int vgic_set_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 {
 	int r;
 
@@ -2525,8 +2477,7 @@ static int vgic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 	return -ENXIO;
 }
 
-static int vgic_get_common_attr(struct kvm_device *dev,
-				struct kvm_device_attr *attr)
+int vgic_get_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 {
 	int r = -ENXIO;
 
@@ -2581,13 +2532,12 @@ static int vgic_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 	return -ENXIO;
 }
 
-static int vgic_has_attr_regs(const struct mmio_range *ranges,
-			      phys_addr_t offset)
+int vgic_has_attr_regs(const struct kvm_mmio_range *ranges, phys_addr_t offset)
 {
 	struct kvm_exit_mmio dev_attr_mmio;
 
 	dev_attr_mmio.len = 4;
-	if (find_matching_range(ranges, &dev_attr_mmio, offset))
+	if (vgic_find_range(ranges, &dev_attr_mmio, offset))
 		return 0;
 	else
 		return -ENXIO;
@@ -2617,12 +2567,12 @@ static int vgic_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 	return -ENXIO;
 }
 
-static void vgic_destroy(struct kvm_device *dev)
+void vgic_destroy(struct kvm_device *dev)
 {
 	kfree(dev);
 }
 
-static int vgic_create(struct kvm_device *dev, u32 type)
+int vgic_create(struct kvm_device *dev, u32 type)
 {
 	return kvm_vgic_create(dev->kvm, type);
 }
virt/kvm/arm/vgic.h new file +119 -0

119 lines not on this page · raw message

Signature · 1 line
1.7.9.5
#30 lore raw

PATCH v5 13/19 arm/arm64: KVM: split GICv2 specific emulation code from vgic.c

In reply to Andre Przywara

vgic.c is currently a mixture of generic vGIC emulation code and functions specific to emulating a GICv2. To ease the addition of GICv3, split off strictly v2 specific parts into a new file vgic-v2-emul.c.

-------
As the diff isn't always obvious here (and to aid eventual rebases), here is a list of high-level changes done to the code:
* added new file to respective arm/arm64 Makefiles
* moved GICv2 specific functions to vgic-v2-emul.c:
- handle_mmio_misc()
- handle_mmio_set_enable_reg()
- handle_mmio_clear_enable_reg()
- handle_mmio_set_pending_reg()
- handle_mmio_clear_pending_reg()
- handle_mmio_priority_reg()
- vgic_get_target_reg()
- vgic_set_target_reg()
- handle_mmio_target_reg()
- handle_mmio_cfg_reg()
- handle_mmio_sgi_reg()
- vgic_v2_unqueue_sgi()
- read_set_clear_sgi_pend_reg()
- write_set_clear_sgi_pend_reg()
- handle_mmio_sgi_set()
- handle_mmio_sgi_clear()
- vgic_v2_handle_mmio()
- vgic_get_sgi_sources()
- vgic_dispatch_sgi()
- vgic_v2_queue_sgi()
- vgic_v2_init()
- vgic_v2_init_emulation()
- handle_cpu_mmio_misc()
- handle_mmio_abpr()
- handle_cpu_mmio_ident()
- vgic_attr_regs_access()
- vgic_create() (renamed to vgic_v2_create())
- vgic_destroy() (renamed to vgic_v2_destroy())
- vgic_has_attr() (renamed to vgic_v2_has_attr())
- vgic_set_attr() (renamed to vgic_v2_set_attr())
- vgic_get_attr() (renamed to vgic_v2_get_attr())
- struct kvm_mmio_range vgic_dist_ranges[]
- struct kvm_mmio_range vgic_cpu_ranges[]
- struct kvm_device_ops kvm_arm_vgic_v2_ops {}
---
Changelog v4...v5:
(move adapted to changes in previous patches)

Changelog v3...v4:
- (move adapted to previous patches)
- moving vgic_create() and vgic_destroy() also

 arch/arm/kvm/Makefile       |    1 +
 arch/arm64/kvm/Makefile     |    1 +
 virt/kvm/arm/vgic-v2-emul.c |  811 +++++++++++++++++++++++++++++++++++++++++++
 virt/kvm/arm/vgic.c         |  768 +---------------------------------------
 4 files changed, 814 insertions(+), 767 deletions(-)
 create mode 100644 virt/kvm/arm/vgic-v2-emul.c
arch/arm/kvm/Makefile +1 -0
index f7057ed..443b8be 100644
@@ -22,4 +22,5 @@ obj-y += arm.o handle_exit.o guest.o mmu.o emulate.o reset.o
 obj-y += coproc.o coproc_a15.o coproc_a7.o mmio.o psci.o perf.o
 obj-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic.o
 obj-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v2.o
+obj-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v2-emul.o
 obj-$(CONFIG_KVM_ARM_TIMER) += $(KVM)/arm/arch_timer.o
arch/arm64/kvm/Makefile +1 -0
index 32a0961..d957353 100644
@@ -21,6 +21,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += guest.o reset.o sys_regs.o sys_regs_generic_v8.o
 
 kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v2.o
+kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v2-emul.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += vgic-v2-switch.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v3.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += vgic-v3-switch.o
virt/kvm/arm/vgic-v2-emul.c new file +811 -0

811 lines not on this page · raw message

virt/kvm/arm/vgic.c +1 -767

864 lines not on this page · raw message

Signature · 1 line
1.7.9.5
#31 lore raw

PATCH v5 14/19 arm/arm64: KVM: add opaque private pointer to MMIO data

In reply to Andre Przywara

For a GICv2 there is always only one (v)CPU involved: the one that does the access. On a GICv3 the access to a CPU redistributor is memory-mapped, but not banked, so the (v)CPU affected is determined by looking at the MMIO address region being accessed.
To allow passing the affected CPU into the accessors later, extend struct kvm_exit_mmio to add an opaque private pointer parameter. The current GICv2 emulation just does not use it.

---
Changelog v4...v5:
(add Acked-by:)

Changelog v3...v4:
- moving private parameter from each accessor into struct kvm_exit_mmio

 arch/arm/include/asm/kvm_mmio.h   |    1 +
 arch/arm64/include/asm/kvm_mmio.h |    1 +
 virt/kvm/arm/vgic.c               |    1 +
 3 files changed, 3 insertions(+)
arch/arm/include/asm/kvm_mmio.h +1 -0
index adcc0d7..3f83db2 100644
@@ -37,6 +37,7 @@ struct kvm_exit_mmio {
 	u8		data[8];
 	u32		len;
 	bool		is_write;
+	void		*private;
 };
 
 static inline void kvm_prepare_mmio(struct kvm_run *run,
arch/arm64/include/asm/kvm_mmio.h +1 -0
index fc2f689..9f52beb 100644
@@ -40,6 +40,7 @@ struct kvm_exit_mmio {
 	u8		data[8];
 	u32		len;
 	bool		is_write;
+	void		*private;
 };
 
 static inline void kvm_prepare_mmio(struct kvm_run *run,
virt/kvm/arm/vgic.c +1 -0
index 910282e..57866ce 100644
@@ -716,6 +716,7 @@ static bool call_range_handler(struct kvm_vcpu *vcpu,
 
 	mmio32.len = 4;
 	mmio32.is_write = mmio->is_write;
+	mmio32.private = mmio->private;
 
 	mmio32.phys_addr = mmio->phys_addr + 4;
 	if (mmio->is_write)
Signature · 1 line
1.7.9.5
#32 lore raw

PATCH v5 15/19 arm/arm64: KVM: add virtual GICv3 distributor emulation

In reply to Andre Przywara

With everything separated and prepared, we implement a model of a GICv3 distributor and redistributors by using the existing framework to provide handler functions for each register group.

Currently we limit the emulation to a model enforcing a single security state, with SRE==1 (forcing system register access) and ARE==1 (allowing more than 8 VCPUs).

We share some of the functions provided for GICv2 emulation, but take the different ways of addressing (v)CPUs into account. Save and restore is currently not implemented.

Similar to the split-off of the GICv2 specific code, the new emulation code goes into a new file (vgic-v3-emul.c).

---
Changelog v4...v5:
- correct GICD_IROUTER region length
- adapt GICv3 specific memory allocation to new init_maps()
- minor styles changes and fixed comments

Changelog v3...v4:
- remove ICC_SGI1R_EL1 register handling (moved into later patch)
- add definitions for single security state
- document emulation limitations in vgic-v3-emul.c header
- move CTLR, TYPER and IIDR handling into separate functions
- add RAO/WI handling for IGROUPRn registers
- remove unneeded offset masking on calling vgic_reg_access()
- rework handle_mmio_route_reg() to only handle SPIs
- refine IROUTERn register range
- use non-atomic bitops functions (__clear_bit() and __set_bit())
- rename vgic_dist_ranges[] to vgic_v3_dist_ranges[]
- add (RAZ/WI) implementation of GICD_STATUSR
- add (RAZ/WI) implementations of MBI registers
- adapt to new private passing (in struct kvm_exit_mmio instead of a paramter)
- fix vcpu_id calculation bug in handle CFG registers
- always use hexadecimal numbers for .len member
- simplify vgic_v3_handle_mmio()
- add vgic_v3_create() and vgic_v3_destroy()
- swap vgic_v3_[sg]et_attr() code location
- add and improve comments
- (adaptions to changes from earlier patches)

 arch/arm64/kvm/Makefile            |    1 +
 include/kvm/arm_vgic.h             |    9 +-
 include/linux/irqchip/arm-gic-v3.h |   32 ++
 include/linux/kvm_host.h           |    1 +
 include/uapi/linux/kvm.h           |    2 +
 virt/kvm/arm/vgic-v3-emul.c        |  899 ++++++++++++++++++++++++++++++++++++
 virt/kvm/arm/vgic.c                |   11 +-
 virt/kvm/arm/vgic.h                |    3 +
 8 files changed, 955 insertions(+), 3 deletions(-)
 create mode 100644 virt/kvm/arm/vgic-v3-emul.c
arch/arm64/kvm/Makefile +1 -0
index d957353..4e6e09e 100644
@@ -24,5 +24,6 @@ kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v2.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v2-emul.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += vgic-v2-switch.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v3.o
+kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v3-emul.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += vgic-v3-switch.o
 kvm-$(CONFIG_KVM_ARM_TIMER) += $(KVM)/arm/arch_timer.o
include/kvm/arm_vgic.h +8 -1
index f7570f83..668d575 100644
@@ -162,7 +162,11 @@ struct vgic_dist {
 
 	/* Distributor and vcpu interface mapping in the guest */
 	phys_addr_t		vgic_dist_base;
-	phys_addr_t		vgic_cpu_base;
+	/* GICv2 and GICv3 use different mapped register blocks */
+	union {
+		phys_addr_t		vgic_cpu_base;
+		phys_addr_t		vgic_redist_base;
+	};
 
 	/* Distributor enabled */
 	u32			enabled;
@@ -224,6 +228,9 @@ struct vgic_dist {
 	 */
 	struct vgic_bitmap	*irq_spi_target;
 
+	/* Target MPIDR for each IRQ (needed for GICv3 IROUTERn) only */
+	u32			*irq_spi_mpidr;
+
 	/* Bitmap indicating which CPU has something pending */
 	unsigned long		*irq_pending_on_cpu;
 
include/linux/irqchip/arm-gic-v3.h +32 -0
index 03a4ea3..726d898 100644
@@ -33,6 +33,7 @@
 #define GICD_SETSPI_SR			0x0050
 #define GICD_CLRSPI_SR			0x0058
 #define GICD_SEIR			0x0068
+#define GICD_IGROUPR			0x0080
 #define GICD_ISENABLER			0x0100
 #define GICD_ICENABLER			0x0180
 #define GICD_ISPENDR			0x0200
@@ -41,14 +42,37 @@
 #define GICD_ICACTIVER			0x0380
 #define GICD_IPRIORITYR			0x0400
 #define GICD_ICFGR			0x0C00
+#define GICD_IGRPMODR			0x0D00
+#define GICD_NSACR			0x0E00
 #define GICD_IROUTER			0x6000
+#define GICD_IDREGS			0xFFD0
 #define GICD_PIDR2			0xFFE8
 
+/*
+ * Those registers are actually from GICv2, but the spec demands that they
+ * are implemented as RES0 if ARE is 1 (which we do in KVM's emulated GICv3).
+ */
+#define GICD_ITARGETSR			0x0800
+#define GICD_SGIR			0x0F00
+#define GICD_CPENDSGIR			0x0F10
+#define GICD_SPENDSGIR			0x0F20
+
 #define GICD_CTLR_RWP			(1U << 31)
+#define GICD_CTLR_DS			(1U << 6)
 #define GICD_CTLR_ARE_NS		(1U << 4)
 #define GICD_CTLR_ENABLE_G1A		(1U << 1)
 #define GICD_CTLR_ENABLE_G1		(1U << 0)
 
+/*
+ * In systems with a single security state (what we emulate in KVM)
+ * the meaning of the interrupt group enable bits is slightly different
+ */
+#define GICD_CTLR_ENABLE_SS_G1		(1U << 1)
+#define GICD_CTLR_ENABLE_SS_G0		(1U << 0)
+
+#define GICD_TYPER_LPIS			(1U << 17)
+#define GICD_TYPER_MBIS			(1U << 16)
+
 #define GICD_IROUTER_SPI_MODE_ONE	(0U << 31)
 #define GICD_IROUTER_SPI_MODE_ANY	(1U << 31)
 
@@ -56,6 +80,8 @@
 #define GIC_PIDR2_ARCH_GICv3		0x30
 #define GIC_PIDR2_ARCH_GICv4		0x40
 
+#define GIC_V3_DIST_SIZE		0x10000
+
 /*
  * Re-Distributor registers, offsets from RD_base
  */
@@ -74,6 +100,7 @@
 #define GICR_SYNCR			0x00C0
 #define GICR_MOVLPIR			0x0100
 #define GICR_MOVALLR			0x0110
+#define GICR_IDREGS			GICD_IDREGS
 #define GICR_PIDR2			GICD_PIDR2
 
 #define GICR_WAKER_ProcessorSleep	(1U << 1)
@@ -82,6 +109,7 @@
 /*
  * Re-Distributor registers, offsets from SGI_base
  */
+#define GICR_IGROUPR0			GICD_IGROUPR
 #define GICR_ISENABLER0			GICD_ISENABLER
 #define GICR_ICENABLER0			GICD_ICENABLER
 #define GICR_ISPENDR0			GICD_ISPENDR
@@ -90,10 +118,14 @@
 #define GICR_ICACTIVER0			GICD_ICACTIVER
 #define GICR_IPRIORITYR0		GICD_IPRIORITYR
 #define GICR_ICFGR0			GICD_ICFGR
+#define GICR_IGRPMODR0			GICD_IGRPMODR
+#define GICR_NSACR			GICD_NSACR
 
 #define GICR_TYPER_VLPIS		(1U << 1)
 #define GICR_TYPER_LAST			(1U << 4)
 
+#define GIC_V3_REDIST_SIZE		0x20000
+
 /*
  * CPU interface registers
  */
include/linux/kvm_host.h +1 -0
index e674693..b1aa88d 100644
@@ -1085,6 +1085,7 @@ void kvm_unregister_device_ops(u32 type);
 extern struct kvm_device_ops kvm_mpic_ops;
 extern struct kvm_device_ops kvm_xics_ops;
 extern struct kvm_device_ops kvm_arm_vgic_v2_ops;
+extern struct kvm_device_ops kvm_arm_vgic_v3_ops;
 
 #ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
 
include/uapi/linux/kvm.h +2 -0
index 6076882..24cb129 100644
@@ -960,6 +960,8 @@ enum kvm_device_type {
 #define KVM_DEV_TYPE_ARM_VGIC_V2	KVM_DEV_TYPE_ARM_VGIC_V2
 	KVM_DEV_TYPE_FLIC,
 #define KVM_DEV_TYPE_FLIC		KVM_DEV_TYPE_FLIC
+	KVM_DEV_TYPE_ARM_VGIC_V3,
+#define KVM_DEV_TYPE_ARM_VGIC_V3	KVM_DEV_TYPE_ARM_VGIC_V3
 	KVM_DEV_TYPE_MAX,
 };
 
virt/kvm/arm/vgic-v3-emul.c new file +899 -0

899 lines not on this page · raw message

virt/kvm/arm/vgic.c +9 -2
index 57866ce..d7d9258 100644
@@ -1256,7 +1256,7 @@ static bool vgic_update_irq_pending(struct kvm *kvm, int cpuid,
 	struct kvm_vcpu *vcpu;
 	int edge_triggered, level_triggered;
 	int enabled;
-	bool ret = true;
+	bool ret = true, can_inject = true;
 
 	spin_lock(&dist->lock);
 
@@ -1271,6 +1271,11 @@ static bool vgic_update_irq_pending(struct kvm *kvm, int cpuid,
 
 	if (irq_num >= VGIC_NR_PRIVATE_IRQS) {
 		cpuid = dist->irq_spi_cpu[irq_num - VGIC_NR_PRIVATE_IRQS];
+		if (cpuid == VCPU_NOT_ALLOCATED) {
+			/* Pretend we use CPU0, and prevent injection */
+			cpuid = 0;
+			can_inject = false;
+		}
 		vcpu = kvm_get_vcpu(kvm, cpuid);
 	}
 
@@ -1292,7 +1297,7 @@ static bool vgic_update_irq_pending(struct kvm *kvm, int cpuid,
 
 	enabled = vgic_irq_is_enabled(vcpu, irq_num);
 
-	if (!enabled) {
+	if (!enabled || !can_inject) {
 		ret = false;
 		goto out;
 	}
@@ -1445,6 +1450,7 @@ void kvm_vgic_destroy(struct kvm *kvm)
 	}
 	kfree(dist->irq_sgi_sources);
 	kfree(dist->irq_spi_cpu);
+	kfree(dist->irq_spi_mpidr);
 	kfree(dist->irq_spi_target);
 	kfree(dist->irq_pending_on_cpu);
 	dist->irq_sgi_sources = NULL;
@@ -1635,6 +1641,7 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
 	kvm->arch.vgic.vctrl_base = vgic->vctrl_base;
 	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
 	kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
+	kvm->arch.vgic.vgic_redist_base = VGIC_ADDR_UNDEF;
 
 out_unlock:
 	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
virt/kvm/arm/vgic.h +3 -0
index 08ba3d6..614559a 100644
@@ -35,6 +35,8 @@
 #define ACCESS_WRITE_VALUE	(3 << 1)
 #define ACCESS_WRITE_MASK(x)	((x) & (3 << 1))
 
+#define VCPU_NOT_ALLOCATED	((u8)-1)
+
 unsigned long *vgic_bitmap_get_shared_map(struct vgic_bitmap *x);
 
 void vgic_update_state(struct kvm *kvm);
@@ -115,5 +117,6 @@ int vgic_set_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr);
 int vgic_get_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr);
 
 int vgic_v2_init_emulation(struct kvm *kvm);
+int vgic_v3_init_emulation(struct kvm *kvm);
 
 #endif
Signature · 1 line
1.7.9.5
#33 lore raw

PATCH v5 15/19 arm/arm64: KVM: add virtual GICv3 distributor emulation

Quoted 449 lines · Andre Przywara

With everything separated and prepared, we implement a model of a GICv3 distributor and redistributors by using the existing framework to provide handler functions for each register group.

Currently we limit the emulation to a model enforcing a single security state, with SRE==1 (forcing system register access) and ARE==1 (allowing more than 8 VCPUs).

We share some of the functions provided for GICv2 emulation, but take the different ways of addressing (v)CPUs into account. Save and restore is currently not implemented.

Similar to the split-off of the GICv2 specific code, the new emulation code goes into a new file (vgic-v3-emul.c).

---
Changelog v4...v5:
- correct GICD_IROUTER region length
- adapt GICv3 specific memory allocation to new init_maps()
- minor styles changes and fixed comments

Changelog v3...v4:
- remove ICC_SGI1R_EL1 register handling (moved into later patch)
- add definitions for single security state
- document emulation limitations in vgic-v3-emul.c header
- move CTLR, TYPER and IIDR handling into separate functions
- add RAO/WI handling for IGROUPRn registers
- remove unneeded offset masking on calling vgic_reg_access()
- rework handle_mmio_route_reg() to only handle SPIs
- refine IROUTERn register range
- use non-atomic bitops functions (__clear_bit() and __set_bit())
- rename vgic_dist_ranges[] to vgic_v3_dist_ranges[]
- add (RAZ/WI) implementation of GICD_STATUSR
- add (RAZ/WI) implementations of MBI registers
- adapt to new private passing (in struct kvm_exit_mmio instead of a paramter)
- fix vcpu_id calculation bug in handle CFG registers
- always use hexadecimal numbers for .len member
- simplify vgic_v3_handle_mmio()
- add vgic_v3_create() and vgic_v3_destroy()
- swap vgic_v3_[sg]et_attr() code location
- add and improve comments
- (adaptions to changes from earlier patches)

 arch/arm64/kvm/Makefile            |    1 +
 include/kvm/arm_vgic.h             |    9 +-
 include/linux/irqchip/arm-gic-v3.h |   32 ++
 include/linux/kvm_host.h           |    1 +
 include/uapi/linux/kvm.h           |    2 +
 virt/kvm/arm/vgic-v3-emul.c        |  899 ++++++++++++++++++++++++++++++++++++
 virt/kvm/arm/vgic.c                |   11 +-
 virt/kvm/arm/vgic.h                |    3 +
 8 files changed, 955 insertions(+), 3 deletions(-)
 create mode 100644 virt/kvm/arm/vgic-v3-emul.c
arch/arm64/kvm/Makefile +1 -0
index d957353..4e6e09e 100644
@@ -24,5 +24,6 @@ kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v2.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v2-emul.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += vgic-v2-switch.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v3.o
+kvm-$(CONFIG_KVM_ARM_VGIC) += $(KVM)/arm/vgic-v3-emul.o
 kvm-$(CONFIG_KVM_ARM_VGIC) += vgic-v3-switch.o
 kvm-$(CONFIG_KVM_ARM_TIMER) += $(KVM)/arm/arch_timer.o
include/kvm/arm_vgic.h +8 -1
index f7570f83..668d575 100644
@@ -162,7 +162,11 @@ struct vgic_dist {
 
 	/* Distributor and vcpu interface mapping in the guest */
 	phys_addr_t		vgic_dist_base;
-	phys_addr_t		vgic_cpu_base;
+	/* GICv2 and GICv3 use different mapped register blocks */
+	union {
+		phys_addr_t		vgic_cpu_base;
+		phys_addr_t		vgic_redist_base;
+	};
 
 	/* Distributor enabled */
 	u32			enabled;
@@ -224,6 +228,9 @@ struct vgic_dist {
 	 */
 	struct vgic_bitmap	*irq_spi_target;
 
+	/* Target MPIDR for each IRQ (needed for GICv3 IROUTERn) only */
+	u32			*irq_spi_mpidr;
+
 	/* Bitmap indicating which CPU has something pending */
 	unsigned long		*irq_pending_on_cpu;
 
include/linux/irqchip/arm-gic-v3.h +32 -0
index 03a4ea3..726d898 100644
@@ -33,6 +33,7 @@
 #define GICD_SETSPI_SR			0x0050
 #define GICD_CLRSPI_SR			0x0058
 #define GICD_SEIR			0x0068
+#define GICD_IGROUPR			0x0080
 #define GICD_ISENABLER			0x0100
 #define GICD_ICENABLER			0x0180
 #define GICD_ISPENDR			0x0200
@@ -41,14 +42,37 @@
 #define GICD_ICACTIVER			0x0380
 #define GICD_IPRIORITYR			0x0400
 #define GICD_ICFGR			0x0C00
+#define GICD_IGRPMODR			0x0D00
+#define GICD_NSACR			0x0E00
 #define GICD_IROUTER			0x6000
+#define GICD_IDREGS			0xFFD0
 #define GICD_PIDR2			0xFFE8
 
+/*
+ * Those registers are actually from GICv2, but the spec demands that they
+ * are implemented as RES0 if ARE is 1 (which we do in KVM's emulated GICv3).
+ */
+#define GICD_ITARGETSR			0x0800
+#define GICD_SGIR			0x0F00
+#define GICD_CPENDSGIR			0x0F10
+#define GICD_SPENDSGIR			0x0F20
+
 #define GICD_CTLR_RWP			(1U << 31)
+#define GICD_CTLR_DS			(1U << 6)
 #define GICD_CTLR_ARE_NS		(1U << 4)
 #define GICD_CTLR_ENABLE_G1A		(1U << 1)
 #define GICD_CTLR_ENABLE_G1		(1U << 0)
 
+/*
+ * In systems with a single security state (what we emulate in KVM)
+ * the meaning of the interrupt group enable bits is slightly different
+ */
+#define GICD_CTLR_ENABLE_SS_G1		(1U << 1)
+#define GICD_CTLR_ENABLE_SS_G0		(1U << 0)
+
+#define GICD_TYPER_LPIS			(1U << 17)
+#define GICD_TYPER_MBIS			(1U << 16)
+
 #define GICD_IROUTER_SPI_MODE_ONE	(0U << 31)
 #define GICD_IROUTER_SPI_MODE_ANY	(1U << 31)
 
@@ -56,6 +80,8 @@
 #define GIC_PIDR2_ARCH_GICv3		0x30
 #define GIC_PIDR2_ARCH_GICv4		0x40
 
+#define GIC_V3_DIST_SIZE		0x10000
+
 /*
  * Re-Distributor registers, offsets from RD_base
  */
@@ -74,6 +100,7 @@
 #define GICR_SYNCR			0x00C0
 #define GICR_MOVLPIR			0x0100
 #define GICR_MOVALLR			0x0110
+#define GICR_IDREGS			GICD_IDREGS
 #define GICR_PIDR2			GICD_PIDR2
 
 #define GICR_WAKER_ProcessorSleep	(1U << 1)
@@ -82,6 +109,7 @@
 /*
  * Re-Distributor registers, offsets from SGI_base
  */
+#define GICR_IGROUPR0			GICD_IGROUPR
 #define GICR_ISENABLER0			GICD_ISENABLER
 #define GICR_ICENABLER0			GICD_ICENABLER
 #define GICR_ISPENDR0			GICD_ISPENDR
@@ -90,10 +118,14 @@
 #define GICR_ICACTIVER0			GICD_ICACTIVER
 #define GICR_IPRIORITYR0		GICD_IPRIORITYR
 #define GICR_ICFGR0			GICD_ICFGR
+#define GICR_IGRPMODR0			GICD_IGRPMODR
+#define GICR_NSACR			GICD_NSACR
 
 #define GICR_TYPER_VLPIS		(1U << 1)
 #define GICR_TYPER_LAST			(1U << 4)
 
+#define GIC_V3_REDIST_SIZE		0x20000
+
 /*
  * CPU interface registers
  */
include/linux/kvm_host.h +1 -0
index e674693..b1aa88d 100644
@@ -1085,6 +1085,7 @@ void kvm_unregister_device_ops(u32 type);
 extern struct kvm_device_ops kvm_mpic_ops;
 extern struct kvm_device_ops kvm_xics_ops;
 extern struct kvm_device_ops kvm_arm_vgic_v2_ops;
+extern struct kvm_device_ops kvm_arm_vgic_v3_ops;
 
 #ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
 
include/uapi/linux/kvm.h +2 -0
index 6076882..24cb129 100644
@@ -960,6 +960,8 @@ enum kvm_device_type {
 #define KVM_DEV_TYPE_ARM_VGIC_V2	KVM_DEV_TYPE_ARM_VGIC_V2
 	KVM_DEV_TYPE_FLIC,
 #define KVM_DEV_TYPE_FLIC		KVM_DEV_TYPE_FLIC
+	KVM_DEV_TYPE_ARM_VGIC_V3,
+#define KVM_DEV_TYPE_ARM_VGIC_V3	KVM_DEV_TYPE_ARM_VGIC_V3
 	KVM_DEV_TYPE_MAX,
 };
 
virt/kvm/arm/vgic-v3-emul.c new file +235 -0
new file mode 100644
index 0000000..18af188
@@ -0,0 +1,899 @@
+/*
+ * GICv3 distributor and redistributor emulation
+ *
+ * GICv3 emulation is currently only supported on a GICv3 host (because
+ * we rely on the hardware's CPU interface virtualization support), but
+ * supports both hardware with or without the optional GICv2 backwards
+ * compatibility features.
+ *
+ * Limitations of the emulation:
+ * (RAZ/WI: read as zero, write ignore, RAO/WI: read as one, write ignore)
+ * - We do not support LPIs (yet). TYPER.LPIS is reported as 0 and is RAZ/WI.
+ * - We do not support the message based interrupts (MBIs) triggered by
+ *   writes to the GICD_{SET,CLR}SPI_* registers. TYPER.MBIS is reported as 0.
+ * - We do not support the (optional) backwards compatibility feature.
+ *   GICD_CTLR.ARE resets to 1 and is RAO/WI. If the _host_ GIC supports
+ *   the compatiblity feature, you can use a GICv2 in the guest, though.
+ * - We only support a single security state. GICD_CTLR.DS is 1 and is RAO/WI.
+ * - Priorities are not emulated (same as the GICv2 emulation). Linux
+ *   as a guest is fine with this, because it does not use priorities.
+ * - We only support Group1 interrupts. Again Linux uses only those.
+ *
+ * Copyright (C) 2014 ARM Ltd.
+ * Author: Andre Przywara <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/cpu.h>
+#include <linux/kvm.h>
+#include <linux/kvm_host.h>
+#include <linux/interrupt.h>
+
+#include <linux/irqchip/arm-gic-v3.h>
+#include <kvm/arm_vgic.h>
+
+#include <asm/kvm_emulate.h>
+#include <asm/kvm_arm.h>
+#include <asm/kvm_mmu.h>
+
+#include "vgic.h"
+
+static bool handle_mmio_rao_wi(struct kvm_vcpu *vcpu,
+			       struct kvm_exit_mmio *mmio, phys_addr_t offset)
+{
+	u32 reg = 0xffffffff;
+
+	vgic_reg_access(mmio, &reg, offset,
+			ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
+
+	return false;
+}
+
+static bool handle_mmio_ctlr(struct kvm_vcpu *vcpu,
+			     struct kvm_exit_mmio *mmio, phys_addr_t offset)
+{
+	u32 reg = 0;
+
+	/*
+	 * Force ARE and DS to 1, the guest cannot change this.
+	 * For the time being we only support Group1 interrupts.
+	 */
+	if (vcpu->kvm->arch.vgic.enabled)
+		reg = GICD_CTLR_ENABLE_SS_G1;
+	reg |= GICD_CTLR_ARE_NS | GICD_CTLR_DS;
+
+	vgic_reg_access(mmio, &reg, offset,
+			ACCESS_READ_VALUE | ACCESS_WRITE_VALUE);
+	if (mmio->is_write) {
+		if (reg & GICD_CTLR_ENABLE_SS_G0)
+			kvm_info("guest tried to enable unsupported Group0 interrupts\n");
+		vcpu->kvm->arch.vgic.enabled = !!(reg & GICD_CTLR_ENABLE_SS_G1);
+		vgic_update_state(vcpu->kvm);
+		return true;
+	}
+	return false;
+}
+
+/*
+ * As this implementation does not provide compatibility
+ * with GICv2 (ARE==1), we report zero CPUs in bits [5..7].
+ * Also LPIs and MBIs are not supported, so we set the respective bits to 0.
+ * Also we report at most 2**10=1024 interrupt IDs (to match 1024 SPIs).
+ */
+#define INTERRUPT_ID_BITS 10
+static bool handle_mmio_typer(struct kvm_vcpu *vcpu,
+			      struct kvm_exit_mmio *mmio, phys_addr_t offset)
+{
+	u32 reg;
+
+	reg = (min(vcpu->kvm->arch.vgic.nr_irqs, 1024) >> 5) - 1;
+
+	reg |= (INTERRUPT_ID_BITS - 1) << 19;
+
+	vgic_reg_access(mmio, &reg, offset,
+			ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
+
+	return false;
+}
+
+static bool handle_mmio_iidr(struct kvm_vcpu *vcpu,
+			     struct kvm_exit_mmio *mmio, phys_addr_t offset)
+{
+	u32 reg;
+
+	reg = (PRODUCT_ID_KVM << 24) | (IMPLEMENTER_ARM << 0);
+	vgic_reg_access(mmio, &reg, offset,
+			ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
+
+	return false;
+}
+
+static bool handle_mmio_set_enable_reg_dist(struct kvm_vcpu *vcpu,
+					    struct kvm_exit_mmio *mmio,
+					    phys_addr_t offset)
+{
+	if (likely(offset >= VGIC_NR_PRIVATE_IRQS / 8))
+		return vgic_handle_enable_reg(vcpu->kvm, mmio, offset,
+					      vcpu->vcpu_id,
+					      ACCESS_WRITE_SETBIT);
+
+	vgic_reg_access(mmio, NULL, offset,
+			ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+	return false;
+}
+
+static bool handle_mmio_clear_enable_reg_dist(struct kvm_vcpu *vcpu,
+					      struct kvm_exit_mmio *mmio,
+					      phys_addr_t offset)
+{
+	if (likely(offset >= VGIC_NR_PRIVATE_IRQS / 8))
+		return vgic_handle_enable_reg(vcpu->kvm, mmio, offset,
+					      vcpu->vcpu_id,
+					      ACCESS_WRITE_CLEARBIT);
+
+	vgic_reg_access(mmio, NULL, offset,
+			ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+	return false;
+}
+
+static bool handle_mmio_set_pending_reg_dist(struct kvm_vcpu *vcpu,
+					     struct kvm_exit_mmio *mmio,
+					     phys_addr_t offset)
+{
+	if (likely(offset >= VGIC_NR_PRIVATE_IRQS / 8))
+		return vgic_handle_set_pending_reg(vcpu->kvm, mmio, offset,
+						   vcpu->vcpu_id);
+
+	vgic_reg_access(mmio, NULL, offset,
+			ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+	return false;
+}
+
+static bool handle_mmio_clear_pending_reg_dist(struct kvm_vcpu *vcpu,
+					       struct kvm_exit_mmio *mmio,
+					       phys_addr_t offset)
+{
+	if (likely(offset >= VGIC_NR_PRIVATE_IRQS / 8))
+		return vgic_handle_clear_pending_reg(vcpu->kvm, mmio, offset,
+						     vcpu->vcpu_id);
+
+	vgic_reg_access(mmio, NULL, offset,
+			ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+	return false;
+}
+
+static bool handle_mmio_priority_reg_dist(struct kvm_vcpu *vcpu,
+					  struct kvm_exit_mmio *mmio,
+					  phys_addr_t offset)
+{
+	u32 *reg;
+
+	if (unlikely(offset < VGIC_NR_PRIVATE_IRQS)) {
+		vgic_reg_access(mmio, NULL, offset,
+				ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+		return false;
+	}
+
+	reg = vgic_bytemap_get_reg(&vcpu->kvm->arch.vgic.irq_priority,
+				   vcpu->vcpu_id, offset);
+	vgic_reg_access(mmio, reg, offset,
+		ACCESS_READ_VALUE | ACCESS_WRITE_VALUE);
+	return false;
+}
+
+static bool handle_mmio_cfg_reg_dist(struct kvm_vcpu *vcpu,
+				     struct kvm_exit_mmio *mmio,
+				     phys_addr_t offset)
+{
+	u32 *reg;
+
+	if (unlikely(offset < VGIC_NR_PRIVATE_IRQS / 4)) {
+		vgic_reg_access(mmio, NULL, offset,
+				ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+		return false;
+	}
+
+	reg = vgic_bitmap_get_reg(&vcpu->kvm->arch.vgic.irq_cfg,
+				  vcpu->vcpu_id, offset >> 1);
+
+	return vgic_handle_cfg_reg(reg, mmio, offset);
+}
+
+/*
+ * We use a compressed version of the MPIDR (all 32 bits in one 32-bit word)
+ * when we store the target MPIDR written by the guest.
+ */
+static u32 compress_mpidr(unsigned long mpidr)
+{
+	u32 ret;
+
+	ret = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	ret |= MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8;
+	ret |= MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16;
+	ret |= MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24;
+
+	return ret;
+}
+
+static unsigned long uncompress_mpidr(u32 value)
+{
+	unsigned long mpidr;
+
+	mpidr  = ((value >>  0) & 0xFF) << MPIDR_LEVEL_SHIFT(0);
+	mpidr |= ((value >>  8) & 0xFF) << MPIDR_LEVEL_SHIFT(1);
+	mpidr |= ((value >> 16) & 0xFF) << MPIDR_LEVEL_SHIFT(2);

virt/kvm/arm/vgic-v3-emul.c line 245

Quoted 10 lines
virt/kvm/arm/vgic-v3-emul.c new file +10 -0
+	mpidr |= (u64)((value >> 24) & 0xFF) << MPIDR_LEVEL_SHIFT(3);
+
+	return mpidr;
+}
+
+/*
+ * Lookup the given MPIDR value to get the vcpu_id (if there is one)
+ * and store that in the irq_spi_cpu[] array.
+ * This limits the number of VCPUs to 255 for now, extending the data
+ * type (or storing kvm_vcpu poiners) should lift the limit.

s/poiners/pointers/

Quoted 323 lines
virt/kvm/arm/vgic-v3-emul.c new file +323 -0
+ * Store the original MPIDR value in an extra array to support read-as-written.
+ * Unallocated MPIDRs are translated to a special value and caught
+ * before any array accesses.
+ */
+static bool handle_mmio_route_reg(struct kvm_vcpu *vcpu,
+				  struct kvm_exit_mmio *mmio,
+				  phys_addr_t offset)
+{
+	struct kvm *kvm = vcpu->kvm;
+	struct vgic_dist *dist = &kvm->arch.vgic;
+	int spi;
+	u32 reg;
+	int vcpu_id;
+	unsigned long *bmap, mpidr;
+
+	/*
+	 * The upper 32 bits of each 64 bit register are zero,
+	 * as we don't support Aff3.
+	 */
+	if ((offset & 4)) {
+		vgic_reg_access(mmio, NULL, offset,
+				ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+		return false;
+	}
+
+	/* This region only covers SPIs, so no handling of private IRQs here. */
+	spi = offset / 8;
+
+	/* get the stored MPIDR for this IRQ */
+	mpidr = uncompress_mpidr(dist->irq_spi_mpidr[spi]);
+	reg = mpidr;
+
+	vgic_reg_access(mmio, &reg, offset,
+			ACCESS_READ_VALUE | ACCESS_WRITE_VALUE);
+
+	if (!mmio->is_write)
+		return false;
+
+	/*
+	 * Now clear the currently assigned vCPU from the map, making room
+	 * for the new one to be written below
+	 */
+	vcpu = kvm_mpidr_to_vcpu(kvm, mpidr);
+	if (likely(vcpu)) {
+		vcpu_id = vcpu->vcpu_id;
+		bmap = vgic_bitmap_get_shared_map(&dist->irq_spi_target[vcpu_id]);
+		__clear_bit(spi, bmap);
+	}
+
+	dist->irq_spi_mpidr[spi] = compress_mpidr(reg);
+	vcpu = kvm_mpidr_to_vcpu(kvm, reg & MPIDR_HWID_BITMASK);
+
+	/*
+	 * The spec says that non-existent MPIDR values should not be
+	 * forwarded to any existent (v)CPU, but should be able to become
+	 * pending anyway. We simply keep the irq_spi_target[] array empty, so
+	 * the interrupt will never be injected.
+	 * irq_spi_cpu[irq] gets a magic value in this case.
+	 */
+	if (likely(vcpu)) {
+		vcpu_id = vcpu->vcpu_id;
+		dist->irq_spi_cpu[spi] = vcpu_id;
+		bmap = vgic_bitmap_get_shared_map(&dist->irq_spi_target[vcpu_id]);
+		__set_bit(spi, bmap);
+	} else {
+		dist->irq_spi_cpu[spi] = VCPU_NOT_ALLOCATED;
+	}
+
+	vgic_update_state(kvm);
+
+	return true;
+}
+
+/*
+ * We should be careful about promising too much when a guest reads
+ * this register. Don't claim to be like any hardware implementation,
+ * but just report the GIC as version 3 - which is what a Linux guest
+ * would check.
+ */
+static bool handle_mmio_idregs(struct kvm_vcpu *vcpu,
+			       struct kvm_exit_mmio *mmio,
+			       phys_addr_t offset)
+{
+	u32 reg = 0;
+
+	switch (offset + GICD_IDREGS) {
+	case GICD_PIDR2:
+		reg = 0x3b;
+		break;
+	}
+
+	vgic_reg_access(mmio, &reg, offset,
+			ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
+
+	return false;
+}
+
+static const struct kvm_mmio_range vgic_v3_dist_ranges[] = {
+	{
+		.base           = GICD_CTLR,
+		.len            = 0x04,
+		.bits_per_irq   = 0,
+		.handle_mmio    = handle_mmio_ctlr,
+	},
+	{
+		.base           = GICD_TYPER,
+		.len            = 0x04,
+		.bits_per_irq   = 0,
+		.handle_mmio    = handle_mmio_typer,
+	},
+	{
+		.base           = GICD_IIDR,
+		.len            = 0x04,
+		.bits_per_irq   = 0,
+		.handle_mmio    = handle_mmio_iidr,
+	},
+	{
+		/* this register is optional, it is RAZ/WI if not implemented */
+		.base           = GICD_STATUSR,
+		.len            = 0x04,
+		.bits_per_irq   = 0,
+		.handle_mmio    = handle_mmio_raz_wi,
+	},
+	{
+		/* this write only register is WI when TYPER.MBIS=0 */
+		.base		= GICD_SETSPI_NSR,
+		.len		= 0x04,
+		.bits_per_irq	= 0,
+		.handle_mmio	= handle_mmio_raz_wi,
+	},
+	{
+		/* this write only register is WI when TYPER.MBIS=0 */
+		.base		= GICD_CLRSPI_NSR,
+		.len		= 0x04,
+		.bits_per_irq	= 0,
+		.handle_mmio	= handle_mmio_raz_wi,
+	},
+	{
+		/* this is RAZ/WI when DS=1 */
+		.base		= GICD_SETSPI_SR,
+		.len		= 0x04,
+		.bits_per_irq	= 0,
+		.handle_mmio	= handle_mmio_raz_wi,
+	},
+	{
+		/* this is RAZ/WI when DS=1 */
+		.base		= GICD_CLRSPI_SR,
+		.len		= 0x04,
+		.bits_per_irq	= 0,
+		.handle_mmio	= handle_mmio_raz_wi,
+	},
+	{
+		.base		= GICD_IGROUPR,
+		.len		= 0x80,
+		.bits_per_irq	= 1,
+		.handle_mmio	= handle_mmio_rao_wi,
+	},
+	{
+		.base		= GICD_ISENABLER,
+		.len		= 0x80,
+		.bits_per_irq	= 1,
+		.handle_mmio	= handle_mmio_set_enable_reg_dist,
+	},
+	{
+		.base		= GICD_ICENABLER,
+		.len		= 0x80,
+		.bits_per_irq	= 1,
+		.handle_mmio	= handle_mmio_clear_enable_reg_dist,
+	},
+	{
+		.base		= GICD_ISPENDR,
+		.len		= 0x80,
+		.bits_per_irq	= 1,
+		.handle_mmio	= handle_mmio_set_pending_reg_dist,
+	},
+	{
+		.base		= GICD_ICPENDR,
+		.len		= 0x80,
+		.bits_per_irq	= 1,
+		.handle_mmio	= handle_mmio_clear_pending_reg_dist,
+	},
+	{
+		.base		= GICD_ISACTIVER,
+		.len		= 0x80,
+		.bits_per_irq	= 1,
+		.handle_mmio	= handle_mmio_raz_wi,
+	},
+	{
+		.base		= GICD_ICACTIVER,
+		.len		= 0x80,
+		.bits_per_irq	= 1,
+		.handle_mmio	= handle_mmio_raz_wi,
+	},
+	{
+		.base		= GICD_IPRIORITYR,
+		.len		= 0x400,
+		.bits_per_irq	= 8,
+		.handle_mmio	= handle_mmio_priority_reg_dist,
+	},
+	{
+		/* TARGETSRn is RES0 when ARE=1 */
+		.base		= GICD_ITARGETSR,
+		.len		= 0x400,
+		.bits_per_irq	= 8,
+		.handle_mmio	= handle_mmio_raz_wi,
+	},
+	{
+		.base		= GICD_ICFGR,
+		.len		= 0x100,
+		.bits_per_irq	= 2,
+		.handle_mmio	= handle_mmio_cfg_reg_dist,
+	},
+	{
+		/* this is RAZ/WI when DS=1 */
+		.base		= GICD_IGRPMODR,
+		.len		= 0x80,
+		.bits_per_irq	= 1,
+		.handle_mmio	= handle_mmio_raz_wi,
+	},
+	{
+		/* this is RAZ/WI when DS=1 */
+		.base		= GICD_NSACR,
+		.len		= 0x100,
+		.bits_per_irq	= 2,
+		.handle_mmio	= handle_mmio_raz_wi,
+	},
+	{
+		/* this is RAZ/WI when ARE=1 */
+		.base		= GICD_SGIR,
+		.len		= 0x04,
+		.handle_mmio	= handle_mmio_raz_wi,
+	},
+	{
+		/* this is RAZ/WI when ARE=1 */
+		.base		= GICD_CPENDSGIR,
+		.len		= 0x10,
+		.handle_mmio	= handle_mmio_raz_wi,
+	},
+	{
+		/* this is RAZ/WI when ARE=1 */
+		.base           = GICD_SPENDSGIR,
+		.len            = 0x10,
+		.handle_mmio    = handle_mmio_raz_wi,
+	},
+	{
+		.base		= GICD_IROUTER + 0x100,
+		.len		= 0x1ee0,
+		.bits_per_irq	= 64,
+		.handle_mmio	= handle_mmio_route_reg,
+	},
+	{
+		.base           = GICD_IDREGS,
+		.len            = 0x30,
+		.bits_per_irq   = 0,
+		.handle_mmio    = handle_mmio_idregs,
+	},
+	{},
+};
+
+static bool handle_mmio_set_enable_reg_redist(struct kvm_vcpu *vcpu,
+					      struct kvm_exit_mmio *mmio,
+					      phys_addr_t offset)
+{
+	struct kvm_vcpu *redist_vcpu = mmio->private;
+
+	return vgic_handle_enable_reg(vcpu->kvm, mmio, offset,
+				      redist_vcpu->vcpu_id,
+				      ACCESS_WRITE_SETBIT);
+}
+
+static bool handle_mmio_clear_enable_reg_redist(struct kvm_vcpu *vcpu,
+						struct kvm_exit_mmio *mmio,
+						phys_addr_t offset)
+{
+	struct kvm_vcpu *redist_vcpu = mmio->private;
+
+	return vgic_handle_enable_reg(vcpu->kvm, mmio, offset,
+				      redist_vcpu->vcpu_id,
+				      ACCESS_WRITE_CLEARBIT);
+}
+
+static bool handle_mmio_set_pending_reg_redist(struct kvm_vcpu *vcpu,
+					       struct kvm_exit_mmio *mmio,
+					       phys_addr_t offset)
+{
+	struct kvm_vcpu *redist_vcpu = mmio->private;
+
+	return vgic_handle_set_pending_reg(vcpu->kvm, mmio, offset,
+					   redist_vcpu->vcpu_id);
+}
+
+static bool handle_mmio_clear_pending_reg_redist(struct kvm_vcpu *vcpu,
+						 struct kvm_exit_mmio *mmio,
+						 phys_addr_t offset)
+{
+	struct kvm_vcpu *redist_vcpu = mmio->private;
+
+	return vgic_handle_clear_pending_reg(vcpu->kvm, mmio, offset,
+					     redist_vcpu->vcpu_id);
+}
+
+static bool handle_mmio_priority_reg_redist(struct kvm_vcpu *vcpu,
+					    struct kvm_exit_mmio *mmio,
+					    phys_addr_t offset)
+{
+	struct kvm_vcpu *redist_vcpu = mmio->private;
+	u32 *reg;
+
+	reg = vgic_bytemap_get_reg(&vcpu->kvm->arch.vgic.irq_priority,
+				   redist_vcpu->vcpu_id, offset);
+	vgic_reg_access(mmio, reg, offset,
+			ACCESS_READ_VALUE | ACCESS_WRITE_VALUE);
+	return false;
+}
+
+static bool handle_mmio_cfg_reg_redist(struct kvm_vcpu *vcpu,
+				       struct kvm_exit_mmio *mmio,
+				       phys_addr_t offset)
+{
+	struct kvm_vcpu *redist_vcpu = mmio->private;
+
+	u32 *reg = vgic_bitmap_get_reg(&vcpu->kvm->arch.vgic.irq_cfg,
+				       redist_vcpu->vcpu_id, offset >> 1);

virt/kvm/arm/vgic-v3-emul.c line 578

Quoted 10 lines
virt/kvm/arm/vgic-v3-emul.c new file +10 -0
+
+	return vgic_handle_cfg_reg(reg, mmio, offset);
+}
+
+static const struct kvm_mmio_range vgic_redist_sgi_ranges[] = {
+	{
+		.base		= GICR_IGROUPR0,
+		.len		= 0x04,
+		.bits_per_irq	= 1,
+		.handle_mmio	= handle_mmio_raz_wi,

weren't we going to change this to handle_mmio_rao_wi()?

(this is the third time I'm bringing it up)

Assuming you fix the two minor issues above:

#34 lore raw

PATCH v5 15/19 arm/arm64: KVM: add virtual GICv3 distributor emulation

Hej Christoffer,

thanks for going through all of this ... again ;-)

Quoted 17 lines · Christoffer Dall
Quoted 16 lines · Andre Przywara

With everything separated and prepared, we implement a model of a GICv3 distributor and redistributors by using the existing framework to provide handler functions for each register group.

Currently we limit the emulation to a model enforcing a single security state, with SRE==1 (forcing system register access) and ARE==1 (allowing more than 8 VCPUs).

We share some of the functions provided for GICv2 emulation, but take the different ways of addressing (v)CPUs into account. Save and restore is currently not implemented.

Similar to the split-off of the GICv2 specific code, the new emulation code goes into a new file (vgic-v3-emul.c).

[...]

Quoted 6 lines
Quoted 6 lines
virt/kvm/arm/vgic-v3-emul.c new file +0 -0
new file mode 100644
index 0000000..18af188
@@ -0,0 +1,899 @@

[...]

Quoted 11 lines
Quoted 11 lines
virt/kvm/arm/vgic-v3-emul.c new file +11 -0
+static unsigned long uncompress_mpidr(u32 value)
+{
+     unsigned long mpidr;
+
+     mpidr  = ((value >>  0) & 0xFF) << MPIDR_LEVEL_SHIFT(0);
+     mpidr |= ((value >>  8) & 0xFF) << MPIDR_LEVEL_SHIFT(1);
+     mpidr |= ((value >> 16) & 0xFF) << MPIDR_LEVEL_SHIFT(2);
+     mpidr |= (u64)((value >> 24) & 0xFF) << MPIDR_LEVEL_SHIFT(3);
+
+     return mpidr;
+}
Quoted 8 lines

virt/kvm/arm/vgic-v3-emul.c line 245

Quoted 6 lines
virt/kvm/arm/vgic-v3-emul.c new file +6 -0
+
+/*
+ * Lookup the given MPIDR value to get the vcpu_id (if there is one)
+ * and store that in the irq_spi_cpu[] array.
+ * This limits the number of VCPUs to 255 for now, extending the data
+ * type (or storing kvm_vcpu poiners) should lift the limit.

s/poiners/pointers/

Sorry and thanks for catching that. I accidentally merged the fix for this in 17/19. Will fix this in the next revision.

Quoted 8 lines
Quoted 8 lines

+ * Store the original MPIDR value in an extra array to support read-as-written.
+ * Unallocated MPIDRs are translated to a special value and caught
+ * before any array accesses.
+ */

virt/kvm/arm/vgic-v3-emul.c new file +4 -0
+static bool handle_mmio_route_reg(struct kvm_vcpu *vcpu,
+                               struct kvm_exit_mmio *mmio,
+                               phys_addr_t offset)
+{

[...]

Quoted 10 lines

virt/kvm/arm/vgic-v3-emul.c line 578

Quoted 8 lines
virt/kvm/arm/vgic-v3-emul.c new file +8 -0
+}
+
+static const struct kvm_mmio_range vgic_redist_sgi_ranges[] = {
+     {
+             .base           = GICR_IGROUPR0,
+             .len            = 0x04,
+             .bits_per_irq   = 1,
+             .handle_mmio    = handle_mmio_raz_wi,

weren't we going to change this to handle_mmio_rao_wi()?

My bad, I missed this one (the distributor part is correct, though).

Quoted 1 line

Thanks for that!

Cheers,
Andre.

#35 lore raw

PATCH v5 16/19 arm64: GICv3: introduce symbolic names for GICv3 ICC_SGI1R_EL1 fields

In reply to Andre Przywara

The gic_send_sgi() function used hardcoded bit shift values to generate the ICC_SGI1R_EL1 register value.
Replace this with symbolic names to allow reusing them later.

---
Changelog v4...v5:
(add Reviewed-by:)

Changelog v3...v4:
- (new patch)

 drivers/irqchip/irq-gic-v3.c       |   14 +++++++++-----
 include/linux/irqchip/arm-gic-v3.h |   12 ++++++++++++
 2 files changed, 21 insertions(+), 5 deletions(-)
drivers/irqchip/irq-gic-v3.c +9 -5
index aa17ae8..e163749 100644
@@ -467,15 +467,19 @@ out:
 	return tlist;
 }
 
+#define MPIDR_TO_SGI_AFFINITY(cluster_id, level) \
+	(MPIDR_AFFINITY_LEVEL(cluster_id, level) \
+		<< ICC_SGI1R_AFFINITY_## level ##_SHIFT)
+
 static void gic_send_sgi(u64 cluster_id, u16 tlist, unsigned int irq)
 {
 	u64 val;
 
-	val = (MPIDR_AFFINITY_LEVEL(cluster_id, 3) << 48	|
-	       MPIDR_AFFINITY_LEVEL(cluster_id, 2) << 32	|
-	       irq << 24			    		|
-	       MPIDR_AFFINITY_LEVEL(cluster_id, 1) << 16	|
-	       tlist);
+	val = (MPIDR_TO_SGI_AFFINITY(cluster_id, 3)	|
+	       MPIDR_TO_SGI_AFFINITY(cluster_id, 2)	|
+	       irq << ICC_SGI1R_SGI_ID_SHIFT		|
+	       MPIDR_TO_SGI_AFFINITY(cluster_id, 1)	|
+	       tlist << ICC_SGI1R_TARGET_LIST_SHIFT);
 
 	pr_debug("CPU%d: ICC_SGI1R_EL1 %llx\n", smp_processor_id(), val);
 	gic_write_sgi1r(val);
include/linux/irqchip/arm-gic-v3.h +12 -0
index 726d898..b7946c3 100644
@@ -174,6 +174,18 @@
 #define ICC_SRE_EL2_SRE			(1 << 0)
 #define ICC_SRE_EL2_ENABLE		(1 << 3)
 
+#define ICC_SGI1R_TARGET_LIST_SHIFT	0
+#define ICC_SGI1R_TARGET_LIST_MASK	(0xffff << ICC_SGI1R_TARGET_LIST_SHIFT)
+#define ICC_SGI1R_AFFINITY_1_SHIFT	16
+#define ICC_SGI1R_AFFINITY_1_MASK	(0xff << ICC_SGI1R_AFFINITY_1_SHIFT)
+#define ICC_SGI1R_SGI_ID_SHIFT		24
+#define ICC_SGI1R_SGI_ID_MASK		(0xff << ICC_SGI1R_SGI_ID_SHIFT)
+#define ICC_SGI1R_AFFINITY_2_SHIFT	32
+#define ICC_SGI1R_AFFINITY_2_MASK	(0xffULL << ICC_SGI1R_AFFINITY_1_SHIFT)
+#define ICC_SGI1R_IRQ_ROUTING_MODE_BIT	40
+#define ICC_SGI1R_AFFINITY_3_SHIFT	48
+#define ICC_SGI1R_AFFINITY_3_MASK	(0xffULL << ICC_SGI1R_AFFINITY_1_SHIFT)
+
 /*
  * System register definitions
  */
Signature · 1 line
1.7.9.5
#36 lore raw

PATCH v5 17/19 arm64: KVM: add SGI generation register emulation

In reply to Andre Przywara

While the generation of a (virtual) inter-processor interrupt (SGI) on a GICv2 works by writing to a MMIO register, GICv3 uses the system register ICC_SGI1R_EL1 to trigger them.
Add a trap handler function that calls the new SGI register handler in the GICv3 code. As ICC_SRE_EL1.SRE at this point is still always 0, this will not trap yet, but will only be used later when all the data structures have been initialized properly.

---
Changelog v4...v5:
- fix merge conflict due to 3.18.0 rebase
- minor style changes and comments fixes

Changelog v3...v4:
- moved addition of vgic_v3_dispatch_sgi() from earlier patch into here
- move MPIDR comparison into extra function
- use new ICC_SGI1R_ field names
- improve readability of vgic_v3_dispatch_sgi()
- add and refine comments

 arch/arm64/kvm/sys_regs.c   |   26 ++++++++++
 include/kvm/arm_vgic.h      |    1 +
 virt/kvm/arm/vgic-v3-emul.c |  113 ++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 139 insertions(+), 1 deletion(-)
arch/arm64/kvm/sys_regs.c +26 -0
index 136e679..3bcee19 100644
@@ -165,6 +165,27 @@ static bool access_sctlr(struct kvm_vcpu *vcpu,
 	return true;
 }
 
+/*
+ * Trap handler for the GICv3 SGI generation system register.
+ * Forward the request to the VGIC emulation.
+ * The cp15_64 code makes sure this automatically works
+ * for both AArch64 and AArch32 accesses.
+ */
+static bool access_gic_sgi(struct kvm_vcpu *vcpu,
+			   const struct sys_reg_params *p,
+			   const struct sys_reg_desc *r)
+{
+	u64 val;
+
+	if (!p->is_write)
+		return read_from_write_only(vcpu, p);
+
+	val = *vcpu_reg(vcpu, p->Rt);
+	vgic_v3_dispatch_sgi(vcpu, val);
+
+	return true;
+}
+
 static bool trap_raz_wi(struct kvm_vcpu *vcpu,
 			const struct sys_reg_params *p,
 			const struct sys_reg_desc *r)
@@ -434,6 +455,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ Op0(0b11), Op1(0b000), CRn(0b1100), CRm(0b0000), Op2(0b000),
 	  NULL, reset_val, VBAR_EL1, 0 },
 
+	/* ICC_SGI1R_EL1 */
+	{ Op0(0b11), Op1(0b000), CRn(0b1100), CRm(0b1011), Op2(0b101),
+	  access_gic_sgi },
 	/* ICC_SRE_EL1 */
 	{ Op0(0b11), Op1(0b000), CRn(0b1100), CRm(0b1100), Op2(0b101),
 	  trap_raz_wi },
@@ -666,6 +690,8 @@ static const struct sys_reg_desc cp14_64_regs[] = {
  * register).
  */
 static const struct sys_reg_desc cp15_regs[] = {
+	{ Op1( 0), CRn( 0), CRm(12), Op2( 0), access_gic_sgi },
+
 	{ Op1( 0), CRn( 1), CRm( 0), Op2( 0), access_sctlr, NULL, c1_SCTLR },
 	{ Op1( 0), CRn( 2), CRm( 0), Op2( 0), access_vm_reg, NULL, c2_TTBR0 },
 	{ Op1( 0), CRn( 2), CRm( 0), Op2( 1), access_vm_reg, NULL, c2_TTBR1 },
include/kvm/arm_vgic.h +1 -0
index 668d575..b8dc144 100644
@@ -307,6 +307,7 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu);
 void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu);
 int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
 			bool level);
+void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
 int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
 bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		      struct kvm_exit_mmio *mmio);
virt/kvm/arm/vgic-v3-emul.c +112 -1

125 lines not on this page · raw message

Signature · 1 line
1.7.9.5
#37 lore raw

PATCH v5 18/19 arm/arm64: KVM: enable kernel side of GICv3 emulation

In reply to Andre Przywara

With all the necessary GICv3 emulation code in place, we can now connect the code to the GICv3 backend in the kernel.
The LR register handling is different depending on the emulated GIC model, so provide different implementations for each. Also allow non-v2-compatible GICv3 implementations (which don't provide MMIO regions for the virtual CPU interface in the DT), but restrict those hosts to support GICv3 guests only.

---
Changelog v4...v5:
- minor whitespace and comments fixes

Changelog v3...v4:
- handle differences between GICv2-on-v3 and GICv3-on-v3 in existing functions
- remove init_*_emul() functions
- remove max_vcpus setting (done in earlier patches now)
- adapt to new vgic_v<n>_init_emulation behaviour

 virt/kvm/arm/vgic-v3.c |   84 ++++++++++++++++++++++++++++++++----------------
 virt/kvm/arm/vgic.c    |    5 +++
 2 files changed, 61 insertions(+), 28 deletions(-)
virt/kvm/arm/vgic-v3.c +56 -28
index 5249048..88e2ed8 100644
@@ -34,6 +34,7 @@
 #define GICH_LR_VIRTUALID		(0x3ffUL << 0)
 #define GICH_LR_PHYSID_CPUID_SHIFT	(10)
 #define GICH_LR_PHYSID_CPUID		(7UL << GICH_LR_PHYSID_CPUID_SHIFT)
+#define ICH_LR_VIRTUALID_MASK		(BIT_ULL(32) - 1)
 
 /*
  * LRs are stored in reverse order in memory. make sure we index them
@@ -48,12 +49,17 @@ static struct vgic_lr vgic_v3_get_lr(const struct kvm_vcpu *vcpu, int lr)
 	struct vgic_lr lr_desc;
 	u64 val = vcpu->arch.vgic_cpu.vgic_v3.vgic_lr[LR_INDEX(lr)];
 
-	lr_desc.irq	= val & GICH_LR_VIRTUALID;
-	if (lr_desc.irq <= 15)
-		lr_desc.source	= (val >> GICH_LR_PHYSID_CPUID_SHIFT) & 0x7;
+	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		lr_desc.irq = val & ICH_LR_VIRTUALID_MASK;
 	else
-		lr_desc.source = 0;
-	lr_desc.state	= 0;
+		lr_desc.irq = val & GICH_LR_VIRTUALID;
+
+	lr_desc.source = 0;
+	if (lr_desc.irq <= 15 &&
+	    vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2)
+		lr_desc.source = (val >> GICH_LR_PHYSID_CPUID_SHIFT) & 0x7;
+
+	lr_desc.state = 0;
 
 	if (val & ICH_LR_PENDING_BIT)
 		lr_desc.state |= LR_STATE_PENDING;
@@ -68,8 +74,20 @@ static struct vgic_lr vgic_v3_get_lr(const struct kvm_vcpu *vcpu, int lr)
 static void vgic_v3_set_lr(struct kvm_vcpu *vcpu, int lr,
 			   struct vgic_lr lr_desc)
 {
-	u64 lr_val = (((u32)lr_desc.source << GICH_LR_PHYSID_CPUID_SHIFT) |
-		      lr_desc.irq);
+	u64 lr_val;
+
+	lr_val = lr_desc.irq;
+
+	/*
+	 * Currently all guest IRQs are Group1, as Group0 would result
+	 * in a FIQ in the guest, which it wouldn't expect.
+	 * Eventually we want to make this configurable, so we may revisit
+	 * this in the future.
+	 */
+	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		lr_val |= ICH_LR_GROUP;
+	else
+		lr_val |= (u32)lr_desc.source << GICH_LR_PHYSID_CPUID_SHIFT;
 
 	if (lr_desc.state & LR_STATE_PENDING)
 		lr_val |= ICH_LR_PENDING_BIT;
@@ -154,7 +172,15 @@ static void vgic_v3_enable(struct kvm_vcpu *vcpu)
 	 */
 	vgic_v3->vgic_vmcr = 0;
 
-	vgic_v3->vgic_sre = 0;
+	/*
+	 * If we are emulating a GICv3, we do it in an non-GICv2-compatible
+	 * way, so we force SRE to 1 to demonstrate this to the guest.
+	 * This goes with the spec allowing the value to be RAO/WI.
+	 */
+	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		vgic_v3->vgic_sre = ICC_SRE_EL1_SRE;
+	else
+		vgic_v3->vgic_sre = 0;
 
 	/* Get the show on the road... */
 	vgic_v3->vgic_hcr = ICH_HCR_EN;
@@ -215,28 +241,30 @@ int vgic_v3_probe(struct device_node *vgic_node,
 
 	gicv_idx += 3; /* Also skip GICD, GICC, GICH */
 	if (of_address_to_resource(vgic_node, gicv_idx, &vcpu_res)) {
-		kvm_err("Cannot obtain GICV region\n");
-		ret = -ENXIO;
-		goto out;
-	}
-
-	if (!PAGE_ALIGNED(vcpu_res.start)) {
-		kvm_err("GICV physical address 0x%llx not page aligned\n",
-			(unsigned long long)vcpu_res.start);
-		ret = -ENXIO;
-		goto out;
-	}
-
-	if (!PAGE_ALIGNED(resource_size(&vcpu_res))) {
-		kvm_err("GICV size 0x%llx not a multiple of page size 0x%lx\n",
-			(unsigned long long)resource_size(&vcpu_res),
-			PAGE_SIZE);
-		ret = -ENXIO;
-		goto out;
+		kvm_info("GICv3: GICv2 emulation not available\n");
+		vgic->vcpu_base = 0;
+	} else {
+		if (!PAGE_ALIGNED(vcpu_res.start)) {
+			kvm_err("GICV physical address 0x%llx not page aligned\n",
+				(unsigned long long)vcpu_res.start);
+			ret = -ENXIO;
+			goto out;
+		}
+
+		if (!PAGE_ALIGNED(resource_size(&vcpu_res))) {
+			kvm_err("GICV size 0x%llx not a multiple of page size 0x%lx\n",
+				(unsigned long long)resource_size(&vcpu_res),
+				PAGE_SIZE);
+			ret = -ENXIO;
+			goto out;
+		}
+
+		vgic->vcpu_base = vcpu_res.start;
+		kvm_register_device_ops(&kvm_arm_vgic_v2_ops,
+					KVM_DEV_TYPE_ARM_VGIC_V2);
 	}
-	kvm_register_device_ops(&kvm_arm_vgic_v2_ops, KVM_DEV_TYPE_ARM_VGIC_V2);
+	kvm_register_device_ops(&kvm_arm_vgic_v3_ops, KVM_DEV_TYPE_ARM_VGIC_V3);
 
-	vgic->vcpu_base = vcpu_res.start;
 	vgic->vctrl_base = NULL;
 	vgic->type = VGIC_V3;
 	vgic->max_gic_vcpus = KVM_MAX_VCPUS;
virt/kvm/arm/vgic.c +5 -0
index d7d9258..a57d139 100644
@@ -1584,6 +1584,11 @@ static int init_vgic_model(struct kvm *kvm, int type)
 	case KVM_DEV_TYPE_ARM_VGIC_V2:
 		ret = vgic_v2_init_emulation(kvm);
 		break;
+#ifdef CONFIG_ARM_GIC_V3
+	case KVM_DEV_TYPE_ARM_VGIC_V3:
+		ret = vgic_v3_init_emulation(kvm);
+		break;
+#endif
 	default:
 		ret = -ENODEV;
 		break;
Signature · 1 line
1.7.9.5
#38 lore raw

PATCH v5 18/19 arm/arm64: KVM: enable kernel side of GICv3 emulation

Quoted 127 lines · Andre Przywara

With all the necessary GICv3 emulation code in place, we can now connect the code to the GICv3 backend in the kernel.
The LR register handling is different depending on the emulated GIC model, so provide different implementations for each. Also allow non-v2-compatible GICv3 implementations (which don't provide MMIO regions for the virtual CPU interface in the DT), but restrict those hosts to support GICv3 guests only.

---
Changelog v4...v5:
- minor whitespace and comments fixes

Changelog v3...v4:
- handle differences between GICv2-on-v3 and GICv3-on-v3 in existing functions
- remove init_*_emul() functions
- remove max_vcpus setting (done in earlier patches now)
- adapt to new vgic_v<n>_init_emulation behaviour

 virt/kvm/arm/vgic-v3.c |   84 ++++++++++++++++++++++++++++++++----------------
 virt/kvm/arm/vgic.c    |    5 +++
 2 files changed, 61 insertions(+), 28 deletions(-)
virt/kvm/arm/vgic-v3.c +41 -26
index 5249048..88e2ed8 100644
@@ -34,6 +34,7 @@
 #define GICH_LR_VIRTUALID		(0x3ffUL << 0)
 #define GICH_LR_PHYSID_CPUID_SHIFT	(10)
 #define GICH_LR_PHYSID_CPUID		(7UL << GICH_LR_PHYSID_CPUID_SHIFT)
+#define ICH_LR_VIRTUALID_MASK		(BIT_ULL(32) - 1)
 
 /*
  * LRs are stored in reverse order in memory. make sure we index them
@@ -48,12 +49,17 @@ static struct vgic_lr vgic_v3_get_lr(const struct kvm_vcpu *vcpu, int lr)
 	struct vgic_lr lr_desc;
 	u64 val = vcpu->arch.vgic_cpu.vgic_v3.vgic_lr[LR_INDEX(lr)];
 
-	lr_desc.irq	= val & GICH_LR_VIRTUALID;
-	if (lr_desc.irq <= 15)
-		lr_desc.source	= (val >> GICH_LR_PHYSID_CPUID_SHIFT) & 0x7;
+	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		lr_desc.irq = val & ICH_LR_VIRTUALID_MASK;
 	else
-		lr_desc.source = 0;
-	lr_desc.state	= 0;
+		lr_desc.irq = val & GICH_LR_VIRTUALID;
+
+	lr_desc.source = 0;
+	if (lr_desc.irq <= 15 &&
+	    vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2)
+		lr_desc.source = (val >> GICH_LR_PHYSID_CPUID_SHIFT) & 0x7;
+
+	lr_desc.state = 0;
 
 	if (val & ICH_LR_PENDING_BIT)
 		lr_desc.state |= LR_STATE_PENDING;
@@ -68,8 +74,20 @@ static struct vgic_lr vgic_v3_get_lr(const struct kvm_vcpu *vcpu, int lr)
 static void vgic_v3_set_lr(struct kvm_vcpu *vcpu, int lr,
 			   struct vgic_lr lr_desc)
 {
-	u64 lr_val = (((u32)lr_desc.source << GICH_LR_PHYSID_CPUID_SHIFT) |
-		      lr_desc.irq);
+	u64 lr_val;
+
+	lr_val = lr_desc.irq;
+
+	/*
+	 * Currently all guest IRQs are Group1, as Group0 would result
+	 * in a FIQ in the guest, which it wouldn't expect.
+	 * Eventually we want to make this configurable, so we may revisit
+	 * this in the future.
+	 */
+	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		lr_val |= ICH_LR_GROUP;
+	else
+		lr_val |= (u32)lr_desc.source << GICH_LR_PHYSID_CPUID_SHIFT;
 
 	if (lr_desc.state & LR_STATE_PENDING)
 		lr_val |= ICH_LR_PENDING_BIT;
@@ -154,7 +172,15 @@ static void vgic_v3_enable(struct kvm_vcpu *vcpu)
 	 */
 	vgic_v3->vgic_vmcr = 0;
 
-	vgic_v3->vgic_sre = 0;
+	/*
+	 * If we are emulating a GICv3, we do it in an non-GICv2-compatible
+	 * way, so we force SRE to 1 to demonstrate this to the guest.
+	 * This goes with the spec allowing the value to be RAO/WI.
+	 */
+	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		vgic_v3->vgic_sre = ICC_SRE_EL1_SRE;
+	else
+		vgic_v3->vgic_sre = 0;
 
 	/* Get the show on the road... */
 	vgic_v3->vgic_hcr = ICH_HCR_EN;
@@ -215,28 +241,30 @@ int vgic_v3_probe(struct device_node *vgic_node,
 
 	gicv_idx += 3; /* Also skip GICD, GICC, GICH */
 	if (of_address_to_resource(vgic_node, gicv_idx, &vcpu_res)) {
-		kvm_err("Cannot obtain GICV region\n");
-		ret = -ENXIO;
-		goto out;
-	}
-
-	if (!PAGE_ALIGNED(vcpu_res.start)) {
-		kvm_err("GICV physical address 0x%llx not page aligned\n",
-			(unsigned long long)vcpu_res.start);
-		ret = -ENXIO;
-		goto out;
-	}
-
-	if (!PAGE_ALIGNED(resource_size(&vcpu_res))) {
-		kvm_err("GICV size 0x%llx not a multiple of page size 0x%lx\n",
-			(unsigned long long)resource_size(&vcpu_res),
-			PAGE_SIZE);
-		ret = -ENXIO;
-		goto out;
+		kvm_info("GICv3: GICv2 emulation not available\n");
+		vgic->vcpu_base = 0;
+	} else {
+		if (!PAGE_ALIGNED(vcpu_res.start)) {
+			kvm_err("GICV physical address 0x%llx not page aligned\n",
+				(unsigned long long)vcpu_res.start);
+			ret = -ENXIO;

virt/kvm/arm/vgic-v3.c int vgic_v3_probe(struct device_node *vgic_node, · line 260

Quoted 10 lines
virt/kvm/arm/vgic-v3.c +10 -0
+			goto out;
+		}
+
+		if (!PAGE_ALIGNED(resource_size(&vcpu_res))) {
+			kvm_err("GICV size 0x%llx not a multiple of page size 0x%lx\n",
+				(unsigned long long)resource_size(&vcpu_res),
+				PAGE_SIZE);
+			ret = -ENXIO;
+			goto out;
+		}

I still think this is wrong. Why whouldn't a user create be able to create a virtual GICv3 with the system register interface just because the GICv2 addresss are not aligned correctly?

Unless someone has a good technical argument for why something is likely to break, we should aim for supporting as many platforms as possible, so please change the error outs here to warnings and proceed with registering the v3 ops.

-Christoffer

#39 lore raw

PATCH v5 18/19 arm/arm64: KVM: enable kernel side of GICv3 emulation

Hi Christoffer,

Quoted 143 lines · Christoffer Dall
Quoted 127 lines · Andre Przywara

With all the necessary GICv3 emulation code in place, we can now connect the code to the GICv3 backend in the kernel.
The LR register handling is different depending on the emulated GIC model, so provide different implementations for each. Also allow non-v2-compatible GICv3 implementations (which don't provide MMIO regions for the virtual CPU interface in the DT), but restrict those hosts to support GICv3 guests only.

---
Changelog v4...v5:
- minor whitespace and comments fixes

Changelog v3...v4:
- handle differences between GICv2-on-v3 and GICv3-on-v3 in existing functions
- remove init_*_emul() functions
- remove max_vcpus setting (done in earlier patches now)
- adapt to new vgic_v<n>_init_emulation behaviour

 virt/kvm/arm/vgic-v3.c |   84 ++++++++++++++++++++++++++++++++----------------
 virt/kvm/arm/vgic.c    |    5 +++
 2 files changed, 61 insertions(+), 28 deletions(-)
virt/kvm/arm/vgic-v3.c +41 -26
index 5249048..88e2ed8 100644
@@ -34,6 +34,7 @@
 #define GICH_LR_VIRTUALID		(0x3ffUL << 0)
 #define GICH_LR_PHYSID_CPUID_SHIFT	(10)
 #define GICH_LR_PHYSID_CPUID		(7UL << GICH_LR_PHYSID_CPUID_SHIFT)
+#define ICH_LR_VIRTUALID_MASK		(BIT_ULL(32) - 1)
 
 /*
  * LRs are stored in reverse order in memory. make sure we index them
@@ -48,12 +49,17 @@ static struct vgic_lr vgic_v3_get_lr(const struct kvm_vcpu *vcpu, int lr)
 	struct vgic_lr lr_desc;
 	u64 val = vcpu->arch.vgic_cpu.vgic_v3.vgic_lr[LR_INDEX(lr)];
 
-	lr_desc.irq	= val & GICH_LR_VIRTUALID;
-	if (lr_desc.irq <= 15)
-		lr_desc.source	= (val >> GICH_LR_PHYSID_CPUID_SHIFT) & 0x7;
+	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		lr_desc.irq = val & ICH_LR_VIRTUALID_MASK;
 	else
-		lr_desc.source = 0;
-	lr_desc.state	= 0;
+		lr_desc.irq = val & GICH_LR_VIRTUALID;
+
+	lr_desc.source = 0;
+	if (lr_desc.irq <= 15 &&
+	    vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2)
+		lr_desc.source = (val >> GICH_LR_PHYSID_CPUID_SHIFT) & 0x7;
+
+	lr_desc.state = 0;
 
 	if (val & ICH_LR_PENDING_BIT)
 		lr_desc.state |= LR_STATE_PENDING;
@@ -68,8 +74,20 @@ static struct vgic_lr vgic_v3_get_lr(const struct kvm_vcpu *vcpu, int lr)
 static void vgic_v3_set_lr(struct kvm_vcpu *vcpu, int lr,
 			   struct vgic_lr lr_desc)
 {
-	u64 lr_val = (((u32)lr_desc.source << GICH_LR_PHYSID_CPUID_SHIFT) |
-		      lr_desc.irq);
+	u64 lr_val;
+
+	lr_val = lr_desc.irq;
+
+	/*
+	 * Currently all guest IRQs are Group1, as Group0 would result
+	 * in a FIQ in the guest, which it wouldn't expect.
+	 * Eventually we want to make this configurable, so we may revisit
+	 * this in the future.
+	 */
+	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		lr_val |= ICH_LR_GROUP;
+	else
+		lr_val |= (u32)lr_desc.source << GICH_LR_PHYSID_CPUID_SHIFT;
 
 	if (lr_desc.state & LR_STATE_PENDING)
 		lr_val |= ICH_LR_PENDING_BIT;
@@ -154,7 +172,15 @@ static void vgic_v3_enable(struct kvm_vcpu *vcpu)
 	 */
 	vgic_v3->vgic_vmcr = 0;
 
-	vgic_v3->vgic_sre = 0;
+	/*
+	 * If we are emulating a GICv3, we do it in an non-GICv2-compatible
+	 * way, so we force SRE to 1 to demonstrate this to the guest.
+	 * This goes with the spec allowing the value to be RAO/WI.
+	 */
+	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+		vgic_v3->vgic_sre = ICC_SRE_EL1_SRE;
+	else
+		vgic_v3->vgic_sre = 0;
 
 	/* Get the show on the road... */
 	vgic_v3->vgic_hcr = ICH_HCR_EN;
@@ -215,28 +241,30 @@ int vgic_v3_probe(struct device_node *vgic_node,
 
 	gicv_idx += 3; /* Also skip GICD, GICC, GICH */
 	if (of_address_to_resource(vgic_node, gicv_idx, &vcpu_res)) {
-		kvm_err("Cannot obtain GICV region\n");
-		ret = -ENXIO;
-		goto out;
-	}
-
-	if (!PAGE_ALIGNED(vcpu_res.start)) {
-		kvm_err("GICV physical address 0x%llx not page aligned\n",
-			(unsigned long long)vcpu_res.start);
-		ret = -ENXIO;
-		goto out;
-	}
-
-	if (!PAGE_ALIGNED(resource_size(&vcpu_res))) {
-		kvm_err("GICV size 0x%llx not a multiple of page size 0x%lx\n",
-			(unsigned long long)resource_size(&vcpu_res),
-			PAGE_SIZE);
-		ret = -ENXIO;
-		goto out;
+		kvm_info("GICv3: GICv2 emulation not available\n");
+		vgic->vcpu_base = 0;
+	} else {
+		if (!PAGE_ALIGNED(vcpu_res.start)) {
+			kvm_err("GICV physical address 0x%llx not page aligned\n",
+				(unsigned long long)vcpu_res.start);
+			ret = -ENXIO;

virt/kvm/arm/vgic-v3.c int vgic_v3_probe(struct device_node *vgic_node, · line 260

Quoted 10 lines
virt/kvm/arm/vgic-v3.c +10 -0
+			goto out;
+		}
+
+		if (!PAGE_ALIGNED(resource_size(&vcpu_res))) {
+			kvm_err("GICV size 0x%llx not a multiple of page size 0x%lx\n",
+				(unsigned long long)resource_size(&vcpu_res),
+				PAGE_SIZE);
+			ret = -ENXIO;
+			goto out;
+		}

I still think this is wrong. Why whouldn't a user create be able to create a virtual GICv3 with the system register interface just because the GICv2 addresss are not aligned correctly?

Quoted 4 lines

Unless someone has a good technical argument for why something is likely to break, we should aim for supporting as many platforms as possible, so please change the error outs here to warnings and proceed with registering the v3 ops.

OK, I changed this as suggested.

I had some trouble with rebasing and testing on top of v3.19-rc, so v6 couldn't hit the list still last year. But expect it there anytime soon.

Happy New Year!
Andre.

#40 lore raw

PATCH v5 19/19 arm/arm64: KVM: allow userland to request a virtual GICv3

In reply to Andre Przywara

With all of the GICv3 code in place now we allow userland to ask the kernel for using a virtual GICv3 in the guest.
Also we provide the necessary support for guests setting the memory addresses for the virtual distributor and redistributors. This requires some userland code to make use of that feature and explicitly ask for a virtual GICv3.

---
Changelog v4...v5:
- improving documentation text

Changelog v3...v4:
- refine commit message
- add documentation of new GICv3 KVM device

 Documentation/virtual/kvm/devices/arm-vgic.txt |   22 ++++++++++--
 arch/arm64/include/uapi/asm/kvm.h              |    7 ++++
 include/kvm/arm_vgic.h                         |    4 +--
 virt/kvm/arm/vgic-v3-emul.c                    |    3 ++
 virt/kvm/arm/vgic.c                            |   46 +++++++++++++++++-------
 5 files changed, 65 insertions(+), 17 deletions(-)
Documentation/virtual/kvm/devices/arm-vgic.txt +20 -2
index df8b0c7..563463e 100644
@@ -3,22 +3,38 @@ ARM Virtual Generic Interrupt Controller (VGIC)
 
 Device types supported:
   KVM_DEV_TYPE_ARM_VGIC_V2     ARM Generic Interrupt Controller v2.0
+  KVM_DEV_TYPE_ARM_VGIC_V3     ARM Generic Interrupt Controller v3.0
 
 Only one VGIC instance may be instantiated through either this API or the
 legacy KVM_CREATE_IRQCHIP api.  The created VGIC will act as the VM interrupt
 controller, requiring emulated user-space devices to inject interrupts to the
 VGIC instead of directly to CPUs.
 
+Creating a guest GICv3 device requires a host GICv3 as well.
+GICv3 implementations with hardware compatibility support allow a guest GICv2
+as well.
+
 Groups:
   KVM_DEV_ARM_VGIC_GRP_ADDR
   Attributes:
     KVM_VGIC_V2_ADDR_TYPE_DIST (rw, 64-bit)
       Base address in the guest physical address space of the GIC distributor
-      register mappings.
+      register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V2.
 
     KVM_VGIC_V2_ADDR_TYPE_CPU (rw, 64-bit)
       Base address in the guest physical address space of the GIC virtual cpu
-      interface register mappings.
+      interface register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V2.
+
+    KVM_VGIC_V3_ADDR_TYPE_DIST (rw, 64-bit)
+      Base address in the guest physical address space of the GICv3 distributor
+      register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
+
+    KVM_VGIC_V3_ADDR_TYPE_REDIST (rw, 64-bit)
+      Base address in the guest physical address space of the GICv3
+      redistributor register mappings. There are two 64K pages for each
+      VCPU and all of the redistributor pages are contiguous.
+      Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
+
 
   KVM_DEV_ARM_VGIC_GRP_DIST_REGS
   Attributes:
@@ -36,6 +52,7 @@ Groups:
     the register.
   Limitations:
     - Priorities are not implemented, and registers are RAZ/WI
+    - Currently only implemented for KVM_DEV_TYPE_ARM_VGIC_V2.
   Errors:
     -ENODEV: Getting or setting this register is not yet supported
     -EBUSY: One or more VCPUs are running
@@ -68,6 +85,7 @@ Groups:
 
   Limitations:
     - Priorities are not implemented, and registers are RAZ/WI
+    - Currently only implemented for KVM_DEV_TYPE_ARM_VGIC_V2.
   Errors:
     -ENODEV: Getting or setting this register is not yet supported
     -EBUSY: One or more VCPUs are running
arch/arm64/include/uapi/asm/kvm.h +7 -0
index 8e38878..2ed873a 100644
@@ -78,6 +78,13 @@ struct kvm_regs {
 #define KVM_VGIC_V2_DIST_SIZE		0x1000
 #define KVM_VGIC_V2_CPU_SIZE		0x2000
 
+/* Supported VGICv3 address types  */
+#define KVM_VGIC_V3_ADDR_TYPE_DIST	2
+#define KVM_VGIC_V3_ADDR_TYPE_REDIST	3
+
+#define KVM_VGIC_V3_DIST_SIZE		SZ_64K
+#define KVM_VGIC_V3_REDIST_SIZE		(2 * SZ_64K)
+
 #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
 #define KVM_ARM_VCPU_EL1_32BIT		1 /* CPU running a 32bit VM */
 #define KVM_ARM_VCPU_PSCI_0_2		2 /* CPU uses PSCI v0.2 */
include/kvm/arm_vgic.h +2 -2
index b8dc144..655d0fa 100644
@@ -36,8 +36,8 @@
 #define VGIC_V2_MAX_CPUS	8
 
 /* Sanity checks... */
-#if (KVM_MAX_VCPUS > 8)
-#error	Invalid number of CPU interfaces
+#if (KVM_MAX_VCPUS > 255)
+#error Too many KVM VCPUs, the VGIC only supports up to 255 VCPUs for now
 #endif
 
 #if (VGIC_NR_IRQS_LEGACY & 31)
virt/kvm/arm/vgic-v3-emul.c +3 -0
index ff0126d..ef57c6d 100644
@@ -989,6 +989,9 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
 		case KVM_VGIC_V2_ADDR_TYPE_DIST:
 		case KVM_VGIC_V2_ADDR_TYPE_CPU:
 			return -ENXIO;
+		case KVM_VGIC_V3_ADDR_TYPE_DIST:
+		case KVM_VGIC_V3_ADDR_TYPE_REDIST:
+			return 0;
 		}
 		break;
 	case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
virt/kvm/arm/vgic.c +33 -13

69 lines not on this page · raw message

Signature · 1 line
1.7.9.5
#41 lore raw

PATCH v5 19/19 arm/arm64: KVM: allow userland to request a virtual GICv3

Quoted 49 lines · Andre Przywara

With all of the GICv3 code in place now we allow userland to ask the kernel for using a virtual GICv3 in the guest.
Also we provide the necessary support for guests setting the memory addresses for the virtual distributor and redistributors. This requires some userland code to make use of that feature and explicitly ask for a virtual GICv3.

---
Changelog v4...v5:
- improving documentation text

Changelog v3...v4:
- refine commit message
- add documentation of new GICv3 KVM device

 Documentation/virtual/kvm/devices/arm-vgic.txt |   22 ++++++++++--
 arch/arm64/include/uapi/asm/kvm.h              |    7 ++++
 include/kvm/arm_vgic.h                         |    4 +--
 virt/kvm/arm/vgic-v3-emul.c                    |    3 ++
 virt/kvm/arm/vgic.c                            |   46 +++++++++++++++++-------
 5 files changed, 65 insertions(+), 17 deletions(-)
Documentation/virtual/kvm/devices/arm-vgic.txt +6 -1
index df8b0c7..563463e 100644
@@ -3,22 +3,38 @@ ARM Virtual Generic Interrupt Controller (VGIC)
 
 Device types supported:
   KVM_DEV_TYPE_ARM_VGIC_V2     ARM Generic Interrupt Controller v2.0
+  KVM_DEV_TYPE_ARM_VGIC_V3     ARM Generic Interrupt Controller v3.0
 
 Only one VGIC instance may be instantiated through either this API or the
 legacy KVM_CREATE_IRQCHIP api.  The created VGIC will act as the VM interrupt
 controller, requiring emulated user-space devices to inject interrupts to the
 VGIC instead of directly to CPUs.
 
+Creating a guest GICv3 device requires a host GICv3 as well.
+GICv3 implementations with hardware compatibility support allow a guest GICv2
+as well.
+
 Groups:
   KVM_DEV_ARM_VGIC_GRP_ADDR
   Attributes:
     KVM_VGIC_V2_ADDR_TYPE_DIST (rw, 64-bit)
       Base address in the guest physical address space of the GIC distributor
-      register mappings.
+      register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V2.

Documentation/virtual/kvm/devices/arm-vgic.txt ARM Virtual Generic Interrupt Controller (VGIC) · line 31

Quoted 10 lines
Documentation/virtual/kvm/devices/arm-vgic.txt +6 -1
 
     KVM_VGIC_V2_ADDR_TYPE_CPU (rw, 64-bit)
       Base address in the guest physical address space of the GIC virtual cpu
-      interface register mappings.
+      interface register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V2.
+
+    KVM_VGIC_V3_ADDR_TYPE_DIST (rw, 64-bit)
+      Base address in the guest physical address space of the GICv3 distributor
+      register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
+

Are there no alignment restrictions to worry about that we should document here?

-Christoffer

#42 lore raw

PATCH v5 19/19 arm/arm64: KVM: allow userland to request a virtual GICv3

Hi Christoffer,

Quoted 51 lines · Christoffer Dall
Quoted 50 lines · Andre Przywara

With all of the GICv3 code in place now we allow userland to ask the kernel for using a virtual GICv3 in the guest.
Also we provide the necessary support for guests setting the memory addresses for the virtual distributor and redistributors. This requires some userland code to make use of that feature and explicitly ask for a virtual GICv3.

---
Changelog v4...v5:
- improving documentation text

Changelog v3...v4:
- refine commit message
- add documentation of new GICv3 KVM device

 Documentation/virtual/kvm/devices/arm-vgic.txt |   22 ++++++++++--
 arch/arm64/include/uapi/asm/kvm.h              |    7 ++++
 include/kvm/arm_vgic.h                         |    4 +--
 virt/kvm/arm/vgic-v3-emul.c                    |    3 ++
 virt/kvm/arm/vgic.c                            |   46 +++++++++++++++++-------
 5 files changed, 65 insertions(+), 17 deletions(-)
Documentation/virtual/kvm/devices/arm-vgic.txt +6 -1
index df8b0c7..563463e 100644
@@ -3,22 +3,38 @@ ARM Virtual Generic Interrupt Controller (VGIC)
 
 Device types supported:
   KVM_DEV_TYPE_ARM_VGIC_V2     ARM Generic Interrupt Controller v2.0
+  KVM_DEV_TYPE_ARM_VGIC_V3     ARM Generic Interrupt Controller v3.0
 
 Only one VGIC instance may be instantiated through either this API or the
 legacy KVM_CREATE_IRQCHIP api.  The created VGIC will act as the VM interrupt
 controller, requiring emulated user-space devices to inject interrupts to the
 VGIC instead of directly to CPUs.
 
+Creating a guest GICv3 device requires a host GICv3 as well.
+GICv3 implementations with hardware compatibility support allow a guest GICv2
+as well.
+
 Groups:
   KVM_DEV_ARM_VGIC_GRP_ADDR
   Attributes:
     KVM_VGIC_V2_ADDR_TYPE_DIST (rw, 64-bit)
       Base address in the guest physical address space of the GIC distributor
-      register mappings.
+      register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V2.
 
Quoted 12 lines

Documentation/virtual/kvm/devices/arm-vgic.txt ARM Virtual Generic Interrupt Controller (VGIC) · line 31

Quoted 9 lines
Documentation/virtual/kvm/devices/arm-vgic.txt +6 -1
     KVM_VGIC_V2_ADDR_TYPE_CPU (rw, 64-bit)
       Base address in the guest physical address space of the GIC virtual cpu
-      interface register mappings.
+      interface register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V2.
+
+    KVM_VGIC_V3_ADDR_TYPE_DIST (rw, 64-bit)
+      Base address in the guest physical address space of the GICv3 distributor
+      register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
+

Are there no alignment restrictions to worry about that we should document here?

I think so, but that is not GICv3 specific, but applies to the VGICv2 also, right? So shall I add a sentence like
"The base address needs to be aligned to a 64 KB page boundary." for GICv3 and
"The base address needs to be aligned to a 4 KB page boundary." for GICv2?
Shouldn't the 64K alignment actually apply for GICv2 on AArch64 also? That would avoid some trouble in the future, wouldn't it?

Also I think we need to actually check for the 64K alignment for GICv3 (we only check for 4K atm).

Cheers,
Andre.

#43 lore raw

PATCH v5 19/19 arm/arm64: KVM: allow userland to request a virtual GICv3

Quoted 76 lines · Andre Przywara

Hi Christoffer,

Quoted 51 lines · Christoffer Dall
Quoted 50 lines · Andre Przywara

With all of the GICv3 code in place now we allow userland to ask the kernel for using a virtual GICv3 in the guest.
Also we provide the necessary support for guests setting the memory addresses for the virtual distributor and redistributors. This requires some userland code to make use of that feature and explicitly ask for a virtual GICv3.

---
Changelog v4...v5:
- improving documentation text

Changelog v3...v4:
- refine commit message
- add documentation of new GICv3 KVM device

 Documentation/virtual/kvm/devices/arm-vgic.txt |   22 ++++++++++--
 arch/arm64/include/uapi/asm/kvm.h              |    7 ++++
 include/kvm/arm_vgic.h                         |    4 +--
 virt/kvm/arm/vgic-v3-emul.c                    |    3 ++
 virt/kvm/arm/vgic.c                            |   46 +++++++++++++++++-------
 5 files changed, 65 insertions(+), 17 deletions(-)
Documentation/virtual/kvm/devices/arm-vgic.txt +6 -1
index df8b0c7..563463e 100644
@@ -3,22 +3,38 @@ ARM Virtual Generic Interrupt Controller (VGIC)
 
 Device types supported:
   KVM_DEV_TYPE_ARM_VGIC_V2     ARM Generic Interrupt Controller v2.0
+  KVM_DEV_TYPE_ARM_VGIC_V3     ARM Generic Interrupt Controller v3.0
 
 Only one VGIC instance may be instantiated through either this API or the
 legacy KVM_CREATE_IRQCHIP api.  The created VGIC will act as the VM interrupt
 controller, requiring emulated user-space devices to inject interrupts to the
 VGIC instead of directly to CPUs.
 
+Creating a guest GICv3 device requires a host GICv3 as well.
+GICv3 implementations with hardware compatibility support allow a guest GICv2
+as well.
+
 Groups:
   KVM_DEV_ARM_VGIC_GRP_ADDR
   Attributes:
     KVM_VGIC_V2_ADDR_TYPE_DIST (rw, 64-bit)
       Base address in the guest physical address space of the GIC distributor
-      register mappings.
+      register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V2.
 
Quoted 12 lines

Documentation/virtual/kvm/devices/arm-vgic.txt ARM Virtual Generic Interrupt Controller (VGIC) · line 31

Quoted 9 lines
Documentation/virtual/kvm/devices/arm-vgic.txt +6 -1
     KVM_VGIC_V2_ADDR_TYPE_CPU (rw, 64-bit)
       Base address in the guest physical address space of the GIC virtual cpu
-      interface register mappings.
+      interface register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V2.
+
+    KVM_VGIC_V3_ADDR_TYPE_DIST (rw, 64-bit)
+      Base address in the guest physical address space of the GICv3 distributor
+      register mappings. Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
+

Are there no alignment restrictions to worry about that we should document here?

I think so, but that is not GICv3 specific, but applies to the VGICv2 also, right? So shall I add a sentence like
"The base address needs to be aligned to a 64 KB page boundary." for GICv3 and
"The base address needs to be aligned to a 4 KB page boundary." for GICv2?
Shouldn't the 64K alignment actually apply for GICv2 on AArch64 also? That would avoid some trouble in the future, wouldn't it?

Quoted 3 lines

Also I think we need to actually check for the 64K alignment for GICv3 (we only check for 4K atm).

Yeah, I think we should clarify this and check that the code does what we expect.

Note that the CPU interface needs to be aligned according the same offset as the hardware puts that GICV if not on page boundaries etc. Marc had a patch to also inform userspace about this, which may be better suited for a follow-up later on, but I feel like the distributor allocation stuff should be a simple thing to check and get right for at least GICv3 now, and if we also introduce some safety for GICv2 at the same time, even better.

Thanks,
-Christoffer

#44 lore raw

v5 cover KVM GICv3 emulation

In reply to Andre Przywara

Hi Andre,

Quoted 20 lines · Andre Przywara

This is version 5 of the GICv3 guest emulation series (not for 3.19).

As the changes this time were much smaller, I updated to tree to 3.18.0, as it includes some bug fixes in the VGIC.

I addressed the remaining comments from Christoffer and Eric, thanks for the review! The changes this times were much smaller, most of them cosmetic or rewordings of commit messages and comments.
I updated the kvm-gicv3/v4 branch in my repo[1] to carry all the delta patches. Those patches are just for reference to see what has changed between v4 and v5. For review and all other purposes please use the v5 branch.

For a changelog summary see below, also each patch carries a changelog now.
Only patches 05, 08, 12, 15, 17, 18 and 19 have been changed compared to v4. I dropped Christoffer's Reviewed-by: tag from 05/19 because of the newly added function, but added the respective tags to the other commit messages.

Quoted 4 lines

I quickly tested this version with a GICv3 capable fast model in all endianness modes (LE guest on LE host, BE on LE, LE on BE, BE on BE). Both a GICv2 and a GICv3 guest were booted in all four combinations.

So this is overall looking like it's getting ready to be merged for v3.20.

However, here are the things we need resolved before putting it into kvmarm/next:

1. You need to address the few remaining comments I had on this version.

2. You must rebase on the vgic_init series and the vcpu_nit series. Both are in kvmarm/queue, so I suggest you rebase on that or wait until both of those series hit kvm/next (later next week). You also need to base this on Eric's VGIC init ioctl series
(https://lists.cs.columbia.edu/pipermail/kvmarm/2014-December/012643.html),
but with dropping patch 3. As soon as Marc has taken a look at that series, I will merge it onto queue as well.
This is likely going to be a pain (sorry about that) since the whole init/init_maps sequence has changed.

3. Get rid of any on-demand calls to vgic_init() for GICv3. For GICv3, the only valid call to vgic_init() should be from the new device control ioctl, and all other paths that rely on vgic_init() must fail.

4. Resubmit a new (and hopefully final) version of the series soon after the merge window closes.

Then we'll queue this in kvmarm/next early so that we have time to test it and expose it to a wider audience.

Thanks,
-Christoffer

#45 lore raw

v5 cover KVM GICv3 emulation

Hej Christoffer,

Quoted 50 lines · Christoffer Dall

Hi Andre,

Quoted 20 lines · Andre Przywara

This is version 5 of the GICv3 guest emulation series (not for 3.19).

As the changes this time were much smaller, I updated to tree to 3.18.0, as it includes some bug fixes in the VGIC.

I addressed the remaining comments from Christoffer and Eric, thanks for the review! The changes this times were much smaller, most of them cosmetic or rewordings of commit messages and comments.
I updated the kvm-gicv3/v4 branch in my repo[1] to carry all the delta patches. Those patches are just for reference to see what has changed between v4 and v5. For review and all other purposes please use the v5 branch.

For a changelog summary see below, also each patch carries a changelog now.
Only patches 05, 08, 12, 15, 17, 18 and 19 have been changed compared to v4. I dropped Christoffer's Reviewed-by: tag from 05/19 because of the newly added function, but added the respective tags to the other commit messages.

Quoted 4 lines

I quickly tested this version with a GICv3 capable fast model in all endianness modes (LE guest on LE host, BE on LE, LE on BE, BE on BE). Both a GICv2 and a GICv3 guest were booted in all four combinations.

So this is overall looking like it's getting ready to be merged for v3.20.

However, here are the things we need resolved before putting it into kvmarm/next:

1. You need to address the few remaining comments I had on this version.

2. You must rebase on the vgic_init series and the vcpu_nit series. Both are in kvmarm/queue, so I suggest you rebase on that or wait until both of those series hit kvm/next (later next week). You also need to base this on Eric's VGIC init ioctl series
(https://lists.cs.columbia.edu/pipermail/kvmarm/2014-December/012643.html),
but with dropping patch 3. As soon as Marc has taken a look at that series, I will merge it onto queue as well.
This is likely going to be a pain (sorry about that) since the whole init/init_maps sequence has changed.

3. Get rid of any on-demand calls to vgic_init() for GICv3. For GICv3, the only valid call to vgic_init() should be from the new device control ioctl, and all other paths that rely on vgic_init() must fail.

Quoted 5 lines

4. Resubmit a new (and hopefully final) version of the series soon after the merge window closes.

Then we'll queue this in kvmarm/next early so that we have time to test it and expose it to a wider audience.

Alright, thanks for that, sounds like a plan. I will try my best to do the rebasing this week still, so that the new series arrives as early as possible after -rc1.

Cheers,
Andre.