[PATCH v2 0/9] arm/arm64: KVM: dynamic VGIC sizing

[PATCH v2 0/9] arm/arm64: KVM: dynamic VGIC sizing

[Marc Zyngier]

So far, the VGIC data structures have been statically sized, meaning
that we always have to support more interrupts than we actually want,
and more CPU interfaces than we should. This is a waste of resource,
and is the kind of things that should be tuneable.

This series addresses that issue by changing the data structures to be
dynamically allocated, and adds a new configuration attribute to
allocate the number of interrupts. When the attribute is not used, we
fallback to the old behaviour of allocating a fixed number of
interrupts.

The last patch of the series is a bit out of context, but tends to fit
well here code-wise. It solves an interesting issue having to do with
the placement of the GICV interface in Stage-2 when using 64k pages
(if the HW is not 64k aligned, we need to tell userspace about the
"sub-page offset" so it can correctly place the guest's GICC region).

This series is also the base for Andre Przywara's GICv3 distributor
emulation code (which can support far more than 8 vcpus and 1020
interrupts).

This has been tested on both ARM (TC2) and arm64 (model).

Marc Zyngier (9):
  KVM: ARM: vgic: plug irq injection race
  arm/arm64: KVM: vgic: switch to dynamic allocation
  arm/arm64: KVM: vgic: Parametrize VGIC_NR_SHARED_IRQS
  arm/arm64: KVM: vgic: kill VGIC_MAX_CPUS
  arm/arm64: KVM: vgic: handle out-of-range MMIO accesses
  arm/arm64: KVM: vgic: kill VGIC_NR_IRQS
  arm/arm64: KVM: vgic: delay vgic allocation until init time
  arm/arm64: KVM: vgic: make number of irqs a configurable attribute
  arm64: KVM: vgic: deal with GIC sub-page alignment

 arch/arm/include/uapi/asm/kvm.h   |   2 +
 arch/arm/kvm/arm.c                |  10 +-
 arch/arm64/include/uapi/asm/kvm.h |   2 +
 include/kvm/arm_vgic.h            |  54 +++---
 virt/kvm/arm/vgic.c               | 357 ++++++++++++++++++++++++++++++++------
 5 files changed, 339 insertions(+), 86 deletions(-)

-- 
1.8.3.4

[PATCH v2 0/9] arm/arm64: KVM: dynamic VGIC sizing

[Marc Zyngier]

So far, the VGIC data structures have been statically sized, meaning
that we always have to support more interrupts than we actually want,
and more CPU interfaces than we should. This is a waste of resource,
and is the kind of things that should be tuneable.

This series addresses that issue by changing the data structures to be
dynamically allocated, and adds a new configuration attribute to
allocate the number of interrupts. When the attribute is not used, we
fallback to the old behaviour of allocating a fixed number of
interrupts.

The last patch of the series is a bit out of context, but tends to fit
well here code-wise. It solves an interesting issue having to do with
the placement of the GICV interface in Stage-2 when using 64k pages
(if the HW is not 64k aligned, we need to tell userspace about the
"sub-page offset" so it can correctly place the guest's GICC region).

This series is also the base for Andre Przywara's GICv3 distributor
emulation code (which can support far more than 8 vcpus and 1020
interrupts).

This has been tested on both ARM (TC2) and arm64 (model).

Marc Zyngier (9):
  KVM: ARM: vgic: plug irq injection race
  arm/arm64: KVM: vgic: switch to dynamic allocation
  arm/arm64: KVM: vgic: Parametrize VGIC_NR_SHARED_IRQS
  arm/arm64: KVM: vgic: kill VGIC_MAX_CPUS
  arm/arm64: KVM: vgic: handle out-of-range MMIO accesses
  arm/arm64: KVM: vgic: kill VGIC_NR_IRQS
  arm/arm64: KVM: vgic: delay vgic allocation until init time
  arm/arm64: KVM: vgic: make number of irqs a configurable attribute
  arm64: KVM: vgic: deal with GIC sub-page alignment

 arch/arm/include/uapi/asm/kvm.h   |   2 +
 arch/arm/kvm/arm.c                |  10 +-
 arch/arm64/include/uapi/asm/kvm.h |   2 +
 include/kvm/arm_vgic.h            |  54 +++---
 virt/kvm/arm/vgic.c               | 357 ++++++++++++++++++++++++++++++++------
 5 files changed, 339 insertions(+), 86 deletions(-)

-- 
1.8.3.4

[PATCH v2 1/9] KVM: ARM: vgic: plug irq injection race

[Marc Zyngier]

As it stands, nothing prevents userspace from injecting an interrupt
before the guest's GIC is actually initialized.

This goes unnoticed so far (as everything is pretty much statically
allocated), but ends up exploding in a spectacular way once we switch
to a more dynamic allocation (the GIC data structure isn't there yet).

The fix is to test for the "ready" flag in the VGIC distributor before
trying to inject the interrupt. Note that in order to avoid breaking
userspace, we have to ignore what is essentially an error.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 virt/kvm/arm/vgic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 795ab48..c6da748 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1446,7 +1446,8 @@ out:
 int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
 			bool level)
 {
-	if (vgic_update_irq_state(kvm, cpuid, irq_num, level))
+	if (likely(vgic_initialized(kvm)) &&
+	    vgic_update_irq_state(kvm, cpuid, irq_num, level))
 		vgic_kick_vcpus(kvm);
 
 	return 0;
-- 
1.8.3.4

[PATCH v2 1/9] KVM: ARM: vgic: plug irq injection race

[Marc Zyngier]

As it stands, nothing prevents userspace from injecting an interrupt
before the guest's GIC is actually initialized.

This goes unnoticed so far (as everything is pretty much statically
allocated), but ends up exploding in a spectacular way once we switch
to a more dynamic allocation (the GIC data structure isn't there yet).

The fix is to test for the "ready" flag in the VGIC distributor before
trying to inject the interrupt. Note that in order to avoid breaking
userspace, we have to ignore what is essentially an error.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 virt/kvm/arm/vgic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 795ab48..c6da748 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1446,7 +1446,8 @@ out:
 int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
 			bool level)
 {
-	if (vgic_update_irq_state(kvm, cpuid, irq_num, level))
+	if (likely(vgic_initialized(kvm)) &&
+	    vgic_update_irq_state(kvm, cpuid, irq_num, level))
 		vgic_kick_vcpus(kvm);
 
 	return 0;
-- 
1.8.3.4

[PATCH v2 2/9] arm/arm64: KVM: vgic: switch to dynamic allocation

[Marc Zyngier]

So far, all the VGIC data structures are statically defined by the
*maximum* number of vcpus and interrupts it supports. It means that
we always have to oversize it to cater for the worse case.

Start by changing the data structures to be dynamically sizeable,
and allocate them at runtime.

The sizes are still very static though.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/arm.c     |   5 +-
 include/kvm/arm_vgic.h |  44 ++++++-----
 virt/kvm/arm/vgic.c    | 203 ++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 206 insertions(+), 46 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 3c82b37..9548bc9 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -182,6 +182,8 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 			kvm->vcpus[i] = NULL;
 		}
 	}
+
+	kvm_vgic_destroy(kvm);
 }
 
 int kvm_dev_ioctl_check_extension(long ext)
@@ -290,6 +292,7 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
 {
 	kvm_mmu_free_memory_caches(vcpu);
 	kvm_timer_vcpu_terminate(vcpu);
+	kvm_vgic_vcpu_destroy(vcpu);
 	kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
@@ -808,7 +811,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, VGIC_MAX_CPUS, VGIC_NR_IRQS);
 		else
 			return -ENXIO;
 	}
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 35b0c12..c57ffd0 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -54,19 +54,24 @@
  * - a bunch of shared interrupts (SPI)
  */
 struct vgic_bitmap {
-	union {
-		u32 reg[VGIC_NR_PRIVATE_IRQS / 32];
-		DECLARE_BITMAP(reg_ul, VGIC_NR_PRIVATE_IRQS);
-	} percpu[VGIC_MAX_CPUS];
-	union {
-		u32 reg[VGIC_NR_SHARED_IRQS / 32];
-		DECLARE_BITMAP(reg_ul, VGIC_NR_SHARED_IRQS);
-	} shared;
+	/*
+	 * - One UL per VCPU for private interrupts (assumes UL is at
+	 * least 32 bits)
+	 * - As many UL as necessary for shared interrupts.
+	 */
+	int nr_cpus;
+	unsigned long *private;
+	unsigned long *shared;
 };
 
 struct vgic_bytemap {
-	u32 percpu[VGIC_MAX_CPUS][VGIC_NR_PRIVATE_IRQS / 4];
-	u32 shared[VGIC_NR_SHARED_IRQS  / 4];
+	/*
+	 * - 8 u32 per VCPU for private interrupts
+	 * - As many u32 as necessary for shared interrupts.
+	 */
+	int nr_cpus;
+	u32 *private;
+	u32 *shared;
 };
 
 struct kvm_vcpu;
@@ -127,6 +132,9 @@ struct vgic_dist {
 	bool			in_kernel;
 	bool			ready;
 
+	int			nr_cpus;
+	int			nr_irqs;
+
 	/* Virtual control interface mapping */
 	void __iomem		*vctrl_base;
 
@@ -152,12 +160,12 @@ struct vgic_dist {
 	/* Level/edge triggered */
 	struct vgic_bitmap	irq_cfg;
 
-	/* Source CPU per SGI and target CPU */
-	u8			irq_sgi_sources[VGIC_MAX_CPUS][VGIC_NR_SGIS];
+	/* Source CPU per SGI and target CPU : 16 bytes per CPU */
+	u8			*irq_sgi_sources;
 
 	/* Target CPU for each IRQ */
-	u8			irq_spi_cpu[VGIC_NR_SHARED_IRQS];
-	struct vgic_bitmap	irq_spi_target[VGIC_MAX_CPUS];
+	u8			*irq_spi_cpu;
+	struct vgic_bitmap	*irq_spi_target;
 
 	/* Bitmap indicating which CPU has something pending */
 	unsigned long		irq_pending_on_cpu;
@@ -190,11 +198,11 @@ struct vgic_v3_cpu_if {
 struct vgic_cpu {
 #ifdef CONFIG_KVM_ARM_VGIC
 	/* per IRQ to LR mapping */
-	u8		vgic_irq_lr_map[VGIC_NR_IRQS];
+	u8		*vgic_irq_lr_map;
 
 	/* Pending interrupts on this VCPU */
 	DECLARE_BITMAP(	pending_percpu, VGIC_NR_PRIVATE_IRQS);
-	DECLARE_BITMAP(	pending_shared, VGIC_NR_SHARED_IRQS);
+	unsigned long	*pending_shared;
 
 	/* Bitmap of used/free list registers */
 	DECLARE_BITMAP(	lr_used, VGIC_V2_MAX_LRS);
@@ -224,8 +232,10 @@ 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, int nr_cpus, int nr_irqs);
+void kvm_vgic_destroy(struct kvm *kvm);
 int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu);
+void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu);
 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,
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index c6da748..220b215 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -89,6 +89,7 @@ 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);
@@ -98,23 +99,44 @@ 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;
 
