kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Inform users about busy device assignment attempt
@ 2009-12-09 18:04 Alexander Graf
  2009-12-09 18:19 ` Daniel P. Berrange
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Graf @ 2009-12-09 18:04 UTC (permalink / raw)
  To: kvm

When using -pcidevice on a device that is already in use by a kernel driver
all the user gets is the following (very useful) information:

  Failed to assign device "04:00.0" : Device or resource busy
  Failed to deassign device "04:00.0" : Invalid argument
  Error initializing device pci-assign

Since I usually prefer to have my computer do the thinking for me, I figured
it might be a good idea to check and see if a device is actually used by a
driver. If so, tell the user.

So with this patch applied you get the following output:

  Failed to assign device "04:00.0" : Device or resource busy
  *** The driver 'igb' is occupying your device 04:00.0.
  *** Try running "rmmod igb" on the commandline
  Failed to deassign device "04:00.0" : Invalid argument
  Error initializing device pci-assign

That should keep people like me from doing the most obvious misuses :-).

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/device-assignment.c |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index beb488c..2612c25 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -747,7 +747,8 @@ static uint32_t calc_assigned_dev_id(uint8_t bus, uint8_t devfn)
 static int assign_device(AssignedDevice *dev)
 {
     struct kvm_assigned_pci_dev assigned_dev_data;
-    int r;
+    char dir[128], driver[128], *ns;
+    int r, v;
 
     memset(&assigned_dev_data, 0, sizeof(assigned_dev_data));
     assigned_dev_data.assigned_dev_id  =
@@ -769,9 +770,25 @@ static int assign_device(AssignedDevice *dev)
 #endif
 
     r = kvm_assign_pci_device(kvm_context, &assigned_dev_data);
-    if (r < 0)
+    if (r < 0) {
 	fprintf(stderr, "Failed to assign device \"%s\" : %s\n",
                 dev->dev.qdev.id, strerror(-r));
+
+        snprintf(dir, sizeof(dir),
+                 "/sys/bus/pci/devices/0000:%02x:%02x.%x/driver",
+	         dev->host.bus, dev->host.dev, dev->host.func);
+
+        v = readlink(dir, driver, sizeof(driver));
+        if ((v > 0) && (ns = strrchr(driver, '/'))) {
+            ns++;
+            fprintf(stderr, "*** The driver '%s' is occupying your "
+                                 "device %02x:%02x.%x.\n"
+                            "*** Try running \"rmmod %s\" on the command"
+                                 "line\n",
+                            ns, dev->host.bus, dev->host.dev, dev->host.func,
+                            ns);
+        }
+    }
     return r;
 }
 
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH] Inform users about busy device assignment attempt
@ 2009-12-09 20:13 Alexander Graf
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Graf @ 2009-12-09 20:13 UTC (permalink / raw)
  To: kvm; +Cc: Daniel P. Berrange

When using -pcidevice on a device that is already in use by a kernel driver
all the user gets is the following (very useful) information:

  Failed to assign device "04:00.0" : Device or resource busy
  Failed to deassign device "04:00.0" : Invalid argument
  Error initializing device pci-assign

Since I usually prefer to have my computer do the thinking for me, I figured
it might be a good idea to check and see if a device is actually used by a
driver. If so, tell the user.

So with this patch applied you get the following output:

  Failed to assign device "04:00.0" : Device or resource busy
  *** The driver 'igb' is occupying your device 04:00.0.
  ***
  *** You can try the following commands to free it:
  ***
  *** $ echo "8086 150a" > /sys/bus/pci/drivers/pci-stub/new_id
  *** $ echo "04:00.0" > /sys/bus/pci/drivers/igb/unbind
  *** $ echo "04:00.0" > /sys/bus/pci/drivers/pci-stub/bind
  *** $ echo "8086 150a" > /sys/bus/pci/drivers/pci-stub/remove_id
  ***
  Failed to deassign device "04:00.0" : Invalid argument
  Error initializing device pci-assign

That should keep people like me from doing the most obvious misuses :-).

CC: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - add more helpful guidance thanks to Daniel Berrange
---
 hw/device-assignment.c |  106 +++++++++++++++++++++++++++++++++++++----------
 1 files changed, 83 insertions(+), 23 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index beb488c..9fbddcf 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -572,6 +572,36 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
     return 0;
 }
 
+static int get_real_id(const char *devpath, const char *idname, uint16_t *val)
+{
+    FILE *f;
+    char name[128];
+    long id;
+
+    snprintf(name, sizeof(name), "%s%s", devpath, idname);
+    f = fopen(name, "r");
+    if (f == NULL) {
+        fprintf(stderr, "%s: %s: %m\n", __func__, name);
+        return -1;
+    }
+    if (fscanf(f, "%li\n", &id) == 1) {
+        *val = id;
+    }
+    fclose(f);
+
+    return 0;
+}
+
+static int get_real_vendor_id(const char *devpath, uint16_t *val)
+{
+    return get_real_id(devpath, "vendor", val);
+}
+
+static int get_real_device_id(const char *devpath, uint16_t *val)
+{
+    return get_real_id(devpath, "device", val);
+}
+
 static int get_real_device(AssignedDevice *pci_dev, uint8_t r_bus,
                            uint8_t r_dev, uint8_t r_func)
 {
@@ -579,7 +609,7 @@ static int get_real_device(AssignedDevice *pci_dev, uint8_t r_bus,
     int fd, r = 0;
     FILE *f;
     unsigned long long start, end, size, flags;
-    unsigned long id;
+    uint16_t id;
     struct stat statbuf;
     PCIRegion *rp;
     PCIDevRegions *dev = &pci_dev->real_device;
@@ -645,31 +675,21 @@ again:
 
     fclose(f);
 
-    /* read and fill device ID */
-    snprintf(name, sizeof(name), "%svendor", dir);
-    f = fopen(name, "r");
-    if (f == NULL) {
-        fprintf(stderr, "%s: %s: %m\n", __func__, name);
+    /* read and fill vendor ID */
+    r = get_real_vendor_id(dir, &id);
+    if (r) {
         return 1;
     }
-    if (fscanf(f, "%li\n", &id) == 1) {
-	pci_dev->dev.config[0] = id & 0xff;
-	pci_dev->dev.config[1] = (id & 0xff00) >> 8;
-    }
-    fclose(f);
+    pci_dev->dev.config[0] = id & 0xff;
+    pci_dev->dev.config[1] = (id & 0xff00) >> 8;
 
-    /* read and fill vendor ID */
-    snprintf(name, sizeof(name), "%sdevice", dir);
-    f = fopen(name, "r");
-    if (f == NULL) {
-        fprintf(stderr, "%s: %s: %m\n", __func__, name);
+    /* read and fill device ID */
+    r = get_real_device_id(dir, &id);
+    if (r) {
         return 1;
     }
-    if (fscanf(f, "%li\n", &id) == 1) {
-	pci_dev->dev.config[2] = id & 0xff;
-	pci_dev->dev.config[3] = (id & 0xff00) >> 8;
-    }
-    fclose(f);
+    pci_dev->dev.config[2] = id & 0xff;
+    pci_dev->dev.config[3] = (id & 0xff00) >> 8;
 
     /* dealing with virtual function device */
     snprintf(name, sizeof(name), "%sphysfn/", dir);
@@ -747,7 +767,9 @@ static uint32_t calc_assigned_dev_id(uint8_t bus, uint8_t devfn)
 static int assign_device(AssignedDevice *dev)
 {
     struct kvm_assigned_pci_dev assigned_dev_data;
-    int r;
+    char name[128], dir[128], driver[128], *ns;
+    uint16_t vendor_id, device_id;
+    int r, v;
 
     memset(&assigned_dev_data, 0, sizeof(assigned_dev_data));
     assigned_dev_data.assigned_dev_id  =
@@ -769,9 +791,47 @@ static int assign_device(AssignedDevice *dev)
 #endif
 
     r = kvm_assign_pci_device(kvm_context, &assigned_dev_data);
-    if (r < 0)
+    if (r < 0) {
 	fprintf(stderr, "Failed to assign device \"%s\" : %s\n",
                 dev->dev.qdev.id, strerror(-r));
+
+        snprintf(dir, sizeof(dir),
+                 "/sys/bus/pci/devices/0000:%02x:%02x.%x/",
+	         dev->host.bus, dev->host.dev, dev->host.func);
+
+        snprintf(name, sizeof(name), "%sdriver", dir);
+
+        v = readlink(name, driver, sizeof(driver));
+        if ((v <= 0) || !(ns = strrchr(driver, '/'))) {
+            return r;
+        }
+
+        ns++;
+
+        if (get_real_vendor_id(dir, &vendor_id) ||
+            get_real_device_id(dir, &device_id)) {
+            return r;
+        }
+
+        fprintf(stderr, "*** The driver '%s' is occupying your device "
+                        "%02x:%02x.%x.\n",
+                ns, dev->host.bus, dev->host.dev, dev->host.func);
+        fprintf(stderr, "***\n");
+        fprintf(stderr, "*** You can try the following commands to free "
+                        "it:\n");
+        fprintf(stderr, "***\n");
+        fprintf(stderr, "*** $ echo \"%04x %04x\" > /sys/bus/pci/drivers/"
+                        "pci-stub/new_id\n", vendor_id, device_id);
+        fprintf(stderr, "*** $ echo \"%02x:%02x.%x\" > /sys/bus/pci/drivers/%s"
+                        "/unbind\n", dev->host.bus, dev->host.dev,
+                                     dev->host.func, ns);
+        fprintf(stderr, "*** $ echo \"%02x:%02x.%x\" > /sys/bus/pci/drivers/"
+                        "pci-stub/bind\n", dev->host.bus, dev->host.dev,
+                                           dev->host.func);
+        fprintf(stderr, "*** $ echo \"%x %x\" > /sys/bus/pci/drivers/pci-stub"
+                        "/remove_id\n", vendor_id, device_id);
+        fprintf(stderr, "***\n");
+    }
     return r;
 }
 
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH] Enable non page boundary BAR device assignment
@ 2009-12-10 23:06 Alexander Graf
  2009-12-10 23:06 ` [PATCH] Inform users about busy device assignment attempt Alexander Graf
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Graf @ 2009-12-10 23:06 UTC (permalink / raw)
  To: kvm list; +Cc: Gleb Natapov, Michael S. Tsirkin, Muli Ben-Yehuda

