Re: [PATCH v3 3/5] KVM: ARM VGIC add kvm_io_bus_ frontend
On Tue, Jan 27, 2015 at 7:44 PM, Andre Przywara wrote:
> Hi,
>
> On 27/01/15 17:26, Eric Auger wrote:
>> On 01/27/2015 05:51 PM, Nikolay Nikolaev wrote:
>>> Hi Andre,
>>>
>>> On Tue, Jan 27, 2015 at 3:31 PM, Andre Przywara
>>> wrote:
Hi Nikolay,
On 24/01/15 11:59, Nikolay Nikolaev wrote:
> In io_mem_abort remove the call to vgic_handle_mmio. The target is to have
> a single MMIO handling path - that is through the kvm_io_bus_ API.
>
> Register a kvm_io_device in kvm_vgic_init on the whole vGIC MMIO region.
> Both read and write calls are redirected to vgic_io_dev_access where
> kvm_exit_mmio is composed to pass it to vm_ops.handle_mmio.
>
>
> Signed-off-by: Nikolay Nikolaev
> ---
> arch/arm/kvm/mmio.c|3 -
> include/kvm/arm_vgic.h |3 -
> virt/kvm/arm/vgic.c| 123
>
> 3 files changed, 114 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
> index d852137..8dc2fde 100644
> --- a/arch/arm/kvm/mmio.c
> +++ b/arch/arm/kvm/mmio.c
> @@ -230,9 +230,6 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct
> kvm_run *run,
> fault_ipa, 0);
> }
>
> - if (vgic_handle_mmio(vcpu, run, &mmio))
> - return 1;
> -
Why is this (whole patch) actually needed? Is that just to make it nicer
by pulling everything under one umbrella?
>>>
>>>
>>> It started from this mail form Christofer:
>>> https://lkml.org/lkml/2014/3/28/403
>> Hi Nikolay, Andre,
>>
>> I also understood that the target was to handle all kernel mmio through
>> the same API, hence the first patch. This patch shows that at least for
>> GICv2 it was doable without upheavals in vgic code and it also serves
>> ioeventd which is good. Andre do you think the price to pay to integrate
>> missing redistributors and forthcoming components is too high?
>
> Hopefully not, actually I reckon that moving the "upper level" MMIO
> dispatching out of vgic.c and letting the specific VGIC models register
> what they need themselves (in their -emul.c files) sounds quite promising.
> But this particular patch does not serve this purpose:
> a) we replace two lines with a bunch of more layered code
> b) we copy the MMIOed data to convert between the interfaces
> c) we miss GICv3 emulation
>
> So this needs to be addressed in a more general way (which maybe I will
> give a try). That being sad I don't see why we would need to do this
Andre,
we've already overspent our resource budget on this so I would be more
than glad if you can take over the VGIC part here.
Of course I'll help with whatever I can (review, testing, patches
logistics etc)
Here is the v3 of the patches in case you need to clone and base your
work on top of it:
https://git.virtualopensystems.com/common/linux/commits/ioeventfd_v3
regards,
Nikolay Nikolaev
> right now and hold back ioeventfd by this rather orthogonal issue.
>
> Christoffer, what's your take on this?
>
> Cheers,
> Andre.
>
>> Best Regards
>>
>> Eric
>>
>>
>>>
For enabling ioeventfd you actually don't need this patch, right?
>>> Yes, we don't need it.
(I am asking because this breaks GICv3 emulation, see below)
> if (handle_kernel_mmio(vcpu, run, &mmio))
> return 1;
>
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 7c55dd5..60639b1 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -237,6 +237,7 @@ struct vgic_dist {
> unsigned long *irq_pending_on_cpu;
>
> struct vgic_vm_ops vm_ops;
> + struct kvm_io_device*io_dev;
> #endif
> };
>
> @@ -311,8 +312,6 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid,
> unsigned int irq_num,
> bool level);
> void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
> int kvm_vgic_vcpu_pending_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.c b/virt/kvm/arm/vgic.c
> index 0cc6ab6..195d2ba 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -31,6 +31,9 @@
> #include
> #include
> #include
> +#include
> +
> +#include "iodev.h"
>
> /*
> * How the whole thing works (courtesy of Christoffer Dall):
> @@ -77,6 +80,7 @@
>
> #include "vgic.h"
>
> +static int vgic_register_kvm_io_dev(struct kvm *kvm);
> static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
> static void vgic_retire_lr(int lr_nr, int irq, struct kv
Re: [PATCH v3 3/5] KVM: ARM VGIC add kvm_io_bus_ frontend
On Tue, Jan 27, 2015 at 05:44:26PM +, Andre Przywara wrote: > Hi, > > On 27/01/15 17:26, Eric Auger wrote: > > On 01/27/2015 05:51 PM, Nikolay Nikolaev wrote: > >> Hi Andre, > >> > >> On Tue, Jan 27, 2015 at 3:31 PM, Andre Przywara > >> wrote: > >>> > >>> Hi Nikolay, > >>> > >>> On 24/01/15 11:59, Nikolay Nikolaev wrote: > In io_mem_abort remove the call to vgic_handle_mmio. The target is to > have > a single MMIO handling path - that is through the kvm_io_bus_ API. > > Register a kvm_io_device in kvm_vgic_init on the whole vGIC MMIO region. > Both read and write calls are redirected to vgic_io_dev_access where > kvm_exit_mmio is composed to pass it to vm_ops.handle_mmio. > > > Signed-off-by: Nikolay Nikolaev > --- > arch/arm/kvm/mmio.c|3 - > include/kvm/arm_vgic.h |3 - > virt/kvm/arm/vgic.c| 123 > > 3 files changed, 114 insertions(+), 15 deletions(-) > > diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c > index d852137..8dc2fde 100644 > --- a/arch/arm/kvm/mmio.c > +++ b/arch/arm/kvm/mmio.c > @@ -230,9 +230,6 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct > kvm_run *run, > fault_ipa, 0); > } > > - if (vgic_handle_mmio(vcpu, run, &mmio)) > - return 1; > - > >>> > >>> Why is this (whole patch) actually needed? Is that just to make it nicer > >>> by pulling everything under one umbrella? > >> > >> > >> It started from this mail form Christofer: > >> https://lkml.org/lkml/2014/3/28/403 > > Hi Nikolay, Andre, > > > > I also understood that the target was to handle all kernel mmio through > > the same API, hence the first patch. This patch shows that at least for > > GICv2 it was doable without upheavals in vgic code and it also serves > > ioeventd which is good. Andre do you think the price to pay to integrate > > missing redistributors and forthcoming components is too high? > > Hopefully not, actually I reckon that moving the "upper level" MMIO > dispatching out of vgic.c and letting the specific VGIC models register > what they need themselves (in their -emul.c files) sounds quite promising. > But this particular patch does not serve this purpose: > a) we replace two lines with a bunch of more layered code > b) we copy the MMIOed data to convert between the interfaces > c) we miss GICv3 emulation > > So this needs to be addressed in a more general way (which maybe I will > give a try). That being sad I don't see why we would need to do this > right now and hold back ioeventfd by this rather orthogonal issue. > > Christoffer, what's your take on this? > Well, I'd like to not special-case the vgic handling function just because we want to get this in sooner. The fact that this is conflicting with gicv3 that just got in and that we're at -rc6 now, makes me think it's probably too late to do proper testing and review of this before queuing it, so why not fix it right instead of saying "we'll fix this later" and never get to it... -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/5] KVM: ARM VGIC add kvm_io_bus_ frontend
Hi,
On 27/01/15 17:26, Eric Auger wrote:
> On 01/27/2015 05:51 PM, Nikolay Nikolaev wrote:
>> Hi Andre,
>>
>> On Tue, Jan 27, 2015 at 3:31 PM, Andre Przywara
>> wrote:
>>>
>>> Hi Nikolay,
>>>
>>> On 24/01/15 11:59, Nikolay Nikolaev wrote:
In io_mem_abort remove the call to vgic_handle_mmio. The target is to have
a single MMIO handling path - that is through the kvm_io_bus_ API.
Register a kvm_io_device in kvm_vgic_init on the whole vGIC MMIO region.
Both read and write calls are redirected to vgic_io_dev_access where
kvm_exit_mmio is composed to pass it to vm_ops.handle_mmio.
Signed-off-by: Nikolay Nikolaev
---
arch/arm/kvm/mmio.c|3 -
include/kvm/arm_vgic.h |3 -
virt/kvm/arm/vgic.c| 123
3 files changed, 114 insertions(+), 15 deletions(-)
diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
index d852137..8dc2fde 100644
--- a/arch/arm/kvm/mmio.c
+++ b/arch/arm/kvm/mmio.c
@@ -230,9 +230,6 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run
*run,
fault_ipa, 0);
}
- if (vgic_handle_mmio(vcpu, run, &mmio))
- return 1;
-
>>>
>>> Why is this (whole patch) actually needed? Is that just to make it nicer
>>> by pulling everything under one umbrella?
>>
>>
>> It started from this mail form Christofer:
>> https://lkml.org/lkml/2014/3/28/403
> Hi Nikolay, Andre,
>
> I also understood that the target was to handle all kernel mmio through
> the same API, hence the first patch. This patch shows that at least for
> GICv2 it was doable without upheavals in vgic code and it also serves
> ioeventd which is good. Andre do you think the price to pay to integrate
> missing redistributors and forthcoming components is too high?
Hopefully not, actually I reckon that moving the "upper level" MMIO
dispatching out of vgic.c and letting the specific VGIC models register
what they need themselves (in their -emul.c files) sounds quite promising.
But this particular patch does not serve this purpose:
a) we replace two lines with a bunch of more layered code
b) we copy the MMIOed data to convert between the interfaces
c) we miss GICv3 emulation
So this needs to be addressed in a more general way (which maybe I will
give a try). That being sad I don't see why we would need to do this
right now and hold back ioeventfd by this rather orthogonal issue.
Christoffer, what's your take on this?
Cheers,
Andre.
> Best Regards
>
> Eric
>
>
>>
>>>
>>> For enabling ioeventfd you actually don't need this patch, right?
>> Yes, we don't need it.
>>> (I am asking because this breaks GICv3 emulation, see below)
>>>
if (handle_kernel_mmio(vcpu, run, &mmio))
return 1;
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 7c55dd5..60639b1 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -237,6 +237,7 @@ struct vgic_dist {
unsigned long *irq_pending_on_cpu;
struct vgic_vm_ops vm_ops;
+ struct kvm_io_device*io_dev;
#endif
};
@@ -311,8 +312,6 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid,
unsigned int irq_num,
bool level);
void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
int kvm_vgic_vcpu_pending_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.c b/virt/kvm/arm/vgic.c
index 0cc6ab6..195d2ba 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -31,6 +31,9 @@
#include
#include
#include
+#include
+
+#include "iodev.h"
/*
* How the whole thing works (courtesy of Christoffer Dall):
@@ -77,6 +80,7 @@
#include "vgic.h"
+static int vgic_register_kvm_io_dev(struct kvm *kvm);
static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
static void vgic_retire_lr(int lr_nr, int irq, struct kvm_vcpu *vcpu);
static struct vgic_lr vgic_get_lr(const struct kvm_vcpu *vcpu, int lr);
@@ -97,6 +101,7 @@ static bool queue_sgi(struct kvm_vcpu *vcpu, int irq)
int kvm_vgic_map_resources(struct kvm *kvm)
{
+ vgic_register_kvm_io_dev(kvm);
return kvm->arch.vgic.vm_ops.map_resources(kvm, vgic);
}
@@ -776,27 +781,123 @@ bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu,
struct kvm_run *run,
}
/**
- * vgic_handle_mmio - handle an in-kernel MMIO access for the GIC
emulation
+ * vgic_io_dev_ac
Re: [PATCH v3 3/5] KVM: ARM VGIC add kvm_io_bus_ frontend
Hi Andre,
On Tue, Jan 27, 2015 at 3:31 PM, Andre Przywara wrote:
>
> Hi Nikolay,
>
> On 24/01/15 11:59, Nikolay Nikolaev wrote:
> > In io_mem_abort remove the call to vgic_handle_mmio. The target is to have
> > a single MMIO handling path - that is through the kvm_io_bus_ API.
> >
> > Register a kvm_io_device in kvm_vgic_init on the whole vGIC MMIO region.
> > Both read and write calls are redirected to vgic_io_dev_access where
> > kvm_exit_mmio is composed to pass it to vm_ops.handle_mmio.
> >
> >
> > Signed-off-by: Nikolay Nikolaev
> > ---
> > arch/arm/kvm/mmio.c|3 -
> > include/kvm/arm_vgic.h |3 -
> > virt/kvm/arm/vgic.c| 123
> >
> > 3 files changed, 114 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
> > index d852137..8dc2fde 100644
> > --- a/arch/arm/kvm/mmio.c
> > +++ b/arch/arm/kvm/mmio.c
> > @@ -230,9 +230,6 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run
> > *run,
> > fault_ipa, 0);
> > }
> >
> > - if (vgic_handle_mmio(vcpu, run, &mmio))
> > - return 1;
> > -
>
> Why is this (whole patch) actually needed? Is that just to make it nicer
> by pulling everything under one umbrella?
It started from this mail form Christofer:
https://lkml.org/lkml/2014/3/28/403
>
> For enabling ioeventfd you actually don't need this patch, right?
Yes, we don't need it.
> (I am asking because this breaks GICv3 emulation, see below)
>
> > if (handle_kernel_mmio(vcpu, run, &mmio))
> > return 1;
> >
> > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> > index 7c55dd5..60639b1 100644
> > --- a/include/kvm/arm_vgic.h
> > +++ b/include/kvm/arm_vgic.h
> > @@ -237,6 +237,7 @@ struct vgic_dist {
> > unsigned long *irq_pending_on_cpu;
> >
> > struct vgic_vm_ops vm_ops;
> > + struct kvm_io_device*io_dev;
> > #endif
> > };
> >
> > @@ -311,8 +312,6 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid,
> > unsigned int irq_num,
> > bool level);
> > void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
> > int kvm_vgic_vcpu_pending_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.c b/virt/kvm/arm/vgic.c
> > index 0cc6ab6..195d2ba 100644
> > --- a/virt/kvm/arm/vgic.c
> > +++ b/virt/kvm/arm/vgic.c
> > @@ -31,6 +31,9 @@
> > #include
> > #include
> > #include
> > +#include
> > +
> > +#include "iodev.h"
> >
> > /*
> > * How the whole thing works (courtesy of Christoffer Dall):
> > @@ -77,6 +80,7 @@
> >
> > #include "vgic.h"
> >
> > +static int vgic_register_kvm_io_dev(struct kvm *kvm);
> > static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
> > static void vgic_retire_lr(int lr_nr, int irq, struct kvm_vcpu *vcpu);
> > static struct vgic_lr vgic_get_lr(const struct kvm_vcpu *vcpu, int lr);
> > @@ -97,6 +101,7 @@ static bool queue_sgi(struct kvm_vcpu *vcpu, int irq)
> >
> > int kvm_vgic_map_resources(struct kvm *kvm)
> > {
> > + vgic_register_kvm_io_dev(kvm);
> > return kvm->arch.vgic.vm_ops.map_resources(kvm, vgic);
> > }
> >
> > @@ -776,27 +781,123 @@ bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu,
> > struct kvm_run *run,
> > }
> >
> > /**
> > - * vgic_handle_mmio - handle an in-kernel MMIO access for the GIC emulation
> > + * vgic_io_dev_access - 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
> > + * @this: pointer to the kvm_io_device structure
> > + * @addr: the MMIO address being accessed
> > + * @len: the length of the accessed data
> > + * @val: pointer to the value being written,
> > + * or where the read operation will store its result
> > + * @is_write: flag to show whether a write access is performed
> > *
> > - * returns true if the MMIO access has been performed in kernel space,
> > - * and false if it needs to be emulated in user space.
> > + * returns 0 if the MMIO access has been performed in kernel space,
> > + * and 1 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)
> > +static int vgic_io_dev_access(struct kvm_vcpu *vcpu, struct kvm_io_device
> > *this,
> > + gpa_t addr, int len, void *val, bool is_write)
> > {
> > - if (!irqchip_in_kernel(vcpu->kvm))
> > - return false;
> > +
Re: [PATCH v3 3/5] KVM: ARM VGIC add kvm_io_bus_ frontend
Hi Nikolay,
On 24/01/15 11:59, Nikolay Nikolaev wrote:
> In io_mem_abort remove the call to vgic_handle_mmio. The target is to have
> a single MMIO handling path - that is through the kvm_io_bus_ API.
>
> Register a kvm_io_device in kvm_vgic_init on the whole vGIC MMIO region.
> Both read and write calls are redirected to vgic_io_dev_access where
> kvm_exit_mmio is composed to pass it to vm_ops.handle_mmio.
>
>
> Signed-off-by: Nikolay Nikolaev
> ---
> arch/arm/kvm/mmio.c|3 -
> include/kvm/arm_vgic.h |3 -
> virt/kvm/arm/vgic.c| 123
>
> 3 files changed, 114 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
> index d852137..8dc2fde 100644
> --- a/arch/arm/kvm/mmio.c
> +++ b/arch/arm/kvm/mmio.c
> @@ -230,9 +230,6 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run
> *run,
> fault_ipa, 0);
> }
>
> - if (vgic_handle_mmio(vcpu, run, &mmio))
> - return 1;
> -
Why is this (whole patch) actually needed? Is that just to make it nicer
by pulling everything under one umbrella?
For enabling ioeventfd you actually don't need this patch, right?
(I am asking because this breaks GICv3 emulation, see below)
> if (handle_kernel_mmio(vcpu, run, &mmio))
> return 1;
>
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 7c55dd5..60639b1 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -237,6 +237,7 @@ struct vgic_dist {
> unsigned long *irq_pending_on_cpu;
>
> struct vgic_vm_ops vm_ops;
> + struct kvm_io_device*io_dev;
> #endif
> };
>
> @@ -311,8 +312,6 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid,
> unsigned int irq_num,
> bool level);
> void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
> int kvm_vgic_vcpu_pending_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.c b/virt/kvm/arm/vgic.c
> index 0cc6ab6..195d2ba 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -31,6 +31,9 @@
> #include
> #include
> #include
> +#include
> +
> +#include "iodev.h"
>
> /*
> * How the whole thing works (courtesy of Christoffer Dall):
> @@ -77,6 +80,7 @@
>
> #include "vgic.h"
>
> +static int vgic_register_kvm_io_dev(struct kvm *kvm);
> static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
> static void vgic_retire_lr(int lr_nr, int irq, struct kvm_vcpu *vcpu);
> static struct vgic_lr vgic_get_lr(const struct kvm_vcpu *vcpu, int lr);
> @@ -97,6 +101,7 @@ static bool queue_sgi(struct kvm_vcpu *vcpu, int irq)
>
> int kvm_vgic_map_resources(struct kvm *kvm)
> {
> + vgic_register_kvm_io_dev(kvm);
> return kvm->arch.vgic.vm_ops.map_resources(kvm, vgic);
> }
>
> @@ -776,27 +781,123 @@ bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu,
> struct kvm_run *run,
> }
>
> /**
> - * vgic_handle_mmio - handle an in-kernel MMIO access for the GIC emulation
> + * vgic_io_dev_access - 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
> + * @this: pointer to the kvm_io_device structure
> + * @addr: the MMIO address being accessed
> + * @len: the length of the accessed data
> + * @val: pointer to the value being written,
> + * or where the read operation will store its result
> + * @is_write: flag to show whether a write access is performed
> *
> - * returns true if the MMIO access has been performed in kernel space,
> - * and false if it needs to be emulated in user space.
> + * returns 0 if the MMIO access has been performed in kernel space,
> + * and 1 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)
> +static int vgic_io_dev_access(struct kvm_vcpu *vcpu, struct kvm_io_device
> *this,
> + gpa_t addr, int len, void *val, bool is_write)
> {
> - if (!irqchip_in_kernel(vcpu->kvm))
> - return false;
> + struct kvm_exit_mmio mmio;
> + bool ret;
> +
> + mmio = (struct kvm_exit_mmio) {
> + .phys_addr = addr,
> + .len = len,
> + .is_write = is_write,
> + };
> +
> + if (is_write)
> + memcpy(mmio.data, val, len);
>
> /*
>* This will currently call either vgic_v2_handle_mmio() or
>* vgic_v3_handle_mmio(), which i
[PATCH v3 3/5] KVM: ARM VGIC add kvm_io_bus_ frontend
In io_mem_abort remove the call to vgic_handle_mmio. The target is to have
a single MMIO handling path - that is through the kvm_io_bus_ API.
Register a kvm_io_device in kvm_vgic_init on the whole vGIC MMIO region.
Both read and write calls are redirected to vgic_io_dev_access where
kvm_exit_mmio is composed to pass it to vm_ops.handle_mmio.
Signed-off-by: Nikolay Nikolaev
---
arch/arm/kvm/mmio.c|3 -
include/kvm/arm_vgic.h |3 -
virt/kvm/arm/vgic.c| 123
3 files changed, 114 insertions(+), 15 deletions(-)
diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
index d852137..8dc2fde 100644
--- a/arch/arm/kvm/mmio.c
+++ b/arch/arm/kvm/mmio.c
@@ -230,9 +230,6 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
fault_ipa, 0);
}
- if (vgic_handle_mmio(vcpu, run, &mmio))
- return 1;
-
if (handle_kernel_mmio(vcpu, run, &mmio))
return 1;
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 7c55dd5..60639b1 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -237,6 +237,7 @@ struct vgic_dist {
unsigned long *irq_pending_on_cpu;
struct vgic_vm_ops vm_ops;
+ struct kvm_io_device*io_dev;
#endif
};
@@ -311,8 +312,6 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid,
unsigned int irq_num,
bool level);
void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
int kvm_vgic_vcpu_pending_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.c b/virt/kvm/arm/vgic.c
index 0cc6ab6..195d2ba 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -31,6 +31,9 @@
#include
#include
#include
+#include
+
+#include "iodev.h"
/*
* How the whole thing works (courtesy of Christoffer Dall):
@@ -77,6 +80,7 @@
#include "vgic.h"
+static int vgic_register_kvm_io_dev(struct kvm *kvm);
static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
static void vgic_retire_lr(int lr_nr, int irq, struct kvm_vcpu *vcpu);
static struct vgic_lr vgic_get_lr(const struct kvm_vcpu *vcpu, int lr);
@@ -97,6 +101,7 @@ static bool queue_sgi(struct kvm_vcpu *vcpu, int irq)
int kvm_vgic_map_resources(struct kvm *kvm)
{
+ vgic_register_kvm_io_dev(kvm);
return kvm->arch.vgic.vm_ops.map_resources(kvm, vgic);
}
@@ -776,27 +781,123 @@ bool vgic_handle_mmio_range(struct kvm_vcpu *vcpu,
struct kvm_run *run,
}
/**
- * vgic_handle_mmio - handle an in-kernel MMIO access for the GIC emulation
+ * vgic_io_dev_access - 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
+ * @this: pointer to the kvm_io_device structure
+ * @addr: the MMIO address being accessed
+ * @len: the length of the accessed data
+ * @val: pointer to the value being written,
+ * or where the read operation will store its result
+ * @is_write: flag to show whether a write access is performed
*
- * returns true if the MMIO access has been performed in kernel space,
- * and false if it needs to be emulated in user space.
+ * returns 0 if the MMIO access has been performed in kernel space,
+ * and 1 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)
+static int vgic_io_dev_access(struct kvm_vcpu *vcpu, struct kvm_io_device
*this,
+ gpa_t addr, int len, void *val, bool is_write)
{
- if (!irqchip_in_kernel(vcpu->kvm))
- return false;
+ struct kvm_exit_mmio mmio;
+ bool ret;
+
+ mmio = (struct kvm_exit_mmio) {
+ .phys_addr = addr,
+ .len = len,
+ .is_write = is_write,
+ };
+
+ if (is_write)
+ memcpy(mmio.data, val, len);
/*
* 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);
+ ret = vcpu->kvm->arch.vgic.vm_ops.handle_mmio(vcpu, vcpu->run, &mmio);
+
+ if (!is_write)
+ memcpy(val, mmio.data, len);
+
+ return ret ? 0 : 1;
+}
+
+static int vgic_io_dev_read(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
+ gpa_t addr, int len, void *val)
+{
+ return vgic_
