Re: [RFC v3 1/5] KVM: add initial support for KVM_SET_IOREGION

Cornelia Huck
2021-03-05T13:12:42Z

On Sun, 21 Feb 2021 15:04:37 +0300
Elena Afanasova <eafanasova@gmail.com> wrote:

> This vm ioctl adds or removes an ioregionfd MMIO/PIO region. Guest
> read and write accesses are dispatched through the given ioregionfd
> instead of returning from ioctl(KVM_RUN).
>
> Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
> ---
> v3:
> - add FAST_MMIO bus support
> - add KVM_IOREGION_DEASSIGN flag
> - rename kvm_ioregion read/write file descriptors
>
> arch/x86/kvm/Kconfig | 1 +
> arch/x86/kvm/Makefile | 1 +
> arch/x86/kvm/x86.c | 1 +
> include/linux/kvm_host.h | 18 +++
> include/uapi/linux/kvm.h | 25 ++++
> virt/kvm/Kconfig | 3 +
> virt/kvm/eventfd.c | 25 ++++
> virt/kvm/eventfd.h | 14 +++
> virt/kvm/ioregion.c | 265 +++++++++++++++++++++++++++++++++++++++
> virt/kvm/ioregion.h | 15 +++
> virt/kvm/kvm_main.c | 11 ++
> 11 files changed, 379 insertions(+)
> create mode 100644 virt/kvm/eventfd.h
> create mode 100644 virt/kvm/ioregion.c
> create mode 100644 virt/kvm/ioregion.h
>

(...)

> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index c2323c27a28b..aadb73903f8b 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -27,6 +27,7 @@
> #include <trace/events/kvm.h>
>
> #include <kvm/iodev.h>
> +#include "ioregion.h"
>
> #ifdef CONFIG_HAVE_KVM_IRQFD
>
> @@ -755,6 +756,23 @@ static const struct kvm_io_device_ops ioeventfd_ops = {
> .destructor = ioeventfd_destructor,
> };
>
> +#ifdef CONFIG_KVM_IOREGION
> +/* assumes kvm->slots_lock held */
> +bool kvm_eventfd_collides(struct kvm *kvm, int bus_idx,
> + u64 start, u64 size)
> +{
> + struct _ioeventfd *_p;
> +
> + list_for_each_entry(_p, &kvm->ioeventfds, list)
> + if (_p->bus_idx == bus_idx &&
> + overlap(start, size, _p->addr,
> + !_p->length ? 8 : _p->length))

I'm wondering whether we should enable a bus-specific overlap()
function; that would make it possible to wire up weird things like ccw.
But not really needed right now, and can be easily added when it
becomes relevant.

> + return true;
> +
> + return false;
> +}
> +#endif
> +
> /* assumes kvm->slots_lock held */
> static bool
> ioeventfd_check_collision(struct kvm *kvm, struct _ioeventfd *p)
> @@ -770,6 +788,13 @@ ioeventfd_check_collision(struct kvm *kvm, struct _ioeventfd *p)
> _p->datamatch == p->datamatch))))
> return true;
>
> +#ifdef CONFIG_KVM_IOREGION

This might benefit from a comment why you only check these two
(especially as you also check FAST_MMIO during ioregionfd setup.)

> + if (p->bus_idx == KVM_MMIO_BUS || p->bus_idx == KVM_PIO_BUS)
> + if (kvm_ioregion_collides(kvm, p->bus_idx, p->addr,
> + !p->length ? 8 : p->length))
> + return true;
> +#endif
> +
> return false;
> }
>

(...)