linux-kernel - Re: [PATCH] PCI: Reset IOV state on FLR to PF

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: <20220112144903.GA253960@bhelgaas>
Date:   Wed, 12 Jan 2022 08:49:03 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Lukasz Maniak <lukasz.maniak@...ux.intel.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Łukasz Gieryk <lukasz.gieryk@...ux.intel.com>
Subject: Re: [PATCH] PCI: Reset IOV state on FLR to PF

On Wed, Dec 22, 2021 at 08:19:57PM +0100, Lukasz Maniak wrote:
> As per PCI Express specification, FLR to a PF resets the PF state as
> well as the SR-IOV extended capability including VF Enable which means
> that VFs no longer exist.

Can you add a specific reference to the spec, please?

> Currently, the IOV state is not updated during FLR, resulting in
> non-compliant PCI driver behavior.

And include a little detail about what problem is observed?  How would
a user know this problem is occurring?

> This patch introduces a simple function, called on the FLR path, that
> removes the virtual function devices from the PCI bus and their
> corresponding sysfs links with a final clear of the num_vfs value in IOV
> state.
> 
> Signed-off-by: Lukasz Maniak <lukasz.maniak@...ux.intel.com>
> ---
>  drivers/pci/iov.c | 21 +++++++++++++++++++++
>  drivers/pci/pci.c |  2 ++
>  drivers/pci/pci.h |  4 ++++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 0267977c9f17..69ee321027b4 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -1013,6 +1013,27 @@ int pci_iov_bus_range(struct pci_bus *bus)
>  	return max ? max - bus->number : 0;
>  }
>  
> +/**
> + * pci_reset_iov_state - reset the state of the IOV capability
> + * @dev: the PCI device
> + */
> +void pci_reset_iov_state(struct pci_dev *dev)
> +{
> +	struct pci_sriov *iov = dev->sriov;
> +
> +	if (!dev->is_physfn)
> +		return;
> +	if (!iov->num_VFs)
> +		return;
> +
> +	sriov_del_vfs(dev);
> +
> +	if (iov->link != dev->devfn)
> +		sysfs_remove_link(&dev->dev.kobj, "dep_link");
> +
> +	iov->num_VFs = 0;
> +}
> +
>  /**
>   * pci_enable_sriov - enable the SR-IOV capability
>   * @dev: the PCI device
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 3d2fb394986a..535f19d37e8d 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4694,6 +4694,8 @@ EXPORT_SYMBOL(pci_wait_for_pending_transaction);
>   */
>  int pcie_flr(struct pci_dev *dev)
>  {
> +	pci_reset_iov_state(dev);
> +
>  	if (!pci_wait_for_pending_transaction(dev))
>  		pci_err(dev, "timed out waiting for pending transaction; performing function level reset anyway\n");
>  
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 3d60cabde1a1..7bb144fbec76 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -480,6 +480,7 @@ void pci_iov_update_resource(struct pci_dev *dev, int resno);
>  resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno);
>  void pci_restore_iov_state(struct pci_dev *dev);
>  int pci_iov_bus_range(struct pci_bus *bus);
> +void pci_reset_iov_state(struct pci_dev *dev);
>  extern const struct attribute_group sriov_pf_dev_attr_group;
>  extern const struct attribute_group sriov_vf_dev_attr_group;
>  #else
> @@ -501,6 +502,9 @@ static inline int pci_iov_bus_range(struct pci_bus *bus)
>  {
>  	return 0;
>  }
> +static inline void pci_reset_iov_state(struct pci_dev *dev)
> +{
> +}
>  
>  #endif /* CONFIG_PCI_IOV */
>  
> 
> base-commit: fa55b7dcdc43c1aa1ba12bca9d2dd4318c2a0dbf
> -- 
> 2.25.1
>