netdev - Re: [PATCH V1 mlx5-next 12/13] vfio/pci: Add infrastructure to let vfio_pci

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211015135237.759fe688.alex.williamson@redhat.com>
Date:   Fri, 15 Oct 2021 13:52:37 -0600
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Yishai Hadas <yishaih@...dia.com>
Cc:     <bhelgaas@...gle.com>, <jgg@...dia.com>, <saeedm@...dia.com>,
        <linux-pci@...r.kernel.org>, <kvm@...r.kernel.org>,
        <netdev@...r.kernel.org>, <kuba@...nel.org>, <leonro@...dia.com>,
        <kwankhede@...dia.com>, <mgurtovoy@...dia.com>, <maorg@...dia.com>
Subject: Re: [PATCH V1 mlx5-next 12/13] vfio/pci: Add infrastructure to let
 vfio_pci_core drivers trap device RESET

On Wed, 13 Oct 2021 12:47:06 +0300
Yishai Hadas <yishaih@...dia.com> wrote:

> Add infrastructure to let vfio_pci_core drivers trap device RESET.
> 
> The motivation for this is to let the underlay driver be aware that
> reset was done and set its internal state accordingly.

I think the intention of the uAPI here is that the migration error
state is exited specifically via the reset ioctl.  Maybe that should be
made more clear, but variant drivers can already wrap the core ioctl
for the purpose of determining that mechanism of reset has occurred.
Thanks,

Alex

 
> Signed-off-by: Yishai Hadas <yishaih@...dia.com>
> Signed-off-by: Leon Romanovsky <leonro@...dia.com>
> ---
>  drivers/vfio/pci/vfio_pci_config.c |  8 ++++++--
>  drivers/vfio/pci/vfio_pci_core.c   |  2 ++
>  include/linux/vfio_pci_core.h      | 10 ++++++++++
>  3 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index 6e58b4bf7a60..002198376f43 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -859,7 +859,9 @@ static int vfio_exp_config_write(struct vfio_pci_core_device *vdev, int pos,
>  
>  		if (!ret && (cap & PCI_EXP_DEVCAP_FLR)) {
>  			vfio_pci_zap_and_down_write_memory_lock(vdev);
> -			pci_try_reset_function(vdev->pdev);
> +			ret = pci_try_reset_function(vdev->pdev);
> +			if (!ret && vdev->ops && vdev->ops->reset_done)
> +				vdev->ops->reset_done(vdev);
>  			up_write(&vdev->memory_lock);
>  		}
>  	}
> @@ -941,7 +943,9 @@ static int vfio_af_config_write(struct vfio_pci_core_device *vdev, int pos,
>  
>  		if (!ret && (cap & PCI_AF_CAP_FLR) && (cap & PCI_AF_CAP_TP)) {
>  			vfio_pci_zap_and_down_write_memory_lock(vdev);
> -			pci_try_reset_function(vdev->pdev);
> +			ret = pci_try_reset_function(vdev->pdev);
> +			if (!ret && vdev->ops && vdev->ops->reset_done)
> +				vdev->ops->reset_done(vdev);
>  			up_write(&vdev->memory_lock);
>  		}
>  	}
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index e581a327f90d..d2497a8ed7f1 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -923,6 +923,8 @@ long vfio_pci_core_ioctl(struct vfio_device *core_vdev, unsigned int cmd,
>  
>  		vfio_pci_zap_and_down_write_memory_lock(vdev);
>  		ret = pci_try_reset_function(vdev->pdev);
> +		if (!ret && vdev->ops && vdev->ops->reset_done)
> +			vdev->ops->reset_done(vdev);
>  		up_write(&vdev->memory_lock);
>  
>  		return ret;
> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
> index ef9a44b6cf5d..6ccf5824f098 100644
> --- a/include/linux/vfio_pci_core.h
> +++ b/include/linux/vfio_pci_core.h
> @@ -95,6 +95,15 @@ struct vfio_pci_mmap_vma {
>  	struct list_head	vma_next;
>  };
>  
> +/**
> + * struct vfio_pci_core_device_ops - VFIO PCI driver device callbacks
> + *
> + * @reset_done: Called when the device was reset
> + */
> +struct vfio_pci_core_device_ops {
> +	void	(*reset_done)(struct vfio_pci_core_device *vdev);
> +};
> +
>  struct vfio_pci_core_device {
>  	struct vfio_device	vdev;
>  	struct pci_dev		*pdev;
> @@ -137,6 +146,7 @@ struct vfio_pci_core_device {
>  	struct mutex		vma_lock;
>  	struct list_head	vma_list;
>  	struct rw_semaphore	memory_lock;
> +	const struct vfio_pci_core_device_ops *ops;
>  };
>  
>  #define is_intx(vdev) (vdev->irq_type == VFIO_PCI_INTX_IRQ_INDEX)