linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: Support SRIOV on Legacy EndPoint device
@ 2016-02-04 14:48 Kelly Zytaruk
  2016-02-04 15:14 ` Bjorn Helgaas
  0 siblings, 1 reply; 9+ messages in thread
From: Kelly Zytaruk @ 2016-02-04 14:48 UTC (permalink / raw)
  To: bhelgaas, linux-pci, linux-kernel; +Cc: Kelly Zytaruk

Some AMD GPUs have hardware support for grapics SRIOV.
If the GPU has a display output then the GPU needs to support Legacy VGA operation.
If CLASS_CODE = VGA then the device should have a Port Type = Legacy EndPoint.
Therefore in order to enable SRIOV on a GPU with a display output LEGACY_END_POINT is supported as a valid Port Type.

Signed-off-by: Kelly Zytaruk <kelly.zytaruk@amd.com>
---
 drivers/pci/iov.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 31f31d4..da4fbac 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -388,7 +388,8 @@ static int sriov_init(struct pci_dev *dev, int pos)
 	struct pci_dev *pdev;
 
 	if (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END &&
-	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT)
+	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT &&
+	    pci_pcie_type(dev) != PCI_EXP_TYPE_LEG_END)
 		return -ENODEV;
 
 	pci_read_config_word(dev, pos + PCI_SRIOV_CTRL, &ctrl);
-- 
1.7.10.4


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

* [PATCH] PCI: Support SRIOV on Legacy EndPoint device
@ 2016-02-04 15:08 Zytaruk, Kelly
  0 siblings, 0 replies; 9+ messages in thread
From: Zytaruk, Kelly @ 2016-02-04 15:08 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
  Cc: Zytaruk, Kelly

Some AMD GPUs have hardware support for grapics SRIOV.
If the GPU has a display output then the GPU needs to support Legacy VGA operation.
If CLASS_CODE = VGA then the device should have a Port Type = Legacy EndPoint.
Therefore in order to enable SRIOV on a GPU with a display output LEGACY_END_POINT is supported as a valid Port Type.

Signed-off-by: Kelly Zytaruk <kelly.zytaruk@amd.com>
---
 drivers/pci/iov.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index 31f31d4..da4fbac 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -388,7 +388,8 @@ static int sriov_init(struct pci_dev *dev, int pos)
 	struct pci_dev *pdev;
 
 	if (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END &&
-	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT)
+	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT &&
+	    pci_pcie_type(dev) != PCI_EXP_TYPE_LEG_END)
 		return -ENODEV;
 
 	pci_read_config_word(dev, pos + PCI_SRIOV_CTRL, &ctrl);
--
1.7.10.4

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

* Re: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
  2016-02-04 14:48 [PATCH] PCI: Support SRIOV on Legacy EndPoint device Kelly Zytaruk
@ 2016-02-04 15:14 ` Bjorn Helgaas
  2016-02-04 16:21   ` Zytaruk, Kelly
  0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2016-02-04 15:14 UTC (permalink / raw)
  To: Kelly Zytaruk; +Cc: bhelgaas, linux-pci, linux-kernel, Alex Williamson, Yu Zhao

[+cc Alex, Yu (participants in previous discussion)]

Hi Kelly,

On Thu, Feb 04, 2016 at 09:48:26AM -0500, Kelly Zytaruk wrote:
> Some AMD GPUs have hardware support for grapics SRIOV.
> If the GPU has a display output then the GPU needs to support Legacy VGA operation.
> If CLASS_CODE = VGA then the device should have a Port Type = Legacy EndPoint.
> Therefore in order to enable SRIOV on a GPU with a display output LEGACY_END_POINT is supported as a valid Port Type.
> 
> Signed-off-by: Kelly Zytaruk <kelly.zytaruk@amd.com>

We had an interesting discussion about this two years ago:
http://lkml.kernel.org/r/B756807489D6244883AC0B799A6EEC15EAB2E8@storexdag02.amd.com

Please include a reference to that discussion in your changelog.  In that
discussion, I also asked for some details (dmesg and lspci info) that
motivate the change, so please collect and add a reference to them as well.

It's not clear to me why we check the device type at all.  If there's no
spec restriction on the types of devices that can have an SR-IOV
capability, we should consider removing the test altogether (Alex mentioned
this possiblity in the earlier discussion).

Bjorn

> ---
>  drivers/pci/iov.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 31f31d4..da4fbac 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -388,7 +388,8 @@ static int sriov_init(struct pci_dev *dev, int pos)
>  	struct pci_dev *pdev;
>  
>  	if (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END &&
> -	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT)
> +	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT &&
> +	    pci_pcie_type(dev) != PCI_EXP_TYPE_LEG_END)
>  		return -ENODEV;
>  
>  	pci_read_config_word(dev, pos + PCI_SRIOV_CTRL, &ctrl);
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* RE: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
  2016-02-04 15:14 ` Bjorn Helgaas
