linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] PCI/IOV: Use VF0 cached config space size for other VFs
@ 2018-10-10 16:00 KarimAllah Ahmed
  2018-10-11 16:51 ` Bjorn Helgaas
  0 siblings, 1 reply; 3+ messages in thread
From: KarimAllah Ahmed @ 2018-10-10 16:00 UTC (permalink / raw)
  To: linux-kernel, linux-pci, bhelgaas; +Cc: KarimAllah Ahmed

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>

---
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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] PCI/IOV: Use VF0 cached config space size for other VFs
  2018-10-10 16:00 [PATCH v2] PCI/IOV: Use VF0 cached config space size for other VFs KarimAllah Ahmed
@ 2018-10-11 16:51 ` Bjorn Helgaas
  2018-10-11 16:59   ` Raslan, KarimAllah
  0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Helgaas @ 2018-10-11 16:51 UTC (permalink / raw)
  To: KarimAllah Ahmed; +Cc: linux-kernel, linux-pci, bhelgaas

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.  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.

> ---
> 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;
 

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] PCI/IOV: Use VF0 cached config space size for other VFs
  2018-10-11 16:51 ` Bjorn Helgaas
@ 2018-10-11 16:59   ` Raslan, KarimAllah
  0 siblings, 0 replies; 3+ messages in thread
From: Raslan, KarimAllah @ 2018-10-11 16:59 UTC (permalink / raw)
  To: helgaas; +Cc: linux-kernel, linux-pci, bhelgaas

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-10-11 16:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-10 16:00 [PATCH v2] PCI/IOV: Use VF0 cached config space size for other VFs KarimAllah Ahmed
2018-10-11 16:51 ` Bjorn Helgaas
2018-10-11 16:59   ` Raslan, KarimAllah

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).