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

["Zytaruk, Kelly" <Kelly.Zytaruk@amd.com>]

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