From d9b201e99c616001b4a51627820377b293479384 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 5 Apr 2021 18:39:33 +0200 Subject: KVM: arm64: vgic-v3: Fix some error codes when setting RDIST base KVM_DEV_ARM_VGIC_GRP_ADDR group doc says we should return -EEXIST in case the base address of the redist is already set. We currently return -EINVAL. However we need to return -EINVAL in case a legacy REDIST address is attempted to be set while REDIST_REGIONS were set. This case is discriminated by looking at the count field. Signed-off-by: Eric Auger Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20210405163941.510258-2-eric.auger@redhat.com --- arch/arm64/kvm/vgic/vgic-mmio-v3.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'arch/arm64/kvm') diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c index 15a6c98ee92f..ef9baf0d01b5 100644 --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c @@ -791,10 +791,6 @@ static int vgic_v3_insert_redist_region(struct kvm *kvm, uint32_t index, size_t size = count * KVM_VGIC_V3_REDIST_SIZE; int ret; - /* single rdist region already set ?*/ - if (!count && !list_empty(rd_regions)) - return -EINVAL; - /* cross the end of memory ? */ if (base + size < base) return -EINVAL; @@ -805,11 +801,15 @@ static int vgic_v3_insert_redist_region(struct kvm *kvm, uint32_t index, } else { rdreg = list_last_entry(rd_regions, struct vgic_redist_region, list); - if (index != rdreg->index + 1) + + /* Don't mix single region and discrete redist regions */ + if (!count && rdreg->count) return -EINVAL; - /* Cannot add an explicitly sized regions after legacy region */ - if (!rdreg->count) + if (!count) + return -EEXIST; + + if (index != rdreg->index + 1) return -EINVAL; } -- cgit v1.2.3 From 53b16dd6ba5cf64ed147ac3523ec34651d553cb0 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 5 Apr 2021 18:39:34 +0200 Subject: KVM: arm64: Fix KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION read The doc says: "The characteristics of a specific redistributor region can be read by presetting the index field in the attr data. Only valid for KVM_DEV_TYPE_ARM_VGIC_V3" Unfortunately the existing code fails to read the input attr data. Fixes: 04c110932225 ("KVM: arm/arm64: Implement KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION") Cc: stable@vger.kernel.org#v4.17+ Signed-off-by: Eric Auger Reviewed-by: Alexandru Elisei Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20210405163941.510258-3-eric.auger@redhat.com --- arch/arm64/kvm/vgic/vgic-kvm-device.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'arch/arm64/kvm') diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c index 44419679f91a..2f66cf247282 100644 --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c @@ -226,6 +226,9 @@ static int vgic_get_common_attr(struct kvm_device *dev, u64 addr; unsigned long type = (unsigned long)attr->attr; + if (copy_from_user(&addr, uaddr, sizeof(addr))) + return -EFAULT; + r = kvm_vgic_addr(dev->kvm, type, &addr, false); if (r) return (r == -ENODEV) ? -ENXIO : r; -- cgit v1.2.3 From 8542a8f95a67ff6b76d6868ec0af58c464bdf041 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 5 Apr 2021 18:39:35 +0200 Subject: KVM: arm64: vgic-v3: Fix error handling in vgic_v3_set_redist_base() vgic_v3_insert_redist_region() may succeed while vgic_register_all_redist_iodevs fails. For example this happens while adding a redistributor region overlapping a dist region. The failure only is detected on vgic_register_all_redist_iodevs when vgic_v3_check_base() gets called in vgic_register_redist_iodev(). In such a case, remove the newly added redistributor region and free it. Signed-off-by: Eric Auger Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20210405163941.510258-4-eric.auger@redhat.com --- arch/arm64/kvm/vgic/vgic-mmio-v3.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'arch/arm64/kvm') diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c index ef9baf0d01b5..fec0555529c0 100644 --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c @@ -861,8 +861,14 @@ int vgic_v3_set_redist_base(struct kvm *kvm, u32 index, u64 addr, u32 count) * afterwards will register the iodevs when needed. */ ret = vgic_register_all_redist_iodevs(kvm); - if (ret) + if (ret) { + struct vgic_redist_region *rdreg; + + rdreg = vgic_v3_rdist_region_from_index(kvm, index); + list_del(&rdreg->list); + kfree(rdreg); return ret; + } return 0; } -- cgit v1.2.3 From 3a5211612764fa3948e5db5254734168e9e763de Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 5 Apr 2021 18:39:36 +0200 Subject: KVM: arm/arm64: vgic: Reset base address on kvm_vgic_dist_destroy() On vgic_dist_destroy(), the addresses are not reset. However for kvm selftest purpose this would allow to continue the test execution even after a failure when running KVM_RUN. So let's reset the base addresses. Signed-off-by: Eric Auger Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20210405163941.510258-5-eric.auger@redhat.com --- arch/arm64/kvm/vgic/vgic-init.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'arch/arm64/kvm') diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c index 052917deb149..cf6faa0aeddb 100644 --- a/arch/arm64/kvm/vgic/vgic-init.c +++ b/arch/arm64/kvm/vgic/vgic-init.c @@ -335,13 +335,16 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm) kfree(dist->spis); dist->spis = NULL; dist->nr_spis = 0; + dist->vgic_dist_base = VGIC_ADDR_UNDEF; - if (kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) { + if (dist->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); + } else { + dist->vgic_cpu_base = VGIC_ADDR_UNDEF; } if (vgic_has_its(kvm)) @@ -362,6 +365,7 @@ void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu) vgic_flush_pending_lpis(vcpu); INIT_LIST_HEAD(&vgic_cpu->ap_list_head); + vgic_cpu->rd_iodev.base_addr = VGIC_ADDR_UNDEF; } /* To be called with kvm->lock held */ -- cgit v1.2.3 From da3853097679022e14a2d125983f11a67fd2f96a Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 5 Apr 2021 18:39:38 +0200 Subject: KVM: arm64: Simplify argument passing to vgic_uaccess_[read|write] vgic_uaccess() takes a struct vgic_io_device argument, converts it to a struct kvm_io_device and passes it to the read/write accessor functions, which convert it back to a struct vgic_io_device. Avoid the indirection by passing the struct vgic_io_device argument directly to vgic_uaccess_{read,write}. Signed-off-by: Eric Auger Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20210405163941.510258-7-eric.auger@redhat.com --- arch/arm64/kvm/vgic/vgic-mmio.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'arch/arm64/kvm') diff --git a/arch/arm64/kvm/vgic/vgic-mmio.c b/arch/arm64/kvm/vgic/vgic-mmio.c index b2d73fc0d1ef..48c6067fc5ec 100644 --- a/arch/arm64/kvm/vgic/vgic-mmio.c +++ b/arch/arm64/kvm/vgic/vgic-mmio.c @@ -938,10 +938,9 @@ vgic_get_mmio_region(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev, return region; } -static int vgic_uaccess_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev, +static int vgic_uaccess_read(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev, gpa_t addr, u32 *val) { - struct vgic_io_device *iodev = kvm_to_vgic_iodev(dev); const struct vgic_register_region *region; struct kvm_vcpu *r_vcpu; @@ -960,10 +959,9 @@ static int vgic_uaccess_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev, return 0; } -static int vgic_uaccess_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev, +static int vgic_uaccess_write(struct kvm_vcpu *vcpu, struct vgic_io_device *iodev, gpa_t addr, const u32 *val) { - struct vgic_io_device *iodev = kvm_to_vgic_iodev(dev); const struct vgic_register_region *region; struct kvm_vcpu *r_vcpu; @@ -986,9 +984,9 @@ int vgic_uaccess(struct kvm_vcpu *vcpu, struct vgic_io_device *dev, bool is_write, int offset, u32 *val) { if (is_write) - return vgic_uaccess_write(vcpu, &dev->dev, offset, val); + return vgic_uaccess_write(vcpu, dev, offset, val); else - return vgic_uaccess_read(vcpu, &dev->dev, offset, val); + return vgic_uaccess_read(vcpu, dev, offset, val); } static int dispatch_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev, -- cgit v1.2.3 From e5a35635464bc5304674b84ea42615a3fd0bd949 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 5 Apr 2021 18:39:39 +0200 Subject: kvm: arm64: vgic-v3: Introduce vgic_v3_free_redist_region() To improve the readability, we introduce the new vgic_v3_free_redist_region helper and also rename vgic_v3_insert_redist_region into vgic_v3_alloc_redist_region Signed-off-by: Eric Auger Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20210405163941.510258-8-eric.auger@redhat.com --- arch/arm64/kvm/vgic/vgic-init.c | 6 ++---- arch/arm64/kvm/vgic/vgic-mmio-v3.c | 17 +++++++++++------ arch/arm64/kvm/vgic/vgic.h | 1 + 3 files changed, 14 insertions(+), 10 deletions(-) (limited to 'arch/arm64/kvm') diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c index cf6faa0aeddb..58cbda00e56d 100644 --- a/arch/arm64/kvm/vgic/vgic-init.c +++ b/arch/arm64/kvm/vgic/vgic-init.c @@ -338,10 +338,8 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm) dist->vgic_dist_base = VGIC_ADDR_UNDEF; if (dist->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); - } + list_for_each_entry_safe(rdreg, next, &dist->rd_regions, list) + vgic_v3_free_redist_region(rdreg); INIT_LIST_HEAD(&dist->rd_regions); } else { dist->vgic_cpu_base = VGIC_ADDR_UNDEF; diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c index fec0555529c0..e1ed0c5a8eaa 100644 --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c @@ -768,7 +768,7 @@ static int vgic_register_all_redist_iodevs(struct kvm *kvm) } /** - * vgic_v3_insert_redist_region - Insert a new redistributor region + * vgic_v3_alloc_redist_region - Allocate 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 @@ -782,8 +782,8 @@ static int vgic_register_all_redist_iodevs(struct kvm *kvm) * * 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) +static int vgic_v3_alloc_redist_region(struct kvm *kvm, uint32_t index, + gpa_t base, uint32_t count) { struct vgic_dist *d = &kvm->arch.vgic; struct vgic_redist_region *rdreg; @@ -848,11 +848,17 @@ free: return ret; } +void vgic_v3_free_redist_region(struct vgic_redist_region *rdreg) +{ + list_del(&rdreg->list); + kfree(rdreg); +} + int vgic_v3_set_redist_base(struct kvm *kvm, u32 index, u64 addr, u32 count) { int ret; - ret = vgic_v3_insert_redist_region(kvm, index, addr, count); + ret = vgic_v3_alloc_redist_region(kvm, index, addr, count); if (ret) return ret; @@ -865,8 +871,7 @@ int vgic_v3_set_redist_base(struct kvm *kvm, u32 index, u64 addr, u32 count) struct vgic_redist_region *rdreg; rdreg = vgic_v3_rdist_region_from_index(kvm, index); - list_del(&rdreg->list); - kfree(rdreg); + vgic_v3_free_redist_region(rdreg); return ret; } diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h index 64fcd7511110..bc418c2c1214 100644 --- a/arch/arm64/kvm/vgic/vgic.h +++ b/arch/arm64/kvm/vgic/vgic.h @@ -293,6 +293,7 @@ vgic_v3_rd_region_size(struct kvm *kvm, struct vgic_redist_region *rdreg) struct vgic_redist_region *vgic_v3_rdist_region_from_index(struct kvm *kvm, u32 index); +void vgic_v3_free_redist_region(struct vgic_redist_region *rdreg); bool vgic_v3_rdist_overlap(struct kvm *kvm, gpa_t base, size_t size); -- cgit v1.2.3 From 28e9d4bce3be9b8fec6c854f87923db99c8fb874 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 5 Apr 2021 18:39:40 +0200 Subject: KVM: arm64: vgic-v3: Expose GICR_TYPER.Last for userspace Commit 23bde34771f1 ("KVM: arm64: vgic-v3: Drop the reporting of GICR_TYPER.Last for userspace") temporarily fixed a bug identified when attempting to access the GICR_TYPER register before the redistributor region setting, but dropped the support of the LAST bit. Emulating the GICR_TYPER.Last bit still makes sense for architecture compliance though. This patch restores its support (if the redistributor region was set) while keeping the code safe. We introduce a new helper, vgic_mmio_vcpu_rdist_is_last() which computes whether a redistributor is the highest one of a series of redistributor contributor pages. With this new implementation we do not need to have a uaccess read accessor anymore. Signed-off-by: Eric Auger Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20210405163941.510258-9-eric.auger@redhat.com --- arch/arm64/kvm/vgic/vgic-mmio-v3.c | 46 ++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 19 deletions(-) (limited to 'arch/arm64/kvm') diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c index e1ed0c5a8eaa..03a253785700 100644 --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c @@ -251,30 +251,35 @@ static void vgic_mmio_write_v3r_ctlr(struct kvm_vcpu *vcpu, vgic_enable_lpis(vcpu); } -static unsigned long vgic_mmio_read_v3r_typer(struct kvm_vcpu *vcpu, - gpa_t addr, unsigned int len) +static bool vgic_mmio_vcpu_rdist_is_last(struct kvm_vcpu *vcpu) { - unsigned long mpidr = kvm_vcpu_get_mpidr_aff(vcpu); + struct vgic_dist *vgic = &vcpu->kvm->arch.vgic; 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; + struct vgic_redist_region *iter, *rdreg = vgic_cpu->rdreg; - value = (u64)(mpidr & GENMASK(23, 0)) << 32; - value |= ((target_vcpu_id & 0xffff) << 8); + if (!rdreg) + return false; - if (addr == last_rdist_typer) - value |= GICR_TYPER_LAST; - if (vgic_has_its(vcpu->kvm)) - value |= GICR_TYPER_PLPIS; + if (vgic_cpu->rdreg_index < rdreg->free_index - 1) { + return false; + } else if (rdreg->count && vgic_cpu->rdreg_index == (rdreg->count - 1)) { + struct list_head *rd_regions = &vgic->rd_regions; + gpa_t end = rdreg->base + rdreg->count * KVM_VGIC_V3_REDIST_SIZE; - return extract_bytes(value, addr & 7, len); + /* + * the rdist is the last one of the redist region, + * check whether there is no other contiguous rdist region + */ + list_for_each_entry(iter, rd_regions, list) { + if (iter->base == end && iter->free_index > 0) + return false; + } + } + return true; } -static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, - gpa_t addr, unsigned int len) +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); int target_vcpu_id = vcpu->vcpu_id; @@ -286,7 +291,9 @@ static unsigned long vgic_uaccess_read_v3r_typer(struct kvm_vcpu *vcpu, if (vgic_has_its(vcpu->kvm)) value |= GICR_TYPER_PLPIS; - /* reporting of the Last bit is not supported for userspace */ + if (vgic_mmio_vcpu_rdist_is_last(vcpu)) + value |= GICR_TYPER_LAST; + return extract_bytes(value, addr & 7, len); } @@ -612,7 +619,7 @@ static const struct vgic_register_region vgic_v3_rd_registers[] = { VGIC_ACCESS_32bit), REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_TYPER, vgic_mmio_read_v3r_typer, vgic_mmio_write_wi, - vgic_uaccess_read_v3r_typer, vgic_mmio_uaccess_write_wi, 8, + NULL, vgic_mmio_uaccess_write_wi, 8, VGIC_ACCESS_64bit | VGIC_ACCESS_32bit), REGISTER_DESC_WITH_LENGTH(GICR_WAKER, vgic_mmio_read_raz, vgic_mmio_write_wi, 4, @@ -714,6 +721,7 @@ int vgic_register_redist_iodev(struct kvm_vcpu *vcpu) return -EINVAL; vgic_cpu->rdreg = rdreg; + vgic_cpu->rdreg_index = rdreg->free_index; rd_base = rdreg->base + rdreg->free_index * KVM_VGIC_V3_REDIST_SIZE; -- cgit v1.2.3 From 94ac0835391efc1a30feda6fc908913ec012951e Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 12 Apr 2021 17:00:34 +0200 Subject: KVM: arm/arm64: Fix KVM_VGIC_V3_ADDR_TYPE_REDIST read When reading the base address of the a REDIST region through KVM_VGIC_V3_ADDR_TYPE_REDIST we expect the redistributor region list to be populated with a single element. However list_first_entry() expects the list to be non empty. Instead we should use list_first_entry_or_null which effectively returns NULL if the list is empty. Fixes: dbd9733ab674 ("KVM: arm/arm64: Replace the single rdist region by a list") Cc: # v4.18+ Signed-off-by: Eric Auger Reported-by: Gavin Shan Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20210412150034.29185-1-eric.auger@redhat.com --- arch/arm64/kvm/vgic/vgic-kvm-device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'arch/arm64/kvm') diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c index 2f66cf247282..7740995de982 100644 --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c @@ -87,8 +87,8 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write) r = vgic_v3_set_redist_base(kvm, 0, *addr, 0); goto out; } - rdreg = list_first_entry(&vgic->rd_regions, - struct vgic_redist_region, list); + rdreg = list_first_entry_or_null(&vgic->rd_regions, + struct vgic_redist_region, list); if (!rdreg) addr_ptr = &undef_value; else -- cgit v1.2.3