2018-04-27 14:21:01

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 00/12] KVM: arm/arm64: Allow multiple GICv3 redistributor regions

At the moment the KVM VGICv3 only supports a single redistributor
region (whose base address is set through the GICv3 kvm device
KVM_DEV_ARM_VGIC_GRP_ADDR/KVM_VGIC_V3_ADDR_TYPE_REDIST). There,
all the redistributors are laid out contiguously. The size of this
single redistributor region is not set explicitly but instead
induced at a late stage by the number of online vcpus.

The GIC specification does not mandate all redistributors to be
contiguous. Moreover DT and ACPI were specified so that multiple
redistributors regions can be defined.

The current interface brings a limitation on QEMU where ARM
virt machine available GPA holes only allowed to assign a
redistributor region fitting a max of 123 vcpus. Overcoming this
limitation would force either to create a new machine or relocate
the single rdist region or allow the allocation of multiple rdist
regions.

This series enables this last alternative. A new GICv3 KVM device
KVM_DEV_ARM_VGIC_GRP_ADDR/KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION allows
to register individual redistributor regions whose size is defined
explicitly. Those rdist regions then are filled by vcpu rdist frames
according to the need. The vgic init and related base address checks
are impacted.

Best Regards

Eric

Git: complete series available at
https://github.com/eauger/linux/tree/v4.17-rc2-rdist-regions-v4
Previous version:
https://github.com/eauger/linux/tree/v4.16-rdist-regions-v3

History:

v3 -> v4:
- Collected Christoffer's R-b
- reworked vgic_v3_rdist_overlap and introduced vgic_dist_overlap
- vgic_v3_insert_redist_region cleanup
- inverse order for patches 6 and 7

v2 -> v3:
- Add details to the user API documentation
- early exit if vgic_v3_rdist_region_from_index() fails
- return -EINVAL if legacy and new redist region API are mixed

v1 -> v2:
- Rework the uapi. Only bits [51:16] of the redist region are
exposed. Also a new flags field was introduced
- Do not store the last bit in the vgic_cpu struct anymore
- remove dist->spis check in 1st patch
- add last patch to bump VGIC_V3_MAX_CPUS to 512
- advertise the new attribute


Eric Auger (12):
KVM: arm/arm64: Set dist->spis to NULL after kfree
KVM: arm/arm64: Document KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION
KVM: arm/arm64: Replace the single rdist region by a list
KVM: arm/arm64: Helper to locate free rdist index
KVM: arm/arm64: Revisit Redistributor TYPER last bit computation
KVM: arm/arm64: Adapt vgic_v3_check_base to multiple rdist regions
KVM: arm/arm64: Helper to register a new redistributor region
KVM: arm/arm64: Check vcpu redist base before registering an iodev
KVM: arm/arm64: Check all vcpu redistributors are set on map_resources
KVM: arm/arm64: Add KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION
KVM: arm/arm64: Implement KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION
KVM: arm/arm64: Bump VGIC_V3_MAX_CPUS to 512

Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 25 ++++-
arch/arm/include/uapi/asm/kvm.h | 1 +
arch/arm64/include/uapi/asm/kvm.h | 1 +
include/kvm/arm_vgic.h | 16 ++-
virt/kvm/arm/vgic/vgic-init.c | 20 +++-
virt/kvm/arm/vgic/vgic-kvm-device.c | 55 ++++++++++-
virt/kvm/arm/vgic/vgic-mmio-v3.c | 114 +++++++++++++++++++---
virt/kvm/arm/vgic/vgic-v3.c | 101 +++++++++++++++----
virt/kvm/arm/vgic/vgic.h | 42 +++++++-
9 files changed, 330 insertions(+), 45 deletions(-)

--
2.5.5



2018-04-27 14:17:22

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 05/12] KVM: arm/arm64: Revisit Redistributor TYPER last bit computation

The TYPER of an redistributor reflects whether the rdist is
the last one of the redistributor region. Let's compare the TYPER
GPA against the address of the last occupied slot within the
redistributor region.

Signed-off-by: Eric Auger <[email protected]>
Reviewed-by: Christoffer Dall <[email protected]>

---

v3 -> v4:
- added Christoffer's R-b
---
virt/kvm/arm/vgic/vgic-mmio-v3.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index 49ca176..ce5c927 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -184,12 +184,17 @@ static unsigned long vgic_mmio_read_v3r_typer(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len)
{
unsigned long mpidr = kvm_vcpu_get_mpidr_aff(vcpu);
+ struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+ struct vgic_redist_region *rdreg = vgic_cpu->rdreg;
int target_vcpu_id = vcpu->vcpu_id;
+ gpa_t last_rdist_typer = rdreg->base + GICR_TYPER +
+ (rdreg->free_index - 1) * KVM_VGIC_V3_REDIST_SIZE;
u64 value;

value = (u64)(mpidr & GENMASK(23, 0)) << 32;
value |= ((target_vcpu_id & 0xffff) << 8);
- if (target_vcpu_id == atomic_read(&vcpu->kvm->online_vcpus) - 1)
+
+ if (addr == last_rdist_typer)
value |= GICR_TYPER_LAST;
if (vgic_has_its(vcpu->kvm))
value |= GICR_TYPER_PLPIS;
--
2.5.5


2018-04-27 14:17:29

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 07/12] KVM: arm/arm64: Helper to register a new redistributor region

We introduce a new helper that creates and inserts a new redistributor
region into the rdist region list. This helper both handles the case
where the redistributor region size is known at registration time
and the legacy case where it is not (eventually depending on the number
of online vcpus). Depending on pfns, we perform all the possible checks
that we can do:

- end of memory crossing
- incorrect alignment of the base address
- collision with distributor region if already defined
- collision with already registered rdist regions
- check of the new index

Rdist regions must be inserted by increasing order of indices. Indices
must be contiguous.

Signed-off-by: Eric Auger <[email protected]>

---

v3 -> v4:
- inversed check in vgic_v3_rdist_overlap
- use list_last_entry in vgic_v3_insert_redist_region
- improve comment in vgic_v3_insert_redist_region
- introduce vgic_dist_overlap
---
virt/kvm/arm/vgic/vgic-mmio-v3.c | 89 ++++++++++++++++++++++++++++++++--------
virt/kvm/arm/vgic/vgic.h | 8 ++++
2 files changed, 81 insertions(+), 16 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index ce5c927..cbf8f4e 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -680,14 +680,63 @@ static int vgic_register_all_redist_iodevs(struct kvm *kvm)
return ret;
}

