RE: [PATCH 2/2] kvm: qemu: check device assignment command

["Han, Weidong" <weidong.han@intel.com>]

Avi Kivity wrote:
> Han, Weidong wrote:
>>> I suggest replacing the parsing code with pci_parse_devaddr() (needs
>>> to be extended to support functions) so that all the checking and
>>> parsing is done in one place. 
>>> 
>> 
>> If use pci_parse_devaddr(), it needs to add domain section to
>> assigning command, and add function section to pci_add/pci_del
>> commands. What's more, pci_parse_devaddr() parses guest device bdf,
>> there are some assumption, such as function is 0. But here parse
>> host bdf. It's a little complex to combine them together.    
>> 
> 
> Right, but we end up with overall better code.

pci_parse_devaddr parses [[<domain>:][<bus>:]<slot>, it's valid when even enter only slot, whereas it must be bus:slot.func in device assignment command  (-pcidevice host=bus:slot.func). So I implemented a dedicated function to parse device bdf in device assignment command, rather than mix two parsing function together.

Signed-off-by: Weidong Han <weidong.han@intel.com>

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index cef7c8a..53375ff 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -1195,8 +1195,7 @@ out:
  */
 AssignedDevInfo *add_assigned_device(const char *arg)
 {
-    char *cp, *cp1;
-    char device[8];
+    char device[16];
     char dma[6];
     int r;
     AssignedDevInfo *adev;
@@ -1207,6 +1206,13 @@ AssignedDevInfo *add_assigned_device(const char *arg)
         return NULL;
     }
     r = get_param_value(device, sizeof(device), "host", arg);
+    if (!r)
+         goto bad;
+
+    r = pci_parse_host_devaddr(device, &adev->bus, &adev->dev, &adev->func);
+    if (r)
+        goto bad;
+
     r = get_param_value(adev->name, sizeof(adev->name), "name", arg);
     if (!r)
 	snprintf(adev->name, sizeof(adev->name), "%s", device);
@@ -1216,18 +1222,6 @@ AssignedDevInfo *add_assigned_device(const char *arg)
     if (r && !strncmp(dma, "none", 4))
         adev->disable_iommu = 1;
 #endif
-    cp = device;
-    adev->bus = strtoul(cp, &cp1, 16);
-    if (*cp1 != ':')
-        goto bad;
-    cp = cp1 + 1;
-
-    adev->dev = strtoul(cp, &cp1, 16);
-    if (*cp1 != '.')
-        goto bad;
-    cp = cp1 + 1;
-
-    adev->func = strtoul(cp, &cp1, 16);
 
     LIST_INSERT_HEAD(&adev_head, adev, next);
     return adev;
diff --git a/qemu/hw/pci.c b/qemu/hw/pci.c
index eca0517..bf97c8c 100644
--- a/qemu/hw/pci.c
+++ b/qemu/hw/pci.c
@@ -163,6 +163,7 @@ static int pci_set_default_subsystem_id(PCIDevice *pci_dev)
 }
 
 /*
+ * Parse pci address in qemu command
  * Parse [[<domain>:]<bus>:]<slot>, return -1 on error
  */
 static int pci_parse_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
@@ -211,6 +212,55 @@ static int pci_parse_devaddr(const char *addr, int *domp, int *busp, unsigned *s
     return 0;
 }
 
+/*
+ * Parse device bdf in device assignment command:
+ *
+ * -pcidevice host=bus:dev.func
+ *
+ * Parse <bus>:<slot>.<func> return -1 on error
+ */
+int pci_parse_host_devaddr(const char *addr, int *busp,
+                           int *slotp, int *funcp)
+{
+    const char *p;
+    char *e;
+    int val;
+    int bus = 0, slot = 0, func = 0;
+
+    p = addr;
+    val = strtoul(p, &e, 16);
+    if (e == p)
+	return -1;
+    if (*e == ':') {
+	bus = val;
+	p = e + 1;
+	val = strtoul(p, &e, 16);
+	if (e == p)
+	    return -1;
+	if (*e == '.') {
+	    slot = val;
+	    p = e + 1;
+	    val = strtoul(p, &e, 16);
+	    if (e == p)
+		return -1;
+	    func = val;
+	} else
+	    return -1;
+    } else
+	return -1;
+
+    if (bus > 0xff || slot > 0x1f || func > 0x7)
+	return -1;
+
+    if (*e)
+	return -1;
+
+    *busp = bus;
+    *slotp = slot;
+    *funcp = func;
+    return 0;
+}
+
 int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp)
 {
     char devaddr[32];
diff --git a/qemu/hw/pci.h b/qemu/hw/pci.h
index a7438f2..bfdd29a 100644
--- a/qemu/hw/pci.h
+++ b/qemu/hw/pci.h
@@ -227,6 +227,9 @@ PCIDevice *pci_find_device(int bus_num, int slot, int function);
 int pci_read_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp);
 int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp);
 
+int pci_parse_host_devaddr(const char *addr, int *busp,
+                           int *slotp, int *funcp);
+
 void pci_info(Monitor *mon);
 PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
                         pci_map_irq_fn map_irq, const char *name);