linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus
@ 2015-03-26 14:39 Andre Przywara
  2015-03-26 14:39 ` [PATCH v3 01/11] KVM: Redesign kvm_io_bus_ API to pass VCPU structure to the callbacks Andre Przywara
                   ` (10 more replies)
  0 siblings, 11 replies; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

This series converts the VGIC MMIO handling routines to the generic
kvm_io_bus framework. The framework is needed for the ioeventfd
functionality, some people on the list wanted to see the VGIC
converted over to use it, too.
Beside from now moving to a generic framework instead of relying on
an ARM specific one we also clean up quite some code and get rid of
some unnecessary copying.
On that way the MMIO abort handling for ARM has changed quite a bit,
so please have a closer look and test it on your setup if possible.

Compared to v2 I merged in a build fix for patch 1.
The old 06/12 is now gone, instead I changed the code to only call
kvm_io_bus_unregister_dev when rolling back a failed init and letting
the VM teardown take care of the io_bus destruction entirely.
This affects the old patches 08/12-10/12.
Also there is now a new patch (09/11), which optimises the GICv3
redistributor handling to only have one region for the register
frame. This save half of the kvm_io_bus devices in the next patch.
Also I merged in a fix when using QEMU with this series [2] and
merged the last two patches (enabling new framework and removing old
code), because it didn't build properly and fixing that up didn't
seem worthwile.

The series is loosely based on Nikolay's work[1], thanks especially
for the tedious first patch.
I totally reworked Nikolay's 3/5 to avoid adding another MMIO handling
layer on top of the already quite convoluted VGIC MMIO handling.
Also Nikolay's 2/5 get extended and changed significantly, that's why
I dropped his Signed-off-by.

Unfortunately kvm_io_bus lacks an opaque pointer to pass in some data,
so I worked around this by using container_of.
Now for every struct kvm_mmio_range array a KVM I/O device is
registered (one for VGICv2, nr_vcpus + 1 for VGICv3), using the
struct kvm_io_device variable as an anchor into the new
struct vgic_io_device. This one holds the base address, the
vgic_io_range pointer and (in case of the GICv3 redistributor) the
associated vCPU, so that we can access all instance-specific data
easily.

Patch 2 moves the iodev.h header file around, that solves a problem
when embedding a struct in arm_vgic.h later. That looks like a nice
cleanup anyway, so I added two patches to remove the compiler switch
to add virt/kvm as a include directory. This has been tested for
arm/arm64 and x86. As soon as I get around to compile-test the other
architectures, I can send out the respective patches for those, too.

Patches 5 and 6 tweak the existing code a bit to make it fit for the
conversion.
Patch 7 contains the framework for the new handling, while
patch 8 and 10 enable the GICv2 and GICv3 emulation, respectively.
Patch 9 optimises the GICv3 register handling to ease the GICv3
adaption to the new framework.
Patch 11 finally switches over to the new kvm_io_bus handling,
reworking the early ARM KVM MMIO handling quite a bit. This allows to
removes quite some now unneeded code.

The series goes on top of the kvmarm.git/next branch and was briefly
tested on an arm64 model with a GICv2 and a GICv3 guest and on Midway
(GICv2 guest).

Cheers,
Andre.

[1] https://lists.cs.columbia.edu/pipermail/kvmarm/2015-January/013379.html
[2] https://lists.cs.columbia.edu/pipermail/kvmarm/2015-March/014098.html

Changelog v2 .. v3:
- fix build in first patch
- fix NULL pointer dereference when using QEMU
- merge GICv3 RD_base and SGI_base MMIO register frames
- explicitly unregister kvm_io_bus devices on failing init
- remove vgic_unregister_kvm_io_dev (let kvm_io_bus framework handle this)
- remove marking of destroyed kvm_io_bus'es with NULL
- merge framework switch and code removal patch

Andre Przywara (10):
  KVM: move iodev.h from virt/kvm/ to include/kvm
  KVM: arm/arm64: remove now unneeded include directory from Makefile
  KVM: x86: remove now unneeded include directory from Makefile
  KVM: arm/arm64: rename struct kvm_mmio_range to vgic_io_range
  KVM: arm/arm64: simplify vgic_find_range() and callers
  KVM: arm/arm64: implement kvm_io_bus MMIO handling for the VGIC
  KVM: arm/arm64: prepare GICv2 emulation to be handled by kvm_io_bus
  KVM: arm/arm64: merge GICv3 RD_base and SGI_base register frames
  KVM: arm/arm64: prepare GICv3 emulation to use kvm_io_bus MMIO
    handling
  KVM: arm/arm64: rework MMIO abort handling to use KVM MMIO bus

Nikolay Nikolaev (1):
  KVM: Redesign kvm_io_bus_ API to pass VCPU structure to the
    callbacks.

 arch/arm/include/asm/kvm_mmio.h   |   22 ----
 arch/arm/kvm/Makefile             |    2 +-
 arch/arm/kvm/mmio.c               |   60 ++++++---
 arch/arm64/include/asm/kvm_mmio.h |   22 ----
 arch/arm64/kvm/Makefile           |    2 +-
 arch/powerpc/kvm/mpic.c           |   12 +-
 arch/powerpc/kvm/powerpc.c        |    4 +-
 arch/s390/kvm/diag.c              |    2 +-
 arch/x86/kvm/Makefile             |    2 +-
 arch/x86/kvm/i8254.c              |   14 ++-
 arch/x86/kvm/i8254.h              |    2 +-
 arch/x86/kvm/i8259.c              |   12 +-
 arch/x86/kvm/ioapic.c             |    8 +-
 arch/x86/kvm/ioapic.h             |    2 +-
 arch/x86/kvm/irq.h                |    2 +-
 arch/x86/kvm/lapic.c              |    4 +-
 arch/x86/kvm/lapic.h              |    2 +-
 arch/x86/kvm/vmx.c                |    2 +-
 arch/x86/kvm/x86.c                |   13 +-
 include/kvm/arm_vgic.h            |   17 ++-
 include/kvm/iodev.h               |   76 ++++++++++++
 include/linux/kvm_host.h          |   10 +-
 virt/kvm/arm/vgic-v2-emul.c       |   51 ++++----
 virt/kvm/arm/vgic-v3-emul.c       |  246 ++++++++++++++++++-------------------
 virt/kvm/arm/vgic.c               |  176 ++++++++++++++++----------
 virt/kvm/arm/vgic.h               |   29 +++--
 virt/kvm/coalesced_mmio.c         |    7 +-
 virt/kvm/eventfd.c                |    6 +-
 virt/kvm/iodev.h                  |   70 -----------
 virt/kvm/kvm_main.c               |   34 ++---
 30 files changed, 475 insertions(+), 436 deletions(-)
 create mode 100644 include/kvm/iodev.h
 delete mode 100644 virt/kvm/iodev.h

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 01/11] KVM: Redesign kvm_io_bus_ API to pass VCPU structure to the callbacks.
  2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
@ 2015-03-26 14:39 ` Andre Przywara
  2015-03-26 14:39 ` [PATCH v3 02/11] KVM: move iodev.h from virt/kvm/ to include/kvm Andre Przywara
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

From: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>

This is needed in e.g. ARM vGIC emulation, where the MMIO handling
depends on the VCPU that does the access.

Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/powerpc/kvm/mpic.c    |   10 ++++++----
 arch/powerpc/kvm/powerpc.c |    4 ++--
 arch/s390/kvm/diag.c       |    2 +-
 arch/x86/kvm/i8254.c       |   14 +++++++++-----
 arch/x86/kvm/i8259.c       |   12 ++++++------
 arch/x86/kvm/ioapic.c      |    8 ++++----
 arch/x86/kvm/lapic.c       |    4 ++--
 arch/x86/kvm/vmx.c         |    2 +-
 arch/x86/kvm/x86.c         |   13 +++++++------
 include/linux/kvm_host.h   |   10 +++++-----
 virt/kvm/coalesced_mmio.c  |    5 +++--
 virt/kvm/eventfd.c         |    4 ++--
 virt/kvm/iodev.h           |   23 +++++++++++++++--------
 virt/kvm/kvm_main.c        |   32 ++++++++++++++++----------------
 14 files changed, 79 insertions(+), 64 deletions(-)

diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c
index 39b3a8f..8542f07 100644
--- a/arch/powerpc/kvm/mpic.c
+++ b/arch/powerpc/kvm/mpic.c
@@ -1374,8 +1374,9 @@ static int kvm_mpic_write_internal(struct openpic *opp, gpa_t addr, u32 val)
 	return -ENXIO;
 }
 