-int vgic_v3_set_redist_base(struct kvm *kvm, u64 addr)
+/**
+ * vgic_v3_insert_redist_region - Insert a new redistributor region
+ *
+ * Performs various checks before inserting the rdist region in the list.
+ * Those tests depend on whether the size of the rdist region is known
+ * (ie. count != 0). The list is sorted by rdist region index.
+ *
+ * @kvm: kvm handle
+ * @index: redist region index
+ * @base: base of the new rdist region
+ * @count: number of redistributors the region is made of (of 0 in the old style
+ * single region, whose size is induced from the number of vcpus)
+ *
+ * Return 0 on success, < 0 otherwise
+ */
+static int vgic_v3_insert_redist_region(struct kvm *kvm, uint32_t index,
+ gpa_t base, uint32_t count)
{
- struct vgic_dist *vgic = &kvm->arch.vgic;
+ struct vgic_dist *d = &kvm->arch.vgic;
struct vgic_redist_region *rdreg;
+ struct list_head *rd_regions = &d->rd_regions;
+ struct list_head *last = rd_regions->prev;
+ size_t size = count * KVM_VGIC_V3_REDIST_SIZE;
int ret;

- /* vgic_check_ioaddr makes sure we don't do this twice */
- if (!list_empty(&vgic->rd_regions))
+ /* single rdist region already set ?*/
+ if (!count && !list_empty(rd_regions))
+ return -EINVAL;
+
+ /* cross the end of memory ? */
+ if (base + size < base)
+ return -EINVAL;
+
+ if (list_empty(rd_regions)) {
+ if (index != 0)
+ return -EINVAL;
+ } else {
+ rdreg = list_entry(last, struct vgic_redist_region, list);
+ if (index != rdreg->index + 1)
+ return -EINVAL;
+
+ /* Cannot add an explicitly sized regions after legacy region */
+ if (!rdreg->count)
+ return -EINVAL;
+ }
+
+ /*
+ * For legacy single-region redistributor regions (!count),
+ * check that the redistributor region does not overlap with the
+ * distributor's address space.
+ */
+ if (!count && !IS_VGIC_ADDR_UNDEF(d->vgic_dist_base) &&
+ vgic_dist_overlap(kvm, base, size))
+ return -EINVAL;
+
+ /* collision with any other rdist region? */
+ if (vgic_v3_rdist_overlap(kvm, base, size))
return -EINVAL;

rdreg = kzalloc(sizeof(*rdreg), GFP_KERNEL);
@@ -696,17 +745,29 @@ int vgic_v3_set_redist_base(struct kvm *kvm, u64 addr)

rdreg->base = VGIC_ADDR_UNDEF;

- ret = vgic_check_ioaddr(kvm, &rdreg->base, addr, SZ_64K);
+ ret = vgic_check_ioaddr(kvm, &rdreg->base, base, SZ_64K);
if (ret)
- goto out;
+ goto free;

- rdreg->base = addr;
- if (!vgic_v3_check_base(kvm)) {
- ret = -EINVAL;
- goto out;
- }
+ rdreg->base = base;
+ rdreg->count = count;
+ rdreg->free_index = 0;
+ rdreg->index = index;

- list_add(&rdreg->list, &vgic->rd_regions);
+ list_add_tail(&rdreg->list, rd_regions);
+ return 0;
+free:
+ kfree(rdreg);
+ return ret;
+}
+
+int vgic_v3_set_redist_base(struct kvm *kvm, u64 addr)
+{
+ int ret;
+
+ ret = vgic_v3_insert_redist_region(kvm, 0, addr, 0);
+ if (ret)
+ return ret;

/*
* Register iodevs for each existing VCPU. Adding more VCPUs
@@ -717,10 +778,6 @@ int vgic_v3_set_redist_base(struct kvm *kvm, u64 addr)
return ret;

return 0;
-
-out:
- kfree(rdreg);
- return ret;
}

int vgic_v3_has_attr_regs(struct kvm_device *dev, struct kvm_device_attr *attr)
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index e6e3ae9..6af7d8a 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -272,6 +272,14 @@ vgic_v3_rd_region_size(struct kvm *kvm, struct vgic_redist_region *rdreg)
}
bool vgic_v3_rdist_overlap(struct kvm *kvm, gpa_t base, size_t size);

+static inline bool vgic_dist_overlap(struct kvm *kvm, gpa_t base, size_t size)
+{
+ struct vgic_dist *d = &kvm->arch.vgic;
+
+ return (base + size > d->vgic_dist_base) &&
+ (base < d->vgic_dist_base + KVM_VGIC_V3_DIST_SIZE);
+}
+
int vgic_its_resolve_lpi(struct kvm *kvm, struct vgic_its *its,
u32 devid, u32 eventid, struct vgic_irq **irq);
struct vgic_its *vgic_msi_to_its(struct kvm *kvm, struct kvm_msi *msi);
--
2.5.5


2018-04-27 14:17:40

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 10/12] KVM: arm/arm64: Add KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION

This new attribute allows the userspace to set the base address
of a reditributor region, relaxing the constraint of having all
consecutive redistibutor frames contiguous.

Signed-off-by: Eric Auger <[email protected]>
Acked-by: Christoffer Dall <[email protected]>

---

v3 -> v4:
- keep previous indentation for existing attributes
- added Christoffer's A-b
---
arch/arm/include/uapi/asm/kvm.h | 1 +
arch/arm64/include/uapi/asm/kvm.h | 1 +
2 files changed, 2 insertions(+)

diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index 2ba95d6..e4f243f 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -91,6 +91,7 @@ struct kvm_regs {
#define KVM_VGIC_V3_ADDR_TYPE_DIST 2
#define KVM_VGIC_V3_ADDR_TYPE_REDIST 3
#define KVM_VGIC_ITS_ADDR_TYPE 4
+#define KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION 5

#define KVM_VGIC_V3_DIST_SIZE SZ_64K
#define KVM_VGIC_V3_REDIST_SIZE (2 * SZ_64K)
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 9abbf30..df0f47c 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -91,6 +91,7 @@ struct kvm_regs {
#define KVM_VGIC_V3_ADDR_TYPE_DIST 2
#define KVM_VGIC_V3_ADDR_TYPE_REDIST 3
#define KVM_VGIC_ITS_ADDR_TYPE 4
+#define KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION 5

#define KVM_VGIC_V3_DIST_SIZE SZ_64K
#define KVM_VGIC_V3_REDIST_SIZE (2 * SZ_64K)
--
2.5.5


2018-04-27 14:17:59

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 12/12] KVM: arm/arm64: Bump VGIC_V3_MAX_CPUS to 512

Let's raise the number of supported vcpus along with
vgic v3 now that HW is looming with more physical CPUs.

Signed-off-by: Eric Auger <[email protected]>
---
include/kvm/arm_vgic.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index e5c16d1..a9a9f4a 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -28,7 +28,7 @@

#include <linux/irqchip/arm-gic-v4.h>

-#define VGIC_V3_MAX_CPUS 255
+#define VGIC_V3_MAX_CPUS 512
#define VGIC_V2_MAX_CPUS 8
#define VGIC_NR_IRQS_LEGACY 256
#define VGIC_NR_SGIS 16
--
2.5.5


2018-04-27 14:18:06

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 09/12] KVM: arm/arm64: Check all vcpu redistributors are set on map_resources

On vcpu first run, we eventually know the actual number of vcpus.
This is a synchronization point to check all redistributors
were assigned. On kvm_vgic_map_resources() we check both dist and
redist were set, eventually check potential base address inconsistencies.

Signed-off-by: Eric Auger <[email protected]>

---

v3 -> v4:
- use kvm_debug
---
virt/kvm/arm/vgic/vgic-v3.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index c4a2a46..5563671 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -484,16 +484,25 @@ struct vgic_redist_region *vgic_v3_rdist_free_slot(struct list_head *rd_regions)

int vgic_v3_map_resources(struct kvm *kvm)
{
- int ret = 0;
struct vgic_dist *dist = &kvm->arch.vgic;
- struct vgic_redist_region *rdreg =
- list_first_entry(&dist->rd_regions,
- struct vgic_redist_region, list);
+ struct kvm_vcpu *vcpu;
+ int ret = 0;
+ int c;

if (vgic_ready(kvm))
goto out;

- if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base) || !rdreg) {
+ kvm_for_each_vcpu(c, vcpu, kvm) {
+ struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+
+ if (IS_VGIC_ADDR_UNDEF(vgic_cpu->rd_iodev.base_addr)) {
+ kvm_debug("vcpu %d redistributor base not set\n", c);
+ ret = -ENXIO;
+ goto out;
+ }
+ }
+
+ if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base)) {
kvm_err("Need to set vgic distributor addresses first\n");
ret = -ENXIO;
goto out;
--
2.5.5


2018-04-27 14:18:25

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 11/12] KVM: arm/arm64: Implement KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION

Now all the internals are ready to handle multiple redistributor
regions, let's allow the userspace to register them.

Signed-off-by: Eric Auger <[email protected]>

---
v3 -> v4:
- vgic_v3_rdist_region_from_index is introduced in this patch

v2 -> v3:
- early exit if vgic_v3_rdist_region_from_index() fails
---
virt/kvm/arm/vgic/vgic-kvm-device.c | 42 +++++++++++++++++++++++++++++++++++--
virt/kvm/arm/vgic/vgic-mmio-v3.c | 4 ++--
virt/kvm/arm/vgic/vgic-v3.c | 14 +++++++++++++
virt/kvm/arm/vgic/vgic.h | 13 +++++++++++-
4 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
index e7b5a86..00e03d3 100644
--- a/virt/kvm/arm/vgic/vgic-kvm-device.c
+++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
@@ -65,7 +65,8 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
{
int r = 0;
struct vgic_dist *vgic = &kvm->arch.vgic;
- phys_addr_t *addr_ptr, alignment;
+ phys_addr_t *addr_ptr = NULL;
+ phys_addr_t alignment;
uint64_t undef_value = VGIC_ADDR_UNDEF;

mutex_lock(&kvm->lock);
@@ -92,7 +93,7 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
if (r)
break;
if (write) {
- r = vgic_v3_set_redist_base(kvm, *addr);
+ r = vgic_v3_set_redist_base(kvm, 0, *addr, 0);
goto out;
}
rdreg = list_first_entry(&vgic->rd_regions,
@@ -103,6 +104,42 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
addr_ptr = &rdreg->base;
break;
}
+ case KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION:
+ {
+ struct vgic_redist_region *rdreg;
+ uint8_t index;
+
+ r = vgic_check_type(kvm, KVM_DEV_TYPE_ARM_VGIC_V3);
+ if (r)
+ break;
+
+ index = *addr & KVM_VGIC_V3_RDIST_INDEX_MASK;
+
+ if (write) {
+ gpa_t base = *addr & KVM_VGIC_V3_RDIST_BASE_MASK;
+ uint32_t count = (*addr & KVM_VGIC_V3_RDIST_COUNT_MASK)
+ >> KVM_VGIC_V3_RDIST_COUNT_SHIFT;
+ uint8_t flags = (*addr & KVM_VGIC_V3_RDIST_FLAGS_MASK)
+ >> KVM_VGIC_V3_RDIST_FLAGS_SHIFT;
+
+ if (!count || flags)
+ r = -EINVAL;
+ else
+ r = vgic_v3_set_redist_base(kvm, index,
+ base, count);
+ goto out;
+ }
+
+ rdreg = vgic_v3_rdist_region_from_index(kvm, index);
+ if (!rdreg) {
+ r = -ENODEV;
+ goto out;
+ }
+
+ *addr_ptr = rdreg->base & index &
+ (uint64_t)rdreg->count << KVM_VGIC_V3_RDIST_COUNT_SHIFT;
+ break;
+ }
default:
r = -ENODEV;
}
@@ -674,6 +711,7 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
switch (attr->attr) {
case KVM_VGIC_V3_ADDR_TYPE_DIST:
case KVM_VGIC_V3_ADDR_TYPE_REDIST:
+ case KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION:
return 0;
}
break;
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index 1b1e07f..fcebd42 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -764,11 +764,11 @@ static int vgic_v3_insert_redist_region(struct kvm *kvm, uint32_t index,
return ret;
}

-int vgic_v3_set_redist_base(struct kvm *kvm, u64 addr)
+int vgic_v3_set_redist_base(struct kvm *kvm, u32 index, u64 addr, u32 count)
{
int ret;

- ret = vgic_v3_insert_redist_region(kvm, 0, addr, 0);
+ ret = vgic_v3_insert_redist_region(kvm, index, addr, count);
if (ret)
return ret;

diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index 5563671..ba3baa2 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -482,6 +482,20 @@ struct vgic_redist_region *vgic_v3_rdist_free_slot(struct list_head *rd_regions)
return NULL;
}

+struct vgic_redist_region *vgic_v3_rdist_region_from_index(struct kvm *kvm,
+ uint32_t index)
+{
+ struct list_head *rd_regions = &kvm->arch.vgic.rd_regions;
+ struct vgic_redist_region *rdreg;
+
+ list_for_each_entry(rdreg, rd_regions, list) {
+ if (rdreg->index == index)
+ return rdreg;
+ }
+ return NULL;
+}
+
+
int vgic_v3_map_resources(struct kvm *kvm)
{
struct vgic_dist *dist = &kvm->arch.vgic;
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index 6af7d8a..9552d26 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -96,6 +96,13 @@
/* we only support 64 kB translation table page size */
#define KVM_ITS_L1E_ADDR_MASK GENMASK_ULL(51, 16)

+#define KVM_VGIC_V3_RDIST_INDEX_MASK GENMASK_ULL(11, 0)
+#define KVM_VGIC_V3_RDIST_FLAGS_MASK GENMASK_ULL(15, 12)
+#define KVM_VGIC_V3_RDIST_FLAGS_SHIFT 12
+#define KVM_VGIC_V3_RDIST_BASE_MASK GENMASK_ULL(51, 16)
+#define KVM_VGIC_V3_RDIST_COUNT_MASK GENMASK_ULL(63, 52)
+#define KVM_VGIC_V3_RDIST_COUNT_SHIFT 52
+
/* Requires the irq_lock to be held by the caller. */
static inline bool irq_is_pending(struct vgic_irq *irq)
{
@@ -201,7 +208,7 @@ int vgic_v3_probe(const struct gic_kvm_info *info);
int vgic_v3_map_resources(struct kvm *kvm);
int vgic_v3_lpi_sync_pending_status(struct kvm *kvm, struct vgic_irq *irq);
int vgic_v3_save_pending_tables(struct kvm *kvm);
-int vgic_v3_set_redist_base(struct kvm *kvm, u64 addr);
+int vgic_v3_set_redist_base(struct kvm *kvm, u32 index, u64 addr, u32 count);
int vgic_register_redist_iodev(struct kvm_vcpu *vcpu);
bool vgic_v3_check_base(struct kvm *kvm);

@@ -270,6 +277,10 @@ vgic_v3_rd_region_size(struct kvm *kvm, struct vgic_redist_region *rdreg)
else
return rdreg->count * KVM_VGIC_V3_REDIST_SIZE;
}
+
+struct vgic_redist_region *vgic_v3_rdist_region_from_index(struct kvm *kvm,
+ uint32_t index);
+
bool vgic_v3_rdist_overlap(struct kvm *kvm, gpa_t base, size_t size);

static inline bool vgic_dist_overlap(struct kvm *kvm, gpa_t base, size_t size)
--
2.5.5


2018-04-27 14:19:02

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 08/12] KVM: arm/arm64: Check vcpu redist base before registering an iodev

