| Message ID | CCCF29FA0F52DC4B8E2D1EF38AE07FF7B28712@SHSMSX101.ccr.corp.intel.com |
|---|---|
| State | New |
| Headers | show |
On Mon, May 19, 2014 at 09:42:23AM +0000, Chen, Tiejun wrote: > > -----Original Message----- > > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com] > > Sent: Friday, May 16, 2014 10:06 PM > > To: Chen, Tiejun > > Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com; > > mst@redhat.com; Kelly.Zytaruk@amd.com; peter.maydell@linaro.org; > > xen-devel@lists.xensource.com; weidong.han@intel.com; Kay, Allen M; > > qemu-devel@nongnu.org; jean.guyader@eu.citrix.com; > > anthony@codemonkey.ws; Zhang, Yang Z > > Subject: Re: [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics > > passthrough support > > > > [snip] > > > > +/* > > > + * register VGA resources for the domain with assigned gfx */ int > > > +xen_pt_register_vga_regions(XenHostPCIDevice *dev) { > > > + int ret = 0; > > > + > > > + if (!is_vga_passthrough(dev)) { > > > + 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) { > > > + XEN_PT_ERR(NULL, "VGA region mapping failed\n"); > > > > It would be actually useful to know _which_ of them failed. Perhaps you could > > structure this a bit differently and do: > > > > > > struct _args { > > uint32_t gport; > > uint32_t mport; > > uint32_t nport; > > }; > > > > struct _args args[3] = {{ 0x3B0, 0x3B0, 0xA }, {...}}; > > unsigned int i; > > > > for (i = 0; i < ARRAY_SIZE(args); i++) { > > ret = xc_domain_ioport_mapping(xen_xc, xen_domid, args[i].gport, > > args[i]..) > > if (ret) { > > XEN_PT_ERR_(NULL, "VGA region mapping of 0x%lx for 0x%x > > pages failed with rc:%d\n", > > ... fill in the values) > > return ret; > > } > > > > Looks good so what about this based on the original, > > --- a/hw/xen/xen_pt_graphics.c > +++ b/hw/xen/xen_pt_graphics.c > @@ -14,34 +14,73 @@ static int is_vga_passthrough(XenHostPCIDevice *dev) > && ((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 ret = 0; > + int i = 0; > > if (!is_vga_passthrough(dev)) { > - return ret; > + return -1; > } > > - 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); > + 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 (ret) { > - XEN_PT_ERR(NULL, "VGA region mapping failed\n"); > + 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 ret; > + return 0; > } > > /* > @@ -49,29 +88,33 @@ int xen_pt_register_vga_regions(XenHostPCIDevice *dev) > */ > int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev) > { > - int ret = 0; > + int i = 0; > > if (!is_vga_passthrough(dev)) { > - return ret; > + return -1; > } > > - 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); > + 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 (ret) { > - XEN_PT_ERR(NULL, "VGA region unmapping failed\n"); > + 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 ret; > + return 0; I think you still need to return a non-zero value in case of failure. > } > > static int get_vgabios(unsigned char *buf, XenHostPCIDevice *dev) > > > > > > > + } > > > + > > > + return ret; > > > +} > > > + > > > +/* > > > + * unregister VGA resources for the domain with assigned gfx */ int > > > +xen_pt_unregister_vga_regions(XenHostPCIDevice *dev) { > > > + int ret = 0; > > > + > > > + if (!is_vga_passthrough(dev)) { > > > + 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) { > > > + XEN_PT_ERR(NULL, "VGA region unmapping failed\n"); > > > + } > > > + > > > > The same pattern as above. > > > > See the above. > > > > + return ret; I think you still need to return a non-zero value in case of failure. > > > +} > > > + > > > +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; > > > + > > > + snprintf(rom_file, sizeof(rom_file), > > > + "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom", > > > > I think the format changed to be: /%04x:%02x:%02x.%d in Linux (see > > pci_setup_device in drivers/pci/probe.c) - not that it makes that much > > difference as the function is only up to 7. > > Will improved this as you pointed. > > > > > > + dev->domain, dev->bus, dev->dev, > > > + dev->func); > > > + > > > + if (stat(rom_file, &st)) { > > > + return -1; > > > > ENODEV? > > Fixed. > > > > > > + } > > > + > > > + if (access(rom_file, F_OK)) { > > > + XEN_PT_ERR(NULL, "pci-assign: Insufficient privileges for %s", > > > + rom_file); > > > + return -1; > > > > EPERM? > > Fixed. > > > > + } > > > + > > > + /* Write "1" to the ROM file to enable it */ > > > + fp = fopen(rom_file, "r+"); > > > + if (fp == NULL) { > > > > EACCESS ? > > > > Fixed. > > > > + return -1; > > > + } > > > + val = 1; > > > + if (fwrite(&val, 1, 1, fp) != 1) { > > > + 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)) { > > > + goto close_rom; > > > + } > > > > Don't you want to do that before you write '1' in it? > > > > Definitely, but I really did this above this line :) > > > > + > > > + if (magic != 0xAA55) { > > > + goto close_rom; > > > + } > > > + fseek(fp, 0, SEEK_SET); > > > + > > > + if (!fread(buf, 1, st.st_size, fp)) { > > > + XEN_PT_ERR(NULL, "pci-assign: Cannot read from host %s", > > rom_file); > > > + XEN_PT_LOG(NULL, "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)) { > > > + XEN_PT_LOG("%s\n", "Failed to disable pci-sysfs rom file"); > > > > Should we return -1? (after closing the file of course) > > > > Fixed. > > > > + } > > > + fclose(fp); > > > + return st.st_size; > > > > Ah, that is why your return -1! In that case I presume the caller of this function > > will check the 'errno' to find the underlaying issue > > I will modify something here and xen_pt_setup_vga(). Please see next version. > > Thanks > Tiejun >
Just resend since looks this delivery is delayed to these recipients or groups. Sorry for any inconveniences. Thanks Tiejun > -----Original Message----- > From: Chen, Tiejun > Sent: Tuesday, May 20, 2014 9:30 AM > To: 'Konrad Rzeszutek Wilk' > Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com; > mst@redhat.com; Kelly.Zytaruk@amd.com; peter.maydell@linaro.org; > xen-devel@lists.xensource.com; Kay, Allen M; qemu-devel@nongnu.org; > anthony@codemonkey.ws; Zhang, Yang Z > Subject: RE: [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics > passthrough support > > > -----Original Message----- > > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com] > > Sent: Monday, May 19, 2014 9:36 PM > > To: Chen, Tiejun > > Cc: anthony.perard@citrix.com; stefano.stabellini@eu.citrix.com; > > mst@redhat.com; Kelly.Zytaruk@amd.com; peter.maydell@linaro.org; > > xen-devel@lists.xensource.com; Kay, Allen M; qemu-devel@nongnu.org; > > anthony@codemonkey.ws; Zhang, Yang Z > > Subject: Re: [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic > > graphics passthrough support > > > > [snip] > > > > Looks good so what about this based on the original, > > > > > > --- a/hw/xen/xen_pt_graphics.c > > > +++ b/hw/xen/xen_pt_graphics.c > > > @@ -14,34 +14,73 @@ static int is_vga_passthrough(XenHostPCIDevice > > *dev) > > > && ((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 ret = 0; > > > + int i = 0; > > > > > > if (!is_vga_passthrough(dev)) { > > > - return ret; > > > + return -1; > > > } > > > > > > - 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); > > > + 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 (ret) { > > > - XEN_PT_ERR(NULL, "VGA region mapping failed\n"); > > > + 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 ret; > > > + return 0; > > > } > > > > > > /* > > > @@ -49,29 +88,33 @@ int > xen_pt_register_vga_regions(XenHostPCIDevice > > *dev) > > > */ > > > int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev) { > > > - int ret = 0; > > > + int i = 0; > > > > > > if (!is_vga_passthrough(dev)) { > > > - return ret; > > > + return -1; > > > } > > > > > > - 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); > > > + 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 (ret) { > > > - XEN_PT_ERR(NULL, "VGA region unmapping failed\n"); > > > + 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 ret; > > > + return 0; > > > > I think you still need to return a non-zero value in case of failure. > > > > Okay I will do this like, > > diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c index > 5603a8e..1342f4f 100644 > --- a/hw/xen/xen_pt_graphics.c > +++ b/hw/xen/xen_pt_graphics.c > @@ -77,6 +77,7 @@ int xen_pt_register_vga_regions(XenHostPCIDevice > *dev) > 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; > } > } > > @@ -111,6 +112,7 @@ int xen_pt_unregister_vga_regions(XenHostPCIDevice > *dev) > 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; > } > } > > Thanks > Tiejun
--- a/hw/xen/xen_pt_graphics.c +++ b/hw/xen/xen_pt_graphics.c @@ -14,34 +14,73 @@ static int is_vga_passthrough(XenHostPCIDevice *dev) && ((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 ret = 0; + int i = 0; if (!is_vga_passthrough(dev)) { - return ret; + return -1; } - 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); + 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 (ret) { - XEN_PT_ERR(NULL, "VGA region mapping failed\n"); + 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 ret; + return 0; } /* @@ -49,29 +88,33 @@ int xen_pt_register_vga_regions(XenHostPCIDevice *dev) */ int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev) { - int ret = 0; + int i = 0; if (!is_vga_passthrough(dev)) { - return ret; + return -1; } - 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); + 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 (ret) { - XEN_PT_ERR(NULL, "VGA region unmapping failed\n"); + 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 ret; + return 0; } static int get_vgabios(unsigned char *buf, XenHostPCIDevice *dev)