Re: [Xen-devel] [PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Feb 21, 2014 at 02:44:09PM +0800, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> 
> basic gfx passthrough support:
> - add a vga type for gfx passthrough
> - retrieve VGA bios from host 0xC0000, then load it to guest 0xC0000
> - register/unregister legacy VGA I/O ports and MMIOs for passthroughed gfx
> 
> The original patch is from Weidong Han <weidong.han@xxxxxxxxx>
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> Cc: Weidong Han <weidong.han@xxxxxxxxx>
> ---
>  configure                    |    2 +-
>  hw/xen/Makefile.objs         |    2 +-
>  hw/xen/xen-host-pci-device.c |    5 ++
>  hw/xen/xen-host-pci-device.h |    1 +
>  hw/xen/xen_pt.c              |   10 +++
>  hw/xen/xen_pt.h              |    4 +
>  hw/xen/xen_pt_graphics.c     |  164 
> ++++++++++++++++++++++++++++++++++++++++++
>  qemu-options.hx              |    9 +++
>  vl.c                         |    8 ++
>  9 files changed, 203 insertions(+), 2 deletions(-)
>  create mode 100644 hw/xen/xen_pt_graphics.c
> 
> diff --git a/configure b/configure
> index 4648117..19525ab 100755
> --- a/configure
> +++ b/configure
> @@ -4608,7 +4608,7 @@ case "$target_name" in
>      if test "$xen" = "yes" -a "$target_softmmu" = "yes" ; then
>        echo "CONFIG_XEN=y" >> $config_target_mak
>        if test "$xen_pci_passthrough" = yes; then
> -        echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak"
> +        echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_host_mak"

Why do you need to move this option from config_target to config_host?

>        fi
>      fi
>      ;;
> diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
> index ce640c6..350d337 100644
> --- a/hw/xen/Makefile.objs
> +++ b/hw/xen/Makefile.objs
> @@ -3,4 +3,4 @@ common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o 
> xen_devconfig.o
>  
>  obj-$(CONFIG_XEN_I386) += xen_platform.o xen_apic.o xen_pvdevice.o
>  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
> -obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o 
> xen_pt_msi.o
> +obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o 
> xen_pt_msi.o xen_pt_graphics.o
> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> index 743b37b..a54b7de 100644
> --- a/hw/xen/xen-host-pci-device.c
> +++ b/hw/xen/xen-host-pci-device.c
> @@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice *d, 
> uint16_t domain,
>          goto error;
>      }
>      d->irq = v;
> +    rc = xen_host_pci_get_hex_value(d, "class", &v);
> +    if (rc) {
> +        goto error;
> +    }
> +    d->class_code = v;
>      d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
>  
>      return 0;
> diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
> index c2486f0..f1e1c30 100644
> --- a/hw/xen/xen-host-pci-device.h
> +++ b/hw/xen/xen-host-pci-device.h
> @@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice {
>  
>      uint16_t vendor_id;
>      uint16_t device_id;
> +    uint32_t class_code;
>      int irq;
>  
>      XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1];
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index be4220b..5a36902 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -450,6 +450,7 @@ static int xen_pt_register_regions(XenPCIPassthroughState 
> *s)
>                     d->rom.size, d->rom.base_addr);
>      }
>  
> +    register_vga_regions(d);
>      return 0;
>  }
>  
> @@ -470,6 +471,8 @@ static void 
> xen_pt_unregister_regions(XenPCIPassthroughState *s)
>      if (d->rom.base_addr && d->rom.size) {
>          memory_region_destroy(&s->rom);
>      }
> +
> +    unregister_vga_regions(d);
>  }
>  
>  /* region mapping */
> @@ -693,6 +696,13 @@ static int xen_pt_initfn(PCIDevice *d)
>      /* Handle real device's MMIO/PIO BARs */
>      xen_pt_register_regions(s);
>  
> +    /* Setup VGA bios for passthroughed gfx */
> +    if (setup_vga_pt(&s->real_device) < 0) {
> +        XEN_PT_ERR(d, "Setup VGA BIOS of passthroughed gfx failed!\n");
> +        xen_host_pci_device_put(&s->real_device);
> +        return -1;
> +    }
> +
>      /* reinitialize each config register to be emulated */
>      if (xen_pt_config_init(s)) {
>          XEN_PT_ERR(d, "PCI Config space initialisation failed.\n");
> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
> index 942dc60..c04bbfd 100644
> --- a/hw/xen/xen_pt.h
> +++ b/hw/xen/xen_pt.h
> @@ -298,5 +298,9 @@ static inline bool 
> xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
>      return s->msix && s->msix->bar_index == bar;
>  }
>  
> +extern int gfx_passthru;
> +int register_vga_regions(XenHostPCIDevice *dev);
> +int unregister_vga_regions(XenHostPCIDevice *dev);
> +int setup_vga_pt(XenHostPCIDevice *dev);

