| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20241106043714-mutt-send-email-mst@kernel.org>
Date: Wed, 6 Nov 2024 04:37:57 -0500
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc: jasowang@...hat.co, xuanzhuo@...ux.alibaba.com, eperezma@...hat.com,
virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org,
mie@...l.co.jp
Subject: Re: [PATCH] virtio-pci: Add MSI support
On Fri, Jul 12, 2024 at 07:59:14PM +0530, Manivannan Sadhasivam wrote:
> Virtio spec has so far only supported MSI-X and INTX for receiving the
> interrupts from the virtio device on PCI transport. But this becomes a
> limiting factor for devices supporting only MSI (plus INTX emulation) as
> they have to use the legacy INTX emulation which is limited to one IRQ per
> PCIe function.
>
> But this now addressed with the help of a proposal to the Virtio spec
> adding MSI support [1]. Based on that, let's implement MSI support in the
> virtio-pci driver.
>
> The Virtio spec proposal reuses the existing MSI-X infrastructure, like the
> config_msix_vector/queue_msix_vector fields of the Virito common config
> structure. Following that, MSI support in virtio-pci driver is also added
> on top of the existing MSI-X implementation and it mostly reuses the MSI-X
> code base. The existing vp_find_vqs_msix() API is modified to support MSI
> along with MSI-X.
>
> The preference for interrupt allocation is still given to MSI-X as per the
> spec. The driver will try to allocate MSI only if both of the MSI-X
> allocations (one vector for each queue and 2 vectors) fails. As like MSI-X,
> driver will try to allocate one MSI vector for each queue first, and if
> that fails, it will try to allocate 2 vectors (one for config queue and one
> shared for queues). If both of them fails, driver will fallback to the
> legacy INTX as usual.
>
> For keeping the changes minimal, existing 'virtio_pci_device::msix_enabled'
> flag is used to indicate the status of MSI and MSI-X. Rest of the MSI-X
> functionalities such as IRQ affinity are also reused for MSI (but the
> affinity setting really depends on the underlying IRQCHIP controller).
>
> [1] https://lore.kernel.org/virtio-comment/20240712140144.12066-1-manivannan.sadhasivam@linaro.org/
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
So I think the spec versions are mostly acceptable to people.
So you want TC to vote as is or tweak it?
In any case, this patch has to be rebased to be applied.
> ---
> drivers/virtio/virtio_pci_common.c | 24 ++++++++++++++++++------
> drivers/virtio/virtio_pci_common.h | 2 +-
> 2 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index f6b0b00e4599..6f80b0c46c5f 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -100,11 +100,11 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
> }
>
> static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
> - bool per_vq_vectors, struct irq_affinity *desc)
> + bool per_vq_vectors, struct irq_affinity *desc,
> + unsigned int flags)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> const char *name = dev_name(&vp_dev->vdev.dev);
> - unsigned int flags = PCI_IRQ_MSIX;
> unsigned int i, v;
> int err = -ENOMEM;
>
> @@ -288,7 +288,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs,
> struct virtqueue *vqs[], vq_callback_t *callbacks[],
> const char * const names[], bool per_vq_vectors,
> const bool *ctx,
> - struct irq_affinity *desc)
> + struct irq_affinity *desc, unsigned int flags)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> u16 msix_vec;
> @@ -310,7 +310,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs,
> }
>
> err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors,
> - per_vq_vectors ? desc : NULL);
> + per_vq_vectors ? desc : NULL, flags);
> if (err)
> goto error_find;
>
> @@ -407,11 +407,23 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> int err;
>
> /* Try MSI-X with one vector per queue. */
> - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx, desc);
> + err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx,
> + desc, PCI_IRQ_MSIX);
> if (!err)
> return 0;
> /* Fallback: MSI-X with one vector for config, one shared for queues. */
> - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx, desc);
> + err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx,
> + desc, PCI_IRQ_MSIX);
> + if (!err)
> + return 0;
> + /* Try MSI with one vector per queue. */
> + err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx,
> + desc, PCI_IRQ_MSI);
> + if (!err)
> + return 0;
> + /* Fallback: MSI with one vector for config, one shared for queues. */
> + err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx,
> + desc, PCI_IRQ_MSI);
> if (!err)
> return 0;
> /* Is there an interrupt? If not give up. */
> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> index 7fef52bee455..a5062ca85f3b 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -77,7 +77,7 @@ struct virtio_pci_device {
>
> struct virtio_pci_admin_vq admin_vq;
>
> - /* MSI-X support */
> + /* MSI/MSI-X support */
> int msix_enabled;
> int intx_enabled;
> cpumask_var_t *msix_affinity_masks;
> --
> 2.25.1
Powered by blists - more mailing lists