Re: [PATCH] Pass-through a graphic card

["Daniel P. Berrange" <berrange@redhat.com>]

On Fri, May 09, 2008 at 02:56:47PM +0100, Jean Guyader wrote:
> 
> Here, a new patch with the modifications that Samuel suggested.

> +static void dom0_read(void *opaque)
> +{
> +    struct input_event  event[5];
> +    int                 i = 0;
> +    int                 read_sz = 0;
> +    int                 fd = *(int *)opaque;
> +
> +    read_sz = read(fd, event, sizeof (struct input_event) * 5);

Could simply do  'sizeof(event)' here I believe.

> +static void dom0_driver_event_init()
> +{
> +    char                dev_name[strlen(EVENT_PATH) + 3];
> +    int                 fd = -1;
> +    int                 i = 0;
> +
> +    do
> +    {
> +        snprintf(dev_name, sizeof (dev_name), "%s%d", EVENT_PATH, i++);
> +        if ((fd = open(dev_name, O_RDONLY)) == -1)
> +            return;
> +        printf("Using %s\n", dev_name);
> +
> +        driver.event_fds = realloc(driver.event_fds,
> +                                   driver.event_nb + 1);

Error checking on reallocs...

> +        ioctl(fd, EVIOCGRAB, 1);

ioctls can fail.

> +        driver.event_fds[driver.event_nb] = fd;
> +        qemu_set_fd_handler(fd, dom0_read, NULL,
> +                            &driver.event_fds[driver.event_nb]);
> +        driver.event_nb++;
> +    }
> +    while (1);
> +}

> diff -r 810d8c3ac992 tools/ioemu/hw/pc.c
> --- a/tools/ioemu/hw/pc.c	Thu May 08 16:58:33 2008 +0100
> +++ b/tools/ioemu/hw/pc.c	Fri May 09 14:53:11 2008 +0100
> @@ -814,6 +814,13 @@ static void pc_init1(uint64_t ram_size, 
>      CPUState *env;
>      NICInfo *nd;
>      int rc;
> +#ifdef CONFIG_DM
> +    unsigned long vga_pt_enabled = 0;
> +#endif /* CONFIG_DM */
> +
> +#ifdef CONFIG_DM
> +    xc_get_hvm_param(xc_handle, domid, HVM_PARAM_VGA_PT_ENABLED, &vga_pt_enabled);
> +#endif /* CONFIG_DM */

This API can return a failure code.

> @@ -152,9 +152,122 @@ static int loadelfimage(
>      return rc;
>  }
>  
> +static int linux_get_vgabios(int               xc_handle,
> +                             unsigned char     *buf,
> +                             int               len)
> +{
> +    char        buff[1024];
> +    FILE        *fd;
> +    int         mem;
> +    char        *end_ptr;
> +    uint32_t    start, end, size;
> +
> +    if (!(fd = fopen("/proc/iomem", "r")))
> +        return 0;
> +
> +    while (fgets(buff, 1024, fd))
> +        if (strstr(buff, "Video ROM"))
> +            break;
> +
> +    if (feof(fd))

Should be also checking  ferror(fd)

> +    {
> +        fclose(fd);
> +        return 0; 
> +    }
> +
> +    fclose(fd);
> +    start = strtol(buff, &end_ptr, 16);
> +    end = strtol(end_ptr + 1, NULL, 16);
> +    size = end - start + 1;
> +
> +    if ((mem = open("/dev/mem", O_RDONLY)) < 0)
> +        return 0;
> +
> +    lseek(mem, start, SEEK_SET);
> +    read(mem, buf, size);

Check for failure / partial reads.

> +static int  linux_map_vga_ioport(int            xc_handler,
> +                                 uint32_t       dom)
> +{
> +    FILE        *fd = NULL;
> +    char        buff[256];
> +    uint32_t    start, end;
> +    char        *buff_end = NULL;
> +    
> +    if (!(fd = fopen("/proc/ioports", "r")))
> +        return -1;
> +    
> +    while (fgets(buff, 256, fd))
> +        if (strstr(buff, "vga"))
> +            break;
> +    
> +    if (feof(fd))

Also check  ferror(fd)

> +    {
> +        fclose(fd);
> +        return -1;
> +    }
> +
> +    fclose(fd);
> +    
> +    start = strtol(buff, &buff_end, 16);
> +    end = strtol(buff_end + 1, NULL, 16);
> +
> +    return xc_domain_ioport_mapping(xc_handler, dom,
> +                                    start, start, end - start + 1, 1);
> +}

> +
> +static int  setup_vga_pt(int                    xc_handle,
> +                         uint32_t               dom,
> +                         uint32_t               paddr,
> +                         struct hvm_info_table  *hvm_info)
> +{
> +    int                 rc = 0;
> +    unsigned char       *bios = NULL;
> +    int                 bios_size = 0;
> +    char                *va_bios = NULL;
> +    uint32_t            pfn = 0;
> +    
> +    /* Allocated 128K for the vga bios */
> +    if (!(bios = malloc(128 * 1024)))
> +        return -1;
> +
> +    /* Align paddr on the first next page */
> +    pfn = (paddr >> XC_PAGE_SHIFT) + 1; 
> +
> +    bios_size = linux_get_vgabios(xc_handle, bios, 128 * 1024);
> +
> +    if (bios_size <= 0)
> +    {
> +        free(bios);
> +        return -1;
> +    }
> +   
> +    va_bios = xc_map_foreign_range(xc_handle, dom,
> +                                   bios_size + bios_size % XC_PAGE_SIZE,
> +                                   PROT_READ | PROT_WRITE, pfn);

This call can fail.

> @@ -348,9 +471,10 @@ int xc_hvm_build(int xc_handle,
>  int xc_hvm_build(int xc_handle,
>                   uint32_t domid,
>                   int memsize,
> -                 const char *image_name)
> +                 const char *image_name,
> +                 struct hvm_info_table *hvm_info)
>  {
> -    char *image;
> +   char *image;

Needless whitespace error.


Regards,
Daniel
-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|