As we are going to register several redist regions,
vgic_register_all_redist_iodevs() may be called several times. We need
to register a redist_iodev for a given vcpu only once. So let's
check if the base address has already been set. Initialize this latter
in kvm_vgic_vcpu_early_init().

Signed-off-by: Eric Auger <[email protected]>
---
virt/kvm/arm/vgic/vgic-init.c | 3 +++
virt/kvm/arm/vgic/vgic-mmio-v3.c | 3 +++
2 files changed, 6 insertions(+)

diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 6456371..7e040e7 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -82,6 +82,9 @@ void kvm_vgic_vcpu_early_init(struct kvm_vcpu *vcpu)
INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
spin_lock_init(&vgic_cpu->ap_list_lock);

+ vgic_cpu->rd_iodev.base_addr = VGIC_ADDR_UNDEF;
+ vgic_cpu->sgi_iodev.base_addr = VGIC_ADDR_UNDEF;
+
/*
* Enable and configure all SGIs to be edge-triggered and
* configure all PPIs as level-triggered.
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index cbf8f4e..1b1e07f 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -592,6 +592,9 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu)
gpa_t rd_base, sgi_base;
int ret;

+ if (!IS_VGIC_ADDR_UNDEF(vgic_cpu->rd_iodev.base_addr))
+ return 0;
+
/*
* We may be creating VCPUs before having set the base address for the
* redistributor region, in which case we will come back to this
--
2.5.5


2018-04-27 14:19:33

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 06/12] KVM: arm/arm64: Adapt vgic_v3_check_base to multiple rdist regions

vgic_v3_check_base() currently only handles the case of a unique
legacy redistributor region whose size is not explicitly set but
infered, instead, from the number of online vcpus.

We adapt it to handle the case of multiple redistributor regions
with explicitly defined size. We rely on two new helpers:
- vgic_v3_rdist_overlap() is used to detect overlap with the dist
region if defined
- vgic_v3_rd_region_size computes the size of the redist region,
would it be a legacy unique region or a new explicitly sized
region.

Signed-off-by: Eric Auger <[email protected]>

---

v3 -> v4:
- squash vgic_v3_check_base adaptation and vgic_v3_rdist_overlap
+ vgic_v3_rd_region_size introduction and put this patch
before v3 patch 6
---
virt/kvm/arm/vgic/vgic-v3.c | 49 +++++++++++++++++++++++++++++----------------
virt/kvm/arm/vgic/vgic.h | 10 +++++++++
2 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index f81a63a..c4a2a46 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -410,6 +410,29 @@ int vgic_v3_save_pending_tables(struct kvm *kvm)
return 0;
}

+/**
+ * vgic_v3_rdist_overlap - check if a region overlaps with any
+ * existing redistributor region
+ *
+ * @kvm: kvm handle
+ * @base: base of the region
+ * @size: size of region
+ *
+ * Return: true if there is an overlap
+ */
+bool vgic_v3_rdist_overlap(struct kvm *kvm, gpa_t base, size_t size)
+{
+ struct vgic_dist *d = &kvm->arch.vgic;
+ struct vgic_redist_region *rdreg;
+
+ list_for_each_entry(rdreg, &d->rd_regions, list) {
+ if ((base + size > rdreg->base) &&
+ (base < rdreg->base + vgic_v3_rd_region_size(kvm, rdreg)))
+ return true;
+ }
+ return false;
+}
+
/*
* Check for overlapping regions and for regions crossing the end of memory
* for base addresses which have already been set.
@@ -417,31 +440,23 @@ int vgic_v3_save_pending_tables(struct kvm *kvm)
bool vgic_v3_check_base(struct kvm *kvm)
{
struct vgic_dist *d = &kvm->arch.vgic;
- gpa_t redist_size = KVM_VGIC_V3_REDIST_SIZE;
- struct vgic_redist_region *rdreg =
- list_first_entry(&d->rd_regions,
- struct vgic_redist_region, list);
-
- redist_size *= atomic_read(&kvm->online_vcpus);
+ struct vgic_redist_region *rdreg;

if (!IS_VGIC_ADDR_UNDEF(d->vgic_dist_base) &&
d->vgic_dist_base + KVM_VGIC_V3_DIST_SIZE < d->vgic_dist_base)
return false;

- if (rdreg && (rdreg->base + redist_size < rdreg->base))
- return false;
+ list_for_each_entry(rdreg, &d->rd_regions, list) {
+ if (rdreg->base + vgic_v3_rd_region_size(kvm, rdreg) <
+ rdreg->base)
+ return false;
+ }

- /* Both base addresses must be set to check if they overlap */
- if (IS_VGIC_ADDR_UNDEF(d->vgic_dist_base) || !rdreg)
+ if (IS_VGIC_ADDR_UNDEF(d->vgic_dist_base))
return true;

