From: Christoffer Dall <christoffer.dall@arm.com>
To: kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org
Cc: kvm@vger.kernel.org, Marc Zyngier <marc.zyngier@arm.com>,
Andre Przywara <andre.przywara@arm.com>
Subject: [PATCH v4 06/10] KVM: arm/arm64: vgic: Permit uaccess writes to return errors
Date: Mon, 16 Jul 2018 15:06:23 +0200 [thread overview]
Message-ID: <1531746387-7033-7-git-send-email-christoffer.dall@arm.com> (raw)
In-Reply-To: <1531746387-7033-1-git-send-email-christoffer.dall@arm.com>
Currently we do not allow any vgic mmio write operations to fail, which
makes sense from mmio traps from the guest. However, we should be able
to report failures to userspace, if userspace writes incompatible values
to read-only registers. Rework the internal interface to allow errors
to be returned on the write side for userspace writes.
Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
---
virt/kvm/arm/vgic/vgic-mmio-v3.c | 12 +++++++-----
virt/kvm/arm/vgic/vgic-mmio.c | 18 +++++++++++++-----
virt/kvm/arm/vgic/vgic-mmio.h | 19 +++++++++++--------
3 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index ebe10a0..ef57a1a 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -249,9 +249,9 @@ static unsigned long vgic_v3_uaccess_read_pending(struct kvm_vcpu *vcpu,
return value;
}
-static void vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,
- gpa_t addr, unsigned int len,
- unsigned long val)
+static int vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,
+ gpa_t addr, unsigned int len,
+ unsigned long val)
{
u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
int i;
@@ -276,6 +276,8 @@ static void vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,
vgic_put_irq(vcpu->kvm, irq);
}
+
+ return 0;
}
/* We want to avoid outer shareable. */
@@ -468,7 +470,7 @@ static const struct vgic_register_region vgic_v3_dist_registers[] = {
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICPENDR,
vgic_mmio_read_pending, vgic_mmio_write_cpending,
- vgic_mmio_read_raz, vgic_mmio_write_wi, 1,
+ vgic_mmio_read_raz, vgic_mmio_uaccess_write_wi, 1,
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISACTIVER,
vgic_mmio_read_active, vgic_mmio_write_sactive,
@@ -541,7 +543,7 @@ static const struct vgic_register_region vgic_v3_sgibase_registers[] = {
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ICPENDR0,
vgic_mmio_read_pending, vgic_mmio_write_cpending,
- vgic_mmio_read_raz, vgic_mmio_write_wi, 4,
+ vgic_mmio_read_raz, vgic_mmio_uaccess_write_wi, 4,
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ISACTIVER0,
vgic_mmio_read_active, vgic_mmio_write_sactive,
diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
index ff9655c..e1e7998 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.c
+++ b/virt/kvm/arm/vgic/vgic-mmio.c
@@ -40,6 +40,13 @@ void vgic_mmio_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
/* Ignore */
}
+int vgic_mmio_uaccess_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
+ unsigned int len, unsigned long val)
+{
+ /* Ignore */
+ return 0;
+}
+
/*
* Read accesses to both GICD_ICENABLER and GICD_ISENABLER return the value
* of the enabled bit, so there is only one function for both here.
@@ -363,11 +370,12 @@ void vgic_mmio_write_cactive(struct kvm_vcpu *vcpu,
mutex_unlock(&vcpu->kvm->lock);
}
-void vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
+int vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
{
__vgic_mmio_write_cactive(vcpu, addr, len, val);
+ return 0;
}
static void __vgic_mmio_write_sactive(struct kvm_vcpu *vcpu,
@@ -399,11 +407,12 @@ void vgic_mmio_write_sactive(struct kvm_vcpu *vcpu,
mutex_unlock(&vcpu->kvm->lock);
}
-void vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
+int vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
{
__vgic_mmio_write_sactive(vcpu, addr, len, val);
+ return 0;
}
unsigned long vgic_mmio_read_priority(struct kvm_vcpu *vcpu,
@@ -735,10 +744,9 @@ static int vgic_uaccess_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
r_vcpu = iodev->redist_vcpu ? iodev->redist_vcpu : vcpu;
if (region->uaccess_write)
- region->uaccess_write(r_vcpu, addr, sizeof(u32), *val);
- else
- region->write(r_vcpu, addr, sizeof(u32), *val);
+ return region->uaccess_write(r_vcpu, addr, sizeof(u32), *val);
+ region->write(r_vcpu, addr, sizeof(u32), *val);
return 0;
}
diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
index 5693f6df..9c3d6d3 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.h
+++ b/virt/kvm/arm/vgic/vgic-mmio.h
@@ -37,8 +37,8 @@ struct vgic_register_region {
unsigned long (*uaccess_read)(struct kvm_vcpu *vcpu, gpa_t addr,
unsigned int len);
union {
- void (*uaccess_write)(struct kvm_vcpu *vcpu, gpa_t addr,
- unsigned int len, unsigned long val);
+ int (*uaccess_write)(struct kvm_vcpu *vcpu, gpa_t addr,
+ unsigned int len, unsigned long val);
int (*uaccess_its_write)(struct kvm *kvm, struct vgic_its *its,
gpa_t addr, unsigned int len,
unsigned long val);
@@ -134,6 +134,9 @@ unsigned long vgic_mmio_read_rao(struct kvm_vcpu *vcpu,
void vgic_mmio_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
unsigned int len, unsigned long val);
+int vgic_mmio_uaccess_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
+ unsigned int len, unsigned long val);
+
unsigned long vgic_mmio_read_enable(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len);
@@ -167,13 +170,13 @@ void vgic_mmio_write_sactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val);
-void vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
- gpa_t addr, unsigned int len,
- unsigned long val);
+int vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
+ gpa_t addr, unsigned int len,
+ unsigned long val);
-void vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
- gpa_t addr, unsigned int len,
- unsigned long val);
+int vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
+ gpa_t addr, unsigned int len,
+ unsigned long val);
unsigned long vgic_mmio_read_priority(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len);
--
2.7.4
WARNING: multiple messages have this Message-ID (diff)
From: christoffer.dall@arm.com (Christoffer Dall)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 06/10] KVM: arm/arm64: vgic: Permit uaccess writes to return errors
Date: Mon, 16 Jul 2018 15:06:23 +0200 [thread overview]
Message-ID: <1531746387-7033-7-git-send-email-christoffer.dall@arm.com> (raw)
In-Reply-To: <1531746387-7033-1-git-send-email-christoffer.dall@arm.com>
Currently we do not allow any vgic mmio write operations to fail, which
makes sense from mmio traps from the guest. However, we should be able
to report failures to userspace, if userspace writes incompatible values
to read-only registers. Rework the internal interface to allow errors
to be returned on the write side for userspace writes.
Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
---
virt/kvm/arm/vgic/vgic-mmio-v3.c | 12 +++++++-----
virt/kvm/arm/vgic/vgic-mmio.c | 18 +++++++++++++-----
virt/kvm/arm/vgic/vgic-mmio.h | 19 +++++++++++--------
3 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index ebe10a0..ef57a1a 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -249,9 +249,9 @@ static unsigned long vgic_v3_uaccess_read_pending(struct kvm_vcpu *vcpu,
return value;
}
-static void vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,
- gpa_t addr, unsigned int len,
- unsigned long val)
+static int vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,
+ gpa_t addr, unsigned int len,
+ unsigned long val)
{
u32 intid = VGIC_ADDR_TO_INTID(addr, 1);
int i;
@@ -276,6 +276,8 @@ static void vgic_v3_uaccess_write_pending(struct kvm_vcpu *vcpu,
vgic_put_irq(vcpu->kvm, irq);
}
+
+ return 0;
}
/* We want to avoid outer shareable. */
@@ -468,7 +470,7 @@ static const struct vgic_register_region vgic_v3_dist_registers[] = {
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ICPENDR,
vgic_mmio_read_pending, vgic_mmio_write_cpending,
- vgic_mmio_read_raz, vgic_mmio_write_wi, 1,
+ vgic_mmio_read_raz, vgic_mmio_uaccess_write_wi, 1,
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_BITS_PER_IRQ_SHARED(GICD_ISACTIVER,
vgic_mmio_read_active, vgic_mmio_write_sactive,
@@ -541,7 +543,7 @@ static const struct vgic_register_region vgic_v3_sgibase_registers[] = {
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ICPENDR0,
vgic_mmio_read_pending, vgic_mmio_write_cpending,
- vgic_mmio_read_raz, vgic_mmio_write_wi, 4,
+ vgic_mmio_read_raz, vgic_mmio_uaccess_write_wi, 4,
VGIC_ACCESS_32bit),
REGISTER_DESC_WITH_LENGTH_UACCESS(GICR_ISACTIVER0,
vgic_mmio_read_active, vgic_mmio_write_sactive,
diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
index ff9655c..e1e7998 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.c
+++ b/virt/kvm/arm/vgic/vgic-mmio.c
@@ -40,6 +40,13 @@ void vgic_mmio_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
/* Ignore */
}
+int vgic_mmio_uaccess_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
+ unsigned int len, unsigned long val)
+{
+ /* Ignore */
+ return 0;
+}
+
/*
* Read accesses to both GICD_ICENABLER and GICD_ISENABLER return the value
* of the enabled bit, so there is only one function for both here.
@@ -363,11 +370,12 @@ void vgic_mmio_write_cactive(struct kvm_vcpu *vcpu,
mutex_unlock(&vcpu->kvm->lock);
}
-void vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
+int vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
{
__vgic_mmio_write_cactive(vcpu, addr, len, val);
+ return 0;
}
static void __vgic_mmio_write_sactive(struct kvm_vcpu *vcpu,
@@ -399,11 +407,12 @@ void vgic_mmio_write_sactive(struct kvm_vcpu *vcpu,
mutex_unlock(&vcpu->kvm->lock);
}
-void vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
+int vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val)
{
__vgic_mmio_write_sactive(vcpu, addr, len, val);
+ return 0;
}
unsigned long vgic_mmio_read_priority(struct kvm_vcpu *vcpu,
@@ -735,10 +744,9 @@ static int vgic_uaccess_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
r_vcpu = iodev->redist_vcpu ? iodev->redist_vcpu : vcpu;
if (region->uaccess_write)
- region->uaccess_write(r_vcpu, addr, sizeof(u32), *val);
- else
- region->write(r_vcpu, addr, sizeof(u32), *val);
+ return region->uaccess_write(r_vcpu, addr, sizeof(u32), *val);
+ region->write(r_vcpu, addr, sizeof(u32), *val);
return 0;
}
diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
index 5693f6df..9c3d6d3 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.h
+++ b/virt/kvm/arm/vgic/vgic-mmio.h
@@ -37,8 +37,8 @@ struct vgic_register_region {
unsigned long (*uaccess_read)(struct kvm_vcpu *vcpu, gpa_t addr,
unsigned int len);
union {
- void (*uaccess_write)(struct kvm_vcpu *vcpu, gpa_t addr,
- unsigned int len, unsigned long val);
+ int (*uaccess_write)(struct kvm_vcpu *vcpu, gpa_t addr,
+ unsigned int len, unsigned long val);
int (*uaccess_its_write)(struct kvm *kvm, struct vgic_its *its,
gpa_t addr, unsigned int len,
unsigned long val);
@@ -134,6 +134,9 @@ unsigned long vgic_mmio_read_rao(struct kvm_vcpu *vcpu,
void vgic_mmio_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
unsigned int len, unsigned long val);
+int vgic_mmio_uaccess_write_wi(struct kvm_vcpu *vcpu, gpa_t addr,
+ unsigned int len, unsigned long val);
+
unsigned long vgic_mmio_read_enable(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len);
@@ -167,13 +170,13 @@ void vgic_mmio_write_sactive(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len,
unsigned long val);
-void vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
- gpa_t addr, unsigned int len,
- unsigned long val);
+int vgic_mmio_uaccess_write_cactive(struct kvm_vcpu *vcpu,
+ gpa_t addr, unsigned int len,
+ unsigned long val);
-void vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
- gpa_t addr, unsigned int len,
- unsigned long val);
+int vgic_mmio_uaccess_write_sactive(struct kvm_vcpu *vcpu,
+ gpa_t addr, unsigned int len,
+ unsigned long val);
unsigned long vgic_mmio_read_priority(struct kvm_vcpu *vcpu,
gpa_t addr, unsigned int len);
--
2.7.4
next prev parent reply other threads:[~2018-07-16 13:06 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-16 13:06 [PATCH v4 00/10] KVM: arm/arm64: vgic: Virtual interrupt grouping support Christoffer Dall
2018-07-16 13:06 ` Christoffer Dall
2018-07-16 13:06 ` [PATCH v4 01/10] KVM: arm/arm64: vgic: Define GICD_IIDR fields for GICv2 and GIv3 Christoffer Dall
2018-07-16 13:06 ` Christoffer Dall
2018-07-19 16:08 ` Andrew Jones
2018-07-19 16:08 ` Andrew Jones
2018-07-16 13:06 ` [PATCH v4 02/10] KVM: arm/arm64: vgic: Keep track of implementation revision Christoffer Dall
2018-07-16 13:06 ` Christoffer Dall
2018-07-19 16:11 ` Andrew Jones
2018-07-19 16:11 ` Andrew Jones
2018-07-16 13:06 ` [PATCH v4 03/10] KVM: arm/arm64: vgic: GICv2 IGROUPR should read as zero Christoffer Dall
2018-07-16 13:06 ` Christoffer Dall
2018-07-19 18:16 ` Andrew Jones
2018-07-19 18:16 ` Andrew Jones
2018-07-19 20:56 ` Christoffer Dall
2018-07-19 20:56 ` Christoffer Dall
2018-07-16 13:06 ` [PATCH v4 04/10] KVM: arm/arm64: vgic: Add group field to struct irq Christoffer Dall
2018-07-16 13:06 ` Christoffer Dall
2018-07-19 16:16 ` Andrew Jones
2018-07-19 16:16 ` Andrew Jones
2018-07-16 13:06 ` [PATCH v4 05/10] KVM: arm/arm64: vgic: Signal IRQs using their configured group Christoffer Dall
2018-07-16 13:06 ` Christoffer Dall
2018-07-19 16:28 ` Andrew Jones
2018-07-19 16:28 ` Andrew Jones
2018-07-16 13:06 ` Christoffer Dall [this message]
2018-07-16 13:06 ` [PATCH v4 06/10] KVM: arm/arm64: vgic: Permit uaccess writes to return errors Christoffer Dall
2018-07-19 16:31 ` Andrew Jones
2018-07-19 16:31 ` Andrew Jones
2018-07-16 13:06 ` [PATCH v4 07/10] KVM: arm/arm64: vgic: Return error on incompatible uaccess GICD_IIDR writes Christoffer Dall
2018-07-16 13:06 ` Christoffer Dall
2018-07-16 15:43 ` Bharat Bhushan
2018-07-16 15:43 ` Bharat Bhushan
2018-07-17 9:10 ` Christoffer Dall
2018-07-17 9:10 ` Christoffer Dall
2018-07-19 17:09 ` Andrew Jones
2018-07-19 17:09 ` Andrew Jones
2018-07-16 13:06 ` [PATCH v4 08/10] KVM: arm/arm64: vgic: Allow configuration of interrupt groups Christoffer Dall
2018-07-16 13:06 ` Christoffer Dall
2018-07-19 17:26 ` Andrew Jones
2018-07-19 17:26 ` Andrew Jones
2018-07-16 13:06 ` [PATCH v4 09/10] KVM: arm/arm64: vgic: Let userspace opt-in to writable v2 IGROUPR Christoffer Dall
2018-07-16 13:06 ` Christoffer Dall
2018-07-19 18:17 ` Andrew Jones
2018-07-19 18:17 ` Andrew Jones
2018-07-16 13:06 ` [PATCH v4 10/10] KVM: arm/arm64: vgic: Update documentation of the GIC devices wrt IIDR Christoffer Dall
2018-07-16 13:06 ` Christoffer Dall
2018-07-16 15:59 ` Bharat Bhushan
2018-07-16 15:59 ` Bharat Bhushan
2018-07-17 9:13 ` Christoffer Dall
2018-07-17 9:13 ` Christoffer Dall
2018-07-18 10:45 ` Bharat Bhushan
2018-07-18 10:45 ` Bharat Bhushan
2018-07-19 17:08 ` Andrew Jones
2018-07-19 17:08 ` Andrew Jones
2018-07-19 20:57 ` Christoffer Dall
2018-07-19 20:57 ` Christoffer Dall
2018-07-20 10:12 ` [PATCH v4 00/10] KVM: arm/arm64: vgic: Virtual interrupt grouping support Marc Zyngier
2018-07-20 10:12 ` Marc Zyngier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1531746387-7033-7-git-send-email-christoffer.dall@arm.com \
--to=christoffer.dall@arm.com \
--cc=andre.przywara@arm.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=marc.zyngier@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.