All of lore.kernel.org
 help / color / mirror / Atom feed
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 09/10] KVM: arm/arm64: vgic: Let userspace opt-in to writable v2 IGROUPR
Date: Mon, 16 Jul 2018 15:06:26 +0200	[thread overview]
Message-ID: <1531746387-7033-10-git-send-email-christoffer.dall@arm.com> (raw)
In-Reply-To: <1531746387-7033-1-git-send-email-christoffer.dall@arm.com>

Simply letting IGROUPR be writable from userspace would break
migration from old kernels to newer kernels, because old kernels
incorrectly report interrupt groups as group 1.  This would not be a big
problem if userspace wrote GICD_IIDR as read from the kernel, because we
could detect the incompatibility and return an error to userspace.
Unfortunately, this is not the case with current userspace
implementations and simply letting IGROUPR be writable from userspace for
an emulated GICv2 silently breaks migration and causes the destination
VM to no longer run after migration.

We now encourage userspace to write the read and expected value of
GICD_IIDR as the first part of a GIC register restore, and if we observe
a write to GICD_IIDR we know that userspace has been updated and has had
a chance to cope with older kernels (VGICv2 IIDR.Revision == 0)
incorrectly reporting interrupts as group 1, and therefore we now allow
groups to be user writable.

Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
---
 include/kvm/arm_vgic.h           |  3 +++
 virt/kvm/arm/vgic/vgic-mmio-v2.c | 16 +++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index c661d0e..c134790 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -221,6 +221,9 @@ struct vgic_dist {
 	/* Implementation revision as reported in the GICD_IIDR */
 	u32			implementation_rev;
 
+	/* Userspace can write to GICv2 IGROUPR */
+	bool			v2_groups_user_writable;
+
 	/* Do injected MSIs require an additional device ID? */
 	bool			msis_require_devid;
 
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
index ee164f8..26654f4 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
@@ -85,6 +85,18 @@ static int vgic_mmio_uaccess_write_v2_misc(struct kvm_vcpu *vcpu,
 	case GIC_DIST_IIDR:
 		if (val != vgic_mmio_read_v2_misc(vcpu, addr, len))
 			return -EINVAL;
+
+		/*
+		 * If we observe a write to GICD_IIDR we know that userspace
+		 * has been updated and has had a chance to cope with older
+		 * kernels (VGICv2 IIDR.Revision == 0) incorrectly reporting
+		 * interrupts as group 1, and therefore we now allow groups to
+		 * be user writable.  Doing this by default would break
+		 * migration from old kernels to new kernels with legacy
+		 * userspace.
+		 */
+		vcpu->kvm->arch.vgic.v2_groups_user_writable = true;
+		return 0;
 	}
 
 	vgic_mmio_write_v2_misc(vcpu, addr, len, val);
@@ -95,7 +107,9 @@ static int vgic_mmio_uaccess_write_v2_group(struct kvm_vcpu *vcpu,
 					    gpa_t addr, unsigned int len,
 					    unsigned long val)
 {
-	/* Ignore writes from userspace */
+	if (vcpu->kvm->arch.vgic.v2_groups_user_writable)
+		vgic_mmio_write_group(vcpu, addr, len, val);
+
 	return 0;
 }
 
-- 
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 09/10] KVM: arm/arm64: vgic: Let userspace opt-in to writable v2 IGROUPR
Date: Mon, 16 Jul 2018 15:06:26 +0200	[thread overview]
Message-ID: <1531746387-7033-10-git-send-email-christoffer.dall@arm.com> (raw)
In-Reply-To: <1531746387-7033-1-git-send-email-christoffer.dall@arm.com>

Simply letting IGROUPR be writable from userspace would break
migration from old kernels to newer kernels, because old kernels
incorrectly report interrupt groups as group 1.  This would not be a big
problem if userspace wrote GICD_IIDR as read from the kernel, because we
could detect the incompatibility and return an error to userspace.
Unfortunately, this is not the case with current userspace
implementations and simply letting IGROUPR be writable from userspace for
an emulated GICv2 silently breaks migration and causes the destination
VM to no longer run after migration.

We now encourage userspace to write the read and expected value of
GICD_IIDR as the first part of a GIC register restore, and if we observe
a write to GICD_IIDR we know that userspace has been updated and has had
a chance to cope with older kernels (VGICv2 IIDR.Revision == 0)
incorrectly reporting interrupts as group 1, and therefore we now allow
groups to be user writable.

Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
---
 include/kvm/arm_vgic.h           |  3 +++
 virt/kvm/arm/vgic/vgic-mmio-v2.c | 16 +++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index c661d0e..c134790 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -221,6 +221,9 @@ struct vgic_dist {
 	/* Implementation revision as reported in the GICD_IIDR */
 	u32			implementation_rev;
 
+	/* Userspace can write to GICv2 IGROUPR */
+	bool			v2_groups_user_writable;
+
 	/* Do injected MSIs require an additional device ID? */
 	bool			msis_require_devid;
 
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
index ee164f8..26654f4 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
@@ -85,6 +85,18 @@ static int vgic_mmio_uaccess_write_v2_misc(struct kvm_vcpu *vcpu,
 	case GIC_DIST_IIDR:
 		if (val != vgic_mmio_read_v2_misc(vcpu, addr, len))
 			return -EINVAL;
+
+		/*
+		 * If we observe a write to GICD_IIDR we know that userspace
+		 * has been updated and has had a chance to cope with older
+		 * kernels (VGICv2 IIDR.Revision == 0) incorrectly reporting
+		 * interrupts as group 1, and therefore we now allow groups to
+		 * be user writable.  Doing this by default would break
+		 * migration from old kernels to new kernels with legacy
+		 * userspace.
+		 */
+		vcpu->kvm->arch.vgic.v2_groups_user_writable = true;
+		return 0;
 	}
 
 	vgic_mmio_write_v2_misc(vcpu, addr, len, val);
@@ -95,7 +107,9 @@ static int vgic_mmio_uaccess_write_v2_group(struct kvm_vcpu *vcpu,
 					    gpa_t addr, unsigned int len,
 					    unsigned long val)
 {
-	/* Ignore writes from userspace */
+	if (vcpu->kvm->arch.vgic.v2_groups_user_writable)
+		vgic_mmio_write_group(vcpu, addr, len, val);
+
 	return 0;
 }
 
-- 
2.7.4

  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 ` [PATCH v4 06/10] KVM: arm/arm64: vgic: Permit uaccess writes to return errors Christoffer Dall
2018-07-16 13:06   ` 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 ` Christoffer Dall [this message]
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-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-10-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.