| Message ID | 1425632903-5502-2-git-send-email-tiejun.chen@intel.com |
|---|---|
| State | New |
| Headers | show |
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@intel.com> > --- > 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. > + int read_items; > + unsigned long pci_device_vendor; uint16_t? Same comments apply to _get_device function. > + > + 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); > + 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. > + 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. :-) Wei. > + } > + } > + > + return 0; > +} > + > /* > * A brief comment about slots. I don't know what slots are for; however, > * I have by experimentation determined: > -- > 1.9.1
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@intel.com> >> --- >> 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
On Mon, Mar 09, 2015 at 02:27:46PM +0800, Chen, Tiejun wrote: [...] > >>+ > >>+ 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? > Sure. Just eliminate any warnings / errors. Wei.
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) +{ + char *pci_device_vendor_path = + libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/vendor", + pcidev->domain, pcidev->bus, pcidev->dev, + pcidev->func); + int read_items; + unsigned long pci_device_vendor; + + 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); + 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; +} + +static unsigned long sysfs_dev_get_device(libxl__gc *gc, + libxl_device_pci *pcidev) +{ + char *pci_device_device_path = + libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/device", + pcidev->domain, pcidev->bus, pcidev->dev, + pcidev->func); + int read_items; + unsigned long pci_device_device; + + FILE *f = fopen(pci_device_device_path, "r"); + if (!f) { + LOGE(ERROR, + "pci device "PCI_BDF" does not have device attribute", + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); + return 0xffff; + } + read_items = fscanf(f, "0x%lx\n", &pci_device_device); + fclose(f); + if (read_items != 1) { + LOGE(ERROR, + "cannot read device of pci device "PCI_BDF, + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); + return 0xffff; + } + + return pci_device_device; +} + +typedef struct { + uint16_t vendor; + uint16_t device; +} pci_info; + +static const pci_info fixup_ids[] = { + /* Intel HSW Classic */ + {0x8086, 0x0402}, /* HSWGT1D, HSWD_w7 */ + {0x8086, 0x0406}, /* HSWGT1M, HSWM_w7 */ + {0x8086, 0x0412}, /* HSWGT2D, HSWD_w7 */ + {0x8086, 0x0416}, /* HSWGT2M, HSWM_w7 */ + {0x8086, 0x041E}, /* HSWGT15D, HSWD_w7 */ + /* Intel HSW ULT */ + {0x8086, 0x0A06}, /* HSWGT1UT, HSWM_w7 */ + {0x8086, 0x0A16}, /* HSWGT2UT, HSWM_w7 */ + {0x8086, 0x0A26}, /* HSWGT3UT, HSWM_w7 */ + {0x8086, 0x0A2E}, /* HSWGT3UT28W, HSWM_w7 */ + {0x8086, 0x0A1E}, /* HSWGT2UX, HSWM_w7 */ + {0x8086, 0x0A0E}, /* HSWGT1ULX, HSWM_w7 */ + /* Intel HSW CRW */ + {0x8086, 0x0D26}, /* HSWGT3CW, HSWM_w7 */ + {0x8086, 0x0D22}, /* HSWGT3CWDT, HSWD_w7 */ + /* Intel HSW Server */ + {0x8086, 0x041A}, /* HSWSVGT2, HSWD_w7 */ + /* Intel HSW SRVR */ + {0x8086, 0x040A}, /* HSWSVGT1, HSWD_w7 */ + /* Intel BSW */ + {0x8086, 0x1606}, /* BDWULTGT1, BDWM_w7 */ + {0x8086, 0x1616}, /* BDWULTGT2, BDWM_w7 */ + {0x8086, 0x1626}, /* BDWULTGT3, BDWM_w7 */ + {0x8086, 0x160E}, /* BDWULXGT1, BDWM_w7 */ + {0x8086, 0x161E}, /* BDWULXGT2, BDWM_w7 */ + {0x8086, 0x1602}, /* BDWHALOGT1, BDWM_w7 */ + {0x8086, 0x1612}, /* BDWHALOGT2, BDWM_w7 */ + {0x8086, 0x1622}, /* BDWHALOGT3, BDWM_w7 */ + {0x8086, 0x162B}, /* BDWHALO28W, BDWM_w7 */ + {0x8086, 0x162A}, /* BDWGT3WRKS, BDWM_w7 */ + {0x8086, 0x162D}, /* BDWGT3SRVR, BDWM_w7 */ +}; + +/* + * 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, + 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; + } + } + + return 0; +} + /* * A brief comment about slots. I don't know what slots are for; however, * I have by experimentation determined:
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@intel.com> --- tools/libxl/libxl_internal.h | 2 + tools/libxl/libxl_pci.c | 124 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+)