@ 2016-02-04 16:21   ` Zytaruk, Kelly
  2016-02-05 16:46     ` Bjorn Helgaas
  0 siblings, 1 reply; 9+ messages in thread
From: Zytaruk, Kelly @ 2016-02-04 16:21 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: bhelgaas@google.com, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, Alex Williamson, Yu Zhao

Hi Bjorn, It been a long time.

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: Thursday, February 04, 2016 10:14 AM
> To: Zytaruk, Kelly
> Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; Alex Williamson; Yu Zhao
> Subject: Re: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
> 
> [+cc Alex, Yu (participants in previous discussion)]
> 
> Hi Kelly,
> 
> On Thu, Feb 04, 2016 at 09:48:26AM -0500, Kelly Zytaruk wrote:
> > Some AMD GPUs have hardware support for grapics SRIOV.
> > If the GPU has a display output then the GPU needs to support Legacy VGA
> operation.
> > If CLASS_CODE = VGA then the device should have a Port Type = Legacy
> EndPoint.
> > Therefore in order to enable SRIOV on a GPU with a display output
> LEGACY_END_POINT is supported as a valid Port Type.
> >
> > Signed-off-by: Kelly Zytaruk <kelly.zytaruk@amd.com>
> 
> We had an interesting discussion about this two years ago:
> http://lkml.kernel.org/r/B756807489D6244883AC0B799A6EEC15EAB2E8@store
> xdag02.amd.com

Unfortunately 2 years ago I couldn't complete your request as it would have disclosed Information about an unannounced technology that we were working on.  We have recently made that technology public and I can now send you the requested information.

> 
> Please include a reference to that discussion in your changelog.  In that
> discussion, I also asked for some details (dmesg and lspci info) that motivate the
> change, so please collect and add a reference to them as well.

The information that you ask for is included below.  I have abbreviated it so that this does not become a huge email.  I can send full logs if you want them.

'lspci' before enabling sriov

00:14.7 SD Host controller: Advanced Micro Devices [AMD] FCH SD Flash Controller
00:15.0 PCI bridge: Advanced Micro Devices [AMD] Hudson PCI to PCI bridge (PCIE port 0)
00:15.2 PCI bridge: Advanced Micro Devices [AMD] Hudson PCI to PCI bridge (PCIE port 2)
00:18.0 Host bridge: Advanced Micro Devices [AMD] Device 141a
00:18.1 Host bridge: Advanced Micro Devices [AMD] Device 141b
00:18.2 Host bridge: Advanced Micro Devices [AMD] Device 141c
00:18.3 Host bridge: Advanced Micro Devices [AMD] Device 141d
00:18.4 Host bridge: Advanced Micro Devices [AMD] Device 141e
00:18.5 Host bridge: Advanced Micro Devices [AMD] Device 141f
01:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI Device 6929
05:00.0 FireWire (IEEE 1394): VIA Technologies, Inc. VT6315 Series Firewire Controller (rev 01)
06:00.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5761 Gigabit Ethernet PCIe (rev 10)

'lspci -vvv' showing the sriov config space after sriov is enbled.  Note that I have enabled 4 VFs.

        Capabilities: [328 v1] Alternative Routing-ID Interpretation (ARI)
                ARICap: MFVC- ACS-, Next Function: 0
                ARICtl: MFVC- ACS-, Function Group: 0
        Capabilities: [330 v1] Single Root I/O Virtualization (SR-IOV)
                IOVCap: Migration-, Interrupt Message Number: 000
                IOVCtl: Enable+ Migration- Interrupt- MSE+ ARIHierarchy+
                IOVSta: Migration-
                Initial VFs: 16, Total VFs: 16, Number of VFs: 4, Function Dependency Link: 00
                VF offset: 256, stride: 1, Device ID: 692f
                Supported Page Size: 00000553, System Page Size: 00000001
                Region 0: Memory at 0000000050000000 (64-bit, prefetchable)
                Region 2: Memory at 00000000d0000000 (64-bit, prefetchable)
                Region 5: Memory at d5000000 (32-bit, non-prefetchable)
                VF Migration: offset: 00000000, BIR: 0
        Capabilities: [400 v1] Vendor Specific Information: ID=0002 Rev=1 Len=070 <?>

'lspci' after enabling sriov on the device.  Four VFs are created on Bus 2. The PF is indicated as device ID 6929 and the VFs are device ID 692f.

00:14.7 SD Host controller: Advanced Micro Devices [AMD] FCH SD Flash Controller
00:15.0 PCI bridge: Advanced Micro Devices [AMD] Hudson PCI to PCI bridge (PCIE port 0)
00:15.2 PCI bridge: Advanced Micro Devices [AMD] Hudson PCI to PCI bridge (PCIE port 2)
00:18.0 Host bridge: Advanced Micro Devices [AMD] Device 141a
00:18.1 Host bridge: Advanced Micro Devices [AMD] Device 141b
00:18.2 Host bridge: Advanced Micro Devices [AMD] Device 141c
00:18.3 Host bridge: Advanced Micro Devices [AMD] Device 141d
00:18.4 Host bridge: Advanced Micro Devices [AMD] Device 141e
00:18.5 Host bridge: Advanced Micro Devices [AMD] Device 141f
01:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI Device 6929
02:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI Device 692f
02:00.1 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI Device 692f
02:00.2 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI Device 692f
02:00.3 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI Device 692f
05:00.0 FireWire (IEEE 1394): VIA Technologies, Inc. VT6315 Series Firewire Controller (rev 01)
06:00.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5761 Gigabit Ethernet PCIe (rev 10)

Below is a snippet from the dmesg log when sriov is enabled on the device. This represents the delta from dmesg before the enabling sriov and dmesg after enabling sriov. Num_vfs is set to 4.  The last 2 lines of the dmesg are generated from my test module that I used to enable/disable sriov, they can be ignored.

[  157.336265] pci 0000:02:00.0: [1002:692f] type 00 class 0x030000
[  157.336321] pci 0000:02:00.0: Max Payload Size set to 256 (was 128, max 256)
[  157.336355] pci 0000:02:00.0: calling quirk_no_pm_reset+0x0/0x1a
[  157.336541] vgaarb: device added: PCI:0000:02:00.0,decodes=io+mem,owns=none,locks=none
[  157.336585] iommu: Adding device 0000:02:00.0 to group 2
[  157.336644] pci 0000:02:00.0: calling pci_fixup_video+0x0/0xb1
[  157.336874] pci 0000:02:00.1: [1002:692f] type 00 class 0x030000
[  157.336927] pci 0000:02:00.1: Max Payload Size set to 256 (was 128, max 256)
[  157.336948] pci 0000:02:00.1: calling quirk_no_pm_reset+0x0/0x1a
[  157.337265] vgaarb: device added: PCI:0000:02:00.1,decodes=io+mem,owns=none,locks=none
[  157.337368] iommu: Adding device 0000:02:00.1 to group 2
[  157.337414] pci 0000:02:00.1: calling pci_fixup_video+0x0/0xb1
[  157.337556] pci 0000:02:00.2: [1002:692f] type 00 class 0x030000
[  157.337609] pci 0000:02:00.2: Max Payload Size set to 256 (was 128, max 256)
[  157.337631] pci 0000:02:00.2: calling quirk_no_pm_reset+0x0/0x1a
[  157.337826] vgaarb: device added: PCI:0000:02:00.2,decodes=io+mem,owns=none,locks=none
[  157.337875] iommu: Adding device 0000:02:00.2 to group 2
[  157.337920] pci 0000:02:00.2: calling pci_fixup_video+0x0/0xb1
[  157.338016] pci 0000:02:00.3: [1002:692f] type 00 class 0x030000
[  157.338065] pci 0000:02:00.3: Max Payload Size set to 256 (was 128, max 256)
[  157.338084] pci 0000:02:00.3: calling quirk_no_pm_reset+0x0/0x1a
[  157.338221] vgaarb: device added: PCI:0000:02:00.3,decodes=io+mem,owns=none,locks=none
[  157.338255] iommu: Adding device 0000:02:00.3 to group 2
[  157.338287] pci 0000:02:00.3: calling pci_fixup_video+0x0/0xb1
[  157.338360] pci_enable_sriov() returns 0
[  157.338363] BDF = 0x0100, numVF = 4

> 
> It's not clear to me why we check the device type at all.  If there's no spec
> restriction on the types of devices that can have an SR-IOV capability, we should
> consider removing the test altogether (Alex mentioned this possiblity in the
> earlier discussion).

I am as well not clear why the check is in there.  I would be just as happy either adding TYPE_LEG_END or removing the check all together.  I don't know what the side effects of removing the check would be. I don't have any sriov devices other than a graphics card to test with so I wouldn't be able to test other scenarios.

> 
> Bjorn
> 
> > ---
> >  drivers/pci/iov.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index
> > 31f31d4..da4fbac 100644
> > --- a/drivers/pci/iov.c
> > +++ b/drivers/pci/iov.c
> > @@ -388,7 +388,8 @@ static int sriov_init(struct pci_dev *dev, int pos)
> >  	struct pci_dev *pdev;
> >
> >  	if (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END &&
> > -	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT)
> > +	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT &&
> > +	    pci_pcie_type(dev) != PCI_EXP_TYPE_LEG_END)
> >  		return -ENODEV;
> >
> >  	pci_read_config_word(dev, pos + PCI_SRIOV_CTRL, &ctrl);
> > --
> > 1.7.10.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci"
> > in the body of a message to majordomo@vger.kernel.org More majordomo
> > info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
  2016-02-04 16:21   ` Zytaruk, Kelly
@ 2016-02-05 16:46     ` Bjorn Helgaas
  2016-02-05 16:50       ` Zytaruk, Kelly
  0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2016-02-05 16:46 UTC (permalink / raw)
  To: Zytaruk, Kelly
  Cc: bhelgaas@google.com, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, Alex Williamson, Yu Zhao

On Thu, Feb 04, 2016 at 04:21:08PM +0000, Zytaruk, Kelly wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> > Sent: Thursday, February 04, 2016 10:14 AM
> > To: Zytaruk, Kelly
> > Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Alex Williamson; Yu Zhao
> > Subject: Re: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
> > 
> > [+cc Alex, Yu (participants in previous discussion)]
> > 
> > Hi Kelly,
> > 
> > On Thu, Feb 04, 2016 at 09:48:26AM -0500, Kelly Zytaruk wrote:
> > > Some AMD GPUs have hardware support for grapics SRIOV.
> > > If the GPU has a display output then the GPU needs to support Legacy VGA
> > operation.
> > > If CLASS_CODE = VGA then the device should have a Port Type = Legacy
> > EndPoint.
> > > Therefore in order to enable SRIOV on a GPU with a display output
> > LEGACY_END_POINT is supported as a valid Port Type.
> > >
> > > Signed-off-by: Kelly Zytaruk <kelly.zytaruk@amd.com>
> > 
> > We had an interesting discussion about this two years ago:
> > http://lkml.kernel.org/r/B756807489D6244883AC0B799A6EEC15EAB2E8@store
> > xdag02.amd.com
> 
> Unfortunately 2 years ago I couldn't complete your request as it
> would have disclosed Information about an unannounced technology
> that we were working on.  We have recently made that technology
> public and I can now send you the requested information.

> > Please include a reference to that discussion in your changelog.  In that
> > discussion, I also asked for some details (dmesg and lspci info) that motivate the
> > change, so please collect and add a reference to them as well.
> 
> The information that you ask for is included below.  I have
> abbreviated it so that this does not become a huge email.  I can
> send full logs if you want them.

Can you open a bugzilla at http://bugzilla.kernel.org and attach the full
logs there?  Then we can include the URL in your patch changelog.

> > It's not clear to me why we check the device type at all.  If
> > there's no spec restriction on the types of devices that can have
> > an SR-IOV capability, we should consider removing the test
> > altogether (Alex mentioned this possiblity in the earlier
> > discussion).
> 
> I am as well not clear why the check is in there.  I would be just
> as happy either adding TYPE_LEG_END or removing the check all
> together.  I don't know what the side effects of removing the check
> would be. I don't have any sriov devices other than a graphics card
> to test with so I wouldn't be able to test other scenarios.

If we don't have a reason to do the check, I think we should just
remove it altogether.

Bjorn

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

* RE: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
  2016-02-05 16:46     ` Bjorn Helgaas