- if (d->vgic_dist_base + KVM_VGIC_V3_DIST_SIZE <= rdreg->base)
- return true;
-
- if (rdreg->base + redist_size <= d->vgic_dist_base)
- return true;
-
- return false;
+ return !vgic_v3_rdist_overlap(kvm, d->vgic_dist_base,
+ KVM_VGIC_V3_DIST_SIZE);
}

/**
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index fea32cb..e6e3ae9 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -262,6 +262,16 @@ vgic_v3_redist_region_full(struct vgic_redist_region *region)

struct vgic_redist_region *vgic_v3_rdist_free_slot(struct list_head *rdregs);

+static inline size_t
+vgic_v3_rd_region_size(struct kvm *kvm, struct vgic_redist_region *rdreg)
+{
+ if (!rdreg->count)
+ return atomic_read(&kvm->online_vcpus) * KVM_VGIC_V3_REDIST_SIZE;
+ else
+ return rdreg->count * KVM_VGIC_V3_REDIST_SIZE;
+}
+bool vgic_v3_rdist_overlap(struct kvm *kvm, gpa_t base, size_t size);
+
int vgic_its_resolve_lpi(struct kvm *kvm, struct vgic_its *its,
u32 devid, u32 eventid, struct vgic_irq **irq);
struct vgic_its *vgic_msi_to_its(struct kvm *kvm, struct kvm_msi *msi);
--
2.5.5


2018-04-27 14:20:11

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 04/12] KVM: arm/arm64: Helper to locate free rdist index

We introduce vgic_v3_rdist_free_slot to help identifying
where we can place a new 2x64KB redistributor.

Signed-off-by: Eric Auger <[email protected]>
Reviewed-by: Christoffer Dall <[email protected]>

---

v3 -> v4:
- add details to vgic_v3_rdist_free_slot kernel doc comment
- Added Christoffer's R-b
---
virt/kvm/arm/vgic/vgic-mmio-v3.c | 3 +--
virt/kvm/arm/vgic/vgic-v3.c | 23 +++++++++++++++++++++++
virt/kvm/arm/vgic/vgic.h | 11 +++++++++++
3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index d1aab18..49ca176 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -593,8 +593,7 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu)
* function for all VCPUs when the base address is set. Just return
* without doing any work for now.
*/
- rdreg = list_first_entry(&vgic->rd_regions,
- struct vgic_redist_region, list);
+ rdreg = vgic_v3_rdist_free_slot(&vgic->rd_regions);
if (!rdreg)
return 0;

diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index 94de6cd..f81a63a 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -444,6 +444,29 @@ bool vgic_v3_check_base(struct kvm *kvm)
return false;
}

+/**
+ * vgic_v3_rdist_free_slot - Look up registered rdist regions and identify one
+ * which has free space to put a new rdist region.
+ *
+ * @rd_regions: redistributor region list head
+ *
+ * A redistributor regions maps n redistributors, n = region size / (2 x 64kB).
+ * Stride between redistributors is 0 and regions are filled in the index order.
+ *
+ * Return: the redist region handle, if any, that has space to map a new rdist
+ * region.
+ */
+struct vgic_redist_region *vgic_v3_rdist_free_slot(struct list_head *rd_regions)
+{
+ struct vgic_redist_region *rdreg;
+
+ list_for_each_entry(rdreg, rd_regions, list) {
+ if (!vgic_v3_redist_region_full(rdreg))
+ return rdreg;
+ }
+ return NULL;
+}
+
int vgic_v3_map_resources(struct kvm *kvm)
{
int ret = 0;
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index 830e815..fea32cb 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -251,6 +251,17 @@ static inline int vgic_v3_max_apr_idx(struct kvm_vcpu *vcpu)
}
}

+static inline bool
+vgic_v3_redist_region_full(struct vgic_redist_region *region)
+{
+ if (!region->count)
+ return false;
+
+ return (region->free_index >= region->count);
+}
+
+struct vgic_redist_region *vgic_v3_rdist_free_slot(struct list_head *rdregs);
+
int vgic_its_resolve_lpi(struct kvm *kvm, struct vgic_its *its,
u32 devid, u32 eventid, struct vgic_irq **irq);
struct vgic_its *vgic_msi_to_its(struct kvm *kvm, struct kvm_msi *msi);
--
2.5.5


2018-04-27 14:20:20

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 01/12] KVM: arm/arm64: Set dist->spis to NULL after kfree

in case kvm_vgic_map_resources() fails, typically if the vgic
distributor is not defined, __kvm_vgic_destroy will be called
several times. Indeed kvm_vgic_map_resources() is called on
first vcpu run. As a result dist->spis is freeed more than once
and on the second time it causes a "kernel BUG at mm/slub.c:3912!"

Set dist->spis to NULL to avoid the crash.

Fixes: ad275b8bb1e6 ("KVM: arm/arm64: vgic-new: vgic_init: implement
vgic_init")

Signed-off-by: Eric Auger <[email protected]>
Reviewed-by: Marc Zyngier <[email protected]>
Reviewed-by: Christoffer Dall <[email protected]>

---

v2 -> v3:
- added Marc's R-b and Fixed commit
---
virt/kvm/arm/vgic/vgic-init.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 68378fe..c52f03d 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -308,6 +308,7 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm)
dist->initialized = false;

kfree(dist->spis);
+ dist->spis = NULL;
dist->nr_spis = 0;

if (vgic_supports_direct_msis(kvm))
--
2.5.5


2018-04-27 14:20:55

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 02/12] KVM: arm/arm64: Document KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION

We introduce a new KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION attribute in
KVM_DEV_ARM_VGIC_GRP_ADDR group. It allows userspace to provide the
base address and size of a redistributor region

Compared to KVM_VGIC_V3_ADDR_TYPE_REDIST, this new attribute allows
to declare several separate redistributor regions.

So the whole redist space does not need to be contiguous anymore.

Signed-off-by: Eric Auger <[email protected]>
Reviewed-by: Peter Maydell <[email protected]>

