> -----Original Message----- > From: linux-pci-owner@xxxxxxxxxxxxxxx [mailto:linux-pci- > owner@xxxxxxxxxxxxxxx] On Behalf Of Bjorn Helgaas > Sent: Tuesday, December 02, 2014 4:02 PM > To: Zytaruk, Kelly > Cc: Yu Zhao; linux-pci@xxxxxxxxxxxxxxx; 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@xxxxxxx> > wrote: > > > > > >> -----Original Message----- > >> From: Bjorn Helgaas [mailto:bhelgaas@xxxxxxxxxx] > >> Sent: Monday, February 24, 2014 3:52 PM > >> To: Zytaruk, Kelly > >> Cc: Yu Zhao; linux-pci@xxxxxxxxxxxxxxx; 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@xxxxxxx> > >> wrote: > >> > > >> >>On Fri, Feb 21, 2014 at 2:03 PM, Alex Williamson > >> <alex.williamson@xxxxxxxxxx> 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@xxxxxxx> > >> 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 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. Thanks, Kelly > > > 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 > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of > a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at > http://vger.kernel.org/majordomo-info.html ��.n��������+%������w��{.n�����{���"�)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
