|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [v5][PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support
On Wed, Jun 25, 2014 at 05:06:37PM +0800, Chen, Tiejun wrote:
> On 2014/6/25 14:35, Michael S. Tsirkin wrote:
> >On Wed, Jun 25, 2014 at 10:17:17AM +0800, Tiejun Chen wrote:
> >>basic gfx passthrough support:
> >>- add a vga type for gfx passthrough
> >>- retrieve VGA bios from sysfs, then load it to guest at 0xC0000
> >>- register/unregister legacy VGA I/O ports and MMIOs for passthrough GFX
> >>
> >>The original patch is from Weidong Han <weidong.han@xxxxxxxxx>
> >>
> >>Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> >>Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
> >>Cc: Weidong Han <weidong.han@xxxxxxxxx>
> >
> >ROM loading code is duplicated from assigned_dev_load_option_rom.
>
> Yes. Do you hint we need to split this?
Absolutely. These two drivers need to start sharing code.
> >Would it be a problem for you to create a memory region holding
> >the ROM?
>
> Sorry, could you tell me what should do exactly?
Split common code out and reuse :)
> >You won't need cpu_physical_memory_rw then, either, or need
>
> How to make sure this is fixed at 0xc0000?
>
> Thanks
> Tiejun
I guess you just add it as subregion of system memory at this offset,
maybe with a higher priority than RAM.
Right, Paolo?
> >the VGA_BIOS_DEFAULT_SIZE hack.
> >
> >
> >
> >
> >>---
> >>v5:
> >>
> >>* Just rebase.
> >>
> >>v4:
> >>
> >>* Fix some typos in the patch head description.
> >>* Improve some comments.
> >>* Given that xen_pt_register_vga_regions()/xen_pt_unregister_vga_regions()
> >> are called unconditionally, so we just return 0 there.
> >>* Remove one spurious change.
> >>
> >>v3:
> >>
> >>* Fix some typos.
> >>* Add more comments to make that readable.
> >>* Improve some return paths.
> >>
> >>v2:
> >>
> >>* retrieve VGA bios from sysfs properly.
> >>* redefine some function name.
> >>
> >> 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 | 232
> >> +++++++++++++++++++++++++++++++++++++++++++
> >> qemu-options.hx | 9 ++
> >> vl.c | 10 ++
> >> 8 files changed, 272 insertions(+), 1 deletion(-)
> >> create mode 100644 hw/xen/xen_pt_graphics.c
> >>
> >>diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
> >>index a0ca0aa..77b7dae 100644
> >>--- a/hw/xen/Makefile.objs
> >>+++ b/hw/xen/Makefile.objs
> >>@@ -2,4 +2,4 @@
> >> common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.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..dac4238 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);
> >> }
> >>
> >>+ xen_pt_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);
> >> }
> >>+
> >>+ xen_pt_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 passthrough GFX */
> >>+ if (xen_pt_setup_vga(&s->real_device) < 0) {
> >>+ XEN_PT_ERR(d, "Setup VGA BIOS of passthrough 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..4d3a18d 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 xen_has_gfx_passthru;
> >>+int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
> >>+int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
> >>+int xen_pt_setup_vga(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..461e526
> >>--- /dev/null
> >>+++ b/hw/xen/xen_pt_graphics.c
> >>@@ -0,0 +1,232 @@
> >>+/*
> >>+ * graphics passthrough
> >>+ */
> >>+#include "xen_pt.h"
> >>+#include "xen-host-pci-device.h"
> >>+#include "hw/xen/xen_backend.h"
> >>+
> >>+static int is_vga_passthrough(XenHostPCIDevice *dev)
> >>+{
> >>+ return (xen_has_gfx_passthru
> >>+ && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
> >>+}
> >>+
> >>+typedef struct VGARegion {
> >>+ int type; /* Memory or port I/O */
> >>+ uint64_t guest_base_addr;
> >>+ uint64_t machine_base_addr;
> >>+ uint64_t size; /* size of the region */
> >>+ int rc;
> >>+} VGARegion;
> >>+
> >>+#define IORESOURCE_IO 0x00000100
> >>+#define IORESOURCE_MEM 0x00000200
> >>+
> >>+static struct VGARegion vga_args[] = {
> >>+ {
> >>+ .type = IORESOURCE_IO,
> >>+ .guest_base_addr = 0x3B0,
> >>+ .machine_base_addr = 0x3B0,
> >>+ .size = 0xC,
> >>+ .rc = -1,
> >>+ },
> >>+ {
> >>+ .type = IORESOURCE_IO,
> >>+ .guest_base_addr = 0x3C0,
> >>+ .machine_base_addr = 0x3C0,
> >>+ .size = 0x20,
> >>+ .rc = -1,
> >>+ },
> >>+ {
> >>+ .type = IORESOURCE_MEM,
> >>+ .guest_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
> >>+ .machine_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
> >>+ .size = 0x20,
> >>+ .rc = -1,
> >>+ },
> >>+};
> >>+
> >>+/*
> >>+ * register VGA resources for the domain with assigned gfx
> >>+ */
> >>+int xen_pt_register_vga_regions(XenHostPCIDevice *dev)
> >>+{
> >>+ int i = 0;
> >>+
> >>+ if (!is_vga_passthrough(dev)) {
> >>+ return 0;
> >>+ }
> >>+
> >>+ for (i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
> >>+ if (vga_args[i].type == IORESOURCE_IO) {
> >>+ vga_args[i].rc = xc_domain_ioport_mapping(xen_xc, xen_domid,
> >>+ vga_args[i].guest_base_addr,
> >>+ vga_args[i].machine_base_addr,
> >>+ vga_args[i].size, DPCI_ADD_MAPPING);
> >>+ } else {
> >>+ vga_args[i].rc = xc_domain_memory_mapping(xen_xc, xen_domid,
> >>+ vga_args[i].guest_base_addr,
> >>+ vga_args[i].machine_base_addr,
> >>+ vga_args[i].size, DPCI_ADD_MAPPING);
> >>+ }
> >>+
> >>+ if (vga_args[i].rc) {
> >>+ XEN_PT_ERR(NULL, "VGA %s mapping failed! (rc: %i)\n",
> >>+ vga_args[i].type == IORESOURCE_IO ? "ioport" :
> >>"memory",
> >>+ vga_args[i].rc);
> >>+ return vga_args[i].rc;
> >>+ }
> >>+ }
> >>+
> >>+ return 0;
> >>+}
> >>+
> >>+/*
> >>+ * unregister VGA resources for the domain with assigned gfx
> >>+ */
> >>+int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)
> >>+{
> >>+ int i = 0;
> >>+
> >>+ if (!is_vga_passthrough(dev)) {
> >>+ return 0;
> >>+ }
> >>+
> >>+ for (i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
> >>+ if (vga_args[i].type == IORESOURCE_IO) {
> >>+ vga_args[i].rc = xc_domain_ioport_mapping(xen_xc, xen_domid,
> >>+ vga_args[i].guest_base_addr,
> >>+ vga_args[i].machine_base_addr,
> >>+ vga_args[i].size, DPCI_REMOVE_MAPPING);
> >>+ } else {
> >>+ vga_args[i].rc = xc_domain_memory_mapping(xen_xc, xen_domid,
> >>+ vga_args[i].guest_base_addr,
> >>+ vga_args[i].machine_base_addr,
> >>+ vga_args[i].size, DPCI_REMOVE_MAPPING);
> >>+ }
> >>+
> >>+ if (vga_args[i].rc) {
> >>+ XEN_PT_ERR(NULL, "VGA %s unmapping failed! (rc: %i)\n",
> >>+ vga_args[i].type == IORESOURCE_IO ? "ioport" :
> >>"memory",
> >>+ vga_args[i].rc);
> >>+ return vga_args[i].rc;
> >>+ }
> >>+ }
> >>+
> >>+ return 0;
> >>+}
> >>+
> >>+static int get_vgabios(unsigned char *buf, XenHostPCIDevice *dev)
> >>+{
> >>+ char rom_file[64];
> >>+ FILE *fp;
> >>+ uint8_t val;
> >>+ struct stat st;
> >>+ uint16_t magic = 0;
> >>+ int ret = 0;
> >>+
> >>+ snprintf(rom_file, sizeof(rom_file),
> >>+ "/sys/bus/pci/devices/%04x:%02x:%02x.%d/rom",
> >>+ dev->domain, dev->bus, dev->dev,
> >>+ dev->func);
> >>+
> >>+ if (stat(rom_file, &st)) {
> >>+ return -ENODEV;
> >>+ }
> >>+
> >>+ if (access(rom_file, F_OK)) {
> >>+ XEN_PT_ERR(NULL, "pci-assign: Insufficient privileges for %s",
> >>+ rom_file);
> >>+ return -ENODEV;
> >>+ }
> >>+
> >>+ /* Write "1" to the ROM file to enable it */
> >>+ fp = fopen(rom_file, "r+");
> >>+ if (fp == NULL) {
> >>+ return -EACCES;
> >>+ }
> >>+ val = 1;
> >>+ if (fwrite(&val, 1, 1, fp) != 1) {
> >>+ XEN_PT_LOG("%s\n", "Failed to enable pci-sysfs rom file");
> >>+ ret = -EIO;
> >>+ goto close_rom;
> >>+ }
> >>+ fseek(fp, 0, SEEK_SET);
> >>+
> >>+ /*
> >>+ * Check if it a real bios extension.
> >>+ * The magic number is 0xAA55.
> >>+ */
> >>+ if (!fread(&magic, sizeof(magic), 1, fp)) {
> >>+ XEN_PT_ERR(NULL, "VGA: can't get magic.\n");
> >>+ ret = -ENODEV;
> >>+ goto close_rom;
> >>+ }
> >>+ if (magic != 0xAA55) {
> >>+ XEN_PT_ERR(NULL, "VGA: wrong magic %x.\n", magic);
> >>+ ret = -ENODEV;
> >>+ goto close_rom;
> >>+ }
> >>+ fseek(fp, 0, SEEK_SET);
> >>+
> >>+ if (!fread(buf, 1, st.st_size, fp)) {
> >>+ XEN_PT_ERR(NULL, "VGA: pci-assign: Cannot read from host %s",
> >>rom_file);
> >>+ XEN_PT_LOG(NULL, "VGA: Device option ROM contents are probably
> >>invalid "
> >>+ "(check dmesg).\nSkip option ROM probe with rombar=0,
> >>"
> >>+ "or load from file with romfile=\n");
> >>+ }
> >>+
> >>+close_rom:
> >>+ /* Write "0" to disable ROM */
> >>+ fseek(fp, 0, SEEK_SET);
> >>+ val = 0;
> >>+ if (!fwrite(&val, 1, 1, fp)) {
> >>+ ret = -EIO;
> >>+ XEN_PT_LOG("%s\n", "Failed to disable pci-sysfs rom file");
> >>+ }
> >>+ fclose(fp);
> >>+ return ret;
> >>+}
> >>+
> >>+/* Allocated 128K for the vga bios */
> >>+#define VGA_BIOS_DEFAULT_SIZE (0x20000)
> >>+
> >>+int xen_pt_setup_vga(XenHostPCIDevice *dev)
> >>+{
> >>+ unsigned char *bios = NULL;
> >>+ int bios_size = 0;
> >>+ char *c = NULL;
> >>+ char checksum = 0;
> >>+ int rc = 0;
> >>+
> >>+ if (!is_vga_passthrough(dev)) {
> >>+ return rc;
> >>+ }
> >>+
> >>+ bios = g_malloc0(VGA_BIOS_DEFAULT_SIZE);
> >>+
> >>+ bios_size = get_vgabios(bios, dev);
> >>+ if (bios_size) {
> >>+ XEN_PT_ERR(NULL, "VGA: Error %d getting VBIOS!\n", bios_size);
> >>+ if (bios_size < 0) {
> >>+ XEN_PT_ERR(NULL, "VGA: Error (%s).\n", strerror(bios_size));
> >>+ }
> >>+ rc = -1;
> >>+ goto out;
> >>+ }
> >>+
> >>+ /* Adjust the bios checksum */
> >
> >It's easy to see that you do this here, but *why* do you do it?
> >That's what the comment should explain.
> >Do you see many ROMs with an invalid checksum
> >in the field?
> >
> >>+ 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");
> >>+ }
> >>+
> >>+ /* Currently we fixed this address as a primary. */
> >>+ cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
> >>+out:
> >>+ g_free(bios);
> >>+ return rc;
> >>+}
> >>diff --git a/qemu-options.hx b/qemu-options.hx
> >>index ff76ad4..1c77baa 100644
> >>--- a/qemu-options.hx
> >>+++ b/qemu-options.hx
> >>@@ -1066,6 +1066,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|tcx|cg3|none]\n"
> >> " select video card type\n", QEMU_ARCH_ALL)
> >>diff --git a/vl.c b/vl.c
> >>index a1686ef..c5b572d 100644
> >>--- a/vl.c
> >>+++ b/vl.c
> >>@@ -1418,6 +1418,13 @@ static void configure_msg(QemuOpts *opts)
> >> enable_timestamp_msg = qemu_opt_get_bool(opts, "timestamp", true);
> >> }
> >>
> >>+/* We still need this for compatibility with XEN. */
> >>+int xen_has_gfx_passthru;
> >>+static void xen_gfx_passthru(const char *optarg)
> >>+{
> >>+ xen_has_gfx_passthru = 1;
> >>+}
> >>+
> >> /***********************************************************/
> >> /* USB devices */
> >>
> >>@@ -3945,6 +3952,9 @@ int main(int argc, char **argv, char **envp)
> >> exit(1);
> >> }
> >> break;
> >>+ case QEMU_OPTION_gfx_passthru:
> >>+ xen_gfx_passthru(optarg);
> >>+ break;
> >> default:
> >> os_parse_cmd_args(popt->index, optarg);
> >> }
> >>--
> >>1.9.1
> >
> >
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |