On Sun, Jan 09, 2022 at 02:33:43PM +0100, Cédric Le Goater wrote:
> Hello Cindy,
>
> On 1/8/22 02:04, Michael S. Tsirkin wrote:
> > From: Cindy Lu <l...@redhat.com>
> >
> > Add support for configure interrupt, The process is used kvm_irqfd_assign
> > to set the gsi to kernel. When the configure notifier was signal by
> > host, qemu will inject a msix interrupt to guest
> >
> > Signed-off-by: Cindy Lu <l...@redhat.com>
> > Message-Id: <20211104164827.21911-11-l...@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> > hw/virtio/virtio-pci.h | 4 +-
> > hw/virtio/virtio-pci.c | 92 ++++++++++++++++++++++++++++++++++++------
> > 2 files changed, 83 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> > index 2446dcd9ae..b704acc5a8 100644
> > --- a/hw/virtio/virtio-pci.h
> > +++ b/hw/virtio/virtio-pci.h
> > @@ -251,5 +251,7 @@ void virtio_pci_types_register(const
> > VirtioPCIDeviceTypeInfo *t);
> > * @fixed_queues.
> > */
> > unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
> > -
> > +void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev,
> > VirtQueue *vq,
> > + int n, bool assign,
> > + bool with_irqfd);
> > #endif
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 7201cf3dc1..98fb5493ae 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -727,7 +727,8 @@ static int virtio_pci_get_notifier(VirtIOPCIProxy
> > *proxy, int queue_no,
> > VirtQueue *vq;
> > if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> > - return -1;
> > + *n = virtio_config_get_guest_notifier(vdev);
> > + *vector = vdev->config_vector;
> > } else {
> > if (!virtio_queue_get_num(vdev, queue_no)) {
> > return -1;
> > @@ -802,6 +803,10 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy
> > *proxy, int nvqs)
> > return ret;
> > }
> > +static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
> > +{
> > + return kvm_virtio_pci_vector_use_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
> > +}
> > static void kvm_virtio_pci_vector_release_one(VirtIOPCIProxy *proxy,
> > int queue_no)
> > @@ -839,6 +844,11 @@ static void
> > kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> > }
> > }
> > +static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
> > +{
> > + kvm_virtio_pci_vector_release_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
> > +}
> > +
> > static int virtio_pci_one_vector_unmask(VirtIOPCIProxy *proxy,
> > unsigned int queue_no,
> > unsigned int vector,
> > @@ -920,9 +930,17 @@ static int virtio_pci_vector_unmask(PCIDevice *dev,
> > unsigned vector,
> > }
> > vq = virtio_vector_next_queue(vq);
> > }
> > -
> > + /* unmask config intr */
> > + n = virtio_config_get_guest_notifier(vdev);
> > + ret = virtio_pci_one_vector_unmask(proxy, VIRTIO_CONFIG_IRQ_IDX,
> > vector,
> > + msg, n);
> > + if (ret < 0) {
> > + goto undo_config;
> > + }
> > return 0;
> > -
> > +undo_config:
> > + n = virtio_config_get_guest_notifier(vdev);
> > + virtio_pci_one_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
> > undo:
> > vq = virtio_vector_first_queue(vdev, vector);
> > while (vq && unmasked >= 0) {
> > @@ -956,6 +974,8 @@ static void virtio_pci_vector_mask(PCIDevice *dev,
> > unsigned vector)
> > }
> > vq = virtio_vector_next_queue(vq);
> > }
> > + n = virtio_config_get_guest_notifier(vdev);
> > + virtio_pci_one_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
> > }
> > static void virtio_pci_vector_poll(PCIDevice *dev,
> > @@ -987,6 +1007,34 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
> > msix_set_pending(dev, vector);
> > }
> > }
> > + /* poll the config intr */
> > + ret = virtio_pci_get_notifier(proxy, VIRTIO_CONFIG_IRQ_IDX, ¬ifier,
> > + &vector);
> > + if (ret < 0) {
> > + return;
> > + }
> > + if (vector < vector_start || vector >= vector_end ||
> > + !msix_is_masked(dev, vector)) {
> > + return;
> > + }
> > + if (k->guest_notifier_pending) {
> > + if (k->guest_notifier_pending(vdev, VIRTIO_CONFIG_IRQ_IDX)) {
> > + msix_set_pending(dev, vector);
> > + }
> > + } else if (event_notifier_test_and_clear(notifier)) {
> > + msix_set_pending(dev, vector);
> > + }
> > +}
> > +
> > +void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev,
> > VirtQueue *vq,
> > + int n, bool assign,
> > + bool with_irqfd)
> > +{
> > + if (n == VIRTIO_CONFIG_IRQ_IDX) {
> > + virtio_config_set_guest_notifier_fd_handler(vdev, assign,
> > with_irqfd);
> > + } else {
> > + virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);
> > + }
> > }
> > static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool
> > assign,
> > @@ -995,17 +1043,25 @@ static int virtio_pci_set_guest_notifier(DeviceState
> > *d, int n, bool assign,
> > VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> > - VirtQueue *vq = virtio_get_queue(vdev, n);
> > - EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
> > + VirtQueue *vq = NULL;
> > + EventNotifier *notifier = NULL;
> > +
> > + if (n == VIRTIO_CONFIG_IRQ_IDX) {
> > + notifier = virtio_config_get_guest_notifier(vdev);
> > + } else {
> > + vq = virtio_get_queue(vdev, n);
> > + notifier = virtio_queue_get_guest_notifier(vq);
> > + }
> > if (assign) {
> > int r = event_notifier_init(notifier, 0);
> > if (r < 0) {
> > return r;
> > }
> > - virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
> > + virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, true,
> > with_irqfd);
> > } else {
> > - virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
> > + virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, false,
> > + with_irqfd);
> > event_notifier_cleanup(notifier);
> > }
> > @@ -1047,6 +1103,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState
> > *d, int nvqs, bool assign)
> > msix_unset_vector_notifiers(&proxy->pci_dev);
> > if (proxy->vector_irqfd) {
> > kvm_virtio_pci_vector_release(proxy, nvqs);
> > + kvm_virtio_pci_vector_config_release(proxy);
> > g_free(proxy->vector_irqfd);
> > proxy->vector_irqfd = NULL;
> > }
> > @@ -1062,7 +1119,11 @@ static int
> > virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> > goto assign_error;
> > }
> > }
> > -
> > + r = virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, assign,
> > + with_irqfd);
> > + if (r < 0) {
> > + goto config_assign_error;
> > + }
> > /* Must set vector notifier after guest notifier has been assigned */
> > if ((with_irqfd || k->guest_notifier_mask) && assign) {
> > if (with_irqfd) {
> > @@ -1071,11 +1132,14 @@ static int
> > virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> > msix_nr_vectors_allocated(&proxy->pci_dev));
> > r = kvm_virtio_pci_vector_use(proxy, nvqs);
> > if (r < 0) {
> > - goto assign_error;
> > + goto config_assign_error;
> > }
> > }
> > - r = msix_set_vector_notifiers(&proxy->pci_dev,
> > - virtio_pci_vector_unmask,
> > + r = kvm_virtio_pci_vector_config_use(proxy);
> > + if (r < 0) {
> > + goto config_error;
> > + }
>
>
> This is crashing a QEMU TCG machine using vhost (no irqfd). Below is a fix
> but I doubt it is complete.
>
> Thanks,
>
> C.
>
>
> From dde4d7b21c851a33d2d03bddd18464ae4e777a3f Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <c...@kaod.org>
> Date: Sun, 9 Jan 2022 10:40:47 +0100
> Subject: [PATCH] vhost: Fix support for configure interrupt
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> irqfd is not necessarily in use (TCG using vhost).
>
> Cc: Cindy Lu <l...@redhat.com>
> Fixes: d5d24d859c39 ("virtio-pci: add support for configure interrupt")
> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> ---
> hw/virtio/virtio-pci.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 98fb5493ae06..39ebb042860d 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1134,10 +1134,12 @@ static int virtio_pci_set_guest_notifiers(DeviceState
> *d, int nvqs, bool assign)
> if (r < 0) {
> goto config_assign_error;
> }
> - }
> - r = kvm_virtio_pci_vector_config_use(proxy);
> - if (r < 0) {
> - goto config_error;
> +
> + r = kvm_virtio_pci_vector_config_use(proxy);
> + if (r < 0) {
> + abort();
> + goto config_error;
> + }
> }
> r = msix_set_vector_notifiers(&proxy->pci_dev,
> virtio_pci_vector_unmask,
> virtio_pci_vector_mask,
> --
> 2.31.1
Yes, we need to fix up cleanup too, but besides that it seems good,
right?
So like this then? Cindy?
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 98fb5493ae..722ce12b45 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1134,10 +1134,10 @@ static int virtio_pci_set_guest_notifiers(DeviceState
*d, int nvqs, bool assign)
if (r < 0) {
goto config_assign_error;
}
- }
- r = kvm_virtio_pci_vector_config_use(proxy);
- if (r < 0) {
- goto config_error;
+ r = kvm_virtio_pci_vector_config_use(proxy);
+ if (r < 0) {
+ goto config_error;
+ }
}
r = msix_set_vector_notifiers(&proxy->pci_dev,
virtio_pci_vector_unmask,
virtio_pci_vector_mask,
@@ -1155,7 +1155,9 @@ notifiers_error:
kvm_virtio_pci_vector_release(proxy, nvqs);
}
config_error:
- kvm_virtio_pci_vector_config_release(proxy);
+ if (with_irqfd) {
+ kvm_virtio_pci_vector_config_release(proxy);
+ }
config_assign_error:
virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, !assign,
with_irqfd);
