* [PATCH v3 1/9] KVM: ARM: vgic: plug irq injection race
2014-07-08 11:08 [PATCH v3 0/9] arm/arm64: KVM: dynamic VGIC sizing Marc Zyngier
@ 2014-07-08 11:09 ` Marc Zyngier
2014-07-08 11:09 ` [PATCH v3 2/9] arm/arm64: KVM: vgic: switch to dynamic allocation Marc Zyngier
` (7 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2014-07-08 11:09 UTC (permalink / raw)
To: linux-arm-kernel
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 ede8f64..eebaf54 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1466,7 +1466,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;
--
2.0.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 2/9] arm/arm64: KVM: vgic: switch to dynamic allocation
2014-07-08 11:08 [PATCH v3 0/9] arm/arm64: KVM: dynamic VGIC sizing Marc Zyngier
2014-07-08 11:09 ` [PATCH v3 1/9] KVM: ARM: vgic: plug irq injection race Marc Zyngier
@ 2014-07-08 11:09 ` Marc Zyngier
2014-08-05 13:39 ` Christoffer Dall
2014-07-08 11:09 ` [PATCH v3 3/9] arm/arm64: KVM: vgic: Parametrize VGIC_NR_SHARED_IRQS Marc Zyngier
` (6 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2014-07-08 11:09 UTC (permalink / raw)
To: linux-arm-kernel
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 | 3 +
include/kvm/arm_vgic.h | 44 ++++++----
virt/kvm/arm/vgic.c | 232 ++++++++++++++++++++++++++++++++++++++++++-------
3 files changed, 231 insertions(+), 48 deletions(-)
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 3c82b37..782632e 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);
}
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 35b0c12..2246f4c 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,15 +160,15 @@ 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;
+ unsigned long *irq_pending_on_cpu;
#endif
};
@@ -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);
@@ -225,7 +233,9 @@ 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);
+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 eebaf54..754d1cd 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);
@@ -118,23 +119,45 @@ static const struct vgic_params *vgic;
#define REG_OFFSET_SWIZZLE 0
#endif
+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);
+ b->private = NULL;
+}
+
static u32 *vgic_bitmap_get_reg(struct vgic_bitmap *x,
int cpuid, u32 offset)
{
offset >>= 2;
if (!offset)
- return x->percpu[cpuid].reg + (offset ^ REG_OFFSET_SWIZZLE);
+ return (u32 *)(x->private + cpuid) + REG_OFFSET_SWIZZLE;
else
- return x->shared.reg + ((offset - 1) ^ REG_OFFSET_SWIZZLE);
+ return (u32 *)(x->shared) + ((offset - 1) ^ REG_OFFSET_SWIZZLE);
}
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,
@@ -143,9 +166,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;
}
@@ -157,24 +180,49 @@ 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);
+ b->private = NULL;
}
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
@@ -653,7 +701,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);
@@ -685,7 +733,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);
@@ -709,14 +757,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;
}
}
@@ -900,6 +949,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;
@@ -933,7 +987,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);
}
@@ -980,14 +1034,14 @@ static void vgic_update_state(struct kvm *kvm)
int c;
if (!dist->enabled) {
- set_bit(0, &dist->irq_pending_on_cpu);
+ set_bit(0, dist->irq_pending_on_cpu);
return;
}
kvm_for_each_vcpu(c, vcpu, kvm) {
if (compute_pending_for_cpu(vcpu)) {
pr_debug("CPU%d has pending interrupts\n", c);
- set_bit(c, &dist->irq_pending_on_cpu);
+ set_bit(c, dist->irq_pending_on_cpu);
}
}
}
@@ -1144,14 +1198,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
@@ -1239,7 +1293,7 @@ epilog:
* us. Claim we don't have anything pending. We'll
* adjust that if needed while exiting.
*/
- clear_bit(vcpu_id, &dist->irq_pending_on_cpu);
+ clear_bit(vcpu_id, dist->irq_pending_on_cpu);
}
}
@@ -1322,7 +1376,7 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
/* Check if we still have something up our sleeve... */
pending = find_first_zero_bit(elrsr_ptr, vgic->nr_lr);
if (level_pending || pending < vgic->nr_lr)
- set_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
+ set_bit(vcpu->vcpu_id, dist->irq_pending_on_cpu);
}
void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
@@ -1356,7 +1410,7 @@ int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu)
if (!irqchip_in_kernel(vcpu->kvm))
return 0;
- return test_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
+ return test_bit(vcpu->vcpu_id, dist->irq_pending_on_cpu);
}
static void vgic_kick_vcpus(struct kvm *kvm)
@@ -1440,7 +1494,7 @@ static bool vgic_update_irq_state(struct kvm *kvm, int cpuid,
if (level) {
vgic_cpu_irq_set(vcpu, irq_num);
- set_bit(cpuid, &dist->irq_pending_on_cpu);
+ set_bit(cpuid, dist->irq_pending_on_cpu);
}
out:
@@ -1484,6 +1538,32 @@ static irqreturn_t vgic_maintenance_handler(int irq, void *data)
return IRQ_HANDLED;
}
+void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
+{
+ struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+
+ kfree(vgic_cpu->pending_shared);
+ kfree(vgic_cpu->vgic_irq_lr_map);
+ vgic_cpu->pending_shared = NULL;
+ vgic_cpu->vgic_irq_lr_map = NULL;
+}
+
+static int vgic_vcpu_init_maps(struct kvm_vcpu *vcpu, int nr_irqs)
+{
+ struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+
+ 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) {
+ kvm_vgic_vcpu_destroy(vcpu);
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
/**
* kvm_vgic_vcpu_init - Initialize per-vcpu VGIC state
* @vcpu: pointer to the vcpu struct
@@ -1600,6 +1680,92 @@ out_free_irq:
return ret;
}
+void kvm_vgic_destroy(struct kvm *kvm)
+{
+ struct vgic_dist *dist = &kvm->arch.vgic;
+ struct kvm_vcpu *vcpu;
+ int i;
+
+ kvm_for_each_vcpu(i, vcpu, kvm)
+ kvm_vgic_vcpu_destroy(vcpu);
+
+ 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);
+ kfree(dist->irq_pending_on_cpu);
+ dist->irq_sgi_sources = NULL;
+ dist->irq_spi_cpu = NULL;
+ dist->irq_spi_target = NULL;
+ dist->irq_pending_on_cpu = NULL;
+}
+
+/*
+ * Allocate and initialize the various data structures. Must be called
+ * with kvm->lock held!
+ */
+static int vgic_init_maps(struct kvm *kvm)
+{
+ struct vgic_dist *dist = &kvm->arch.vgic;
+ struct kvm_vcpu *vcpu;
+ int nr_cpus, nr_irqs;
+ int ret, i;
+
+ nr_cpus = dist->nr_cpus = VGIC_MAX_CPUS;
+ nr_irqs = dist->nr_irqs = VGIC_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)
+ goto out;
+
+ 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);
+ dist->irq_pending_on_cpu = kzalloc(BITS_TO_LONGS(nr_cpus) * sizeof(unsigned long),
+ GFP_KERNEL);
+ if (!dist->irq_sgi_sources ||
+ !dist->irq_spi_cpu ||
+ !dist->irq_spi_target ||
+ !dist->irq_pending_on_cpu) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ for (i = 0; i < nr_cpus; i++)
+ ret |= vgic_init_bitmap(&dist->irq_spi_target[i],
+ nr_cpus, nr_irqs);
+
+ if (ret)
+ goto out;
+
+ kvm_for_each_vcpu(i, vcpu, kvm) {
+ ret = vgic_vcpu_init_maps(vcpu, nr_irqs);
+ if (ret) {
+ kvm_err("Failed to allocate vcpu memory\n");
+ break;
+ }
+ }
+
+out:
+ if (ret)
+ kvm_vgic_destroy(kvm);
+
+ return ret;
+}
+
/**
* kvm_vgic_init - Initialize global VGIC state before running any VCPUs
* @kvm: pointer to the kvm struct
@@ -1680,6 +1846,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);
+ 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);
--
2.0.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 2/9] arm/arm64: KVM: vgic: switch to dynamic allocation
2014-07-08 11:09 ` [PATCH v3 2/9] arm/arm64: KVM: vgic: switch to dynamic allocation Marc Zyngier
@ 2014-08-05 13:39 ` Christoffer Dall
0 siblings, 0 replies; 18+ messages in thread
From: Christoffer Dall @ 2014-08-05 13:39 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 08, 2014 at 12:09:01PM +0100, Marc Zyngier wrote:
> 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 | 3 +
> include/kvm/arm_vgic.h | 44 ++++++----
> virt/kvm/arm/vgic.c | 232 ++++++++++++++++++++++++++++++++++++++++++-------
> 3 files changed, 231 insertions(+), 48 deletions(-)
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 3c82b37..782632e 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);
> }
>
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 35b0c12..2246f4c 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)
nit: consider indenting the second line two spaces.
> + * - As many UL as necessary for shared interrupts.
> + */
strictly speaking we're not documenting where in the allocated array one
is supposed to find the state for VCPU0 or where in the array one would
find the bit for IRQ 32 (I assume it's vgic_bitmap->private[0] and
vgic_bitmap->shared[0] & 1, respectively). Maybe I'm being ridiculous
and this is obvious.
> + 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;
why are we adding this field? I can understand if you wanted to have
the data structure be self-contained, but then the destroy code should
rely on it. As far as I can see, the patches only ever set this value
and never read it...
> + 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,15 +160,15 @@ 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;
the data layout definition is not complete, I think you're grouping this
per CPU, not the other way around. Can you be slightly more specific in
the comment?
(think I asked about this in my original review way back too).
>
> /* 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;
As I commented on before, it gets really hard to know what these are for
when the array sizes are gone, can we clarify that each byte in
irq_spi_cpu gives you the (mask or number?) of the target CPU, indexed
by irq_num-32.
could we add something like the above before the irq_spi_target:
/* Reverse lookup of irq_spi_cpu for faster compute pending */ ?
I know we have accessors, but if we find bugs in any of that it's really
helpful to know what the intention behind this was.
>
> /* Bitmap indicating which CPU has something pending */
> - unsigned long irq_pending_on_cpu;
> + unsigned long *irq_pending_on_cpu;
> #endif
> };
>
> @@ -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);
> @@ -225,7 +233,9 @@ 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);
> +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 eebaf54..754d1cd 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);
> @@ -118,23 +119,45 @@ static const struct vgic_params *vgic;
> #define REG_OFFSET_SWIZZLE 0
> #endif
>
> +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);
> + b->private = NULL;
set b->shared = NULL too?
> +}
> +
> static u32 *vgic_bitmap_get_reg(struct vgic_bitmap *x,
> int cpuid, u32 offset)
> {
> offset >>= 2;
> if (!offset)
you should update the comment about REG_OFFSET_SWIZZLE now, because it
talks about unions that are no longer there.
> - return x->percpu[cpuid].reg + (offset ^ REG_OFFSET_SWIZZLE);
> + return (u32 *)(x->private + cpuid) + REG_OFFSET_SWIZZLE;
> else
> - return x->shared.reg + ((offset - 1) ^ REG_OFFSET_SWIZZLE);
> + return (u32 *)(x->shared) + ((offset - 1) ^ REG_OFFSET_SWIZZLE);
> }
>
> 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,
> @@ -143,9 +166,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;
> }
>
> @@ -157,24 +180,49 @@ 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);
> + b->private = NULL;
b->shared = NULL ?
> }
>
> 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) {
if (offset < VGIC_NR_PRIVATE_IRQS) ?
> + 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
> @@ -653,7 +701,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);
>
> @@ -685,7 +733,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);
> @@ -709,14 +757,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;
> }
> }
>
> @@ -900,6 +949,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;
> @@ -933,7 +987,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);
> }
>
> @@ -980,14 +1034,14 @@ static void vgic_update_state(struct kvm *kvm)
> int c;
>
> if (!dist->enabled) {
> - set_bit(0, &dist->irq_pending_on_cpu);
> + set_bit(0, dist->irq_pending_on_cpu);
> return;
> }
>
> kvm_for_each_vcpu(c, vcpu, kvm) {
> if (compute_pending_for_cpu(vcpu)) {
> pr_debug("CPU%d has pending interrupts\n", c);
> - set_bit(c, &dist->irq_pending_on_cpu);
> + set_bit(c, dist->irq_pending_on_cpu);
> }
> }
> }
> @@ -1144,14 +1198,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
> @@ -1239,7 +1293,7 @@ epilog:
> * us. Claim we don't have anything pending. We'll
> * adjust that if needed while exiting.
> */
> - clear_bit(vcpu_id, &dist->irq_pending_on_cpu);
> + clear_bit(vcpu_id, dist->irq_pending_on_cpu);
> }
> }
>
> @@ -1322,7 +1376,7 @@ static void __kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
> /* Check if we still have something up our sleeve... */
> pending = find_first_zero_bit(elrsr_ptr, vgic->nr_lr);
> if (level_pending || pending < vgic->nr_lr)
> - set_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
> + set_bit(vcpu->vcpu_id, dist->irq_pending_on_cpu);
> }
>
> void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
> @@ -1356,7 +1410,7 @@ int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu)
> if (!irqchip_in_kernel(vcpu->kvm))
> return 0;
>
> - return test_bit(vcpu->vcpu_id, &dist->irq_pending_on_cpu);
> + return test_bit(vcpu->vcpu_id, dist->irq_pending_on_cpu);
> }
>
> static void vgic_kick_vcpus(struct kvm *kvm)
> @@ -1440,7 +1494,7 @@ static bool vgic_update_irq_state(struct kvm *kvm, int cpuid,
>
> if (level) {
> vgic_cpu_irq_set(vcpu, irq_num);
> - set_bit(cpuid, &dist->irq_pending_on_cpu);
> + set_bit(cpuid, dist->irq_pending_on_cpu);
> }
>
> out:
> @@ -1484,6 +1538,32 @@ static irqreturn_t vgic_maintenance_handler(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
> +{
> + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> +
> + kfree(vgic_cpu->pending_shared);
> + kfree(vgic_cpu->vgic_irq_lr_map);
> + vgic_cpu->pending_shared = NULL;
> + vgic_cpu->vgic_irq_lr_map = NULL;
> +}
> +
> +static int vgic_vcpu_init_maps(struct kvm_vcpu *vcpu, int nr_irqs)
> +{
> + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> +
> + int sz = (nr_irqs - VGIC_NR_PRIVATE_IRQS) / 8;
are we guaranteed that the numerator is always a multiple of 8? if not,
won't you end up allocating one less byte than needed?
> + 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) {
> + kvm_vgic_vcpu_destroy(vcpu);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> /**
> * kvm_vgic_vcpu_init - Initialize per-vcpu VGIC state
> * @vcpu: pointer to the vcpu struct
> @@ -1600,6 +1680,92 @@ out_free_irq:
> return ret;
> }
>
> +void kvm_vgic_destroy(struct kvm *kvm)
> +{
> + struct vgic_dist *dist = &kvm->arch.vgic;
> + struct kvm_vcpu *vcpu;
> + int i;
> +
> + kvm_for_each_vcpu(i, vcpu, kvm)
> + kvm_vgic_vcpu_destroy(vcpu);
> +
> + 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]);
nit: braces for the if statement with multi-line content
> + kfree(dist->irq_sgi_sources);
> + kfree(dist->irq_spi_cpu);
> + kfree(dist->irq_spi_target);
> + kfree(dist->irq_pending_on_cpu);
> + dist->irq_sgi_sources = NULL;
> + dist->irq_spi_cpu = NULL;
> + dist->irq_spi_target = NULL;
> + dist->irq_pending_on_cpu = NULL;
> +}
> +
> +/*
> + * Allocate and initialize the various data structures. Must be called
> + * with kvm->lock held!
> + */
> +static int vgic_init_maps(struct kvm *kvm)
> +{
> + struct vgic_dist *dist = &kvm->arch.vgic;
> + struct kvm_vcpu *vcpu;
> + int nr_cpus, nr_irqs;
> + int ret, i;
> +
> + nr_cpus = dist->nr_cpus = VGIC_MAX_CPUS;
> + nr_irqs = dist->nr_irqs = VGIC_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)
> + goto out;
> +
> + dist->irq_sgi_sources = kzalloc(nr_cpus * VGIC_NR_SGIS, GFP_KERNEL);
nit: is the tab before GFP_KERNEL on purpose?
> + 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);
> + dist->irq_pending_on_cpu = kzalloc(BITS_TO_LONGS(nr_cpus) * sizeof(unsigned long),
you could drop the unsigned in the sizeof and get a slightly shorter
line.
> + GFP_KERNEL);
> + if (!dist->irq_sgi_sources ||
> + !dist->irq_spi_cpu ||
> + !dist->irq_spi_target ||
> + !dist->irq_pending_on_cpu) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + for (i = 0; i < nr_cpus; i++)
> + ret |= vgic_init_bitmap(&dist->irq_spi_target[i],
> + nr_cpus, nr_irqs);
> +
> + if (ret)
> + goto out;
> +
> + kvm_for_each_vcpu(i, vcpu, kvm) {
> + ret = vgic_vcpu_init_maps(vcpu, nr_irqs);
> + if (ret) {
> + kvm_err("Failed to allocate vcpu memory\n");
can you prepend the message with "VGIC: " or change the wording to
something about failing to allocate vgic_vcpu memory?
> + break;
> + }
> + }
> +
> +out:
> + if (ret)
> + kvm_vgic_destroy(kvm);
> +
> + return ret;
> +}
> +
> /**
> * kvm_vgic_init - Initialize global VGIC state before running any VCPUs
> * @kvm: pointer to the kvm struct
> @@ -1680,6 +1846,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);
> + 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);
> --
> 2.0.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 3/9] arm/arm64: KVM: vgic: Parametrize VGIC_NR_SHARED_IRQS
2014-07-08 11:08 [PATCH v3 0/9] arm/arm64: KVM: dynamic VGIC sizing Marc Zyngier
2014-07-08 11:09 ` [PATCH v3 1/9] KVM: ARM: vgic: plug irq injection race Marc Zyngier
2014-07-08 11:09 ` [PATCH v3 2/9] arm/arm64: KVM: vgic: switch to dynamic allocation Marc Zyngier
@ 2014-07-08 11:09 ` Marc Zyngier
2014-08-05 13:42 ` Christoffer Dall
2014-07-08 11:09 ` [PATCH v3 4/9] arm/arm64: KVM: vgic: kill VGIC_MAX_CPUS Marc Zyngier
` (5 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2014-07-08 11:09 UTC (permalink / raw)
To: linux-arm-kernel
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 2246f4c..b8a6337 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 754d1cd..6f7cf85 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -995,11 +995,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;
@@ -1012,15 +1018,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));
}
/*
@@ -1277,7 +1283,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;
}
--
2.0.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 3/9] arm/arm64: KVM: vgic: Parametrize VGIC_NR_SHARED_IRQS
2014-07-08 11:09 ` [PATCH v3 3/9] arm/arm64: KVM: vgic: Parametrize VGIC_NR_SHARED_IRQS Marc Zyngier
@ 2014-08-05 13:42 ` Christoffer Dall
0 siblings, 0 replies; 18+ messages in thread
From: Christoffer Dall @ 2014-08-05 13:42 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 08, 2014 at 12:09:02PM +0100, Marc Zyngier wrote:
> 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 2246f4c..b8a6337 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 754d1cd..6f7cf85 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -995,11 +995,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);
nit: prefer nr_shared
> int vcpu_id;
>
> vcpu_id = vcpu->vcpu_id;
> @@ -1012,15 +1018,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));
> }
>
> /*
> @@ -1277,7 +1283,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;
> }
> --
> 2.0.0
>
There are a number of places in patch 2 where you do "nr_irqs -
VGIC_NR_PRIVATE_IRQS" which you could change to use this as well now.
Otherwise:
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 4/9] arm/arm64: KVM: vgic: kill VGIC_MAX_CPUS
2014-07-08 11:08 [PATCH v3 0/9] arm/arm64: KVM: dynamic VGIC sizing Marc Zyngier
` (2 preceding siblings ...)
2014-07-08 11:09 ` [PATCH v3 3/9] arm/arm64: KVM: vgic: Parametrize VGIC_NR_SHARED_IRQS Marc Zyngier
@ 2014-07-08 11:09 ` Marc Zyngier
2014-08-05 13:49 ` Christoffer Dall
2014-07-08 11:09 ` [PATCH v3 5/9] arm/arm64: KVM: vgic: handle out-of-range MMIO accesses Marc Zyngier
` (4 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2014-07-08 11:09 UTC (permalink / raw)
To: linux-arm-kernel
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>
---
include/kvm/arm_vgic.h | 3 +--
virt/kvm/arm/vgic.c | 6 +++---
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index b8a6337..99ad8af 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 6f7cf85..3cb667c 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1206,7 +1206,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);
}
@@ -1583,7 +1583,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
int i;
- if (vcpu->vcpu_id >= VGIC_MAX_CPUS)
+ if (vcpu->vcpu_id >= dist->nr_cpus)
return -EBUSY;
for (i = 0; i < VGIC_NR_IRQS; i++) {
@@ -1724,7 +1724,7 @@ static int vgic_init_maps(struct kvm *kvm)
int nr_cpus, nr_irqs;
int ret, i;
- nr_cpus = dist->nr_cpus = VGIC_MAX_CPUS;
+ nr_cpus = dist->nr_cpus = KVM_MAX_VCPUS;
nr_irqs = dist->nr_irqs = VGIC_NR_IRQS;
ret = vgic_init_bitmap(&dist->irq_enabled, nr_cpus, nr_irqs);
--
2.0.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 4/9] arm/arm64: KVM: vgic: kill VGIC_MAX_CPUS
2014-07-08 11:09 ` [PATCH v3 4/9] arm/arm64: KVM: vgic: kill VGIC_MAX_CPUS Marc Zyngier
@ 2014-08-05 13:49 ` Christoffer Dall
0 siblings, 0 replies; 18+ messages in thread
From: Christoffer Dall @ 2014-08-05 13:49 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 08, 2014 at 12:09:03PM +0100, Marc Zyngier wrote:
> 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>
> ---
> include/kvm/arm_vgic.h | 3 +--
> virt/kvm/arm/vgic.c | 6 +++---
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index b8a6337..99ad8af 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 6f7cf85..3cb667c 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -1206,7 +1206,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);
> }
> @@ -1583,7 +1583,7 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
> struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
> int i;
>
> - if (vcpu->vcpu_id >= VGIC_MAX_CPUS)
> + if (vcpu->vcpu_id >= dist->nr_cpus)
> return -EBUSY;
>
> for (i = 0; i < VGIC_NR_IRQS; i++) {
> @@ -1724,7 +1724,7 @@ static int vgic_init_maps(struct kvm *kvm)
> int nr_cpus, nr_irqs;
> int ret, i;
>
> - nr_cpus = dist->nr_cpus = VGIC_MAX_CPUS;
> + nr_cpus = dist->nr_cpus = KVM_MAX_VCPUS;
> nr_irqs = dist->nr_irqs = VGIC_NR_IRQS;
>
> ret = vgic_init_bitmap(&dist->irq_enabled, nr_cpus, nr_irqs);
> --
> 2.0.0
>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 5/9] arm/arm64: KVM: vgic: handle out-of-range MMIO accesses
2014-07-08 11:08 [PATCH v3 0/9] arm/arm64: KVM: dynamic VGIC sizing Marc Zyngier
` (3 preceding siblings ...)
2014-07-08 11:09 ` [PATCH v3 4/9] arm/arm64: KVM: vgic: kill VGIC_MAX_CPUS Marc Zyngier
@ 2014-07-08 11:09 ` Marc Zyngier
2014-08-05 13:56 ` Christoffer Dall
2014-07-08 11:09 ` [PATCH v3 6/9] arm/arm64: KVM: vgic: kill VGIC_NR_IRQS Marc Zyngier
` (3 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2014-07-08 11:09 UTC (permalink / raw)
To: linux-arm-kernel
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 99ad8af..98ab604 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 3cb667c..b2ef7ff 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -804,6 +804,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);
};
@@ -812,56 +813,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,
},
{
@@ -899,6 +911,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
@@ -938,7 +966,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);
--
2.0.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 5/9] arm/arm64: KVM: vgic: handle out-of-range MMIO accesses
2014-07-08 11:09 ` [PATCH v3 5/9] arm/arm64: KVM: vgic: handle out-of-range MMIO accesses Marc Zyngier
@ 2014-08-05 13:56 ` Christoffer Dall
0 siblings, 0 replies; 18+ messages in thread
From: Christoffer Dall @ 2014-08-05 13:56 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 08, 2014 at 12:09:04PM +0100, Marc Zyngier wrote:
> 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 99ad8af..98ab604 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 3cb667c..b2ef7ff 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -804,6 +804,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);
> };
> @@ -812,56 +813,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,
> },
> {
> @@ -899,6 +911,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
> @@ -938,7 +966,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);
> --
> 2.0.0
>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 6/9] arm/arm64: KVM: vgic: kill VGIC_NR_IRQS
2014-07-08 11:08 [PATCH v3 0/9] arm/arm64: KVM: dynamic VGIC sizing Marc Zyngier
` (4 preceding siblings ...)
2014-07-08 11:09 ` [PATCH v3 5/9] arm/arm64: KVM: vgic: handle out-of-range MMIO accesses Marc Zyngier
@ 2014-07-08 11:09 ` Marc Zyngier
2014-08-05 13:58 ` Christoffer Dall
2014-07-08 11:09 ` [PATCH v3 7/9] arm/arm64: KVM: vgic: delay vgic allocation until init time Marc Zyngier
` (2 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2014-07-08 11:09 UTC (permalink / raw)
To: linux-arm-kernel
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>
---
include/kvm/arm_vgic.h | 6 +++---
virt/kvm/arm/vgic.c | 17 +++++++++++------
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 98ab604..9feb7fe 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 b2ef7ff..47a14a1 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -395,7 +395,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, ®, word_offset,
ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
break;
@@ -1186,13 +1186,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);
@@ -1409,7 +1410,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;
}
@@ -1620,7 +1621,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);
@@ -1759,7 +1760,11 @@ static int vgic_init_maps(struct kvm *kvm)
int ret, i;
nr_cpus = dist->nr_cpus = KVM_MAX_VCPUS;
- nr_irqs = dist->nr_irqs = VGIC_NR_IRQS;
+
+ if (!dist->nr_irqs)
+ dist->nr_irqs = VGIC_NR_IRQS_LEGACY;
+
+ nr_irqs = dist->nr_irqs;
ret = vgic_init_bitmap(&dist->irq_enabled, nr_cpus, nr_irqs);
ret |= vgic_init_bitmap(&dist->irq_state, nr_cpus, nr_irqs);
@@ -1841,7 +1846,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;
--
2.0.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 6/9] arm/arm64: KVM: vgic: kill VGIC_NR_IRQS
2014-07-08 11:09 ` [PATCH v3 6/9] arm/arm64: KVM: vgic: kill VGIC_NR_IRQS Marc Zyngier
@ 2014-08-05 13:58 ` Christoffer Dall
0 siblings, 0 replies; 18+ messages in thread
From: Christoffer Dall @ 2014-08-05 13:58 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 08, 2014 at 12:09:05PM +0100, Marc Zyngier wrote:
> 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>
> ---
> include/kvm/arm_vgic.h | 6 +++---
> virt/kvm/arm/vgic.c | 17 +++++++++++------
> 2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 98ab604..9feb7fe 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 b2ef7ff..47a14a1 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -395,7 +395,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, ®, word_offset,
> ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
> break;
> @@ -1186,13 +1186,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);
>
> @@ -1409,7 +1410,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;
> }
>
> @@ -1620,7 +1621,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);
> @@ -1759,7 +1760,11 @@ static int vgic_init_maps(struct kvm *kvm)
> int ret, i;
>
> nr_cpus = dist->nr_cpus = KVM_MAX_VCPUS;
> - nr_irqs = dist->nr_irqs = VGIC_NR_IRQS;
> +
> + if (!dist->nr_irqs)
> + dist->nr_irqs = VGIC_NR_IRQS_LEGACY;
> +
> + nr_irqs = dist->nr_irqs;
>
> ret = vgic_init_bitmap(&dist->irq_enabled, nr_cpus, nr_irqs);
> ret |= vgic_init_bitmap(&dist->irq_state, nr_cpus, nr_irqs);
> @@ -1841,7 +1846,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;
> --
> 2.0.0
>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 7/9] arm/arm64: KVM: vgic: delay vgic allocation until init time
2014-07-08 11:08 [PATCH v3 0/9] arm/arm64: KVM: dynamic VGIC sizing Marc Zyngier
` (5 preceding siblings ...)
2014-07-08 11:09 ` [PATCH v3 6/9] arm/arm64: KVM: vgic: kill VGIC_NR_IRQS Marc Zyngier
@ 2014-07-08 11:09 ` Marc Zyngier
2014-08-05 15:34 ` Christoffer Dall
2014-07-08 11:09 ` [PATCH v3 8/9] arm/arm64: KVM: vgic: make number of irqs a configurable attribute Marc Zyngier
2014-07-08 11:09 ` [PATCH v3 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment Marc Zyngier
8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2014-07-08 11:09 UTC (permalink / raw)
To: linux-arm-kernel
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 | 7 -------
include/kvm/arm_vgic.h | 1 -
virt/kvm/arm/vgic.c | 42 +++++++++++++++++++++++++++++-------------
3 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 782632e..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);
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 9feb7fe..311a0f0 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -233,7 +233,6 @@ int kvm_vgic_hyp_init(void);
int kvm_vgic_init(struct kvm *kvm);
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 47a14a1..708aed9 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1612,15 +1612,12 @@ static int vgic_vcpu_init_maps(struct kvm_vcpu *vcpu, int nr_irqs)
* 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 void 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;
- 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,
@@ -1640,8 +1637,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
vgic_cpu->nr_lr = vgic->nr_lr;
vgic_enable(vcpu);
-
- return 0;
}
static void vgic_init_maintenance_interrupt(void *info)
@@ -1759,8 +1754,17 @@ static int vgic_init_maps(struct kvm *kvm)
int nr_cpus, nr_irqs;
int ret, i;
- nr_cpus = dist->nr_cpus = KVM_MAX_VCPUS;
+ if (dist->nr_cpus) /* Already allocated */
+ return 0;
+
+ nr_cpus = dist->nr_cpus = atomic_read(&kvm->online_vcpus);
+ if (!nr_cpus) /* No vcpus? Can't be good... */
+ return -EINVAL;
+ /*
+ * If nobody configured the number of interrupts, use the
+ * legacy one.
+ */
if (!dist->nr_irqs)
dist->nr_irqs = VGIC_NR_IRQS_LEGACY;
@@ -1804,6 +1808,9 @@ static int vgic_init_maps(struct kvm *kvm)
}
}
+ for (i = VGIC_NR_PRIVATE_IRQS; i < dist->nr_irqs; i += 4)
+ vgic_set_target_reg(kvm, 0, i);
+
out:
if (ret)
kvm_vgic_destroy(kvm);
@@ -1822,6 +1829,7 @@ out:
*/
int kvm_vgic_init(struct kvm *kvm)
{
+ struct kvm_vcpu *vcpu;
int ret = 0, i;
if (!irqchip_in_kernel(kvm))
@@ -1839,6 +1847,12 @@ int kvm_vgic_init(struct kvm *kvm)
goto out;
}
+ ret = vgic_init_maps(kvm);
+ if (ret) {
+ kvm_err("Unable to allocate maps\n");
+ goto out;
+ }
+
ret = kvm_phys_addr_ioremap(kvm, kvm->arch.vgic.vgic_cpu_base,
vgic->vcpu_base, KVM_VGIC_V2_CPU_SIZE);
if (ret) {
@@ -1846,11 +1860,13 @@ int kvm_vgic_init(struct kvm *kvm)
goto out;
}
- for (i = VGIC_NR_PRIVATE_IRQS; i < kvm->arch.vgic.nr_irqs; i += 4)
- vgic_set_target_reg(kvm, 0, i);
+ kvm_for_each_vcpu(i, vcpu, kvm)
+ kvm_vgic_vcpu_init(vcpu);
kvm->arch.vgic.ready = true;
out:
+ if (ret)
+ kvm_vgic_destroy(kvm);
mutex_unlock(&kvm->lock);
return ret;
}
@@ -1891,10 +1907,6 @@ 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);
- 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);
@@ -2095,6 +2107,10 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
mutex_lock(&dev->kvm->lock);
+ ret = vgic_init_maps(dev->kvm);
+ if (ret)
+ goto out;
+
if (cpuid >= atomic_read(&dev->kvm->online_vcpus)) {
ret = -EINVAL;
goto out;
--
2.0.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 7/9] arm/arm64: KVM: vgic: delay vgic allocation until init time
2014-07-08 11:09 ` [PATCH v3 7/9] arm/arm64: KVM: vgic: delay vgic allocation until init time Marc Zyngier
@ 2014-08-05 15:34 ` Christoffer Dall
0 siblings, 0 replies; 18+ messages in thread
From: Christoffer Dall @ 2014-08-05 15:34 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 08, 2014 at 12:09:06PM +0100, Marc Zyngier wrote:
> 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
the first starting ?
> 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 | 7 -------
> include/kvm/arm_vgic.h | 1 -
> virt/kvm/arm/vgic.c | 42 +++++++++++++++++++++++++++++-------------
> 3 files changed, 29 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index 782632e..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);
>
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 9feb7fe..311a0f0 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -233,7 +233,6 @@ int kvm_vgic_hyp_init(void);
> int kvm_vgic_init(struct kvm *kvm);
> 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 47a14a1..708aed9 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -1612,15 +1612,12 @@ static int vgic_vcpu_init_maps(struct kvm_vcpu *vcpu, int nr_irqs)
> * 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 void 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;
>
> - 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,
> @@ -1640,8 +1637,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
> vgic_cpu->nr_lr = vgic->nr_lr;
>
> vgic_enable(vcpu);
> -
> - return 0;
> }
>
> static void vgic_init_maintenance_interrupt(void *info)
> @@ -1759,8 +1754,17 @@ static int vgic_init_maps(struct kvm *kvm)
> int nr_cpus, nr_irqs;
> int ret, i;
>
> - nr_cpus = dist->nr_cpus = KVM_MAX_VCPUS;
> + if (dist->nr_cpus) /* Already allocated */
> + return 0;
> +
> + nr_cpus = dist->nr_cpus = atomic_read(&kvm->online_vcpus);
> + if (!nr_cpus) /* No vcpus? Can't be good... */
> + return -EINVAL;
>
> + /*
> + * If nobody configured the number of interrupts, use the
> + * legacy one.
> + */
> if (!dist->nr_irqs)
> dist->nr_irqs = VGIC_NR_IRQS_LEGACY;
>
> @@ -1804,6 +1808,9 @@ static int vgic_init_maps(struct kvm *kvm)
> }
> }
>
> + for (i = VGIC_NR_PRIVATE_IRQS; i < dist->nr_irqs; i += 4)
> + vgic_set_target_reg(kvm, 0, i);
> +
> out:
> if (ret)
> kvm_vgic_destroy(kvm);
> @@ -1822,6 +1829,7 @@ out:
> */
> int kvm_vgic_init(struct kvm *kvm)
> {
> + struct kvm_vcpu *vcpu;
> int ret = 0, i;
>
> if (!irqchip_in_kernel(kvm))
> @@ -1839,6 +1847,12 @@ int kvm_vgic_init(struct kvm *kvm)
> goto out;
> }
>
> + ret = vgic_init_maps(kvm);
> + if (ret) {
> + kvm_err("Unable to allocate maps\n");
> + goto out;
> + }
> +
> ret = kvm_phys_addr_ioremap(kvm, kvm->arch.vgic.vgic_cpu_base,
> vgic->vcpu_base, KVM_VGIC_V2_CPU_SIZE);
> if (ret) {
> @@ -1846,11 +1860,13 @@ int kvm_vgic_init(struct kvm *kvm)
> goto out;
> }
>
> - for (i = VGIC_NR_PRIVATE_IRQS; i < kvm->arch.vgic.nr_irqs; i += 4)
> - vgic_set_target_reg(kvm, 0, i);
> + kvm_for_each_vcpu(i, vcpu, kvm)
> + kvm_vgic_vcpu_init(vcpu);
>
> kvm->arch.vgic.ready = true;
> out:
> + if (ret)
> + kvm_vgic_destroy(kvm);
> mutex_unlock(&kvm->lock);
> return ret;
> }
> @@ -1891,10 +1907,6 @@ 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);
> - 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);
> @@ -2095,6 +2107,10 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
>
> mutex_lock(&dev->kvm->lock);
>
> + ret = vgic_init_maps(dev->kvm);
> + if (ret)
> + goto out;
> +
> if (cpuid >= atomic_read(&dev->kvm->online_vcpus)) {
> ret = -EINVAL;
> goto out;
> --
> 2.0.0
>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 8/9] arm/arm64: KVM: vgic: make number of irqs a configurable attribute
2014-07-08 11:08 [PATCH v3 0/9] arm/arm64: KVM: dynamic VGIC sizing Marc Zyngier
` (6 preceding siblings ...)
2014-07-08 11:09 ` [PATCH v3 7/9] arm/arm64: KVM: vgic: delay vgic allocation until init time Marc Zyngier
@ 2014-07-08 11:09 ` Marc Zyngier
2014-08-05 15:39 ` Christoffer Dall
2014-07-08 11:09 ` [PATCH v3 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment Marc Zyngier
8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2014-07-08 11:09 UTC (permalink / raw)
To: linux-arm-kernel
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 708aed9..aee10da 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2208,6 +2208,28 @@ static int vgic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
return vgic_attr_regs_access(dev, attr, ®, 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) || dev->kvm->arch.vgic.nr_irqs)
+ ret = -EBUSY;
+ else
+ dev->kvm->arch.vgic.nr_irqs = val;
+
+ mutex_unlock(&dev->kvm->lock);
+
+ return ret;
+ }
}
@@ -2244,6 +2266,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;
+ }
}
@@ -2280,6 +2307,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;
}
--
2.0.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 8/9] arm/arm64: KVM: vgic: make number of irqs a configurable attribute
2014-07-08 11:09 ` [PATCH v3 8/9] arm/arm64: KVM: vgic: make number of irqs a configurable attribute Marc Zyngier
@ 2014-08-05 15:39 ` Christoffer Dall
0 siblings, 0 replies; 18+ messages in thread
From: Christoffer Dall @ 2014-08-05 15:39 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 08, 2014 at 12:09:07PM +0100, Marc Zyngier wrote:
> In order to make the number of interrupt configurable, use the new
interrupts
> 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 708aed9..aee10da 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -2208,6 +2208,28 @@ static int vgic_set_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
>
> return vgic_attr_regs_access(dev, attr, ®, 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;
> VGIC_MAX_IRQS ?
> +
> + mutex_lock(&dev->kvm->lock);
> +
> + if (vgic_initialized(dev->kvm) || dev->kvm->arch.vgic.nr_irqs)
> + ret = -EBUSY;
> + else
> + dev->kvm->arch.vgic.nr_irqs = val;
> +
> + mutex_unlock(&dev->kvm->lock);
> +
> + return ret;
> + }
>
> }
>
> @@ -2244,6 +2266,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;
> + }
>
> }
>
> @@ -2280,6 +2307,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;
> }
> --
> 2.0.0
>
Please add Documentation of this ABI extension to:
Documentation/virtual/kvm/devices/arm-vgic.txt
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment
2014-07-08 11:08 [PATCH v3 0/9] arm/arm64: KVM: dynamic VGIC sizing Marc Zyngier
` (7 preceding siblings ...)
2014-07-08 11:09 ` [PATCH v3 8/9] arm/arm64: KVM: vgic: make number of irqs a configurable attribute Marc Zyngier
@ 2014-07-08 11:09 ` Marc Zyngier
2014-08-05 15:43 ` Christoffer Dall
8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2014-07-08 11:09 UTC (permalink / raw)
To: linux-arm-kernel
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 aee10da..1e60981 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2271,6 +2271,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;
+ }
}
@@ -2308,6 +2314,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;
--
2.0.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment
2014-07-08 11:09 ` [PATCH v3 9/9] arm64: KVM: vgic: deal with GIC sub-page alignment Marc Zyngier
@ 2014-08-05 15:43 ` Christoffer Dall
0 siblings, 0 replies; 18+ messages in thread
From: Christoffer Dall @ 2014-08-05 15:43 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 08, 2014 at 12:09:08PM +0100, 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.
>
> 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 aee10da..1e60981 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -2271,6 +2271,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;
> + }
>
> }
>
> @@ -2308,6 +2314,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;
> --
> 2.0.0
>
Also here, add documentation to the fancy ABI:
Documentation/virtual/kvm/devices/arm-vgic.txt
When rebased onto the recent patches this will never return anything
else than 0 right? Otherwise KVM would have failed to initialize and
bailed out. What is our solution for this problem again?
-Christoffer
^ permalink raw reply [flat|nested] 18+ messages in thread