[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/2] libxl: introduce libxl__is_igd_vga_passthru



On 2015/3/6 20:40, Wei Liu wrote:
On Fri, Mar 06, 2015 at 05:08:22PM +0800, Tiejun Chen wrote:
While working with qemu, IGD is a specific device in the case of pass through
so we need to identify that to handle more later. Here we define a table to
record all IGD types currently we can support. Also we need to introduce two
helper functions to get vendor and device ids to lookup that table.

Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
---
  tools/libxl/libxl_internal.h |   2 +
  tools/libxl/libxl_pci.c      | 124 +++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 126 insertions(+)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 934465a..8b952b8 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1176,6 +1176,8 @@ _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t 
domid, libxl_device_pc
  _hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
                                        libxl_device_pci *pcidev, int num);
  _hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid);
+_hidden int libxl__is_igd_vga_passthru(libxl__gc *gc,
+                                       const libxl_domain_config *d_config);

  /*----- xswait: wait for a xenstore node to be suitable -----*/

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index f3ae132..dc5a89e 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -491,6 +491,130 @@ static int sysfs_dev_unbind(libxl__gc *gc, 
libxl_device_pci *pcidev,
      return 0;
  }

+static unsigned long sysfs_dev_get_vendor(libxl__gc *gc,
+                                          libxl_device_pci *pcidev)

uint16_t?

+{
+    char *pci_device_vendor_path =
+            libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/vendor",
+                           pcidev->domain, pcidev->bus, pcidev->dev,
+                           pcidev->func);

Please use GCSPRINTF macro.

Okay.


+    int read_items;
+    unsigned long pci_device_vendor;

uint16_t?

Yes, I can but I don't see other similar helpers are doing this in this file :)


Same comments apply to _get_device function.

And especially, if we really set that as uint16_t,


+
+    FILE *f = fopen(pci_device_vendor_path, "r");
+    if (!f) {
+        LOGE(ERROR,
+             "pci device "PCI_BDF" does not have vendor attribute",
+             pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+        return 0xffff;
+    }
+    read_items = fscanf(f, "0x%lx\n", &pci_device_vendor);

we have to refactor this as well,

read_items = fscanf(f, "0x%hx\n", &pci_device_vendor);

Right?

+    fclose(f);
+    if (read_items != 1) {
+        LOGE(ERROR,
+             "cannot read vendor of pci device "PCI_BDF,
+             pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+        return 0xffff;
+    }
+
+    return pci_device_vendor;
+}
+

[...]

+/*
+ * Some devices may need some ways to work well. Here like IGD,
+ * we have to pass a specific option to qemu.
+ */
+int libxl__is_igd_vga_passthru(libxl__gc *gc,

bool.

Okay.


+                               const libxl_domain_config *d_config)
+{
+    unsigned int i, j, num = ARRAY_SIZE(fixup_ids);
+    uint16_t vendor, device;
+
+    for (i = 0 ; i < d_config->num_pcidevs ; i++) {
+        libxl_device_pci *pcidev = &d_config->pcidevs[i];
+
+        for (j = 0 ; j < num ; j++) {
+            vendor = fixup_ids[j].vendor;
+            device = fixup_ids[j].device;
+
+            if (sysfs_dev_get_vendor(gc, pcidev) == vendor &&
+                sysfs_dev_get_device(gc, pcidev) == device)
+                return 1;

Get vendor and device in outer loop to avoid wasting cpu cycles. :-)


Yeah.

Thanks
Tiejun

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.