| Message ID | 1419927707-14462-1-git-send-email-ethan.zhao@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hi, Ethan Zhao wrote: > This patch exports assignment flag of PCI device via sysfs, so user > space application could know if PCI device was assigned to guest OS > or not via interface as following from viewpoint of host device > management: > > PCI device was assigned to guest OS > $cat /sys/devices/pci0000:00/0000:00:1f.3/assigned > 1 > PCI device isn't assigned to guest OS > $cat /sys/devices/pci0000:00/0000:00:1f.3/assigned > 0 For such a sysfs API (and similar cases in general!), do we actually want this relatively simple interface of "assigned" false/true? Or should it rather be something like an "ownership" attribute which could then indicate interface-fixed(!) string values such as "system"/"vm_guest"/FUTURE_EXTENSION_POSSIBILITY? IOW, "assigned" is quite a bit less flexible (and quite possibly even less usability-suitable from a naming POV - "assigned"...??) but of course simpler (which may or may not pose an advantage from an interface versioning/evolution POV). To further clarify things, this "assigned" interface as-is can merely tell apart the "device is occupied" vs. "device is not occupied" cases yet it could easily provide exact ownership details instead. And I'm also wondering whether or not it's good to have it directly/symmetrically named "assigned" due to it currently(!!) being based internally on an equally-named pci_is_dev_assigned() API. I should probably read up on sysfs item design hints/guidelines which I'd trust to be accurately explained in a Documentation/ file ;) Hmm, seems Documentation/sysfs-rules.txt has only one closing remark which seems relevant: ==== Userspace applications can, however, expect the format and contents of the attribute files to remain consistent in the absence of a version attribute change in the context of a given attribute. ==== This would be fulfilled by both the current and my suggested variant of this interface, AFAICS. Thanks, Andreas Mohr -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
----- Original Message ----- > This patch exports assignment flag of PCI device via sysfs, so user > space application could know if PCI device was assigned to guest OS > or not via interface as following from viewpoint of host device > management: > > PCI device was assigned to guest OS > $cat /sys/devices/pci0000:00/0000:00:1f.3/assigned > 1 > > PCI device isn't assigned to guest OS > $cat /sys/devices/pci0000:00/0000:00:1f.3/assigned > 0 > > Compiled and tested with v3.18.1 > > Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com> > --- > drivers/pci/pci-sysfs.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c > index 2c6643f..0ac3780 100644 > --- a/drivers/pci/pci-sysfs.c > +++ b/drivers/pci/pci-sysfs.c > @@ -547,6 +547,15 @@ static ssize_t driver_override_show(struct device *dev, > } > static DEVICE_ATTR_RW(driver_override); > > +static ssize_t assigned_show(struct device *dev, struct device_attribute > *attr, > + char *buf) > +{ > + struct pci_dev *pdev = to_pci_dev(dev); > + > + return sprintf(buf, "%d\n", pci_is_dev_assigned(pdev) ? 1 : 0); > +} > +static DEVICE_ATTR_RO(assigned); > + > static struct attribute *pci_dev_attrs[] = { > &dev_attr_resource.attr, > &dev_attr_vendor.attr, > @@ -573,6 +582,7 @@ static struct attribute *pci_dev_attrs[] = { > &dev_attr_devspec.attr, > #endif > &dev_attr_driver_override.attr, > + &dev_attr_assigned.attr, > NULL, > }; > NAK. The assigned attribute of a device is already in contention, it's racy and not universally used. If we can't even make it a consistent and reliable internal interface, we certainly shouldn't be exposing it to userspace. Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index 2c6643f..0ac3780 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -547,6 +547,15 @@ static ssize_t driver_override_show(struct device *dev, } static DEVICE_ATTR_RW(driver_override); +static ssize_t assigned_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct pci_dev *pdev = to_pci_dev(dev); + + return sprintf(buf, "%d\n", pci_is_dev_assigned(pdev) ? 1 : 0); +} +static DEVICE_ATTR_RO(assigned); + static struct attribute *pci_dev_attrs[] = { &dev_attr_resource.attr, &dev_attr_vendor.attr, @@ -573,6 +582,7 @@ static struct attribute *pci_dev_attrs[] = { &dev_attr_devspec.attr, #endif &dev_attr_driver_override.attr, + &dev_attr_assigned.attr, NULL, };
This patch exports assignment flag of PCI device via sysfs, so user space application could know if PCI device was assigned to guest OS or not via interface as following from viewpoint of host device management: PCI device was assigned to guest OS $cat /sys/devices/pci0000:00/0000:00:1f.3/assigned 1 PCI device isn't assigned to guest OS $cat /sys/devices/pci0000:00/0000:00:1f.3/assigned 0 Compiled and tested with v3.18.1 Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com> --- drivers/pci/pci-sysfs.c | 10 ++++++++++ 1 file changed, 10 insertions(+)