| 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: <20240205153502.051ff2bf.alex.williamson@redhat.com>
Date: Mon, 5 Feb 2024 15:35:02 -0700
From: Alex Williamson <alex.williamson@...hat.com>
To: Reinette Chatre <reinette.chatre@...el.com>
Cc: jgg@...dia.com, yishaih@...dia.com,
shameerali.kolothum.thodi@...wei.com, kevin.tian@...el.com,
kvm@...r.kernel.org, dave.jiang@...el.com, ashok.raj@...el.com,
linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH 06/17] vfio/pci: Remove interrupt index interpretation
from wrappers
On Thu, 1 Feb 2024 20:57:00 -0800
Reinette Chatre <reinette.chatre@...el.com> wrote:
> vfio_pci_set_intx_trigger() and vfio_pci_set_msi_trigger() have similar
> enough flows that can be converged into one shared flow instead of
> duplicated.
>
> To share code between management of interrupt types it is necessary that
> the type of the interrupt is only interpreted by the code specific to
> the interrupt type being managed.
>
> Remove interrupt type interpretation from what will eventually be
> shared code (vfio_pci_set_msi_trigger(), vfio_msi_set_block()) by
> pushing this interpretation to be within the interrupt
> type specific code (vfio_msi_enable(),
> (temporary) vfio_msi_set_vector_signal()).
>
> Signed-off-by: Reinette Chatre <reinette.chatre@...el.com>
> ---
> Note to maintainers:
> Originally formed part of the IMS submission below, but is not
> specific to IMS.
> https://lore.kernel.org/lkml/cover.1696609476.git.reinette.chatre@intel.com
>
> drivers/vfio/pci/vfio_pci_intrs.c | 38 +++++++++++++++++--------------
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c
> index 97a3bb22b186..31f73c70fcd2 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -346,16 +346,19 @@ static irqreturn_t vfio_msihandler(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> -static int vfio_msi_enable(struct vfio_pci_core_device *vdev, int nvec, bool msix)
> +static int vfio_msi_enable(struct vfio_pci_core_device *vdev, int nvec,
> + unsigned int index)
> {
> struct pci_dev *pdev = vdev->pdev;
> - unsigned int flag = msix ? PCI_IRQ_MSIX : PCI_IRQ_MSI;
> + unsigned int flag;
> int ret;
> u16 cmd;
>
> if (!is_irq_none(vdev))
> return -EINVAL;
>
> + flag = (index == VFIO_PCI_MSIX_IRQ_INDEX) ? PCI_IRQ_MSIX : PCI_IRQ_MSI;
> +
> /* return the number of supported vectors if we can't get all: */
> cmd = vfio_pci_memory_lock_and_enable(vdev);
> ret = pci_alloc_irq_vectors(pdev, 1, nvec, flag);
> @@ -367,10 +370,9 @@ static int vfio_msi_enable(struct vfio_pci_core_device *vdev, int nvec, bool msi
> }
> vfio_pci_memory_unlock_and_restore(vdev, cmd);
>
> - vdev->irq_type = msix ? VFIO_PCI_MSIX_IRQ_INDEX :
> - VFIO_PCI_MSI_IRQ_INDEX;
> + vdev->irq_type = index;
>
> - if (!msix) {
> + if (index == VFIO_PCI_MSI_IRQ_INDEX) {
> /*
> * Compute the virtual hardware field for max msi vectors -
> * it is the log base 2 of the number of vectors.
> @@ -413,8 +415,10 @@ static int vfio_msi_alloc_irq(struct vfio_pci_core_device *vdev,
> }
>
> static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev,
> - unsigned int vector, int fd, bool msix)
> + unsigned int vector, int fd,
> + unsigned int index)
> {
> + bool msix = (index == VFIO_PCI_MSIX_IRQ_INDEX) ? true : false;
Cleanup the unnecessary ternary while here, this can just be:
bool msix = (index == VFIO_PCI_MSIX_IRQ_INDEX);
Thanks,
Alex
> struct pci_dev *pdev = vdev->pdev;
> struct vfio_pci_irq_ctx *ctx;
> struct eventfd_ctx *trigger;
> @@ -505,25 +509,26 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev,
>
> static int vfio_msi_set_block(struct vfio_pci_core_device *vdev,
> unsigned int start, unsigned int count,
> - int32_t *fds, bool msix)
> + int32_t *fds, unsigned int index)
> {
> unsigned int i, j;
> int ret = 0;
>
> for (i = 0, j = start; i < count && !ret; i++, j++) {
> int fd = fds ? fds[i] : -1;
> - ret = vfio_msi_set_vector_signal(vdev, j, fd, msix);
> + ret = vfio_msi_set_vector_signal(vdev, j, fd, index);
> }
>
> if (ret) {
> for (i = start; i < j; i++)
> - vfio_msi_set_vector_signal(vdev, i, -1, msix);
> + vfio_msi_set_vector_signal(vdev, i, -1, index);
> }
>
> return ret;
> }
>
> -static void vfio_msi_disable(struct vfio_pci_core_device *vdev, bool msix)
> +static void vfio_msi_disable(struct vfio_pci_core_device *vdev,
> + unsigned int index)
> {
> struct pci_dev *pdev = vdev->pdev;
> struct vfio_pci_irq_ctx *ctx;
> @@ -533,7 +538,7 @@ static void vfio_msi_disable(struct vfio_pci_core_device *vdev, bool msix)
> xa_for_each(&vdev->ctx, i, ctx) {
> vfio_virqfd_disable(&ctx->unmask);
> vfio_virqfd_disable(&ctx->mask);
> - vfio_msi_set_vector_signal(vdev, i, -1, msix);
> + vfio_msi_set_vector_signal(vdev, i, -1, index);
> }
>
> cmd = vfio_pci_memory_lock_and_enable(vdev);
> @@ -656,10 +661,9 @@ static int vfio_pci_set_msi_trigger(struct vfio_pci_core_device *vdev,
> {
> struct vfio_pci_irq_ctx *ctx;
> unsigned int i;
> - bool msix = (index == VFIO_PCI_MSIX_IRQ_INDEX) ? true : false;
>
> if (irq_is(vdev, index) && !count && (flags & VFIO_IRQ_SET_DATA_NONE)) {
> - vfio_msi_disable(vdev, msix);
> + vfio_msi_disable(vdev, index);
> return 0;
> }
>
> @@ -672,15 +676,15 @@ static int vfio_pci_set_msi_trigger(struct vfio_pci_core_device *vdev,
>
> if (vdev->irq_type == index)
> return vfio_msi_set_block(vdev, start, count,
> - fds, msix);
> + fds, index);
>
> - ret = vfio_msi_enable(vdev, start + count, msix);
> + ret = vfio_msi_enable(vdev, start + count, index);
> if (ret)
> return ret;
>
> - ret = vfio_msi_set_block(vdev, start, count, fds, msix);
> + ret = vfio_msi_set_block(vdev, start, count, fds, index);
> if (ret)
> - vfio_msi_disable(vdev, msix);
> + vfio_msi_disable(vdev, index);
>
> return ret;
> }
Powered by blists - more mailing lists