Re: [PATCH v2] PCI/IOV: Use VF0 cached config space size for other VFs

["Raslan, KarimAllah" <karahmed@amazon.de>]

On Thu, 2018-10-11 at 11:51 -0500, Bjorn Helgaas wrote:
> On Wed, Oct 10, 2018 at 06:00:10PM +0200, KarimAllah Ahmed wrote:
> > 
> > Cache the config space size from VF0 and use it for all other VFs instead
> > of reading it from the config space of each VF. We assume that it will be
> > the same across all associated VFs.
> > 
> > This is an optimization when enabling SR-IOV on a device with many VFs.
> > 
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: linux-pci@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
> 
> Applied to pci/virtualization for v4.20, thanks!
> 
> As I mentioned last time, I think CONFIG_PCI_ATS is the wrong symbol to
> test here, so I changed that to CONFIG_PCI_IOV.

Ooops! Sorry, it has been a long time and I fogot :D

> I also moved the #ifdef wrapper so the caller doesn't need an ifdef.
> Please let me know if these break anything.  The patch I applied is appended.

Looks good to me. Thanks!

> > 
> > ---
> > v1 -> v2:
> > - Drop the __pci_cfg_space_size (bhelgaas@)
> > - Extend pci_cfg_space_size to return the cached value for all VFs except
> >   VF0 (bhelgaas@)
> > ---
> >  drivers/pci/iov.c   |  2 ++
> >  drivers/pci/pci.h   |  1 +
> >  drivers/pci/probe.c | 17 +++++++++++++++++
> >  3 files changed, 20 insertions(+)
> > 
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > index c5f3cd4e..4238b53 100644
> > --- a/drivers/pci/iov.c
> > +++ b/drivers/pci/iov.c
> > @@ -133,6 +133,8 @@ static void pci_read_vf_config_common(struct pci_dev *virtfn)
> >  			     &physfn->sriov->subsystem_vendor);
> >  	pci_read_config_word(virtfn, PCI_SUBSYSTEM_ID,
> >  			     &physfn->sriov->subsystem_device);
> > +
> > +	physfn->sriov->cfg_size = pci_cfg_space_size(virtfn);
> >  }
> >  
> >  int pci_iov_add_virtfn(struct pci_dev *dev, int id)
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > index 6e0d152..2f14542 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -285,6 +285,7 @@ struct pci_sriov {
> >  	u16		driver_max_VFs;	/* Max num VFs driver supports */
> >  	struct pci_dev	*dev;		/* Lowest numbered PF */
> >  	struct pci_dev	*self;		/* This PF */
> > +	u32		cfg_size;	/* VF config space size */
> >  	u32		class;		/* VF device */
> >  	u8		hdr_type;	/* VF header type */
> >  	u16		subsystem_vendor; /* VF subsystem vendor */
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 201f9e5..8c0f428 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1438,12 +1438,29 @@ static int pci_cfg_space_size_ext(struct pci_dev *dev)
> >  	return PCI_CFG_SPACE_EXP_SIZE;
> >  }
> >  
> > +#ifdef CONFIG_PCI_ATS
> > +static bool is_vf0(struct pci_dev *dev)
> > +{
> > +	if (pci_iov_virtfn_devfn(dev->physfn, 0) == dev->devfn &&
> > +	    pci_iov_virtfn_bus(dev->physfn, 0) == dev->bus->number)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +#endif
> > +
> >  int pci_cfg_space_size(struct pci_dev *dev)
> >  {
> >  	int pos;
> >  	u32 status;
> >  	u16 class;
> >  
> > +#ifdef CONFIG_PCI_ATS
> > +	/* Read cached value for all VFs except for VF0 */
> > +	if (dev->is_virtfn && !is_vf0(dev))
> > +		return dev->physfn->sriov->cfg_size;
> > +#endif
> > +
> >  	if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG)
> >  		return PCI_CFG_SPACE_SIZE;
> >  
> > -- 
> > 2.7.4
> > 
> 
> commit 601f9f6679157b70a7a4e752baa590bd2af69ffb
> Author: KarimAllah Ahmed <karahmed@amazon.de>
> Date:   Thu Oct 11 11:49:58 2018 -0500
> 
>     PCI/IOV: Use VF0 cached config space size for other VFs
>     
>     Cache the config space size from VF0 and use it for all other VFs instead
>     of reading it from the config space of each VF.  We assume that it will be
>     the same across all associated VFs.
>     
>     This is an optimization when enabling SR-IOV on a device with many VFs.
>     
>     Signed-off-by: KarimAllah Ahmed <karahmed@amazon.de>
>     [bhelgaas: use CONFIG_PCI_IOV (not CONFIG_PCI_ATS), adjust is_vf0() wrapper
>     so caller doesn't need ifdef]
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index c5f3cd4ed766..4238b539f9d8 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -133,6 +133,8 @@ static void pci_read_vf_config_common(struct pci_dev *virtfn)
>  			     &physfn->sriov->subsystem_vendor);
>  	pci_read_config_word(virtfn, PCI_SUBSYSTEM_ID,
>  			     &physfn->sriov->subsystem_device);
> +
> +	physfn->sriov->cfg_size = pci_cfg_space_size(virtfn);
>  }
>  
>  int pci_iov_add_virtfn(struct pci_dev *dev, int id)
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 6e0d1528d471..2f1454209257 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -285,6 +285,7 @@ struct pci_sriov {
>  	u16		driver_max_VFs;	/* Max num VFs driver supports */
>  	struct pci_dev	*dev;		/* Lowest numbered PF */
>  	struct pci_dev	*self;		/* This PF */
> +	u32		cfg_size;	/* VF config space size */
>  	u32		class;		/* VF device */
>  	u8		hdr_type;	/* VF header type */
>  	u16		subsystem_vendor; /* VF subsystem vendor */
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 201f9e5ff55c..9c20d9667e61 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1438,12 +1438,27 @@ static int pci_cfg_space_size_ext(struct pci_dev *dev)
>  	return PCI_CFG_SPACE_EXP_SIZE;
>  }
>  
> +static bool is_vf0(struct pci_dev *dev)
> +{
> +#ifdef CONFIG_PCI_IOV
> +	if (pci_iov_virtfn_devfn(dev->physfn, 0) == dev->devfn &&
> +	    pci_iov_virtfn_bus(dev->physfn, 0) == dev->bus->number)
> +		return true;
> +#endif
> +
> +	return false;
> +}
> +
>  int pci_cfg_space_size(struct pci_dev *dev)
>  {
>  	int pos;
>  	u32 status;
>  	u16 class;
>  
> +	/* Read cached value for all VFs except for VF0 */
> +	if (dev->is_virtfn && !is_vf0(dev))
> +		return dev->physfn->sriov->cfg_size;
> +
>  	if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG)
>  		return PCI_CFG_SPACE_SIZE;
>  
> 
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B