+static int vgic_init_bitmap(struct vgic_bitmap *b, int nr_cpus, int nr_irqs)
+{
+	int nr_longs;
+
+	nr_longs = nr_cpus + BITS_TO_LONGS(nr_irqs - VGIC_NR_PRIVATE_IRQS);
+
+	b->private = kzalloc(sizeof(unsigned long) * nr_longs, GFP_KERNEL);
+	if (!b->private)
+		return -ENOMEM;
+
+	b->shared = b->private + nr_cpus;
+
+	b->nr_cpus = nr_cpus;
+	return 0;
+}
+
+static void vgic_free_bitmap(struct vgic_bitmap *b)
+{
+	kfree(b->private);
+}
+
 static u32 *vgic_bitmap_get_reg(struct vgic_bitmap *x,
 				int cpuid, u32 offset)
 {
 	offset >>= 2;
 	if (!offset)
-		return x->percpu[cpuid].reg;
+		return (u32 *)(x->private + cpuid);
 	else
-		return x->shared.reg + offset - 1;
+		return (u32 *)(x->shared) + offset - 1;
 }
 
 static int vgic_bitmap_get_irq_val(struct vgic_bitmap *x,
 				   int cpuid, int irq)
 {
 	if (irq < VGIC_NR_PRIVATE_IRQS)
-		return test_bit(irq, x->percpu[cpuid].reg_ul);
+		return test_bit(irq, x->private + cpuid);
 
-	return test_bit(irq - VGIC_NR_PRIVATE_IRQS, x->shared.reg_ul);
+	return test_bit(irq - VGIC_NR_PRIVATE_IRQS, x->shared);
 }
 
 static void vgic_bitmap_set_irq_val(struct vgic_bitmap *x, int cpuid,
@@ -123,9 +145,9 @@ static void vgic_bitmap_set_irq_val(struct vgic_bitmap *x, int cpuid,
 	unsigned long *reg;
 
 	if (irq < VGIC_NR_PRIVATE_IRQS) {
-		reg = x->percpu[cpuid].reg_ul;
+		reg = x->private + cpuid;
 	} else {
-		reg =  x->shared.reg_ul;
+		reg = x->shared;
 		irq -= VGIC_NR_PRIVATE_IRQS;
 	}
 
@@ -137,24 +159,48 @@ static void vgic_bitmap_set_irq_val(struct vgic_bitmap *x, int cpuid,
 
 static unsigned long *vgic_bitmap_get_cpu_map(struct vgic_bitmap *x, int cpuid)
 {
-	if (unlikely(cpuid >= VGIC_MAX_CPUS))
-		return NULL;
-	return x->percpu[cpuid].reg_ul;
+	return x->private + cpuid;
 }
 
 static unsigned long *vgic_bitmap_get_shared_map(struct vgic_bitmap *x)
 {
-	return x->shared.reg_ul;
+	return x->shared;
+}
+
+static int vgic_init_bytemap(struct vgic_bytemap *x, int nr_cpus, int nr_irqs)
+{
+	int size;
+
+	size  = nr_cpus * VGIC_NR_PRIVATE_IRQS;
+	size += nr_irqs - VGIC_NR_PRIVATE_IRQS;
+
+	x->private = kzalloc(size, GFP_KERNEL);
+	if (!x->private)
+		return -ENOMEM;
+
+	x->shared = x->private + nr_cpus * VGIC_NR_PRIVATE_IRQS / sizeof(u32);
+	x->nr_cpus = nr_cpus;
+	return 0;
+}
+
+static void vgic_free_bytemap(struct vgic_bytemap *b)
+{
+	kfree(b->private);
 }
 
 static u32 *vgic_bytemap_get_reg(struct vgic_bytemap *x, int cpuid, u32 offset)
 {
-	offset >>= 2;
-	BUG_ON(offset > (VGIC_NR_IRQS / 4));
-	if (offset < 8)
-		return x->percpu[cpuid] + offset;
-	else
-		return x->shared + offset - 8;
+	u32 *reg;
+
+	if (offset < 32) {
+		reg = x->private;
+		offset += cpuid * VGIC_NR_PRIVATE_IRQS;
+	} else {
+		reg = x->shared;
+		offset -= VGIC_NR_PRIVATE_IRQS;
+	}
+
+	return reg + (offset / sizeof(u32));
 }
 
 #define VGIC_CFG_LEVEL	0
@@ -633,7 +679,7 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
 		 */
 		vgic_dist_irq_set(vcpu, lr.irq);
 		if (lr.irq < VGIC_NR_SGIS)
-			dist->irq_sgi_sources[vcpu_id][lr.irq] |= 1 << lr.source;
+			*vgic_get_sgi_sources(dist, vcpu_id, lr.irq) |= 1 << lr.source;
 		lr.state &= ~LR_STATE_PENDING;
 		vgic_set_lr(vcpu, i, lr);
 
@@ -665,7 +711,7 @@ static bool read_set_clear_sgi_pend_reg(struct kvm_vcpu *vcpu,
 	/* Copy source SGIs from distributor side */
 	for (sgi = min_sgi; sgi <= max_sgi; sgi++) {
 		int shift = 8 * (sgi - min_sgi);
-		reg |= (u32)dist->irq_sgi_sources[vcpu_id][sgi] << shift;
+		reg |= ((u32)*vgic_get_sgi_sources(dist, vcpu_id, sgi)) << shift;
 	}
 
 	mmio_data_write(mmio, ~0, reg);
@@ -689,14 +735,15 @@ static bool write_set_clear_sgi_pend_reg(struct kvm_vcpu *vcpu,
 	/* Clear pending SGIs on the distributor */
 	for (sgi = min_sgi; sgi <= max_sgi; sgi++) {
 		u8 mask = reg >> (8 * (sgi - min_sgi));
+		u8 *src = vgic_get_sgi_sources(dist, vcpu_id, sgi);
 		if (set) {
-			if ((dist->irq_sgi_sources[vcpu_id][sgi] & mask) != mask)
+			if ((*src & mask) != mask)
 				updated = true;
-			dist->irq_sgi_sources[vcpu_id][sgi] |= mask;
+			*src |= mask;
 		} else {
-			if (dist->irq_sgi_sources[vcpu_id][sgi] & mask)
+			if (*src & mask)
 				updated = true;
-			dist->irq_sgi_sources[vcpu_id][sgi] &= ~mask;
+			*src &= ~mask;
 		}
 	}
 
@@ -880,6 +927,11 @@ bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	return true;
 }
 
+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;
+}
+
 static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg)
 {
 	struct kvm *kvm = vcpu->kvm;
@@ -913,7 +965,7 @@ static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg)
 		if (target_cpus & 1) {
 			/* Flag the SGI as pending */
 			vgic_dist_irq_set(vcpu, sgi);
-			dist->irq_sgi_sources[c][sgi] |= 1 << vcpu_id;
+			*vgic_get_sgi_sources(dist, c, sgi) |= 1 << vcpu_id;
 			kvm_debug("SGI%d from CPU%d to CPU%d\n", sgi, vcpu_id, c);
 		}
 
@@ -1124,14 +1176,14 @@ static bool vgic_queue_sgi(struct kvm_vcpu *vcpu, int irq)
 	int vcpu_id = vcpu->vcpu_id;
 	int c;
 
-	sources = dist->irq_sgi_sources[vcpu_id][irq];
+	sources = *vgic_get_sgi_sources(dist, vcpu_id, irq);
 
 	for_each_set_bit(c, &sources, VGIC_MAX_CPUS) {
 		if (vgic_queue_irq(vcpu, c, irq))
 			clear_bit(c, &sources);
 	}
 
-	dist->irq_sgi_sources[vcpu_id][irq] = sources;
+	*vgic_get_sgi_sources(dist, vcpu_id, irq) = sources;
 
 	/*
 	 * If the sources bitmap has been cleared it means that we
@@ -1464,6 +1516,31 @@ static irqreturn_t vgic_maintenance_handler(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void vgic_vcpu_free_maps(struct vgic_cpu *vgic_cpu)
+{
+	kfree(vgic_cpu->pending_shared);
+	kfree(vgic_cpu->vgic_irq_lr_map);
+}
+
+static int vgic_vcpu_init_maps(struct vgic_cpu *vgic_cpu, int nr_irqs)
+{
+	int sz = (nr_irqs - VGIC_NR_PRIVATE_IRQS) / 8;
+	vgic_cpu->pending_shared = kzalloc(sz, GFP_KERNEL);
+	vgic_cpu->vgic_irq_lr_map = kzalloc(nr_irqs, GFP_KERNEL);
+
+	if (!vgic_cpu->pending_shared || !vgic_cpu->vgic_irq_lr_map) {
+		vgic_vcpu_free_maps(vgic_cpu);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
+{
+	vgic_vcpu_free_maps(&vcpu->arch.vgic_cpu);
+}
+
 /**
  * kvm_vgic_vcpu_init - Initialize per-vcpu VGIC state
  * @vcpu: pointer to the vcpu struct
@@ -1475,7 +1552,11 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
-	int i;
+	int i, ret;
+
+	ret = vgic_vcpu_init_maps(vgic_cpu, dist->nr_irqs);
+	if (ret)
+		return ret;
 
 	if (vcpu->vcpu_id >= VGIC_MAX_CPUS)
 		return -EBUSY;
@@ -1580,6 +1661,68 @@ out_free_irq:
 	return ret;
 }
 
+static void vgic_free_maps(struct vgic_dist *dist)
+{
+	int i;
+
+	vgic_free_bitmap(&dist->irq_enabled);
+	vgic_free_bitmap(&dist->irq_state);
+	vgic_free_bitmap(&dist->irq_active);
+	vgic_free_bitmap(&dist->irq_cfg);
+	vgic_free_bytemap(&dist->irq_priority);
+	if (dist->irq_spi_target)
+		for (i = 0; i < dist->nr_cpus; i++)
+			vgic_free_bitmap(&dist->irq_spi_target[i]);
+	kfree(dist->irq_sgi_sources);
+	kfree(dist->irq_spi_cpu);
+	kfree(dist->irq_spi_target);
+}
+
+static int vgic_init_maps(struct vgic_dist *dist, int nr_cpus, int nr_irqs)
+{
+	int ret, i;
+
+	dist->nr_cpus = nr_cpus;
+	dist->nr_irqs = nr_irqs;
+
+	ret  = vgic_init_bitmap(&dist->irq_enabled, nr_cpus, nr_irqs);
+	ret |= vgic_init_bitmap(&dist->irq_state, nr_cpus, nr_irqs);
+	ret |= vgic_init_bitmap(&dist->irq_active, nr_cpus, nr_irqs);
+	ret |= vgic_init_bitmap(&dist->irq_cfg, nr_cpus, nr_irqs);
+	ret |= vgic_init_bytemap(&dist->irq_priority, nr_cpus, nr_irqs);
+
+	if (!ret) {
+		dist->irq_sgi_sources = kzalloc(nr_cpus * VGIC_NR_SGIS,
+						GFP_KERNEL);
+		dist->irq_spi_cpu = kzalloc(nr_irqs - VGIC_NR_PRIVATE_IRQS,
+					    GFP_KERNEL);
+		dist->irq_spi_target = kzalloc(sizeof(*dist->irq_spi_target) * nr_cpus,
+					       GFP_KERNEL);
+		if (!dist->irq_sgi_sources ||
+		    !dist->irq_spi_cpu ||
+		    !dist->irq_spi_target) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		for (i = 0; i < nr_cpus; i++)
+			ret |= vgic_init_bitmap(&dist->irq_spi_target[i],
+						nr_cpus, nr_irqs);
+
+	}
+
+out:
+	if (ret)
+		vgic_free_maps(dist);
+
+	return ret;
+}
+
+void kvm_vgic_destroy(struct kvm *kvm)
+{
+	vgic_free_maps(&kvm->arch.vgic);
+}
+
 /**
  * kvm_vgic_init - Initialize global VGIC state before running any VCPUs
  * @kvm: pointer to the kvm struct
@@ -1624,7 +1767,7 @@ out:
 	return ret;
 }
 
-int kvm_vgic_create(struct kvm *kvm)
+int kvm_vgic_create(struct kvm *kvm, int nr_cpus, int nr_irqs)
 {
 	int i, vcpu_lock_idx = -1, ret = 0;
 	struct kvm_vcpu *vcpu;
@@ -1660,6 +1803,10 @@ int kvm_vgic_create(struct kvm *kvm)
 	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
 	kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
 
+	ret = vgic_init_maps(&kvm->arch.vgic, nr_cpus, nr_irqs);
+	if (ret)
+		kvm_err("Unable to allocate maps\n");
+
 out_unlock:
 	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
 		vcpu = kvm_get_vcpu(kvm, vcpu_lock_idx);
@@ -2040,7 +2187,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, VGIC_MAX_CPUS, VGIC_NR_IRQS);
 }
 
 struct kvm_device_ops kvm_arm_vgic_v2_ops = {
-- 
1.8.3.4

[PATCH v2 2/9] arm/arm64: KVM: vgic: switch to dynamic allocation

[Marc Zyngier]

So far, all the VGIC data structures are statically defined by the
*maximum* number of vcpus and interrupts it supports. It means that
we always have to oversize it to cater for the worse case.

Start by changing the data structures to be dynamically sizeable,
and allocate them at runtime.

The sizes are still very static though.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/arm.c     |   5 +-
 include/kvm/arm_vgic.h |  44 ++++++-----
 virt/kvm/arm/vgic.c    | 203 ++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 206 insertions(+), 46 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 3c82b37..9548bc9 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -182,6 +182,8 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 			kvm->vcpus[i] = NULL;
 		}
 	}
+
+	kvm_vgic_destroy(kvm);
 }
 
 int kvm_dev_ioctl_check_extension(long ext)
@@ -290,6 +292,7 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
 {
 	kvm_mmu_free_memory_caches(vcpu);
 	kvm_timer_vcpu_terminate(vcpu);
+	kvm_vgic_vcpu_destroy(vcpu);
 	kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
@@ -808,7 +811,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, VGIC_MAX_CPUS, VGIC_NR_IRQS);
 		else
 			return -ENXIO;
 	}
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 35b0c12..c57ffd0 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -54,19 +54,24 @@
  * - a bunch of shared interrupts (SPI)
  */
 struct vgic_bitmap {
-	union {
-		u32 reg[VGIC_NR_PRIVATE_IRQS / 32];
-		DECLARE_BITMAP(reg_ul, VGIC_NR_PRIVATE_IRQS);
-	} percpu[VGIC_MAX_CPUS];
-	union {
-		u32 reg[VGIC_NR_SHARED_IRQS / 32];
-		DECLARE_BITMAP(reg_ul, VGIC_NR_SHARED_IRQS);
-	} shared;
+	/*
+	 * - One UL per VCPU for private interrupts (assumes UL is at
+	 * least 32 bits)
+	 * - As many UL as necessary for shared interrupts.
+	 */
+	int nr_cpus;
+	unsigned long *private;
+	unsigned long *shared;
 };
 
 struct vgic_bytemap {
-	u32 percpu[VGIC_MAX_CPUS][VGIC_NR_PRIVATE_IRQS / 4];
-	u32 shared[VGIC_NR_SHARED_IRQS  / 4];
+	/*
+	 * - 8 u32 per VCPU for private interrupts
+	 * - As many u32 as necessary for shared interrupts.
+	 */
+	int nr_cpus;
+	u32 *private;
+	u32 *shared;
 };
 
 struct kvm_vcpu;
@@ -127,6 +132,9 @@ struct vgic_dist {
 	bool			in_kernel;
 	bool			ready;
 
+	int			nr_cpus;
+	int			nr_irqs;
+
 	/* Virtual control interface mapping */
 	void __iomem		*vctrl_base;
 
@@ -152,12 +160,12 @@ struct vgic_dist {
 	/* Level/edge triggered */
 	struct vgic_bitmap	irq_cfg;
 
-	/* Source CPU per SGI and target CPU */
-	u8			irq_sgi_sources[VGIC_MAX_CPUS][VGIC_NR_SGIS];
+	/* Source CPU per SGI and target CPU : 16 bytes per CPU */
+	u8			*irq_sgi_sources;
 
 	/* Target CPU for each IRQ */
-	u8			irq_spi_cpu[VGIC_NR_SHARED_IRQS];
-	struct vgic_bitmap	irq_spi_target[VGIC_MAX_CPUS];
+	u8			*irq_spi_cpu;
+	struct vgic_bitmap	*irq_spi_target;
 
 	/* Bitmap indicating which CPU has something pending */
 	unsigned long		irq_pending_on_cpu;
@@ -190,11 +198,11 @@ struct vgic_v3_cpu_if {
 struct vgic_cpu {
 #ifdef CONFIG_KVM_ARM_VGIC
 	/* per IRQ to LR mapping */
-	u8		vgic_irq_lr_map[VGIC_NR_IRQS];
+	u8		*vgic_irq_lr_map;
 
 	/* Pending interrupts on this VCPU */
 	DECLARE_BITMAP(	pending_percpu, VGIC_NR_PRIVATE_IRQS);
-	DECLARE_BITMAP(	pending_shared, VGIC_NR_SHARED_IRQS);
+	unsigned long	*pending_shared;
 
 	/* Bitmap of used/free list registers */
 	DECLARE_BITMAP(	lr_used, VGIC_V2_MAX_LRS);
@@ -224,8 +232,10 @@ 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, int nr_cpus, int nr_irqs);
+void kvm_vgic_destroy(struct kvm *kvm);
 int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu);
+void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu);
 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,
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index c6da748..220b215 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -89,6 +89,7 @@ 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);
@@ -98,23 +99,44 @@ 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;
 
+static int vgic_init_bitmap(struct vgic_bitmap *b, int nr_cpus, int nr_irqs)
+{
+	int nr_longs;
+
+	nr_longs = nr_cpus + BITS_TO_LONGS(nr_irqs - VGIC_NR_PRIVATE_IRQS);
+
+	b->private = kzalloc(sizeof(unsigned long) * nr_longs, GFP_KERNEL);
+	if (!b->private)
+		return -ENOMEM;
+
+	b->shared = b->private + nr_cpus;
+
+	b->nr_cpus = nr_cpus;
+	return 0;
+}
+
+static void vgic_free_bitmap(struct vgic_bitmap *b)
+{
+	kfree(b->private);
+}
+
 static u32 *vgic_bitmap_get_reg(struct vgic_bitmap *x,
 				int cpuid, u32 offset)
 {
 	offset >>= 2;
 	if (!offset)
-		return x->percpu[cpuid].reg;
+		return (u32 *)(x->private + cpuid);
 	else
-		return x->shared.reg + offset - 1;
+		return (u32 *)(x->shared) + offset - 1;
 }
 
 static int vgic_bitmap_get_irq_val(struct vgic_bitmap *x,
 				   int cpuid, int irq)
 {
 	if (irq < VGIC_NR_PRIVATE_IRQS)
-		return test_bit(irq, x->percpu[cpuid].reg_ul);
+		return test_bit(irq, x->private + cpuid);
 
-	return test_bit(irq - VGIC_NR_PRIVATE_IRQS, x->shared.reg_ul);
+	return test_bit(irq - VGIC_NR_PRIVATE_IRQS, x->shared);
 }
 
 static void vgic_bitmap_set_irq_val(struct vgic_bitmap *x, int cpuid,
@@ -123,9 +145,9 @@ static void vgic_bitmap_set_irq_val(struct vgic_bitmap *x, int cpuid,
 	unsigned long *reg;
 
 	if (irq < VGIC_NR_PRIVATE_IRQS) {
-		reg = x->percpu[cpuid].reg_ul;
+		reg = x->private + cpuid;
 	} else {
-		reg =  x->shared.reg_ul;
+		reg = x->shared;
 		irq -= VGIC_NR_PRIVATE_IRQS;
 	}
 
@@ -137,24 +159,48 @@ static void vgic_bitmap_set_irq_val(struct vgic_bitmap *x, int cpuid,
 
 static unsigned long *vgic_bitmap_get_cpu_map(struct vgic_bitmap *x, int cpuid)
 {
-	if (unlikely(cpuid >= VGIC_MAX_CPUS))
-		return NULL;
-	return x->percpu[cpuid].reg_ul;
+	return x->private + cpuid;
 }
 
 static unsigned long *vgic_bitmap_get_shared_map(struct vgic_bitmap *x)
 {
-	return x->shared.reg_ul;
+	return x->shared;
+}
+
+static int vgic_init_bytemap(struct vgic_bytemap *x, int nr_cpus, int nr_irqs)
+{
+	int size;
+
+	size  = nr_cpus * VGIC_NR_PRIVATE_IRQS;
+	size += nr_irqs - VGIC_NR_PRIVATE_IRQS;
+
+	x->private = kzalloc(size, GFP_KERNEL);
+	if (!x->private)
+		return -ENOMEM;
+
+	x->shared = x->private + nr_cpus * VGIC_NR_PRIVATE_IRQS / sizeof(u32);
+	x->nr_cpus = nr_cpus;
+	return 0;
+}
+
+static void vgic_free_bytemap(struct vgic_bytemap *b)
+{
+	kfree(b->private);
 }
 
 static u32 *vgic_bytemap_get_reg(struct vgic_bytemap *x, int cpuid, u32 offset)
 {
-	offset >>= 2;
-	BUG_ON(offset > (VGIC_NR_IRQS / 4));
-	if (offset < 8)
-		return x->percpu[cpuid] + offset;
-	else
-		return x->shared + offset - 8;
+	u32 *reg;
+
+	if (offset < 32) {
+		reg = x->private;
+		offset += cpuid * VGIC_NR_PRIVATE_IRQS;
+	} else {
+		reg = x->shared;
+		offset -= VGIC_NR_PRIVATE_IRQS;
+	}
+
+	return reg + (offset / sizeof(u32));
 }
 
 #define VGIC_CFG_LEVEL	0
@@ -633,7 +679,7 @@ static void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
 		 */
 		vgic_dist_irq_set(vcpu, lr.irq);
 		if (lr.irq < VGIC_NR_SGIS)
-			dist->irq_sgi_sources[vcpu_id][lr.irq] |= 1 << lr.source;
+			*vgic_get_sgi_sources(dist, vcpu_id, lr.irq) |= 1 << lr.source;
 		lr.state &= ~LR_STATE_PENDING;
 		vgic_set_lr(vcpu, i, lr);
 
@@ -665,7 +711,7 @@ static bool read_set_clear_sgi_pend_reg(struct kvm_vcpu *vcpu,
 	/* Copy source SGIs from distributor side */
 	for (sgi = min_sgi; sgi <= max_sgi; sgi++) {
 		int shift = 8 * (sgi - min_sgi);
-		reg |= (u32)dist->irq_sgi_sources[vcpu_id][sgi] << shift;
+		reg |= ((u32)*vgic_get_sgi_sources(dist, vcpu_id, sgi)) << shift;
 	}
 
 	mmio_data_write(mmio, ~0, reg);
@@ -689,14 +735,15 @@ static bool write_set_clear_sgi_pend_reg(struct kvm_vcpu *vcpu,
 	/* Clear pending SGIs on the distributor */
 	for (sgi = min_sgi; sgi <= max_sgi; sgi++) {
 		u8 mask = reg >> (8 * (sgi - min_sgi));
+		u8 *src = vgic_get_sgi_sources(dist, vcpu_id, sgi);
 		if (set) {
-			if ((dist->irq_sgi_sources[vcpu_id][sgi] & mask) != mask)
+			if ((*src & mask) != mask)
 				updated = true;
-			dist->irq_sgi_sources[vcpu_id][sgi] |= mask;
+			*src |= mask;
 		} else {
-			if (dist->irq_sgi_sources[vcpu_id][sgi] & mask)
+			if (*src & mask)
 				updated = true;
-			dist->irq_sgi_sources[vcpu_id][sgi] &= ~mask;
+			*src &= ~mask;
 		}
 	}
 
@@ -880,6 +927,11 @@ bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	return true;
 }
 
+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;
+}
+
 static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg)
 {
 	struct kvm *kvm = vcpu->kvm;
@@ -913,7 +965,7 @@ static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg)
 		if (target_cpus & 1) {
 			/* Flag the SGI as pending */
 			vgic_dist_irq_set(vcpu, sgi);
-			dist->irq_sgi_sources[c][sgi] |= 1 << vcpu_id;
+			*vgic_get_sgi_sources(dist, c, sgi) |= 1 << vcpu_id;
 			kvm_debug("SGI%d from CPU%d to CPU%d\n", sgi, vcpu_id, c);
 		}
 
@@ -1124,14 +1176,14 @@ static bool vgic_queue_sgi(struct kvm_vcpu *vcpu, int irq)
 	int vcpu_id = vcpu->vcpu_id;
 	int c;
 
-	sources = dist->irq_sgi_sources[vcpu_id][irq];
+	sources = *vgic_get_sgi_sources(dist, vcpu_id, irq);
 
 	for_each_set_bit(c, &sources, VGIC_MAX_CPUS) {
 		if (vgic_queue_irq(vcpu, c, irq))
 			clear_bit(c, &sources);
 	}
 
-	dist->irq_sgi_sources[vcpu_id][irq] = sources;
+	*vgic_get_sgi_sources(dist, vcpu_id, irq) = sources;
 
 	/*
 	 * If the sources bitmap has been cleared it means that we
@@ -1464,6 +1516,31 @@ static irqreturn_t vgic_maintenance_handler(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void vgic_vcpu_free_maps(struct vgic_cpu *vgic_cpu)
+{
+	kfree(vgic_cpu->pending_shared);
+	kfree(vgic_cpu->vgic_irq_lr_map);
+}
+
+static int vgic_vcpu_init_maps(struct vgic_cpu *vgic_cpu, int nr_irqs)
+{
+	int sz = (nr_irqs - VGIC_NR_PRIVATE_IRQS) / 8;
+	vgic_cpu->pending_shared = kzalloc(sz, GFP_KERNEL);
+	vgic_cpu->vgic_irq_lr_map = kzalloc(nr_irqs, GFP_KERNEL);
+
+	if (!vgic_cpu->pending_shared || !vgic_cpu->vgic_irq_lr_map) {
+		vgic_vcpu_free_maps(vgic_cpu);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
+{
+	vgic_vcpu_free_maps(&vcpu->arch.vgic_cpu);
+}
+
 /**
  * kvm_vgic_vcpu_init - Initialize per-vcpu VGIC state
  * @vcpu: pointer to the vcpu struct
@@ -1475,7 +1552,11 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
-	int i;
+	int i, ret;
+
+	ret = vgic_vcpu_init_maps(vgic_cpu, dist->nr_irqs);
+	if (ret)
+		return ret;
 
 	if (vcpu->vcpu_id >= VGIC_MAX_CPUS)
 		return -EBUSY;
@@ -1580,6 +1661,68 @@ out_free_irq:
 	return ret;
 }
 
+static void vgic_free_maps(struct vgic_dist *dist)
+{
+	int i;
+
+	vgic_free_bitmap(&dist->irq_enabled);
+	vgic_free_bitmap(&dist->irq_state);
+	vgic_free_bitmap(&dist->irq_active);
+	vgic_free_bitmap(&dist->irq_cfg);
+	vgic_free_bytemap(&dist->irq_priority);
+	if (dist->irq_spi_target)
+		for (i = 0; i < dist->nr_cpus; i++)
+			vgic_free_bitmap(&dist->irq_spi_target[i]);
+	kfree(dist->irq_sgi_sources);
+	kfree(dist->irq_spi_cpu);
+	kfree(dist->irq_spi_target);
+}
+
+static int vgic_init_maps(struct vgic_dist *dist, int nr_cpus, int nr_irqs)
+{
+	int ret, i;
+
+	dist->nr_cpus = nr_cpus;
+	dist->nr_irqs = nr_irqs;
+
+	ret  = vgic_init_bitmap(&dist->irq_enabled, nr_cpus, nr_irqs);
+	ret |= vgic_init_bitmap(&dist->irq_state, nr_cpus, nr_irqs);
+	ret |= vgic_init_bitmap(&dist->irq_active, nr_cpus, nr_irqs);
+	ret |= vgic_init_bitmap(&dist->irq_cfg, nr_cpus, nr_irqs);
+	ret |= vgic_init_bytemap(&dist->irq_priority, nr_cpus, nr_irqs);
+
+	if (!ret) {
+		dist->irq_sgi_sources = kzalloc(nr_cpus * VGIC_NR_SGIS,
+						GFP_KERNEL);
+		dist->irq_spi_cpu = kzalloc(nr_irqs - VGIC_NR_PRIVATE_IRQS,
+					    GFP_KERNEL);
+		dist->irq_spi_target = kzalloc(sizeof(*dist->irq_spi_target) * nr_cpus,
+					       GFP_KERNEL);
+		if (!dist->irq_sgi_sources ||
+		    !dist->irq_spi_cpu ||
+		    !dist->irq_spi_target) {
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		for (i = 0; i < nr_cpus; i++)
+			ret |= vgic_init_bitmap(&dist->irq_spi_target[i],
+						nr_cpus, nr_irqs);
+
+	}
+
+out:
+	if (ret)
+		vgic_free_maps(dist);
+
+	return ret;
+}
+
+void kvm_vgic_destroy(struct kvm *kvm)
+{
+	vgic_free_maps(&kvm->arch.vgic);
+}
+
 /**
  * kvm_vgic_init - Initialize global VGIC state before running any VCPUs
  * @kvm: pointer to the kvm struct
@@ -1624,7 +1767,7 @@ out:
 	return ret;
 }
 
-int kvm_vgic_create(struct kvm *kvm)
+int kvm_vgic_create(struct kvm *kvm, int nr_cpus, int nr_irqs)
 {
 	int i, vcpu_lock_idx = -1, ret = 0;
 	struct kvm_vcpu *vcpu;
@@ -1660,6 +1803,10 @@ int kvm_vgic_create(struct kvm *kvm)
 	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
 	kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
 
+	ret = vgic_init_maps(&kvm->arch.vgic, nr_cpus, nr_irqs);
+	if (ret)
+		kvm_err("Unable to allocate maps\n");
+
 out_unlock:
 	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
 		vcpu = kvm_get_vcpu(kvm, vcpu_lock_idx);
@@ -2040,7 +2187,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, VGIC_MAX_CPUS, VGIC_NR_IRQS);
 }
 
 struct kvm_device_ops kvm_arm_vgic_v2_ops = {
-- 
1.8.3.4

[PATCH v2 3/9] arm/arm64: KVM: vgic: Parametrize VGIC_NR_SHARED_IRQS

[Marc Zyngier]

Having a dynamic number of supported interrupts means that we
cannot relly on VGIC_NR_SHARED_IRQS being fixed anymore.

Instead, make it take the distributor structure as a parameter,
so it can return the right value.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/kvm/arm_vgic.h |  1 -
 virt/kvm/arm/vgic.c    | 16 +++++++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index c57ffd0..b5072b7 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -29,7 +29,6 @@
 #define VGIC_NR_SGIS		16
 #define VGIC_NR_PPIS		16
 #define VGIC_NR_PRIVATE_IRQS	(VGIC_NR_SGIS + VGIC_NR_PPIS)
-#define VGIC_NR_SHARED_IRQS	(VGIC_NR_IRQS - VGIC_NR_PRIVATE_IRQS)
 #define VGIC_MAX_CPUS		KVM_MAX_VCPUS
 
 #define VGIC_V2_MAX_LRS		(1 << 6)
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 220b215..4b1f0a6 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -973,11 +973,17 @@ static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg)
 	}
 }
 
+static int vgic_nr_shared_irqs(struct vgic_dist *dist)
+{
+	return dist->nr_irqs - VGIC_NR_PRIVATE_IRQS;
+}
+
 static int compute_pending_for_cpu(struct kvm_vcpu *vcpu)
 {
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
 	unsigned long *pending, *enabled, *pend_percpu, *pend_shared;
 	unsigned long pending_private, pending_shared;
+	int shared = vgic_nr_shared_irqs(dist);
 	int vcpu_id;
 
 	vcpu_id = vcpu->vcpu_id;
@@ -990,15 +996,15 @@ static int compute_pending_for_cpu(struct kvm_vcpu *vcpu)
 
 	pending = vgic_bitmap_get_shared_map(&dist->irq_state);
 	enabled = vgic_bitmap_get_shared_map(&dist->irq_enabled);
-	bitmap_and(pend_shared, pending, enabled, VGIC_NR_SHARED_IRQS);
+	bitmap_and(pend_shared, pending, enabled, shared);
 	bitmap_and(pend_shared, pend_shared,
 		   vgic_bitmap_get_shared_map(&dist->irq_spi_target[vcpu_id]),
-		   VGIC_NR_SHARED_IRQS);
+		   shared);
 
 	pending_private = find_first_bit(pend_percpu, VGIC_NR_PRIVATE_IRQS);
-	pending_shared = find_first_bit(pend_shared, VGIC_NR_SHARED_IRQS);
+	pending_shared = find_first_bit(pend_shared, shared);
 	return (pending_private < VGIC_NR_PRIVATE_IRQS ||
-		pending_shared < VGIC_NR_SHARED_IRQS);
+		pending_shared < vgic_nr_shared_irqs(dist));
 }
 
 /*
@@ -1255,7 +1261,7 @@ static void __kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
 	}
 
 	/* SPIs */
-	for_each_set_bit(i, vgic_cpu->pending_shared, VGIC_NR_SHARED_IRQS) {
+	for_each_set_bit(i, vgic_cpu->pending_shared, vgic_nr_shared_irqs(dist)) {
 		if (!vgic_queue_hwirq(vcpu, i + VGIC_NR_PRIVATE_IRQS))
 			overflow = 1;
 	}
-- 
1.8.3.4

[PATCH v2 3/9] arm/arm64: KVM: vgic: Parametrize VGIC_NR_SHARED_IRQS

[Marc Zyngier]

Having a dynamic number of supported interrupts means that we
cannot relly on VGIC_NR_SHARED_IRQS being fixed anymore.

Instead, make it take the distributor structure as a parameter,
so it can return the right value.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/kvm/arm_vgic.h |  1 -
 virt/kvm/arm/vgic.c    | 16 +++++++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index c57ffd0..b5072b7 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -29,7 +29,6 @@
 #define VGIC_NR_SGIS		16
 #define VGIC_NR_PPIS		16
 #define VGIC_NR_PRIVATE_IRQS	(VGIC_NR_SGIS + VGIC_NR_PPIS)
-#define VGIC_NR_SHARED_IRQS	(VGIC_NR_IRQS - VGIC_NR_PRIVATE_IRQS)
 #define VGIC_MAX_CPUS		KVM_MAX_VCPUS
 
 #define VGIC_V2_MAX_LRS		(1 << 6)
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 220b215..4b1f0a6 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -973,11 +973,17 @@ static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg)
 	}
 }
 
+static int vgic_nr_shared_irqs(struct vgic_dist *dist)
+{
+	return dist->nr_irqs - VGIC_NR_PRIVATE_IRQS;
+}
+
 static int compute_pending_for_cpu(struct kvm_vcpu *vcpu)
 {
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
 	unsigned long *pending, *enabled, *pend_percpu, *pend_shared;
 	unsigned long pending_private, pending_shared;
+	int shared = vgic_nr_shared_irqs(dist);
 	int vcpu_id;
 
 	vcpu_id = vcpu->vcpu_id;
@@ -990,15 +996,15 @@ static int compute_pending_for_cpu(struct kvm_vcpu *vcpu)
 
 	pending = vgic_bitmap_get_shared_map(&dist->irq_state);
 	enabled = vgic_bitmap_get_shared_map(&dist->irq_enabled);
-	bitmap_and(pend_shared, pending, enabled, VGIC_NR_SHARED_IRQS);
+	bitmap_and(pend_shared, pending, enabled, shared);
 	bitmap_and(pend_shared, pend_shared,
 		   vgic_bitmap_get_shared_map(&dist->irq_spi_target[vcpu_id]),
-		   VGIC_NR_SHARED_IRQS);
+		   shared);
 
 	pending_private = find_first_bit(pend_percpu, VGIC_NR_PRIVATE_IRQS);
-	pending_shared = find_first_bit(pend_shared, VGIC_NR_SHARED_IRQS);
+	pending_shared = find_first_bit(pend_shared, shared);
 	return (pending_private < VGIC_NR_PRIVATE_IRQS ||
-		pending_shared < VGIC_NR_SHARED_IRQS);
+		pending_shared < vgic_nr_shared_irqs(dist));
 }
 
 /*
@@ -1255,7 +1261,7 @@ static void __kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
 	}
 
 	/* SPIs */