---
v3 -> v4:
- Added Peter's R-b
---
Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 25 ++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
index 9293b45..cbc4328 100644
--- a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
+++ b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
@@ -27,9 +27,32 @@ Groups:
VCPU and all of the redistributor pages are contiguous.
Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
This address needs to be 64K aligned.
+
+ KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION (rw, 64-bit)
+ The attr field of kvm_device_attr encodes 3 values:
+ bits: | 63 .... 52 | 51 .... 16 | 15 - 12 |11 - 0
+ values: | count | base | flags | index
+ - index encodes the unique redistributor region index
+ - flags: reserved for future use, currently 0
+ - base field encodes bits [51:16] of the guest physical base address
+ of the first redistributor in the region.
+ - count encodes the number of redistributors in the region. Must be
+ greater than 0.
+ There are two 64K pages for each redistributor in the region and
+ redistributors are laid out contiguously within the region. Regions
+ are filled with redistributors in the index order. The sum of all
+ region count fields must be greater than or equal to the number of
+ VCPUs. Redistributor regions must be registered in the incremental
+ index order, starting from index 0.
+ Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
+
+ It is invalid to mix calls with KVM_VGIC_V3_ADDR_TYPE_REDIST and
+ KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION attributes.
+
Errors:
-E2BIG: Address outside of addressable IPA range
- -EINVAL: Incorrectly aligned address
+ -EINVAL: Incorrectly aligned address, bad redistributor region
+ count/index, mixed redistributor region attribute usage
-EEXIST: Address already configured
-ENXIO: The group or attribute is unknown/unsupported for this device
or hardware support is missing.
--
2.5.5


2018-04-27 14:21:08

by Eric Auger

[permalink] [raw]
Subject: [PATCH v4 03/12] KVM: arm/arm64: Replace the single rdist region by a list

At the moment KVM supports a single rdist region. We want to
support several separate rdist regions so let's introduce a list
of them. This patch currently only cares about a single
entry in this list as the functionality to register several redist
regions is not yet there. So this only translates the existing code
into something functionally similar using that new data struct.

The redistributor region handle is stored in the vgic_cpu structure
to allow later computation of the TYPER last bit.

Signed-off-by: Eric Auger <[email protected]>
Reviewed-by: Christoffer Dall <[email protected]>

---
v3 -> v4:
- Added Christoffer's R-b
---
include/kvm/arm_vgic.h | 14 +++++++++----
virt/kvm/arm/vgic/vgic-init.c | 16 ++++++++++++--
virt/kvm/arm/vgic/vgic-kvm-device.c | 13 ++++++++++--
virt/kvm/arm/vgic/vgic-mmio-v3.c | 42 ++++++++++++++++++++++++++++---------
virt/kvm/arm/vgic/vgic-v3.c | 20 +++++++++++-------
5 files changed, 79 insertions(+), 26 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 24f0394..e5c16d1 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -200,6 +200,14 @@ struct vgic_its {

struct vgic_state_iter;

+struct vgic_redist_region {
+ uint32_t index;
+ gpa_t base;
+ uint32_t count; /* number of redistributors or 0 if single region */
+ uint32_t free_index; /* index of the next free redistributor */
+ struct list_head list;
+};
+
struct vgic_dist {
bool in_kernel;
bool ready;
@@ -219,10 +227,7 @@ struct vgic_dist {
/* either a GICv2 CPU interface */
gpa_t vgic_cpu_base;
/* or a number of GICv3 redistributor regions */
- struct {
- gpa_t vgic_redist_base;
- gpa_t vgic_redist_free_offset;
- };
+ struct list_head rd_regions;
};

/* distributor enabled */
@@ -310,6 +315,7 @@ struct vgic_cpu {
*/
struct vgic_io_device rd_iodev;
struct vgic_io_device sgi_iodev;
+ struct vgic_redist_region *rdreg;

/* Contains the attributes and gpa of the LPI pending tables. */
u64 pendbaser;
diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index c52f03d..6456371 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -167,8 +167,11 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
kvm->arch.vgic.vgic_model = type;

kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
- kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
- kvm->arch.vgic.vgic_redist_base = VGIC_ADDR_UNDEF;
+
+ if (type == KVM_DEV_TYPE_ARM_VGIC_V2)
+ kvm->arch.vgic.vgic_cpu_base = VGIC_ADDR_UNDEF;
+ else
+ INIT_LIST_HEAD(&kvm->arch.vgic.rd_regions);

out_unlock:
for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
@@ -303,6 +306,7 @@ int vgic_init(struct kvm *kvm)
static void kvm_vgic_dist_destroy(struct kvm *kvm)
{
struct vgic_dist *dist = &kvm->arch.vgic;
+ struct vgic_redist_region *rdreg, *next;

dist->ready = false;
dist->initialized = false;
@@ -311,6 +315,14 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm)
dist->spis = NULL;
dist->nr_spis = 0;

+ if (kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
+ list_for_each_entry_safe(rdreg, next, &dist->rd_regions, list) {
+ list_del(&rdreg->list);
+ kfree(rdreg);
+ }
+ INIT_LIST_HEAD(&dist->rd_regions);
+ }
+
if (vgic_supports_direct_msis(kvm))
vgic_v4_teardown(kvm);
}
diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
index 10ae6f3..e7b5a86 100644
--- a/virt/kvm/arm/vgic/vgic-kvm-device.c
+++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
@@ -66,6 +66,7 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
int r = 0;
struct vgic_dist *vgic = &kvm->arch.vgic;
phys_addr_t *addr_ptr, alignment;
+ uint64_t undef_value = VGIC_ADDR_UNDEF;

mutex_lock(&kvm->lock);
switch (type) {
@@ -84,7 +85,9 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
addr_ptr = &vgic->vgic_dist_base;
alignment = SZ_64K;
break;
- case KVM_VGIC_V3_ADDR_TYPE_REDIST:
+ case KVM_VGIC_V3_ADDR_TYPE_REDIST: {
+ struct vgic_redist_region *rdreg;
+
r = vgic_check_type(kvm, KVM_DEV_TYPE_ARM_VGIC_V3);
if (r)
break;
@@ -92,8 +95,14 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
r = vgic_v3_set_redist_base(kvm, *addr);
goto out;
}
- addr_ptr = &vgic->vgic_redist_base;
+ rdreg = list_first_entry(&vgic->rd_regions,
+ struct vgic_redist_region, list);
+ if (!rdreg)
+ addr_ptr = &undef_value;
+ else
+ addr_ptr = &rdreg->base;
break;
+ }
default:
r = -ENODEV;
}
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index 671fe81..d1aab18 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -580,8 +580,10 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu)
{
struct kvm *kvm = vcpu->kvm;
struct vgic_dist *vgic = &kvm->arch.vgic;
+ struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
struct vgic_io_device *rd_dev = &vcpu->arch.vgic_cpu.rd_iodev;
struct vgic_io_device *sgi_dev = &vcpu->arch.vgic_cpu.sgi_iodev;
+ struct vgic_redist_region *rdreg;
gpa_t rd_base, sgi_base;
int ret;

@@ -591,13 +593,17 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu)
* function for all VCPUs when the base address is set. Just return
* without doing any work for now.
*/
- if (IS_VGIC_ADDR_UNDEF(vgic->vgic_redist_base))
+ rdreg = list_first_entry(&vgic->rd_regions,
+ struct vgic_redist_region, list);
+ if (!rdreg)
return 0;

if (!vgic_v3_check_base(kvm))
return -EINVAL;

- rd_base = vgic->vgic_redist_base + vgic->vgic_redist_free_offset;
+ vgic_cpu->rdreg = rdreg;
+
+ rd_base = rdreg->base + rdreg->free_index * KVM_VGIC_V3_REDIST_SIZE;
sgi_base = rd_base + SZ_64K;

kvm_iodevice_init(&rd_dev->dev, &kvm_io_gic_ops);
@@ -631,7 +637,7 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu)
goto out;
}

