2017-08-28 13:39:52

by Filippo Sironi

[permalink] [raw]
Subject: [PATCH 1/2] pci: Cache the VF device ID in the SR-IOV structure

... and use it instead of reading it over and over from the PF config
space capability.

Signed-off-by: Filippo Sironi <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/pci/iov.c | 5 +++--
drivers/pci/pci.h | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 120485d6f352..e8f7eafaba6a 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -134,7 +134,7 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id, int reset)

virtfn->devfn = pci_iov_virtfn_devfn(dev, id);
virtfn->vendor = dev->vendor;
- pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_DID, &virtfn->device);
+ virtfn->device = iov->vf_did;
rc = pci_setup_device(virtfn);
if (rc)
goto failed0;
@@ -448,6 +448,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
iov->nres = nres;
iov->ctrl = ctrl;
iov->total_VFs = total;
+ pci_read_config_word(dev, pos + PCI_SRIOV_VF_DID, &iov->vf_did);
iov->pgsz = pgsz;
iov->self = dev;
iov->drivers_autoprobe = true;
@@ -723,7 +724,7 @@ int pci_vfs_assigned(struct pci_dev *dev)
* determine the device ID for the VFs, the vendor ID will be the
* same as the PF so there is no need to check for that one
*/
- pci_read_config_word(dev, dev->sriov->pos + PCI_SRIOV_VF_DID, &dev_id);
+ dev_id = dev->sriov->vf_did;

/* loop through all the VFs to see if we own any that are assigned */
vfdev = pci_get_device(dev->vendor, dev_id, NULL);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 22e061738c6f..a7270e11e1ef 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -262,6 +262,7 @@ struct pci_sriov {
u16 num_VFs; /* number of VFs available */
u16 offset; /* first VF Routing ID offset */
u16 stride; /* following VF stride */
+ u16 vf_did; /* VF device ID */
u32 pgsz; /* page size for BAR alignment */
u8 link; /* Function Dependency Link */
u8 max_VF_buses; /* max buses consumed by VFs */
--
2.7.4


2017-08-28 13:39:41

by Filippo Sironi

[permalink] [raw]
Subject: [PATCH 2/2] pci: Expose offset, stride, and VF device ID via sysfs

... to make it easier for userspace applications consumption.

Signed-off-by: Filippo Sironi <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/pci/pci-sysfs.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 2f3780b50723..f920afe7cff3 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -648,6 +648,33 @@ static ssize_t sriov_numvfs_store(struct device *dev,
return count;
}

+static ssize_t sriov_offset_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ return sprintf(buf, "%u\n", pdev->sriov->offset);
+}
+
+static ssize_t sriov_stride_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ return sprintf(buf, "%u\n", pdev->sriov->stride);
+}
+
+static ssize_t sriov_vf_did_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ return sprintf(buf, "%x\n", pdev->sriov->vf_did);
+}
+
static ssize_t sriov_drivers_autoprobe_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -676,6 +703,9 @@ static struct device_attribute sriov_totalvfs_attr = __ATTR_RO(sriov_totalvfs);
static struct device_attribute sriov_numvfs_attr =
__ATTR(sriov_numvfs, (S_IRUGO|S_IWUSR|S_IWGRP),
sriov_numvfs_show, sriov_numvfs_store);
+static struct device_attribute sriov_offset_attr = __ATTR_RO(sriov_offset);
+static struct device_attribute sriov_stride_attr = __ATTR_RO(sriov_stride);
+static struct device_attribute sriov_vf_did_attr = __ATTR_RO(sriov_vf_did);
static struct device_attribute sriov_drivers_autoprobe_attr =
__ATTR(sriov_drivers_autoprobe, (S_IRUGO|S_IWUSR|S_IWGRP),
sriov_drivers_autoprobe_show, sriov_drivers_autoprobe_store);
@@ -1744,6 +1774,9 @@ static struct attribute_group pci_dev_hp_attr_group = {
static struct attribute *sriov_dev_attrs[] = {
&sriov_totalvfs_attr.attr,
&sriov_numvfs_attr.attr,
+ &sriov_offset_attr.attr,
+ &sriov_stride_attr.attr,
+ &sriov_vf_did_attr.attr,
&sriov_drivers_autoprobe_attr.attr,
NULL,
};
--
2.7.4

2017-09-25 18:55:28

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci: Expose offset, stride, and VF device ID via sysfs

Hi Filippo,

On Mon, Aug 28, 2017 at 03:38:50PM +0200, Filippo Sironi wrote:
> +static ssize_t sriov_vf_did_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> +
> + return sprintf(buf, "%x\n", pdev->sriov->vf_did);
> +}

