[1/5] xen, gfx passthrough: basic graphics passthrough support

From: Yang Zhang <yang.z.zhang@Intel.com>

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@intel.com>

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
Cc: Weidong Han <weidong.han@intel.com>
---
 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

On Fri, Feb 21, 2014 at 02:44:09PM +0800, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> 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@intel.com>
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> Cc: Weidong Han <weidong.han@intel.com>
> ---
>  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);
>              }

> -----Original Message-----
> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> bounces@lists.xen.org] On Behalf Of Yang Zhang
> Sent: Friday, February 21, 2014 1:44 AM
> To: qemu-devel@nongnu.org
> Cc: peter.maydell@linaro.org; xen-devel@lists.xensource.com;
> stefano.stabellini@eu.citrix.com; allen.m.kay@intel.com;
> weidong.han@intel.com; jean.guyader@eu.citrix.com; Yang Zhang;
> anthony@codemonkey.ws; anthony.perard@citrix.com
> Subject: [Xen-devel] [PATCH 1/5] xen, gfx passthrough: basic graphics
> passthrough support
> 
> From: Yang Zhang <yang.z.zhang@Intel.com>
> 
> 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@intel.com>
> 
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> Cc: Weidong Han <weidong.han@intel.com>
> ---
>  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"
>        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);
> 
>  #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)) {
> +        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;
> +

There is an inherent logic problem with this function.  This logic will not work if there are multiple graphics adapters in the system and you are passing the secondary graphics adapter to the guest as a primary.  Chances are very good that the VBios image at 0xC0000 does not match with the VBios image in the secondary adapter (unless they are absolutely identical and have the same VBios versions).

The proper way to do a get_vgabios() is to extract the VBios from the OPTION_ROM of the graphics device that is being passed into the guest.  You cannot assume that the image at 0xC0000 is a valid image for the device that is being passed into the guest when there are multiple adapters.

The logic that you present will work only if the primary graphics boot adapter is being passed into the guest but this is not always the case.


> +    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);
> +    /* 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
> +
>  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);
>              }
> --
> 1.7.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


Thanks,
Kelly

Anthony PERARD wrote on 2014-03-22:
> On Fri, Feb 21, 2014 at 02:44:09PM +0800, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> 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@intel.com>
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> Cc: Weidong Han <weidong.han@intel.com>

Hi all, 

Thanks for your comments. 

I am sorry for late reply. I have been busy working on other task so that I have no time to push this patch in the past two months. Now I and Tiejun will continue working on this and will send out the second version which is modified according your previous comments. Please help to review the second one again.

>> ---
>>  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);
>>              }
>


Best regards,
Yang