On Fri, Mar 13, 2015 at 04:10:08PM +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.przyw...@arm.com>
> ---
> include/kvm/arm_vgic.h | 9 ++++
> virt/kvm/arm/vgic.c | 111
> ++++++++++++++++++++++++++++++++++++++++++++++++
> virt/kvm/arm/vgic.h | 7 +++
> 3 files changed, 127 insertions(+)
>
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index b81630b..4bfc6a3 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 7aae19b..71389b8 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):
> @@ -774,6 +776,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);
we're not the only user of kvm_exit_mmio I believe, so we could rename
this to vgic_io as well and you could change the mmio.data array to be a
void *val pointer, which just gets set to the pointer passed into this
function (which I think points to the kvm_run structs data array) and
you can avoid all these memcopies, right?
> + 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
> @@ -797,6 +859,55 @@ 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,
> +};
> +
can you add kdocs to this exported function?
> +int vgic_register_kvm_io_dev(struct kvm *kvm, gpa_t base, int len,
> + const struct vgic_io_range *ranges,
> + int redist_id,
nit: consider renaming to redist_vcpu_id
> + struct vgic_io_device *iodev)
> +{
> + struct kvm_vcpu *vcpu = NULL;
> + int ret;
> +
> + if (redist_id >= 0)
> + vcpu = kvm_get_vcpu(kvm, redist_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);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
kvm_io_bus_register_dev returns either 0 or -ERRNO, so you can just
return ret here.
> +}
> +
> 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 ffafb15..f2063a7 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
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