What does the vf_did part look like in sysfs? Do we have a directory with
both "device" and "vf_did" in it? If so, why do we have both and do we
need both? Could we put the vf_did in the "device" file?

> static ssize_t sriov_drivers_autoprobe_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -676,6 +703,9 @@ static struct device_attribute sriov_totalvfs_attr = __ATTR_RO(sriov_totalvfs);
> static struct device_attribute sriov_numvfs_attr =
> __ATTR(sriov_numvfs, (S_IRUGO|S_IWUSR|S_IWGRP),
> sriov_numvfs_show, sriov_numvfs_store);
> +static struct device_attribute sriov_offset_attr = __ATTR_RO(sriov_offset);
> +static struct device_attribute sriov_stride_attr = __ATTR_RO(sriov_stride);
> +static struct device_attribute sriov_vf_did_attr = __ATTR_RO(sriov_vf_did);
> static struct device_attribute sriov_drivers_autoprobe_attr =
> __ATTR(sriov_drivers_autoprobe, (S_IRUGO|S_IWUSR|S_IWGRP),
> sriov_drivers_autoprobe_show, sriov_drivers_autoprobe_store);
> @@ -1744,6 +1774,9 @@ static struct attribute_group pci_dev_hp_attr_group = {
> static struct attribute *sriov_dev_attrs[] = {
> &sriov_totalvfs_attr.attr,
> &sriov_numvfs_attr.attr,
> + &sriov_offset_attr.attr,
> + &sriov_stride_attr.attr,
> + &sriov_vf_did_attr.attr,
> &sriov_drivers_autoprobe_attr.attr,
> NULL,
> };
> --
> 2.7.4
>

2017-09-29 07:53:43

by Filippo Sironi

[permalink] [raw]
Subject: Re: [PATCH 2/2] pci: Expose offset, stride, and VF device ID via sysfs


Hi Bjorn,

> On 25. Sep 2017, at 20:55, Bjorn Helgaas <[email protected]> wrote:
>
> Hi Filippo,
>
> On Mon, Aug 28, 2017 at 03:38:50PM +0200, Filippo Sironi wrote:
>> +static ssize_t sriov_vf_did_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct pci_dev *pdev = to_pci_dev(dev);
>> +
>> + return sprintf(buf, "%x\n", pdev->sriov->vf_did);
>> +}
>
> What does the vf_did part look like in sysfs? Do we have a directory with
> both "device" and "vf_did" in it? If so, why do we have both and do we
> need both? Could we put the vf_did in the "device" file?

On my machine:

