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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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.

> +    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

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