-	for_each_set_bit(i, vgic_cpu->pending_shared, VGIC_NR_SHARED_IRQS) {
+	for_each_set_bit(i, vgic_cpu->pending_shared, vgic_nr_shared_irqs(dist)) {
 		if (!vgic_queue_hwirq(vcpu, i + VGIC_NR_PRIVATE_IRQS))
 			overflow = 1;
 	}
-- 
1.8.3.4

[PATCH v2 4/9] arm/arm64: KVM: vgic: kill VGIC_MAX_CPUS

[Marc Zyngier]

We now have the information about the number of CPU interfaces in
the distributor itself. Let's get rid of VGIC_MAX_CPUS, and just
rely on KVM_MAX_VCPUS where we don't have the choice. Yet.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/arm.c     | 2 +-
 include/kvm/arm_vgic.h | 3 +--
 virt/kvm/arm/vgic.c    | 6 +++---
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 9548bc9..14ba035 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -811,7 +811,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	switch (ioctl) {
 	case KVM_CREATE_IRQCHIP: {
 		if (vgic_present)
-			return kvm_vgic_create(kvm, VGIC_MAX_CPUS, VGIC_NR_IRQS);
+			return kvm_vgic_create(kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS);
 		else
 			return -ENXIO;
 	}
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index b5072b7..5853a67 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -29,13 +29,12 @@
 #define VGIC_NR_SGIS		16
 #define VGIC_NR_PPIS		16
 #define VGIC_NR_PRIVATE_IRQS	(VGIC_NR_SGIS + VGIC_NR_PPIS)
-#define VGIC_MAX_CPUS		KVM_MAX_VCPUS
 
 #define VGIC_V2_MAX_LRS		(1 << 6)
 #define VGIC_V3_MAX_LRS		16
 
 /* Sanity checks... */
-#if (VGIC_MAX_CPUS > 8)
+#if (KVM_MAX_VCPUS > 8)
 #error	Invalid number of CPU interfaces
 #endif
 
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 4b1f0a6..07b6450 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1184,7 +1184,7 @@ static bool vgic_queue_sgi(struct kvm_vcpu *vcpu, int irq)
 
 	sources = *vgic_get_sgi_sources(dist, vcpu_id, irq);
 
-	for_each_set_bit(c, &sources, VGIC_MAX_CPUS) {
+	for_each_set_bit(c, &sources, dist->nr_cpus) {
 		if (vgic_queue_irq(vcpu, c, irq))
 			clear_bit(c, &sources);
 	}
@@ -1564,7 +1564,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 	if (ret)
 		return ret;
 
-	if (vcpu->vcpu_id >= VGIC_MAX_CPUS)
+	if (vcpu->vcpu_id >= dist->nr_cpus)
 		return -EBUSY;
 
 	for (i = 0; i < VGIC_NR_IRQS; i++) {
@@ -2193,7 +2193,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, VGIC_MAX_CPUS, VGIC_NR_IRQS);
+	return kvm_vgic_create(dev->kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS);
 }
 
 struct kvm_device_ops kvm_arm_vgic_v2_ops = {
-- 
1.8.3.4

[PATCH v2 4/9] arm/arm64: KVM: vgic: kill VGIC_MAX_CPUS

[Marc Zyngier]

We now have the information about the number of CPU interfaces in
the distributor itself. Let's get rid of VGIC_MAX_CPUS, and just
rely on KVM_MAX_VCPUS where we don't have the choice. Yet.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/arm.c     | 2 +-
 include/kvm/arm_vgic.h | 3 +--
 virt/kvm/arm/vgic.c    | 6 +++---
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 9548bc9..14ba035 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -811,7 +811,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	switch (ioctl) {
 	case KVM_CREATE_IRQCHIP: {
 		if (vgic_present)
-			return kvm_vgic_create(kvm, VGIC_MAX_CPUS, VGIC_NR_IRQS);
+			return kvm_vgic_create(kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS);
 		else
 			return -ENXIO;
 	}
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index b5072b7..5853a67 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -29,13 +29,12 @@
 #define VGIC_NR_SGIS		16
 #define VGIC_NR_PPIS		16
 #define VGIC_NR_PRIVATE_IRQS	(VGIC_NR_SGIS + VGIC_NR_PPIS)
-#define VGIC_MAX_CPUS		KVM_MAX_VCPUS
 
 #define VGIC_V2_MAX_LRS		(1 << 6)
 #define VGIC_V3_MAX_LRS		16
 
 /* Sanity checks... */
-#if (VGIC_MAX_CPUS > 8)
+#if (KVM_MAX_VCPUS > 8)
 #error	Invalid number of CPU interfaces
 #endif
 
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 4b1f0a6..07b6450 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1184,7 +1184,7 @@ static bool vgic_queue_sgi(struct kvm_vcpu *vcpu, int irq)
 
 	sources = *vgic_get_sgi_sources(dist, vcpu_id, irq);
 
-	for_each_set_bit(c, &sources, VGIC_MAX_CPUS) {
+	for_each_set_bit(c, &sources, dist->nr_cpus) {
 		if (vgic_queue_irq(vcpu, c, irq))
 			clear_bit(c, &sources);
 	}
@@ -1564,7 +1564,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 	if (ret)
 		return ret;
 
-	if (vcpu->vcpu_id >= VGIC_MAX_CPUS)
+	if (vcpu->vcpu_id >= dist->nr_cpus)
 		return -EBUSY;
 
 	for (i = 0; i < VGIC_NR_IRQS; i++) {
@@ -2193,7 +2193,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, VGIC_MAX_CPUS, VGIC_NR_IRQS);
+	return kvm_vgic_create(dev->kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS);
 }
 
 struct kvm_device_ops kvm_arm_vgic_v2_ops = {
-- 
1.8.3.4

[PATCH v2 5/9] arm/arm64: KVM: vgic: handle out-of-range MMIO accesses

[Marc Zyngier]

Now that we can (almost) dynamically size the number of interrupts,
we're facing an interesting issue:

We have to evaluate at runtime whether or not an access hits a valid
register, based on the sizing of this particular instance of the
distributor. Furthermore, the GIC spec says that accessing a reserved
register is RAZ/WI.

For this, add a new field to our range structure, indicating the number
of bits a single interrupts uses. That allows us to find out whether or
not the access is in range.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/kvm/arm_vgic.h |  3 ++-
 virt/kvm/arm/vgic.c    | 56 ++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 5853a67..3014145 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -32,6 +32,7 @@
 
 #define VGIC_V2_MAX_LRS		(1 << 6)
 #define VGIC_V3_MAX_LRS		16
+#define VGIC_MAX_IRQS		1024
 
 /* Sanity checks... */
 #if (KVM_MAX_VCPUS > 8)
@@ -42,7 +43,7 @@
 #error "VGIC_NR_IRQS must be a multiple of 32"
 #endif
 
-#if (VGIC_NR_IRQS > 1024)
+#if (VGIC_NR_IRQS > VGIC_MAX_IRQS)
 #error "VGIC_NR_IRQS must be <= 1024"
 #endif
 
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 07b6450..13ead5d 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -782,6 +782,7 @@ static bool handle_mmio_sgi_clear(struct kvm_vcpu *vcpu,
 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);
 };
@@ -790,56 +791,67 @@ static const struct mmio_range vgic_dist_ranges[] = {
 	{
 		.base		= GIC_DIST_CTRL,
 		.len		= 12,
+		.bits_per_irq	= 0,
 		.handle_mmio	= handle_mmio_misc,
 	},
 	{
 		.base		= GIC_DIST_IGROUP,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_raz_wi,
 	},
 	{
 		.base		= GIC_DIST_ENABLE_SET,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_set_enable_reg,
 	},
 	{
 		.base		= GIC_DIST_ENABLE_CLEAR,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_clear_enable_reg,
 	},
 	{
 		.base		= GIC_DIST_PENDING_SET,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_set_pending_reg,
 	},
 	{
 		.base		= GIC_DIST_PENDING_CLEAR,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_clear_pending_reg,
 	},
 	{
 		.base		= GIC_DIST_ACTIVE_SET,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_raz_wi,
 	},
 	{
 		.base		= GIC_DIST_ACTIVE_CLEAR,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_raz_wi,
 	},
 	{
 		.base		= GIC_DIST_PRI,
-		.len		= VGIC_NR_IRQS,
+		.len		= VGIC_MAX_IRQS,
+		.bits_per_irq	= 8,
 		.handle_mmio	= handle_mmio_priority_reg,
 	},
 	{
 		.base		= GIC_DIST_TARGET,
-		.len		= VGIC_NR_IRQS,
+		.len		= VGIC_MAX_IRQS,
+		.bits_per_irq	= 8,
 		.handle_mmio	= handle_mmio_target_reg,
 	},
 	{
 		.base		= GIC_DIST_CONFIG,
-		.len		= VGIC_NR_IRQS / 4,
+		.len		= VGIC_MAX_IRQS / 4,
+		.bits_per_irq	= 2,
 		.handle_mmio	= handle_mmio_cfg_reg,
 	},
 	{
@@ -877,6 +889,22 @@ struct mmio_range *find_matching_range(const struct mmio_range *ranges,
 	return NULL;
 }
 
+static bool vgic_validate_access(const struct vgic_dist *dist,
+				 const struct mmio_range *range,
+				 unsigned long offset)
+{
+	int irq;
+
+	if (!range->bits_per_irq)
+		return true;	/* Not an irq-based access */
+
+	irq = offset * 8 / range->bits_per_irq;
+	if (irq >= dist->nr_irqs)
+		return false;
+
+	return true;
+}
+
 /**
  * vgic_handle_mmio - handle an in-kernel MMIO access
  * @vcpu:	pointer to the vcpu performing the access
@@ -916,7 +944,13 @@ 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;
-	updated_state = range->handle_mmio(vcpu, mmio, offset);
+	if (vgic_validate_access(dist, range, offset)) {
+		updated_state = range->handle_mmio(vcpu, mmio, offset);
+	} else {
+		vgic_reg_access(mmio, NULL, offset,
+				ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+		updated_state = false;
+	}
 	spin_unlock(&vcpu->kvm->arch.vgic.lock);
 	kvm_prepare_mmio(run, mmio);
 	kvm_handle_mmio_return(vcpu, run);
-- 
1.8.3.4

[PATCH v2 5/9] arm/arm64: KVM: vgic: handle out-of-range MMIO accesses

[Marc Zyngier]

Now that we can (almost) dynamically size the number of interrupts,
we're facing an interesting issue:

We have to evaluate at runtime whether or not an access hits a valid
register, based on the sizing of this particular instance of the
distributor. Furthermore, the GIC spec says that accessing a reserved
register is RAZ/WI.

For this, add a new field to our range structure, indicating the number
of bits a single interrupts uses. That allows us to find out whether or
not the access is in range.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/kvm/arm_vgic.h |  3 ++-
 virt/kvm/arm/vgic.c    | 56 ++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 5853a67..3014145 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -32,6 +32,7 @@
 
 #define VGIC_V2_MAX_LRS		(1 << 6)
 #define VGIC_V3_MAX_LRS		16
+#define VGIC_MAX_IRQS		1024
 
 /* Sanity checks... */
 #if (KVM_MAX_VCPUS > 8)
@@ -42,7 +43,7 @@
 #error "VGIC_NR_IRQS must be a multiple of 32"
 #endif
 
-#if (VGIC_NR_IRQS > 1024)
+#if (VGIC_NR_IRQS > VGIC_MAX_IRQS)
 #error "VGIC_NR_IRQS must be <= 1024"
 #endif
 
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 07b6450..13ead5d 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -782,6 +782,7 @@ static bool handle_mmio_sgi_clear(struct kvm_vcpu *vcpu,
 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);
 };
@@ -790,56 +791,67 @@ static const struct mmio_range vgic_dist_ranges[] = {
 	{
 		.base		= GIC_DIST_CTRL,
 		.len		= 12,
+		.bits_per_irq	= 0,
 		.handle_mmio	= handle_mmio_misc,
 	},
 	{
 		.base		= GIC_DIST_IGROUP,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_raz_wi,
 	},
 	{
 		.base		= GIC_DIST_ENABLE_SET,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_set_enable_reg,
 	},
 	{
 		.base		= GIC_DIST_ENABLE_CLEAR,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_clear_enable_reg,
 	},
 	{
 		.base		= GIC_DIST_PENDING_SET,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_set_pending_reg,
 	},
 	{
 		.base		= GIC_DIST_PENDING_CLEAR,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_clear_pending_reg,
 	},
 	{
 		.base		= GIC_DIST_ACTIVE_SET,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_raz_wi,
 	},
 	{
 		.base		= GIC_DIST_ACTIVE_CLEAR,
-		.len		= VGIC_NR_IRQS / 8,
+		.len		= VGIC_MAX_IRQS / 8,
+		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_raz_wi,
 	},
 	{
 		.base		= GIC_DIST_PRI,
-		.len		= VGIC_NR_IRQS,
+		.len		= VGIC_MAX_IRQS,
+		.bits_per_irq	= 8,
 		.handle_mmio	= handle_mmio_priority_reg,
 	},
 	{
 		.base		= GIC_DIST_TARGET,
-		.len		= VGIC_NR_IRQS,
+		.len		= VGIC_MAX_IRQS,
+		.bits_per_irq	= 8,
 		.handle_mmio	= handle_mmio_target_reg,
 	},
 	{
 		.base		= GIC_DIST_CONFIG,
-		.len		= VGIC_NR_IRQS / 4,
+		.len		= VGIC_MAX_IRQS / 4,
+		.bits_per_irq	= 2,
 		.handle_mmio	= handle_mmio_cfg_reg,
 	},
 	{
@@ -877,6 +889,22 @@ struct mmio_range *find_matching_range(const struct mmio_range *ranges,
 	return NULL;
 }
 
+static bool vgic_validate_access(const struct vgic_dist *dist,
+				 const struct mmio_range *range,
+				 unsigned long offset)
+{
+	int irq;
+
+	if (!range->bits_per_irq)
+		return true;	/* Not an irq-based access */
+
+	irq = offset * 8 / range->bits_per_irq;
+	if (irq >= dist->nr_irqs)
+		return false;
+
+	return true;
+}
+
 /**
  * vgic_handle_mmio - handle an in-kernel MMIO access
  * @vcpu:	pointer to the vcpu performing the access
@@ -916,7 +944,13 @@ 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;
-	updated_state = range->handle_mmio(vcpu, mmio, offset);
+	if (vgic_validate_access(dist, range, offset)) {
+		updated_state = range->handle_mmio(vcpu, mmio, offset);
+	} else {
+		vgic_reg_access(mmio, NULL, offset,
+				ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+		updated_state = false;
+	}
 	spin_unlock(&vcpu->kvm->arch.vgic.lock);
 	kvm_prepare_mmio(run, mmio);
 	kvm_handle_mmio_return(vcpu, run);
-- 
1.8.3.4

[PATCH v2 6/9] arm/arm64: KVM: vgic: kill VGIC_NR_IRQS

[Marc Zyngier]

Nuke VGIC_NR_IRQS entierly, now that the distributor instance
contains the number of IRQ allocated to this GIC.

Also add VGIC_NR_IRQS_LEGACY to preserve the current API.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/arm.c     |  2 +-
 include/kvm/arm_vgic.h |  6 +++---
 virt/kvm/arm/vgic.c    | 13 +++++++------
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 14ba035..66c14ef 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -811,7 +811,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	switch (ioctl) {
 	case KVM_CREATE_IRQCHIP: {
 		if (vgic_present)
-			return kvm_vgic_create(kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS);
+			return kvm_vgic_create(kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS_LEGACY);
 		else
 			return -ENXIO;
 	}
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 3014145..cf17b68 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -25,7 +25,7 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
-#define VGIC_NR_IRQS		256
+#define VGIC_NR_IRQS_LEGACY	256
 #define VGIC_NR_SGIS		16
 #define VGIC_NR_PPIS		16
 #define VGIC_NR_PRIVATE_IRQS	(VGIC_NR_SGIS + VGIC_NR_PPIS)
@@ -39,11 +39,11 @@
 #error	Invalid number of CPU interfaces
 #endif
 
-#if (VGIC_NR_IRQS & 31)
+#if (VGIC_NR_IRQS_LEGACY & 31)
 #error "VGIC_NR_IRQS must be a multiple of 32"
 #endif
 
-#if (VGIC_NR_IRQS > VGIC_MAX_IRQS)
+#if (VGIC_NR_IRQS_LEGACY > VGIC_MAX_IRQS)
 #error "VGIC_NR_IRQS must be <= 1024"
 #endif
 
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 13ead5d..17ceb8a 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -373,7 +373,7 @@ static bool handle_mmio_misc(struct kvm_vcpu *vcpu,
 
 	case 4:			/* GICD_TYPER */
 		reg  = (atomic_read(&vcpu->kvm->online_vcpus) - 1) << 5;
-		reg |= (VGIC_NR_IRQS >> 5) - 1;
+		reg |= (vcpu->kvm->arch.vgic.nr_irqs >> 5) - 1;
 		vgic_reg_access(mmio, &reg, word_offset,
 				ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
 		break;
@@ -1164,13 +1164,14 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)
 static 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;
 	struct vgic_lr vlr;
 	int lr;
 
 	/* Sanitize the input... */
 	BUG_ON(sgi_source_id & ~7);
 	BUG_ON(sgi_source_id && irq >= VGIC_NR_SGIS);
-	BUG_ON(irq >= VGIC_NR_IRQS);
+	BUG_ON(irq >= dist->nr_irqs);
 
 	kvm_debug("Queue IRQ%d\n", irq);
 
@@ -1387,7 +1388,7 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
 
 		vlr = vgic_get_lr(vcpu, lr);
 
-		BUG_ON(vlr.irq >= VGIC_NR_IRQS);
+		BUG_ON(vlr.irq >= dist->nr_irqs);
 		vgic_cpu->vgic_irq_lr_map[vlr.irq] = LR_EMPTY;
 	}
 
@@ -1601,7 +1602,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 	if (vcpu->vcpu_id >= dist->nr_cpus)
 		return -EBUSY;
 
-	for (i = 0; i < VGIC_NR_IRQS; i++) {
+	for (i = 0; i < dist->nr_irqs; i++) {
 		if (i < VGIC_NR_PPIS)
 			vgic_bitmap_set_irq_val(&dist->irq_enabled,
 						vcpu->vcpu_id, i, 1);
@@ -1798,7 +1799,7 @@ int kvm_vgic_init(struct kvm *kvm)
 		goto out;
 	}
 
-	for (i = VGIC_NR_PRIVATE_IRQS; i < VGIC_NR_IRQS; i += 4)
+	for (i = VGIC_NR_PRIVATE_IRQS; i < kvm->arch.vgic.nr_irqs; i += 4)
 		vgic_set_target_reg(kvm, 0, i);
 
 	kvm->arch.vgic.ready = true;
@@ -2227,7 +2228,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, KVM_MAX_VCPUS, VGIC_NR_IRQS);
+	return kvm_vgic_create(dev->kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS_LEGACY);
 }
 
 struct kvm_device_ops kvm_arm_vgic_v2_ops = {
-- 
1.8.3.4

[PATCH v2 6/9] arm/arm64: KVM: vgic: kill VGIC_NR_IRQS

[Marc Zyngier]

Nuke VGIC_NR_IRQS entierly, now that the distributor instance
contains the number of IRQ allocated to this GIC.

Also add VGIC_NR_IRQS_LEGACY to preserve the current API.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/arm.c     |  2 +-
 include/kvm/arm_vgic.h |  6 +++---
 virt/kvm/arm/vgic.c    | 13 +++++++------
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 14ba035..66c14ef 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -811,7 +811,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	switch (ioctl) {
 	case KVM_CREATE_IRQCHIP: {
 		if (vgic_present)
-			return kvm_vgic_create(kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS);
+			return kvm_vgic_create(kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS_LEGACY);
 		else
 			return -ENXIO;
 	}
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 3014145..cf17b68 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -25,7 +25,7 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
-#define VGIC_NR_IRQS		256
+#define VGIC_NR_IRQS_LEGACY	256
 #define VGIC_NR_SGIS		16
 #define VGIC_NR_PPIS		16
 #define VGIC_NR_PRIVATE_IRQS	(VGIC_NR_SGIS + VGIC_NR_PPIS)
@@ -39,11 +39,11 @@
 #error	Invalid number of CPU interfaces
 #endif
 
-#if (VGIC_NR_IRQS & 31)
+#if (VGIC_NR_IRQS_LEGACY & 31)
 #error "VGIC_NR_IRQS must be a multiple of 32"
 #endif
 
-#if (VGIC_NR_IRQS > VGIC_MAX_IRQS)
+#if (VGIC_NR_IRQS_LEGACY > VGIC_MAX_IRQS)
 #error "VGIC_NR_IRQS must be <= 1024"
 #endif
 
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 13ead5d..17ceb8a 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -373,7 +373,7 @@ static bool handle_mmio_misc(struct kvm_vcpu *vcpu,
 
 	case 4:			/* GICD_TYPER */
 		reg  = (atomic_read(&vcpu->kvm->online_vcpus) - 1) << 5;
-		reg |= (VGIC_NR_IRQS >> 5) - 1;
+		reg |= (vcpu->kvm->arch.vgic.nr_irqs >> 5) - 1;
 		vgic_reg_access(mmio, &reg, word_offset,
 				ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
 		break;
@@ -1164,13 +1164,14 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu)
 static 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;
 	struct vgic_lr vlr;
 	int lr;
 
 	/* Sanitize the input... */
 	BUG_ON(sgi_source_id & ~7);
 	BUG_ON(sgi_source_id && irq >= VGIC_NR_SGIS);
-	BUG_ON(irq >= VGIC_NR_IRQS);
+	BUG_ON(irq >= dist->nr_irqs);
 
 	kvm_debug("Queue IRQ%d\n", irq);
 
@@ -1387,7 +1388,7 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
 
 		vlr = vgic_get_lr(vcpu, lr);
 
-		BUG_ON(vlr.irq >= VGIC_NR_IRQS);
+		BUG_ON(vlr.irq >= dist->nr_irqs);
 		vgic_cpu->vgic_irq_lr_map[vlr.irq] = LR_EMPTY;
 	}
 
@@ -1601,7 +1602,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 	if (vcpu->vcpu_id >= dist->nr_cpus)
 		return -EBUSY;
 
-	for (i = 0; i < VGIC_NR_IRQS; i++) {
+	for (i = 0; i < dist->nr_irqs; i++) {
 		if (i < VGIC_NR_PPIS)
 			vgic_bitmap_set_irq_val(&dist->irq_enabled,
 						vcpu->vcpu_id, i, 1);
@@ -1798,7 +1799,7 @@ int kvm_vgic_init(struct kvm *kvm)
 		goto out;
 	}
 
-	for (i = VGIC_NR_PRIVATE_IRQS; i < VGIC_NR_IRQS; i += 4)
+	for (i = VGIC_NR_PRIVATE_IRQS; i < kvm->arch.vgic.nr_irqs; i += 4)
 		vgic_set_target_reg(kvm, 0, i);
 
 	kvm->arch.vgic.ready = true;
@@ -2227,7 +2228,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, KVM_MAX_VCPUS, VGIC_NR_IRQS);
+	return kvm_vgic_create(dev->kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS_LEGACY);
 }
 
 struct kvm_device_ops kvm_arm_vgic_v2_ops = {
-- 
1.8.3.4

[PATCH v2 7/9] arm/arm64: KVM: vgic: delay vgic allocation until init time

[Marc Zyngier]

It is now quite easy to delay the allocation of the vgic tables
until we actually require it to be up and running (when the first
starting to kick around).

This allow us to allocate memory for the exact number of CPUs we
have. As nobody configures the number of interrupts just yet,
use a fallback to VGIC_NR_IRQS_LEGACY.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/arm.c     |  9 +-------
 include/kvm/arm_vgic.h |  3 +--
 virt/kvm/arm/vgic.c    | 56 ++++++++++++++++++++++++++++++++++----------------
 3 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 66c14ef..9b3957d 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -308,16 +308,9 @@ int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
 
 int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 {
-	int ret;
-
 	/* Force users to call KVM_ARM_VCPU_INIT */
 	vcpu->arch.target = -1;
 
-	/* Set up VGIC */
-	ret = kvm_vgic_vcpu_init(vcpu);
-	if (ret)
-		return ret;
-
 	/* Set up the timer */
 	kvm_timer_vcpu_init(vcpu);
 
@@ -811,7 +804,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	switch (ioctl) {
 	case KVM_CREATE_IRQCHIP: {
 		if (vgic_present)
-			return kvm_vgic_create(kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS_LEGACY);
+			return kvm_vgic_create(kvm);
 		else
 			return -ENXIO;
 	}
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index cf17b68..f5788cf 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -231,9 +231,8 @@ 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 nr_cpus, int nr_irqs);
+int kvm_vgic_create(struct kvm *kvm);
 void kvm_vgic_destroy(struct kvm *kvm);
-int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu);
 void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu);
 void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu);
 void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 17ceb8a..6e13ff9 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1589,7 +1589,7 @@ void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
  * Initialize the vgic_cpu struct and vgic_dist struct fields pertaining to
  * this vcpu and enable the VGIC for this VCPU
  */
-int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
+static int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
@@ -1599,9 +1599,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 	if (ret)
 		return ret;
 
-	if (vcpu->vcpu_id >= dist->nr_cpus)
-		return -EBUSY;
-
 	for (i = 0; i < dist->nr_irqs; i++) {
 		if (i < VGIC_NR_PPIS)
 			vgic_bitmap_set_irq_val(&dist->irq_enabled,
@@ -1723,9 +1720,6 @@ static int vgic_init_maps(struct vgic_dist *dist, int nr_cpus, int nr_irqs)
 {
 	int ret, i;
 
-	dist->nr_cpus = nr_cpus;
-	dist->nr_irqs = nr_irqs;
-
 	ret  = vgic_init_bitmap(&dist->irq_enabled, nr_cpus, nr_irqs);
 	ret |= vgic_init_bitmap(&dist->irq_state, nr_cpus, nr_irqs);
 	ret |= vgic_init_bitmap(&dist->irq_active, nr_cpus, nr_irqs);
@@ -1775,7 +1769,9 @@ void kvm_vgic_destroy(struct kvm *kvm)
  */
 int kvm_vgic_init(struct kvm *kvm)
 {
-	int ret = 0, i;
+	struct vgic_dist *dist = &kvm->arch.vgic;
+	struct kvm_vcpu *vcpu;
+	int ret = 0, i, v;
 
 	if (!irqchip_in_kernel(kvm))
 		return 0;
@@ -1785,30 +1781,58 @@ int kvm_vgic_init(struct kvm *kvm)
 	if (vgic_initialized(kvm))
 		goto out;
 
-	if (IS_VGIC_ADDR_UNDEF(kvm->arch.vgic.vgic_dist_base) ||
-	    IS_VGIC_ADDR_UNDEF(kvm->arch.vgic.vgic_cpu_base)) {
+	dist->nr_cpus = atomic_read(&kvm->online_vcpus);
+
+	/*
+	 * If nobody configured the number of interrupts, use the
+	 * legacy one.
+	 */
+	if (!dist->nr_irqs)
+		dist->nr_irqs = VGIC_NR_IRQS_LEGACY;
+
+	if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base) ||
+	    IS_VGIC_ADDR_UNDEF(dist->vgic_cpu_base)) {
 		kvm_err("Need to set vgic cpu and dist addresses first\n");
 		ret = -ENXIO;
 		goto out;
 	}
 
-	ret = kvm_phys_addr_ioremap(kvm, kvm->arch.vgic.vgic_cpu_base,
+	ret = vgic_init_maps(dist, dist->nr_cpus, dist->nr_irqs);
+	if (ret) {
+		kvm_err("Unable to allocate maps\n");
+		goto out;
+	}
+
+	ret = kvm_phys_addr_ioremap(kvm, dist->vgic_cpu_base,
 				    vgic->vcpu_base, KVM_VGIC_V2_CPU_SIZE);
 	if (ret) {
 		kvm_err("Unable to remap VGIC CPU to VCPU\n");
 		goto out;
 	}
 
-	for (i = VGIC_NR_PRIVATE_IRQS; i < kvm->arch.vgic.nr_irqs; i += 4)
+	for (i = VGIC_NR_PRIVATE_IRQS; i < dist->nr_irqs; i += 4)
 		vgic_set_target_reg(kvm, 0, i);
 
+	kvm_for_each_vcpu(v, vcpu, kvm) {
+		ret = kvm_vgic_vcpu_init(vcpu);
+		if (ret) {
+			kvm_err("Failed to allocate vcpu memory\n");
+			break;
+		}
+	}
+
 	kvm->arch.vgic.ready = true;
 out:
+	if (ret) {
+		kvm_for_each_vcpu(v, vcpu, kvm)
+			kvm_vgic_vcpu_destroy(vcpu);
+		vgic_free_maps(dist);
+	}
 	mutex_unlock(&kvm->lock);
 	return ret;
 }
 
-int kvm_vgic_create(struct kvm *kvm, int nr_cpus, int nr_irqs)
+int kvm_vgic_create(struct kvm *kvm)
 {
 	int i, vcpu_lock_idx = -1, ret = 0;
 	struct kvm_vcpu *vcpu;
@@ -1844,10 +1868,6 @@ int kvm_vgic_create(struct kvm *kvm, int nr_cpus, int nr_irqs)
 	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
 	kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
 
-	ret = vgic_init_maps(&kvm->arch.vgic, nr_cpus, nr_irqs);
-	if (ret)
-		kvm_err("Unable to allocate maps\n");
-
 out_unlock:
 	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
 		vcpu = kvm_get_vcpu(kvm, vcpu_lock_idx);
@@ -2228,7 +2248,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, KVM_MAX_VCPUS, VGIC_NR_IRQS_LEGACY);
+	return kvm_vgic_create(dev->kvm);
 }
 
 struct kvm_device_ops kvm_arm_vgic_v2_ops = {
-- 
1.8.3.4

[PATCH v2 7/9] arm/arm64: KVM: vgic: delay vgic allocation until init time

[Marc Zyngier]

It is now quite easy to delay the allocation of the vgic tables
until we actually require it to be up and running (when the first
starting to kick around).

This allow us to allocate memory for the exact number of CPUs we
have. As nobody configures the number of interrupts just yet,
use a fallback to VGIC_NR_IRQS_LEGACY.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/arm.c     |  9 +-------
 include/kvm/arm_vgic.h |  3 +--
 virt/kvm/arm/vgic.c    | 56 ++++++++++++++++++++++++++++++++++----------------
 3 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 66c14ef..9b3957d 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -308,16 +308,9 @@ int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
 
 int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 {
-	int ret;
-
 	/* Force users to call KVM_ARM_VCPU_INIT */
 	vcpu->arch.target = -1;
 
-	/* Set up VGIC */
-	ret = kvm_vgic_vcpu_init(vcpu);
-	if (ret)
-		return ret;
-
 	/* Set up the timer */
 	kvm_timer_vcpu_init(vcpu);
 
@@ -811,7 +804,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	switch (ioctl) {
 	case KVM_CREATE_IRQCHIP: {
 		if (vgic_present)
-			return kvm_vgic_create(kvm, KVM_MAX_VCPUS, VGIC_NR_IRQS_LEGACY);
+			return kvm_vgic_create(kvm);
 		else
 			return -ENXIO;
 	}
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index cf17b68..f5788cf 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -231,9 +231,8 @@ 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 nr_cpus, int nr_irqs);
+int kvm_vgic_create(struct kvm *kvm);
 void kvm_vgic_destroy(struct kvm *kvm);
-int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu);
 void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu);
 void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu);
 void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 17ceb8a..6e13ff9 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1589,7 +1589,7 @@ void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
  * Initialize the vgic_cpu struct and vgic_dist struct fields pertaining to
  * this vcpu and enable the VGIC for this VCPU
  */
-int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
+static int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
@@ -1599,9 +1599,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 	if (ret)
 		return ret;
 
-	if (vcpu->vcpu_id >= dist->nr_cpus)
-		return -EBUSY;
-
 	for (i = 0; i < dist->nr_irqs; i++) {
 		if (i < VGIC_NR_PPIS)
 			vgic_bitmap_set_irq_val(&dist->irq_enabled,
@@ -1723,9 +1720,6 @@ static int vgic_init_maps(struct vgic_dist *dist, int nr_cpus, int nr_irqs)
 {
 	int ret, i;
 
-	dist->nr_cpus = nr_cpus;
-	dist->nr_irqs = nr_irqs;
-
 	ret  = vgic_init_bitmap(&dist->irq_enabled, nr_cpus, nr_irqs);
 	ret |= vgic_init_bitmap(&dist->irq_state, nr_cpus, nr_irqs);
 	ret |= vgic_init_bitmap(&dist->irq_active, nr_cpus, nr_irqs);
@@ -1775,7 +1769,9 @@ void kvm_vgic_destroy(struct kvm *kvm)
  */
 int kvm_vgic_init(struct kvm *kvm)
 {
-	int ret = 0, i;
+	struct vgic_dist *dist = &kvm->arch.vgic;
+	struct kvm_vcpu *vcpu;
+	int ret = 0, i, v;
 
 	if (!irqchip_in_kernel(kvm))
 		return 0;
@@ -1785,30 +1781,58 @@ int kvm_vgic_init(struct kvm *kvm)
 	if (vgic_initialized(kvm))
 		goto out;
 
-	if (IS_VGIC_ADDR_UNDEF(kvm->arch.vgic.vgic_dist_base) ||
-	    IS_VGIC_ADDR_UNDEF(kvm->arch.vgic.vgic_cpu_base)) {
+	dist->nr_cpus = atomic_read(&kvm->online_vcpus);
+
+	/*
+	 * If nobody configured the number of interrupts, use the
+	 * legacy one.
+	 */
+	if (!dist->nr_irqs)
+		dist->nr_irqs = VGIC_NR_IRQS_LEGACY;
+
+	if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base) ||
+	    IS_VGIC_ADDR_UNDEF(dist->vgic_cpu_base)) {
 		kvm_err("Need to set vgic cpu and dist addresses first\n");
 		ret = -ENXIO;
 		goto out;
 	}
 
-	ret = kvm_phys_addr_ioremap(kvm, kvm->arch.vgic.vgic_cpu_base,
+	ret = vgic_init_maps(dist, dist->nr_cpus, dist->nr_irqs);
+	if (ret) {
+		kvm_err("Unable to allocate maps\n");
+		goto out;
+	}
+
+	ret = kvm_phys_addr_ioremap(kvm, dist->vgic_cpu_base,
 				    vgic->vcpu_base, KVM_VGIC_V2_CPU_SIZE);
 	if (ret) {
 		kvm_err("Unable to remap VGIC CPU to VCPU\n");
 		goto out;
 	}
 
-	for (i = VGIC_NR_PRIVATE_IRQS; i < kvm->arch.vgic.nr_irqs; i += 4)
+	for (i = VGIC_NR_PRIVATE_IRQS; i < dist->nr_irqs; i += 4)
 		vgic_set_target_reg(kvm, 0, i);
 
+	kvm_for_each_vcpu(v, vcpu, kvm) {
+		ret = kvm_vgic_vcpu_init(vcpu);
+		if (ret) {
+			kvm_err("Failed to allocate vcpu memory\n");
+			break;
+		}
+	}
+
 	kvm->arch.vgic.ready = true;
 out:
+	if (ret) {
+		kvm_for_each_vcpu(v, vcpu, kvm)
+			kvm_vgic_vcpu_destroy(vcpu);
+		vgic_free_maps(dist);
+	}
 	mutex_unlock(&kvm->lock);
 	return ret;
 }
 
-int kvm_vgic_create(struct kvm *kvm, int nr_cpus, int nr_irqs)
+int kvm_vgic_create(struct kvm *kvm)
 {
 	int i, vcpu_lock_idx = -1, ret = 0;
 	struct kvm_vcpu *vcpu;
@@ -1844,10 +1868,6 @@ int kvm_vgic_create(struct kvm *kvm, int nr_cpus, int nr_irqs)
 	kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
 	kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
 
-	ret = vgic_init_maps(&kvm->arch.vgic, nr_cpus, nr_irqs);
-	if (ret)
-		kvm_err("Unable to allocate maps\n");
-
 out_unlock:
 	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
 		vcpu = kvm_get_vcpu(kvm, vcpu_lock_idx);
@@ -2228,7 +2248,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, KVM_MAX_VCPUS, VGIC_NR_IRQS_LEGACY);
+	return kvm_vgic_create(dev->kvm);
 }
 
 struct kvm_device_ops kvm_arm_vgic_v2_ops = {
-- 
1.8.3.4

[PATCH v2 8/9] arm/arm64: KVM: vgic: make number of irqs a configurable attribute

[Marc Zyngier]

In order to make the number of interrupt configurable, use the new
fancy device management API to add KVM_DEV_ARM_VGIC_GRP_NR_IRQS as
a VGIC configurable attribute.

Userspace can now specify the exact size of the GIC (by increments
of 32 interrupts).

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/uapi/asm/kvm.h   |  1 +
 arch/arm64/include/uapi/asm/kvm.h |  1 +
 virt/kvm/arm/vgic.c               | 29 +++++++++++++++++++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index e6ebdd3..8b51c1a 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -173,6 +173,7 @@ struct kvm_arch_memory_slot {
 #define   KVM_DEV_ARM_VGIC_CPUID_MASK	(0xffULL << KVM_DEV_ARM_VGIC_CPUID_SHIFT)
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
+#define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
 
 /* KVM_IRQ_LINE irq field index values */
 #define KVM_ARM_IRQ_TYPE_SHIFT		24
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index e633ff8..b5cd6ed 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -159,6 +159,7 @@ struct kvm_arch_memory_slot {
 #define   KVM_DEV_ARM_VGIC_CPUID_MASK	(0xffULL << KVM_DEV_ARM_VGIC_CPUID_SHIFT)
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
+#define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
 
 /* KVM_IRQ_LINE irq field index values */
 #define KVM_ARM_IRQ_TYPE_SHIFT		24
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 6e13ff9..b0cd417 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2165,6 +2165,28 @@ static int vgic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 
 		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;
+		int ret = 0;
+
+		if (get_user(val, uaddr))
+			return -EFAULT;
+
+		if (val > 1024 || (val & 31))
+			return -EINVAL;
+
+		mutex_lock(&dev->kvm->lock);
+
+		if (vgic_initialized(dev->kvm))
+			ret = -EBUSY;
+		else
+			dev->kvm->arch.vgic.nr_irqs = val;
+
+		mutex_unlock(&dev->kvm->lock);
+
+		return ret;
+	}
 
 	}
 
@@ -2201,6 +2223,11 @@ static int vgic_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 		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;
+	}
 
 	}
 
@@ -2237,6 +2264,8 @@ static int vgic_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 	case KVM_DEV_ARM_VGIC_GRP_CPU_REGS:
 		offset = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK;
 		return vgic_has_attr_regs(vgic_cpu_ranges, offset);
+	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
+		return 0;
 	}
 	return -ENXIO;
 }
-- 
1.8.3.4

[PATCH v2 8/9] arm/arm64: KVM: vgic: make number of irqs a configurable attribute

[Marc Zyngier]

In order to make the number of interrupt configurable, use the new
fancy device management API to add KVM_DEV_ARM_VGIC_GRP_NR_IRQS as
a VGIC configurable attribute.

Userspace can now specify the exact size of the GIC (by increments
of 32 interrupts).

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/uapi/asm/kvm.h   |  1 +
 arch/arm64/include/uapi/asm/kvm.h |  1 +
 virt/kvm/arm/vgic.c               | 29 +++++++++++++++++++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index e6ebdd3..8b51c1a 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -173,6 +173,7 @@ struct kvm_arch_memory_slot {
 #define   KVM_DEV_ARM_VGIC_CPUID_MASK	(0xffULL << KVM_DEV_ARM_VGIC_CPUID_SHIFT)
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
+#define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
 
 /* KVM_IRQ_LINE irq field index values */
 #define KVM_ARM_IRQ_TYPE_SHIFT		24
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index e633ff8..b5cd6ed 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -159,6 +159,7 @@ struct kvm_arch_memory_slot {
 #define   KVM_DEV_ARM_VGIC_CPUID_MASK	(0xffULL << KVM_DEV_ARM_VGIC_CPUID_SHIFT)
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
+#define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
 
 /* KVM_IRQ_LINE irq field index values */
 #define KVM_ARM_IRQ_TYPE_SHIFT		24
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 6e13ff9..b0cd417 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2165,6 +2165,28 @@ static int vgic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 
 		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;
+		int ret = 0;
+
+		if (get_user(val, uaddr))
+			return -EFAULT;
+
+		if (val > 1024 || (val & 31))
+			return -EINVAL;
+
+		mutex_lock(&dev->kvm->lock);
+
+		if (vgic_initialized(dev->kvm))
+			ret = -EBUSY;
+		else
+			dev->kvm->arch.vgic.nr_irqs = val;
+
+		mutex_unlock(&dev->kvm->lock);
+
+		return ret;
+	}
 
 	}
 
@@ -2201,6 +2223,11 @@ static int vgic_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 		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;
+	}
 
 	}
 
@@ -2237,6 +2264,8 @@ static int vgic_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 	case KVM_DEV_ARM_VGIC_GRP_CPU_REGS:
 		offset = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK;
 		return vgic_has_attr_regs(vgic_cpu_ranges, offset);
+	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
+		return 0;
 	}
 	return -ENXIO;
 }
-- 
1.8.3.4

[PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Marc Zyngier]

The GIC CPU interface is always 4k aligned. If the host is using
64k pages, it is critical to place the guest's GICC interface at the
same relative alignment as the host's GICV. Failure to do so results
in an impossibility for the guest to deal with interrupts.

Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
userspace to retrieve the GICV offset in a page. It becomes then trivial
to adjust the GICC base address for the guest.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/uapi/asm/kvm.h   | 1 +
 arch/arm64/include/uapi/asm/kvm.h | 1 +
 virt/kvm/arm/vgic.c               | 7 +++++++
 3 files changed, 9 insertions(+)

diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index 8b51c1a..056b782 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -174,6 +174,7 @@ struct kvm_arch_memory_slot {
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
 #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
+#define KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET 4
 
 /* KVM_IRQ_LINE irq field index values */
 #define KVM_ARM_IRQ_TYPE_SHIFT		24
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index b5cd6ed..5513de4 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -160,6 +160,7 @@ struct kvm_arch_memory_slot {
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
 #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
+#define KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET 4
 
 /* KVM_IRQ_LINE irq field index values */
 #define KVM_ARM_IRQ_TYPE_SHIFT		24
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index b0cd417..68ac9c6 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2228,6 +2228,12 @@ static int vgic_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 		r = put_user(dev->kvm->arch.vgic.nr_irqs, uaddr);
 		break;
 	}
+	case KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET: {
+		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
+		u32 val = vgic->vcpu_base & ~PAGE_MASK;
+		r = put_user(val, uaddr);
+		break;
+	}
 
 	}
 
@@ -2265,6 +2271,7 @@ static int vgic_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 		offset = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK;
 		return vgic_has_attr_regs(vgic_cpu_ranges, offset);
 	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
+	case KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET:
 		return 0;
 	}
 	return -ENXIO;