-static int kvm_mpic_read(struct kvm_io_device *this, gpa_t addr,
-			 int len, void *ptr)
+static int kvm_mpic_read(struct kvm_vcpu *vcpu,
+			 struct kvm_io_device *this,
+			 gpa_t addr, int len, void *ptr)
 {
 	struct openpic *opp = container_of(this, struct openpic, mmio);
 	int ret;
@@ -1415,8 +1416,9 @@ static int kvm_mpic_read(struct kvm_io_device *this, gpa_t addr,
 	return ret;
 }
 
-static int kvm_mpic_write(struct kvm_io_device *this, gpa_t addr,
-			  int len, const void *ptr)
+static int kvm_mpic_write(struct kvm_vcpu *vcpu,
+			  struct kvm_io_device *this,
+			  gpa_t addr, int len, const void *ptr)
 {
 	struct openpic *opp = container_of(this, struct openpic, mmio);
 	int ret;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 27c0fac..24bfe40 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -807,7 +807,7 @@ int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
 
 	idx = srcu_read_lock(&vcpu->kvm->srcu);
 
-	ret = kvm_io_bus_read(vcpu->kvm, KVM_MMIO_BUS, run->mmio.phys_addr,
+	ret = kvm_io_bus_read(vcpu, KVM_MMIO_BUS, run->mmio.phys_addr,
 			      bytes, &run->mmio.data);
 
 	srcu_read_unlock(&vcpu->kvm->srcu, idx);
@@ -880,7 +880,7 @@ int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu,
 
 	idx = srcu_read_lock(&vcpu->kvm->srcu);
 
-	ret = kvm_io_bus_write(vcpu->kvm, KVM_MMIO_BUS, run->mmio.phys_addr,
+	ret = kvm_io_bus_write(vcpu, KVM_MMIO_BUS, run->mmio.phys_addr,
 			       bytes, &run->mmio.data);
 
 	srcu_read_unlock(&vcpu->kvm->srcu, idx);
diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
index 9254aff..329ec75 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -213,7 +213,7 @@ static int __diag_virtio_hypercall(struct kvm_vcpu *vcpu)
 	 * - gpr 3 contains the virtqueue index (passed as datamatch)
 	 * - gpr 4 contains the index on the bus (optionally)
 	 */
-	ret = kvm_io_bus_write_cookie(vcpu->kvm, KVM_VIRTIO_CCW_NOTIFY_BUS,
+	ret = kvm_io_bus_write_cookie(vcpu, KVM_VIRTIO_CCW_NOTIFY_BUS,
 				      vcpu->run->s.regs.gprs[2] & 0xffffffff,
 				      8, &vcpu->run->s.regs.gprs[3],
 				      vcpu->run->s.regs.gprs[4]);
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 298781d..4dce6f8 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -443,7 +443,8 @@ static inline int pit_in_range(gpa_t addr)
 		(addr < KVM_PIT_BASE_ADDRESS + KVM_PIT_MEM_LENGTH));
 }
 
-static int pit_ioport_write(struct kvm_io_device *this,
+static int pit_ioport_write(struct kvm_vcpu *vcpu,
+				struct kvm_io_device *this,
 			    gpa_t addr, int len, const void *data)
 {
 	struct kvm_pit *pit = dev_to_pit(this);
@@ -519,7 +520,8 @@ static int pit_ioport_write(struct kvm_io_device *this,
 	return 0;
 }
 
-static int pit_ioport_read(struct kvm_io_device *this,
+static int pit_ioport_read(struct kvm_vcpu *vcpu,
+			   struct kvm_io_device *this,
 			   gpa_t addr, int len, void *data)
 {
 	struct kvm_pit *pit = dev_to_pit(this);
@@ -589,7 +591,8 @@ static int pit_ioport_read(struct kvm_io_device *this,
 	return 0;
 }
 
-static int speaker_ioport_write(struct kvm_io_device *this,
+static int speaker_ioport_write(struct kvm_vcpu *vcpu,
+				struct kvm_io_device *this,
 				gpa_t addr, int len, const void *data)
 {
 	struct kvm_pit *pit = speaker_to_pit(this);
@@ -606,8 +609,9 @@ static int speaker_ioport_write(struct kvm_io_device *this,
 	return 0;
 }
 
-static int speaker_ioport_read(struct kvm_io_device *this,
-			       gpa_t addr, int len, void *data)
+static int speaker_ioport_read(struct kvm_vcpu *vcpu,
+				   struct kvm_io_device *this,
+				   gpa_t addr, int len, void *data)
 {
 	struct kvm_pit *pit = speaker_to_pit(this);
 	struct kvm_kpit_state *pit_state = &pit->pit_state;
diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index cc31f7c..8ff4eaa 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -528,42 +528,42 @@ static int picdev_read(struct kvm_pic *s,
 	return 0;
 }
 
-static int picdev_master_write(struct kvm_io_device *dev,
+static int picdev_master_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
 			       gpa_t addr, int len, const void *val)
 {
 	return picdev_write(container_of(dev, struct kvm_pic, dev_master),
 			    addr, len, val);
 }
 
-static int picdev_master_read(struct kvm_io_device *dev,
+static int picdev_master_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
 			      gpa_t addr, int len, void *val)
 {
 	return picdev_read(container_of(dev, struct kvm_pic, dev_master),
 			    addr, len, val);
 }
 
-static int picdev_slave_write(struct kvm_io_device *dev,
+static int picdev_slave_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
 			      gpa_t addr, int len, const void *val)
 {
 	return picdev_write(container_of(dev, struct kvm_pic, dev_slave),
 			    addr, len, val);
 }
 
-static int picdev_slave_read(struct kvm_io_device *dev,
+static int picdev_slave_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
 			     gpa_t addr, int len, void *val)
 {
 	return picdev_read(container_of(dev, struct kvm_pic, dev_slave),
 			    addr, len, val);
 }
 
-static int picdev_eclr_write(struct kvm_io_device *dev,
+static int picdev_eclr_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
 			     gpa_t addr, int len, const void *val)
 {
 	return picdev_write(container_of(dev, struct kvm_pic, dev_eclr),
 			    addr, len, val);
 }
 
-static int picdev_eclr_read(struct kvm_io_device *dev,
+static int picdev_eclr_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
 			    gpa_t addr, int len, void *val)
 {
 	return picdev_read(container_of(dev, struct kvm_pic, dev_eclr),
diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index b1947e0..8bf2e49 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -498,8 +498,8 @@ static inline int ioapic_in_range(struct kvm_ioapic *ioapic, gpa_t addr)
 		 (addr < ioapic->base_address + IOAPIC_MEM_LENGTH)));
 }
 
-static int ioapic_mmio_read(struct kvm_io_device *this, gpa_t addr, int len,
-			    void *val)
+static int ioapic_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
+				gpa_t addr, int len, void *val)
 {
 	struct kvm_ioapic *ioapic = to_ioapic(this);
 	u32 result;
@@ -541,8 +541,8 @@ static int ioapic_mmio_read(struct kvm_io_device *this, gpa_t addr, int len,
 	return 0;
 }
 
-static int ioapic_mmio_write(struct kvm_io_device *this, gpa_t addr, int len,
-			     const void *val)
+static int ioapic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
+				 gpa_t addr, int len, const void *val)
 {
 	struct kvm_ioapic *ioapic = to_ioapic(this);
 	u32 data;
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index e55b5fc..ba57bb7 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1038,7 +1038,7 @@ static int apic_mmio_in_range(struct kvm_lapic *apic, gpa_t addr)
 	    addr < apic->base_address + LAPIC_MMIO_LENGTH;
 }
 
-static int apic_mmio_read(struct kvm_io_device *this,
+static int apic_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 			   gpa_t address, int len, void *data)
 {
 	struct kvm_lapic *apic = to_lapic(this);
@@ -1358,7 +1358,7 @@ static int apic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 	return ret;
 }
 
-static int apic_mmio_write(struct kvm_io_device *this,
+static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 			    gpa_t address, int len, const void *data)
 {
 	struct kvm_lapic *apic = to_lapic(this);
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index f7b20b4..317da9b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5822,7 +5822,7 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
 	gpa_t gpa;
 
 	gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
-	if (!kvm_io_bus_write(vcpu->kvm, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) {
+	if (!kvm_io_bus_write(vcpu, KVM_FAST_MMIO_BUS, gpa, 0, NULL)) {
 		skip_emulated_instruction(vcpu);
 		return 1;
 	}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index bd7a70b..5573d63 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4115,8 +4115,8 @@ static int vcpu_mmio_write(struct kvm_vcpu *vcpu, gpa_t addr, int len,
 	do {
 		n = min(len, 8);
 		if (!(vcpu->arch.apic &&
-		      !kvm_iodevice_write(&vcpu->arch.apic->dev, addr, n, v))
-		    && kvm_io_bus_write(vcpu->kvm, KVM_MMIO_BUS, addr, n, v))
+		      !kvm_iodevice_write(vcpu, &vcpu->arch.apic->dev, addr, n, v))
+		    && kvm_io_bus_write(vcpu, KVM_MMIO_BUS, addr, n, v))
 			break;
 		handled += n;
 		addr += n;
@@ -4135,8 +4135,9 @@ static int vcpu_mmio_read(struct kvm_vcpu *vcpu, gpa_t addr, int len, void *v)
 	do {
 		n = min(len, 8);
 		if (!(vcpu->arch.apic &&
-		      !kvm_iodevice_read(&vcpu->arch.apic->dev, addr, n, v))
-		    && kvm_io_bus_read(vcpu->kvm, KVM_MMIO_BUS, addr, n, v))
+		      !kvm_iodevice_read(vcpu, &vcpu->arch.apic->dev,
+					 addr, n, v))
+		    && kvm_io_bus_read(vcpu, KVM_MMIO_BUS, addr, n, v))
 			break;
 		trace_kvm_mmio(KVM_TRACE_MMIO_READ, n, addr, *(u64 *)v);
 		handled += n;
@@ -4630,10 +4631,10 @@ static int kernel_pio(struct kvm_vcpu *vcpu, void *pd)
 	int r;
 
 	if (vcpu->arch.pio.in)
-		r = kvm_io_bus_read(vcpu->kvm, KVM_PIO_BUS, vcpu->arch.pio.port,
+		r = kvm_io_bus_read(vcpu, KVM_PIO_BUS, vcpu->arch.pio.port,
 				    vcpu->arch.pio.size, pd);
 	else
-		r = kvm_io_bus_write(vcpu->kvm, KVM_PIO_BUS,
+		r = kvm_io_bus_write(vcpu, KVM_PIO_BUS,
 				     vcpu->arch.pio.port, vcpu->arch.pio.size,
 				     pd);
 	return r;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index ae9c720..9605e46 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -165,12 +165,12 @@ enum kvm_bus {
 	KVM_NR_BUSES
 };
 
-int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
+int kvm_io_bus_write(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
 		     int len, const void *val);
-int kvm_io_bus_write_cookie(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
-			    int len, const void *val, long cookie);
-int kvm_io_bus_read(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr, int len,
-		    void *val);
+int kvm_io_bus_write_cookie(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx,
+			    gpa_t addr, int len, const void *val, long cookie);
+int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
+		    int len, void *val);
 int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 			    int len, struct kvm_io_device *dev);
 int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
index 00d8642..c831a40 100644
--- a/virt/kvm/coalesced_mmio.c
+++ b/virt/kvm/coalesced_mmio.c
@@ -60,8 +60,9 @@ static int coalesced_mmio_has_room(struct kvm_coalesced_mmio_dev *dev)
 	return 1;
 }
 
-static int coalesced_mmio_write(struct kvm_io_device *this,
-				gpa_t addr, int len, const void *val)
+static int coalesced_mmio_write(struct kvm_vcpu *vcpu,
+				struct kvm_io_device *this, gpa_t addr,
+				int len, const void *val)
 {
 	struct kvm_coalesced_mmio_dev *dev = to_mmio(this);
 	struct kvm_coalesced_mmio_ring *ring = dev->kvm->coalesced_mmio_ring;
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index fc5f43e..26c72f3 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -715,8 +715,8 @@ ioeventfd_in_range(struct _ioeventfd *p, gpa_t addr, int len, const void *val)
 
 /* MMIO/PIO writes trigger an event if the addr/val match */
 static int
-ioeventfd_write(struct kvm_io_device *this, gpa_t addr, int len,
-		const void *val)
+ioeventfd_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this, gpa_t addr,
+		int len, const void *val)
 {
 	struct _ioeventfd *p = to_ioeventfd(this);
 
diff --git a/virt/kvm/iodev.h b/virt/kvm/iodev.h
index 12fd3ca..9ef709c 100644
--- a/virt/kvm/iodev.h
+++ b/virt/kvm/iodev.h
@@ -20,6 +20,7 @@
 #include <asm/errno.h>
 
 struct kvm_io_device;
+struct kvm_vcpu;
 
 /**
  * kvm_io_device_ops are called under kvm slots_lock.
@@ -27,11 +28,13 @@ struct kvm_io_device;
  * or non-zero to have it passed to the next device.
  **/
 struct kvm_io_device_ops {
-	int (*read)(struct kvm_io_device *this,
+	int (*read)(struct kvm_vcpu *vcpu,
+		    struct kvm_io_device *this,
 		    gpa_t addr,
 		    int len,
 		    void *val);
-	int (*write)(struct kvm_io_device *this,
+	int (*write)(struct kvm_vcpu *vcpu,
+		     struct kvm_io_device *this,
 		     gpa_t addr,
 		     int len,
 		     const void *val);
@@ -49,16 +52,20 @@ static inline void kvm_iodevice_init(struct kvm_io_device *dev,
 	dev->ops = ops;
 }
 
-static inline int kvm_iodevice_read(struct kvm_io_device *dev,
-				    gpa_t addr, int l, void *v)
+static inline int kvm_iodevice_read(struct kvm_vcpu *vcpu,
+				    struct kvm_io_device *dev, gpa_t addr,
+				    int l, void *v)
 {
-	return dev->ops->read ? dev->ops->read(dev, addr, l, v) : -EOPNOTSUPP;
+	return dev->ops->read ? dev->ops->read(vcpu, dev, addr, l, v)
+				: -EOPNOTSUPP;
 }
 
-static inline int kvm_iodevice_write(struct kvm_io_device *dev,
-				     gpa_t addr, int l, const void *v)
+static inline int kvm_iodevice_write(struct kvm_vcpu *vcpu,
+				     struct kvm_io_device *dev, gpa_t addr,
+				     int l, const void *v)
 {
-	return dev->ops->write ? dev->ops->write(dev, addr, l, v) : -EOPNOTSUPP;
+	return dev->ops->write ? dev->ops->write(vcpu, dev, addr, l, v)
+				 : -EOPNOTSUPP;
 }
 
 static inline void kvm_iodevice_destructor(struct kvm_io_device *dev)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index a109370..664d67a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2997,7 +2997,7 @@ static int kvm_io_bus_get_first_dev(struct kvm_io_bus *bus,
 	return off;
 }
 
-static int __kvm_io_bus_write(struct kvm_io_bus *bus,
+static int __kvm_io_bus_write(struct kvm_vcpu *vcpu, struct kvm_io_bus *bus,
 			      struct kvm_io_range *range, const void *val)
 {
 	int idx;
@@ -3008,7 +3008,7 @@ static int __kvm_io_bus_write(struct kvm_io_bus *bus,
 
 	while (idx < bus->dev_count &&
 		kvm_io_bus_cmp(range, &bus->range[idx]) == 0) {
-		if (!kvm_iodevice_write(bus->range[idx].dev, range->addr,
+		if (!kvm_iodevice_write(vcpu, bus->range[idx].dev, range->addr,
 					range->len, val))
 			return idx;
 		idx++;
@@ -3018,7 +3018,7 @@ static int __kvm_io_bus_write(struct kvm_io_bus *bus,
 }
 
 /* kvm_io_bus_write - called under kvm->slots_lock */
-int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
+int kvm_io_bus_write(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
 		     int len, const void *val)
 {
 	struct kvm_io_bus *bus;
@@ -3030,14 +3030,14 @@ int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 		.len = len,
 	};
 
-	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);
-	r = __kvm_io_bus_write(bus, &range, val);
+	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
+	r = __kvm_io_bus_write(vcpu, bus, &range, val);
 	return r < 0 ? r : 0;
 }
 
 /* kvm_io_bus_write_cookie - called under kvm->slots_lock */
-int kvm_io_bus_write_cookie(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
-			    int len, const void *val, long cookie)
+int kvm_io_bus_write_cookie(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx,
+			    gpa_t addr, int len, const void *val, long cookie)
 {
 	struct kvm_io_bus *bus;
 	struct kvm_io_range range;
@@ -3047,12 +3047,12 @@ int kvm_io_bus_write_cookie(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 		.len = len,
 	};
 
-	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);
+	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
 
 	/* First try the device referenced by cookie. */
 	if ((cookie >= 0) && (cookie < bus->dev_count) &&
 	    (kvm_io_bus_cmp(&range, &bus->range[cookie]) == 0))
-		if (!kvm_iodevice_write(bus->range[cookie].dev, addr, len,
+		if (!kvm_iodevice_write(vcpu, bus->range[cookie].dev, addr, len,
 					val))
 			return cookie;
 
@@ -3060,11 +3060,11 @@ int kvm_io_bus_write_cookie(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 	 * cookie contained garbage; fall back to search and return the
 	 * correct cookie value.
 	 */
-	return __kvm_io_bus_write(bus, &range, val);
+	return __kvm_io_bus_write(vcpu, bus, &range, val);
 }
 
-static int __kvm_io_bus_read(struct kvm_io_bus *bus, struct kvm_io_range *range,
-			     void *val)
+static int __kvm_io_bus_read(struct kvm_vcpu *vcpu, struct kvm_io_bus *bus,
+			     struct kvm_io_range *range, void *val)
 {
 	int idx;
 
@@ -3074,7 +3074,7 @@ static int __kvm_io_bus_read(struct kvm_io_bus *bus, struct kvm_io_range *range,
 
 	while (idx < bus->dev_count &&
 		kvm_io_bus_cmp(range, &bus->range[idx]) == 0) {
-		if (!kvm_iodevice_read(bus->range[idx].dev, range->addr,
+		if (!kvm_iodevice_read(vcpu, bus->range[idx].dev, range->addr,
 				       range->len, val))
 			return idx;
 		idx++;
@@ -3085,7 +3085,7 @@ static int __kvm_io_bus_read(struct kvm_io_bus *bus, struct kvm_io_range *range,
 EXPORT_SYMBOL_GPL(kvm_io_bus_write);
 
 /* kvm_io_bus_read - called under kvm->slots_lock */
-int kvm_io_bus_read(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
+int kvm_io_bus_read(struct kvm_vcpu *vcpu, enum kvm_bus bus_idx, gpa_t addr,
 		    int len, void *val)
 {
 	struct kvm_io_bus *bus;
@@ -3097,8 +3097,8 @@ int kvm_io_bus_read(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 		.len = len,
 	};
 
-	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);
-	r = __kvm_io_bus_read(bus, &range, val);
+	bus = srcu_dereference(vcpu->kvm->buses[bus_idx], &vcpu->kvm->srcu);
+	r = __kvm_io_bus_read(vcpu, bus, &range, val);
 	return r < 0 ? r : 0;
 }
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 02/11] KVM: move iodev.h from virt/kvm/ to include/kvm
  2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
  2015-03-26 14:39 ` [PATCH v3 01/11] KVM: Redesign kvm_io_bus_ API to pass VCPU structure to the callbacks Andre Przywara
@ 2015-03-26 14:39 ` Andre Przywara
  2015-03-26 14:39 ` [PATCH v3 03/11] KVM: arm/arm64: remove now unneeded include directory from Makefile Andre Przywara
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

iodev.h contains definitions for the kvm_io_bus framework. This is
needed both by the generic KVM code in virt/kvm as well as by
architecture specific code under arch/. Putting the header file in
virt/kvm and using local includes in the architecture part seems at
least dodgy to me, so let's move the file into include/kvm, so that a
more natural "#include <kvm/iodev.h>" can be used by all of the code.
This also solves a problem later when using struct kvm_io_device
in arm_vgic.h.
Fixing up the FSF address in the GPL header and a wrong include path
on the way.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
---
 arch/powerpc/kvm/mpic.c   |    2 +-
 arch/x86/kvm/i8254.h      |    2 +-
 arch/x86/kvm/ioapic.h     |    2 +-
 arch/x86/kvm/irq.h        |    2 +-
 arch/x86/kvm/lapic.h      |    2 +-
 include/kvm/iodev.h       |   76 ++++++++++++++++++++++++++++++++++++++++++++
 virt/kvm/coalesced_mmio.c |    2 +-
 virt/kvm/eventfd.c        |    2 +-
 virt/kvm/iodev.h          |   77 ---------------------------------------------
 virt/kvm/kvm_main.c       |    2 +-
 10 files changed, 84 insertions(+), 85 deletions(-)
 create mode 100644 include/kvm/iodev.h
 delete mode 100644 virt/kvm/iodev.h

diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c
index 8542f07..4703fad 100644
--- a/arch/powerpc/kvm/mpic.c
+++ b/arch/powerpc/kvm/mpic.c
@@ -34,7 +34,7 @@
 #include <asm/kvm_para.h>
 #include <asm/kvm_host.h>
 #include <asm/kvm_ppc.h>
-#include "iodev.h"
+#include <kvm/iodev.h>
 
 #define MAX_CPU     32
 #define MAX_SRC     256
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h
index dd1b16b..c84990b 100644
--- a/arch/x86/kvm/i8254.h
+++ b/arch/x86/kvm/i8254.h
@@ -3,7 +3,7 @@
 
 #include <linux/kthread.h>
 
-#include "iodev.h"
+#include <kvm/iodev.h>
 
 struct kvm_kpit_channel_state {
 	u32 count; /* can be 65536 */
diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
index c2e36d9..d9e02ca 100644
--- a/arch/x86/kvm/ioapic.h
+++ b/arch/x86/kvm/ioapic.h
@@ -3,7 +3,7 @@
 
 #include <linux/kvm_host.h>
 
-#include "iodev.h"
+#include <kvm/iodev.h>
 
 struct kvm;
 struct kvm_vcpu;
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 2d03568..ad68c73 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -27,7 +27,7 @@
 #include <linux/kvm_host.h>
 #include <linux/spinlock.h>
 
-#include "iodev.h"
+#include <kvm/iodev.h>
 #include "ioapic.h"
 #include "lapic.h"
 
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 0bc6c65..e284c28 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -1,7 +1,7 @@
 #ifndef __KVM_X86_LAPIC_H
 #define __KVM_X86_LAPIC_H
 
-#include "iodev.h"
+#include <kvm/iodev.h>
 
 #include <linux/kvm_host.h>
 
diff --git a/include/kvm/iodev.h b/include/kvm/iodev.h
new file mode 100644
index 0000000..a6d208b
--- /dev/null
+++ b/include/kvm/iodev.h
@@ -0,0 +1,76 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __KVM_IODEV_H__
+#define __KVM_IODEV_H__
+
+#include <linux/kvm_types.h>
+#include <linux/errno.h>
+
+struct kvm_io_device;
+struct kvm_vcpu;
+
+/**
+ * kvm_io_device_ops are called under kvm slots_lock.
+ * read and write handlers return 0 if the transaction has been handled,
+ * or non-zero to have it passed to the next device.
+ **/
+struct kvm_io_device_ops {
+	int (*read)(struct kvm_vcpu *vcpu,
+		    struct kvm_io_device *this,
+		    gpa_t addr,
+		    int len,
+		    void *val);
+	int (*write)(struct kvm_vcpu *vcpu,
+		     struct kvm_io_device *this,
+		     gpa_t addr,
+		     int len,
+		     const void *val);
+	void (*destructor)(struct kvm_io_device *this);
+};
+
+
+struct kvm_io_device {
+	const struct kvm_io_device_ops *ops;
+};
+
+static inline void kvm_iodevice_init(struct kvm_io_device *dev,
+				     const struct kvm_io_device_ops *ops)
+{
+	dev->ops = ops;
+}
+
+static inline int kvm_iodevice_read(struct kvm_vcpu *vcpu,
+				    struct kvm_io_device *dev, gpa_t addr,
+				    int l, void *v)
+{
+	return dev->ops->read ? dev->ops->read(vcpu, dev, addr, l, v)
+				: -EOPNOTSUPP;
+}
+
+static inline int kvm_iodevice_write(struct kvm_vcpu *vcpu,
+				     struct kvm_io_device *dev, gpa_t addr,
+				     int l, const void *v)
+{
+	return dev->ops->write ? dev->ops->write(vcpu, dev, addr, l, v)
+				 : -EOPNOTSUPP;
+}
+
+static inline void kvm_iodevice_destructor(struct kvm_io_device *dev)
+{
+	if (dev->ops->destructor)
+		dev->ops->destructor(dev);
+}
+
+#endif /* __KVM_IODEV_H__ */
diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
index c831a40..571c1ce 100644
--- a/virt/kvm/coalesced_mmio.c
+++ b/virt/kvm/coalesced_mmio.c
@@ -8,7 +8,7 @@
  *
  */
 
-#include "iodev.h"
+#include <kvm/iodev.h>
 
 #include <linux/kvm_host.h>
 #include <linux/slab.h>
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 26c72f3..9ff4193 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -36,7 +36,7 @@
 #include <linux/seqlock.h>
 #include <trace/events/kvm.h>
 
-#include "iodev.h"
+#include <kvm/iodev.h>
 
 #ifdef CONFIG_HAVE_KVM_IRQFD
 /*
diff --git a/virt/kvm/iodev.h b/virt/kvm/iodev.h
deleted file mode 100644
index 9ef709c..0000000
--- a/virt/kvm/iodev.h
+++ /dev/null
@@ -1,77 +0,0 @@
-/*
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
- */
-
-#ifndef __KVM_IODEV_H__
-#define __KVM_IODEV_H__
-
-#include <linux/kvm_types.h>
-#include <asm/errno.h>
-
-struct kvm_io_device;
-struct kvm_vcpu;
-
-/**
- * kvm_io_device_ops are called under kvm slots_lock.
- * read and write handlers return 0 if the transaction has been handled,
- * or non-zero to have it passed to the next device.
- **/
-struct kvm_io_device_ops {
-	int (*read)(struct kvm_vcpu *vcpu,
-		    struct kvm_io_device *this,
-		    gpa_t addr,
-		    int len,
-		    void *val);
-	int (*write)(struct kvm_vcpu *vcpu,
-		     struct kvm_io_device *this,
-		     gpa_t addr,
-		     int len,
-		     const void *val);
-	void (*destructor)(struct kvm_io_device *this);
-};
-
-
-struct kvm_io_device {
-	const struct kvm_io_device_ops *ops;
-};
-
-static inline void kvm_iodevice_init(struct kvm_io_device *dev,
-				     const struct kvm_io_device_ops *ops)
-{
-	dev->ops = ops;
-}
-
-static inline int kvm_iodevice_read(struct kvm_vcpu *vcpu,
-				    struct kvm_io_device *dev, gpa_t addr,
-				    int l, void *v)
-{
-	return dev->ops->read ? dev->ops->read(vcpu, dev, addr, l, v)
-				: -EOPNOTSUPP;
-}
-
-static inline int kvm_iodevice_write(struct kvm_vcpu *vcpu,
-				     struct kvm_io_device *dev, gpa_t addr,
-				     int l, const void *v)
-{
-	return dev->ops->write ? dev->ops->write(vcpu, dev, addr, l, v)
-				 : -EOPNOTSUPP;
-}
-
-static inline void kvm_iodevice_destructor(struct kvm_io_device *dev)
-{
-	if (dev->ops->destructor)
-		dev->ops->destructor(dev);
-}
-
-#endif /* __KVM_IODEV_H__ */
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 664d67a..c5460b6 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -16,7 +16,7 @@
  *
  */
 
-#include "iodev.h"
+#include <kvm/iodev.h>
 
 #include <linux/kvm_host.h>
 #include <linux/kvm.h>
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 03/11] KVM: arm/arm64: remove now unneeded include directory from Makefile
  2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
  2015-03-26 14:39 ` [PATCH v3 01/11] KVM: Redesign kvm_io_bus_ API to pass VCPU structure to the callbacks Andre Przywara
  2015-03-26 14:39 ` [PATCH v3 02/11] KVM: move iodev.h from virt/kvm/ to include/kvm Andre Przywara
@ 2015-03-26 14:39 ` Andre Przywara
  2015-03-26 14:39 ` [PATCH v3 04/11] KVM: x86: " Andre Przywara
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

virt/kvm was never really a good include directory for anything else
than locally included headers.
With the move of iodev.h there is no need anymore to add this
directory the compiler's include path, so remove it from the arm and
arm64 kvm Makefile.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/Makefile   |    2 +-
 arch/arm64/kvm/Makefile |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile
index a093bf1..139e46c 100644
--- a/arch/arm/kvm/Makefile
+++ b/arch/arm/kvm/Makefile
@@ -7,7 +7,7 @@ ifeq ($(plus_virt),+virt)
 	plus_virt_def := -DREQUIRES_VIRT=1
 endif
 
-ccflags-y += -Ivirt/kvm -Iarch/arm/kvm
+ccflags-y += -Iarch/arm/kvm
 CFLAGS_arm.o := -I. $(plus_virt_def)
 CFLAGS_mmu.o := -I.
 
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index b22c636..d5904f8 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -2,7 +2,7 @@
 # Makefile for Kernel-based Virtual Machine module
 #
 
-ccflags-y += -Ivirt/kvm -Iarch/arm64/kvm
+ccflags-y += -Iarch/arm64/kvm
 CFLAGS_arm.o := -I.
 CFLAGS_mmu.o := -I.
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 04/11] KVM: x86: remove now unneeded include directory from Makefile
  2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
                   ` (2 preceding siblings ...)
  2015-03-26 14:39 ` [PATCH v3 03/11] KVM: arm/arm64: remove now unneeded include directory from Makefile Andre Przywara
@ 2015-03-26 14:39 ` Andre Przywara
  2015-03-26 14:39 ` [PATCH v3 05/11] KVM: arm/arm64: rename struct kvm_mmio_range to vgic_io_range Andre Przywara
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

virt/kvm was never really a good include directory for anything else
than locally included headers.
With the move of iodev.h there is no need anymore to add this
directory the compiler's include path, so remove it from the x86 kvm
Makefile.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Marcelo Tosatti <mtosatti@redhat.com>
---
 arch/x86/kvm/Makefile |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index 08f790d..16e8f96 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -1,5 +1,5 @@
 
-ccflags-y += -Ivirt/kvm -Iarch/x86/kvm
+ccflags-y += -Iarch/x86/kvm
 
 CFLAGS_x86.o := -I.
 CFLAGS_svm.o := -I.
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 05/11] KVM: arm/arm64: rename struct kvm_mmio_range to vgic_io_range
  2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
                   ` (3 preceding siblings ...)
  2015-03-26 14:39 ` [PATCH v3 04/11] KVM: x86: " Andre Przywara
@ 2015-03-26 14:39 ` Andre Przywara
  2015-03-26 14:39 ` [PATCH v3 06/11] KVM: arm/arm64: simplify vgic_find_range() and callers Andre Przywara
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

The name "kvm_mmio_range" is a bit bold, given that it only covers
the VGIC's MMIO ranges. To avoid confusion with kvm_io_range, rename
it to vgic_io_range.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 virt/kvm/arm/vgic-v2-emul.c |    6 +++---
 virt/kvm/arm/vgic-v3-emul.c |    8 ++++----
 virt/kvm/arm/vgic.c         |   18 +++++++++---------
 virt/kvm/arm/vgic.h         |   12 ++++++------
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
index c818662..ddb3135 100644
--- a/virt/kvm/arm/vgic-v2-emul.c
+++ b/virt/kvm/arm/vgic-v2-emul.c
@@ -319,7 +319,7 @@ static bool handle_mmio_sgi_clear(struct kvm_vcpu *vcpu,
 		return write_set_clear_sgi_pend_reg(vcpu, mmio, offset, false);
 }
 
-static const struct kvm_mmio_range vgic_dist_ranges[] = {
+static const struct vgic_io_range vgic_dist_ranges[] = {
 	{
 		.base		= GIC_DIST_CTRL,
 		.len		= 12,
@@ -647,7 +647,7 @@ static bool handle_cpu_mmio_ident(struct kvm_vcpu *vcpu,
  * CPU Interface Register accesses - these are not accessed by the VM, but by
  * user space for saving and restoring VGIC state.
  */
-static const struct kvm_mmio_range vgic_cpu_ranges[] = {
+static const struct vgic_io_range vgic_cpu_ranges[] = {
 	{
 		.base		= GIC_CPU_CTRL,
 		.len		= 12,
@@ -674,7 +674,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 				 struct kvm_device_attr *attr,
 				 u32 *reg, bool is_write)
 {
-	const struct kvm_mmio_range *r = NULL, *ranges;
+	const struct vgic_io_range *r = NULL, *ranges;
 	phys_addr_t offset;
 	int ret, cpuid, c;
 	struct kvm_vcpu *vcpu, *tmp_vcpu;
diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c
index b3f1546..14943e3 100644
--- a/virt/kvm/arm/vgic-v3-emul.c
+++ b/virt/kvm/arm/vgic-v3-emul.c
@@ -340,7 +340,7 @@ static bool handle_mmio_idregs(struct kvm_vcpu *vcpu,
 	return false;
 }
 
-static const struct kvm_mmio_range vgic_v3_dist_ranges[] = {
+static const struct vgic_io_range vgic_v3_dist_ranges[] = {
 	{
 		.base           = GICD_CTLR,
 		.len            = 0x04,
@@ -570,7 +570,7 @@ static bool handle_mmio_cfg_reg_redist(struct kvm_vcpu *vcpu,
 	return vgic_handle_cfg_reg(reg, mmio, offset);
 }
 
-static const struct kvm_mmio_range vgic_redist_sgi_ranges[] = {
+static const struct vgic_io_range vgic_redist_sgi_ranges[] = {
 	{
 		.base		= GICR_IGROUPR0,
 		.len		= 0x04,
@@ -676,7 +676,7 @@ static bool handle_mmio_typer_redist(struct kvm_vcpu *vcpu,
 	return false;
 }
 
-static const struct kvm_mmio_range vgic_redist_ranges[] = {
+static const struct vgic_io_range vgic_redist_ranges[] = {
 	{
 		.base           = GICR_CTLR,
 		.len            = 0x04,
@@ -726,7 +726,7 @@ static bool vgic_v3_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	unsigned long rdbase = dist->vgic_redist_base;
 	int nrcpus = atomic_read(&vcpu->kvm->online_vcpus);
 	int vcpu_id;
-	const struct kvm_mmio_range *mmio_range;
+	const struct vgic_io_range *mmio_range;
 
 	if (is_in_range(mmio->phys_addr, mmio->len, dbase, GIC_V3_DIST_SIZE)) {
 		return vgic_handle_mmio_range(vcpu, run, mmio,
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index ffd937c..21a3550 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -712,11 +712,11 @@ void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
 }
 
 const
-struct kvm_mmio_range *vgic_find_range(const struct kvm_mmio_range *ranges,
-				       struct kvm_exit_mmio *mmio,
-				       phys_addr_t offset)
+struct vgic_io_range *vgic_find_range(const struct vgic_io_range *ranges,
+				      struct kvm_exit_mmio *mmio,
+				      phys_addr_t offset)
 {
-	const struct kvm_mmio_range *r = ranges;
+	const struct vgic_io_range *r = ranges;
 
 	while (r->len) {
 		if (offset >= r->base &&
@@ -729,7 +729,7 @@ struct kvm_mmio_range *vgic_find_range(const struct kvm_mmio_range *ranges,
 }
 
 static bool vgic_validate_access(const struct vgic_dist *dist,
-				 const struct kvm_mmio_range *range,
+				 const struct vgic_io_range *range,
 				 unsigned long offset)
 {
 	int irq;
@@ -757,7 +757,7 @@ static bool vgic_validate_access(const struct vgic_dist *dist,
 static bool call_range_handler(struct kvm_vcpu *vcpu,
 			       struct kvm_exit_mmio *mmio,
 			       unsigned long offset,
-			       const struct kvm_mmio_range *range)
+			       const struct vgic_io_range *range)
 {
 	u32 *data32 = (void *)mmio->data;
 	struct kvm_exit_mmio mmio32;
@@ -804,10 +804,10 @@ static bool call_range_handler(struct kvm_vcpu *vcpu,
  */
 bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
 			    struct kvm_exit_mmio *mmio,
-			    const struct kvm_mmio_range *ranges,
+			    const struct vgic_io_range *ranges,
 			    unsigned long mmio_base)
 {
-	const struct kvm_mmio_range *range;
+	const struct vgic_io_range *range;
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
 	bool updated_state;
 	unsigned long offset;
@@ -1984,7 +1984,7 @@ int vgic_get_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 	return r;
 }
 
-int vgic_has_attr_regs(const struct kvm_mmio_range *ranges, phys_addr_t offset)
+int vgic_has_attr_regs(const struct vgic_io_range *ranges, phys_addr_t offset)
 {
 	struct kvm_exit_mmio dev_attr_mmio;
 
diff --git a/virt/kvm/arm/vgic.h b/virt/kvm/arm/vgic.h
index 1e5a381..6fccb96 100644
--- a/virt/kvm/arm/vgic.h
+++ b/virt/kvm/arm/vgic.h
@@ -74,7 +74,7 @@ void mmio_data_write(struct kvm_exit_mmio *mmio, u32 mask, u32 value)
 	*((u32 *)mmio->data) = cpu_to_le32(value) & mask;
 }
 
-struct kvm_mmio_range {
+struct vgic_io_range {
 	phys_addr_t base;
 	unsigned long len;
 	int bits_per_irq;
@@ -89,13 +89,13 @@ static inline bool is_in_range(phys_addr_t addr, unsigned long len,
 }
 
 const
-struct kvm_mmio_range *vgic_find_range(const struct kvm_mmio_range *ranges,
-				       struct kvm_exit_mmio *mmio,
-				       phys_addr_t offset);
+struct vgic_io_range *vgic_find_range(const struct vgic_io_range *ranges,
+				      struct kvm_exit_mmio *mmio,
+				      phys_addr_t offset);
 
 bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
 			    struct kvm_exit_mmio *mmio,
-			    const struct kvm_mmio_range *ranges,
+			    const struct vgic_io_range *ranges,
 			    unsigned long mmio_base);
 
 bool vgic_handle_enable_reg(struct kvm *kvm, struct kvm_exit_mmio *mmio,
@@ -120,7 +120,7 @@ bool vgic_handle_cfg_reg(u32 *reg, struct kvm_exit_mmio *mmio,
 
 void vgic_kick_vcpus(struct kvm *kvm);
 
-int vgic_has_attr_regs(const struct kvm_mmio_range *ranges, phys_addr_t offset);
+int vgic_has_attr_regs(const struct vgic_io_range *ranges, phys_addr_t offset);
 int vgic_set_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr);
 int vgic_get_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 06/11] KVM: arm/arm64: simplify vgic_find_range() and callers
  2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
                   ` (4 preceding siblings ...)
  2015-03-26 14:39 ` [PATCH v3 05/11] KVM: arm/arm64: rename struct kvm_mmio_range to vgic_io_range Andre Przywara
@ 2015-03-26 14:39 ` Andre Przywara
  2015-03-26 14:39 ` [PATCH v3 07/11] KVM: arm/arm64: implement kvm_io_bus MMIO handling for the VGIC Andre Przywara
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

The vgic_find_range() function in vgic.c takes a struct kvm_exit_mmio
argument, but actually only used the length field in there. Since we
need to get rid of that structure in that part of the code anyway,
let's rework the function (and it's callers) to pass the length
argument to the function directly.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 virt/kvm/arm/vgic-v2-emul.c |    2 +-
 virt/kvm/arm/vgic.c         |   22 ++++++++--------------
 virt/kvm/arm/vgic.h         |    3 +--
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
index ddb3135..1dd183e 100644
--- a/virt/kvm/arm/vgic-v2-emul.c
+++ b/virt/kvm/arm/vgic-v2-emul.c
@@ -715,7 +715,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 	default:
 		BUG();
 	}
-	r = vgic_find_range(ranges, &mmio, offset);
+	r = vgic_find_range(ranges, 4, offset);
 
 	if (unlikely(!r || !r->handle_mmio)) {
 		ret = -ENXIO;
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 21a3550..8802ad7 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -713,16 +713,13 @@ void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
 
 const
 struct vgic_io_range *vgic_find_range(const struct vgic_io_range *ranges,
-				      struct kvm_exit_mmio *mmio,
-				      phys_addr_t offset)
+				      int len, gpa_t offset)
 {
-	const struct vgic_io_range *r = ranges;
-
-	while (r->len) {
-		if (offset >= r->base &&
-		    (offset + mmio->len) <= (r->base + r->len))
-			return r;
-		r++;
+	while (ranges->len) {
+		if (offset >= ranges->base &&
+		    (offset + len) <= (ranges->base + ranges->len))
+			return ranges;
+		ranges++;
 	}
 
 	return NULL;
@@ -813,7 +810,7 @@ bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	unsigned long offset;
 
 	offset = mmio->phys_addr - mmio_base;
-	range = vgic_find_range(ranges, mmio, offset);
+	range = vgic_find_range(ranges, mmio->len, offset);
 	if (unlikely(!range || !range->handle_mmio)) {
 		pr_warn("Unhandled access %d %08llx %d\n",
 			mmio->is_write, mmio->phys_addr, mmio->len);
@@ -1986,10 +1983,7 @@ int vgic_get_common_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
 
 int vgic_has_attr_regs(const struct vgic_io_range *ranges, phys_addr_t offset)
 {
-	struct kvm_exit_mmio dev_attr_mmio;
-
-	dev_attr_mmio.len = 4;
-	if (vgic_find_range(ranges, &dev_attr_mmio, offset))
+	if (vgic_find_range(ranges, 4, offset))
 		return 0;
 	else
 		return -ENXIO;
diff --git a/virt/kvm/arm/vgic.h b/virt/kvm/arm/vgic.h
index 6fccb96..01aa622 100644
--- a/virt/kvm/arm/vgic.h
+++ b/virt/kvm/arm/vgic.h
@@ -90,8 +90,7 @@ static inline bool is_in_range(phys_addr_t addr, unsigned long len,
 
 const
 struct vgic_io_range *vgic_find_range(const struct vgic_io_range *ranges,
-				      struct kvm_exit_mmio *mmio,
-				      phys_addr_t offset);
+				      int len, gpa_t offset);
 
 bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
 			    struct kvm_exit_mmio *mmio,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 07/11] KVM: arm/arm64: implement kvm_io_bus MMIO handling for the VGIC
  2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
                   ` (5 preceding siblings ...)
  2015-03-26 14:39 ` [PATCH v3 06/11] KVM: arm/arm64: simplify vgic_find_range() and callers Andre Przywara
@ 2015-03-26 14:39 ` Andre Przywara
  2015-03-27 16:00   ` Christoffer Dall
  2015-03-26 14:39 ` [PATCH v3 08/11] KVM: arm/arm64: prepare GICv2 emulation to be handled by kvm_io_bus Andre Przywara
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

Currently we use a lot of VGIC specific code to do the MMIO
dispatching.
Use the previous reworks to add kvm_io_bus style MMIO handlers.

Those are not yet called by the MMIO abort handler, also the actual
VGIC emulator function do not make use of it yet, but will be enabled
with the following patches.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/kvm/arm_vgic.h |    9 ++++
 virt/kvm/arm/vgic.c    |  129 ++++++++++++++++++++++++++++++++++++++++++++++++
 virt/kvm/arm/vgic.h    |    7 +++
 3 files changed, 145 insertions(+)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 9092fad..f90140c 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -24,6 +24,7 @@
 #include <linux/irqreturn.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
+#include <kvm/iodev.h>
 
 #define VGIC_NR_IRQS_LEGACY	256
 #define VGIC_NR_SGIS		16
@@ -147,6 +148,14 @@ struct vgic_vm_ops {
 	int	(*map_resources)(struct kvm *, const struct vgic_params *);
 };
 
+struct vgic_io_device {
+	gpa_t addr;
+	int len;
+	const struct vgic_io_range *reg_ranges;
+	struct kvm_vcpu *redist_vcpu;
+	struct kvm_io_device dev;
+};
+
 struct vgic_dist {
 	spinlock_t		lock;
 	bool			in_kernel;
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 8802ad7..e968179 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -32,6 +32,8 @@
 #include <asm/kvm_arm.h>
 #include <asm/kvm_mmu.h>
 #include <trace/events/kvm.h>
+#include <asm/kvm.h>
+#include <kvm/iodev.h>
 
 /*
  * How the whole thing works (courtesy of Christoffer Dall):
@@ -837,6 +839,66 @@ bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
 }
 
 /**
+ * vgic_handle_mmio_access - handle an in-kernel MMIO access
+ * This is called by the read/write KVM IO device wrappers below.
+ * @vcpu:	pointer to the vcpu performing the access
+ * @this:	pointer to the KVM IO device in charge
+ * @addr:	guest physical address of the access
+ * @len:	size of the access
+ * @val:	pointer to the data region
+ * @is_write:	read or write access
+ *
+ * returns true if the MMIO access could be performed
+ */
+static int vgic_handle_mmio_access(struct kvm_vcpu *vcpu,
+				   struct kvm_io_device *this, gpa_t addr,
+				   int len, void *val, bool is_write)
+{
+	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+	struct vgic_io_device *iodev = container_of(this,
+						    struct vgic_io_device, dev);
+	struct kvm_run *run = vcpu->run;
+	const struct vgic_io_range *range;
+	struct kvm_exit_mmio mmio;
+	bool updated_state;
+	gpa_t offset;
+
+	offset = addr - iodev->addr;
+	range = vgic_find_range(iodev->reg_ranges, len, offset);
+	if (unlikely(!range || !range->handle_mmio)) {
+		pr_warn("Unhandled access %d %08llx %d\n", is_write, addr, len);
+		return -ENXIO;
+	}
+
+	mmio.phys_addr = addr;
+	mmio.len = len;
+	mmio.is_write = is_write;
+	if (is_write)
+		memcpy(mmio.data, val, len);
+	mmio.private = iodev->redist_vcpu;
+
+	spin_lock(&dist->lock);
+	offset -= range->base;
+	if (vgic_validate_access(dist, range, offset)) {
+		updated_state = call_range_handler(vcpu, &mmio, offset, range);
+		if (!is_write)
+			memcpy(val, mmio.data, len);
+	} else {
+		if (!is_write)
+			memset(val, 0, len);
+		updated_state = false;
+	}
+	spin_unlock(&dist->lock);
+	kvm_prepare_mmio(run, &mmio);
+	kvm_handle_mmio_return(vcpu, run);
+
+	if (updated_state)
+		vgic_kick_vcpus(vcpu->kvm);
+
+	return 0;
+}
+
+/**
  * vgic_handle_mmio - handle an in-kernel MMIO access for the GIC emulation
  * @vcpu:      pointer to the vcpu performing the access
  * @run:       pointer to the kvm_run structure
@@ -860,6 +922,73 @@ bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	return vcpu->kvm->arch.vgic.vm_ops.handle_mmio(vcpu, run, mmio);
 }
 
+static int vgic_handle_mmio_read(struct kvm_vcpu *vcpu,
+				 struct kvm_io_device *this,
+				 gpa_t addr, int len, void *val)
+{
+	return vgic_handle_mmio_access(vcpu, this, addr, len, val, false);
+}
+
+static int vgic_handle_mmio_write(struct kvm_vcpu *vcpu,
+				  struct kvm_io_device *this,
+				  gpa_t addr, int len, const void *val)
+{
+	return vgic_handle_mmio_access(vcpu, this, addr, len, (void *)val,
+				       true);
+}
+
+struct kvm_io_device_ops vgic_io_ops = {
+	.read	= vgic_handle_mmio_read,
+	.write	= vgic_handle_mmio_write,
+};
+
+/**
+ * vgic_register_kvm_io_dev - register VGIC register frame on the KVM I/O bus
+ * @kvm:            The VM structure pointer
+ * @base:           The (guest) base address for the register frame
+ * @len:            Length of the register frame window
+ * @ranges:         Describing the handler functions for each register
+ * @redist_vcpu_id: The VCPU ID to pass on to the handlers on call
+ * @iodev:          Points to memory to be passed on to the handler
+ *
+ * @iodev stores the parameters of this function to be usable by the handler
+ * respectively the dispatcher function (since the KVM I/O bus framework lacks
+ * an opaque parameter). Initialization is done in this function, but the
+ * reference should be valid and unique for the whole VGIC lifetime.
+ * If the register frame is not mapped for a specific VCPU, pass -1 to
+ * @redist_vcpu_id.
+ */
+int vgic_register_kvm_io_dev(struct kvm *kvm, gpa_t base, int len,
+			     const struct vgic_io_range *ranges,
+			     int redist_vcpu_id,
+			     struct vgic_io_device *iodev)
+{
+	struct kvm_vcpu *vcpu = NULL;
+	int ret;
+
+	if (redist_vcpu_id >= 0)
+		vcpu = kvm_get_vcpu(kvm, redist_vcpu_id);
+
+	iodev->addr		= base;
+	iodev->len		= len;
+	iodev->reg_ranges	= ranges;
+	iodev->redist_vcpu	= vcpu;
+
+	kvm_iodevice_init(&iodev->dev, &vgic_io_ops);
+
+	mutex_lock(&kvm->slots_lock);
+
+	ret = kvm_io_bus_register_dev(kvm, KVM_MMIO_BUS, base, len,
+				      &iodev->dev);
+	mutex_unlock(&kvm->slots_lock);
+
+	/* Mark the iodev as invalid if registration fails. */
+	if (ret)
+		iodev->dev.ops = NULL;
+
+	return ret;
+}
+
 static int vgic_nr_shared_irqs(struct vgic_dist *dist)
 {
 	return dist->nr_irqs - VGIC_NR_PRIVATE_IRQS;
diff --git a/virt/kvm/arm/vgic.h b/virt/kvm/arm/vgic.h
index 01aa622..28fa3aa 100644
--- a/virt/kvm/arm/vgic.h
+++ b/virt/kvm/arm/vgic.h
@@ -20,6 +20,8 @@
 #ifndef __KVM_VGIC_H__
 #define __KVM_VGIC_H__
 
+#include <kvm/iodev.h>
+
 #define VGIC_ADDR_UNDEF		(-1)
 #define IS_VGIC_ADDR_UNDEF(_x)  ((_x) == VGIC_ADDR_UNDEF)
 
@@ -82,6 +84,11 @@ struct vgic_io_range {
 			    phys_addr_t offset);
 };
 
+int vgic_register_kvm_io_dev(struct kvm *kvm, gpa_t base, int len,
+			     const struct vgic_io_range *ranges,
+			     int redist_id,
+			     struct vgic_io_device *iodev);
+
 static inline bool is_in_range(phys_addr_t addr, unsigned long len,
 			       phys_addr_t baseaddr, unsigned long size)
 {
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 08/11] KVM: arm/arm64: prepare GICv2 emulation to be handled by kvm_io_bus
  2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
                   ` (6 preceding siblings ...)
  2015-03-26 14:39 ` [PATCH v3 07/11] KVM: arm/arm64: implement kvm_io_bus MMIO handling for the VGIC Andre Przywara
@ 2015-03-26 14:39 ` Andre Przywara
  2015-03-26 21:46   ` Marc Zyngier
  2015-03-27 16:01   ` Christoffer Dall
  2015-03-26 14:39 ` [PATCH v3 09/11] KVM: arm/arm64: merge GICv3 RD_base and SGI_base register frames Andre Przywara
                   ` (2 subsequent siblings)
  10 siblings, 2 replies; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

Using the framework provided by the recent vgic.c changes we register
a kvm_io_bus device when initializing the virtual GICv2.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/kvm/arm_vgic.h      |    1 +
 virt/kvm/arm/vgic-v2-emul.c |   22 ++++++++++++++++------
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index f90140c..4523984 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -251,6 +251,7 @@ struct vgic_dist {
 	unsigned long		*irq_active_on_cpu;
 
 	struct vgic_vm_ops	vm_ops;
+	struct vgic_io_device	dist_iodev;
 };
 
 struct vgic_v2_cpu_if {
diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
index 1dd183e..7460b37 100644
--- a/virt/kvm/arm/vgic-v2-emul.c
+++ b/virt/kvm/arm/vgic-v2-emul.c
@@ -506,6 +506,7 @@ static bool vgic_v2_queue_sgi(struct kvm_vcpu *vcpu, int irq)
 static int vgic_v2_map_resources(struct kvm *kvm,
 				 const struct vgic_params *params)
 {
+	struct vgic_dist *dist = &kvm->arch.vgic;
 	int ret = 0;
 
 	if (!irqchip_in_kernel(kvm))
@@ -516,13 +517,17 @@ static int vgic_v2_map_resources(struct kvm *kvm,
 	if (vgic_ready(kvm))
 		goto out;
 
-	if (IS_VGIC_ADDR_UNDEF(kvm->arch.vgic.vgic_dist_base) ||
-	    IS_VGIC_ADDR_UNDEF(kvm->arch.vgic.vgic_cpu_base)) {
+	if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base) ||
+	    IS_VGIC_ADDR_UNDEF(dist->vgic_cpu_base)) {
 		kvm_err("Need to set vgic cpu and dist addresses first\n");
 		ret = -ENXIO;
 		goto out;
 	}
 
+	vgic_register_kvm_io_dev(kvm, dist->vgic_dist_base,
+				 KVM_VGIC_V2_DIST_SIZE,
+				 vgic_dist_ranges, -1, &dist->dist_iodev);
+
 	/*
 	 * Initialize the vgic if this hasn't already been done on demand by
 	 * accessing the vgic state from userspace.
@@ -530,18 +535,23 @@ static int vgic_v2_map_resources(struct kvm *kvm,
 	ret = vgic_init(kvm);
 	if (ret) {
 		kvm_err("Unable to allocate maps\n");
-		goto out;
+		goto out_unregister;
 	}
 
-	ret = kvm_phys_addr_ioremap(kvm, kvm->arch.vgic.vgic_cpu_base,
+	ret = kvm_phys_addr_ioremap(kvm, dist->vgic_cpu_base,
 				    params->vcpu_base, KVM_VGIC_V2_CPU_SIZE,
 				    true);
 	if (ret) {
 		kvm_err("Unable to remap VGIC CPU to VCPU\n");
-		goto out;
+		goto out_unregister;
 	}
 
-	kvm->arch.vgic.ready = true;
+	dist->ready = true;
+	goto out;
+
+out_unregister:
+	kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, &dist->dist_iodev.dev);
+
 out:
 	if (ret)
 		kvm_vgic_destroy(kvm);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 09/11] KVM: arm/arm64: merge GICv3 RD_base and SGI_base register frames
  2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
                   ` (7 preceding siblings ...)
  2015-03-26 14:39 ` [PATCH v3 08/11] KVM: arm/arm64: prepare GICv2 emulation to be handled by kvm_io_bus Andre Przywara
@ 2015-03-26 14:39 ` Andre Przywara
  2015-03-26 21:53   ` Marc Zyngier
  2015-03-27 16:01   ` Christoffer Dall
  2015-03-26 14:39 ` [PATCH v3 10/11] KVM: arm/arm64: prepare GICv3 emulation to use kvm_io_bus MMIO handling Andre Przywara
  2015-03-26 14:39 ` [PATCH v3 11/11] KVM: arm/arm64: rework MMIO abort handling to use KVM MMIO bus Andre Przywara
  10 siblings, 2 replies; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

Currently we handle the redistributor registers in two separate MMIO
regions, one for the overall behaviour and SPIs and one for the
SGIs/PPIs. That latter forces the creation of _two_ KVM I/O bus
devices for each redistributor.
Since the spec mandates those two pages to be contigious, we could as
well merge them and save the churn with the second KVM I/O bus device.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 virt/kvm/arm/vgic-v3-emul.c |  174 +++++++++++++++++++++----------------------
 1 file changed, 83 insertions(+), 91 deletions(-)

diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c
index 14943e3..2f03a36 100644
--- a/virt/kvm/arm/vgic-v3-emul.c
+++ b/virt/kvm/arm/vgic-v3-emul.c
@@ -502,6 +502,43 @@ static const struct vgic_io_range vgic_v3_dist_ranges[] = {
 	{},
 };
 
+static bool handle_mmio_ctlr_redist(struct kvm_vcpu *vcpu,
+				    struct kvm_exit_mmio *mmio,
+				    phys_addr_t offset)
+{
+	/* since we don't support LPIs, this register is zero for now */
+	vgic_reg_access(mmio, NULL, offset,
+			ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
+	return false;
+}
+
+static bool handle_mmio_typer_redist(struct kvm_vcpu *vcpu,
+				     struct kvm_exit_mmio *mmio,
+				     phys_addr_t offset)
+{
+	u32 reg;
+	u64 mpidr;
+	struct kvm_vcpu *redist_vcpu = mmio->private;
+	int target_vcpu_id = redist_vcpu->vcpu_id;
+
+	/* the upper 32 bits contain the affinity value */
+	if ((offset & ~3) == 4) {
+		mpidr = kvm_vcpu_get_mpidr_aff(redist_vcpu);
+		reg = compress_mpidr(mpidr);
+
+		vgic_reg_access(mmio, &reg, offset,
+				ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
+		return false;
+	}
+
+	reg = redist_vcpu->vcpu_id << 8;
+	if (target_vcpu_id == atomic_read(&vcpu->kvm->online_vcpus) - 1)
+		reg |= GICR_TYPER_LAST;
+	vgic_reg_access(mmio, &reg, offset,
+			ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
+	return false;
+}
+
 static bool handle_mmio_set_enable_reg_redist(struct kvm_vcpu *vcpu,
 					      struct kvm_exit_mmio *mmio,
 					      phys_addr_t offset)
@@ -570,146 +607,107 @@ static bool handle_mmio_cfg_reg_redist(struct kvm_vcpu *vcpu,
 	return vgic_handle_cfg_reg(reg, mmio, offset);
 }
 
-static const struct vgic_io_range vgic_redist_sgi_ranges[] = {
+#define SGI_base(x) ((x) + SZ_64K)
+
+static const struct vgic_io_range vgic_redist_ranges[] = {
+	{
+		.base           = GICR_CTLR,
+		.len            = 0x04,
+		.bits_per_irq   = 0,
+		.handle_mmio    = handle_mmio_ctlr_redist,
+	},
 	{
-		.base		= GICR_IGROUPR0,
+		.base           = GICR_TYPER,
+		.len            = 0x08,
+		.bits_per_irq   = 0,
+		.handle_mmio    = handle_mmio_typer_redist,
+	},
+	{
+		.base           = GICR_IIDR,
+		.len            = 0x04,
+		.bits_per_irq   = 0,
+		.handle_mmio    = handle_mmio_iidr,
+	},
+	{
+		.base           = GICR_WAKER,
+		.len            = 0x04,
+		.bits_per_irq   = 0,
+		.handle_mmio    = handle_mmio_raz_wi,
+	},
+	{
+		.base           = GICR_IDREGS,
+		.len            = 0x30,
+		.bits_per_irq   = 0,
+		.handle_mmio    = handle_mmio_idregs,
+	},
+	{
+		.base		= SGI_base(GICR_IGROUPR0),
 		.len		= 0x04,
 		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_rao_wi,
 	},
 	{
-		.base		= GICR_ISENABLER0,
+		.base		= SGI_base(GICR_ISENABLER0),
 		.len		= 0x04,
 		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_set_enable_reg_redist,
 	},
 	{
-		.base		= GICR_ICENABLER0,
+		.base		= SGI_base(GICR_ICENABLER0),
 		.len		= 0x04,
 		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_clear_enable_reg_redist,
 	},
 	{
-		.base		= GICR_ISPENDR0,
+		.base		= SGI_base(GICR_ISPENDR0),
 		.len		= 0x04,
 		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_set_pending_reg_redist,
 	},
 	{
-		.base		= GICR_ICPENDR0,
+		.base		= SGI_base(GICR_ICPENDR0),
 		.len		= 0x04,
 		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_clear_pending_reg_redist,
 	},
 	{
-		.base		= GICR_ISACTIVER0,
+		.base		= SGI_base(GICR_ISACTIVER0),
 		.len		= 0x04,
 		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_raz_wi,
 	},
 	{
-		.base		= GICR_ICACTIVER0,
+		.base		= SGI_base(GICR_ICACTIVER0),
 		.len		= 0x04,
 		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_raz_wi,
 	},
 	{
-		.base		= GICR_IPRIORITYR0,
+		.base		= SGI_base(GICR_IPRIORITYR0),
 		.len		= 0x20,
 		.bits_per_irq	= 8,
 		.handle_mmio	= handle_mmio_priority_reg_redist,
 	},
 	{
-		.base		= GICR_ICFGR0,
+		.base		= SGI_base(GICR_ICFGR0),
 		.len		= 0x08,
 		.bits_per_irq	= 2,
 		.handle_mmio	= handle_mmio_cfg_reg_redist,
 	},
 	{
-		.base		= GICR_IGRPMODR0,
+		.base		= SGI_base(GICR_IGRPMODR0),
 		.len		= 0x04,
 		.bits_per_irq	= 1,
 		.handle_mmio	= handle_mmio_raz_wi,
 	},
 	{
-		.base		= GICR_NSACR,
+		.base		= SGI_base(GICR_NSACR),
 		.len		= 0x04,
 		.handle_mmio	= handle_mmio_raz_wi,
 	},
 	{},
 };
 
-static bool handle_mmio_ctlr_redist(struct kvm_vcpu *vcpu,
-				    struct kvm_exit_mmio *mmio,
-				    phys_addr_t offset)
-{
-	/* since we don't support LPIs, this register is zero for now */
-	vgic_reg_access(mmio, NULL, offset,
-			ACCESS_READ_RAZ | ACCESS_WRITE_IGNORED);
-	return false;
-}
-
-static bool handle_mmio_typer_redist(struct kvm_vcpu *vcpu,
-				     struct kvm_exit_mmio *mmio,
-				     phys_addr_t offset)
-{
-	u32 reg;
-	u64 mpidr;
-	struct kvm_vcpu *redist_vcpu = mmio->private;
-	int target_vcpu_id = redist_vcpu->vcpu_id;
-
-	/* the upper 32 bits contain the affinity value */
-	if ((offset & ~3) == 4) {
-		mpidr = kvm_vcpu_get_mpidr_aff(redist_vcpu);
-		reg = compress_mpidr(mpidr);
-
-		vgic_reg_access(mmio, &reg, offset,
-				ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
-		return false;
-	}
-
-	reg = redist_vcpu->vcpu_id << 8;
-	if (target_vcpu_id == atomic_read(&vcpu->kvm->online_vcpus) - 1)
-		reg |= GICR_TYPER_LAST;
-	vgic_reg_access(mmio, &reg, offset,
-			ACCESS_READ_VALUE | ACCESS_WRITE_IGNORED);
-	return false;
-}
-
-static const struct vgic_io_range vgic_redist_ranges[] = {
-	{
-		.base           = GICR_CTLR,
-		.len            = 0x04,
-		.bits_per_irq   = 0,
-		.handle_mmio    = handle_mmio_ctlr_redist,
-	},
-	{
-		.base           = GICR_TYPER,
-		.len            = 0x08,
-		.bits_per_irq   = 0,
-		.handle_mmio    = handle_mmio_typer_redist,
-	},
-	{
-		.base           = GICR_IIDR,
-		.len            = 0x04,
-		.bits_per_irq   = 0,
-		.handle_mmio    = handle_mmio_iidr,
-	},
-	{
-		.base           = GICR_WAKER,
-		.len            = 0x04,
-		.bits_per_irq   = 0,
-		.handle_mmio    = handle_mmio_raz_wi,
-	},
-	{
-		.base           = GICR_IDREGS,
-		.len            = 0x30,
-		.bits_per_irq   = 0,
-		.handle_mmio    = handle_mmio_idregs,
-	},
-	{},
-};
-
 /*
  * This function splits accesses between the distributor and the two
  * redistributor parts (private/SPI). As each redistributor is accessible
@@ -726,7 +724,6 @@ static bool vgic_v3_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	unsigned long rdbase = dist->vgic_redist_base;
 	int nrcpus = atomic_read(&vcpu->kvm->online_vcpus);
 	int vcpu_id;
-	const struct vgic_io_range *mmio_range;
 
 	if (is_in_range(mmio->phys_addr, mmio->len, dbase, GIC_V3_DIST_SIZE)) {
 		return vgic_handle_mmio_range(vcpu, run, mmio,
@@ -741,13 +738,8 @@ static bool vgic_v3_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	rdbase += (vcpu_id * GIC_V3_REDIST_SIZE);
 	mmio->private = kvm_get_vcpu(vcpu->kvm, vcpu_id);
 
-	if (mmio->phys_addr >= rdbase + SGI_BASE_OFFSET) {
-		rdbase += SGI_BASE_OFFSET;
-		mmio_range = vgic_redist_sgi_ranges;
-	} else {
-		mmio_range = vgic_redist_ranges;
-	}
-	return vgic_handle_mmio_range(vcpu, run, mmio, mmio_range, rdbase);
+	return vgic_handle_mmio_range(vcpu, run, mmio, vgic_redist_ranges,
+				      rdbase);
 }
 
 static bool vgic_v3_queue_sgi(struct kvm_vcpu *vcpu, int irq)
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 10/11] KVM: arm/arm64: prepare GICv3 emulation to use kvm_io_bus MMIO handling
  2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
                   ` (8 preceding siblings ...)
  2015-03-26 14:39 ` [PATCH v3 09/11] KVM: arm/arm64: merge GICv3 RD_base and SGI_base register frames Andre Przywara
@ 2015-03-26 14:39 ` Andre Przywara
  2015-03-26 22:06   ` Marc Zyngier
  2015-03-27 16:01   ` Christoffer Dall
  2015-03-26 14:39 ` [PATCH v3 11/11] KVM: arm/arm64: rework MMIO abort handling to use KVM MMIO bus Andre Przywara
  10 siblings, 2 replies; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

Using the framework provided by the recent vgic.c changes, we
register a kvm_io_bus device on mapping the virtual GICv3 resources.
The distributor mapping is pretty straight forward, but the
redistributors need some more love, since they need to be tagged with
the respective redistributor (read: VCPU) they are connected with.
We use the kvm_io_bus framework to register one devices per VCPU.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/kvm/arm_vgic.h      |    1 +
 virt/kvm/arm/vgic-v3-emul.c |   39 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 4523984..d6705f4 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -252,6 +252,7 @@ struct vgic_dist {
 
 	struct vgic_vm_ops	vm_ops;
 	struct vgic_io_device	dist_iodev;
+	struct vgic_io_device	*redist_iodevs;
 };
 
 struct vgic_v2_cpu_if {
diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c
index 2f03a36..eb1a797 100644
--- a/virt/kvm/arm/vgic-v3-emul.c
+++ b/virt/kvm/arm/vgic-v3-emul.c
@@ -758,6 +758,9 @@ static int vgic_v3_map_resources(struct kvm *kvm,
 {
 	int ret = 0;
 	struct vgic_dist *dist = &kvm->arch.vgic;
+	gpa_t rdbase = dist->vgic_redist_base;
+	struct vgic_io_device *iodevs = NULL;
+	int i;
 
 	if (!irqchip_in_kernel(kvm))
 		return 0;
@@ -783,7 +786,41 @@ static int vgic_v3_map_resources(struct kvm *kvm,
 		goto out;
 	}
 
-	kvm->arch.vgic.ready = true;
+	ret = vgic_register_kvm_io_dev(kvm, dist->vgic_dist_base,
+				       GIC_V3_DIST_SIZE, vgic_v3_dist_ranges,
+				       -1, &dist->dist_iodev);
+	if (ret)
+		goto out;
+
+	iodevs = kcalloc(dist->nr_cpus, sizeof(iodevs[0]), GFP_KERNEL);
+	if (!iodevs) {
+		ret = -ENOMEM;
+		goto out_unregister;
+	}
+
+	for (i = 0; i < dist->nr_cpus; i++) {
+		ret = vgic_register_kvm_io_dev(kvm, rdbase,
+					       SZ_128K, vgic_redist_ranges,
+					       i, &iodevs[i]);
+		if (ret)
+			goto out_unregister;
+		rdbase += GIC_V3_REDIST_SIZE;
+	}
+
+	dist->redist_iodevs = iodevs;
+	dist->ready = true;
+	goto out;
+
+out_unregister:
+	kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, &dist->dist_iodev.dev);
+	if (iodevs) {
+		for (i = 0; i < dist->nr_cpus; i++) {
+			if (iodevs[i].dev.ops)
+				kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS,
+							  &iodevs[i].dev);
+		}
+	}
+
 out:
 	if (ret)
 		kvm_vgic_destroy(kvm);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 11/11] KVM: arm/arm64: rework MMIO abort handling to use KVM MMIO bus
  2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
                   ` (9 preceding siblings ...)
  2015-03-26 14:39 ` [PATCH v3 10/11] KVM: arm/arm64: prepare GICv3 emulation to use kvm_io_bus MMIO handling Andre Przywara
@ 2015-03-26 14:39 ` Andre Przywara
  2015-03-27 16:00   ` Christoffer Dall
  10 siblings, 1 reply; 25+ messages in thread
From: Andre Przywara @ 2015-03-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

Currently we have struct kvm_exit_mmio for encapsulating MMIO abort
data to be passed on from syndrome decoding all the way down to the
VGIC register handlers. Now as we switch the MMIO handling to be
routed through the KVM MMIO bus, it does not make sense anymore to
use that structure already from the beginning. So we put the data into
kvm_run very early and use that encapsulation till the MMIO bus call.
Then we fill kvm_exit_mmio in the VGIC only, making it a VGIC private
structure. On that way we replace the data buffer in that structure
with a pointer pointing to a single location in kvm_run, so we get
rid of some copying on the way.
With all of the virtual GIC emulation code now being registered with
the kvm_io_bus, we can remove all of the old MMIO handling code and
its dispatching functionality.

I didn't bother to rename kvm_exit_mmio (to vgic_mmio or something),
because that touches a lot of code lines without any good reason.

This is based on an original patch by Nikolay.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
---
 arch/arm/include/asm/kvm_mmio.h   |   22 ---------
 arch/arm/kvm/mmio.c               |   60 ++++++++++++++++++-------
 arch/arm64/include/asm/kvm_mmio.h |   22 ---------
 include/kvm/arm_vgic.h            |    6 ---
 virt/kvm/arm/vgic-v2-emul.c       |   21 +--------
 virt/kvm/arm/vgic-v3-emul.c       |   35 ---------------
 virt/kvm/arm/vgic.c               |   89 ++-----------------------------------
 virt/kvm/arm/vgic.h               |   13 +++---
 8 files changed, 57 insertions(+), 211 deletions(-)

diff --git a/arch/arm/include/asm/kvm_mmio.h b/arch/arm/include/asm/kvm_mmio.h
index 3f83db2..d8e90c8 100644
--- a/arch/arm/include/asm/kvm_mmio.h
+++ b/arch/arm/include/asm/kvm_mmio.h
@@ -28,28 +28,6 @@ struct kvm_decode {
 	bool sign_extend;
 };
 
-/*
- * The in-kernel MMIO emulation code wants to use a copy of run->mmio,
- * which is an anonymous type. Use our own type instead.
- */
-struct kvm_exit_mmio {
-	phys_addr_t	phys_addr;
-	u8		data[8];
-	u32		len;
-	bool		is_write;
-	void		*private;
-};
-
-static inline void kvm_prepare_mmio(struct kvm_run *run,
-				    struct kvm_exit_mmio *mmio)
-{
-	run->mmio.phys_addr	= mmio->phys_addr;
-	run->mmio.len		= mmio->len;
-	run->mmio.is_write	= mmio->is_write;
-	memcpy(run->mmio.data, mmio->data, mmio->len);
-	run->exit_reason	= KVM_EXIT_MMIO;
-}
-
 int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run);
 int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		 phys_addr_t fault_ipa);
diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
index 5d3bfc0..bb2ab44 100644
--- a/arch/arm/kvm/mmio.c
+++ b/arch/arm/kvm/mmio.c
@@ -122,7 +122,7 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
 }
 
 static int decode_hsr(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
-		      struct kvm_exit_mmio *mmio)
+		      struct kvm_run *run)
 {
 	unsigned long rt;
 	int len;
@@ -148,9 +148,9 @@ static int decode_hsr(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	sign_extend = kvm_vcpu_dabt_issext(vcpu);
 	rt = kvm_vcpu_dabt_get_rd(vcpu);
 
-	mmio->is_write = is_write;
-	mmio->phys_addr = fault_ipa;
-	mmio->len = len;
+	run->mmio.is_write = is_write;
+	run->mmio.phys_addr = fault_ipa;
+	run->mmio.len = len;
 	vcpu->arch.mmio_decode.sign_extend = sign_extend;
 	vcpu->arch.mmio_decode.rt = rt;
 
@@ -162,23 +162,49 @@ static int decode_hsr(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	return 0;
 }
 
+/**
+ * handle_kernel_mmio - handle an in-kernel MMIO access
+ * @vcpu:	pointer to the vcpu performing the access
+ * @run:	pointer to the kvm_run structure
+ *
+ * returns true if the MMIO access has been performed in kernel space,
+ * and false if it needs to be emulated in user space.
+ */
+static bool handle_kernel_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+	int ret;
+
+	if (run->mmio.is_write) {
+		ret = kvm_io_bus_write(vcpu, KVM_MMIO_BUS, run->mmio.phys_addr,
+				       run->mmio.len, run->mmio.data);
+
+	} else {
+		ret = kvm_io_bus_read(vcpu, KVM_MMIO_BUS, run->mmio.phys_addr,
+				      run->mmio.len, run->mmio.data);
+	}
+	if (!ret) {
+		kvm_handle_mmio_return(vcpu, run);
+		return true;
+	}
+
+	return false;
+}
+
 int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		 phys_addr_t fault_ipa)
 {
-	struct kvm_exit_mmio mmio;
 	unsigned long data;
 	unsigned long rt;
 	int ret;
 
 	/*
-	 * Prepare MMIO operation. First stash it in a private
-	 * structure that we can use for in-kernel emulation. If the
-	 * kernel can't handle it, copy it into run->mmio and let user
-	 * space do its magic.
+	 * Prepare MMIO operation. First put the MMIO data into run->mmio.
+	 * Then try if some in-kernel emulation feels responsible, otherwise
+	 * let user space do its magic.
 	 */
 
 	if (kvm_vcpu_dabt_isvalid(vcpu)) {
-		ret = decode_hsr(vcpu, fault_ipa, &mmio);
+		ret = decode_hsr(vcpu, fault_ipa, run);
 		if (ret)
 			return ret;
 	} else {
@@ -188,21 +214,21 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 
 	rt = vcpu->arch.mmio_decode.rt;
 
-	if (mmio.is_write) {
+	if (run->mmio.is_write) {
 		data = vcpu_data_guest_to_host(vcpu, *vcpu_reg(vcpu, rt),
-					       mmio.len);
+					       run->mmio.len);
 
-		trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, mmio.len,
+		trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, run->mmio.len,
 			       fault_ipa, data);
-		mmio_write_buf(mmio.data, mmio.len, data);
+		mmio_write_buf(run->mmio.data, run->mmio.len, data);
 	} else {
-		trace_kvm_mmio(KVM_TRACE_MMIO_READ_UNSATISFIED, mmio.len,
+		trace_kvm_mmio(KVM_TRACE_MMIO_READ_UNSATISFIED, run->mmio.len,
 			       fault_ipa, 0);
 	}
 
-	if (vgic_handle_mmio(vcpu, run, &mmio))
+	if (handle_kernel_mmio(vcpu, run))
 		return 1;
 
-	kvm_prepare_mmio(run, &mmio);
+	run->exit_reason = KVM_EXIT_MMIO;
 	return 0;
 }
diff --git a/arch/arm64/include/asm/kvm_mmio.h b/arch/arm64/include/asm/kvm_mmio.h
index 9f52beb..889c908 100644
--- a/arch/arm64/include/asm/kvm_mmio.h
+++ b/arch/arm64/include/asm/kvm_mmio.h
@@ -31,28 +31,6 @@ struct kvm_decode {
 	bool sign_extend;
 };
 
-/*
- * The in-kernel MMIO emulation code wants to use a copy of run->mmio,
- * which is an anonymous type. Use our own type instead.
- */
-struct kvm_exit_mmio {
-	phys_addr_t	phys_addr;
-	u8		data[8];
-	u32		len;
-	bool		is_write;
-	void		*private;
-};
-
-static inline void kvm_prepare_mmio(struct kvm_run *run,
-				    struct kvm_exit_mmio *mmio)
-{
-	run->mmio.phys_addr	= mmio->phys_addr;
-	run->mmio.len		= mmio->len;
-	run->mmio.is_write	= mmio->is_write;
-	memcpy(run->mmio.data, mmio->data, mmio->len);
-	run->exit_reason	= KVM_EXIT_MMIO;
-}
-
 int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run);
 int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		 phys_addr_t fault_ipa);
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index d6705f4..16ec2c8 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -140,8 +140,6 @@ struct vgic_params {
 };
 
 struct vgic_vm_ops {
-	bool	(*handle_mmio)(struct kvm_vcpu *, struct kvm_run *,
-			       struct kvm_exit_mmio *);
 	bool	(*queue_sgi)(struct kvm_vcpu *, int irq);
 	void	(*add_sgi_source)(struct kvm_vcpu *, int irq, int source);
 	int	(*init_model)(struct kvm *);
@@ -313,8 +311,6 @@ struct vgic_cpu {
 
 struct kvm;
 struct kvm_vcpu;
-struct kvm_run;
-struct kvm_exit_mmio;
 
 int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write);
 int kvm_vgic_hyp_init(void);
@@ -330,8 +326,6 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
 void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
 int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
 int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
-bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
-		      struct kvm_exit_mmio *mmio);
 
 #define irqchip_in_kernel(k)	(!!((k)->arch.vgic.in_kernel))
 #define vgic_initialized(k)	(!!((k)->arch.vgic.nr_cpus))
diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
index 7460b37..1390797 100644
--- a/virt/kvm/arm/vgic-v2-emul.c
+++ b/virt/kvm/arm/vgic-v2-emul.c
@@ -404,24 +404,6 @@ static const struct vgic_io_range vgic_dist_ranges[] = {
 	{}
 };
 
-static bool vgic_v2_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
-				struct kvm_exit_mmio *mmio)
-{
-	unsigned long base = vcpu->kvm->arch.vgic.vgic_dist_base;
-
-	if (!is_in_range(mmio->phys_addr, mmio->len, base,
-			 KVM_VGIC_V2_DIST_SIZE))
-		return false;
-
-	/* GICv2 does not support accesses wider than 32 bits */
-	if (mmio->len > 4) {
-		kvm_inject_dabt(vcpu, mmio->phys_addr);
-		return true;
-	}
-
-	return vgic_handle_mmio_range(vcpu, run, mmio, vgic_dist_ranges, base);
-}
-
 static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg)
 {
 	struct kvm *kvm = vcpu->kvm;
@@ -580,7 +562,6 @@ void vgic_v2_init_emulation(struct kvm *kvm)
 {
 	struct vgic_dist *dist = &kvm->arch.vgic;
 
-	dist->vm_ops.handle_mmio = vgic_v2_handle_mmio;
 	dist->vm_ops.queue_sgi = vgic_v2_queue_sgi;
 	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
 	dist->vm_ops.init_model = vgic_v2_init_model;
@@ -690,6 +671,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 	struct kvm_vcpu *vcpu, *tmp_vcpu;
 	struct vgic_dist *vgic;
 	struct kvm_exit_mmio mmio;
+	u32 data;
 
 	offset = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK;
 	cpuid = (attr->attr & KVM_DEV_ARM_VGIC_CPUID_MASK) >>
@@ -711,6 +693,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 
 	mmio.len = 4;
 	mmio.is_write = is_write;
+	mmio.data = &data;
 	if (is_write)
 		mmio_data_write(&mmio, ~0, *reg);
 	switch (attr->group) {
diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c
index eb1a797..e9c3a7a 100644
--- a/virt/kvm/arm/vgic-v3-emul.c
+++ b/virt/kvm/arm/vgic-v3-emul.c
@@ -708,40 +708,6 @@ static const struct vgic_io_range vgic_redist_ranges[] = {
 	{},
 };
 
-/*
- * This function splits accesses between the distributor and the two
- * redistributor parts (private/SPI). As each redistributor is accessible
- * from any CPU, we have to determine the affected VCPU by taking the faulting
- * address into account. We then pass this VCPU to the handler function via
- * the private parameter.
- */
-#define SGI_BASE_OFFSET SZ_64K
-static bool vgic_v3_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
-				struct kvm_exit_mmio *mmio)
-{
-	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
-	unsigned long dbase = dist->vgic_dist_base;
-	unsigned long rdbase = dist->vgic_redist_base;
-	int nrcpus = atomic_read(&vcpu->kvm->online_vcpus);
-	int vcpu_id;
-
-	if (is_in_range(mmio->phys_addr, mmio->len, dbase, GIC_V3_DIST_SIZE)) {
-		return vgic_handle_mmio_range(vcpu, run, mmio,
-					      vgic_v3_dist_ranges, dbase);
-	}
-
-	if (!is_in_range(mmio->phys_addr, mmio->len, rdbase,
-	    GIC_V3_REDIST_SIZE * nrcpus))
-		return false;
-
-	vcpu_id = (mmio->phys_addr - rdbase) / GIC_V3_REDIST_SIZE;
-	rdbase += (vcpu_id * GIC_V3_REDIST_SIZE);
-	mmio->private = kvm_get_vcpu(vcpu->kvm, vcpu_id);
-
-	return vgic_handle_mmio_range(vcpu, run, mmio, vgic_redist_ranges,
-				      rdbase);
-}
-
 static bool vgic_v3_queue_sgi(struct kvm_vcpu *vcpu, int irq)
 {
 	if (vgic_queue_irq(vcpu, 0, irq)) {
@@ -861,7 +827,6 @@ void vgic_v3_init_emulation(struct kvm *kvm)
 {
 	struct vgic_dist *dist = &kvm->arch.vgic;
 
-	dist->vm_ops.handle_mmio = vgic_v3_handle_mmio;
 	dist->vm_ops.queue_sgi = vgic_v3_queue_sgi;
 	dist->vm_ops.add_sgi_source = vgic_v3_add_sgi_source;
 	dist->vm_ops.init_model = vgic_v3_init_model;
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index e968179..901c130 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -758,7 +758,6 @@ static bool call_range_handler(struct kvm_vcpu *vcpu,
 			       unsigned long offset,
 			       const struct vgic_io_range *range)
 {
-	u32 *data32 = (void *)mmio->data;
 	struct kvm_exit_mmio mmio32;
 	bool ret;
 
@@ -775,70 +774,17 @@ static bool call_range_handler(struct kvm_vcpu *vcpu,
 	mmio32.private = mmio->private;
 
 	mmio32.phys_addr = mmio->phys_addr + 4;
-	if (mmio->is_write)
-		*(u32 *)mmio32.data = data32[1];
+	mmio32.data = &((u32 *)mmio->data)[1];
 	ret = range->handle_mmio(vcpu, &mmio32, offset + 4);
-	if (!mmio->is_write)
-		data32[1] = *(u32 *)mmio32.data;
 
 	mmio32.phys_addr = mmio->phys_addr;
-	if (mmio->is_write)
-		*(u32 *)mmio32.data = data32[0];
+	mmio32.data = &((u32 *)mmio->data)[0];
 	ret |= range->handle_mmio(vcpu, &mmio32, offset);
-	if (!mmio->is_write)
-		data32[0] = *(u32 *)mmio32.data;
 
 	return ret;
 }
 
 /**
- * vgic_handle_mmio_range - handle an in-kernel MMIO access
- * @vcpu:	pointer to the vcpu performing the access
- * @run:	pointer to the kvm_run structure
- * @mmio:	pointer to the data describing the access
- * @ranges:	array of MMIO ranges in a given region
- * @mmio_base:	base address of that region
- *
- * returns true if the MMIO access could be performed
- */
-bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
-			    struct kvm_exit_mmio *mmio,
-			    const struct vgic_io_range *ranges,
-			    unsigned long mmio_base)
-{
-	const struct vgic_io_range *range;
-	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
-	bool updated_state;
-	unsigned long offset;
-
-	offset = mmio->phys_addr - mmio_base;
-	range = vgic_find_range(ranges, mmio->len, offset);
-	if (unlikely(!range || !range->handle_mmio)) {
-		pr_warn("Unhandled access %d %08llx %d\n",
-			mmio->is_write, mmio->phys_addr, mmio->len);
-		return false;
-	}
-
-	spin_lock(&vcpu->kvm->arch.vgic.lock);
-	offset -= range->base;
-	if (vgic_validate_access(dist, range, offset)) {
-		updated_state = call_range_handler(vcpu, mmio, offset, range);
-	} else {
-		if (!mmio->is_write)
-			memset(mmio->data, 0, mmio->len);
-		updated_state = false;
-	}
-	spin_unlock(&vcpu->kvm->arch.vgic.lock);
-	kvm_prepare_mmio(run, mmio);
-	kvm_handle_mmio_return(vcpu, run);
-
-	if (updated_state)
-		vgic_kick_vcpus(vcpu->kvm);
-
-	return true;
-}
-
-/**
  * vgic_handle_mmio_access - handle an in-kernel MMIO access
  * This is called by the read/write KVM IO device wrappers below.
  * @vcpu:	pointer to the vcpu performing the access
@@ -873,23 +819,20 @@ static int vgic_handle_mmio_access(struct kvm_vcpu *vcpu,
 	mmio.phys_addr = addr;
 	mmio.len = len;
 	mmio.is_write = is_write;
-	if (is_write)
-		memcpy(mmio.data, val, len);
+	mmio.data = val;
 	mmio.private = iodev->redist_vcpu;
 
 	spin_lock(&dist->lock);
 	offset -= range->base;
 	if (vgic_validate_access(dist, range, offset)) {
 		updated_state = call_range_handler(vcpu, &mmio, offset, range);
-		if (!is_write)
-			memcpy(val, mmio.data, len);
 	} else {
 		if (!is_write)
 			memset(val, 0, len);
 		updated_state = false;
 	}
 	spin_unlock(&dist->lock);
-	kvm_prepare_mmio(run, &mmio);
+	run->exit_reason = KVM_EXIT_MMIO;
 	kvm_handle_mmio_return(vcpu, run);
 
 	if (updated_state)
@@ -898,30 +841,6 @@ static int vgic_handle_mmio_access(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
-/**
- * vgic_handle_mmio - handle an in-kernel MMIO access for the GIC emulation
- * @vcpu:      pointer to the vcpu performing the access
- * @run:       pointer to the kvm_run structure
- * @mmio:      pointer to the data describing the access
- *
- * returns true if the MMIO access has been performed in kernel space,
- * and false if it needs to be emulated in user space.
- * Calls the actual handling routine for the selected VGIC model.
- */
-bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
-		      struct kvm_exit_mmio *mmio)
-{
-	if (!irqchip_in_kernel(vcpu->kvm))
-		return false;
-
-	/*
-	 * This will currently call either vgic_v2_handle_mmio() or
-	 * vgic_v3_handle_mmio(), which in turn will call
-	 * vgic_handle_mmio_range() defined above.
-	 */
-	return vcpu->kvm->arch.vgic.vm_ops.handle_mmio(vcpu, run, mmio);
-}
-
 static int vgic_handle_mmio_read(struct kvm_vcpu *vcpu,
 				 struct kvm_io_device *this,
 				 gpa_t addr, int len, void *val)
diff --git a/virt/kvm/arm/vgic.h b/virt/kvm/arm/vgic.h
index 28fa3aa..0df74cb 100644
--- a/virt/kvm/arm/vgic.h
+++ b/virt/kvm/arm/vgic.h
@@ -59,6 +59,14 @@ void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
 bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq);
 void vgic_unqueue_irqs(struct kvm_vcpu *vcpu);
 
+struct kvm_exit_mmio {
+	phys_addr_t	phys_addr;
+	void		*data;
+	u32		len;
+	bool		is_write;
+	void		*private;
+};
+
 void vgic_reg_access(struct kvm_exit_mmio *mmio, u32 *reg,
 		     phys_addr_t offset, int mode);
 bool handle_mmio_raz_wi(struct kvm_vcpu *vcpu, struct kvm_exit_mmio *mmio,
@@ -99,11 +107,6 @@ const
 struct vgic_io_range *vgic_find_range(const struct vgic_io_range *ranges,
 				      int len, gpa_t offset);
 
-bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
-			    struct kvm_exit_mmio *mmio,
-			    const struct vgic_io_range *ranges,
-			    unsigned long mmio_base);
-
 bool vgic_handle_enable_reg(struct kvm *kvm, struct kvm_exit_mmio *mmio,
 			    phys_addr_t offset, int vcpu_id, int access);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 08/11] KVM: arm/arm64: prepare GICv2 emulation to be handled by kvm_io_bus
  2015-03-26 14:39 ` [PATCH v3 08/11] KVM: arm/arm64: prepare GICv2 emulation to be handled by kvm_io_bus Andre Przywara
@ 2015-03-26 21:46   ` Marc Zyngier
  2015-03-27 16:01   ` Christoffer Dall
  1 sibling, 0 replies; 25+ messages in thread
From: Marc Zyngier @ 2015-03-26 21:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 26 Mar 2015 14:39:35 +0000
Andre Przywara <andre.przywara@arm.com> wrote:

> Using the framework provided by the recent vgic.c changes we register
> a kvm_io_bus device when initializing the virtual GICv2.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 09/11] KVM: arm/arm64: merge GICv3 RD_base and SGI_base register frames
  2015-03-26 14:39 ` [PATCH v3 09/11] KVM: arm/arm64: merge GICv3 RD_base and SGI_base register frames Andre Przywara
@ 2015-03-26 21:53   ` Marc Zyngier
  2015-03-27  0:14     ` Andre Przywara
  2015-03-27 16:01   ` Christoffer Dall
  1 sibling, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2015-03-26 21:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 26 Mar 2015 14:39:36 +0000
Andre Przywara <andre.przywara@arm.com> wrote:

> Currently we handle the redistributor registers in two separate MMIO
> regions, one for the overall behaviour and SPIs and one for the
> SGIs/PPIs. That latter forces the creation of _two_ KVM I/O bus
> devices for each redistributor.
> Since the spec mandates those two pages to be contigious, we could as
> well merge them and save the churn with the second KVM I/O bus device.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

I wonder why we didn't have it that way the first place...

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 10/11] KVM: arm/arm64: prepare GICv3 emulation to use kvm_io_bus MMIO handling
  2015-03-26 14:39 ` [PATCH v3 10/11] KVM: arm/arm64: prepare GICv3 emulation to use kvm_io_bus MMIO handling Andre Przywara
@ 2015-03-26 22:06   ` Marc Zyngier
  2015-03-27  0:14     ` Andre Przywara
  2015-03-27 16:01   ` Christoffer Dall
  1 sibling, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2015-03-26 22:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 26 Mar 2015 14:39:37 +0000
Andre Przywara <andre.przywara@arm.com> wrote:

> Using the framework provided by the recent vgic.c changes, we
> register a kvm_io_bus device on mapping the virtual GICv3 resources.
> The distributor mapping is pretty straight forward, but the
> redistributors need some more love, since they need to be tagged with
> the respective redistributor (read: VCPU) they are connected with.
> We use the kvm_io_bus framework to register one devices per VCPU.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  include/kvm/arm_vgic.h      |    1 +
>  virt/kvm/arm/vgic-v3-emul.c |   39 ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 4523984..d6705f4 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -252,6 +252,7 @@ struct vgic_dist {
>  
>  	struct vgic_vm_ops	vm_ops;
>  	struct vgic_io_device	dist_iodev;
> +	struct vgic_io_device	*redist_iodevs;
>  };
>  
>  struct vgic_v2_cpu_if {
> diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c
> index 2f03a36..eb1a797 100644
> --- a/virt/kvm/arm/vgic-v3-emul.c
> +++ b/virt/kvm/arm/vgic-v3-emul.c
> @@ -758,6 +758,9 @@ static int vgic_v3_map_resources(struct kvm *kvm,
>  {
>  	int ret = 0;
>  	struct vgic_dist *dist = &kvm->arch.vgic;
> +	gpa_t rdbase = dist->vgic_redist_base;
> +	struct vgic_io_device *iodevs = NULL;
> +	int i;
>  
>  	if (!irqchip_in_kernel(kvm))
>  		return 0;
> @@ -783,7 +786,41 @@ static int vgic_v3_map_resources(struct kvm *kvm,
>  		goto out;
>  	}
>  
> -	kvm->arch.vgic.ready = true;
> +	ret = vgic_register_kvm_io_dev(kvm, dist->vgic_dist_base,
> +				       GIC_V3_DIST_SIZE, vgic_v3_dist_ranges,
> +				       -1, &dist->dist_iodev);

> +	if (ret)
> +		goto out;
> +
> +	iodevs = kcalloc(dist->nr_cpus, sizeof(iodevs[0]), GFP_KERNEL);
> +	if (!iodevs) {
> +		ret = -ENOMEM;
> +		goto out_unregister;
> +	}
> +
> +	for (i = 0; i < dist->nr_cpus; i++) {
> +		ret = vgic_register_kvm_io_dev(kvm, rdbase,
> +					       SZ_128K, vgic_redist_ranges,
> +					       i, &iodevs[i]);

This looks really weird. You seems to be mapping all redistributors at
the same IPA. Have you actually tested this with an SMP guest?

Or maybe I don't get how this works?

	M.
-- 
Jazz is not dead. It just smells funny.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 09/11] KVM: arm/arm64: merge GICv3 RD_base and SGI_base register frames
  2015-03-26 21:53   ` Marc Zyngier
@ 2015-03-27  0:14     ` Andre Przywara
  0 siblings, 0 replies; 25+ messages in thread
From: Andre Przywara @ 2015-03-27  0:14 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/26/2015 09:53 PM, Marc Zyngier wrote:
> On Thu, 26 Mar 2015 14:39:36 +0000
> Andre Przywara <andre.przywara@arm.com> wrote:
> 
>> Currently we handle the redistributor registers in two separate MMIO
>> regions, one for the overall behaviour and SPIs and one for the
>> SGIs/PPIs. That latter forces the creation of _two_ KVM I/O bus
>> devices for each redistributor.
>> Since the spec mandates those two pages to be contigious, we could as
>> well merge them and save the churn with the second KVM I/O bus device.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> I wonder why we didn't have it that way the first place...

Me too ;-)

> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

Thanks!
Andre.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 10/11] KVM: arm/arm64: prepare GICv3 emulation to use kvm_io_bus MMIO handling
  2015-03-26 22:06   ` Marc Zyngier
@ 2015-03-27  0:14     ` Andre Przywara
  2015-03-27  7:34       ` Marc Zyngier
  0 siblings, 1 reply; 25+ messages in thread
From: Andre Przywara @ 2015-03-27  0:14 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/26/2015 10:06 PM, Marc Zyngier wrote:
> On Thu, 26 Mar 2015 14:39:37 +0000
> Andre Przywara <andre.przywara@arm.com> wrote:
> 
>> Using the framework provided by the recent vgic.c changes, we
>> register a kvm_io_bus device on mapping the virtual GICv3 resources.
>> The distributor mapping is pretty straight forward, but the
>> redistributors need some more love, since they need to be tagged with
>> the respective redistributor (read: VCPU) they are connected with.
>> We use the kvm_io_bus framework to register one devices per VCPU.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  include/kvm/arm_vgic.h      |    1 +
>>  virt/kvm/arm/vgic-v3-emul.c |   39 ++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
>> index 4523984..d6705f4 100644
>> --- a/include/kvm/arm_vgic.h
>> +++ b/include/kvm/arm_vgic.h
>> @@ -252,6 +252,7 @@ struct vgic_dist {
>>  
>>  	struct vgic_vm_ops	vm_ops;
>>  	struct vgic_io_device	dist_iodev;
>> +	struct vgic_io_device	*redist_iodevs;
>>  };
>>  
>>  struct vgic_v2_cpu_if {
>> diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c
>> index 2f03a36..eb1a797 100644
>> --- a/virt/kvm/arm/vgic-v3-emul.c
>> +++ b/virt/kvm/arm/vgic-v3-emul.c
>> @@ -758,6 +758,9 @@ static int vgic_v3_map_resources(struct kvm *kvm,
>>  {
>>  	int ret = 0;
>>  	struct vgic_dist *dist = &kvm->arch.vgic;
>> +	gpa_t rdbase = dist->vgic_redist_base;
>> +	struct vgic_io_device *iodevs = NULL;
>> +	int i;
>>  
>>  	if (!irqchip_in_kernel(kvm))
>>  		return 0;
>> @@ -783,7 +786,41 @@ static int vgic_v3_map_resources(struct kvm *kvm,
>>  		goto out;
>>  	}
>>  
>> -	kvm->arch.vgic.ready = true;
>> +	ret = vgic_register_kvm_io_dev(kvm, dist->vgic_dist_base,
>> +				       GIC_V3_DIST_SIZE, vgic_v3_dist_ranges,
>> +				       -1, &dist->dist_iodev);
> 
>> +	if (ret)
>> +		goto out;
>> +
>> +	iodevs = kcalloc(dist->nr_cpus, sizeof(iodevs[0]), GFP_KERNEL);
>> +	if (!iodevs) {
>> +		ret = -ENOMEM;
>> +		goto out_unregister;
>> +	}
>> +
>> +	for (i = 0; i < dist->nr_cpus; i++) {
>> +		ret = vgic_register_kvm_io_dev(kvm, rdbase,
>> +					       SZ_128K, vgic_redist_ranges,
>> +					       i, &iodevs[i]);
> 
> This looks really weird. You seems to be mapping all redistributors at
> the same IPA. Have you actually tested this with an SMP guest?

But the patch continues:

+		if (ret)
+			goto out_unregister;
+		rdbase += GIC_V3_REDIST_SIZE;

Is that too confusing to re-use the rdbase variable? The spec speaks of
RD_base for each redistributor, so that seemed sane to me.
Shall I add a comment or use "rdbase + i * GIC_V3_REDIST_SIZE" for clarity?

Cheers,
Andre.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 10/11] KVM: arm/arm64: prepare GICv3 emulation to use kvm_io_bus MMIO handling
  2015-03-27  0:14     ` Andre Przywara
@ 2015-03-27  7:34       ` Marc Zyngier
  0 siblings, 0 replies; 25+ messages in thread
From: Marc Zyngier @ 2015-03-27  7:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 27 Mar 2015 00:14:12 +0000
Andre Przywara <andre.przywara@arm.com> wrote:

> On 03/26/2015 10:06 PM, Marc Zyngier wrote:
> > On Thu, 26 Mar 2015 14:39:37 +0000
> > Andre Przywara <andre.przywara@arm.com> wrote:
> > 
> >> Using the framework provided by the recent vgic.c changes, we
> >> register a kvm_io_bus device on mapping the virtual GICv3 resources.
> >> The distributor mapping is pretty straight forward, but the
> >> redistributors need some more love, since they need to be tagged with
> >> the respective redistributor (read: VCPU) they are connected with.
> >> We use the kvm_io_bus framework to register one devices per VCPU.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >> ---
> >>  include/kvm/arm_vgic.h      |    1 +
> >>  virt/kvm/arm/vgic-v3-emul.c |   39 ++++++++++++++++++++++++++++++++++++++-
> >>  2 files changed, 39 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> >> index 4523984..d6705f4 100644
> >> --- a/include/kvm/arm_vgic.h
> >> +++ b/include/kvm/arm_vgic.h
> >> @@ -252,6 +252,7 @@ struct vgic_dist {
> >>  
> >>  	struct vgic_vm_ops	vm_ops;
> >>  	struct vgic_io_device	dist_iodev;
> >> +	struct vgic_io_device	*redist_iodevs;
> >>  };
> >>  
> >>  struct vgic_v2_cpu_if {
> >> diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c
> >> index 2f03a36..eb1a797 100644
> >> --- a/virt/kvm/arm/vgic-v3-emul.c
> >> +++ b/virt/kvm/arm/vgic-v3-emul.c
> >> @@ -758,6 +758,9 @@ static int vgic_v3_map_resources(struct kvm *kvm,
> >>  {
> >>  	int ret = 0;
> >>  	struct vgic_dist *dist = &kvm->arch.vgic;
> >> +	gpa_t rdbase = dist->vgic_redist_base;
> >> +	struct vgic_io_device *iodevs = NULL;
> >> +	int i;
> >>  
> >>  	if (!irqchip_in_kernel(kvm))
> >>  		return 0;
> >> @@ -783,7 +786,41 @@ static int vgic_v3_map_resources(struct kvm *kvm,
> >>  		goto out;
> >>  	}
> >>  
> >> -	kvm->arch.vgic.ready = true;
> >> +	ret = vgic_register_kvm_io_dev(kvm, dist->vgic_dist_base,
> >> +				       GIC_V3_DIST_SIZE, vgic_v3_dist_ranges,
> >> +				       -1, &dist->dist_iodev);
> > 
> >> +	if (ret)
> >> +		goto out;
> >> +
> >> +	iodevs = kcalloc(dist->nr_cpus, sizeof(iodevs[0]), GFP_KERNEL);
> >> +	if (!iodevs) {
> >> +		ret = -ENOMEM;
> >> +		goto out_unregister;
> >> +	}
> >> +
> >> +	for (i = 0; i < dist->nr_cpus; i++) {
> >> +		ret = vgic_register_kvm_io_dev(kvm, rdbase,
> >> +					       SZ_128K, vgic_redist_ranges,
> >> +					       i, &iodevs[i]);
> > 
> > This looks really weird. You seems to be mapping all redistributors at
> > the same IPA. Have you actually tested this with an SMP guest?
> 
> But the patch continues:
> 
> +		if (ret)
> +			goto out_unregister;
> +		rdbase += GIC_V3_REDIST_SIZE;
> 
> Is that too confusing to re-use the rdbase variable? The spec speaks of
> RD_base for each redistributor, so that seemed sane to me.
> Shall I add a comment or use "rdbase + i * GIC_V3_REDIST_SIZE" for clarity?

That would have been nicer, but let's face the harsh truth: I can't
read, ignore me (well, for this time only ;-).

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 11/11] KVM: arm/arm64: rework MMIO abort handling to use KVM MMIO bus
  2015-03-26 14:39 ` [PATCH v3 11/11] KVM: arm/arm64: rework MMIO abort handling to use KVM MMIO bus Andre Przywara
@ 2015-03-27 16:00   ` Christoffer Dall
  2015-03-28  1:13     ` [PATCH v3a " Andre Przywara
  0 siblings, 1 reply; 25+ messages in thread
From: Christoffer Dall @ 2015-03-27 16:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 26, 2015 at 02:39:38PM +0000, Andre Przywara wrote:
> Currently we have struct kvm_exit_mmio for encapsulating MMIO abort
> data to be passed on from syndrome decoding all the way down to the
> VGIC register handlers. Now as we switch the MMIO handling to be
> routed through the KVM MMIO bus, it does not make sense anymore to
> use that structure already from the beginning. So we put the data into
> kvm_run very early and use that encapsulation till the MMIO bus call.
> Then we fill kvm_exit_mmio in the VGIC only, making it a VGIC private
> structure. On that way we replace the data buffer in that structure
> with a pointer pointing to a single location in kvm_run, so we get
> rid of some copying on the way.
> With all of the virtual GIC emulation code now being registered with
> the kvm_io_bus, we can remove all of the old MMIO handling code and
> its dispatching functionality.
> 
> I didn't bother to rename kvm_exit_mmio (to vgic_mmio or something),
> because that touches a lot of code lines without any good reason.
> 
> This is based on an original patch by Nikolay.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> ---
>  arch/arm/include/asm/kvm_mmio.h   |   22 ---------
>  arch/arm/kvm/mmio.c               |   60 ++++++++++++++++++-------
>  arch/arm64/include/asm/kvm_mmio.h |   22 ---------
>  include/kvm/arm_vgic.h            |    6 ---
>  virt/kvm/arm/vgic-v2-emul.c       |   21 +--------
>  virt/kvm/arm/vgic-v3-emul.c       |   35 ---------------
>  virt/kvm/arm/vgic.c               |   89 ++-----------------------------------
>  virt/kvm/arm/vgic.h               |   13 +++---
>  8 files changed, 57 insertions(+), 211 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_mmio.h b/arch/arm/include/asm/kvm_mmio.h
> index 3f83db2..d8e90c8 100644
> --- a/arch/arm/include/asm/kvm_mmio.h
> +++ b/arch/arm/include/asm/kvm_mmio.h
> @@ -28,28 +28,6 @@ struct kvm_decode {
>  	bool sign_extend;
>  };
>  
> -/*
> - * The in-kernel MMIO emulation code wants to use a copy of run->mmio,
> - * which is an anonymous type. Use our own type instead.
> - */
> -struct kvm_exit_mmio {
> -	phys_addr_t	phys_addr;
> -	u8		data[8];
> -	u32		len;
> -	bool		is_write;
> -	void		*private;
> -};
> -
> -static inline void kvm_prepare_mmio(struct kvm_run *run,
> -				    struct kvm_exit_mmio *mmio)
> -{
> -	run->mmio.phys_addr	= mmio->phys_addr;
> -	run->mmio.len		= mmio->len;
> -	run->mmio.is_write	= mmio->is_write;
> -	memcpy(run->mmio.data, mmio->data, mmio->len);
> -	run->exit_reason	= KVM_EXIT_MMIO;
> -}
> -
>  int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run);
>  int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  		 phys_addr_t fault_ipa);
> diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
> index 5d3bfc0..bb2ab44 100644
> --- a/arch/arm/kvm/mmio.c
> +++ b/arch/arm/kvm/mmio.c
> @@ -122,7 +122,7 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  }
>  
>  static int decode_hsr(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> -		      struct kvm_exit_mmio *mmio)
> +		      struct kvm_run *run)
>  {
>  	unsigned long rt;
>  	int len;
> @@ -148,9 +148,9 @@ static int decode_hsr(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>  	sign_extend = kvm_vcpu_dabt_issext(vcpu);
>  	rt = kvm_vcpu_dabt_get_rd(vcpu);
>  
> -	mmio->is_write = is_write;
> -	mmio->phys_addr = fault_ipa;
> -	mmio->len = len;
> +	run->mmio.is_write = is_write;
> +	run->mmio.phys_addr = fault_ipa;
> +	run->mmio.len = len;
>  	vcpu->arch.mmio_decode.sign_extend = sign_extend;
>  	vcpu->arch.mmio_decode.rt = rt;
>  
> @@ -162,23 +162,49 @@ static int decode_hsr(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>  	return 0;
>  }
>  
> +/**
> + * handle_kernel_mmio - handle an in-kernel MMIO access
> + * @vcpu:	pointer to the vcpu performing the access
> + * @run:	pointer to the kvm_run structure
> + *
> + * returns true if the MMIO access has been performed in kernel space,
> + * and false if it needs to be emulated in user space.
> + */
> +static bool handle_kernel_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run)
> +{
> +	int ret;
> +
> +	if (run->mmio.is_write) {
> +		ret = kvm_io_bus_write(vcpu, KVM_MMIO_BUS, run->mmio.phys_addr,
> +				       run->mmio.len, run->mmio.data);
> +
> +	} else {
> +		ret = kvm_io_bus_read(vcpu, KVM_MMIO_BUS, run->mmio.phys_addr,
> +				      run->mmio.len, run->mmio.data);
> +	}
> +	if (!ret) {
> +		kvm_handle_mmio_return(vcpu, run);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>  int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  		 phys_addr_t fault_ipa)
>  {
> -	struct kvm_exit_mmio mmio;
>  	unsigned long data;
>  	unsigned long rt;
>  	int ret;
>  
>  	/*
> -	 * Prepare MMIO operation. First stash it in a private
> -	 * structure that we can use for in-kernel emulation. If the
> -	 * kernel can't handle it, copy it into run->mmio and let user
> -	 * space do its magic.
> +	 * Prepare MMIO operation. First put the MMIO data into run->mmio.
> +	 * Then try if some in-kernel emulation feels responsible, otherwise
> +	 * let user space do its magic.
>  	 */
>  
>  	if (kvm_vcpu_dabt_isvalid(vcpu)) {
> -		ret = decode_hsr(vcpu, fault_ipa, &mmio);
> +		ret = decode_hsr(vcpu, fault_ipa, run);

hmm, this does feel rather obtrusive, because as Marc pointed out the
run structure can be modified from userspace.  So a patch that comes
along later and assumes that run->mmio.len is something sane from the
HSR will break catastrophically.  So I think it'd be better to use a
privat kernel struct for this.


Otherwise, this looks good to me!

Thanks,
-Christoffer

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 07/11] KVM: arm/arm64: implement kvm_io_bus MMIO handling for the VGIC
  2015-03-26 14:39 ` [PATCH v3 07/11] KVM: arm/arm64: implement kvm_io_bus MMIO handling for the VGIC Andre Przywara
@ 2015-03-27 16:00   ` Christoffer Dall
  0 siblings, 0 replies; 25+ messages in thread
From: Christoffer Dall @ 2015-03-27 16:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 26, 2015 at 02:39:34PM +0000, Andre Przywara wrote:
> Currently we use a lot of VGIC specific code to do the MMIO
> dispatching.
> Use the previous reworks to add kvm_io_bus style MMIO handlers.
> 
> Those are not yet called by the MMIO abort handler, also the actual
> VGIC emulator function do not make use of it yet, but will be enabled
> with the following patches.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 08/11] KVM: arm/arm64: prepare GICv2 emulation to be handled by kvm_io_bus
  2015-03-26 14:39 ` [PATCH v3 08/11] KVM: arm/arm64: prepare GICv2 emulation to be handled by kvm_io_bus Andre Przywara
  2015-03-26 21:46   ` Marc Zyngier
@ 2015-03-27 16:01   ` Christoffer Dall
  1 sibling, 0 replies; 25+ messages in thread
From: Christoffer Dall @ 2015-03-27 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 26, 2015 at 02:39:35PM +0000, Andre Przywara wrote:
> Using the framework provided by the recent vgic.c changes we register
> a kvm_io_bus device when initializing the virtual GICv2.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 09/11] KVM: arm/arm64: merge GICv3 RD_base and SGI_base register frames
  2015-03-26 14:39 ` [PATCH v3 09/11] KVM: arm/arm64: merge GICv3 RD_base and SGI_base register frames Andre Przywara
  2015-03-26 21:53   ` Marc Zyngier
@ 2015-03-27 16:01   ` Christoffer Dall
  1 sibling, 0 replies; 25+ messages in thread
From: Christoffer Dall @ 2015-03-27 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 26, 2015 at 02:39:36PM +0000, Andre Przywara wrote:
> Currently we handle the redistributor registers in two separate MMIO
> regions, one for the overall behaviour and SPIs and one for the
> SGIs/PPIs. That latter forces the creation of _two_ KVM I/O bus
> devices for each redistributor.
> Since the spec mandates those two pages to be contigious, we could as
> well merge them and save the churn with the second KVM I/O bus device.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 10/11] KVM: arm/arm64: prepare GICv3 emulation to use kvm_io_bus MMIO handling
  2015-03-26 14:39 ` [PATCH v3 10/11] KVM: arm/arm64: prepare GICv3 emulation to use kvm_io_bus MMIO handling Andre Przywara
  2015-03-26 22:06   ` Marc Zyngier
@ 2015-03-27 16:01   ` Christoffer Dall
  1 sibling, 0 replies; 25+ messages in thread
From: Christoffer Dall @ 2015-03-27 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 26, 2015 at 02:39:37PM +0000, Andre Przywara wrote:
> Using the framework provided by the recent vgic.c changes, we
> register a kvm_io_bus device on mapping the virtual GICv3 resources.
> The distributor mapping is pretty straight forward, but the
> redistributors need some more love, since they need to be tagged with
> the respective redistributor (read: VCPU) they are connected with.
> We use the kvm_io_bus framework to register one devices per VCPU.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3a 11/11] KVM: arm/arm64: rework MMIO abort handling to use KVM MMIO bus
  2015-03-27 16:00   ` Christoffer Dall
@ 2015-03-28  1:13     ` Andre Przywara
  2015-03-30 10:47       ` Marc Zyngier
  0 siblings, 1 reply; 25+ messages in thread
From: Andre Przywara @ 2015-03-28  1:13 UTC (permalink / raw)
  To: linux-arm-kernel

Currently we have struct kvm_exit_mmio for encapsulating MMIO abort
data to be passed on from syndrome decoding all the way down to the
VGIC register handlers. Now as we switch the MMIO handling to be
routed through the KVM MMIO bus, it does not make sense anymore to
use that structure already from the beginning. So we keep the data in
local variables until we put them into the kvm_io_bus framework.
Then we fill kvm_exit_mmio in the VGIC only, making it a VGIC private
structure. On that way we replace the data buffer in that structure
with a pointer pointing to a single location in a local variable, so
we get rid of some copying on the way.
With all of the virtual GIC emulation code now being registered with
the kvm_io_bus, we can remove all of the old MMIO handling code and
its dispatching functionality.

I didn't bother to rename kvm_exit_mmio (to vgic_mmio or something),
because that touches a lot of code lines without any good reason.

This is based on an original patch by Nikolay.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
---
Hi,

this is a new version of PATCH v3 11/11, which does not use kvm_run
in the write case anymore. That should fix the problem that Marc and
Christoffer mentioned. The trick is to not use a structure at all,
but instead just pass on the needed values as parameters. By folding
handle_kernel_mmio() into io_mem_abort() we save some code and avoid
passing too many parameters.
As this is the last patch and the only one changed, I just send out
this one and rely on v3 for the other patches. If a complete respin
on the list is preferred, let me know.

Cheers,
Andre.

 arch/arm/include/asm/kvm_mmio.h   |   22 ---------
 arch/arm/kvm/mmio.c               |   64 ++++++++++++++-----------
 arch/arm64/include/asm/kvm_mmio.h |   22 ---------
 include/kvm/arm_vgic.h            |    6 ---
 virt/kvm/arm/vgic-v2-emul.c       |   21 +--------
 virt/kvm/arm/vgic-v3-emul.c       |   35 --------------
 virt/kvm/arm/vgic.c               |   93 ++++---------------------------------
 virt/kvm/arm/vgic.h               |   13 ++++--
 8 files changed, 55 insertions(+), 221 deletions(-)

diff --git a/arch/arm/include/asm/kvm_mmio.h b/arch/arm/include/asm/kvm_mmio.h
index 3f83db2..d8e90c8 100644
--- a/arch/arm/include/asm/kvm_mmio.h
+++ b/arch/arm/include/asm/kvm_mmio.h
@@ -28,28 +28,6 @@ struct kvm_decode {
 	bool sign_extend;
 };
 
-/*
- * The in-kernel MMIO emulation code wants to use a copy of run->mmio,
- * which is an anonymous type. Use our own type instead.
- */
-struct kvm_exit_mmio {
-	phys_addr_t	phys_addr;
-	u8		data[8];
-	u32		len;
-	bool		is_write;
-	void		*private;
-};
-
-static inline void kvm_prepare_mmio(struct kvm_run *run,
-				    struct kvm_exit_mmio *mmio)
-{
-	run->mmio.phys_addr	= mmio->phys_addr;
-	run->mmio.len		= mmio->len;
-	run->mmio.is_write	= mmio->is_write;
-	memcpy(run->mmio.data, mmio->data, mmio->len);
-	run->exit_reason	= KVM_EXIT_MMIO;
-}
-
 int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run);
 int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		 phys_addr_t fault_ipa);
diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
index 5d3bfc0..974b1c6 100644
--- a/arch/arm/kvm/mmio.c
+++ b/arch/arm/kvm/mmio.c
@@ -121,12 +121,11 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	return 0;
 }
 
-static int decode_hsr(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
-		      struct kvm_exit_mmio *mmio)
+static int decode_hsr(struct kvm_vcpu *vcpu, bool *is_write, int *len)
 {
 	unsigned long rt;
-	int len;
-	bool is_write, sign_extend;
+	int access_size;
+	bool sign_extend;
 
 	if (kvm_vcpu_dabt_isextabt(vcpu)) {
 		/* cache operation on I/O addr, tell guest unsupported */
@@ -140,17 +139,15 @@ static int decode_hsr(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 		return 1;
 	}
 
-	len = kvm_vcpu_dabt_get_as(vcpu);
-	if (unlikely(len < 0))
-		return len;
+	access_size = kvm_vcpu_dabt_get_as(vcpu);
+	if (unlikely(access_size < 0))
+		return access_size;
 
-	is_write = kvm_vcpu_dabt_iswrite(vcpu);
+	*is_write = kvm_vcpu_dabt_iswrite(vcpu);
 	sign_extend = kvm_vcpu_dabt_issext(vcpu);
 	rt = kvm_vcpu_dabt_get_rd(vcpu);
 
-	mmio->is_write = is_write;
-	mmio->phys_addr = fault_ipa;
-	mmio->len = len;
+	*len = access_size;
 	vcpu->arch.mmio_decode.sign_extend = sign_extend;
 	vcpu->arch.mmio_decode.rt = rt;
 
@@ -165,20 +162,20 @@ static int decode_hsr(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		 phys_addr_t fault_ipa)
 {
-	struct kvm_exit_mmio mmio;
 	unsigned long data;
 	unsigned long rt;
 	int ret;
+	bool is_write;
+	int len;
+	u8 data_buf[8];
 
 	/*
-	 * Prepare MMIO operation. First stash it in a private
-	 * structure that we can use for in-kernel emulation. If the
-	 * kernel can't handle it, copy it into run->mmio and let user
-	 * space do its magic.
+	 * Prepare MMIO operation. First decode the syndrome data we get
+	 * from the CPU. Then try if some in-kernel emulation feels
+	 * responsible, otherwise let user space do its magic.
 	 */
-
 	if (kvm_vcpu_dabt_isvalid(vcpu)) {
-		ret = decode_hsr(vcpu, fault_ipa, &mmio);
+		ret = decode_hsr(vcpu, &is_write, &len);
 		if (ret)
 			return ret;
 	} else {
@@ -188,21 +185,34 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 
 	rt = vcpu->arch.mmio_decode.rt;
 
-	if (mmio.is_write) {
-		data = vcpu_data_guest_to_host(vcpu, *vcpu_reg(vcpu, rt),
-					       mmio.len);
+	if (is_write) {
+		data = vcpu_data_guest_to_host(vcpu, *vcpu_reg(vcpu, rt), len);
+
+		trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, len, fault_ipa, data);
+		mmio_write_buf(data_buf, len, data);
 
-		trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, mmio.len,
-			       fault_ipa, data);
-		mmio_write_buf(mmio.data, mmio.len, data);
+		ret = kvm_io_bus_write(vcpu, KVM_MMIO_BUS, fault_ipa, len,
+				       data_buf);
 	} else {
-		trace_kvm_mmio(KVM_TRACE_MMIO_READ_UNSATISFIED, mmio.len,
+		trace_kvm_mmio(KVM_TRACE_MMIO_READ_UNSATISFIED, len,
 			       fault_ipa, 0);
+
+		ret = kvm_io_bus_read(vcpu, KVM_MMIO_BUS, fault_ipa, len,
+				      data_buf);
 	}
 
-	if (vgic_handle_mmio(vcpu, run, &mmio))
+	/* Now prepare kvm_run for the potential return to userland. */
+	run->mmio.is_write	= is_write;
+	run->mmio.phys_addr	= fault_ipa;
+	run->mmio.len		= len;
+	memcpy(run->mmio.data, data_buf, len);
+
+	if (!ret) {
+		/* We handled the access successfully in the kernel. */
+		kvm_handle_mmio_return(vcpu, run);
 		return 1;
+	}
 
-	kvm_prepare_mmio(run, &mmio);
+	run->exit_reason	= KVM_EXIT_MMIO;
 	return 0;
 }
diff --git a/arch/arm64/include/asm/kvm_mmio.h b/arch/arm64/include/asm/kvm_mmio.h
index 9f52beb..889c908 100644
--- a/arch/arm64/include/asm/kvm_mmio.h
+++ b/arch/arm64/include/asm/kvm_mmio.h
@@ -31,28 +31,6 @@ struct kvm_decode {
 	bool sign_extend;
 };
 
-/*
- * The in-kernel MMIO emulation code wants to use a copy of run->mmio,
- * which is an anonymous type. Use our own type instead.
- */
-struct kvm_exit_mmio {
-	phys_addr_t	phys_addr;
-	u8		data[8];
-	u32		len;
-	bool		is_write;
-	void		*private;
-};
-
-static inline void kvm_prepare_mmio(struct kvm_run *run,
-				    struct kvm_exit_mmio *mmio)
-{
-	run->mmio.phys_addr	= mmio->phys_addr;
-	run->mmio.len		= mmio->len;
-	run->mmio.is_write	= mmio->is_write;
-	memcpy(run->mmio.data, mmio->data, mmio->len);
-	run->exit_reason	= KVM_EXIT_MMIO;
-}
-
 int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run);
 int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 		 phys_addr_t fault_ipa);
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index d6705f4..16ec2c8 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -140,8 +140,6 @@ struct vgic_params {
 };
 
 struct vgic_vm_ops {
-	bool	(*handle_mmio)(struct kvm_vcpu *, struct kvm_run *,
-			       struct kvm_exit_mmio *);
 	bool	(*queue_sgi)(struct kvm_vcpu *, int irq);
 	void	(*add_sgi_source)(struct kvm_vcpu *, int irq, int source);
 	int	(*init_model)(struct kvm *);
@@ -313,8 +311,6 @@ struct vgic_cpu {
 
 struct kvm;
 struct kvm_vcpu;
-struct kvm_run;
-struct kvm_exit_mmio;
 
 int kvm_vgic_addr(struct kvm *kvm, unsigned long type, u64 *addr, bool write);
 int kvm_vgic_hyp_init(void);
@@ -330,8 +326,6 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
 void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
 int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
 int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
-bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
-		      struct kvm_exit_mmio *mmio);
 
 #define irqchip_in_kernel(k)	(!!((k)->arch.vgic.in_kernel))
 #define vgic_initialized(k)	(!!((k)->arch.vgic.nr_cpus))
diff --git a/virt/kvm/arm/vgic-v2-emul.c b/virt/kvm/arm/vgic-v2-emul.c
index 7460b37..1390797 100644
--- a/virt/kvm/arm/vgic-v2-emul.c
+++ b/virt/kvm/arm/vgic-v2-emul.c
@@ -404,24 +404,6 @@ static const struct vgic_io_range vgic_dist_ranges[] = {
 	{}
 };
 
-static bool vgic_v2_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
-				struct kvm_exit_mmio *mmio)
-{
-	unsigned long base = vcpu->kvm->arch.vgic.vgic_dist_base;
-
-	if (!is_in_range(mmio->phys_addr, mmio->len, base,
-			 KVM_VGIC_V2_DIST_SIZE))
-		return false;
-
-	/* GICv2 does not support accesses wider than 32 bits */
-	if (mmio->len > 4) {
-		kvm_inject_dabt(vcpu, mmio->phys_addr);
-		return true;
-	}
-
-	return vgic_handle_mmio_range(vcpu, run, mmio, vgic_dist_ranges, base);
-}
-
 static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg)
 {
 	struct kvm *kvm = vcpu->kvm;
@@ -580,7 +562,6 @@ void vgic_v2_init_emulation(struct kvm *kvm)
 {
 	struct vgic_dist *dist = &kvm->arch.vgic;
 
-	dist->vm_ops.handle_mmio = vgic_v2_handle_mmio;
 	dist->vm_ops.queue_sgi = vgic_v2_queue_sgi;
 	dist->vm_ops.add_sgi_source = vgic_v2_add_sgi_source;
 	dist->vm_ops.init_model = vgic_v2_init_model;
@@ -690,6 +671,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 	struct kvm_vcpu *vcpu, *tmp_vcpu;
 	struct vgic_dist *vgic;
 	struct kvm_exit_mmio mmio;
+	u32 data;
 
 	offset = attr->attr & KVM_DEV_ARM_VGIC_OFFSET_MASK;
 	cpuid = (attr->attr & KVM_DEV_ARM_VGIC_CPUID_MASK) >>
@@ -711,6 +693,7 @@ static int vgic_attr_regs_access(struct kvm_device *dev,
 
 	mmio.len = 4;
 	mmio.is_write = is_write;
+	mmio.data = &data;
 	if (is_write)
 		mmio_data_write(&mmio, ~0, *reg);
 	switch (attr->group) {
diff --git a/virt/kvm/arm/vgic-v3-emul.c b/virt/kvm/arm/vgic-v3-emul.c
index eb1a797..e9c3a7a 100644
--- a/virt/kvm/arm/vgic-v3-emul.c
+++ b/virt/kvm/arm/vgic-v3-emul.c
@@ -708,40 +708,6 @@ static const struct vgic_io_range vgic_redist_ranges[] = {
 	{},
 };
 
-/*
- * This function splits accesses between the distributor and the two
- * redistributor parts (private/SPI). As each redistributor is accessible
- * from any CPU, we have to determine the affected VCPU by taking the faulting
- * address into account. We then pass this VCPU to the handler function via
- * the private parameter.
- */
-#define SGI_BASE_OFFSET SZ_64K
-static bool vgic_v3_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
-				struct kvm_exit_mmio *mmio)
-{
-	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
-	unsigned long dbase = dist->vgic_dist_base;
-	unsigned long rdbase = dist->vgic_redist_base;
-	int nrcpus = atomic_read(&vcpu->kvm->online_vcpus);
-	int vcpu_id;
-
-	if (is_in_range(mmio->phys_addr, mmio->len, dbase, GIC_V3_DIST_SIZE)) {
-		return vgic_handle_mmio_range(vcpu, run, mmio,
-					      vgic_v3_dist_ranges, dbase);
-	}
-
-	if (!is_in_range(mmio->phys_addr, mmio->len, rdbase,
-	    GIC_V3_REDIST_SIZE * nrcpus))
-		return false;
-
-	vcpu_id = (mmio->phys_addr - rdbase) / GIC_V3_REDIST_SIZE;
-	rdbase += (vcpu_id * GIC_V3_REDIST_SIZE);
-	mmio->private = kvm_get_vcpu(vcpu->kvm, vcpu_id);
-
-	return vgic_handle_mmio_range(vcpu, run, mmio, vgic_redist_ranges,
-				      rdbase);
-}
-
 static bool vgic_v3_queue_sgi(struct kvm_vcpu *vcpu, int irq)
 {
 	if (vgic_queue_irq(vcpu, 0, irq)) {
@@ -861,7 +827,6 @@ void vgic_v3_init_emulation(struct kvm *kvm)
 {
 	struct vgic_dist *dist = &kvm->arch.vgic;
 
-	dist->vm_ops.handle_mmio = vgic_v3_handle_mmio;
 	dist->vm_ops.queue_sgi = vgic_v3_queue_sgi;
 	dist->vm_ops.add_sgi_source = vgic_v3_add_sgi_source;
 	dist->vm_ops.init_model = vgic_v3_init_model;
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index e968179..b70174e 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -758,7 +758,6 @@ static bool call_range_handler(struct kvm_vcpu *vcpu,
 			       unsigned long offset,
 			       const struct vgic_io_range *range)
 {
-	u32 *data32 = (void *)mmio->data;
 	struct kvm_exit_mmio mmio32;
 	bool ret;
 
@@ -775,70 +774,17 @@ static bool call_range_handler(struct kvm_vcpu *vcpu,
 	mmio32.private = mmio->private;
 
 	mmio32.phys_addr = mmio->phys_addr + 4;
-	if (mmio->is_write)
-		*(u32 *)mmio32.data = data32[1];
+	mmio32.data = &((u32 *)mmio->data)[1];
 	ret = range->handle_mmio(vcpu, &mmio32, offset + 4);
-	if (!mmio->is_write)
-		data32[1] = *(u32 *)mmio32.data;
 
 	mmio32.phys_addr = mmio->phys_addr;
-	if (mmio->is_write)
-		*(u32 *)mmio32.data = data32[0];
+	mmio32.data = &((u32 *)mmio->data)[0];
 	ret |= range->handle_mmio(vcpu, &mmio32, offset);
-	if (!mmio->is_write)
-		data32[0] = *(u32 *)mmio32.data;
 
 	return ret;
 }
 
 /**
- * vgic_handle_mmio_range - handle an in-kernel MMIO access
- * @vcpu:	pointer to the vcpu performing the access
- * @run:	pointer to the kvm_run structure
- * @mmio:	pointer to the data describing the access
- * @ranges:	array of MMIO ranges in a given region
- * @mmio_base:	base address of that region
- *
- * returns true if the MMIO access could be performed
- */
-bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
-			    struct kvm_exit_mmio *mmio,
-			    const struct vgic_io_range *ranges,
-			    unsigned long mmio_base)
-{
-	const struct vgic_io_range *range;
-	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
-	bool updated_state;
-	unsigned long offset;
-
-	offset = mmio->phys_addr - mmio_base;
-	range = vgic_find_range(ranges, mmio->len, offset);
-	if (unlikely(!range || !range->handle_mmio)) {
-		pr_warn("Unhandled access %d %08llx %d\n",
-			mmio->is_write, mmio->phys_addr, mmio->len);
-		return false;
-	}
-
-	spin_lock(&vcpu->kvm->arch.vgic.lock);
-	offset -= range->base;
-	if (vgic_validate_access(dist, range, offset)) {
-		updated_state = call_range_handler(vcpu, mmio, offset, range);
-	} else {
-		if (!mmio->is_write)
-			memset(mmio->data, 0, mmio->len);
-		updated_state = false;
-	}
-	spin_unlock(&vcpu->kvm->arch.vgic.lock);
-	kvm_prepare_mmio(run, mmio);
-	kvm_handle_mmio_return(vcpu, run);
-
-	if (updated_state)
-		vgic_kick_vcpus(vcpu->kvm);
-
-	return true;
-}
-
-/**
  * vgic_handle_mmio_access - handle an in-kernel MMIO access
  * This is called by the read/write KVM IO device wrappers below.
  * @vcpu:	pointer to the vcpu performing the access
@@ -873,23 +819,24 @@ static int vgic_handle_mmio_access(struct kvm_vcpu *vcpu,
 	mmio.phys_addr = addr;
 	mmio.len = len;
 	mmio.is_write = is_write;
-	if (is_write)
-		memcpy(mmio.data, val, len);
+	mmio.data = val;
 	mmio.private = iodev->redist_vcpu;
 
 	spin_lock(&dist->lock);
 	offset -= range->base;
 	if (vgic_validate_access(dist, range, offset)) {
 		updated_state = call_range_handler(vcpu, &mmio, offset, range);
-		if (!is_write)
-			memcpy(val, mmio.data, len);
 	} else {
 		if (!is_write)
 			memset(val, 0, len);
 		updated_state = false;
 	}
 	spin_unlock(&dist->lock);
-	kvm_prepare_mmio(run, &mmio);
+	run->mmio.is_write	= is_write;
+	run->mmio.len		= len;
+	run->mmio.phys_addr	= addr;
+	memcpy(run->mmio.data, val, len);
+
 	kvm_handle_mmio_return(vcpu, run);
 
 	if (updated_state)
@@ -898,30 +845,6 @@ static int vgic_handle_mmio_access(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
-/**
- * vgic_handle_mmio - handle an in-kernel MMIO access for the GIC emulation
- * @vcpu:      pointer to the vcpu performing the access
- * @run:       pointer to the kvm_run structure
- * @mmio:      pointer to the data describing the access
- *
- * returns true if the MMIO access has been performed in kernel space,
- * and false if it needs to be emulated in user space.
- * Calls the actual handling routine for the selected VGIC model.
- */
-bool vgic_handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *run,
-		      struct kvm_exit_mmio *mmio)
-{
-	if (!irqchip_in_kernel(vcpu->kvm))
-		return false;
-
-	/*
-	 * This will currently call either vgic_v2_handle_mmio() or
-	 * vgic_v3_handle_mmio(), which in turn will call
-	 * vgic_handle_mmio_range() defined above.
-	 */
-	return vcpu->kvm->arch.vgic.vm_ops.handle_mmio(vcpu, run, mmio);
-}
-
 static int vgic_handle_mmio_read(struct kvm_vcpu *vcpu,
 				 struct kvm_io_device *this,
 				 gpa_t addr, int len, void *val)
diff --git a/virt/kvm/arm/vgic.h b/virt/kvm/arm/vgic.h
index 28fa3aa..0df74cb 100644
--- a/virt/kvm/arm/vgic.h
+++ b/virt/kvm/arm/vgic.h
@@ -59,6 +59,14 @@ void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
 bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq);
 void vgic_unqueue_irqs(struct kvm_vcpu *vcpu);
 
+struct kvm_exit_mmio {
+	phys_addr_t	phys_addr;
+	void		*data;
+	u32		len;
+	bool		is_write;
+	void		*private;
+};
+
 void vgic_reg_access(struct kvm_exit_mmio *mmio, u32 *reg,
 		     phys_addr_t offset, int mode);
 bool handle_mmio_raz_wi(struct kvm_vcpu *vcpu, struct kvm_exit_mmio *mmio,
@@ -99,11 +107,6 @@ const
 struct vgic_io_range *vgic_find_range(const struct vgic_io_range *ranges,
 				      int len, gpa_t offset);
 
-bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu, struct kvm_run *run,
-			    struct kvm_exit_mmio *mmio,
-			    const struct vgic_io_range *ranges,
-			    unsigned long mmio_base);
-
 bool vgic_handle_enable_reg(struct kvm *kvm, struct kvm_exit_mmio *mmio,
 			    phys_addr_t offset, int vcpu_id, int access);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3a 11/11] KVM: arm/arm64: rework MMIO abort handling to use KVM MMIO bus
  2015-03-28  1:13     ` [PATCH v3a " Andre Przywara
@ 2015-03-30 10:47       ` Marc Zyngier
  0 siblings, 0 replies; 25+ messages in thread
From: Marc Zyngier @ 2015-03-30 10:47 UTC (permalink / raw)
  To: linux-arm-kernel

On 28/03/15 01:13, Andre Przywara wrote:
> Currently we have struct kvm_exit_mmio for encapsulating MMIO abort
> data to be passed on from syndrome decoding all the way down to the
> VGIC register handlers. Now as we switch the MMIO handling to be
> routed through the KVM MMIO bus, it does not make sense anymore to
> use that structure already from the beginning. So we keep the data in
> local variables until we put them into the kvm_io_bus framework.
> Then we fill kvm_exit_mmio in the VGIC only, making it a VGIC private
> structure. On that way we replace the data buffer in that structure
> with a pointer pointing to a single location in a local variable, so
> we get rid of some copying on the way.
> With all of the virtual GIC emulation code now being registered with
> the kvm_io_bus, we can remove all of the old MMIO handling code and
> its dispatching functionality.
> 
> I didn't bother to rename kvm_exit_mmio (to vgic_mmio or something),
> because that touches a lot of code lines without any good reason.
> 
> This is based on an original patch by Nikolay.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
> ---
> Hi,
> 
> this is a new version of PATCH v3 11/11, which does not use kvm_run
> in the write case anymore. That should fix the problem that Marc and
> Christoffer mentioned. The trick is to not use a structure at all,
> but instead just pass on the needed values as parameters. By folding
> handle_kernel_mmio() into io_mem_abort() we save some code and avoid
> passing too many parameters.
> As this is the last patch and the only one changed, I just send out
> this one and rely on v3 for the other patches. If a complete respin
> on the list is preferred, let me know.

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

I've put this into queue, together with Nikolay's (reworked) patch. I'm
currently taking the whole thing for a spin on a few boxesm and will
hopefully push it into -next by the end of the day.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2015-03-30 10:47 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-26 14:39 [PATCH v3 00/11] KVM: arm/arm64: move VGIC MMIO to kvm_io_bus Andre Przywara
2015-03-26 14:39 ` [PATCH v3 01/11] KVM: Redesign kvm_io_bus_ API to pass VCPU structure to the callbacks Andre Przywara
2015-03-26 14:39 ` [PATCH v3 02/11] KVM: move iodev.h from virt/kvm/ to include/kvm Andre Przywara
2015-03-26 14:39 ` [PATCH v3 03/11] KVM: arm/arm64: remove now unneeded include directory from Makefile Andre Przywara
2015-03-26 14:39 ` [PATCH v3 04/11] KVM: x86: " Andre Przywara
2015-03-26 14:39 ` [PATCH v3 05/11] KVM: arm/arm64: rename struct kvm_mmio_range to vgic_io_range Andre Przywara
2015-03-26 14:39 ` [PATCH v3 06/11] KVM: arm/arm64: simplify vgic_find_range() and callers Andre Przywara
2015-03-26 14:39 ` [PATCH v3 07/11] KVM: arm/arm64: implement kvm_io_bus MMIO handling for the VGIC Andre Przywara
2015-03-27 16:00   ` Christoffer Dall
2015-03-26 14:39 ` [PATCH v3 08/11] KVM: arm/arm64: prepare GICv2 emulation to be handled by kvm_io_bus Andre Przywara
2015-03-26 21:46   ` Marc Zyngier
2015-03-27 16:01   ` Christoffer Dall
2015-03-26 14:39 ` [PATCH v3 09/11] KVM: arm/arm64: merge GICv3 RD_base and SGI_base register frames Andre Przywara
2015-03-26 21:53   ` Marc Zyngier
2015-03-27  0:14     ` Andre Przywara
2015-03-27 16:01   ` Christoffer Dall
2015-03-26 14:39 ` [PATCH v3 10/11] KVM: arm/arm64: prepare GICv3 emulation to use kvm_io_bus MMIO handling Andre Przywara
2015-03-26 22:06   ` Marc Zyngier
2015-03-27  0:14     ` Andre Przywara
2015-03-27  7:34       ` Marc Zyngier
2015-03-27 16:01   ` Christoffer Dall
2015-03-26 14:39 ` [PATCH v3 11/11] KVM: arm/arm64: rework MMIO abort handling to use KVM MMIO bus Andre Przywara
2015-03-27 16:00   ` Christoffer Dall
2015-03-28  1:13     ` [PATCH v3a " Andre Przywara
2015-03-30 10:47       ` Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).