While trying to get device passthrough working with an emulex hba, kvm
refused to pass it through because it has a BAR of 256 bytes:

        Region 0: Memory at d2100000 (64-bit, non-prefetchable) [size=4K]
        Region 2: Memory at d2101000 (64-bit, non-prefetchable) [size=256]
        Region 4: I/O ports at b100 [size=256]

Since the page boundary is an arbitrary optimization to allow 1:1 mapping of
physical to virtual addresses, we can still take the old MMIO callback route.

So let's add a second code path that allows for size & 0xFFF != 0 sized regions
by looping it through userspace.

I verified that it works by passing through an e1000 with this additional patch
applied and the card acted the same way it did without this patch:

             map_func = assigned_dev_iomem_map;
-            if (cur_region->size & 0xFFF) {
+            if (i != PCI_ROM_SLOT){
                 fprintf(stderr, "PCI region %d at address 0x%llx "

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - don't use map_func function pointer
  - use the same code for mmap on fast and slow path
---
 hw/device-assignment.c |  123 +++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 116 insertions(+), 7 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 13a86bb..5cee929 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -148,6 +148,105 @@ static uint32_t assigned_dev_ioport_readl(void *opaque, uint32_t addr)
     return value;
 }
 
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+    AssignedDevRegion *d = opaque;
+    uint8_t *in = (uint8_t*)(d->u.r_virtbase + addr);
+    uint32_t r = -1;
+
+    r = *in;
+    DEBUG("slow_bar_readl addr=0x" TARGET_FMT_plx " val=0x%08x\n", addr, r);
+
+    return r;
+}
+
+static uint32_t slow_bar_readw(void *opaque, target_phys_addr_t addr)
+{
+    AssignedDevRegion *d = opaque;
+    uint16_t *in = (uint16_t*)(d->u.r_virtbase + addr);
+    uint32_t r = -1;
+
+    r = *in;
+    DEBUG("slow_bar_readl addr=0x" TARGET_FMT_plx " val=0x%08x\n", addr, r);
+
+    return r;
+}
+
+static uint32_t slow_bar_readl(void *opaque, target_phys_addr_t addr)
+{
+    AssignedDevRegion *d = opaque;
+    uint32_t *in = (uint32_t*)(d->u.r_virtbase + addr);
+    uint32_t r = -1;
+
+    r = *in;
+    DEBUG("slow_bar_readl addr=0x" TARGET_FMT_plx " val=0x%08x\n", addr, r);
+
+    return r;
+}
+
+static void slow_bar_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
+{
+    AssignedDevRegion *d = opaque;
+    uint8_t *out = (uint8_t*)(d->u.r_virtbase + addr);
+
+    DEBUG("slow_bar_writeb addr=0x" TARGET_FMT_plx " val=0x%02x\n", addr, val);
+    *out = val;
+}
+
+static void slow_bar_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
+{
+    AssignedDevRegion *d = opaque;
+    uint16_t *out = (uint16_t*)(d->u.r_virtbase + addr);
+
+    DEBUG("slow_bar_writew addr=0x" TARGET_FMT_plx " val=0x%04x\n", addr, val);
+    *out = val;
+}
+
+static void slow_bar_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
+{
+    AssignedDevRegion *d = opaque;
+    uint32_t *out = (uint32_t*)(d->u.r_virtbase + addr);
+
+    DEBUG("slow_bar_writel addr=0x" TARGET_FMT_plx " val=0x%08x\n", addr, val);
+    *out = val;
+}
+
+static CPUWriteMemoryFunc * const slow_bar_write[] = {
+    &slow_bar_writeb,
+    &slow_bar_writew,
+    &slow_bar_writel
+};
+
+static CPUReadMemoryFunc * const slow_bar_read[] = {
+    &slow_bar_readb,
+    &slow_bar_readw,
+    &slow_bar_readl
+};
+
+static void assigned_dev_iomem_map_slow(PCIDevice *pci_dev, int region_num,
+                                        pcibus_t e_phys, pcibus_t e_size,
+                                        int type)
+{
+    AssignedDevice *r_dev = container_of(pci_dev, AssignedDevice, dev);
+    AssignedDevRegion *region = &r_dev->v_addrs[region_num];
+    PCIRegion *real_region = &r_dev->real_device.regions[region_num];
+    int m;
+
+    DEBUG("slow map\n");
+    m = cpu_register_io_memory(slow_bar_read, slow_bar_write, region);
+    cpu_register_physical_memory(e_phys, e_size, m);
+
+    /* MSI-X MMIO page */
+    if ((e_size > 0) &&
+        real_region->base_addr <= r_dev->msix_table_addr &&
+        real_region->base_addr + real_region->size >= r_dev->msix_table_addr) {
+        int offset = r_dev->msix_table_addr - real_region->base_addr;
+
+        cpu_register_physical_memory(e_phys + offset,
+                TARGET_PAGE_SIZE, r_dev->mmio_index);
+    }
+}
+
 static void assigned_dev_iomem_map(PCIDevice *pci_dev, int region_num,
                                    pcibus_t e_phys, pcibus_t e_size, int type)
 {
@@ -381,15 +480,22 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
 
         /* handle memory io regions */
         if (cur_region->type & IORESOURCE_MEM) {
+            int slow_map = 0;
             int t = cur_region->type & IORESOURCE_PREFETCH
                 ? PCI_BASE_ADDRESS_MEM_PREFETCH
                 : PCI_BASE_ADDRESS_SPACE_MEMORY;
+
             if (cur_region->size & 0xFFF) {
-                fprintf(stderr, "Unable to assign device: PCI region %d "
-                        "at address 0x%llx has size 0x%x, "
-                        " which is not a multiple of 4K\n",
+                fprintf(stderr, "PCI region %d at address 0x%llx "
+                        "has size 0x%x, which is not a multiple of 4K. "
+                        "Using slow map\n",
                         i, (unsigned long long)cur_region->base_addr,
                         cur_region->size);
+                slow_map = 1;
+            }
+
+            if (slow_map && (i == PCI_ROM_SLOT)) {
+                fprintf(stderr, "ROM not aligned - can't continue\n");
                 return -1;
             }
 
@@ -405,7 +511,7 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
             } else {
                 pci_dev->v_addrs[i].u.r_virtbase =
                     mmap(NULL,
-                         (cur_region->size + 0xFFF) & 0xFFFFF000,
+                         cur_region->size,
                          PROT_WRITE | PROT_READ, MAP_SHARED,
                          cur_region->resource_fd, (off_t) 0);
             }
@@ -429,12 +535,15 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
             pci_dev->v_addrs[i].e_size = 0;
 
             /* add offset */
-            pci_dev->v_addrs[i].u.r_virtbase +=
-                (cur_region->base_addr & 0xFFF);
+            if (!slow_map) {
+                pci_dev->v_addrs[i].u.r_virtbase +=
+                    (cur_region->base_addr & 0xFFF);
+            }
 
             pci_register_bar((PCIDevice *) pci_dev, i,
                              cur_region->size, t,
-                             assigned_dev_iomem_map);
+                             slow_map ? assigned_dev_iomem_map_slow
+                                      : assigned_dev_iomem_map);
             continue;
         }
         /* handle port io regions */
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2009-12-15 15:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-09 18:04 [PATCH] Inform users about busy device assignment attempt Alexander Graf
2009-12-09 18:19 ` Daniel P. Berrange
2009-12-09 19:18   ` Alexander Graf
2009-12-09 19:40     ` Daniel P. Berrange
2009-12-09 19:44       ` Alexander Graf
2009-12-09 20:14         ` Daniel P. Berrange
2009-12-09 20:15           ` Alexander Graf
  -- strict thread matches above, loose matches on Subject: below --
2009-12-09 20:13 Alexander Graf
2009-12-10 23:06 [PATCH] Enable non page boundary BAR device assignment Alexander Graf
2009-12-10 23:06 ` [PATCH] Inform users about busy device assignment attempt Alexander Graf
2009-12-11 11:38   ` Michael S. Tsirkin
2009-12-15 15:18     ` Alexander Graf
2009-12-15 15:18       ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).