-- 
1.8.3.4

[PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Marc Zyngier]

The GIC CPU interface is always 4k aligned. If the host is using
64k pages, it is critical to place the guest's GICC interface at the
same relative alignment as the host's GICV. Failure to do so results
in an impossibility for the guest to deal with interrupts.

Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
userspace to retrieve the GICV offset in a page. It becomes then trivial
to adjust the GICC base address for the guest.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/uapi/asm/kvm.h   | 1 +
 arch/arm64/include/uapi/asm/kvm.h | 1 +
 virt/kvm/arm/vgic.c               | 7 +++++++
 3 files changed, 9 insertions(+)

diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index 8b51c1a..056b782 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -174,6 +174,7 @@ struct kvm_arch_memory_slot {
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
 #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
+#define KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET 4
 
 /* KVM_IRQ_LINE irq field index values */
 #define KVM_ARM_IRQ_TYPE_SHIFT		24
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index b5cd6ed..5513de4 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -160,6 +160,7 @@ struct kvm_arch_memory_slot {
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
 #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
+#define KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET 4
 
 /* KVM_IRQ_LINE irq field index values */
 #define KVM_ARM_IRQ_TYPE_SHIFT		24
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index b0cd417..68ac9c6 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2228,6 +2228,12 @@ static int vgic_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 		r = put_user(dev->kvm->arch.vgic.nr_irqs, uaddr);
 		break;
 	}
+	case KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET: {
+		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
+		u32 val = vgic->vcpu_base & ~PAGE_MASK;
+		r = put_user(val, uaddr);
+		break;
+	}
 
 	}
 