- vgic->vgic_redist_free_offset += 2 * SZ_64K;
+ rdreg->free_index++;
out:
mutex_unlock(&kvm->slots_lock);
return ret;
@@ -673,19 +679,31 @@ static int vgic_register_all_redist_iodevs(struct kvm *kvm)
int vgic_v3_set_redist_base(struct kvm *kvm, u64 addr)
{
struct vgic_dist *vgic = &kvm->arch.vgic;
+ struct vgic_redist_region *rdreg;
int ret;

/* vgic_check_ioaddr makes sure we don't do this twice */
- ret = vgic_check_ioaddr(kvm, &vgic->vgic_redist_base, addr, SZ_64K);
- if (ret)
- return ret;
-
- vgic->vgic_redist_base = addr;
- if (!vgic_v3_check_base(kvm)) {
- vgic->vgic_redist_base = VGIC_ADDR_UNDEF;
+ if (!list_empty(&vgic->rd_regions))
return -EINVAL;
+
+ rdreg = kzalloc(sizeof(*rdreg), GFP_KERNEL);
+ if (!rdreg)
+ return -ENOMEM;
+
+ rdreg->base = VGIC_ADDR_UNDEF;
+
+ ret = vgic_check_ioaddr(kvm, &rdreg->base, addr, SZ_64K);
+ if (ret)
+ goto out;
+
+ rdreg->base = addr;
+ if (!vgic_v3_check_base(kvm)) {
+ ret = -EINVAL;
+ goto out;
}

+ list_add(&rdreg->list, &vgic->rd_regions);
+
/*
* Register iodevs for each existing VCPU. Adding more VCPUs
* afterwards will register the iodevs when needed.
@@ -695,6 +713,10 @@ int vgic_v3_set_redist_base(struct kvm *kvm, u64 addr)
return ret;

return 0;
+
+out:
+ kfree(rdreg);
+ return ret;
}

int vgic_v3_has_attr_regs(struct kvm_device *dev, struct kvm_device_attr *attr)
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index 8195f52..94de6cd 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -418,6 +418,9 @@ bool vgic_v3_check_base(struct kvm *kvm)
{
struct vgic_dist *d = &kvm->arch.vgic;
gpa_t redist_size = KVM_VGIC_V3_REDIST_SIZE;
+ struct vgic_redist_region *rdreg =
+ list_first_entry(&d->rd_regions,
+ struct vgic_redist_region, list);

redist_size *= atomic_read(&kvm->online_vcpus);

@@ -425,18 +428,17 @@ bool vgic_v3_check_base(struct kvm *kvm)
d->vgic_dist_base + KVM_VGIC_V3_DIST_SIZE < d->vgic_dist_base)
return false;

- if (!IS_VGIC_ADDR_UNDEF(d->vgic_redist_base) &&
- d->vgic_redist_base + redist_size < d->vgic_redist_base)
+ if (rdreg && (rdreg->base + redist_size < rdreg->base))
return false;

/* Both base addresses must be set to check if they overlap */
- if (IS_VGIC_ADDR_UNDEF(d->vgic_dist_base) ||
- IS_VGIC_ADDR_UNDEF(d->vgic_redist_base))
+ if (IS_VGIC_ADDR_UNDEF(d->vgic_dist_base) || !rdreg)
return true;

- if (d->vgic_dist_base + KVM_VGIC_V3_DIST_SIZE <= d->vgic_redist_base)
+ if (d->vgic_dist_base + KVM_VGIC_V3_DIST_SIZE <= rdreg->base)
return true;
- if (d->vgic_redist_base + redist_size <= d->vgic_dist_base)
+
+ if (rdreg->base + redist_size <= d->vgic_dist_base)
return true;

return false;
@@ -446,12 +448,14 @@ int vgic_v3_map_resources(struct kvm *kvm)
{
int ret = 0;
struct vgic_dist *dist = &kvm->arch.vgic;
+ struct vgic_redist_region *rdreg =
+ list_first_entry(&dist->rd_regions,
+ struct vgic_redist_region, list);

if (vgic_ready(kvm))
goto out;

- if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base) ||
- IS_VGIC_ADDR_UNDEF(dist->vgic_redist_base)) {
+ if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base) || !rdreg) {
kvm_err("Need to set vgic distributor addresses first\n");
ret = -ENXIO;
goto out;
--
2.5.5


2018-04-27 14:21:25

by Eric Auger

[permalink] [raw]
Subject: Re: [PATCH v4 00/12] KVM: arm/arm64: Allow multiple GICv3 redistributor regions

Hi,

On 04/27/2018 04:06 PM, Eric Auger wrote:
> At the moment the KVM VGICv3 only supports a single redistributor
> region (whose base address is set through the GICv3 kvm device
> KVM_DEV_ARM_VGIC_GRP_ADDR/KVM_VGIC_V3_ADDR_TYPE_REDIST). There,
> all the redistributors are laid out contiguously. The size of this
> single redistributor region is not set explicitly but instead
> induced at a late stage by the number of online vcpus.
>
> The GIC specification does not mandate all redistributors to be
> contiguous. Moreover DT and ACPI were specified so that multiple
> redistributors regions can be defined.
>
> The current interface brings a limitation on QEMU where ARM
> virt machine available GPA holes only allowed to assign a
> redistributor region fitting a max of 123 vcpus. Overcoming this
> limitation would force either to create a new machine or relocate
> the single rdist region or allow the allocation of multiple rdist
> regions.
>
> This series enables this last alternative. A new GICv3 KVM device
> KVM_DEV_ARM_VGIC_GRP_ADDR/KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION allows
> to register individual redistributor regions whose size is defined
> explicitly. Those rdist regions then are filled by vcpu rdist frames
> according to the need. The vgic init and related base address checks
> are impacted.

For some unknown reason I got a timeout on the git send-email command,
hence this isolated cover letter. The series was fully resent. Sorry for
the inconvenience.

Thanks

Eric
>
> Best Regards
>
> Eric
>
> Git: complete series available at
> https://github.com/eauger/linux/tree/v4.17-rc2-rdist-regions-v4
> Previous version:
> https://github.com/eauger/linux/tree/v4.16-rdist-regions-v3
>
> History:
>
> v3 -> v4:
> - Collected Christoffer's R-b
> - reworked vgic_v3_rdist_overlap and introduced vgic_dist_overlap
> - vgic_v3_insert_redist_region cleanup
> - inverse order for patches 6 and 7
>
> v2 -> v3:
> - Add details to the user API documentation
> - early exit if vgic_v3_rdist_region_from_index() fails
> - return -EINVAL if legacy and new redist region API are mixed
>
> v1 -> v2:
> - Rework the uapi. Only bits [51:16] of the redist region are
> exposed. Also a new flags field was introduced
> - Do not store the last bit in the vgic_cpu struct anymore
> - remove dist->spis check in 1st patch
> - add last patch to bump VGIC_V3_MAX_CPUS to 512
> - advertise the new attribute
>
> Eric Auger (12):
> KVM: arm/arm64: Set dist->spis to NULL after kfree
> KVM: arm/arm64: Document KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION
> KVM: arm/arm64: Replace the single rdist region by a list
> KVM: arm/arm64: Helper to locate free rdist index
> KVM: arm/arm64: Revisit Redistributor TYPER last bit computation
> KVM: arm/arm64: Adapt vgic_v3_check_base to multiple rdist regions
> KVM: arm/arm64: Helper to register a new redistributor region
> KVM: arm/arm64: Check vcpu redist base before registering an iodev
> KVM: arm/arm64: Check all vcpu redistributors are set on map_resources
> KVM: arm/arm64: Add KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION
> KVM: arm/arm64: Implement KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION
> KVM: arm/arm64: Bump VGIC_V3_MAX_CPUS to 512
>
> Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 25 ++++-
> arch/arm/include/uapi/asm/kvm.h | 1 +
> arch/arm64/include/uapi/asm/kvm.h | 1 +
> include/kvm/arm_vgic.h | 16 ++-
> virt/kvm/arm/vgic/vgic-init.c | 20 +++-
> virt/kvm/arm/vgic/vgic-kvm-device.c | 55 ++++++++++-
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 114 +++++++++++++++++++---
> virt/kvm/arm/vgic/vgic-v3.c | 101 +++++++++++++++----
> virt/kvm/arm/vgic/vgic.h | 42 +++++++-
> 9 files changed, 330 insertions(+), 45 deletions(-)
>

2018-04-27 19:17:30

by Christoffer Dall

[permalink] [raw]
Subject: Re: [PATCH v4 02/12] KVM: arm/arm64: Document KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION

On Fri, Apr 27, 2018 at 04:14:55PM +0200, Eric Auger wrote:
> We introduce a new KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION attribute in
> KVM_DEV_ARM_VGIC_GRP_ADDR group. It allows userspace to provide the
> base address and size of a redistributor region
>
> Compared to KVM_VGIC_V3_ADDR_TYPE_REDIST, this new attribute allows
> to declare several separate redistributor regions.
>
> So the whole redist space does not need to be contiguous anymore.
>
> Signed-off-by: Eric Auger <[email protected]>
> Reviewed-by: Peter Maydell <[email protected]>

Acked-by: Christoffer Dall <[email protected]>

>
> ---
> v3 -> v4:
> - Added Peter's R-b
> ---
> Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 25 ++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
> index 9293b45..cbc4328 100644
> --- a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
> +++ b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
> @@ -27,9 +27,32 @@ Groups:
> VCPU and all of the redistributor pages are contiguous.
> Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
> This address needs to be 64K aligned.
> +
> + KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION (rw, 64-bit)
> + The attr field of kvm_device_attr encodes 3 values:
> + bits: | 63 .... 52 | 51 .... 16 | 15 - 12 |11 - 0
> + values: | count | base | flags | index
> + - index encodes the unique redistributor region index
> + - flags: reserved for future use, currently 0
> + - base field encodes bits [51:16] of the guest physical base address
> + of the first redistributor in the region.
> + - count encodes the number of redistributors in the region. Must be
> + greater than 0.
> + There are two 64K pages for each redistributor in the region and
> + redistributors are laid out contiguously within the region. Regions
> + are filled with redistributors in the index order. The sum of all
> + region count fields must be greater than or equal to the number of
> + VCPUs. Redistributor regions must be registered in the incremental
> + index order, starting from index 0.
> + Only valid for KVM_DEV_TYPE_ARM_VGIC_V3.
> +
> + It is invalid to mix calls with KVM_VGIC_V3_ADDR_TYPE_REDIST and
> + KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION attributes.
> +
> Errors:
> -E2BIG: Address outside of addressable IPA range
> - -EINVAL: Incorrectly aligned address
> + -EINVAL: Incorrectly aligned address, bad redistributor region
> + count/index, mixed redistributor region attribute usage
> -EEXIST: Address already configured
> -ENXIO: The group or attribute is unknown/unsupported for this device
> or hardware support is missing.
> --
> 2.5.5
>
> _______________________________________________
> kvmarm mailing list
> [email protected]
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

2018-04-27 19:19:54

by Christoffer Dall

[permalink] [raw]
Subject: Re: [PATCH v4 11/12] KVM: arm/arm64: Implement KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION

Hi Eric,

On Fri, Apr 27, 2018 at 04:15:04PM +0200, Eric Auger wrote:
> Now all the internals are ready to handle multiple redistributor
> regions, let's allow the userspace to register them.
>
> Signed-off-by: Eric Auger <[email protected]>
>
> ---
> v3 -> v4:
> - vgic_v3_rdist_region_from_index is introduced in this patch
>
> v2 -> v3:
> - early exit if vgic_v3_rdist_region_from_index() fails
> ---
> virt/kvm/arm/vgic/vgic-kvm-device.c | 42 +++++++++++++++++++++++++++++++++++--
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 4 ++--
> virt/kvm/arm/vgic/vgic-v3.c | 14 +++++++++++++
> virt/kvm/arm/vgic/vgic.h | 13 +++++++++++-
> 4 files changed, 68 insertions(+), 5 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
> index e7b5a86..00e03d3 100644
> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> @@ -65,7 +65,8 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
> {
> int r = 0;
> struct vgic_dist *vgic = &kvm->arch.vgic;
> - phys_addr_t *addr_ptr, alignment;
> + phys_addr_t *addr_ptr = NULL;
> + phys_addr_t alignment;
> uint64_t undef_value = VGIC_ADDR_UNDEF;
>
> mutex_lock(&kvm->lock);
> @@ -92,7 +93,7 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
> if (r)
> break;
> if (write) {
> - r = vgic_v3_set_redist_base(kvm, *addr);
> + r = vgic_v3_set_redist_base(kvm, 0, *addr, 0);
> goto out;
> }
> rdreg = list_first_entry(&vgic->rd_regions,
> @@ -103,6 +104,42 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write)
> addr_ptr = &rdreg->base;
> break;
> }
> + case KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION:
> + {
> + struct vgic_redist_region *rdreg;
> + uint8_t index;
> +
> + r = vgic_check_type(kvm, KVM_DEV_TYPE_ARM_VGIC_V3);
> + if (r)
> + break;
> +
> + index = *addr & KVM_VGIC_V3_RDIST_INDEX_MASK;
> +
> + if (write) {
> + gpa_t base = *addr & KVM_VGIC_V3_RDIST_BASE_MASK;
> + uint32_t count = (*addr & KVM_VGIC_V3_RDIST_COUNT_MASK)
> + >> KVM_VGIC_V3_RDIST_COUNT_SHIFT;
> + uint8_t flags = (*addr & KVM_VGIC_V3_RDIST_FLAGS_MASK)
> + >> KVM_VGIC_V3_RDIST_FLAGS_SHIFT;
> +
> + if (!count || flags)
> + r = -EINVAL;
> + else
> + r = vgic_v3_set_redist_base(kvm, index,
> + base, count);
> + goto out;
> + }
> +
> + rdreg = vgic_v3_rdist_region_from_index(kvm, index);
> + if (!rdreg) {
> + r = -ENODEV;
> + goto out;
> + }
> +
> + *addr_ptr = rdreg->base & index &
> + (uint64_t)rdreg->count << KVM_VGIC_V3_RDIST_COUNT_SHIFT;

This still looks wrong, please see my reply to v3.

Thanks,
-Christoffer

2018-04-29 12:35:31

by Christoffer Dall

[permalink] [raw]
Subject: Re: [PATCH v4 06/12] KVM: arm/arm64: Adapt vgic_v3_check_base to multiple rdist regions

On Fri, Apr 27, 2018 at 04:14:59PM +0200, Eric Auger wrote:
> vgic_v3_check_base() currently only handles the case of a unique
> legacy redistributor region whose size is not explicitly set but
> infered, instead, from the number of online vcpus.

nit: inferred

>
> We adapt it to handle the case of multiple redistributor regions
> with explicitly defined size. We rely on two new helpers:
> - vgic_v3_rdist_overlap() is used to detect overlap with the dist
> region if defined
> - vgic_v3_rd_region_size computes the size of the redist region,
> would it be a legacy unique region or a new explicitly sized
> region.
>
> Signed-off-by: Eric Auger <[email protected]>
>
> ---
>
> v3 -> v4:
> - squash vgic_v3_check_base adaptation and vgic_v3_rdist_overlap
> + vgic_v3_rd_region_size introduction and put this patch
> before v3 patch 6
> ---
> virt/kvm/arm/vgic/vgic-v3.c | 49 +++++++++++++++++++++++++++++----------------
> virt/kvm/arm/vgic/vgic.h | 10 +++++++++
> 2 files changed, 42 insertions(+), 17 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
> index f81a63a..c4a2a46 100644
> --- a/virt/kvm/arm/vgic/vgic-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-v3.c
> @@ -410,6 +410,29 @@ int vgic_v3_save_pending_tables(struct kvm *kvm)
> return 0;
> }
>
> +/**
> + * vgic_v3_rdist_overlap - check if a region overlaps with any
> + * existing redistributor region
> + *
> + * @kvm: kvm handle
> + * @base: base of the region
> + * @size: size of region
> + *
> + * Return: true if there is an overlap
> + */
> +bool vgic_v3_rdist_overlap(struct kvm *kvm, gpa_t base, size_t size)
> +{
> + struct vgic_dist *d = &kvm->arch.vgic;
> + struct vgic_redist_region *rdreg;
> +
> + list_for_each_entry(rdreg, &d->rd_regions, list) {
> + if ((base + size > rdreg->base) &&
> + (base < rdreg->base + vgic_v3_rd_region_size(kvm, rdreg)))
> + return true;
> + }
> + return false;
> +}
> +
> /*
> * Check for overlapping regions and for regions crossing the end of memory
> * for base addresses which have already been set.
> @@ -417,31 +440,23 @@ int vgic_v3_save_pending_tables(struct kvm *kvm)
> bool vgic_v3_check_base(struct kvm *kvm)
> {
> struct vgic_dist *d = &kvm->arch.vgic;
> - gpa_t redist_size = KVM_VGIC_V3_REDIST_SIZE;
> - struct vgic_redist_region *rdreg =
> - list_first_entry(&d->rd_regions,
> - struct vgic_redist_region, list);
> -
> - redist_size *= atomic_read(&kvm->online_vcpus);
> + struct vgic_redist_region *rdreg;
>
> if (!IS_VGIC_ADDR_UNDEF(d->vgic_dist_base) &&
> d->vgic_dist_base + KVM_VGIC_V3_DIST_SIZE < d->vgic_dist_base)
> return false;
>
> - if (rdreg && (rdreg->base + redist_size < rdreg->base))
> - return false;
> + list_for_each_entry(rdreg, &d->rd_regions, list) {
> + if (rdreg->base + vgic_v3_rd_region_size(kvm, rdreg) <
> + rdreg->base)
> + return false;
> + }
>
> - /* Both base addresses must be set to check if they overlap */
> - if (IS_VGIC_ADDR_UNDEF(d->vgic_dist_base) || !rdreg)
> + if (IS_VGIC_ADDR_UNDEF(d->vgic_dist_base))
> return true;
>
> - if (d->vgic_dist_base + KVM_VGIC_V3_DIST_SIZE <= rdreg->base)
> - return true;
> -
> - if (rdreg->base + redist_size <= d->vgic_dist_base)
> - return true;
> -
> - return false;
> + return !vgic_v3_rdist_overlap(kvm, d->vgic_dist_base,
> + KVM_VGIC_V3_DIST_SIZE);
> }
>
> /**
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index fea32cb..e6e3ae9 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -262,6 +262,16 @@ vgic_v3_redist_region_full(struct vgic_redist_region *region)
>
> struct vgic_redist_region *vgic_v3_rdist_free_slot(struct list_head *rdregs);
>
> +static inline size_t
> +vgic_v3_rd_region_size(struct kvm *kvm, struct vgic_redist_region *rdreg)
> +{
> + if (!rdreg->count)
> + return atomic_read(&kvm->online_vcpus) * KVM_VGIC_V3_REDIST_SIZE;
> + else
> + return rdreg->count * KVM_VGIC_V3_REDIST_SIZE;
> +}
> +bool vgic_v3_rdist_overlap(struct kvm *kvm, gpa_t base, size_t size);
> +
> int vgic_its_resolve_lpi(struct kvm *kvm, struct vgic_its *its,
> u32 devid, u32 eventid, struct vgic_irq **irq);
> struct vgic_its *vgic_msi_to_its(struct kvm *kvm, struct kvm_msi *msi);
> --
> 2.5.5
>

Reviewed-by: Christoffer Dall <[email protected]>

2018-04-29 12:36:02

by Christoffer Dall

[permalink] [raw]
Subject: Re: [PATCH v4 08/12] KVM: arm/arm64: Check vcpu redist base before registering an iodev

On Fri, Apr 27, 2018 at 04:15:01PM +0200, Eric Auger wrote:
> As we are going to register several redist regions,
> vgic_register_all_redist_iodevs() may be called several times. We need
> to register a redist_iodev for a given vcpu only once. So let's
> check if the base address has already been set. Initialize this latter
> in kvm_vgic_vcpu_early_init().
>
> Signed-off-by: Eric Auger <[email protected]>

Acked-by: Christoffer Dall <[email protected]>

> ---
> virt/kvm/arm/vgic/vgic-init.c | 3 +++
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 3 +++
> 2 files changed, 6 insertions(+)
>
> diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
> index 6456371..7e040e7 100644
> --- a/virt/kvm/arm/vgic/vgic-init.c
> +++ b/virt/kvm/arm/vgic/vgic-init.c
> @@ -82,6 +82,9 @@ void kvm_vgic_vcpu_early_init(struct kvm_vcpu *vcpu)
> INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
> spin_lock_init(&vgic_cpu->ap_list_lock);
>
> + vgic_cpu->rd_iodev.base_addr = VGIC_ADDR_UNDEF;
> + vgic_cpu->sgi_iodev.base_addr = VGIC_ADDR_UNDEF;
> +
> /*
> * Enable and configure all SGIs to be edge-triggered and
> * configure all PPIs as level-triggered.
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index cbf8f4e..1b1e07f 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -592,6 +592,9 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu)
> gpa_t rd_base, sgi_base;
> int ret;
>
> + if (!IS_VGIC_ADDR_UNDEF(vgic_cpu->rd_iodev.base_addr))
> + return 0;
> +
> /*
> * We may be creating VCPUs before having set the base address for the
> * redistributor region, in which case we will come back to this
> --
> 2.5.5
>
> _______________________________________________
> kvmarm mailing list
> [email protected]
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

2018-04-29 12:36:34

by Christoffer Dall

[permalink] [raw]
Subject: Re: [PATCH v4 09/12] KVM: arm/arm64: Check all vcpu redistributors are set on map_resources

On Fri, Apr 27, 2018 at 04:15:02PM +0200, Eric Auger wrote:
> On vcpu first run, we eventually know the actual number of vcpus.
> This is a synchronization point to check all redistributors
> were assigned. On kvm_vgic_map_resources() we check both dist and
> redist were set, eventually check potential base address inconsistencies.
>
> Signed-off-by: Eric Auger <[email protected]>

Reviewed-by: Christoffer Dall <[email protected]>

>
> ---
>
> v3 -> v4:
> - use kvm_debug
> ---
> virt/kvm/arm/vgic/vgic-v3.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
> index c4a2a46..5563671 100644
> --- a/virt/kvm/arm/vgic/vgic-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-v3.c
> @@ -484,16 +484,25 @@ struct vgic_redist_region *vgic_v3_rdist_free_slot(struct list_head *rd_regions)
>
> int vgic_v3_map_resources(struct kvm *kvm)
> {
> - int ret = 0;
> struct vgic_dist *dist = &kvm->arch.vgic;
> - struct vgic_redist_region *rdreg =
> - list_first_entry(&dist->rd_regions,
> - struct vgic_redist_region, list);
> + struct kvm_vcpu *vcpu;
> + int ret = 0;
> + int c;
>
> if (vgic_ready(kvm))
> goto out;
>
> - if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base) || !rdreg) {
> + kvm_for_each_vcpu(c, vcpu, kvm) {
> + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> +
> + if (IS_VGIC_ADDR_UNDEF(vgic_cpu->rd_iodev.base_addr)) {
> + kvm_debug("vcpu %d redistributor base not set\n", c);
> + ret = -ENXIO;
> + goto out;
> + }
> + }
> +
> + if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base)) {
> kvm_err("Need to set vgic distributor addresses first\n");
> ret = -ENXIO;
> goto out;
> --
> 2.5.5
>
> _______________________________________________
> kvmarm mailing list
> [email protected]
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

2018-04-29 12:36:48

by Christoffer Dall

[permalink] [raw]
Subject: Re: [PATCH v4 12/12] KVM: arm/arm64: Bump VGIC_V3_MAX_CPUS to 512

On Fri, Apr 27, 2018 at 04:15:05PM +0200, Eric Auger wrote:
> Let's raise the number of supported vcpus along with
> vgic v3 now that HW is looming with more physical CPUs.
>
> Signed-off-by: Eric Auger <[email protected]>

Acked-by: Christoffer Dall <[email protected]>

> ---
> include/kvm/arm_vgic.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index e5c16d1..a9a9f4a 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -28,7 +28,7 @@
>
> #include <linux/irqchip/arm-gic-v4.h>
>
> -#define VGIC_V3_MAX_CPUS 255
> +#define VGIC_V3_MAX_CPUS 512
> #define VGIC_V2_MAX_CPUS 8
> #define VGIC_NR_IRQS_LEGACY 256
> #define VGIC_NR_SGIS 16
> --
> 2.5.5
>
> _______________________________________________
> kvmarm mailing list
> [email protected]
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm