Architectural question regarding IOV support in Linux 3.13.4

Architectural question regarding IOV support in Linux 3.13.4

[Zytaruk, Kelly]

I am working with SR-IOV and I have a question regarding the function
sriov_init() in ../drivers/pci/iov.c (linux versions 3.4.9 and 3.13.4)

In sriov_init() the code first checks whether the PF is a Root complex 
 endpoint (0x9) or an Express Endpoint (0x0) as shown in the code 
 snippet below.  If it is neither it returns the No device error.

static int sriov_init(struct pci_dev *dev, int pos)
{
         int i;
         int rc;
         int nres;
         u32 pgsz;
         u16 ctrl, total, offset, stride;
         struct pci_sriov *iov;
         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;

My question is why PCI_EXP_TYPE_LEG_END (0x1) is omitted as being a 
 valid endpoint.  By excluding Legacy endpoints it fails enabling 
 SR-IOV on a VGA PF.

Is there a design/specification reason why legacy was excluded or was 
 it just an assumption that VGA would never support SR-IOV?

If there is no valid reason to exclude PCI_EXP_TYPE_LEG_END, I would 
 like to discuss having it included as a valid endpoint for SR-IOV.

Thanks,
Kelly

Re: Architectural question regarding IOV support in Linux 3.13.4

[Bjorn Helgaas]

[+cc Alex, Yu]

On Fri, Feb 21, 2014 at 10:45 AM, Zytaruk, Kelly <Kelly.Zytaruk@amd.com> wrote:
>
> I am working with SR-IOV and I have a question regarding the function
> sriov_init() in ../drivers/pci/iov.c (linux versions 3.4.9 and 3.13.4)
>
> In sriov_init() the code first checks whether the PF is a Root complex
>  endpoint (0x9) or an Express Endpoint (0x0) as shown in the code
>  snippet below.  If it is neither it returns the No device error.
>
> static int sriov_init(struct pci_dev *dev, int pos)
> {
>          int i;
>          int rc;
>          int nres;
>          u32 pgsz;
>          u16 ctrl, total, offset, stride;
>          struct pci_sriov *iov;
>          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;
>
> My question is why PCI_EXP_TYPE_LEG_END (0x1) is omitted as being a
>  valid endpoint.  By excluding Legacy endpoints it fails enabling
>  SR-IOV on a VGA PF.
>
> Is there a design/specification reason why legacy was excluded or was
>  it just an assumption that VGA would never support SR-IOV?
>
> If there is no valid reason to exclude PCI_EXP_TYPE_LEG_END, I would
>  like to discuss having it included as a valid endpoint for SR-IOV.

Good question.  It looks like it's been that way since the beginning
[1], but I don't know why.  I don't see any restriction in the spec
about SR-IOV and legacy endpoints.

I also don't know whether VGA is an issue.  There are some legacy
addressing issues for [mem 0xa0000-0xbffff] and [io 0x3b0-0x3bb] and
[io 0x3c0-0x3df].  For example, when a bridge has its VGA Enable bit
set, it positively decodes [mem 0xa0000-0xbffff] even if that range
isn't included in one of the bridge windows.  I don't know whether a
VGA device is similarly allowed to decode that range even if it's not
in a BAR.  If it is, I could imagine issues if enabling SR-IOV created
several VGA VFs.

Bjorn

[1] http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?id=d1b054da8f599905f3c18a218961dcf17f9d5f13

Re: Architectural question regarding IOV support in Linux 3.13.4

[Alex Williamson]

On Fri, 2014-02-21 at 14:11 -0700, Bjorn Helgaas wrote:
> [+cc Alex, Yu]
> 
> On Fri, Feb 21, 2014 at 10:45 AM, Zytaruk, Kelly <Kelly.Zytaruk@amd.com> wrote:
> >
> > I am working with SR-IOV and I have a question regarding the function
> > sriov_init() in ../drivers/pci/iov.c (linux versions 3.4.9 and 3.13.4)
> >
> > In sriov_init() the code first checks whether the PF is a Root complex
> >  endpoint (0x9) or an Express Endpoint (0x0) as shown in the code
> >  snippet below.  If it is neither it returns the No device error.
> >
> > static int sriov_init(struct pci_dev *dev, int pos)
> > {
> >          int i;
> >          int rc;
> >          int nres;
> >          u32 pgsz;
> >          u16 ctrl, total, offset, stride;
> >          struct pci_sriov *iov;
> >          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;
> >
> > My question is why PCI_EXP_TYPE_LEG_END (0x1) is omitted as being a
> >  valid endpoint.  By excluding Legacy endpoints it fails enabling
> >  SR-IOV on a VGA PF.
> >
> > Is there a design/specification reason why legacy was excluded or was
> >  it just an assumption that VGA would never support SR-IOV?
> >
> > If there is no valid reason to exclude PCI_EXP_TYPE_LEG_END, I would
> >  like to discuss having it included as a valid endpoint for SR-IOV.
> 
> Good question.  It looks like it's been that way since the beginning
> [1], but I don't know why.  I don't see any restriction in the spec
> about SR-IOV and legacy endpoints.
> 
> I also don't know whether VGA is an issue.  There are some legacy
> addressing issues for [mem 0xa0000-0xbffff] and [io 0x3b0-0x3bb] and
> [io 0x3c0-0x3df].  For example, when a bridge has its VGA Enable bit
> set, it positively decodes [mem 0xa0000-0xbffff] even if that range
> isn't included in one of the bridge windows.  I don't know whether a
> VGA device is similarly allowed to decode that range even if it's not
> in a BAR.  If it is, I could imagine issues if enabling SR-IOV created
> several VGA VFs.

VFs cannot support I/O port space by definition, so I don't think a "VGA
VF" could actually exist.  There would be nothing wrong with a non-VGA
GPU VF though.  I also don't see how the differences in Legacy Endpoint
rules versus a standard Endpoint would preclude supporting SR-IOV.  I
don't think the SR-IOV spec makes any demands on whether the PF requires
I/O port resources, which I assume is the main reason for this to call
itself Legacy.  I'd guess it was likely just an oversight and we should
add legacy endpoints (or remove the test altogether and trust that if a
device has an SR-IOV capability, we should initialize it).  Thanks,

Alex

RE: Architectural question regarding IOV support in Linux 3.13.4

[Zytaruk, Kelly]

DQo+T24gRnJpLCBGZWIgMjEsIDIwMTQgYXQgMjowMyBQTSwgQWxleCBXaWxsaWFtc29uIDxhbGV4
LndpbGxpYW1zb25AcmVkaGF0LmNvbT4gd3JvdGU6DQo+T24gRnJpLCAyMDE0LTAyLTIxIGF0IDE0
OjExIC0wNzAwLCBCam9ybiBIZWxnYWFzIHdyb3RlOg0KPj4gWytjYyBBbGV4LCBZdV0NCj4+DQo+
PiBPbiBGcmksIEZlYiAyMSwgMjAxNCBhdCAxMDo0NSBBTSwgWnl0YXJ1aywgS2VsbHkgPEtlbGx5
Llp5dGFydWtAYW1kLmNvbT4gd3JvdGU6DQo+PiA+DQo+PiA+IEkgYW0gd29ya2luZyB3aXRoIFNS
LUlPViBhbmQgSSBoYXZlIGEgcXVlc3Rpb24gcmVnYXJkaW5nIHRoZSBmdW5jdGlvbg0KPj4gPiBz
cmlvdl9pbml0KCkgaW4gLi4vZHJpdmVycy9wY2kvaW92LmMgKGxpbnV4IHZlcnNpb25zIDMuNC45
IGFuZCAzLjEzLjQpDQo+PiA+DQo+PiA+IEluIHNyaW92X2luaXQoKSB0aGUgY29kZSBmaXJzdCBj
aGVja3Mgd2hldGhlciB0aGUgUEYgaXMgYSBSb290IGNvbXBsZXgNCj4+ID4gwqBlbmRwb2ludCAo
MHg5KSBvciBhbiBFeHByZXNzIEVuZHBvaW50ICgweDApIGFzIHNob3duIGluIHRoZSBjb2RlDQo+
PiA+IMKgc25pcHBldCBiZWxvdy4gwqBJZiBpdCBpcyBuZWl0aGVyIGl0IHJldHVybnMgdGhlIE5v
IGRldmljZSBlcnJvci4NCj4+ID4NCj4+ID4gc3RhdGljIGludCBzcmlvdl9pbml0KHN0cnVjdCBw
Y2lfZGV2ICpkZXYsIGludCBwb3MpDQo+PiA+IHsNCj4+ID4gwqAgwqAgwqAgwqAgwqBpbnQgaTsN
Cj4+ID4gwqAgwqAgwqAgwqAgwqBpbnQgcmM7DQo+PiA+IMKgIMKgIMKgIMKgIMKgaW50IG5yZXM7
DQo+PiA+IMKgIMKgIMKgIMKgIMKgdTMyIHBnc3o7DQo+PiA+IMKgIMKgIMKgIMKgIMKgdTE2IGN0
cmwsIHRvdGFsLCBvZmZzZXQsIHN0cmlkZTsNCj4+ID4gwqAgwqAgwqAgwqAgwqBzdHJ1Y3QgcGNp
X3NyaW92ICppb3Y7DQo+PiA+IMKgIMKgIMKgIMKgIMKgc3RydWN0IHJlc291cmNlICpyZXM7DQo+
PiA+IMKgIMKgIMKgIMKgIMKgc3RydWN0IHBjaV9kZXYgKnBkZXY7DQo+PiA+DQo+PiA+IMKgIMKg
IMKgIMKgaWYgKHBjaV9wY2llX3R5cGUoZGV2KSAhPSBQQ0lfRVhQX1RZUEVfUkNfRU5EICYmDQo+
PiA+IMKgIMKgIMKgIMKgIMKgIMKgIMKgcGNpX3BjaWVfdHlwZShkZXYpICE9IFBDSV9FWFBfVFlQ
RV9FTkRQT0lOVCkNCj4+ID4gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqByZXR1cm4gLUVOT0RF
VjsNCj4+ID4NCj4+ID4gTXkgcXVlc3Rpb24gaXMgd2h5IFBDSV9FWFBfVFlQRV9MRUdfRU5EICgw
eDEpIGlzIG9taXR0ZWQgYXMgYmVpbmcgYQ0KPj4gPiDCoHZhbGlkIGVuZHBvaW50LiDCoEJ5IGV4
Y2x1ZGluZyBMZWdhY3kgZW5kcG9pbnRzIGl0IGZhaWxzIGVuYWJsaW5nDQo+PiA+IMKgU1ItSU9W
IG9uIGEgVkdBIFBGLg0KPj4gPg0KPj4gPiBJcyB0aGVyZSBhIGRlc2lnbi9zcGVjaWZpY2F0aW9u
IHJlYXNvbiB3aHkgbGVnYWN5IHdhcyBleGNsdWRlZCBvciB3YXMNCj4+ID4gwqBpdCBqdXN0IGFu
IGFzc3VtcHRpb24gdGhhdCBWR0Egd291bGQgbmV2ZXIgc3VwcG9ydCBTUi1JT1Y/DQo+PiA+DQo+
PiA+IElmIHRoZXJlIGlzIG5vIHZhbGlkIHJlYXNvbiB0byBleGNsdWRlIFBDSV9FWFBfVFlQRV9M
RUdfRU5ELCBJIHdvdWxkDQo+PiA+IMKgbGlrZSB0byBkaXNjdXNzIGhhdmluZyBpdCBpbmNsdWRl
ZCBhcyBhIHZhbGlkIGVuZHBvaW50IGZvciBTUi1JT1YuDQo+Pg0KPj4gR29vZCBxdWVzdGlvbi4g
wqBJdCBsb29rcyBsaWtlIGl0J3MgYmVlbiB0aGF0IHdheSBzaW5jZSB0aGUgYmVnaW5uaW5nDQo+
PiBbMV0sIGJ1dCBJIGRvbid0IGtub3cgd2h5LiDCoEkgZG9uJ3Qgc2VlIGFueSByZXN0cmljdGlv
biBpbiB0aGUgc3BlYw0KPj4gYWJvdXQgU1ItSU9WIGFuZCBsZWdhY3kgZW5kcG9pbnRzLg0KPj4N
Cj4+IEkgYWxzbyBkb24ndCBrbm93IHdoZXRoZXIgVkdBIGlzIGFuIGlzc3VlLiDCoFRoZXJlIGFy
ZSBzb21lIGxlZ2FjeQ0KPj4gYWRkcmVzc2luZyBpc3N1ZXMgZm9yIFttZW0gMHhhMDAwMC0weGJm
ZmZmXSBhbmQgW2lvIDB4M2IwLTB4M2JiXSBhbmQNCj4+IFtpbyAweDNjMC0weDNkZl0uIMKgRm9y
IGV4YW1wbGUsIHdoZW4gYSBicmlkZ2UgaGFzIGl0cyBWR0EgRW5hYmxlIGJpdA0KPj4gc2V0LCBp
dCBwb3NpdGl2ZWx5IGRlY29kZXMgW21lbSAweGEwMDAwLTB4YmZmZmZdIGV2ZW4gaWYgdGhhdCBy
YW5nZQ0KPj4gaXNuJ3QgaW5jbHVkZWQgaW4gb25lIG9mIHRoZSBicmlkZ2Ugd2luZG93cy4gwqBJ
IGRvbid0IGtub3cgd2hldGhlciBhDQo+PiBWR0EgZGV2aWNlIGlzIHNpbWlsYXJseSBhbGxvd2Vk
IHRvIGRlY29kZSB0aGF0IHJhbmdlIGV2ZW4gaWYgaXQncyBub3QNCj4+IGluIGEgQkFSLiDCoElm
IGl0IGlzLCBJIGNvdWxkIGltYWdpbmUgaXNzdWVzIGlmIGVuYWJsaW5nIFNSLUlPViBjcmVhdGVk
DQo+PiBzZXZlcmFsIFZHQSBWRnMuDQo+VkZzIGNhbm5vdCBzdXBwb3J0IEkvTyBwb3J0IHNwYWNl
IGJ5IGRlZmluaXRpb24sIHNvIEkgZG9uJ3QgdGhpbmsgYSAiVkdBDQo+VkYiIGNvdWxkIGFjdHVh
bGx5IGV4aXN0LiDCoFRoZXJlIHdvdWxkIGJlIG5vdGhpbmcgd3Jvbmcgd2l0aCBhIG5vbi1WR0EN
Cj5HUFUgVkYgdGhvdWdoLiDCoEkgYWxzbyBkb24ndCBzZWUgaG93IHRoZSBkaWZmZXJlbmNlcyBp
biBMZWdhY3kgRW5kcG9pbnQNCj5ydWxlcyB2ZXJzdXMgYSBzdGFuZGFyZCBFbmRwb2ludCB3b3Vs
ZCBwcmVjbHVkZSBzdXBwb3J0aW5nIFNSLUlPVi4gwqBJDQo+ZG9uJ3QgdGhpbmsgdGhlIFNSLUlP
ViBzcGVjIG1ha2VzIGFueSBkZW1hbmRzIG9uIHdoZXRoZXIgdGhlIFBGIHJlcXVpcmVzDQo+SS9P
IHBvcnQgcmVzb3VyY2VzLCB3aGljaCBJIGFzc3VtZSBpcyB0aGUgbWFpbiByZWFzb24gZm9yIHRo
aXMgdG8gY2FsbA0KPml0c2VsZiBMZWdhY3kuIMKgSSdkIGd1ZXNzIGl0IHdhcyBsaWtlbHkganVz
dCBhbiBvdmVyc2lnaHQgYW5kIHdlIHNob3VsZA0KPmFkZCBsZWdhY3kgZW5kcG9pbnRzIChvciBy
ZW1vdmUgdGhlIHRlc3QgYWx0b2dldGhlciBhbmQgdHJ1c3QgdGhhdCBpZiBhDQo+ZGV2aWNlIGhh
cyBhbiBTUi1JT1YgY2FwYWJpbGl0eSwgd2Ugc2hvdWxkIGluaXRpYWxpemUgaXQpLiDCoFRoYW5r
cywNCj4NCj5BbGV4DQo+DQo+SSBhZ3JlZS4gSSB2YWd1ZWx5IHJlbWVtYmVyIHRoZXJlIHdhcyBz
b21lIHJlYXNvbiB0aGF0IGV4Y2x1ZGVzIGxlZ2FjeQ0KPmVuZHBvaW50cyBmcm9tIHVzaW5nIFNS
LUlPVi4gQnV0IGFmdGVyIGEgcXVpY2sgbG9vayBhdCB0aGUgbGF0ZXN0IHNwZWNzLA0KPkkgZGlk
bid0IGZpbmQgYW55Lg0KPg0KT25seSBMZWdhY3kgRW5kcG9pbnRzIGNhbiBjbGFpbSBJL08uICBW
RnMgYXJlIG5vdCBhbGxvd2VkIHRvIGNsYWltIEkvTy4NClZHQSBkZXZpY2VzIGNsYWltIEkvTy4N
Cg0KVGhlIFNSLUlPViBzcGVjIDEuMSBzZWN0aW9uIDMuNC4xLjYgc3RhdGVzDQrigJxUaGUgQ2xh
c3MgQ29kZSByZWdpc3RlciBpcyByZWFkLW9ubHkgYW5kIGlzIHVzZWQgdG8gaWRlbnRpZnkgdGhl
IGdlbmVyaWMgZnVuY3Rpb24gDQpvZiB0aGUgZGV2aWNlIGFuZCwgaW4gc29tZSBjYXNlcywgYSBz
cGVjaWZpYyByZWdpc3RlciBsZXZlbCBwcm9ncmFtbWluZyBpbnRlcmZhY2UuIA0KVGhlIGZpZWxk
IGluIHRoZSBQRiBhbmQgYXNzb2NpYXRlZCBWRnMgbXVzdCByZXR1cm4gdGhlIHNhbWUgdmFsdWUg
d2hlbiByZWFkLuKAnQ0KDQpJZiB0aGUgUEYgaXMgYSBWR0EgZGV2aWNlIHRoZW4gYnkgdGhlIGRl
ZmluaXRpb24gaW4gdGhlIFNSLUlPViBzcGVjIHRoZSBjbGFzcyBjb2RlIA0Kb2YgdGhlIFZGIHdv
dWxkIGFsc28gaW5kaWNhdGUgaXQgYXMgYSBWR0EgZGV2aWNlLCBpZSBTdWJjbGFzcyAweDAgPSBW
R0EsIA0Kc3ViY2xhc3MgMHg4MCA9IE9USEVSX0RJU1BMQVlfQ09OVFJPTExFUi4gIA0KDQpJZGVh
bGx5IHdlIHdvdWxkIHdhbnQgdG8gaGF2ZSB0aGUgUEYgc3ViLWNsYXNzIGFzIDB4MCBhbmQgdGhl
IFZGIHN1YmNsYXNzIGFzIDB4ODANCkJ1dCB0aGUgc3BlYyBkb2Vzbid0IHN1cHBvcnQgdGhpcy4N
Cg0KT25lIHNwZWN1bGF0aW9uIGFzIHRvIHdoeSBMZWdhY3kgZW5kcG9pbnRzIHdlcmUgb21pdHRl
ZCBtaWdodCBiZSB0aGUgYXNzdW1wdGlvbg0KdGhhdCBkb2luZyBzbyB3b3VsZCBhbGxvdyBWR0Eg
VkZzIHRvIGJlIGNyZWF0ZWQuDQoNCkl0IGlzIG5vdCByZWFzb25hYmxlIHRvIHByZXZlbnQgYSBW
R0EgUEYgZnJvbSBlbmFibGluZyBTUi1JT1YgYXMgdGhpcyBpcyBhIHJlYWwgd29ybGQgDQpwb3Nz
aWJpbGl0eS4gIFdlIG1pZ2h0IG5lZWQgdG8gYWRkIG1vcmUgY29kZSBlbHNld2hlcmUgaG93ZXZl
ciB0byBwcmV2ZW50IGEgVkYNCmZyb20gYmVjb21pbmcgYSBWR0EgZGV2aWNlIG91dHNpZGUgb2Yg
cGFzc2luZyBpdCB0aHJvdWdoIHRvIGEgZ3Vlc3QgVk0uDQoNCkFueSB0aG91Z2h0cyBvbiB0aGlz
Pw0KDQpLZWxseQ0K

Re: Architectural question regarding IOV support in Linux 3.13.4

[Bjorn Helgaas]

On Mon, Feb 24, 2014 at 12:22 PM, Zytaruk, Kelly <Kelly.Zytaruk@amd.com> wrote:
>
>>On Fri, Feb 21, 2014 at 2:03 PM, Alex Williamson <alex.williamson@redhat.com> wrote:
>>On Fri, 2014-02-21 at 14:11 -0700, Bjorn Helgaas wrote:
>>> [+cc Alex, Yu]
>>>
>>> On Fri, Feb 21, 2014 at 10:45 AM, Zytaruk, Kelly <Kelly.Zytaruk@amd.com> wrote:
>>> >
>>> > I am working with SR-IOV and I have a question regarding the function
>>> > sriov_init() in ../drivers/pci/iov.c (linux versions 3.4.9 and 3.13.4)
>>> >
>>> > In sriov_init() the code first checks whether the PF is a Root complex
>>> >  endpoint (0x9) or an Express Endpoint (0x0) as shown in the code
>>> >  snippet below.  If it is neither it returns the No device error.
>>> >
>>> > static int sriov_init(struct pci_dev *dev, int pos)
>>> > {
>>> >          int i;
>>> >          int rc;
>>> >          int nres;
>>> >          u32 pgsz;
>>> >          u16 ctrl, total, offset, stride;
>>> >          struct pci_sriov *iov;
>>> >          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;
>>> >
>>> > My question is why PCI_EXP_TYPE_LEG_END (0x1) is omitted as being a
>>> >  valid endpoint.  By excluding Legacy endpoints it fails enabling
>>> >  SR-IOV on a VGA PF.
>>> >
>>> > Is there a design/specification reason why legacy was excluded or was
>>> >  it just an assumption that VGA would never support SR-IOV?
>>> >
>>> > If there is no valid reason to exclude PCI_EXP_TYPE_LEG_END, I would
>>> >  like to discuss having it included as a valid endpoint for SR-IOV.
>>>
>>> Good question.  It looks like it's been that way since the beginning
>>> [1], but I don't know why.  I don't see any restriction in the spec
>>> about SR-IOV and legacy endpoints.
>>>
>>> I also don't know whether VGA is an issue.  There are some legacy
>>> addressing issues for [mem 0xa0000-0xbffff] and [io 0x3b0-0x3bb] and
>>> [io 0x3c0-0x3df].  For example, when a bridge has its VGA Enable bit
>>> set, it positively decodes [mem 0xa0000-0xbffff] even if that range
>>> isn't included in one of the bridge windows.  I don't know whether a
>>> VGA device is similarly allowed to decode that range even if it's not
>>> in a BAR.  If it is, I could imagine issues if enabling SR-IOV created
>>> several VGA VFs.
>>VFs cannot support I/O port space by definition, so I don't think a "VGA
>>VF" could actually exist.  There would be nothing wrong with a non-VGA
>>GPU VF though.  I also don't see how the differences in Legacy Endpoint
>>rules versus a standard Endpoint would preclude supporting SR-IOV.  I
>>don't think the SR-IOV spec makes any demands on whether the PF requires
>>I/O port resources, which I assume is the main reason for this to call
>>itself Legacy.  I'd guess it was likely just an oversight and we should
>>add legacy endpoints (or remove the test altogether and trust that if a
>>device has an SR-IOV capability, we should initialize it).  Thanks,
>>
>>Alex
>>
>>I agree. I vaguely remember there was some reason that excludes legacy
>>endpoints from using SR-IOV. But after a quick look at the latest specs,
>>I didn't find any.
>>
> Only Legacy Endpoints can claim I/O.  VFs are not allowed to claim I/O.
> VGA devices claim I/O.
>
> The SR-IOV spec 1.1 section 3.4.1.6 states
> “The Class Code register is read-only and is used to identify the generic function
> of the device and, in some cases, a specific register level programming interface.
> The field in the PF and associated VFs must return the same value when read.”
>
> If the PF is a VGA device then by the definition in the SR-IOV spec the class code
> of the VF would also indicate it as a VGA device, ie Subclass 0x0 = VGA,
> subclass 0x80 = OTHER_DISPLAY_CONTROLLER.
>
> Ideally we would want to have the PF sub-class as 0x0 and the VF subclass as 0x80
> But the spec doesn't support this.
>
> One speculation as to why Legacy endpoints were omitted might be the assumption
> that doing so would allow VGA VFs to be created.
>
> It is not reasonable to prevent a VGA PF from enabling SR-IOV as this is a real world
> possibility.  We might need to add more code elsewhere however to prevent a VF
> from becoming a VGA device outside of passing it through to a guest VM.

Can you try out Yu's patch and see what happens?  If you do turn on
SR-IOV on this device, it would be interesting to see the complete
dmesg log and "lspci -vv" output to see what we do with it.

Bjorn

Re: Architectural question regarding IOV support in Linux 3.13.4

[Alex Williamson]

On Mon, 2014-02-24 at 19:22 +0000, Zytaruk, Kelly wrote:
> >On Fri, Feb 21, 2014 at 2:03 PM, Alex Williamson <alex.williamson@redhat.com> wrote:
> >On Fri, 2014-02-21 at 14:11 -0700, Bjorn Helgaas wrote:
> >> [+cc Alex, Yu]
> >>
> >> On Fri, Feb 21, 2014 at 10:45 AM, Zytaruk, Kelly <Kelly.Zytaruk@amd.com> wrote:
> >> >
> >> > I am working with SR-IOV and I have a question regarding the function
> >> > sriov_init() in ../drivers/pci/iov.c (linux versions 3.4.9 and 3.13.4)
> >> >
> >> > In sriov_init() the code first checks whether the PF is a Root complex
> >> >  endpoint (0x9) or an Express Endpoint (0x0) as shown in the code
> >> >  snippet below.  If it is neither it returns the No device error.
> >> >
> >> > static int sriov_init(struct pci_dev *dev, int pos)
> >> > {
> >> >          int i;
> >> >          int rc;
> >> >          int nres;
> >> >          u32 pgsz;
> >> >          u16 ctrl, total, offset, stride;
> >> >          struct pci_sriov *iov;
> >> >          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;
> >> >
> >> > My question is why PCI_EXP_TYPE_LEG_END (0x1) is omitted as being a
> >> >  valid endpoint.  By excluding Legacy endpoints it fails enabling
> >> >  SR-IOV on a VGA PF.
> >> >
> >> > Is there a design/specification reason why legacy was excluded or was
> >> >  it just an assumption that VGA would never support SR-IOV?
> >> >
> >> > If there is no valid reason to exclude PCI_EXP_TYPE_LEG_END, I would
> >> >  like to discuss having it included as a valid endpoint for SR-IOV.
> >>
> >> Good question.  It looks like it's been that way since the beginning
> >> [1], but I don't know why.  I don't see any restriction in the spec
> >> about SR-IOV and legacy endpoints.
> >>
> >> I also don't know whether VGA is an issue.  There are some legacy
> >> addressing issues for [mem 0xa0000-0xbffff] and [io 0x3b0-0x3bb] and
> >> [io 0x3c0-0x3df].  For example, when a bridge has its VGA Enable bit
> >> set, it positively decodes [mem 0xa0000-0xbffff] even if that range
> >> isn't included in one of the bridge windows.  I don't know whether a
> >> VGA device is similarly allowed to decode that range even if it's not
> >> in a BAR.  If it is, I could imagine issues if enabling SR-IOV created
> >> several VGA VFs.
> >VFs cannot support I/O port space by definition, so I don't think a "VGA
> >VF" could actually exist.  There would be nothing wrong with a non-VGA
> >GPU VF though.  I also don't see how the differences in Legacy Endpoint
> >rules versus a standard Endpoint would preclude supporting SR-IOV.  I
> >don't think the SR-IOV spec makes any demands on whether the PF requires
> >I/O port resources, which I assume is the main reason for this to call
> >itself Legacy.  I'd guess it was likely just an oversight and we should
> >add legacy endpoints (or remove the test altogether and trust that if a
> >device has an SR-IOV capability, we should initialize it).  Thanks,
> >
> >Alex
> >
> >I agree. I vaguely remember there was some reason that excludes legacy
> >endpoints from using SR-IOV. But after a quick look at the latest specs,
> >I didn't find any.
> >
> Only Legacy Endpoints can claim I/O.  VFs are not allowed to claim I/O.
> VGA devices claim I/O.
> 
> The SR-IOV spec 1.1 section 3.4.1.6 states
> “The Class Code register is read-only and is used to identify the generic function 
> of the device and, in some cases, a specific register level programming interface. 
> The field in the PF and associated VFs must return the same value when read.”
> 
> If the PF is a VGA device then by the definition in the SR-IOV spec the class code 
> of the VF would also indicate it as a VGA device, ie Subclass 0x0 = VGA, 
> subclass 0x80 = OTHER_DISPLAY_CONTROLLER.  
> 
> Ideally we would want to have the PF sub-class as 0x0 and the VF subclass as 0x80
> But the spec doesn't support this.
> 
> One speculation as to why Legacy endpoints were omitted might be the assumption
> that doing so would allow VGA VFs to be created.
> 
> It is not reasonable to prevent a VGA PF from enabling SR-IOV as this is a real world 
> possibility.  We might need to add more code elsewhere however to prevent a VF
> from becoming a VGA device outside of passing it through to a guest VM.
> 
> Any thoughts on this?

Good catch, I think you'll need to contact the PCI SIG for
clarification.  A VF with a VGA class code implies I/O space that a VF
is not allowed to have.  I wouldn't be surprised if they had hoped VGA
was a non-issue by this point and legacy-free graphics were standard.

Probably the most appealing solution would be an ECN allowing the VF to
expose a different class code from the PF.  This seems generally useful
even beyond the scope of this legacy resource issue.  Probably a less
appealing option for you would be to expose the PF as a non-VGA display
controller, which avoids both the legacy endpoint problem and the VF
class code problem.  I suspect we wouldn't find a lot of support for a
software only solution to virtualize the class code unless it was
standardized.  Such a change would require support in every hypervisor.
Thanks,

Alex

RE: Architectural question regarding IOV support in Linux 3.13.4

[Zytaruk, Kelly]

DQo+T24gTW9uLCBGZWIgMjQsIDIwMTQgYXQgMTI6MjIgUE0sIFp5dGFydWssIEtlbGx5IDxLZWxs
eS5aeXRhcnVrQGFtZC5jb20+IHdyb3RlOg0KPj4NCj4+Pk9uIEZyaSwgRmViIDIxLCAyMDE0IGF0
IDI6MDMgUE0sIEFsZXggV2lsbGlhbXNvbiA8YWxleC53aWxsaWFtc29uQHJlZGhhdC5jb20+IHdy
b3RlOg0KPj4+T24gRnJpLCAyMDE0LTAyLTIxIGF0IDE0OjExIC0wNzAwLCBCam9ybiBIZWxnYWFz
IHdyb3RlOg0KPj4+PiBbK2NjIEFsZXgsIFl1XQ0KPj4+Pg0KPj4+PiBPbiBGcmksIEZlYiAyMSwg
MjAxNCBhdCAxMDo0NSBBTSwgWnl0YXJ1aywgS2VsbHkgPEtlbGx5Llp5dGFydWtAYW1kLmNvbT4g
d3JvdGU6DQo+Pj4+ID4NCj4+Pj4gPiBJIGFtIHdvcmtpbmcgd2l0aCBTUi1JT1YgYW5kIEkgaGF2
ZSBhIHF1ZXN0aW9uIHJlZ2FyZGluZyB0aGUgDQo+Pj4+ID4gZnVuY3Rpb24NCj4+Pj4gPiBzcmlv
dl9pbml0KCkgaW4gLi4vZHJpdmVycy9wY2kvaW92LmMgKGxpbnV4IHZlcnNpb25zIDMuNC45IGFu
ZCANCj4+Pj4gPiAzLjEzLjQpDQo+Pj4+ID4NCj4+Pj4gPiBJbiBzcmlvdl9pbml0KCkgdGhlIGNv
ZGUgZmlyc3QgY2hlY2tzIHdoZXRoZXIgdGhlIFBGIGlzIGEgUm9vdCANCj4+Pj4gPiBjb21wbGV4
ICBlbmRwb2ludCAoMHg5KSBvciBhbiBFeHByZXNzIEVuZHBvaW50ICgweDApIGFzIHNob3duIGlu
IA0KPj4+PiA+IHRoZSBjb2RlICBzbmlwcGV0IGJlbG93LiAgSWYgaXQgaXMgbmVpdGhlciBpdCBy
ZXR1cm5zIHRoZSBObyBkZXZpY2UgZXJyb3IuDQo+Pj4+ID4NCj4+Pj4gPiBzdGF0aWMgaW50IHNy
aW92X2luaXQoc3RydWN0IHBjaV9kZXYgKmRldiwgaW50IHBvcykgew0KPj4+PiA+ICAgICAgICAg
IGludCBpOw0KPj4+PiA+ICAgICAgICAgIGludCByYzsNCj4+Pj4gPiAgICAgICAgICBpbnQgbnJl
czsNCj4+Pj4gPiAgICAgICAgICB1MzIgcGdzejsNCj4+Pj4gPiAgICAgICAgICB1MTYgY3RybCwg
dG90YWwsIG9mZnNldCwgc3RyaWRlOw0KPj4+PiA+ICAgICAgICAgIHN0cnVjdCBwY2lfc3Jpb3Yg
KmlvdjsNCj4+Pj4gPiAgICAgICAgICBzdHJ1Y3QgcmVzb3VyY2UgKnJlczsNCj4+Pj4gPiAgICAg
ICAgICBzdHJ1Y3QgcGNpX2RldiAqcGRldjsNCj4+Pj4gPg0KPj4+PiA+ICAgICAgICBpZiAocGNp
X3BjaWVfdHlwZShkZXYpICE9IFBDSV9FWFBfVFlQRV9SQ19FTkQgJiYNCj4+Pj4gPiAgICAgICAg
ICAgICAgcGNpX3BjaWVfdHlwZShkZXYpICE9IFBDSV9FWFBfVFlQRV9FTkRQT0lOVCkNCj4+Pj4g
PiAgICAgICAgICAgICAgICAgIHJldHVybiAtRU5PREVWOw0KPj4+PiA+DQo+Pj4+ID4gTXkgcXVl
c3Rpb24gaXMgd2h5IFBDSV9FWFBfVFlQRV9MRUdfRU5EICgweDEpIGlzIG9taXR0ZWQgYXMgYmVp
bmcgDQo+Pj4+ID4gYSAgdmFsaWQgZW5kcG9pbnQuICBCeSBleGNsdWRpbmcgTGVnYWN5IGVuZHBv
aW50cyBpdCBmYWlscyANCj4+Pj4gPiBlbmFibGluZyAgU1ItSU9WIG9uIGEgVkdBIFBGLg0KPj4+
PiA+DQo+Pj4+ID4gSXMgdGhlcmUgYSBkZXNpZ24vc3BlY2lmaWNhdGlvbiByZWFzb24gd2h5IGxl
Z2FjeSB3YXMgZXhjbHVkZWQgb3IgDQo+Pj4+ID4gd2FzICBpdCBqdXN0IGFuIGFzc3VtcHRpb24g
dGhhdCBWR0Egd291bGQgbmV2ZXIgc3VwcG9ydCBTUi1JT1Y/DQo+Pj4+ID4NCj4+Pj4gPiBJZiB0
aGVyZSBpcyBubyB2YWxpZCByZWFzb24gdG8gZXhjbHVkZSBQQ0lfRVhQX1RZUEVfTEVHX0VORCwg
SSANCj4+Pj4gPiB3b3VsZCAgbGlrZSB0byBkaXNjdXNzIGhhdmluZyBpdCBpbmNsdWRlZCBhcyBh
IHZhbGlkIGVuZHBvaW50IGZvciBTUi1JT1YuDQo+Pj4+DQo+Pj4+IEdvb2QgcXVlc3Rpb24uICBJ
dCBsb29rcyBsaWtlIGl0J3MgYmVlbiB0aGF0IHdheSBzaW5jZSB0aGUgYmVnaW5uaW5nIA0KPj4+
PiBbMV0sIGJ1dCBJIGRvbid0IGtub3cgd2h5LiAgSSBkb24ndCBzZWUgYW55IHJlc3RyaWN0aW9u
IGluIHRoZSBzcGVjIA0KPj4+PiBhYm91dCBTUi1JT1YgYW5kIGxlZ2FjeSBlbmRwb2ludHMuDQo+
Pj4+DQo+Pj4+IEkgYWxzbyBkb24ndCBrbm93IHdoZXRoZXIgVkdBIGlzIGFuIGlzc3VlLiAgVGhl
cmUgYXJlIHNvbWUgbGVnYWN5IA0KPj4+PiBhZGRyZXNzaW5nIGlzc3VlcyBmb3IgW21lbSAweGEw
MDAwLTB4YmZmZmZdIGFuZCBbaW8gMHgzYjAtMHgzYmJdIGFuZCANCj4+Pj4gW2lvIDB4M2MwLTB4
M2RmXS4gIEZvciBleGFtcGxlLCB3aGVuIGEgYnJpZGdlIGhhcyBpdHMgVkdBIEVuYWJsZSBiaXQg
DQo+Pj4+IHNldCwgaXQgcG9zaXRpdmVseSBkZWNvZGVzIFttZW0gMHhhMDAwMC0weGJmZmZmXSBl
dmVuIGlmIHRoYXQgcmFuZ2UgDQo+Pj4+IGlzbid0IGluY2x1ZGVkIGluIG9uZSBvZiB0aGUgYnJp
ZGdlIHdpbmRvd3MuICBJIGRvbid0IGtub3cgd2hldGhlciBhIA0KPj4+PiBWR0EgZGV2aWNlIGlz
IHNpbWlsYXJseSBhbGxvd2VkIHRvIGRlY29kZSB0aGF0IHJhbmdlIGV2ZW4gaWYgaXQncyANCj4+
Pj4gbm90IGluIGEgQkFSLiAgSWYgaXQgaXMsIEkgY291bGQgaW1hZ2luZSBpc3N1ZXMgaWYgZW5h
YmxpbmcgU1ItSU9WIA0KPj4+PiBjcmVhdGVkIHNldmVyYWwgVkdBIFZGcy4NCj4+PlZGcyBjYW5u
b3Qgc3VwcG9ydCBJL08gcG9ydCBzcGFjZSBieSBkZWZpbml0aW9uLCBzbyBJIGRvbid0IHRoaW5r
IGEgDQo+Pj4iVkdBIFZGIiBjb3VsZCBhY3R1YWxseSBleGlzdC4gIFRoZXJlIHdvdWxkIGJlIG5v
dGhpbmcgd3Jvbmcgd2l0aCBhIA0KPj4+bm9uLVZHQSBHUFUgVkYgdGhvdWdoLiAgSSBhbHNvIGRv
bid0IHNlZSBob3cgdGhlIGRpZmZlcmVuY2VzIGluIExlZ2FjeSANCj4+PkVuZHBvaW50IHJ1bGVz
IHZlcnN1cyBhIHN0YW5kYXJkIEVuZHBvaW50IHdvdWxkIHByZWNsdWRlIHN1cHBvcnRpbmcgDQo+
Pj5TUi1JT1YuICBJIGRvbid0IHRoaW5rIHRoZSBTUi1JT1Ygc3BlYyBtYWtlcyBhbnkgZGVtYW5k
cyBvbiB3aGV0aGVyIA0KPj4+dGhlIFBGIHJlcXVpcmVzIEkvTyBwb3J0IHJlc291cmNlcywgd2hp
Y2ggSSBhc3N1bWUgaXMgdGhlIG1haW4gcmVhc29uIA0KPj4+Zm9yIHRoaXMgdG8gY2FsbCBpdHNl
bGYgTGVnYWN5LiAgSSdkIGd1ZXNzIGl0IHdhcyBsaWtlbHkganVzdCBhbiANCj4+Pm92ZXJzaWdo
dCBhbmQgd2Ugc2hvdWxkIGFkZCBsZWdhY3kgZW5kcG9pbnRzIChvciByZW1vdmUgdGhlIHRlc3Qg
DQo+Pj5hbHRvZ2V0aGVyIGFuZCB0cnVzdCB0aGF0IGlmIGEgZGV2aWNlIGhhcyBhbiBTUi1JT1Yg
Y2FwYWJpbGl0eSwgd2UgDQo+Pj5zaG91bGQgaW5pdGlhbGl6ZSBpdCkuICBUaGFua3MsDQo+Pj4N
Cj4+PkFsZXgNCj4+Pg0KPj4+SSBhZ3JlZS4gSSB2YWd1ZWx5IHJlbWVtYmVyIHRoZXJlIHdhcyBz
b21lIHJlYXNvbiB0aGF0IGV4Y2x1ZGVzIGxlZ2FjeSANCj4+PmVuZHBvaW50cyBmcm9tIHVzaW5n
IFNSLUlPVi4gQnV0IGFmdGVyIGEgcXVpY2sgbG9vayBhdCB0aGUgbGF0ZXN0IA0KPj4+c3BlY3Ms
IEkgZGlkbid0IGZpbmQgYW55Lg0KPj4+DQo+PiBPbmx5IExlZ2FjeSBFbmRwb2ludHMgY2FuIGNs
YWltIEkvTy4gIFZGcyBhcmUgbm90IGFsbG93ZWQgdG8gY2xhaW0gSS9PLg0KPj4gVkdBIGRldmlj
ZXMgY2xhaW0gSS9PLg0KPj4NCj4+IFRoZSBTUi1JT1Ygc3BlYyAxLjEgc2VjdGlvbiAzLjQuMS42
IHN0YXRlcyDigJxUaGUgQ2xhc3MgQ29kZSByZWdpc3RlciBpcyANCj4+IHJlYWQtb25seSBhbmQg
aXMgdXNlZCB0byBpZGVudGlmeSB0aGUgZ2VuZXJpYyBmdW5jdGlvbiBvZiB0aGUgZGV2aWNlIA0K
Pj4gYW5kLCBpbiBzb21lIGNhc2VzLCBhIHNwZWNpZmljIHJlZ2lzdGVyIGxldmVsIHByb2dyYW1t
aW5nIGludGVyZmFjZS4NCj4+IFRoZSBmaWVsZCBpbiB0aGUgUEYgYW5kIGFzc29jaWF0ZWQgVkZz
IG11c3QgcmV0dXJuIHRoZSBzYW1lIHZhbHVlIHdoZW4gcmVhZC7igJ0NCj4+DQo+PiBJZiB0aGUg
UEYgaXMgYSBWR0EgZGV2aWNlIHRoZW4gYnkgdGhlIGRlZmluaXRpb24gaW4gdGhlIFNSLUlPViBz
cGVjIA0KPj4gdGhlIGNsYXNzIGNvZGUgb2YgdGhlIFZGIHdvdWxkIGFsc28gaW5kaWNhdGUgaXQg
YXMgYSBWR0EgZGV2aWNlLCBpZSANCj4+IFN1YmNsYXNzIDB4MCA9IFZHQSwgc3ViY2xhc3MgMHg4
MCA9IE9USEVSX0RJU1BMQVlfQ09OVFJPTExFUi4NCj4+DQo+PiBJZGVhbGx5IHdlIHdvdWxkIHdh
bnQgdG8gaGF2ZSB0aGUgUEYgc3ViLWNsYXNzIGFzIDB4MCBhbmQgdGhlIFZGIA0KPj4gc3ViY2xh
c3MgYXMgMHg4MCBCdXQgdGhlIHNwZWMgZG9lc24ndCBzdXBwb3J0IHRoaXMuDQo+Pg0KPj4gT25l
IHNwZWN1bGF0aW9uIGFzIHRvIHdoeSBMZWdhY3kgZW5kcG9pbnRzIHdlcmUgb21pdHRlZCBtaWdo
dCBiZSB0aGUgDQo+PiBhc3N1bXB0aW9uIHRoYXQgZG9pbmcgc28gd291bGQgYWxsb3cgVkdBIFZG
cyB0byBiZSBjcmVhdGVkLg0KPj4NCj4+IEl0IGlzIG5vdCByZWFzb25hYmxlIHRvIHByZXZlbnQg
YSBWR0EgUEYgZnJvbSBlbmFibGluZyBTUi1JT1YgYXMgdGhpcyANCj4+IGlzIGEgcmVhbCB3b3Js
ZCBwb3NzaWJpbGl0eS4gIFdlIG1pZ2h0IG5lZWQgdG8gYWRkIG1vcmUgY29kZSBlbHNld2hlcmUg
DQo+PiBob3dldmVyIHRvIHByZXZlbnQgYSBWRiBmcm9tIGJlY29taW5nIGEgVkdBIGRldmljZSBv
dXRzaWRlIG9mIHBhc3NpbmcgaXQgdGhyb3VnaCB0byBhIGd1ZXN0IFZNLg0KPg0KPkNhbiB5b3Ug
dHJ5IG91dCBZdSdzIHBhdGNoIGFuZCBzZWUgd2hhdCBoYXBwZW5zPyAgSWYgeW91IGRvIHR1cm4g
b24gU1ItSU9WIG9uIHRoaXMgZGV2aWNlLCBpdCB3b3VsZCBiZSBpbnRlcmVzdGluZyB0byBzZWUg
dGhlIGNvbXBsZXRlIGRtZXNnIGxvZyBhbmQgImxzcGNpIC12diIgb3V0cHV0IHRvID5zZWUgd2hh
dCB3ZSBkbyB3aXRoIGl0Lg0KPg0KPkJqb3JuDQoNCkkgYW0gYXdheSBmcm9tIG15IHRlc3Qgc3lz
dGVtIGZvciB0aGUgbmV4dCAyIHdlZWtzIGJ1dCBJIHdpbGwgZ2l2ZSBpdCBhIHRyeSB3aGVuIEkg
cmV0dXJuLiAgSSBoYXZlIGJlZW4gd29ya2luZyBhcm91bmQgdGhlIGlzc3VlIGJ5IGNoYW5naW5n
IHRoZSBwY2ktdHlwZSB0byBQQ0lfRVhQX1RZUEVfRU5EUE9JTlQgd2hpY2ggaGFzIHdvcmtlZCBm
b3IgbWUgZXZlbiB0aG91Z2ggaXQgaXMgbm90IGNvcnJlY3QuDQoNClRoYW5rcywNCktlbGx5DQo=

Re: Architectural question regarding IOV support in Linux 3.13.4

[Yu Zhao]

On Mon, Feb 24, 2014 at 01:59:01PM -0700, Alex Williamson wrote:
> On Mon, 2014-02-24 at 19:22 +0000, Zytaruk, Kelly wrote:
> > >On Fri, Feb 21, 2014 at 2:03 PM, Alex Williamson <alex.williamson@redhat.com> wrote:
> > >On Fri, 2014-02-21 at 14:11 -0700, Bjorn Helgaas wrote:
> > >> [+cc Alex, Yu]
> > >>
> > >> On Fri, Feb 21, 2014 at 10:45 AM, Zytaruk, Kelly <Kelly.Zytaruk@amd.com> wrote:
> > >> >
> > >> > I am working with SR-IOV and I have a question regarding the function
> > >> > sriov_init() in ../drivers/pci/iov.c (linux versions 3.4.9 and 3.13.4)
> > >> >
> > >> > In sriov_init() the code first checks whether the PF is a Root complex
> > >> >  endpoint (0x9) or an Express Endpoint (0x0) as shown in the code
> > >> >  snippet below.  If it is neither it returns the No device error.
> > >> >
> > >> > static int sriov_init(struct pci_dev *dev, int pos)
> > >> > {
> > >> >          int i;
> > >> >          int rc;
> > >> >          int nres;
> > >> >          u32 pgsz;
> > >> >          u16 ctrl, total, offset, stride;
> > >> >          struct pci_sriov *iov;
> > >> >          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;
> > >> >
> > >> > My question is why PCI_EXP_TYPE_LEG_END (0x1) is omitted as being a
> > >> >  valid endpoint.  By excluding Legacy endpoints it fails enabling
> > >> >  SR-IOV on a VGA PF.
> > >> >
> > >> > Is there a design/specification reason why legacy was excluded or was
> > >> >  it just an assumption that VGA would never support SR-IOV?
> > >> >
> > >> > If there is no valid reason to exclude PCI_EXP_TYPE_LEG_END, I would
> > >> >  like to discuss having it included as a valid endpoint for SR-IOV.
> > >>
> > >> Good question.  It looks like it's been that way since the beginning
> > >> [1], but I don't know why.  I don't see any restriction in the spec
> > >> about SR-IOV and legacy endpoints.
> > >>
> > >> I also don't know whether VGA is an issue.  There are some legacy
> > >> addressing issues for [mem 0xa0000-0xbffff] and [io 0x3b0-0x3bb] and
> > >> [io 0x3c0-0x3df].  For example, when a bridge has its VGA Enable bit
> > >> set, it positively decodes [mem 0xa0000-0xbffff] even if that range
> > >> isn't included in one of the bridge windows.  I don't know whether a
> > >> VGA device is similarly allowed to decode that range even if it's not
> > >> in a BAR.  If it is, I could imagine issues if enabling SR-IOV created
> > >> several VGA VFs.
> > >VFs cannot support I/O port space by definition, so I don't think a "VGA
> > >VF" could actually exist.  There would be nothing wrong with a non-VGA
> > >GPU VF though.  I also don't see how the differences in Legacy Endpoint
> > >rules versus a standard Endpoint would preclude supporting SR-IOV.  I
> > >don't think the SR-IOV spec makes any demands on whether the PF requires
> > >I/O port resources, which I assume is the main reason for this to call
> > >itself Legacy.  I'd guess it was likely just an oversight and we should
> > >add legacy endpoints (or remove the test altogether and trust that if a
> > >device has an SR-IOV capability, we should initialize it).  Thanks,
> > >
> > >Alex
> > >
> > >I agree. I vaguely remember there was some reason that excludes legacy
> > >endpoints from using SR-IOV. But after a quick look at the latest specs,
> > >I didn't find any.
> > >
> > Only Legacy Endpoints can claim I/O.  VFs are not allowed to claim I/O.
> > VGA devices claim I/O.
> > 
> > The SR-IOV spec 1.1 section 3.4.1.6 states
> > “The Class Code register is read-only and is used to identify the generic function 
> > of the device and, in some cases, a specific register level programming interface. 
> > The field in the PF and associated VFs must return the same value when read.”
> > 
> > If the PF is a VGA device then by the definition in the SR-IOV spec the class code 
> > of the VF would also indicate it as a VGA device, ie Subclass 0x0 = VGA, 
> > subclass 0x80 = OTHER_DISPLAY_CONTROLLER.  
> > 
> > Ideally we would want to have the PF sub-class as 0x0 and the VF subclass as 0x80
> > But the spec doesn't support this.
> > 
> > One speculation as to why Legacy endpoints were omitted might be the assumption
> > that doing so would allow VGA VFs to be created.
> > 
> > It is not reasonable to prevent a VGA PF from enabling SR-IOV as this is a real world 
> > possibility.  We might need to add more code elsewhere however to prevent a VF
> > from becoming a VGA device outside of passing it through to a guest VM.
> > 
> > Any thoughts on this?
> 
> Good catch, I think you'll need to contact the PCI SIG for
> clarification.  A VF with a VGA class code implies I/O space that a VF
> is not allowed to have.  I wouldn't be surprised if they had hoped VGA
> was a non-issue by this point and legacy-free graphics were standard.
> 
> Probably the most appealing solution would be an ECN allowing the VF to
> expose a different class code from the PF.  This seems generally useful
> even beyond the scope of this legacy resource issue.  Probably a less
> appealing option for you would be to expose the PF as a non-VGA display
> controller, which avoids both the legacy endpoint problem and the VF
> class code problem.  I suspect we wouldn't find a lot of support for a
> software only solution to virtualize the class code unless it was
> standardized.  Such a change would require support in every hypervisor.
> Thanks,
> 
> Alex
> 

SR-IOV device can have mixed function types. So the third option would
be to make the function 0 regular VGA device and function 1 PF with
subclass 0x80. By doing this, you retain the compatibility for your
host VGA driver and solve the subclass problem.

RE: Architectural question regarding IOV support in Linux 3.13.4

[Zytaruk, Kelly]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmpvcm4gSGVsZ2FhcyBb
bWFpbHRvOmJoZWxnYWFzQGdvb2dsZS5jb21dDQo+IFNlbnQ6IE1vbmRheSwgRmVicnVhcnkgMjQs
IDIwMTQgMzo1MiBQTQ0KPiBUbzogWnl0YXJ1aywgS2VsbHkNCj4gQ2M6IFl1IFpoYW87IGxpbnV4
LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IEFsZXggV2lsbGlhbXNvbg0KPiBTdWJqZWN0OiBSZTogQXJj
aGl0ZWN0dXJhbCBxdWVzdGlvbiByZWdhcmRpbmcgSU9WIHN1cHBvcnQgaW4gTGludXggMy4xMy40
DQo+IA0KPiBPbiBNb24sIEZlYiAyNCwgMjAxNCBhdCAxMjoyMiBQTSwgWnl0YXJ1aywgS2VsbHkg
PEtlbGx5Llp5dGFydWtAYW1kLmNvbT4NCj4gd3JvdGU6DQo+ID4NCj4gPj5PbiBGcmksIEZlYiAy
MSwgMjAxNCBhdCAyOjAzIFBNLCBBbGV4IFdpbGxpYW1zb24NCj4gPGFsZXgud2lsbGlhbXNvbkBy
ZWRoYXQuY29tPiB3cm90ZToNCj4gPj5PbiBGcmksIDIwMTQtMDItMjEgYXQgMTQ6MTEgLTA3MDAs
IEJqb3JuIEhlbGdhYXMgd3JvdGU6DQo+ID4+PiBbK2NjIEFsZXgsIFl1XQ0KPiA+Pj4NCj4gPj4+
IE9uIEZyaSwgRmViIDIxLCAyMDE0IGF0IDEwOjQ1IEFNLCBaeXRhcnVrLCBLZWxseSA8S2VsbHku
Wnl0YXJ1a0BhbWQuY29tPg0KPiB3cm90ZToNCj4gPj4+ID4NCj4gPj4+ID4gSSBhbSB3b3JraW5n
IHdpdGggU1ItSU9WIGFuZCBJIGhhdmUgYSBxdWVzdGlvbiByZWdhcmRpbmcgdGhlDQo+ID4+PiA+
IGZ1bmN0aW9uDQo+ID4+PiA+IHNyaW92X2luaXQoKSBpbiAuLi9kcml2ZXJzL3BjaS9pb3YuYyAo
bGludXggdmVyc2lvbnMgMy40LjkgYW5kDQo+ID4+PiA+IDMuMTMuNCkNCj4gPj4+ID4NCj4gPj4+
ID4gSW4gc3Jpb3ZfaW5pdCgpIHRoZSBjb2RlIGZpcnN0IGNoZWNrcyB3aGV0aGVyIHRoZSBQRiBp
cyBhIFJvb3QNCj4gPj4+ID4gY29tcGxleCAgZW5kcG9pbnQgKDB4OSkgb3IgYW4gRXhwcmVzcyBF
bmRwb2ludCAoMHgwKSBhcyBzaG93biBpbg0KPiA+Pj4gPiB0aGUgY29kZSAgc25pcHBldCBiZWxv
dy4gIElmIGl0IGlzIG5laXRoZXIgaXQgcmV0dXJucyB0aGUgTm8gZGV2aWNlIGVycm9yLg0KPiA+
Pj4gPg0KPiA+Pj4gPiBzdGF0aWMgaW50IHNyaW92X2luaXQoc3RydWN0IHBjaV9kZXYgKmRldiwg
aW50IHBvcykgew0KPiA+Pj4gPiAgICAgICAgICBpbnQgaTsNCj4gPj4+ID4gICAgICAgICAgaW50
IHJjOw0KPiA+Pj4gPiAgICAgICAgICBpbnQgbnJlczsNCj4gPj4+ID4gICAgICAgICAgdTMyIHBn
c3o7DQo+ID4+PiA+ICAgICAgICAgIHUxNiBjdHJsLCB0b3RhbCwgb2Zmc2V0LCBzdHJpZGU7DQo+
ID4+PiA+ICAgICAgICAgIHN0cnVjdCBwY2lfc3Jpb3YgKmlvdjsNCj4gPj4+ID4gICAgICAgICAg
c3RydWN0IHJlc291cmNlICpyZXM7DQo+ID4+PiA+ICAgICAgICAgIHN0cnVjdCBwY2lfZGV2ICpw
ZGV2Ow0KPiA+Pj4gPg0KPiA+Pj4gPiAgICAgICAgaWYgKHBjaV9wY2llX3R5cGUoZGV2KSAhPSBQ
Q0lfRVhQX1RZUEVfUkNfRU5EICYmDQo+ID4+PiA+ICAgICAgICAgICAgICBwY2lfcGNpZV90eXBl
KGRldikgIT0gUENJX0VYUF9UWVBFX0VORFBPSU5UKQ0KPiA+Pj4gPiAgICAgICAgICAgICAgICAg
IHJldHVybiAtRU5PREVWOw0KPiA+Pj4gPg0KPiA+Pj4gPiBNeSBxdWVzdGlvbiBpcyB3aHkgUENJ
X0VYUF9UWVBFX0xFR19FTkQgKDB4MSkgaXMgb21pdHRlZCBhcyBiZWluZw0KPiA+Pj4gPiBhICB2
YWxpZCBlbmRwb2ludC4gIEJ5IGV4Y2x1ZGluZyBMZWdhY3kgZW5kcG9pbnRzIGl0IGZhaWxzDQo+
ID4+PiA+IGVuYWJsaW5nICBTUi1JT1Ygb24gYSBWR0EgUEYuDQo+ID4+PiA+DQo+ID4+PiA+IElz
IHRoZXJlIGEgZGVzaWduL3NwZWNpZmljYXRpb24gcmVhc29uIHdoeSBsZWdhY3kgd2FzIGV4Y2x1
ZGVkIG9yDQo+ID4+PiA+IHdhcyAgaXQganVzdCBhbiBhc3N1bXB0aW9uIHRoYXQgVkdBIHdvdWxk
IG5ldmVyIHN1cHBvcnQgU1ItSU9WPw0KPiA+Pj4gPg0KPiA+Pj4gPiBJZiB0aGVyZSBpcyBubyB2
YWxpZCByZWFzb24gdG8gZXhjbHVkZSBQQ0lfRVhQX1RZUEVfTEVHX0VORCwgSQ0KPiA+Pj4gPiB3
b3VsZCAgbGlrZSB0byBkaXNjdXNzIGhhdmluZyBpdCBpbmNsdWRlZCBhcyBhIHZhbGlkIGVuZHBv
aW50IGZvciBTUi1JT1YuDQo+ID4+Pg0KPiA+Pj4gR29vZCBxdWVzdGlvbi4gIEl0IGxvb2tzIGxp
a2UgaXQncyBiZWVuIHRoYXQgd2F5IHNpbmNlIHRoZSBiZWdpbm5pbmcNCj4gPj4+IFsxXSwgYnV0
IEkgZG9uJ3Qga25vdyB3aHkuICBJIGRvbid0IHNlZSBhbnkgcmVzdHJpY3Rpb24gaW4gdGhlIHNw
ZWMNCj4gPj4+IGFib3V0IFNSLUlPViBhbmQgbGVnYWN5IGVuZHBvaW50cy4NCj4gPj4+DQo+ID4+
PiBJIGFsc28gZG9uJ3Qga25vdyB3aGV0aGVyIFZHQSBpcyBhbiBpc3N1ZS4gIFRoZXJlIGFyZSBz
b21lIGxlZ2FjeQ0KPiA+Pj4gYWRkcmVzc2luZyBpc3N1ZXMgZm9yIFttZW0gMHhhMDAwMC0weGJm
ZmZmXSBhbmQgW2lvIDB4M2IwLTB4M2JiXSBhbmQNCj4gPj4+IFtpbyAweDNjMC0weDNkZl0uICBG
b3IgZXhhbXBsZSwgd2hlbiBhIGJyaWRnZSBoYXMgaXRzIFZHQSBFbmFibGUgYml0DQo+ID4+PiBz
ZXQsIGl0IHBvc2l0aXZlbHkgZGVjb2RlcyBbbWVtIDB4YTAwMDAtMHhiZmZmZl0gZXZlbiBpZiB0
aGF0IHJhbmdlDQo+ID4+PiBpc24ndCBpbmNsdWRlZCBpbiBvbmUgb2YgdGhlIGJyaWRnZSB3aW5k
b3dzLiAgSSBkb24ndCBrbm93IHdoZXRoZXIgYQ0KPiA+Pj4gVkdBIGRldmljZSBpcyBzaW1pbGFy
bHkgYWxsb3dlZCB0byBkZWNvZGUgdGhhdCByYW5nZSBldmVuIGlmIGl0J3MNCj4gPj4+IG5vdCBp
biBhIEJBUi4gIElmIGl0IGlzLCBJIGNvdWxkIGltYWdpbmUgaXNzdWVzIGlmIGVuYWJsaW5nIFNS
LUlPVg0KPiA+Pj4gY3JlYXRlZCBzZXZlcmFsIFZHQSBWRnMuDQo+ID4+VkZzIGNhbm5vdCBzdXBw
b3J0IEkvTyBwb3J0IHNwYWNlIGJ5IGRlZmluaXRpb24sIHNvIEkgZG9uJ3QgdGhpbmsgYQ0KPiA+
PiJWR0EgVkYiIGNvdWxkIGFjdHVhbGx5IGV4aXN0LiAgVGhlcmUgd291bGQgYmUgbm90aGluZyB3
cm9uZyB3aXRoIGENCj4gPj5ub24tVkdBIEdQVSBWRiB0aG91Z2guICBJIGFsc28gZG9uJ3Qgc2Vl
IGhvdyB0aGUgZGlmZmVyZW5jZXMgaW4gTGVnYWN5DQo+ID4+RW5kcG9pbnQgcnVsZXMgdmVyc3Vz
IGEgc3RhbmRhcmQgRW5kcG9pbnQgd291bGQgcHJlY2x1ZGUgc3VwcG9ydGluZw0KPiA+PlNSLUlP
Vi4gIEkgZG9uJ3QgdGhpbmsgdGhlIFNSLUlPViBzcGVjIG1ha2VzIGFueSBkZW1hbmRzIG9uIHdo
ZXRoZXINCj4gPj50aGUgUEYgcmVxdWlyZXMgSS9PIHBvcnQgcmVzb3VyY2VzLCB3aGljaCBJIGFz
c3VtZSBpcyB0aGUgbWFpbiByZWFzb24NCj4gPj5mb3IgdGhpcyB0byBjYWxsIGl0c2VsZiBMZWdh
Y3kuICBJJ2QgZ3Vlc3MgaXQgd2FzIGxpa2VseSBqdXN0IGFuDQo+ID4+b3ZlcnNpZ2h0IGFuZCB3
ZSBzaG91bGQgYWRkIGxlZ2FjeSBlbmRwb2ludHMgKG9yIHJlbW92ZSB0aGUgdGVzdA0KPiA+PmFs
dG9nZXRoZXIgYW5kIHRydXN0IHRoYXQgaWYgYSBkZXZpY2UgaGFzIGFuIFNSLUlPViBjYXBhYmls
aXR5LCB3ZQ0KPiA+PnNob3VsZCBpbml0aWFsaXplIGl0KS4gIFRoYW5rcywNCj4gPj4NCj4gPj5B
bGV4DQo+ID4+DQo+ID4+SSBhZ3JlZS4gSSB2YWd1ZWx5IHJlbWVtYmVyIHRoZXJlIHdhcyBzb21l
IHJlYXNvbiB0aGF0IGV4Y2x1ZGVzIGxlZ2FjeQ0KPiA+PmVuZHBvaW50cyBmcm9tIHVzaW5nIFNS
LUlPVi4gQnV0IGFmdGVyIGEgcXVpY2sgbG9vayBhdCB0aGUgbGF0ZXN0DQo+ID4+c3BlY3MsIEkg
ZGlkbid0IGZpbmQgYW55Lg0KPiA+Pg0KPiA+IE9ubHkgTGVnYWN5IEVuZHBvaW50cyBjYW4gY2xh
aW0gSS9PLiAgVkZzIGFyZSBub3QgYWxsb3dlZCB0byBjbGFpbSBJL08uDQo+ID4gVkdBIGRldmlj
ZXMgY2xhaW0gSS9PLg0KPiA+DQo+ID4gVGhlIFNSLUlPViBzcGVjIDEuMSBzZWN0aW9uIDMuNC4x
LjYgc3RhdGVzIOKAnFRoZSBDbGFzcyBDb2RlIHJlZ2lzdGVyIGlzDQo+ID4gcmVhZC1vbmx5IGFu
ZCBpcyB1c2VkIHRvIGlkZW50aWZ5IHRoZSBnZW5lcmljIGZ1bmN0aW9uIG9mIHRoZSBkZXZpY2UN
Cj4gPiBhbmQsIGluIHNvbWUgY2FzZXMsIGEgc3BlY2lmaWMgcmVnaXN0ZXIgbGV2ZWwgcHJvZ3Jh
bW1pbmcgaW50ZXJmYWNlLg0KPiA+IFRoZSBmaWVsZCBpbiB0aGUgUEYgYW5kIGFzc29jaWF0ZWQg
VkZzIG11c3QgcmV0dXJuIHRoZSBzYW1lIHZhbHVlIHdoZW4gcmVhZC7igJ0NCj4gPg0KPiA+IElm
IHRoZSBQRiBpcyBhIFZHQSBkZXZpY2UgdGhlbiBieSB0aGUgZGVmaW5pdGlvbiBpbiB0aGUgU1It
SU9WIHNwZWMNCj4gPiB0aGUgY2xhc3MgY29kZSBvZiB0aGUgVkYgd291bGQgYWxzbyBpbmRpY2F0
ZSBpdCBhcyBhIFZHQSBkZXZpY2UsIGllDQo+ID4gU3ViY2xhc3MgMHgwID0gVkdBLCBzdWJjbGFz
cyAweDgwID0gT1RIRVJfRElTUExBWV9DT05UUk9MTEVSLg0KPiA+DQo+ID4gSWRlYWxseSB3ZSB3
b3VsZCB3YW50IHRvIGhhdmUgdGhlIFBGIHN1Yi1jbGFzcyBhcyAweDAgYW5kIHRoZSBWRg0KPiA+
IHN1YmNsYXNzIGFzIDB4ODAgQnV0IHRoZSBzcGVjIGRvZXNuJ3Qgc3VwcG9ydCB0aGlzLg0KPiA+
DQo+ID4gT25lIHNwZWN1bGF0aW9uIGFzIHRvIHdoeSBMZWdhY3kgZW5kcG9pbnRzIHdlcmUgb21p
dHRlZCBtaWdodCBiZSB0aGUNCj4gPiBhc3N1bXB0aW9uIHRoYXQgZG9pbmcgc28gd291bGQgYWxs
b3cgVkdBIFZGcyB0byBiZSBjcmVhdGVkLg0KPiA+DQo+ID4gSXQgaXMgbm90IHJlYXNvbmFibGUg
dG8gcHJldmVudCBhIFZHQSBQRiBmcm9tIGVuYWJsaW5nIFNSLUlPViBhcyB0aGlzDQo+ID4gaXMg
YSByZWFsIHdvcmxkIHBvc3NpYmlsaXR5LiAgV2UgbWlnaHQgbmVlZCB0byBhZGQgbW9yZSBjb2Rl
IGVsc2V3aGVyZQ0KPiA+IGhvd2V2ZXIgdG8gcHJldmVudCBhIFZGIGZyb20gYmVjb21pbmcgYSBW
R0EgZGV2aWNlIG91dHNpZGUgb2YgcGFzc2luZyBpdA0KPiB0aHJvdWdoIHRvIGEgZ3Vlc3QgVk0u
DQo+IA0KPiBDYW4geW91IHRyeSBvdXQgWXUncyBwYXRjaCBhbmQgc2VlIHdoYXQgaGFwcGVucz8g
IElmIHlvdSBkbyB0dXJuIG9uIFNSLUlPViBvbg0KPiB0aGlzIGRldmljZSwgaXQgd291bGQgYmUg
aW50ZXJlc3RpbmcgdG8gc2VlIHRoZSBjb21wbGV0ZSBkbWVzZyBsb2cgYW5kICJsc3BjaSAtdnYi
DQo+IG91dHB1dCB0byBzZWUgd2hhdCB3ZSBkbyB3aXRoIGl0Lg0KPiANCj4gQmpvcm4NCg0KQmpv
cm4sDQpJIGFtIHJlc3VycmVjdGluZyB0aGlzIHRocmVhZCBhcyBJIGhhdmUgYmVlbiBsb29raW5n
IHRocm91Z2ggdGhlIDMuMTggc291cmNlIGNvZGUgYW5kIGRvbid0IHNlZSBhbnkgc3VwcG9ydCBm
b3IgUENJX0VYUF9UWVBFX0xFR19FTkQgZm9yIFNSSU9WLiAgRm9yIGRldmljZXMgdGhhdCBtdXN0
IHJlbWFpbiBkZWZpbmVkIGFzIExlZ2FjeSBlbmRwb2ludHMgd2Ugc3RpbGwgbmVlZCB0byBiZSBh
YmxlIHRvIGVuYWJsZSBTUklPVi4NCg0KRG8geW91IGhhdmUgYW55IGNvbmNlcm5zIG9uIGFkZGlu
ZyB0aGUgbGVnYWN5IHN1cHBvcnQgdG8gc3Jpb3ZfaW5pdCgpIGFzIHNob3duIGJlbG93Pw0KDQoJ
aWYgKChkZXYtPnBjaWVfdHlwZSAhPSBQQ0lfRVhQX1RZUEVfUkNfRU5EKSAmJg0KCSAgICAoZGV2
LT5wY2llX3R5cGUgIT0gUENJX0VYUF9UWVBFX0VORFBPSU5UKSAmJg0KICAgICAgICAgICAgICAg
ICAgKGRldi0+cGNpZV90eXBlICE9IFBDSV9FWFBfVFlQRV9MRUdfRU5EKSkNCgl7DQoJCXJldHVy
biAtRU5PREVWOw0KCX0NCg0KVGhhbmtzLA0KS2VsbHkNCg==

Re: Architectural question regarding IOV support in Linux 3.13.4

[Bjorn Helgaas]

On Tue, Dec 2, 2014 at 9:42 AM, Zytaruk, Kelly <Kelly.Zytaruk@amd.com> wrote:
>
>
>> -----Original Message-----
>> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
>> Sent: Monday, February 24, 2014 3:52 PM
>> To: Zytaruk, Kelly
>> Cc: Yu Zhao; linux-pci@vger.kernel.org; Alex Williamson
>> Subject: Re: Architectural question regarding IOV support in Linux 3.13.4
>>
>> On Mon, Feb 24, 2014 at 12:22 PM, Zytaruk, Kelly <Kelly.Zytaruk@amd.com>
>> wrote:
>> >
>> >>On Fri, Feb 21, 2014 at 2:03 PM, Alex Williamson
>> <alex.williamson@redhat.com> wrote:
>> >>On Fri, 2014-02-21 at 14:11 -0700, Bjorn Helgaas wrote:
>> >>> [+cc Alex, Yu]
>> >>>
>> >>> On Fri, Feb 21, 2014 at 10:45 AM, Zytaruk, Kelly <Kelly.Zytaruk@amd.com>
>> wrote:
>> >>> >
>> >>> > I am working with SR-IOV and I have a question regarding the
>> >>> > function
>> >>> > sriov_init() in ../drivers/pci/iov.c (linux versions 3.4.9 and
>> >>> > 3.13.4)
>> >>> >
>> >>> > In sriov_init() the code first checks whether the PF is a Root
>> >>> > complex  endpoint (0x9) or an Express Endpoint (0x0) as shown in
>> >>> > the code  snippet below.  If it is neither it returns the No device error.
>> >>> >
>> >>> > static int sriov_init(struct pci_dev *dev, int pos) {
>> >>> >          int i;
>> >>> >          int rc;
>> >>> >          int nres;
>> >>> >          u32 pgsz;
>> >>> >          u16 ctrl, total, offset, stride;
>> >>> >          struct pci_sriov *iov;
>> >>> >          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;
>> >>> >
>> >>> > My question is why PCI_EXP_TYPE_LEG_END (0x1) is omitted as being
>> >>> > a  valid endpoint.  By excluding Legacy endpoints it fails
>> >>> > enabling  SR-IOV on a VGA PF.
>> >>> >
>> >>> > Is there a design/specification reason why legacy was excluded or
>> >>> > was  it just an assumption that VGA would never support SR-IOV?
>> >>> >
>> >>> > If there is no valid reason to exclude PCI_EXP_TYPE_LEG_END, I
>> >>> > would  like to discuss having it included as a valid endpoint for SR-IOV.
>> >>>
>> >>> Good question.  It looks like it's been that way since the beginning
>> >>> [1], but I don't know why.  I don't see any restriction in the spec
>> >>> about SR-IOV and legacy endpoints.
>> >>>
>> >>> I also don't know whether VGA is an issue.  There are some legacy
>> >>> addressing issues for [mem 0xa0000-0xbffff] and [io 0x3b0-0x3bb] and
>> >>> [io 0x3c0-0x3df].  For example, when a bridge has its VGA Enable bit
>> >>> set, it positively decodes [mem 0xa0000-0xbffff] even if that range
>> >>> isn't included in one of the bridge windows.  I don't know whether a
>> >>> VGA device is similarly allowed to decode that range even if it's
>> >>> not in a BAR.  If it is, I could imagine issues if enabling SR-IOV
>> >>> created several VGA VFs.
>> >>VFs cannot support I/O port space by definition, so I don't think a
>> >>"VGA VF" could actually exist.  There would be nothing wrong with a
>> >>non-VGA GPU VF though.  I also don't see how the differences in Legacy
>> >>Endpoint rules versus a standard Endpoint would preclude supporting
>> >>SR-IOV.  I don't think the SR-IOV spec makes any demands on whether
>> >>the PF requires I/O port resources, which I assume is the main reason
>> >>for this to call itself Legacy.  I'd guess it was likely just an
>> >>oversight and we should add legacy endpoints (or remove the test
>> >>altogether and trust that if a device has an SR-IOV capability, we
>> >>should initialize it).  Thanks,
>> >>
>> >>Alex
>> >>
>> >>I agree. I vaguely remember there was some reason that excludes legacy
>> >>endpoints from using SR-IOV. But after a quick look at the latest
>> >>specs, I didn't find any.
>> >>
>> > Only Legacy Endpoints can claim I/O.  VFs are not allowed to claim I/O.
>> > VGA devices claim I/O.
>> >
>> > The SR-IOV spec 1.1 section 3.4.1.6 states “The Class Code register is
>> > read-only and is used to identify the generic function of the device
>> > and, in some cases, a specific register level programming interface.
>> > The field in the PF and associated VFs must return the same value when read.”
>> >
>> > If the PF is a VGA device then by the definition in the SR-IOV spec
>> > the class code of the VF would also indicate it as a VGA device, ie
>> > Subclass 0x0 = VGA, subclass 0x80 = OTHER_DISPLAY_CONTROLLER.
>> >
>> > Ideally we would want to have the PF sub-class as 0x0 and the VF
>> > subclass as 0x80 But the spec doesn't support this.
>> >
>> > One speculation as to why Legacy endpoints were omitted might be the
>> > assumption that doing so would allow VGA VFs to be created.
>> >
>> > It is not reasonable to prevent a VGA PF from enabling SR-IOV as this
>> > is a real world possibility.  We might need to add more code elsewhere
>> > however to prevent a VF from becoming a VGA device outside of passing it
>> through to a guest VM.
>>
>> Can you try out Yu's patch and see what happens?  If you do turn on SR-IOV on
>> this device, it would be interesting to see the complete dmesg log and "lspci -vv"
>> output to see what we do with it.
>>
>> Bjorn
>
> Bjorn,
> I am resurrecting this thread as I have been looking through the 3.18 source code and don't see any support for PCI_EXP_TYPE_LEG_END for SRIOV.  For devices that must remain defined as Legacy endpoints we still need to be able to enable SRIOV.

Looking back, I see that Yu Zhao did attach a patch earlier in the
thread.  Are you asking me to apply that one?  I don't remember why I
dropped it from my queue; maybe it was because I was waiting for you
to test it.

Did you test it?  What were the results (complete dmesg log and lspci
-vv output)?  There was also some follow-on discussion that I would
need to evaluate before applying it.

Bjorn

> Do you have any concerns on adding the legacy support to sriov_init() as shown below?
>
>         if ((dev->pcie_type != PCI_EXP_TYPE_RC_END) &&
>             (dev->pcie_type != PCI_EXP_TYPE_ENDPOINT) &&
>                   (dev->pcie_type != PCI_EXP_TYPE_LEG_END))
>         {
>                 return -ENODEV;
>         }
>
> Thanks,
> Kelly

RE: Architectural question regarding IOV support in Linux 3.13.4

[Zytaruk, Kelly]

DQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IGxpbnV4LXBjaS1vd25lckB2
Z2VyLmtlcm5lbC5vcmcgW21haWx0bzpsaW51eC1wY2ktDQo+IG93bmVyQHZnZXIua2VybmVsLm9y
Z10gT24gQmVoYWxmIE9mIEJqb3JuIEhlbGdhYXMNCj4gU2VudDogVHVlc2RheSwgRGVjZW1iZXIg
MDIsIDIwMTQgNDowMiBQTQ0KPiBUbzogWnl0YXJ1aywgS2VsbHkNCj4gQ2M6IFl1IFpoYW87IGxp
bnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IEFsZXggV2lsbGlhbXNvbg0KPiBTdWJqZWN0OiBSZTog
QXJjaGl0ZWN0dXJhbCBxdWVzdGlvbiByZWdhcmRpbmcgSU9WIHN1cHBvcnQgaW4gTGludXggMy4x
My40DQo+IA0KPiBPbiBUdWUsIERlYyAyLCAyMDE0IGF0IDk6NDIgQU0sIFp5dGFydWssIEtlbGx5
IDxLZWxseS5aeXRhcnVrQGFtZC5jb20+DQo+IHdyb3RlOg0KPiA+DQo+ID4NCj4gPj4gLS0tLS1P
cmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPj4gRnJvbTogQmpvcm4gSGVsZ2FhcyBbbWFpbHRvOmJo
ZWxnYWFzQGdvb2dsZS5jb21dDQo+ID4+IFNlbnQ6IE1vbmRheSwgRmVicnVhcnkgMjQsIDIwMTQg
Mzo1MiBQTQ0KPiA+PiBUbzogWnl0YXJ1aywgS2VsbHkNCj4gPj4gQ2M6IFl1IFpoYW87IGxpbnV4
LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IEFsZXggV2lsbGlhbXNvbg0KPiA+PiBTdWJqZWN0OiBSZTog
QXJjaGl0ZWN0dXJhbCBxdWVzdGlvbiByZWdhcmRpbmcgSU9WIHN1cHBvcnQgaW4gTGludXgNCj4g
Pj4gMy4xMy40DQo+ID4+DQo+ID4+IE9uIE1vbiwgRmViIDI0LCAyMDE0IGF0IDEyOjIyIFBNLCBa
eXRhcnVrLCBLZWxseQ0KPiA+PiA8S2VsbHkuWnl0YXJ1a0BhbWQuY29tPg0KPiA+PiB3cm90ZToN
Cj4gPj4gPg0KPiA+PiA+Pk9uIEZyaSwgRmViIDIxLCAyMDE0IGF0IDI6MDMgUE0sIEFsZXggV2ls
bGlhbXNvbg0KPiA+PiA8YWxleC53aWxsaWFtc29uQHJlZGhhdC5jb20+IHdyb3RlOg0KPiA+PiA+
Pk9uIEZyaSwgMjAxNC0wMi0yMSBhdCAxNDoxMSAtMDcwMCwgQmpvcm4gSGVsZ2FhcyB3cm90ZToN
Cj4gPj4gPj4+IFsrY2MgQWxleCwgWXVdDQo+ID4+ID4+Pg0KPiA+PiA+Pj4gT24gRnJpLCBGZWIg
MjEsIDIwMTQgYXQgMTA6NDUgQU0sIFp5dGFydWssIEtlbGx5DQo+ID4+ID4+PiA8S2VsbHkuWnl0
YXJ1a0BhbWQuY29tPg0KPiA+PiB3cm90ZToNCj4gPj4gPj4+ID4NCj4gPj4gPj4+ID4gSSBhbSB3
b3JraW5nIHdpdGggU1ItSU9WIGFuZCBJIGhhdmUgYSBxdWVzdGlvbiByZWdhcmRpbmcgdGhlDQo+
ID4+ID4+PiA+IGZ1bmN0aW9uDQo+ID4+ID4+PiA+IHNyaW92X2luaXQoKSBpbiAuLi9kcml2ZXJz
L3BjaS9pb3YuYyAobGludXggdmVyc2lvbnMgMy40LjkgYW5kDQo+ID4+ID4+PiA+IDMuMTMuNCkN
Cj4gPj4gPj4+ID4NCj4gPj4gPj4+ID4gSW4gc3Jpb3ZfaW5pdCgpIHRoZSBjb2RlIGZpcnN0IGNo
ZWNrcyB3aGV0aGVyIHRoZSBQRiBpcyBhIFJvb3QNCj4gPj4gPj4+ID4gY29tcGxleCAgZW5kcG9p
bnQgKDB4OSkgb3IgYW4gRXhwcmVzcyBFbmRwb2ludCAoMHgwKSBhcyBzaG93bg0KPiA+PiA+Pj4g
PiBpbiB0aGUgY29kZSAgc25pcHBldCBiZWxvdy4gIElmIGl0IGlzIG5laXRoZXIgaXQgcmV0dXJu
cyB0aGUgTm8gZGV2aWNlIGVycm9yLg0KPiA+PiA+Pj4gPg0KPiA+PiA+Pj4gPiBzdGF0aWMgaW50
IHNyaW92X2luaXQoc3RydWN0IHBjaV9kZXYgKmRldiwgaW50IHBvcykgew0KPiA+PiA+Pj4gPiAg
ICAgICAgICBpbnQgaTsNCj4gPj4gPj4+ID4gICAgICAgICAgaW50IHJjOw0KPiA+PiA+Pj4gPiAg
ICAgICAgICBpbnQgbnJlczsNCj4gPj4gPj4+ID4gICAgICAgICAgdTMyIHBnc3o7DQo+ID4+ID4+
PiA+ICAgICAgICAgIHUxNiBjdHJsLCB0b3RhbCwgb2Zmc2V0LCBzdHJpZGU7DQo+ID4+ID4+PiA+
ICAgICAgICAgIHN0cnVjdCBwY2lfc3Jpb3YgKmlvdjsNCj4gPj4gPj4+ID4gICAgICAgICAgc3Ry
dWN0IHJlc291cmNlICpyZXM7DQo+ID4+ID4+PiA+ICAgICAgICAgIHN0cnVjdCBwY2lfZGV2ICpw
ZGV2Ow0KPiA+PiA+Pj4gPg0KPiA+PiA+Pj4gPiAgICAgICAgaWYgKHBjaV9wY2llX3R5cGUoZGV2
KSAhPSBQQ0lfRVhQX1RZUEVfUkNfRU5EICYmDQo+ID4+ID4+PiA+ICAgICAgICAgICAgICBwY2lf
cGNpZV90eXBlKGRldikgIT0gUENJX0VYUF9UWVBFX0VORFBPSU5UKQ0KPiA+PiA+Pj4gPiAgICAg
ICAgICAgICAgICAgIHJldHVybiAtRU5PREVWOw0KPiA+PiA+Pj4gPg0KPiA+PiA+Pj4gPiBNeSBx
dWVzdGlvbiBpcyB3aHkgUENJX0VYUF9UWVBFX0xFR19FTkQgKDB4MSkgaXMgb21pdHRlZCBhcw0K
PiA+PiA+Pj4gPiBiZWluZyBhICB2YWxpZCBlbmRwb2ludC4gIEJ5IGV4Y2x1ZGluZyBMZWdhY3kg
ZW5kcG9pbnRzIGl0DQo+ID4+ID4+PiA+IGZhaWxzIGVuYWJsaW5nICBTUi1JT1Ygb24gYSBWR0Eg
UEYuDQo+ID4+ID4+PiA+DQo+ID4+ID4+PiA+IElzIHRoZXJlIGEgZGVzaWduL3NwZWNpZmljYXRp
b24gcmVhc29uIHdoeSBsZWdhY3kgd2FzIGV4Y2x1ZGVkDQo+ID4+ID4+PiA+IG9yIHdhcyAgaXQg
anVzdCBhbiBhc3N1bXB0aW9uIHRoYXQgVkdBIHdvdWxkIG5ldmVyIHN1cHBvcnQgU1ItSU9WPw0K
PiA+PiA+Pj4gPg0KPiA+PiA+Pj4gPiBJZiB0aGVyZSBpcyBubyB2YWxpZCByZWFzb24gdG8gZXhj
bHVkZSBQQ0lfRVhQX1RZUEVfTEVHX0VORCwgSQ0KPiA+PiA+Pj4gPiB3b3VsZCAgbGlrZSB0byBk
aXNjdXNzIGhhdmluZyBpdCBpbmNsdWRlZCBhcyBhIHZhbGlkIGVuZHBvaW50IGZvciBTUi1JT1Yu
DQo+ID4+ID4+Pg0KPiA+PiA+Pj4gR29vZCBxdWVzdGlvbi4gIEl0IGxvb2tzIGxpa2UgaXQncyBi
ZWVuIHRoYXQgd2F5IHNpbmNlIHRoZQ0KPiA+PiA+Pj4gYmVnaW5uaW5nIFsxXSwgYnV0IEkgZG9u
J3Qga25vdyB3aHkuICBJIGRvbid0IHNlZSBhbnkgcmVzdHJpY3Rpb24NCj4gPj4gPj4+IGluIHRo
ZSBzcGVjIGFib3V0IFNSLUlPViBhbmQgbGVnYWN5IGVuZHBvaW50cy4NCj4gPj4gPj4+DQo+ID4+
ID4+PiBJIGFsc28gZG9uJ3Qga25vdyB3aGV0aGVyIFZHQSBpcyBhbiBpc3N1ZS4gIFRoZXJlIGFy
ZSBzb21lIGxlZ2FjeQ0KPiA+PiA+Pj4gYWRkcmVzc2luZyBpc3N1ZXMgZm9yIFttZW0gMHhhMDAw
MC0weGJmZmZmXSBhbmQgW2lvIDB4M2IwLTB4M2JiXQ0KPiA+PiA+Pj4gYW5kIFtpbyAweDNjMC0w
eDNkZl0uICBGb3IgZXhhbXBsZSwgd2hlbiBhIGJyaWRnZSBoYXMgaXRzIFZHQQ0KPiA+PiA+Pj4g
RW5hYmxlIGJpdCBzZXQsIGl0IHBvc2l0aXZlbHkgZGVjb2RlcyBbbWVtIDB4YTAwMDAtMHhiZmZm
Zl0gZXZlbg0KPiA+PiA+Pj4gaWYgdGhhdCByYW5nZSBpc24ndCBpbmNsdWRlZCBpbiBvbmUgb2Yg
dGhlIGJyaWRnZSB3aW5kb3dzLiAgSQ0KPiA+PiA+Pj4gZG9uJ3Qga25vdyB3aGV0aGVyIGEgVkdB
IGRldmljZSBpcyBzaW1pbGFybHkgYWxsb3dlZCB0byBkZWNvZGUNCj4gPj4gPj4+IHRoYXQgcmFu
Z2UgZXZlbiBpZiBpdCdzIG5vdCBpbiBhIEJBUi4gIElmIGl0IGlzLCBJIGNvdWxkIGltYWdpbmUN
Cj4gPj4gPj4+IGlzc3VlcyBpZiBlbmFibGluZyBTUi1JT1YgY3JlYXRlZCBzZXZlcmFsIFZHQSBW
RnMuDQo+ID4+ID4+VkZzIGNhbm5vdCBzdXBwb3J0IEkvTyBwb3J0IHNwYWNlIGJ5IGRlZmluaXRp
b24sIHNvIEkgZG9uJ3QgdGhpbmsgYQ0KPiA+PiA+PiJWR0EgVkYiIGNvdWxkIGFjdHVhbGx5IGV4
aXN0LiAgVGhlcmUgd291bGQgYmUgbm90aGluZyB3cm9uZyB3aXRoIGENCj4gPj4gPj5ub24tVkdB
IEdQVSBWRiB0aG91Z2guICBJIGFsc28gZG9uJ3Qgc2VlIGhvdyB0aGUgZGlmZmVyZW5jZXMgaW4N
Cj4gPj4gPj5MZWdhY3kgRW5kcG9pbnQgcnVsZXMgdmVyc3VzIGEgc3RhbmRhcmQgRW5kcG9pbnQg
d291bGQgcHJlY2x1ZGUNCj4gPj4gPj5zdXBwb3J0aW5nIFNSLUlPVi4gIEkgZG9uJ3QgdGhpbmsg
dGhlIFNSLUlPViBzcGVjIG1ha2VzIGFueSBkZW1hbmRzDQo+ID4+ID4+b24gd2hldGhlciB0aGUg
UEYgcmVxdWlyZXMgSS9PIHBvcnQgcmVzb3VyY2VzLCB3aGljaCBJIGFzc3VtZSBpcw0KPiA+PiA+
PnRoZSBtYWluIHJlYXNvbiBmb3IgdGhpcyB0byBjYWxsIGl0c2VsZiBMZWdhY3kuICBJJ2QgZ3Vl
c3MgaXQgd2FzDQo+ID4+ID4+bGlrZWx5IGp1c3QgYW4gb3ZlcnNpZ2h0IGFuZCB3ZSBzaG91bGQg
YWRkIGxlZ2FjeSBlbmRwb2ludHMgKG9yDQo+ID4+ID4+cmVtb3ZlIHRoZSB0ZXN0IGFsdG9nZXRo
ZXIgYW5kIHRydXN0IHRoYXQgaWYgYSBkZXZpY2UgaGFzIGFuIFNSLUlPVg0KPiA+PiA+PmNhcGFi
aWxpdHksIHdlIHNob3VsZCBpbml0aWFsaXplIGl0KS4gIFRoYW5rcywNCj4gPj4gPj4NCj4gPj4g
Pj5BbGV4DQo+ID4+ID4+DQo+ID4+ID4+SSBhZ3JlZS4gSSB2YWd1ZWx5IHJlbWVtYmVyIHRoZXJl
IHdhcyBzb21lIHJlYXNvbiB0aGF0IGV4Y2x1ZGVzDQo+ID4+ID4+bGVnYWN5IGVuZHBvaW50cyBm
cm9tIHVzaW5nIFNSLUlPVi4gQnV0IGFmdGVyIGEgcXVpY2sgbG9vayBhdCB0aGUNCj4gPj4gPj5s
YXRlc3Qgc3BlY3MsIEkgZGlkbid0IGZpbmQgYW55Lg0KPiA+PiA+Pg0KPiA+PiA+IE9ubHkgTGVn
YWN5IEVuZHBvaW50cyBjYW4gY2xhaW0gSS9PLiAgVkZzIGFyZSBub3QgYWxsb3dlZCB0byBjbGFp
bSBJL08uDQo+ID4+ID4gVkdBIGRldmljZXMgY2xhaW0gSS9PLg0KPiA+PiA+DQo+ID4+ID4gVGhl
IFNSLUlPViBzcGVjIDEuMSBzZWN0aW9uIDMuNC4xLjYgc3RhdGVzIOKAnFRoZSBDbGFzcyBDb2Rl
IHJlZ2lzdGVyDQo+ID4+ID4gaXMgcmVhZC1vbmx5IGFuZCBpcyB1c2VkIHRvIGlkZW50aWZ5IHRo
ZSBnZW5lcmljIGZ1bmN0aW9uIG9mIHRoZQ0KPiA+PiA+IGRldmljZSBhbmQsIGluIHNvbWUgY2Fz
ZXMsIGEgc3BlY2lmaWMgcmVnaXN0ZXIgbGV2ZWwgcHJvZ3JhbW1pbmcgaW50ZXJmYWNlLg0KPiA+
PiA+IFRoZSBmaWVsZCBpbiB0aGUgUEYgYW5kIGFzc29jaWF0ZWQgVkZzIG11c3QgcmV0dXJuIHRo
ZSBzYW1lIHZhbHVlIHdoZW4NCj4gcmVhZC7igJ0NCj4gPj4gPg0KPiA+PiA+IElmIHRoZSBQRiBp
cyBhIFZHQSBkZXZpY2UgdGhlbiBieSB0aGUgZGVmaW5pdGlvbiBpbiB0aGUgU1ItSU9WIHNwZWMN
Cj4gPj4gPiB0aGUgY2xhc3MgY29kZSBvZiB0aGUgVkYgd291bGQgYWxzbyBpbmRpY2F0ZSBpdCBh
cyBhIFZHQSBkZXZpY2UsIGllDQo+ID4+ID4gU3ViY2xhc3MgMHgwID0gVkdBLCBzdWJjbGFzcyAw
eDgwID0gT1RIRVJfRElTUExBWV9DT05UUk9MTEVSLg0KPiA+PiA+DQo+ID4+ID4gSWRlYWxseSB3
ZSB3b3VsZCB3YW50IHRvIGhhdmUgdGhlIFBGIHN1Yi1jbGFzcyBhcyAweDAgYW5kIHRoZSBWRg0K
PiA+PiA+IHN1YmNsYXNzIGFzIDB4ODAgQnV0IHRoZSBzcGVjIGRvZXNuJ3Qgc3VwcG9ydCB0aGlz
Lg0KPiA+PiA+DQo+ID4+ID4gT25lIHNwZWN1bGF0aW9uIGFzIHRvIHdoeSBMZWdhY3kgZW5kcG9p
bnRzIHdlcmUgb21pdHRlZCBtaWdodCBiZQ0KPiA+PiA+IHRoZSBhc3N1bXB0aW9uIHRoYXQgZG9p
bmcgc28gd291bGQgYWxsb3cgVkdBIFZGcyB0byBiZSBjcmVhdGVkLg0KPiA+PiA+DQo+ID4+ID4g
SXQgaXMgbm90IHJlYXNvbmFibGUgdG8gcHJldmVudCBhIFZHQSBQRiBmcm9tIGVuYWJsaW5nIFNS
LUlPViBhcw0KPiA+PiA+IHRoaXMgaXMgYSByZWFsIHdvcmxkIHBvc3NpYmlsaXR5LiAgV2UgbWln
aHQgbmVlZCB0byBhZGQgbW9yZSBjb2RlDQo+ID4+ID4gZWxzZXdoZXJlIGhvd2V2ZXIgdG8gcHJl
dmVudCBhIFZGIGZyb20gYmVjb21pbmcgYSBWR0EgZGV2aWNlDQo+ID4+ID4gb3V0c2lkZSBvZiBw
YXNzaW5nIGl0DQo+ID4+IHRocm91Z2ggdG8gYSBndWVzdCBWTS4NCj4gPj4NCj4gPj4gQ2FuIHlv
dSB0cnkgb3V0IFl1J3MgcGF0Y2ggYW5kIHNlZSB3aGF0IGhhcHBlbnM/ICBJZiB5b3UgZG8gdHVy
biBvbg0KPiA+PiBTUi1JT1Ygb24gdGhpcyBkZXZpY2UsIGl0IHdvdWxkIGJlIGludGVyZXN0aW5n
IHRvIHNlZSB0aGUgY29tcGxldGUgZG1lc2cgbG9nDQo+IGFuZCAibHNwY2kgLXZ2Ig0KPiA+PiBv
dXRwdXQgdG8gc2VlIHdoYXQgd2UgZG8gd2l0aCBpdC4NCj4gPj4NCj4gPj4gQmpvcm4NCj4gPg0K
PiA+IEJqb3JuLA0KPiA+IEkgYW0gcmVzdXJyZWN0aW5nIHRoaXMgdGhyZWFkIGFzIEkgaGF2ZSBi
ZWVuIGxvb2tpbmcgdGhyb3VnaCB0aGUgMy4xOCBzb3VyY2UNCj4gY29kZSBhbmQgZG9uJ3Qgc2Vl
IGFueSBzdXBwb3J0IGZvciBQQ0lfRVhQX1RZUEVfTEVHX0VORCBmb3IgU1JJT1YuICBGb3INCj4g
ZGV2aWNlcyB0aGF0IG11c3QgcmVtYWluIGRlZmluZWQgYXMgTGVnYWN5IGVuZHBvaW50cyB3ZSBz
dGlsbCBuZWVkIHRvIGJlIGFibGUgdG8NCj4gZW5hYmxlIFNSSU9WLg0KPiANCj4gTG9va2luZyBi
YWNrLCBJIHNlZSB0aGF0IFl1IFpoYW8gZGlkIGF0dGFjaCBhIHBhdGNoIGVhcmxpZXIgaW4gdGhl
IHRocmVhZC4gIEFyZSB5b3UNCj4gYXNraW5nIG1lIHRvIGFwcGx5IHRoYXQgb25lPyAgSSBkb24n
dCByZW1lbWJlciB3aHkgSSBkcm9wcGVkIGl0IGZyb20gbXkgcXVldWU7DQo+IG1heWJlIGl0IHdh
cyBiZWNhdXNlIEkgd2FzIHdhaXRpbmcgZm9yIHlvdSB0byB0ZXN0IGl0Lg0KPiANCj4gRGlkIHlv
dSB0ZXN0IGl0PyAgV2hhdCB3ZXJlIHRoZSByZXN1bHRzIChjb21wbGV0ZSBkbWVzZyBsb2cgYW5k
IGxzcGNpIC12dg0KPiBvdXRwdXQpPyAgVGhlcmUgd2FzIGFsc28gc29tZSBmb2xsb3ctb24gZGlz
Y3Vzc2lvbiB0aGF0IEkgd291bGQgbmVlZCB0bw0KPiBldmFsdWF0ZSBiZWZvcmUgYXBwbHlpbmcg
aXQuDQo+IA0KPiBCam9ybg0KDQpJIGxvc3QgdGhlIG9yaWdpbmFsIHBhdGNoIGJ1dCBpdCB3YXMg
c2ltcGxlIGVub3VnaCB0aGF0IEkgY29kZWQgaXQgbXlzZWxmIChvbmUgbGluZSBjaGFuZ2UpIGxv
Y2FsbHkuICBJIGhhdmUgYmVlbiB0ZXN0aW5nIHdpdGggaXQgZm9yIHNldmVyYWwgbW9udGhzIG5v
dyBhbmQgaGF2ZSBubyBpc3N1ZXMgd2l0aCBpdC4gIEluIGZhY3QgbXkgdGVzdGluZyByZXF1aXJl
cyBpdCB0byBiZSBwcmVzZW50IGluIG9yZGVyIHRvIHN1Y2NlZWQsIGl0IHdpbGwgZmFpbCBpZiBp
dCBpcyBub3QgcHJlc2VudC4NCg0KVGhhbmtzLA0KS2VsbHkNCj4gDQo+ID4gRG8geW91IGhhdmUg
YW55IGNvbmNlcm5zIG9uIGFkZGluZyB0aGUgbGVnYWN5IHN1cHBvcnQgdG8gc3Jpb3ZfaW5pdCgp
IGFzDQo+IHNob3duIGJlbG93Pw0KPiA+DQo+ID4gICAgICAgICBpZiAoKGRldi0+cGNpZV90eXBl
ICE9IFBDSV9FWFBfVFlQRV9SQ19FTkQpICYmDQo+ID4gICAgICAgICAgICAgKGRldi0+cGNpZV90
eXBlICE9IFBDSV9FWFBfVFlQRV9FTkRQT0lOVCkgJiYNCj4gPiAgICAgICAgICAgICAgICAgICAo
ZGV2LT5wY2llX3R5cGUgIT0gUENJX0VYUF9UWVBFX0xFR19FTkQpKQ0KPiA+ICAgICAgICAgew0K
PiA+ICAgICAgICAgICAgICAgICByZXR1cm4gLUVOT0RFVjsNCj4gPiAgICAgICAgIH0NCj4gPg0K
PiA+IFRoYW5rcywNCj4gPiBLZWxseQ0KPiAtLQ0KPiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMg
bGlzdDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtcGNpIiBpbiB0aGUgYm9keSBv
Zg0KPiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZyBNb3JlIG1ham9yZG9t
byBpbmZvIGF0DQo+IGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbA0K

Re: Architectural question regarding IOV support in Linux 3.13.4

[Bjorn Helgaas]

On Tue, Dec 02, 2014 at 09:11:26PM +0000, Zytaruk, Kelly wrote:
> > -----Original Message-----
> > From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> > owner@vger.kernel.org] On Behalf Of Bjorn Helgaas
> > Sent: Tuesday, December 02, 2014 4:02 PM
> > To: Zytaruk, Kelly
> > Cc: Yu Zhao; linux-pci@vger.kernel.org; Alex Williamson
> > Subject: Re: Architectural question regarding IOV support in Linux 3.13.4
> > 
> > On Tue, Dec 2, 2014 at 9:42 AM, Zytaruk, Kelly <Kelly.Zytaruk@amd.com>
> > wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
...

> > >> Can you try out Yu's patch and see what happens?  If you do turn on
> > >> SR-IOV on this device, it would be interesting to see the complete dmesg log
> > and "lspci -vv"
> > >> output to see what we do with it.
> > >
> > > Bjorn,
> > > I am resurrecting this thread as I have been looking through the 3.18 source
> > code and don't see any support for PCI_EXP_TYPE_LEG_END for SRIOV.  For
> > devices that must remain defined as Legacy endpoints we still need to be able to
> > enable SRIOV.
> > 
> > Looking back, I see that Yu Zhao did attach a patch earlier in the thread.  Are you
> > asking me to apply that one?  I don't remember why I dropped it from my queue;
> > maybe it was because I was waiting for you to test it.
> > 
> > Did you test it?  What were the results (complete dmesg log and lspci -vv
> > output)?  There was also some follow-on discussion that I would need to
> > evaluate before applying it.
> > 
> > Bjorn
> 
> I lost the original patch but it was simple enough that I coded it myself (one line change) locally.  I have been testing with it for several months now and have no issues with it.  In fact my testing requires it to be present in order to succeed, it will fail if it is not present.

Huh, I don't see it in the archives, either.  Ah, I see, it was a
multi-part message, which is rejected by the mailing lists.  But since I'm
a nice guy, I'll include it again below.

See https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches
for the usual procedure.

Please test the patch below (or post your patch if you prefer), collect
the info I requested above (dmesg and lspci info), and attach it to a
bugzilla so we can archive the details that motivate this change.

Bjorn



>From e81dfb3e73982ae9d74e3a8c6110a5104e36c3c0 Mon Sep 17 00:00:00 2001
From: Yu Zhao <yuzhao@google.com>
Date: Fri, 21 Feb 2014 14:46:50 -0800
Subject: [PATCH] pci: fix legacy endpoint being excluded from using SR-IOV

Neither PCIe 3.0 nor SR-IOV 1.1 spec excludes device of legacy
endpoint type from having SR-IOV capability. The legacy endpoint
together with root complex integrated and regular endpoints should
be supported by sriov_init().

Signed-off-by: Yu Zhao <yu.zhao@google.com>
---
 drivers/pci/iov.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 9dce7c5..5f98707 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -418,6 +418,7 @@ 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_LEG_END &&
 	    pci_pcie_type(dev) != PCI_EXP_TYPE_ENDPOINT)
 		return -ENODEV;
 
-- 
1.9.0.rc1.175.g0b1dcb5