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