I believe those function names need to be prefix with xen_pt_
(e.g. xen_pt_register_vga_regions).

>  #endif /* !XEN_PT_H */
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> new file mode 100644
> index 0000000..9ad8a74
> --- /dev/null
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -0,0 +1,164 @@
> +/*
> + * graphics passthrough
> + */
> +#include "xen_pt.h"
> +#include "xen-host-pci-device.h"
> +#include "hw/xen/xen_backend.h"
> +
> +/*
> + * register VGA resources for the domain with assigned gfx
> + */
> +int register_vga_regions(XenHostPCIDevice *dev)
> +{
> +    int ret = 0;
> +
> +    if (!gfx_passthru || ((dev->class_code >> 0x8) != 0x0300)) {

Instead of 0x0300, you can use PCI_CLASS_DISPLAY_VGA. The same apply to
the few other places.

> +        return ret;
> +    }
> +
> +    ret |= xc_domain_ioport_mapping(xen_xc, xen_domid, 0x3B0,
> +            0x3B0, 0xA, DPCI_ADD_MAPPING);
> +
> +    ret |= xc_domain_ioport_mapping(xen_xc, xen_domid, 0x3C0,
> +            0x3C0, 0x20, DPCI_ADD_MAPPING);
> +
> +    ret |= xc_domain_memory_mapping(xen_xc, xen_domid,
> +            0xa0000 >> XC_PAGE_SHIFT,
> +            0xa0000 >> XC_PAGE_SHIFT,
> +            0x20,
> +            DPCI_ADD_MAPPING);
> +
> +    if (ret != 0) {
> +        XEN_PT_ERR(NULL, "VGA region mapping failed\n");
> +    }
> +
> +    return ret;
> +}
> +
> +/*
> + * unregister VGA resources for the domain with assigned gfx
> + */
> +int unregister_vga_regions(XenHostPCIDevice *dev)
> +{
> +    int ret = 0;
> +
> +    if (!gfx_passthru || ((dev->class_code >> 0x8) != 0x0300)) {
> +        return ret;
> +    }
> +
> +    ret |= xc_domain_ioport_mapping(xen_xc, xen_domid, 0x3B0,
> +            0x3B0, 0xC, DPCI_REMOVE_MAPPING);
> +
> +    ret |= xc_domain_ioport_mapping(xen_xc, xen_domid, 0x3C0,
> +            0x3C0, 0x20, DPCI_REMOVE_MAPPING);
> +
> +    ret |= xc_domain_memory_mapping(xen_xc, xen_domid,
> +            0xa0000 >> XC_PAGE_SHIFT,
> +            0xa0000 >> XC_PAGE_SHIFT,
> +            20,
> +            DPCI_REMOVE_MAPPING);
> +
> +    if (ret != 0) {
> +        XEN_PT_ERR(NULL, "VGA region unmapping failed\n");
> +    }
> +
> +    return ret;
> +}
> +
> +static int get_vgabios(unsigned char *buf)
> +{
> +    int fd;
> +    uint32_t bios_size = 0;
> +    uint32_t start = 0xC0000;
> +    uint16_t magic = 0;
> +
> +    fd = open("/dev/mem", O_RDONLY);
> +    if (fd < 0) {
> +        XEN_PT_ERR(NULL, "Can't open /dev/mem: %s\n", strerror(errno));
> +        return 0;
> +    }
> +
> +    /*
> +     * Check if it a real bios extension.
> +     * The magic number is 0xAA55.
> +     */
> +    if (start != lseek(fd, start, SEEK_SET)) {
> +        goto out;
> +    }
> +    if (read(fd, &magic, 2) != 2) {
> +        goto out;
> +    }
> +    if (magic != 0xAA55) {
> +        goto out;
> +    }
> +
> +    /* Find the size of the rom extension */
> +    if (start != lseek(fd, start, SEEK_SET)) {
> +        goto out;
> +    }
> +    if (lseek(fd, 2, SEEK_CUR) != (start + 2)) {
> +        goto out;
> +    }
> +    if (read(fd, &bios_size, 1) != 1) {
> +        goto out;
> +    }
> +
> +    /* This size is in 512 bytes */
> +    bios_size *= 512;
> +
> +    /*
> +     * Set the file to the begining of the rombios,
> +     * to start the copy.
> +     */
> +    if (start != lseek(fd, start, SEEK_SET)) {
> +        goto out;
> +    }
> +
> +    if (bios_size != read(fd, buf, bios_size)) {
> +        bios_size = 0;
> +    }
> +
> +out:
> +    close(fd);
> +    return bios_size;
> +}
> +
> +int setup_vga_pt(XenHostPCIDevice *dev)
> +{
> +    unsigned char *bios = NULL;
> +    int bios_size = 0;
> +    char *c = NULL;
> +    char checksum = 0;
> +    int rc = 0;
> +
> +    if (!gfx_passthru || ((dev->class_code >> 0x8) != 0x0300)) {
> +        return rc;
> +    }
> +
> +    bios = malloc(64 * 1024);

I think g_malloc should be used here, instead of malloc, and g_malloc
always return an allocated buffer. (it never fail, or it don't return)

> +    /* Allocated 64K for the vga bios */
> +    if (!bios) {
> +        return -1;
> +    }
> +
> +    bios_size = get_vgabios(bios);
> +    if (bios_size == 0 || bios_size > 64 * 1024) {
> +        XEN_PT_ERR(NULL, "vga bios size (0x%x) is invalid!\n", bios_size);
> +        rc = -1;
> +        goto out;
> +    }
> +
> +    /* Adjust the bios checksum */
> +    for (c = (char *)bios; c < ((char *)bios + bios_size); c++) {
> +        checksum += *c;
> +    }
> +    if (checksum) {
> +        bios[bios_size - 1] -= checksum;
> +        XEN_PT_LOG(NULL, "vga bios checksum is adjusted!\n");
> +    }
> +
> +    cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
> +out:
> +    free(bios);
> +    return rc;
> +}
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 56e5fdf..95de002 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1034,6 +1034,15 @@ STEXI
>  Rotate graphical output some deg left (only PXA LCD).
>  ETEXI
>  
> +DEF("gfx_passthru", 0, QEMU_OPTION_gfx_passthru,
> +    "-gfx_passthru   enable Intel IGD passthrough by XEN\n",
> +    QEMU_ARCH_ALL)
> +STEXI
> +@item -gfx_passthru
> +@findex -gfx_passthru
> +Enable Intel IGD passthrough by XEN
> +ETEXI
> +

Is this options really necessary?  If someone is passing-through a
graphic card, he propably want to pass it through as a graphic card,
without having to enable yet another option.

>  DEF("vga", HAS_ARG, QEMU_OPTION_vga,
>      "-vga [std|cirrus|vmware|qxl|xenfb|none]\n"
>      "                select video card type\n", QEMU_ARCH_ALL)
> diff --git a/vl.c b/vl.c
> index 316de54..8a91054 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -215,6 +215,9 @@ static bool tcg_allowed = true;
>  bool xen_allowed;
>  uint32_t xen_domid;
>  enum xen_mode xen_mode = XEN_EMULATE;
> +#if defined(CONFIG_XEN_PCI_PASSTHROUGH)
> +int gfx_passthru = 0;
> +#endif
>  static int tcg_tb_size;
>  
>  static int default_serial = 1;
> @@ -3775,6 +3778,11 @@ int main(int argc, char **argv, char **envp)
>                  }
>                  configure_msg(opts);
>                  break;
> +#if defined(CONFIG_XEN_PCI_PASSTHROUGH)
> +            case QEMU_OPTION_gfx_passthru:
> +                gfx_passthru = 1;
> +                break;
> +#endif
>              default:
>                  os_parse_cmd_args(popt->index, optarg);
>              }

-- 
Anthony PERARD

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