@@ -2265,6 +2271,7 @@ static int vgic_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 		offset = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK;
 		return vgic_has_attr_regs(vgic_cpu_ranges, offset);
 	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
+	case KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET:
 		return 0;
 	}
 	return -ENXIO;
-- 
1.8.3.4

Re: [PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Joel Schopp]

On 06/19/2014 04:21 AM, Marc Zyngier wrote:
> The GIC CPU interface is always 4k aligned. If the host is using
> 64k pages, it is critical to place the guest's GICC interface at the
> same relative alignment as the host's GICV. Failure to do so results
> in an impossibility for the guest to deal with interrupts.
>
> Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
> userspace to retrieve the GICV offset in a page. It becomes then trivial
> to adjust the GICC base address for the guest.

Does this mean there is a corresponding patch for qemu?

>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   arch/arm/include/uapi/asm/kvm.h   | 1 +
>   arch/arm64/include/uapi/asm/kvm.h | 1 +
>   virt/kvm/arm/vgic.c               | 7 +++++++
>   3 files changed, 9 insertions(+)
>
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index 8b51c1a..056b782 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -174,6 +174,7 @@ struct kvm_arch_memory_slot {
>   #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
>   #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
>   #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
> +#define KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET 4
>   
>   /* KVM_IRQ_LINE irq field index values */
>   #define KVM_ARM_IRQ_TYPE_SHIFT		24
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index b5cd6ed..5513de4 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -160,6 +160,7 @@ struct kvm_arch_memory_slot {
>   #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
>   #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
>   #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
> +#define KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET 4
>   
>   /* KVM_IRQ_LINE irq field index values */
>   #define KVM_ARM_IRQ_TYPE_SHIFT		24
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index b0cd417..68ac9c6 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -2228,6 +2228,12 @@ static int vgic_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
>   		r = put_user(dev->kvm->arch.vgic.nr_irqs, uaddr);
>   		break;
>   	}
> +	case KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET: {
> +		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> +		u32 val = vgic->vcpu_base & ~PAGE_MASK;
> +		r = put_user(val, uaddr);
> +		break;
> +	}
>   
>   	}
>   
> @@ -2265,6 +2271,7 @@ static int vgic_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
>   		offset = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK;
>   		return vgic_has_attr_regs(vgic_cpu_ranges, offset);
>   	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
> +	case KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET:
>   		return 0;
>   	}
>   	return -ENXIO;

[PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Joel Schopp]

On 06/19/2014 04:21 AM, Marc Zyngier wrote:
> The GIC CPU interface is always 4k aligned. If the host is using
> 64k pages, it is critical to place the guest's GICC interface at the
> same relative alignment as the host's GICV. Failure to do so results
> in an impossibility for the guest to deal with interrupts.
>
> Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
> userspace to retrieve the GICV offset in a page. It becomes then trivial
> to adjust the GICC base address for the guest.

Does this mean there is a corresponding patch for qemu?

>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   arch/arm/include/uapi/asm/kvm.h   | 1 +
>   arch/arm64/include/uapi/asm/kvm.h | 1 +
>   virt/kvm/arm/vgic.c               | 7 +++++++
>   3 files changed, 9 insertions(+)
>
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index 8b51c1a..056b782 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -174,6 +174,7 @@ struct kvm_arch_memory_slot {
>   #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
>   #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
>   #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
> +#define KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET 4
>   
>   /* KVM_IRQ_LINE irq field index values */
>   #define KVM_ARM_IRQ_TYPE_SHIFT		24
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index b5cd6ed..5513de4 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -160,6 +160,7 @@ struct kvm_arch_memory_slot {
>   #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
>   #define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
>   #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS	3
> +#define KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET 4
>   
>   /* KVM_IRQ_LINE irq field index values */
>   #define KVM_ARM_IRQ_TYPE_SHIFT		24
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index b0cd417..68ac9c6 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -2228,6 +2228,12 @@ static int vgic_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
>   		r = put_user(dev->kvm->arch.vgic.nr_irqs, uaddr);
>   		break;
>   	}
> +	case KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET: {
> +		u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> +		u32 val = vgic->vcpu_base & ~PAGE_MASK;
> +		r = put_user(val, uaddr);
> +		break;
> +	}
>   
>   	}
>   
> @@ -2265,6 +2271,7 @@ static int vgic_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
>   		offset = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK;
>   		return vgic_has_attr_regs(vgic_cpu_ranges, offset);
>   	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
> +	case KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET:
>   		return 0;
>   	}
>   	return -ENXIO;

Re: [PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Peter Maydell]

On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>
> On 06/19/2014 04:21 AM, Marc Zyngier wrote:
>>
>> The GIC CPU interface is always 4k aligned. If the host is using
>> 64k pages, it is critical to place the guest's GICC interface at the
>> same relative alignment as the host's GICV. Failure to do so results
>> in an impossibility for the guest to deal with interrupts.
>>
>> Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
>> userspace to retrieve the GICV offset in a page. It becomes then trivial
>> to adjust the GICC base address for the guest.
>
>
> Does this mean there is a corresponding patch for qemu?

Not as far as I know. It's a bit awkward on the QEMU end because
we really want to provide the guest a consistent memory map
regardless of the host CPU. So at best we'd probably use it to
say "sorry, can't run on this CPU/host kernel".

(That said, if you think you can make QEMU usefully use the
information and want to write a QEMU patch I'm not averse
to the idea.)

kvmtool is probably better placed to take advantage of it since
it takes more of a "deal with what the host provides you"
philosophy.

thanks
-- PMM

[PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Peter Maydell]

On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>
> On 06/19/2014 04:21 AM, Marc Zyngier wrote:
>>
>> The GIC CPU interface is always 4k aligned. If the host is using
>> 64k pages, it is critical to place the guest's GICC interface at the
>> same relative alignment as the host's GICV. Failure to do so results
>> in an impossibility for the guest to deal with interrupts.
>>
>> Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
>> userspace to retrieve the GICV offset in a page. It becomes then trivial
>> to adjust the GICC base address for the guest.
>
>
> Does this mean there is a corresponding patch for qemu?

Not as far as I know. It's a bit awkward on the QEMU end because
we really want to provide the guest a consistent memory map
regardless of the host CPU. So at best we'd probably use it to
say "sorry, can't run on this CPU/host kernel".

(That said, if you think you can make QEMU usefully use the
information and want to write a QEMU patch I'm not averse
to the idea.)

kvmtool is probably better placed to take advantage of it since
it takes more of a "deal with what the host provides you"
philosophy.

thanks
-- PMM

Re: [PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Joel Schopp]