@ 2016-02-05 16:50       ` Zytaruk, Kelly
  2016-02-05 16:59         ` Bjorn Helgaas
  0 siblings, 1 reply; 9+ messages in thread
From: Zytaruk, Kelly @ 2016-02-05 16:50 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: bhelgaas@google.com, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, Alex Williamson, Yu Zhao



> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: Friday, February 05, 2016 11:47 AM
> To: Zytaruk, Kelly
> Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; Alex Williamson; Yu Zhao
> Subject: Re: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
> 
> On Thu, Feb 04, 2016 at 04:21:08PM +0000, Zytaruk, Kelly wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> > > Sent: Thursday, February 04, 2016 10:14 AM
> > > To: Zytaruk, Kelly
> > > Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; Alex Williamson; Yu Zhao
> > > Subject: Re: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
> > >
> > > [+cc Alex, Yu (participants in previous discussion)]
> > >
> > > Hi Kelly,
> > >
> > > On Thu, Feb 04, 2016 at 09:48:26AM -0500, Kelly Zytaruk wrote:
> > > > Some AMD GPUs have hardware support for grapics SRIOV.
> > > > If the GPU has a display output then the GPU needs to support
> > > > Legacy VGA
> > > operation.
> > > > If CLASS_CODE = VGA then the device should have a Port Type =
> > > > Legacy
> > > EndPoint.
> > > > Therefore in order to enable SRIOV on a GPU with a display output
> > > LEGACY_END_POINT is supported as a valid Port Type.
> > > >
> > > > Signed-off-by: Kelly Zytaruk <kelly.zytaruk@amd.com>
> > >
> > > We had an interesting discussion about this two years ago:
> > >
> http://lkml.kernel.org/r/B756807489D6244883AC0B799A6EEC15EAB2E8@stor
> > > e
> > > xdag02.amd.com
> >
> > Unfortunately 2 years ago I couldn't complete your request as it would
> > have disclosed Information about an unannounced technology that we
> > were working on.  We have recently made that technology public and I
> > can now send you the requested information.
> 
> > > Please include a reference to that discussion in your changelog.  In
> > > that discussion, I also asked for some details (dmesg and lspci
> > > info) that motivate the change, so please collect and add a reference to
> them as well.
> >
> > The information that you ask for is included below.  I have
> > abbreviated it so that this does not become a huge email.  I can send
> > full logs if you want them.
> 
> Can you open a bugzilla at http://bugzilla.kernel.org and attach the full logs
> there?  Then we can include the URL in your patch changelog.

I have never worked with bugzilla before but I will check it out.  So you want me to take the bugzilla URL and paste it into the comments of my patch? Or is this something that you will do?

> 
> > > It's not clear to me why we check the device type at all.  If
> > > there's no spec restriction on the types of devices that can have an
> > > SR-IOV capability, we should consider removing the test altogether
> > > (Alex mentioned this possiblity in the earlier discussion).
> >
> > I am as well not clear why the check is in there.  I would be just as
> > happy either adding TYPE_LEG_END or removing the check all together.
> > I don't know what the side effects of removing the check would be. I
> > don't have any sriov devices other than a graphics card to test with
> > so I wouldn't be able to test other scenarios.
> 
> If we don't have a reason to do the check, I think we should just remove it
> altogether.

So instead of my proposed change do you want me to just remove the check completely and submit a patch for that?

> 
> Bjorn

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

* Re: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
  2016-02-05 16:50       ` Zytaruk, Kelly
@ 2016-02-05 16:59         ` Bjorn Helgaas
  0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2016-02-05 16:59 UTC (permalink / raw)
  To: Zytaruk, Kelly
  Cc: bhelgaas@google.com, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, Alex Williamson, Yu Zhao

