From: "Tian, Kevin" <kevin.tian@intel.com> To: Alex Williamson <alex.williamson@redhat.com>, "Liu, Yi L" <yi.l.liu@intel.com> Cc: "eric.auger@redhat.com" <eric.auger@redhat.com>, "jacob.jun.pan@linux.intel.com" <jacob.jun.pan@linux.intel.com>, "joro@8bytes.org" <joro@8bytes.org>, "Raj, Ashok" <ashok.raj@intel.com>, "Tian, Jun J" <jun.j.tian@intel.com>, "Sun, Yi Y" <yi.y.sun@intel.com>, "jean-philippe@linaro.org" <jean-philippe@linaro.org>, "peterx@redhat.com" <peterx@redhat.com>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "Wu, Hao" <hao.wu@intel.com> Subject: RE: [PATCH v1 2/2] vfio/pci: Emulate PASID/PRI capability for VFs Date: Tue, 7 Apr 2020 04:26:23 +0000 [thread overview] Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D19D80E13D@SHSMSX104.ccr.corp.intel.com> (raw) In-Reply-To: <20200403112545.6c115ba3@w520.home> > From: Alex Williamson <alex.williamson@redhat.com> > Sent: Saturday, April 4, 2020 1:26 AM [...] > > > > + if (!pasid_cap.control_reg.paside) { > > > > + pr_debug("%s: its PF's PASID capability is not enabled\n", > > > > + dev_name(&vdev->pdev->dev)); > > > > + ret = 0; > > > > + goto out; > > > > + } > > > > > > What happens if the PF's PASID gets disabled while we're using it?? > > > > This is actually the open I highlighted in cover letter. Per the reply > > from Baolu, this seems to be an open for bare-metal all the same. > > https://lkml.org/lkml/2020/3/31/95 > > Seems that needs to get sorted out before we can expose this. Maybe > some sort of registration with the PF driver that PASID is being used > by a VF so it cannot be disabled? I guess we may do vSVA for PF first, and then adding VF vSVA later given above additional need. It's not necessarily to enable both in one step. [...] > > > > @@ -1604,6 +1901,18 @@ static int vfio_ecap_init(struct > vfio_pci_device *vdev) > > > > if (!ecaps) > > > > *(u32 *)&vdev->vconfig[PCI_CFG_SPACE_SIZE] = 0; > > > > > > > > +#ifdef CONFIG_PCI_ATS > > > > + if (pdev->is_virtfn) { > > > > + struct pci_dev *physfn = pdev->physfn; > > > > + > > > > + ret = vfio_pci_add_emulated_cap_for_vf(vdev, > > > > + physfn, epos_max, prev); > > > > + if (ret) > > > > + pr_info("%s, failed to add special caps for VF %s\n", > > > > + __func__, dev_name(&vdev->pdev->dev)); > > > > + } > > > > +#endif > > > > > > I can only imagine that we should place the caps at the same location > > > they exist on the PF, we don't know what hidden registers might be > > > hiding in config space. Is there vendor guarantee that hidden registers will locate at the same offset between PF and VF config space? > > > > but we are not sure whether the same location is available on VF. In > > this patch, it actually places the emulated cap physically behind the > > cap which lays farthest (its offset is largest) within VF's config space > > as the PCIe caps are linked in a chain. > > But, as we've found on Broadcom NICs (iirc), hardware developers have a > nasty habit of hiding random registers in PCI config space, outside of > defined capabilities. I feel like IGD might even do this too, is that > true? So I don't think we can guarantee that just because a section of > config space isn't part of a defined capability that its unused. It > only means that it's unused by common code, but it might have device > specific purposes. So of the PCIe spec indicates that VFs cannot > include these capabilities and virtialization software needs to > emulate them, we need somewhere safe to place them in config space, and > simply placing them off the end of known capabilities doesn't give me > any confidence. Also, hardware has no requirement to make compact use > of extended config space. The first capability must be at 0x100, the > very next capability could consume all the way to the last byte of the > 4K extended range, and the next link in the chain could be somewhere in > the middle. Thanks, > Then what would be a viable option? Vendor nasty habit implies no standard, thus I don't see how VFIO can find a safe location by itself. Also curious how those hidden registers are identified by VFIO and employed with proper r/w policy today. If sort of quirks are used, then could such quirk way be extended to also carry the information about vendor specific safe location? When no such quirk info is provided (the majority case), VFIO then finds out a free location to carry the new cap. Thanks Kevin
WARNING: multiple messages have this Message-ID (diff)
From: "Tian, Kevin" <kevin.tian@intel.com> To: Alex Williamson <alex.williamson@redhat.com>, "Liu, Yi L" <yi.l.liu@intel.com> Cc: "jean-philippe@linaro.org" <jean-philippe@linaro.org>, "Raj, Ashok" <ashok.raj@intel.com>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "Sun, Yi Y" <yi.y.sun@intel.com>, "Tian, Jun J" <jun.j.tian@intel.com>, "Wu, Hao" <hao.wu@intel.com> Subject: RE: [PATCH v1 2/2] vfio/pci: Emulate PASID/PRI capability for VFs Date: Tue, 7 Apr 2020 04:26:23 +0000 [thread overview] Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D19D80E13D@SHSMSX104.ccr.corp.intel.com> (raw) In-Reply-To: <20200403112545.6c115ba3@w520.home> > From: Alex Williamson <alex.williamson@redhat.com> > Sent: Saturday, April 4, 2020 1:26 AM [...] > > > > + if (!pasid_cap.control_reg.paside) { > > > > + pr_debug("%s: its PF's PASID capability is not enabled\n", > > > > + dev_name(&vdev->pdev->dev)); > > > > + ret = 0; > > > > + goto out; > > > > + } > > > > > > What happens if the PF's PASID gets disabled while we're using it?? > > > > This is actually the open I highlighted in cover letter. Per the reply > > from Baolu, this seems to be an open for bare-metal all the same. > > https://lkml.org/lkml/2020/3/31/95 > > Seems that needs to get sorted out before we can expose this. Maybe > some sort of registration with the PF driver that PASID is being used > by a VF so it cannot be disabled? I guess we may do vSVA for PF first, and then adding VF vSVA later given above additional need. It's not necessarily to enable both in one step. [...] > > > > @@ -1604,6 +1901,18 @@ static int vfio_ecap_init(struct > vfio_pci_device *vdev) > > > > if (!ecaps) > > > > *(u32 *)&vdev->vconfig[PCI_CFG_SPACE_SIZE] = 0; > > > > > > > > +#ifdef CONFIG_PCI_ATS > > > > + if (pdev->is_virtfn) { > > > > + struct pci_dev *physfn = pdev->physfn; > > > > + > > > > + ret = vfio_pci_add_emulated_cap_for_vf(vdev, > > > > + physfn, epos_max, prev); > > > > + if (ret) > > > > + pr_info("%s, failed to add special caps for VF %s\n", > > > > + __func__, dev_name(&vdev->pdev->dev)); > > > > + } > > > > +#endif > > > > > > I can only imagine that we should place the caps at the same location > > > they exist on the PF, we don't know what hidden registers might be > > > hiding in config space. Is there vendor guarantee that hidden registers will locate at the same offset between PF and VF config space? > > > > but we are not sure whether the same location is available on VF. In > > this patch, it actually places the emulated cap physically behind the > > cap which lays farthest (its offset is largest) within VF's config space > > as the PCIe caps are linked in a chain. > > But, as we've found on Broadcom NICs (iirc), hardware developers have a > nasty habit of hiding random registers in PCI config space, outside of > defined capabilities. I feel like IGD might even do this too, is that > true? So I don't think we can guarantee that just because a section of > config space isn't part of a defined capability that its unused. It > only means that it's unused by common code, but it might have device > specific purposes. So of the PCIe spec indicates that VFs cannot > include these capabilities and virtialization software needs to > emulate them, we need somewhere safe to place them in config space, and > simply placing them off the end of known capabilities doesn't give me > any confidence. Also, hardware has no requirement to make compact use > of extended config space. The first capability must be at 0x100, the > very next capability could consume all the way to the last byte of the > 4K extended range, and the next link in the chain could be somewhere in > the middle. Thanks, > Then what would be a viable option? Vendor nasty habit implies no standard, thus I don't see how VFIO can find a safe location by itself. Also curious how those hidden registers are identified by VFIO and employed with proper r/w policy today. If sort of quirks are used, then could such quirk way be extended to also carry the information about vendor specific safe location? When no such quirk info is provided (the majority case), VFIO then finds out a free location to carry the new cap. Thanks Kevin _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-04-07 4:26 UTC|newest] Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-22 12:33 [PATCH v1 0/2] vfio/pci: expose device's PASID capability to VMs Liu, Yi L 2020-03-22 12:33 ` Liu, Yi L 2020-03-22 12:33 ` [PATCH v1 1/2] vfio/pci: Expose PCIe PASID capability to guest Liu, Yi L 2020-03-22 12:33 ` Liu, Yi L 2020-03-31 6:39 ` Tian, Kevin 2020-03-31 6:39 ` Tian, Kevin 2020-03-31 6:42 ` Liu, Yi L 2020-03-31 6:42 ` Liu, Yi L 2020-03-22 12:33 ` [PATCH v1 2/2] vfio/pci: Emulate PASID/PRI capability for VFs Liu, Yi L 2020-03-22 12:33 ` Liu, Yi L 2020-04-02 22:59 ` Alex Williamson 2020-04-02 22:59 ` Alex Williamson 2020-04-03 7:53 ` Liu, Yi L 2020-04-03 7:53 ` Liu, Yi L 2020-04-03 17:25 ` Alex Williamson 2020-04-03 17:25 ` Alex Williamson 2020-04-07 4:26 ` Tian, Kevin [this message] 2020-04-07 4:26 ` Tian, Kevin 2020-04-07 15:58 ` Alex Williamson 2020-04-07 15:58 ` Alex Williamson 2020-04-08 0:27 ` Tian, Kevin 2020-04-08 0:27 ` Tian, Kevin 2020-04-08 4:00 ` Raj, Ashok 2020-04-08 4:00 ` Raj, Ashok 2020-04-08 16:19 ` Alex Williamson 2020-04-08 16:19 ` Alex Williamson 2020-04-08 16:33 ` Raj, Ashok 2020-04-08 16:33 ` Raj, Ashok 2020-04-09 7:35 ` Jean-Philippe Brucker 2020-04-09 7:35 ` Jean-Philippe Brucker 2020-04-13 19:44 ` Alex Williamson 2020-04-13 19:44 ` Alex Williamson 2020-04-13 3:10 ` Raj, Ashok 2020-04-13 3:10 ` Raj, Ashok 2020-04-13 3:29 ` Raj, Ashok 2020-04-13 3:29 ` Raj, Ashok 2020-04-13 19:10 ` Alex Williamson 2020-04-13 19:10 ` Alex Williamson 2020-04-13 7:54 ` Tian, Kevin 2020-04-13 7:54 ` Tian, Kevin 2020-04-13 8:05 ` Tian, Kevin 2020-04-13 8:05 ` Tian, Kevin 2020-04-13 19:21 ` Alex Williamson 2020-04-13 19:21 ` Alex Williamson 2020-04-14 2:40 ` Tian, Kevin 2020-04-14 2:40 ` Tian, Kevin 2020-04-14 3:28 ` Alex Williamson 2020-04-14 3:28 ` Alex Williamson 2020-04-14 3:42 ` Tian, Kevin 2020-04-14 3:42 ` Tian, Kevin 2020-04-14 15:24 ` Alex Williamson 2020-04-14 15:24 ` Alex Williamson 2020-04-14 23:57 ` Tian, Kevin 2020-04-14 23:57 ` Tian, Kevin 2020-04-15 0:36 ` Alex Williamson 2020-04-15 0:36 ` Alex Williamson 2020-04-15 1:01 ` Tian, Kevin 2020-04-15 1:01 ` Tian, Kevin 2020-04-03 11:42 ` Liu, Yi L 2020-04-03 11:42 ` Liu, Yi L 2020-03-31 6:35 ` [PATCH v1 0/2] vfio/pci: expose device's PASID capability to VMs Tian, Kevin 2020-03-31 6:35 ` Tian, Kevin 2020-03-31 7:08 ` Lu, Baolu 2020-03-31 7:08 ` Lu, Baolu 2020-04-16 22:12 ` Yan Zhao 2020-04-16 22:12 ` Yan Zhao 2020-04-16 22:33 ` Raj, Ashok 2020-04-16 22:33 ` Raj, Ashok 2020-04-17 1:13 ` Yan Zhao 2020-04-17 1:13 ` Yan Zhao
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=AADFC41AFE54684AB9EE6CBC0274A5D19D80E13D@SHSMSX104.ccr.corp.intel.com \ --to=kevin.tian@intel.com \ --cc=alex.williamson@redhat.com \ --cc=ashok.raj@intel.com \ --cc=eric.auger@redhat.com \ --cc=hao.wu@intel.com \ --cc=iommu@lists.linux-foundation.org \ --cc=jacob.jun.pan@linux.intel.com \ --cc=jean-philippe@linaro.org \ --cc=joro@8bytes.org \ --cc=jun.j.tian@intel.com \ --cc=kvm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=peterx@redhat.com \ --cc=yi.l.liu@intel.com \ --cc=yi.y.sun@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.