On 06/24/2014 05:28 PM, Peter Maydell wrote:
> On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>> On 06/19/2014 04:21 AM, Marc Zyngier wrote:
>>> The GIC CPU interface is always 4k aligned. If the host is using
>>> 64k pages, it is critical to place the guest's GICC interface at the
>>> same relative alignment as the host's GICV. Failure to do so results
>>> in an impossibility for the guest to deal with interrupts.
>>>
>>> Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
>>> userspace to retrieve the GICV offset in a page. It becomes then trivial
>>> to adjust the GICC base address for the guest.
>>
>> Does this mean there is a corresponding patch for qemu?
> Not as far as I know. It's a bit awkward on the QEMU end because
> we really want to provide the guest a consistent memory map
> regardless of the host CPU. So at best we'd probably use it to
> say "sorry, can't run on this CPU/host kernel".
I think most arm64 servers are going to run with 64k pages.  It seems 
like a major problem to have qemu not work on these systems.

>
> (That said, if you think you can make QEMU usefully use the
> information and want to write a QEMU patch I'm not averse
> to the idea.)
I'll have to think about this approach some more, but I'm not opposed to 
doing the work if I thought it was the right thing to do.

>
> kvmtool is probably better placed to take advantage of it since
> it takes more of a "deal with what the host provides you"
> philosophy.
kvmtool is fun as a play toy, but in the real world nobody is building 
clouds using kvmtool, they use kvm with qemu.

>
> thanks
> -- PMM

[PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Joel Schopp]

On 06/24/2014 05:28 PM, Peter Maydell wrote:
> On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>> On 06/19/2014 04:21 AM, Marc Zyngier wrote:
>>> The GIC CPU interface is always 4k aligned. If the host is using
>>> 64k pages, it is critical to place the guest's GICC interface at the
>>> same relative alignment as the host's GICV. Failure to do so results
>>> in an impossibility for the guest to deal with interrupts.
>>>
>>> Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
>>> userspace to retrieve the GICV offset in a page. It becomes then trivial
>>> to adjust the GICC base address for the guest.
>>
>> Does this mean there is a corresponding patch for qemu?
> Not as far as I know. It's a bit awkward on the QEMU end because
> we really want to provide the guest a consistent memory map
> regardless of the host CPU. So at best we'd probably use it to
> say "sorry, can't run on this CPU/host kernel".
I think most arm64 servers are going to run with 64k pages.  It seems 
like a major problem to have qemu not work on these systems.

>
> (That said, if you think you can make QEMU usefully use the
> information and want to write a QEMU patch I'm not averse
> to the idea.)
I'll have to think about this approach some more, but I'm not opposed to 
doing the work if I thought it was the right thing to do.

>
> kvmtool is probably better placed to take advantage of it since
> it takes more of a "deal with what the host provides you"
> philosophy.
kvmtool is fun as a play toy, but in the real world nobody is building 
clouds using kvmtool, they use kvm with qemu.

>
> thanks
> -- PMM

Re: [PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Marc Zyngier]

On 25/06/14 15:56, Joel Schopp wrote:
> 
> On 06/24/2014 05:28 PM, Peter Maydell wrote:
>> On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>>> On 06/19/2014 04:21 AM, Marc Zyngier wrote:
>>>> The GIC CPU interface is always 4k aligned. If the host is using
>>>> 64k pages, it is critical to place the guest's GICC interface at the
>>>> same relative alignment as the host's GICV. Failure to do so results
>>>> in an impossibility for the guest to deal with interrupts.
>>>>
>>>> Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
>>>> userspace to retrieve the GICV offset in a page. It becomes then trivial
>>>> to adjust the GICC base address for the guest.
>>>
>>> Does this mean there is a corresponding patch for qemu?
>> Not as far as I know. It's a bit awkward on the QEMU end because
>> we really want to provide the guest a consistent memory map
>> regardless of the host CPU. So at best we'd probably use it to
>> say "sorry, can't run on this CPU/host kernel".
> I think most arm64 servers are going to run with 64k pages.  It seems 
> like a major problem to have qemu not work on these systems.

How many of them will be with the GICC *not* 64kB aligned?

>>
>> (That said, if you think you can make QEMU usefully use the
>> information and want to write a QEMU patch I'm not averse
>> to the idea.)
> I'll have to think about this approach some more, but I'm not opposed to 
> doing the work if I thought it was the right thing to do.
> 
>>
>> kvmtool is probably better placed to take advantage of it since
>> it takes more of a "deal with what the host provides you"
>> philosophy.
> kvmtool is fun as a play toy, but in the real world nobody is building 
> clouds using kvmtool, they use kvm with qemu.

A play toy? Hmmm. Do you realise that most of KVM on arm64 has been
written using this play toy?

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Marc Zyngier]

On 25/06/14 15:56, Joel Schopp wrote:
> 
> On 06/24/2014 05:28 PM, Peter Maydell wrote:
>> On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>>> On 06/19/2014 04:21 AM, Marc Zyngier wrote:
>>>> The GIC CPU interface is always 4k aligned. If the host is using
>>>> 64k pages, it is critical to place the guest's GICC interface at the
>>>> same relative alignment as the host's GICV. Failure to do so results
>>>> in an impossibility for the guest to deal with interrupts.
>>>>
>>>> Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
>>>> userspace to retrieve the GICV offset in a page. It becomes then trivial
>>>> to adjust the GICC base address for the guest.
>>>
>>> Does this mean there is a corresponding patch for qemu?
>> Not as far as I know. It's a bit awkward on the QEMU end because
>> we really want to provide the guest a consistent memory map
>> regardless of the host CPU. So at best we'd probably use it to
>> say "sorry, can't run on this CPU/host kernel".
> I think most arm64 servers are going to run with 64k pages.  It seems 
> like a major problem to have qemu not work on these systems.

How many of them will be with the GICC *not* 64kB aligned?

>>
>> (That said, if you think you can make QEMU usefully use the
>> information and want to write a QEMU patch I'm not averse
>> to the idea.)
> I'll have to think about this approach some more, but I'm not opposed to 
> doing the work if I thought it was the right thing to do.
> 
>>
>> kvmtool is probably better placed to take advantage of it since
>> it takes more of a "deal with what the host provides you"
>> philosophy.
> kvmtool is fun as a play toy, but in the real world nobody is building 
> clouds using kvmtool, they use kvm with qemu.

A play toy? Hmmm. Do you realise that most of KVM on arm64 has been
written using this play toy?

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Joel Schopp]

On 06/25/2014 10:00 AM, Marc Zyngier wrote:
> On 25/06/14 15:56, Joel Schopp wrote:
>> On 06/24/2014 05:28 PM, Peter Maydell wrote:
>>> On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>>>> On 06/19/2014 04:21 AM, Marc Zyngier wrote:
>>>>> The GIC CPU interface is always 4k aligned. If the host is using
>>>>> 64k pages, it is critical to place the guest's GICC interface at the
>>>>> same relative alignment as the host's GICV. Failure to do so results
>>>>> in an impossibility for the guest to deal with interrupts.
>>>>>
>>>>> Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
>>>>> userspace to retrieve the GICV offset in a page. It becomes then trivial
>>>>> to adjust the GICC base address for the guest.
>>>> Does this mean there is a corresponding patch for qemu?
>>> Not as far as I know. It's a bit awkward on the QEMU end because
>>> we really want to provide the guest a consistent memory map
>>> regardless of the host CPU. So at best we'd probably use it to
>>> say "sorry, can't run on this CPU/host kernel".
>> I think most arm64 servers are going to run with 64k pages.  It seems
>> like a major problem to have qemu not work on these systems.
> How many of them will be with the GICC *not* 64kB aligned?

If I'm reading the Server Base System Architecture v2.2 Appendix F 
correctly all of them.  Here's the relevant quote: "In a 64KB 
translation granule system this means that GICC needs to have its base 
at 4KB below a 64KB boundary."
>
>>> (That said, if you think you can make QEMU usefully use the
>>> information and want to write a QEMU patch I'm not averse
>>> to the idea.)
>> I'll have to think about this approach some more, but I'm not opposed to
>> doing the work if I thought it was the right thing to do.
>>
>>> kvmtool is probably better placed to take advantage of it since
>>> it takes more of a "deal with what the host provides you"
>>> philosophy.
>> kvmtool is fun as a play toy, but in the real world nobody is building
>> clouds using kvmtool, they use kvm with qemu.
> A play toy? Hmmm. Do you realise that most of KVM on arm64 has been
> written using this play toy?

I meant no insult.  I really like kvmtool.  I'm just saying that the 
eventual end users of these systems will want to run qemu and not kvmtool.

[PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Joel Schopp]

On 06/25/2014 10:00 AM, Marc Zyngier wrote:
> On 25/06/14 15:56, Joel Schopp wrote:
>> On 06/24/2014 05:28 PM, Peter Maydell wrote:
>>> On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>>>> On 06/19/2014 04:21 AM, Marc Zyngier wrote:
>>>>> The GIC CPU interface is always 4k aligned. If the host is using
>>>>> 64k pages, it is critical to place the guest's GICC interface at the
>>>>> same relative alignment as the host's GICV. Failure to do so results
>>>>> in an impossibility for the guest to deal with interrupts.
>>>>>
>>>>> Add a KVM_DEV_ARM_VGIC_GRP_ADDR_OFFSET attribute for the VGIC, allowing
>>>>> userspace to retrieve the GICV offset in a page. It becomes then trivial
>>>>> to adjust the GICC base address for the guest.
>>>> Does this mean there is a corresponding patch for qemu?
>>> Not as far as I know. It's a bit awkward on the QEMU end because
>>> we really want to provide the guest a consistent memory map
>>> regardless of the host CPU. So at best we'd probably use it to
>>> say "sorry, can't run on this CPU/host kernel".
>> I think most arm64 servers are going to run with 64k pages.  It seems
>> like a major problem to have qemu not work on these systems.
> How many of them will be with the GICC *not* 64kB aligned?

If I'm reading the Server Base System Architecture v2.2 Appendix F 
correctly all of them.  Here's the relevant quote: "In a 64KB 
translation granule system this means that GICC needs to have its base 
at 4KB below a 64KB boundary."
>
>>> (That said, if you think you can make QEMU usefully use the
>>> information and want to write a QEMU patch I'm not averse
>>> to the idea.)
>> I'll have to think about this approach some more, but I'm not opposed to
>> doing the work if I thought it was the right thing to do.
>>
>>> kvmtool is probably better placed to take advantage of it since
>>> it takes more of a "deal with what the host provides you"
>>> philosophy.
>> kvmtool is fun as a play toy, but in the real world nobody is building
>> clouds using kvmtool, they use kvm with qemu.
> A play toy? Hmmm. Do you realise that most of KVM on arm64 has been
> written using this play toy?

I meant no insult.  I really like kvmtool.  I'm just saying that the 
eventual end users of these systems will want to run qemu and not kvmtool.

Re: [PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Peter Maydell]

On 25 June 2014 15:56, Joel Schopp <joel.schopp@amd.com> wrote:
> On 06/24/2014 05:28 PM, Peter Maydell wrote:
>> On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>>> Does this mean there is a corresponding patch for qemu?
>>
>> Not as far as I know. It's a bit awkward on the QEMU end because
>> we really want to provide the guest a consistent memory map
>> regardless of the host CPU. So at best we'd probably use it to
>> say "sorry, can't run on this CPU/host kernel".
>
> I think most arm64 servers are going to run with 64k pages.  It seems like a
> major problem to have qemu not work on these systems.

QEMU should already work fine on servers with 64K pages;
you just need to have the host offset of the GICV within the 64K page
and the guest offset of the GICC within the 64K page be the same
(and at the moment both must also be zero, which I believe is true
for all of them at the moment except possibly the AEM model;
counterexamples welcome). Disclaimer: I haven't personally
tested this, but on the other hand I don't think anybody's
reported it as not working either.

Notice that we don't care at all about the host's GICC offset,
because it's the GICV we're going to use as the guest GICC.

That said, yes, QEMU ought really to be able to provide
support for "use what the host provides", in the same way
that we support "-cpu host" to mean 'virtualize whatever CPU
the host has'. It's just a little awkward because you're working
against the grain of some of QEMU's design; but it ought
to be usable for things like the "virt" machine model.

For the cases where QEMU is being used to emulate
specific hardware to the guest (which we don't do right
now because we don't model any 64 bit boards other than
virt), we could use this ioctl to say "can't run this guest
on this host"; this is basically diagnosing a case in the
same class as "can't run a guest with a GICv2 if your
host's GICv3 doesn't implement v2 compatibility mode".

thanks
-- PMM

[PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Peter Maydell]

On 25 June 2014 15:56, Joel Schopp <joel.schopp@amd.com> wrote:
> On 06/24/2014 05:28 PM, Peter Maydell wrote:
>> On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>>> Does this mean there is a corresponding patch for qemu?
>>
>> Not as far as I know. It's a bit awkward on the QEMU end because
>> we really want to provide the guest a consistent memory map
>> regardless of the host CPU. So at best we'd probably use it to
>> say "sorry, can't run on this CPU/host kernel".
>
> I think most arm64 servers are going to run with 64k pages.  It seems like a
> major problem to have qemu not work on these systems.

QEMU should already work fine on servers with 64K pages;
you just need to have the host offset of the GICV within the 64K page
and the guest offset of the GICC within the 64K page be the same
(and at the moment both must also be zero, which I believe is true
for all of them at the moment except possibly the AEM model;
counterexamples welcome). Disclaimer: I haven't personally
tested this, but on the other hand I don't think anybody's
reported it as not working either.

Notice that we don't care at all about the host's GICC offset,
because it's the GICV we're going to use as the guest GICC.

That said, yes, QEMU ought really to be able to provide
support for "use what the host provides", in the same way
that we support "-cpu host" to mean 'virtualize whatever CPU
the host has'. It's just a little awkward because you're working
against the grain of some of QEMU's design; but it ought
to be usable for things like the "virt" machine model.

For the cases where QEMU is being used to emulate
specific hardware to the guest (which we don't do right
now because we don't model any 64 bit boards other than
virt), we could use this ioctl to say "can't run this guest
on this host"; this is basically diagnosing a case in the
same class as "can't run a guest with a GICv2 if your
host's GICv3 doesn't implement v2 compatibility mode".

thanks
-- PMM

Re: [PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Joel Schopp]

On 06/25/2014 12:34 PM, Peter Maydell wrote:
> On 25 June 2014 15:56, Joel Schopp <joel.schopp@amd.com> wrote:
>> On 06/24/2014 05:28 PM, Peter Maydell wrote:
>>> On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>>>> Does this mean there is a corresponding patch for qemu?
>>> Not as far as I know. It's a bit awkward on the QEMU end because
>>> we really want to provide the guest a consistent memory map
>>> regardless of the host CPU. So at best we'd probably use it to
>>> say "sorry, can't run on this CPU/host kernel".
>> I think most arm64 servers are going to run with 64k pages.  It seems like a
>> major problem to have qemu not work on these systems.
> QEMU should already work fine on servers with 64K pages;
> you just need to have the host offset of the GICV within the 64K page
> and the guest offset of the GICC within the 64K page be the same
> (and at the moment both must also be zero, which I believe is true
> for all of them at the moment except possibly the AEM model;
> counterexamples welcome). Disclaimer: I haven't personally
> tested this, but on the other hand I don't think anybody's
> reported it as not working either.

It doesn't work for me.  Maybe I'm doing something wrong, but I can't 
see what.  I am unique in that I'm running a gic-400 (gicv2m) on aarch64 
hardware with 64k pages.  I'm also unique in that my hardware maps each 
4K gic entry to a 64K page (aliasing each 4k of gic 16 times in a 64K 
page, ie the gic virtual ic is at 0xe1140000 and 0xe1141000 and 
0xe1142000, etc).  This is inline with appendix F of the server base 
system architecture.  This is inconvenient when the size is 0x2000 
(8K).  As a result all the offsets in the device tree entries are to the 
last 4K in the page so that an 8K read will read the last 4k from one 
page and the first 4k from the next and actually get 8k of the gic.


         gic: interrupt-controller@e1101000 {
                 compatible = "arm,gic-400";
                 #interrupt-cells = <3>;
                 #address-cells = <0>;
                 interrupt-controller;
                 msi-controller;
                 reg = <0x0 0xe1110000 0 0x1000>, /* gic dist */
                       <0x0 0xe112f000 0 0x2000>, /* gic cpu */
                       <0x0 0xe114f000 0 0x2000>, /* gic virtual ic*/
                       <0x0 0xe116f000 0 0x2000>, /* gic virtual cpu*/
                       <0x0 0xe1180000 0 0x1000>; /* gic msi */

                 interrupts = <1 8 0xf04>;
         };


My concern here is that if userspace is going to look at 8k starting at 
the beginning of the page, guest offset 0 in your terminology, (say 
0xe1140000) instead of starting at the last 4k of the page, offset 
0xf000 (say 0xe114f000) it is going to get the second 4k wrong by 
reading 0xe1141000 instead of 0xe1150000.

[PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Joel Schopp]

On 06/25/2014 12:34 PM, Peter Maydell wrote:
> On 25 June 2014 15:56, Joel Schopp <joel.schopp@amd.com> wrote:
>> On 06/24/2014 05:28 PM, Peter Maydell wrote:
>>> On 24 June 2014 20:28, Joel Schopp <joel.schopp@amd.com> wrote:
>>>> Does this mean there is a corresponding patch for qemu?
>>> Not as far as I know. It's a bit awkward on the QEMU end because
>>> we really want to provide the guest a consistent memory map
>>> regardless of the host CPU. So at best we'd probably use it to
>>> say "sorry, can't run on this CPU/host kernel".
>> I think most arm64 servers are going to run with 64k pages.  It seems like a
>> major problem to have qemu not work on these systems.
> QEMU should already work fine on servers with 64K pages;
> you just need to have the host offset of the GICV within the 64K page
> and the guest offset of the GICC within the 64K page be the same
> (and at the moment both must also be zero, which I believe is true
> for all of them at the moment except possibly the AEM model;
> counterexamples welcome). Disclaimer: I haven't personally
> tested this, but on the other hand I don't think anybody's
> reported it as not working either.

It doesn't work for me.  Maybe I'm doing something wrong, but I can't 
see what.  I am unique in that I'm running a gic-400 (gicv2m) on aarch64 
hardware with 64k pages.  I'm also unique in that my hardware maps each 
4K gic entry to a 64K page (aliasing each 4k of gic 16 times in a 64K 
page, ie the gic virtual ic is at 0xe1140000 and 0xe1141000 and 
0xe1142000, etc).  This is inline with appendix F of the server base 
system architecture.  This is inconvenient when the size is 0x2000 
(8K).  As a result all the offsets in the device tree entries are to the 
last 4K in the page so that an 8K read will read the last 4k from one 
page and the first 4k from the next and actually get 8k of the gic.


         gic: interrupt-controller at e1101000 {
                 compatible = "arm,gic-400";
                 #interrupt-cells = <3>;
                 #address-cells = <0>;
                 interrupt-controller;
                 msi-controller;
                 reg = <0x0 0xe1110000 0 0x1000>, /* gic dist */
                       <0x0 0xe112f000 0 0x2000>, /* gic cpu */
                       <0x0 0xe114f000 0 0x2000>, /* gic virtual ic*/
                       <0x0 0xe116f000 0 0x2000>, /* gic virtual cpu*/
                       <0x0 0xe1180000 0 0x1000>; /* gic msi */

                 interrupts = <1 8 0xf04>;
         };


My concern here is that if userspace is going to look at 8k starting at 
the beginning of the page, guest offset 0 in your terminology, (say 
0xe1140000) instead of starting at the last 4k of the page, offset 
0xf000 (say 0xe114f000) it is going to get the second 4k wrong by 
reading 0xe1141000 instead of 0xe1150000.

Re: [PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Peter Maydell]

On 25 June 2014 20:34, Joel Schopp <joel.schopp@amd.com> wrote:
> It doesn't work for me.  Maybe I'm doing something wrong, but I can't see
> what.  I am unique in that I'm running a gic-400 (gicv2m) on aarch64
> hardware with 64k pages.  I'm also unique in that my hardware maps each 4K
> gic entry to a 64K page (aliasing each 4k of gic 16 times in a 64K page, ie
> the gic virtual ic is at 0xe1140000 and 0xe1141000 and 0xe1142000, etc).
>
> This is inline with appendix F of the server base system architecture.  This
> is inconvenient when the size is 0x2000 (8K).  As a result all the offsets
> in the device tree entries are to the last 4K in the page so that an 8K read
> will read the last 4k from one page and the first 4k from the next and
> actually get 8k of the gic.
>
>
>         gic: interrupt-controller@e1101000 {
>                 compatible = "arm,gic-400";
>                 #interrupt-cells = <3>;
>                 #address-cells = <0>;
>                 interrupt-controller;
>                 msi-controller;
>                 reg = <0x0 0xe1110000 0 0x1000>, /* gic dist */
>                       <0x0 0xe112f000 0 0x2000>, /* gic cpu */
>                       <0x0 0xe114f000 0 0x2000>, /* gic virtual ic*/
>                       <0x0 0xe116f000 0 0x2000>, /* gic virtual cpu*/
>                       <0x0 0xe1180000 0 0x1000>; /* gic msi */

Right, this is the oddball case we don't yet support for 64K pages
(though as you say it is a permitted configuration per the SBSA).

>                 interrupts = <1 8 0xf04>;
>         };
>
>
> My concern here is that if userspace is going to look at 8k starting at the
> beginning of the page, guest offset 0 in your terminology, (say 0xe1140000)
> instead of starting at the last 4k of the page, offset 0xf000 (say
> 0xe114f000) it is going to get the second 4k wrong by reading 0xe1141000
> instead of 0xe1150000.

Userspace doesn't actually look at anything in the GICC. It just asks
the kernel to put the guest GICC (ie the mapping of the host GICV)
at a particular base address which happens to be a multiple of 64K.
In this case if the host kernel is using 64K pages then the KVM
kernel code ought to say "sorry, can't do that" when we tell it the
base address. (That is, it's impossible to give the guest a VM
where the GICC it sees is at a 64K boundary on your hardware
and host kernel config, and hopefully we report that in a not totally
opaque fashion.)

If you hack QEMU's memory map for the virt board so instead of
    [VIRT_GIC_CPU] = { 0x8010000, 0x10000 },
we have
    [VIRT_GIC_CPU] = { 0x801f000, 0x2000 },

does it work? If QEMU supported this VGIC_GRP_ADDR_OFFSET
query then all it would do would be to change that offset and size.
It would be good to know if there are other problems beyond that...

(Conveniently, Linux guests won't currently try to look at the second
4K page of their GICC...)

thanks
-- PMM

[PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Peter Maydell]

On 25 June 2014 20:34, Joel Schopp <joel.schopp@amd.com> wrote:
> It doesn't work for me.  Maybe I'm doing something wrong, but I can't see
> what.  I am unique in that I'm running a gic-400 (gicv2m) on aarch64
> hardware with 64k pages.  I'm also unique in that my hardware maps each 4K
> gic entry to a 64K page (aliasing each 4k of gic 16 times in a 64K page, ie
> the gic virtual ic is at 0xe1140000 and 0xe1141000 and 0xe1142000, etc).
>
> This is inline with appendix F of the server base system architecture.  This
> is inconvenient when the size is 0x2000 (8K).  As a result all the offsets
> in the device tree entries are to the last 4K in the page so that an 8K read
> will read the last 4k from one page and the first 4k from the next and
> actually get 8k of the gic.
>
>
>         gic: interrupt-controller at e1101000 {
>                 compatible = "arm,gic-400";
>                 #interrupt-cells = <3>;
>                 #address-cells = <0>;
>                 interrupt-controller;
>                 msi-controller;
>                 reg = <0x0 0xe1110000 0 0x1000>, /* gic dist */
>                       <0x0 0xe112f000 0 0x2000>, /* gic cpu */
>                       <0x0 0xe114f000 0 0x2000>, /* gic virtual ic*/
>                       <0x0 0xe116f000 0 0x2000>, /* gic virtual cpu*/
>                       <0x0 0xe1180000 0 0x1000>; /* gic msi */

Right, this is the oddball case we don't yet support for 64K pages
(though as you say it is a permitted configuration per the SBSA).

>                 interrupts = <1 8 0xf04>;
>         };
>
>
> My concern here is that if userspace is going to look at 8k starting at the
> beginning of the page, guest offset 0 in your terminology, (say 0xe1140000)
> instead of starting at the last 4k of the page, offset 0xf000 (say
> 0xe114f000) it is going to get the second 4k wrong by reading 0xe1141000
> instead of 0xe1150000.

Userspace doesn't actually look at anything in the GICC. It just asks
the kernel to put the guest GICC (ie the mapping of the host GICV)
at a particular base address which happens to be a multiple of 64K.
In this case if the host kernel is using 64K pages then the KVM
kernel code ought to say "sorry, can't do that" when we tell it the
base address. (That is, it's impossible to give the guest a VM
where the GICC it sees is at a 64K boundary on your hardware
and host kernel config, and hopefully we report that in a not totally
opaque fashion.)

If you hack QEMU's memory map for the virt board so instead of
    [VIRT_GIC_CPU] = { 0x8010000, 0x10000 },
we have
    [VIRT_GIC_CPU] = { 0x801f000, 0x2000 },

does it work? If QEMU supported this VGIC_GRP_ADDR_OFFSET
query then all it would do would be to change that offset and size.
It would be good to know if there are other problems beyond that...

(Conveniently, Linux guests won't currently try to look at the second
4K page of their GICC...)

thanks
-- PMM

Re: [PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Joel Schopp]

On 06/25/2014 03:45 PM, Peter Maydell wrote:
> On 25 June 2014 20:34, Joel Schopp <joel.schopp@amd.com> wrote:
>> It doesn't work for me.  Maybe I'm doing something wrong, but I can't see
>> what.  I am unique in that I'm running a gic-400 (gicv2m) on aarch64
>> hardware with 64k pages.  I'm also unique in that my hardware maps each 4K
>> gic entry to a 64K page (aliasing each 4k of gic 16 times in a 64K page, ie
>> the gic virtual ic is at 0xe1140000 and 0xe1141000 and 0xe1142000, etc).
>>
>> This is inline with appendix F of the server base system architecture.  This
>> is inconvenient when the size is 0x2000 (8K).  As a result all the offsets
>> in the device tree entries are to the last 4K in the page so that an 8K read
>> will read the last 4k from one page and the first 4k from the next and
>> actually get 8k of the gic.
>>
>>
>>          gic: interrupt-controller@e1101000 {
>>                  compatible = "arm,gic-400";
>>                  #interrupt-cells = <3>;
>>                  #address-cells = <0>;
>>                  interrupt-controller;
>>                  msi-controller;
>>                  reg = <0x0 0xe1110000 0 0x1000>, /* gic dist */
>>                        <0x0 0xe112f000 0 0x2000>, /* gic cpu */
>>                        <0x0 0xe114f000 0 0x2000>, /* gic virtual ic*/
>>                        <0x0 0xe116f000 0 0x2000>, /* gic virtual cpu*/
>>                        <0x0 0xe1180000 0 0x1000>; /* gic msi */
> Right, this is the oddball case we don't yet support for 64K pages
> (though as you say it is a permitted configuration per the SBSA).
At least I know I'm not going crazy.
>
>>                  interrupts = <1 8 0xf04>;
>>          };
>>
>>
>> My concern here is that if userspace is going to look at 8k starting at the
>> beginning of the page, guest offset 0 in your terminology, (say 0xe1140000)
>> instead of starting at the last 4k of the page, offset 0xf000 (say
>> 0xe114f000) it is going to get the second 4k wrong by reading 0xe1141000
>> instead of 0xe1150000.
> Userspace doesn't actually look at anything in the GICC. It just asks
> the kernel to put the guest GICC (ie the mapping of the host GICV)
> at a particular base address which happens to be a multiple of 64K.
> In this case if the host kernel is using 64K pages then the KVM
> kernel code ought to say "sorry, can't do that" when we tell it the
> base address. (That is, it's impossible to give the guest a VM
> where the GICC it sees is at a 64K boundary on your hardware
> and host kernel config, and hopefully we report that in a not totally
> opaque fashion.)
The errors I'm seeing look like:
from qemu:
error: kvm run failed Bad address
Aborted (core dumped)

from kvm:
[ 7931.722965] kvm [1208]: Unsupported fault status: EC=0x20 DFCS=0x14

from kvmtool:
from lkvm (kvmtool):
   Warning: /extra/rootfs/boot/Image is not a bzImage. Trying to load it 
as a flat binary...
   Info: Loaded kernel to 0x80080000 (10212384 bytes)
   Info: Placing fdt at 0x8fe00000 - 0x8fffffff
   Info: virtio-mmio.devices=0x200@0x10000:36

KVM_RUN failed: Bad address


>
> If you hack QEMU's memory map for the virt board so instead of
>      [VIRT_GIC_CPU] = { 0x8010000, 0x10000 },
> we have
>      [VIRT_GIC_CPU] = { 0x801f000, 0x2000 },
No change in result, not to say that this wouldn't work if some other 
unknown problem were fixed.
>
> does it work? If QEMU supported this VGIC_GRP_ADDR_OFFSET
> query then all it would do would be to change that offset and size.
> It would be good to know if there are other problems beyond that...
>
> (Conveniently, Linux guests won't currently try to look at the second
> 4K page of their GICC...)
That's handy.

[PATCH v2 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment

[Joel Schopp]

On 06/25/2014 03:45 PM, Peter Maydell wrote:
> On 25 June 2014 20:34, Joel Schopp <joel.schopp@amd.com> wrote:
>> It doesn't work for me.  Maybe I'm doing something wrong, but I can't see
>> what.  I am unique in that I'm running a gic-400 (gicv2m) on aarch64
>> hardware with 64k pages.  I'm also unique in that my hardware maps each 4K
>> gic entry to a 64K page (aliasing each 4k of gic 16 times in a 64K page, ie
>> the gic virtual ic is at 0xe1140000 and 0xe1141000 and 0xe1142000, etc).
>>
>> This is inline with appendix F of the server base system architecture.  This
>> is inconvenient when the size is 0x2000 (8K).  As a result all the offsets
>> in the device tree entries are to the last 4K in the page so that an 8K read
>> will read the last 4k from one page and the first 4k from the next and
>> actually get 8k of the gic.
>>
>>
>>          gic: interrupt-controller at e1101000 {
>>                  compatible = "arm,gic-400";
>>                  #interrupt-cells = <3>;
>>                  #address-cells = <0>;
>>                  interrupt-controller;
>>                  msi-controller;
>>                  reg = <0x0 0xe1110000 0 0x1000>, /* gic dist */
>>                        <0x0 0xe112f000 0 0x2000>, /* gic cpu */
>>                        <0x0 0xe114f000 0 0x2000>, /* gic virtual ic*/
>>                        <0x0 0xe116f000 0 0x2000>, /* gic virtual cpu*/
>>                        <0x0 0xe1180000 0 0x1000>; /* gic msi */
> Right, this is the oddball case we don't yet support for 64K pages
> (though as you say it is a permitted configuration per the SBSA).
At least I know I'm not going crazy.
>
>>                  interrupts = <1 8 0xf04>;
>>          };
>>
>>
>> My concern here is that if userspace is going to look at 8k starting at the
>> beginning of the page, guest offset 0 in your terminology, (say 0xe1140000)
>> instead of starting at the last 4k of the page, offset 0xf000 (say
>> 0xe114f000) it is going to get the second 4k wrong by reading 0xe1141000
>> instead of 0xe1150000.
> Userspace doesn't actually look at anything in the GICC. It just asks
> the kernel to put the guest GICC (ie the mapping of the host GICV)
> at a particular base address which happens to be a multiple of 64K.
> In this case if the host kernel is using 64K pages then the KVM
> kernel code ought to say "sorry, can't do that" when we tell it the
> base address. (That is, it's impossible to give the guest a VM
> where the GICC it sees is at a 64K boundary on your hardware
> and host kernel config, and hopefully we report that in a not totally
> opaque fashion.)
The errors I'm seeing look like:
from qemu:
error: kvm run failed Bad address
Aborted (core dumped)

from kvm:
[ 7931.722965] kvm [1208]: Unsupported fault status: EC=0x20 DFCS=0x14

from kvmtool:
from lkvm (kvmtool):
   Warning: /extra/rootfs/boot/Image is not a bzImage. Trying to load it 
as a flat binary...
   Info: Loaded kernel to 0x80080000 (10212384 bytes)
   Info: Placing fdt at 0x8fe00000 - 0x8fffffff
   Info: virtio-mmio.devices=0x200 at 0x10000:36

KVM_RUN failed: Bad address


>
> If you hack QEMU's memory map for the virt board so instead of
>      [VIRT_GIC_CPU] = { 0x8010000, 0x10000 },
> we have
>      [VIRT_GIC_CPU] = { 0x801f000, 0x2000 },
No change in result, not to say that this wouldn't work if some other 
unknown problem were fixed.
>
> does it work? If QEMU supported this VGIC_GRP_ADDR_OFFSET
> query then all it would do would be to change that offset and size.
> It would be good to know if there are other problems beyond that...
>
> (Conveniently, Linux guests won't currently try to look at the second
> 4K page of their GICC...)
That's handy.