On Fri, Feb 05, 2016 at 04:50:11PM +0000, Zytaruk, Kelly wrote:
> 
> 
> > -----Original Message-----
> > From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> > Sent: Friday, February 05, 2016 11:47 AM
> > To: Zytaruk, Kelly
> > Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Alex Williamson; Yu Zhao
> > Subject: Re: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
> > 
> > On Thu, Feb 04, 2016 at 04:21:08PM +0000, Zytaruk, Kelly wrote:
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> > > > Sent: Thursday, February 04, 2016 10:14 AM
> > > > To: Zytaruk, Kelly
> > > > Cc: bhelgaas@google.com; linux-pci@vger.kernel.org; linux-
> > > > kernel@vger.kernel.org; Alex Williamson; Yu Zhao
> > > > Subject: Re: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
> > > >
> > > > [+cc Alex, Yu (participants in previous discussion)]
> > > >
> > > > Hi Kelly,
> > > >
> > > > On Thu, Feb 04, 2016 at 09:48:26AM -0500, Kelly Zytaruk wrote:
> > > > > Some AMD GPUs have hardware support for grapics SRIOV.
> > > > > If the GPU has a display output then the GPU needs to support
> > > > > Legacy VGA
> > > > operation.
> > > > > If CLASS_CODE = VGA then the device should have a Port Type =
> > > > > Legacy
> > > > EndPoint.
> > > > > Therefore in order to enable SRIOV on a GPU with a display output
> > > > LEGACY_END_POINT is supported as a valid Port Type.
> > > > >
> > > > > Signed-off-by: Kelly Zytaruk <kelly.zytaruk@amd.com>
> > > >
> > > > We had an interesting discussion about this two years ago:
> > > >
> > http://lkml.kernel.org/r/B756807489D6244883AC0B799A6EEC15EAB2E8@stor
> > > > e
> > > > xdag02.amd.com
> > >
> > > Unfortunately 2 years ago I couldn't complete your request as it would
> > > have disclosed Information about an unannounced technology that we
> > > were working on.  We have recently made that technology public and I
> > > can now send you the requested information.
> > 
> > > > Please include a reference to that discussion in your changelog.  In
> > > > that discussion, I also asked for some details (dmesg and lspci
> > > > info) that motivate the change, so please collect and add a reference to
> > them as well.
> > >
> > > The information that you ask for is included below.  I have
> > > abbreviated it so that this does not become a huge email.  I can send
> > > full logs if you want them.
> > 
> > Can you open a bugzilla at http://bugzilla.kernel.org and attach the full logs
> > there?  Then we can include the URL in your patch changelog.
> 
> I have never worked with bugzilla before but I will check it out.  So you
> want me to take the bugzilla URL and paste it into the comments of my
> patch? Or is this something that you will do?

Put it in the bugzilla drivers/PCI category.  Let me know if you have
any bugzilla troubles.  It should be pretty self-explanatory.  Please
attach the files rather than pasting them in the text box.

If you could include the bugzilla URL in the changelog of a revised
patch, that would be great.  

> > > > It's not clear to me why we check the device type at all.  If
> > > > there's no spec restriction on the types of devices that can have an
> > > > SR-IOV capability, we should consider removing the test altogether
> > > > (Alex mentioned this possiblity in the earlier discussion).
> > >
> > > I am as well not clear why the check is in there.  I would be just as
> > > happy either adding TYPE_LEG_END or removing the check all together.
> > > I don't know what the side effects of removing the check would be. I
> > > don't have any sriov devices other than a graphics card to test with
> > > so I wouldn't be able to test other scenarios.
> > 
> > If we don't have a reason to do the check, I think we should just remove it
> > altogether.
> 
> So instead of my proposed change do you want me to just remove the check
> completely and submit a patch for that?

Please.

Bjorn

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

