Re: [PATCH V7 8/9] PCI/IOV: Add 10-Bit Tag sysfs files for VF devices
From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2021-08-05 00:05:29
Also in:
linux-media, linux-pci
On Wed, Aug 04, 2021 at 09:47:07PM +0800, Dongdong Liu wrote:
PCIe spec 5.0 r1.0 section 2.2.6.2 says that if an Endpoint supports sending Requests to other Endpoints (as opposed to host memory), the Endpoint must not send 10-Bit Tag Requests to another given Endpoint unless an implementation-specific mechanism determines that the Endpoint supports 10-Bit Tag Completer capability. Add sriov_vf_10bit_tag file to query the status of VF 10-Bit Tag Requester Enable. Add sriov_vf_10bit_tag_ctl file to disable the VF 10-Bit Tag Requester. The typical use case is for p2pdma when the peer device does not support 10-BIT Tag Completer.
Fix the usual spec quoting issue. Or maybe this is not actually quoted but is missing blank lines between paragraphs. s/10-BIT/10-Bit/
quoted hunk ↗ jump to hunk
Signed-off-by: Dongdong Liu <redacted> --- Documentation/ABI/testing/sysfs-bus-pci | 20 +++++++++++++ drivers/pci/iov.c | 50 +++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+)diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci index 0e0c97d..8fdbfae 100644 --- a/Documentation/ABI/testing/sysfs-bus-pci +++ b/Documentation/ABI/testing/sysfs-bus-pci@@ -421,3 +421,23 @@ Description: to disable 10-Bit Tag Requester when the driver does not bind the deivce. The typical use case is for p2pdma when the peer device does not support 10-BIT Tag Completer. + +What: /sys/bus/pci/devices/.../sriov_vf_10bit_tag +Date: August 2021 +Contact: Dongdong Liu <liudongdong3@huawei.com> +Description: + This file is associated with a SR-IOV physical function (PF). + It is visible when the device has VF 10-Bit Tag Requester + Supported. It contains the status of VF 10-Bit Tag Requester + Enable. The file is only readable.
s/only readable/read-only/
+What: /sys/bus/pci/devices/.../sriov_vf_10bit_tag_ctl
Why does this file have "_ctl" on the end when the one in patch 7/9 does not?
+Date: August 2021 +Contact: Dongdong Liu [off-list ref] +Description: + This file is associated with a SR-IOV virtual function (VF). + It is visible when the device has VF 10-Bit Tag Requester + Supported. It only allows to write 0 to disable VF 10-Bit + Tag Requester. The file is only writeable when the vf driver + does not bind to a dirver. The typical use case is for p2pdma + when the peer device does not support 10-BIT Tag Completer.
s/vf/VF/ s/dirver/driver/ s/10-BIT/10-Bit/ "when the vr driver does not bind to a driver"? Not quite right. Must be a "device" in there somewhere. So IIUC this file is associated with a VF, but the bit it writes is actually in the *PF*? So writing 0 to any VF's file disables 10-bit tags for *all* VFs? That's worth mentioning here.
quoted hunk ↗ jump to hunk
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index 0d0bed1..04c1298 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c@@ -220,10 +220,38 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev, static DEVICE_ATTR_WO(sriov_vf_msix_count); #endif +static ssize_t sriov_vf_10bit_tag_ctl_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct pci_dev *vf_dev = to_pci_dev(dev); + struct pci_dev *pdev = pci_physfn(vf_dev); + struct pci_sriov *iov; + bool enable; + + if (kstrtobool(buf, &enable) < 0) + return -EINVAL; + + if (enable != false) + return -EINVAL;
Is this "if (enable)" again?
+ if (vf_dev->driver) + return -EBUSY; + + iov = pdev->sriov; + iov->ctrl &= ~PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN; + pci_write_config_word(pdev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); + pci_info(pdev, "disabled SRIOV 10-Bit Tag Requester\n");
s/SRIOV/SR-IOV/ to match spec and other usages.
quoted hunk ↗ jump to hunk
+ + return count; +} +static DEVICE_ATTR_WO(sriov_vf_10bit_tag_ctl); + static struct attribute *sriov_vf_dev_attrs[] = { #ifdef CONFIG_PCI_MSI &dev_attr_sriov_vf_msix_count.attr, #endif + &dev_attr_sriov_vf_10bit_tag_ctl.attr, NULL, };@@ -236,6 +264,11 @@ static umode_t sriov_vf_attrs_are_visible(struct kobject *kobj, if (!pdev->is_virtfn) return 0; + pdev = pci_physfn(pdev); + if ((a == &dev_attr_sriov_vf_10bit_tag_ctl.attr) && + !(pdev->sriov->cap & PCI_SRIOV_CAP_VF_10BIT_TAG_REQ)) + return 0; + return a->mode; }@@ -487,12 +520,23 @@ static ssize_t sriov_drivers_autoprobe_store(struct device *dev, return count; } +static ssize_t sriov_vf_10bit_tag_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct pci_dev *pdev = to_pci_dev(dev); + + return sysfs_emit(buf, "%u\n", + !!(pdev->sriov->ctrl & PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN)); +} + static DEVICE_ATTR_RO(sriov_totalvfs); static DEVICE_ATTR_RW(sriov_numvfs); static DEVICE_ATTR_RO(sriov_offset); static DEVICE_ATTR_RO(sriov_stride); static DEVICE_ATTR_RO(sriov_vf_device); static DEVICE_ATTR_RW(sriov_drivers_autoprobe); +static DEVICE_ATTR_RO(sriov_vf_10bit_tag); static struct attribute *sriov_pf_dev_attrs[] = { &dev_attr_sriov_totalvfs.attr,@@ -501,6 +545,7 @@ static struct attribute *sriov_pf_dev_attrs[] = { &dev_attr_sriov_stride.attr, &dev_attr_sriov_vf_device.attr, &dev_attr_sriov_drivers_autoprobe.attr, + &dev_attr_sriov_vf_10bit_tag.attr, #ifdef CONFIG_PCI_MSI &dev_attr_sriov_vf_total_msix.attr, #endif@@ -511,10 +556,15 @@ static umode_t sriov_pf_attrs_are_visible(struct kobject *kobj, struct attribute *a, int n) { struct device *dev = kobj_to_dev(kobj); + struct pci_dev *pdev = to_pci_dev(dev); if (!dev_is_pf(dev)) return 0; + if ((a == &dev_attr_sriov_vf_10bit_tag.attr) && + !(pdev->sriov->cap & PCI_SRIOV_CAP_VF_10BIT_TAG_REQ)) + return 0; + return a->mode; }-- 2.7.4