[libvirt] [PATCH 1/5] Add pci utility functions
Daniel P. Berrange
berrange at redhat.com
Wed Feb 25 15:54:53 UTC 2009
On Tue, Feb 24, 2009 at 10:21:47PM +0000, Mark McLoughlin wrote:
> Add implementations of dettach, reattach and reset for
> PCI devices.
>
> Background to this code can be found here:
>
> http://marc.info/?l=kvm&m=123454366317045
>
> Some notes:
>
> * pci-stub was first introduced in 2.6.29; if it's
> not available, then dettach can only unbind the
> device from the host driver
>
> * remove_id is queued up for 2.6.30; in it's absence
> reattach cannot re-bind the device to the original
> driver
>
> * If the device supports Function Level Reset, we
> just don't bother doing the reset - KVM will do
> this before and after a guest runs
>
> * Currently, if a reset would affect another device
> on the same bus, or another function on the same
> multi-function device, then we just refuse to do
> the reset. In future, we could allow the reset
> as long as it doesn't affect a device in use by
> the host or assigned to another guest
>
> * The device reset code is similar in concept to
> some of Xen's code
>
> Signed-off-by: Mark McLoughlin <markmc at redhat.com>
I can't claim to understand all the gory PCI code in here,
but a few API level suggestions.
Also, would it be worth (or possible) using libpciaccess.so
for this instead of poking Linux sysfs files directly ?
> +static int
> +pciRead(pciDevice *dev, unsigned pos, uint8_t *buf, unsigned buflen)
> +{
> + memset(buf, 0, buflen);
> +
> + if (pciOpenConfig(dev) < 0)
> + return -1;
> +
> + if (pread(dev->fd, buf, buflen, pos) < 0) {
> + char ebuf[1024];
> + VIR_WARN(_("Failed to read from '%s' : %s"), dev->path,
> + virStrerror(errno, ebuf, sizeof(ebuf)));
> + return -1;
> + }
> + return 0;
> +}
I think this needs to check for a possible short read too.
Perhaps better off doing a seek and then saferead(), or
defining a safepread() function.
> +static int
> +pciWrite(pciDevice *dev, unsigned pos, uint8_t *buf, unsigned buflen)
> +{
> + if (pciOpenConfig(dev) < 0)
> + return -1;
> +
> + if (pwrite(dev->fd, buf, buflen, pos) < 0) {
> + char ebuf[1024];
> + VIR_WARN(_("Failed to write to '%s' : %s"), dev->path,
> + virStrerror(errno, ebuf, sizeof(ebuf)));
> + return -1;
> + }
> + return 0;
> +}
Likewise here needs to validate actual num of bytes written
> +int
> +pciDettachDevice(pciDevice *dev)
> +{
> + char path[PATH_MAX];
> +
> + /* Try loading the pci-stub module if it isn't already loaded;
> + * return an error if there is no pci-stub available.
> + */
> + if (!virFileExists(PCI_SYSFS "drivers/pci-stub")) {
> + const char *const modprobeargv[] = { MODPROBE, "pci-stub", NULL };
> +
> + if (virRun(dev->conn, modprobeargv, NULL) < 0) {
> + char ebuf[1024];
> + VIR_WARN(_("modprobe pci-stub failed: %s"),
> + virStrerror(errno, ebuf, sizeof ebuf));
> + }
> + }
> +
> + if (!virFileExists(PCI_SYSFS "drivers/pci-stub")) {
> + VIR_WARN(_("pci-stub driver not available, cannot bind device %s to it"),
> + dev->name);
> + } else {
> + /* Add the PCI device ID to pci-stub's dynamic ID table;
> + * this is needed to allow us to bind the device to pci-stub.
> + * Note: if the device is not currently bound to any driver,
> + * pci-stub will immediately be bound to the device. Also, note
> + * that if a new device with this ID is hotplugged, or if a probe
> + * is triggered for such a device, it will also be immediately
> + * bound by pci-stub.
> + */
> + if (virFileWriteStr(PCI_SYSFS "drivers/pci-stub/new_id", dev->id) < 0) {
> + virReportSystemError(dev->conn, errno,
> + _("Failed to add PCI device ID '%s' to pci-stub/new_id"),
> + dev->id);
> + return -1;
> + }
> + }
> +
> + /* If the device is already bound to a driver, unbind it.
> + * Note, this will have rather unpleasant side effects if this
> + * PCI device happens to be IDE controller for the disk hosting
> + * your root filesystem.
> + */
> + snprintf(path, sizeof(path),
> + PCI_SYSFS "devices/%s/driver/unbind", dev->name);
> + if (virFileExists(path) && virFileWriteStr(path, dev->name) < 0) {
> + virReportSystemError(dev->conn, errno,
> + _("Failed to unbind PCI device '%s'"), dev->name);
> + return -1;
> + }
> +
> + if (virFileExists(PCI_SYSFS "drivers/pci-stub")) {
> + /* If the device isn't already bound to pci-stub, try binding it now.
> + */
> + snprintf(path, sizeof(path), PCI_SYSFS "devices/%s/driver", dev->name);
> + if (!virFileLinkPointsTo(path, PCI_SYSFS "drivers/pci-stub") &&
> + virFileWriteStr(PCI_SYSFS "drivers/pci-stub/bind", dev->name) < 0) {
> + virReportSystemError(dev->conn, errno,
> + _("Failed to bind PCI device '%s' to pci-stub"),
> + dev->name);
> + return -1;
> + }
> +
> + /* If 'remove_id' exists, remove the device id from pci-stub's dynamic
> + * ID table so that 'drivers_probe' works below.
> + */
> + if (virFileExists(PCI_SYSFS "drivers/pci-stub/remove_id") &&
> + virFileWriteStr(PCI_SYSFS "drivers/pci-stub/remove_id", dev->id) < 0) {
> + virReportSystemError(dev->conn, errno,
> + _("Failed to remove PCI ID '%s' with pci-stub/remove_id"),
> + dev->id);
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> +int
> +pciReAttachDevice(pciDevice *dev)
> +{
> + if (virFileExists(PCI_SYSFS "drivers/pci-stub")) {
> + char path[PATH_MAX];
> +
> + /* If the device is bound to pci-stub, unbind it.
> + */
> + snprintf(path, sizeof(path), PCI_SYSFS "devices/%s/driver", dev->name);
> + if (virFileLinkPointsTo(path, PCI_SYSFS "drivers/pci-stub") &&
> + virFileWriteStr(PCI_SYSFS "drivers/pci-stub/unbind", dev->name) < 0) {
> + virReportSystemError(dev->conn, errno,
> + _("Failed to bind PCI device '%s' to pci-stub"),
> + dev->name);
> + return -1;
> + }
> + }
> +
> + /* Trigger a re-probe of the device is not in pci-stub's dynamic
> + * ID table. If pci-stub is available, but 'remove_id' isn't
> + * available, then re-probing would just cause the device to be
> + * re-bound to pci-stub.
> + */
> + if (!virFileExists(PCI_SYSFS "drivers/pci-stub") ||
> + virFileExists(PCI_SYSFS "drivers/pci-stub/remove_id")) {
> + if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name) < 0) {
> + virReportSystemError(dev->conn, errno,
> + _("Failed to trigger a re-probe for PCI device '%s'"),
> + dev->name);
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
These functions really need to take the driver name as an argument,
so we can pass 'pci-stub' for KVM (or modern pv_ops Xen dom0), or
'pciback' for old pre-pv_ops Xen dom0.
> +pciDevice *pciGetDevice (virConnectPtr conn,
> + unsigned vendor,
> + unsigned product,
> + unsigned domain,
> + unsigned bus,
> + unsigned slot,
> + unsigned function);
> +void pciFreeDevice (pciDevice *dev);
> +int pciDettachDevice (pciDevice *dev);
> +int pciReAttachDevice (pciDevice *dev);
> +int pciResetDevice (pciDevice *dev);
I prefer it to pass the virConnectPtr conn into each of
these calls rather than storing it in the pciDevice
struct. It'd also be good to remove the vendor/product
ID here, and have this code lookup them up from te address
info, since the domain XML format only provides the caller
with the domain/bus/slot/function address info.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
More information about the libvir-list
mailing list