* [PATCH] PCI: Support SRIOV on Legacy EndPoint device
@ 2016-02-09 18:08 kelly.zytaruk
  2016-02-29 18:00 ` Bjorn Helgaas
  0 siblings, 1 reply; 9+ messages in thread
From: kelly.zytaruk @ 2016-02-09 18:08 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci, linux-kernel; +Cc: Kelly Zytaruk

From: Kelly Zytaruk <kelly.zytaruk@amd.com>

It is not neccessary to check for PCI_EXP_TYPE in sriov_init().  There appears to be no reason for the check.

Some AMD GPUs have hardware support for grapics SRIOV.
If the GPU has a display output then the GPU needs to support Legacy VGA operation.
If CLASS_CODE = VGA then the device should have a Port Type = Legacy EndPoint.
Therefore in order to enable SRIOV on a GPU with a display output, LEGACY_END_POINT is supported as a valid Port Type by removing the check for Port Type.

Patch is also logged in Bugzilla #112221

Signed-off-by: Kelly Zytaruk <kelly.zytaruk@amd.com>
---
 drivers/pci/iov.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 31f31d4..fe4bd0a 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -387,10 +387,6 @@ static int sriov_init(struct pci_dev *dev, int pos)
 	struct resource *res;
 	struct pci_dev *pdev;
 
-	if (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END &&
-	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT)
-		return -ENODEV;
-
 	pci_read_config_word(dev, pos + PCI_SRIOV_CTRL, &ctrl);
 	if (ctrl & PCI_SRIOV_CTRL_VFE) {
 		pci_write_config_word(dev, pos + PCI_SRIOV_CTRL, 0);
-- 
1.7.10.4


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

* Re: [PATCH] PCI: Support SRIOV on Legacy EndPoint device
  2016-02-09 18:08 kelly.zytaruk
@ 2016-02-29 18:00 ` Bjorn Helgaas
  0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2016-02-29 18:00 UTC (permalink / raw)
  To: kelly.zytaruk; +Cc: Bjorn Helgaas, linux-pci, linux-kernel

On Tue, Feb 09, 2016 at 01:08:58PM -0500, kelly.zytaruk@amd.com wrote:
> From: Kelly Zytaruk <kelly.zytaruk@amd.com>
> 
> It is not neccessary to check for PCI_EXP_TYPE in sriov_init().  There appears to be no reason for the check.
> 
> Some AMD GPUs have hardware support for grapics SRIOV.
> If the GPU has a display output then the GPU needs to support Legacy VGA operation.
> If CLASS_CODE = VGA then the device should have a Port Type = Legacy EndPoint.
> Therefore in order to enable SRIOV on a GPU with a display output, LEGACY_END_POINT is supported as a valid Port Type by removing the check for Port Type.
> 
> Patch is also logged in Bugzilla #112221
> 
> Signed-off-by: Kelly Zytaruk <kelly.zytaruk@amd.com>

Applied to pci/virtualization for v4.6, with changelog as follows:

    PCI: Support SR-IOV on any function type
    
    Previously, we only supported SR-IOV on PCI Express Endpoints and Root
    Complex Integrated Endpoints.  This restriction has been present since
    d1b054da8f59 ("PCI: initialize and release SR-IOV capability") added SR-IOV
    support, but the spec does not require it.  In fact, the SR-IOV spec r1.1,
    sec 3.3, says the SR-IOV extended capability may be present for any Type 0
    function.
    
    Remove the function type test, so we can support SR-IOV on any function.
    
    Some AMD GPUs have display outputs, use the VGA class code, are Legacy
    Endpoints, and support SR-IOV.  This change allows Linux to enable SR-IOV
    on these devices.
    
    [bhelgaas: changelog]
    Link: https://bugzilla.kernel.org/show_bug.cgi?id=112221
    Signed-off-by: Kelly Zytaruk <kelly.zytaruk@amd.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/iov.c |    4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 31f31d4..fe4bd0a 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -387,10 +387,6 @@ static int sriov_init(struct pci_dev *dev, int pos)
>  	struct resource *res;
>  	struct pci_dev *pdev;
>  
> -	if (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END &&
> -	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT)
> -		return -ENODEV;
> -
>  	pci_read_config_word(dev, pos + PCI_SRIOV_CTRL, &ctrl);
>  	if (ctrl & PCI_SRIOV_CTRL_VFE) {
>  		pci_write_config_word(dev, pos + PCI_SRIOV_CTRL, 0);
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2016-02-29 18:00 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-04 14:48 [PATCH] PCI: Support SRIOV on Legacy EndPoint device Kelly Zytaruk
2016-02-04 15:14 ` Bjorn Helgaas
2016-02-04 16:21   ` Zytaruk, Kelly
2016-02-05 16:46     ` Bjorn Helgaas
2016-02-05 16:50       ` Zytaruk, Kelly
2016-02-05 16:59         ` Bjorn Helgaas
  -- strict thread matches above, loose matches on Subject: below --
2016-02-04 15:08 Zytaruk, Kelly
2016-02-09 18:08 kelly.zytaruk
2016-02-29 18:00 ` Bjorn Helgaas

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