linux-kernel - Re: [PATCH] PCI: Introduce flag for detached virtual functions

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: <20200812104351.3668cc0f@x1.home>
Date:   Wed, 12 Aug 2020 10:43:51 -0600
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Matthew Rosato <mjrosato@...ux.ibm.com>
Cc:     bhelgaas@...gle.com, schnelle@...ux.ibm.com, pmorel@...ux.ibm.com,
        mpe@...erman.id.au, oohall@...il.com, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-pci@...r.kernel.org
Subject: Re: [PATCH] PCI: Introduce flag for detached virtual functions

On Wed, 12 Aug 2020 10:50:17 -0400
Matthew Rosato <mjrosato@...ux.ibm.com> wrote:

> s390x has the notion of providing VFs to the kernel in a manner
> where the associated PF is inaccessible other than via firmware.
> These are not treated as typical VFs and access to them is emulated
> by underlying firmware which can still access the PF.  After
> abafbc55 however these detached VFs were no longer able to work
> with vfio-pci as the firmware does not provide emulation of the
> PCI_COMMAND_MEMORY bit.  In this case, let's explicitly recognize
> these detached VFs so that vfio-pci can allow memory access to
> them again.
> 
> Signed-off-by: Matthew Rosato <mjrosato@...ux.ibm.com>
> Reviewed-by: Pierre Morel <pmorel@...ux.ibm.com>
> Reviewed-by: Niklas Schnelle <schnelle@...ux.ibm.com>
> ---
>  arch/s390/pci/pci.c                | 8 ++++++++
>  drivers/vfio/pci/vfio_pci_config.c | 3 ++-
>  include/linux/pci.h                | 1 +
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 3902c9f..04ac76d 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -581,6 +581,14 @@ int pcibios_enable_device(struct pci_dev *pdev, int mask)
>  {
>  	struct zpci_dev *zdev = to_zpci(pdev);
>  
> +	/*
> +	 * If we have a VF on a non-multifunction bus, it must be a VF that is
> +	 * detached from its parent PF.  We rely on firmware emulation to
> +	 * provide underlying PF details.
> +	 */
> +	if (zdev->vfn && !zdev->zbus->multifunction)
> +		pdev->detached_vf = 1;
> +
>  	zpci_debug_init_device(zdev, dev_name(&pdev->dev));
>  	zpci_fmb_enable_device(zdev);
>  
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index d98843f..17845fc 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -406,7 +406,8 @@ bool __vfio_pci_memory_enabled(struct vfio_pci_device *vdev)
>  	 * PF SR-IOV capability, there's therefore no need to trigger
>  	 * faults based on the virtual value.
>  	 */
> -	return pdev->is_virtfn || (cmd & PCI_COMMAND_MEMORY);
> +	return pdev->is_virtfn || pdev->detached_vf ||
> +	       (cmd & PCI_COMMAND_MEMORY);
>  }
>  
>  /*

Wouldn't we also want to enable the is_virtfn related code in
vfio_basic_config_read() and at least the initial setting of the
command register in vfio_config_init()?  Otherwise we're extending the
incomplete emulation out to userspace.  Thanks,

Alex


> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 8355306..23a6972 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -445,6 +445,7 @@ struct pci_dev {
>  	unsigned int	is_probed:1;		/* Device probing in progress */
>  	unsigned int	link_active_reporting:1;/* Device capable of reporting link active */
>  	unsigned int	no_vf_scan:1;		/* Don't scan for VFs after IOV enablement */
> +	unsigned int	detached_vf:1;		/* VF without local PF access */
>  	pci_dev_flags_t dev_flags;
>  	atomic_t	enable_cnt;	/* pci_enable_device has been called */
>