/sys/bus/pci/devices/0000:03:00.0# ls -l # this is the PF
total 0
-rw-r--r-- 1 root root 4096 Sep 28 19:41 broken_parity_status
-r--r--r-- 1 root root 4096 Sep 28 19:41 class
-rw-r--r-- 1 root root 4096 Sep 28 19:41 config
-r--r--r-- 1 root root 4096 Sep 28 19:41 consistent_dma_mask_bits
-rw-r--r-- 1 root root 4096 Sep 28 19:41 d3cold_allowed
-r--r--r-- 1 root root 4096 Sep 28 19:41 device
-r--r--r-- 1 root root 4096 Sep 28 19:41 dma_mask_bits
lrwxrwxrwx 1 root root 0 Sep 28 19:41 driver -> ../../../../bus/pci/drivers/igb
-rw-r--r-- 1 root root 4096 Sep 28 19:41 driver_override
-rw-r--r-- 1 root root 4096 Sep 28 19:41 enable
lrwxrwxrwx 1 root root 0 Sep 28 19:41 firmware_node -> ../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:4b/device:4c
-r--r--r-- 1 root root 4096 Sep 28 19:41 irq
-r--r--r-- 1 root root 4096 Sep 28 19:41 local_cpulist
-r--r--r-- 1 root root 4096 Sep 28 19:41 local_cpus
-r--r--r-- 1 root root 4096 Sep 28 19:41 modalias
-rw-r--r-- 1 root root 4096 Sep 28 19:41 msi_bus
drwxr-xr-x 2 root root 0 Sep 29 09:44 msi_irqs
drwxr-xr-x 3 root root 0 Sep 28 19:41 net
-rw-r--r-- 1 root root 4096 Sep 28 19:41 numa_node
-r--r--r-- 1 root root 4096 Sep 28 19:41 offset # this is new
drwxr-xr-x 2 root root 0 Sep 28 19:41 power
drwxr-xr-x 3 root root 0 Sep 28 19:41 ptp
--w--w---- 1 root root 4096 Sep 28 19:41 remove
--w--w---- 1 root root 4096 Sep 28 19:41 rescan
--w------- 1 root root 4096 Sep 28 19:41 reset
-r--r--r-- 1 root root 4096 Sep 28 19:41 resource
-rw------- 1 root root 131072 Sep 28 19:41 resource0
-rw------- 1 root root 4194304 Sep 28 19:41 resource1
-rw------- 1 root root 32 Sep 28 19:41 resource2
-rw------- 1 root root 16384 Sep 28 19:41 resource3
-r--r--r-- 1 root root 4096 Sep 28 19:41 revision
-rw-rw-r-- 1 root root 4096 Sep 29 09:44 sriov_numvfs
-r--r--r-- 1 root root 4096 Sep 28 19:41 sriov_totalvfs
-r--r--r-- 1 root root 4096 Sep 28 19:41 stride # this is new
lrwxrwxrwx 1 root root 0 Sep 28 19:41 subsystem -> ../../../../bus/pci
-r--r--r-- 1 root root 4096 Sep 28 19:41 subsystem_device
-r--r--r-- 1 root root 4096 Sep 28 19:41 subsystem_vendor
-rw-r--r-- 1 root root 4096 Sep 28 19:41 uevent
-r--r--r-- 1 root root 4096 Sep 28 19:41 vendor
-r--r--r-- 1 root root 4096 Sep 28 19:41 vf_did # this is new
lrwxrwxrwx 1 root root 0 Sep 29 09:44 virtfn0 -> ../0000:03:10.0

nothing changes on for VFs.
Then:

/sys/bus/pci/devices/0000:03:00.0# cat device
0x10c9

/sys/bus/pci/devices/0000:03:00.0# cat vf_did
0x10ca

Putting the VF device ID in the PF device file would be a change of
that we expose to userspace. Something might break.

vf_did provides a easy way to retrieve the VF device ID without reading
the PF config (looking up the SR-IOV capability and reading it) or without
enabling SR-IOV to read for example virtfn0/device.

Similar considerations (ease of access) apply to offset and stride.


>> static ssize_t sriov_drivers_autoprobe_show(struct device *dev,
>> struct device_attribute *attr,
>> char *buf)
>> @@ -676,6 +703,9 @@ static struct device_attribute sriov_totalvfs_attr = __ATTR_RO(sriov_totalvfs);
>> static struct device_attribute sriov_numvfs_attr =
>> __ATTR(sriov_numvfs, (S_IRUGO|S_IWUSR|S_IWGRP),
>> sriov_numvfs_show, sriov_numvfs_store);
>> +static struct device_attribute sriov_offset_attr = __ATTR_RO(sriov_offset);
>> +static struct device_attribute sriov_stride_attr = __ATTR_RO(sriov_stride);
>> +static struct device_attribute sriov_vf_did_attr = __ATTR_RO(sriov_vf_did);
>> static struct device_attribute sriov_drivers_autoprobe_attr =
>> __ATTR(sriov_drivers_autoprobe, (S_IRUGO|S_IWUSR|S_IWGRP),
>> sriov_drivers_autoprobe_show, sriov_drivers_autoprobe_store);
>> @@ -1744,6 +1774,9 @@ static struct attribute_group pci_dev_hp_attr_group = {
>> static struct attribute *sriov_dev_attrs[] = {
>> &sriov_totalvfs_attr.attr,
>> &sriov_numvfs_attr.attr,
>> + &sriov_offset_attr.attr,
>> + &sriov_stride_attr.attr,
>> + &sriov_vf_did_attr.attr,
>> &sriov_drivers_autoprobe_attr.attr,
>> NULL,
>> };
>> --
>> 2.7.4
>>
>

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B