Jean Guyader
2011-Nov-27 16:23 UTC
[PATCH] qemu-xen: Intel GPU passthrough, fix OpRegion mapping.
The OpRegion shouldn''t be mapped 1:1 because the address in the host can''t be used in the guest directly. This patch traps read and write access to the opregion of the Intel GPU config space (offset 0xfc). To work correctly this patch needs a change in hvmloader. HVMloader will allocate 2 pages for the OpRegion and write this address on the config space of the Intel GPU. Qemu will trap and map the host OpRegion to the guest. Any write to this offset after that won''t have any effect. Any read of this config space offset will return the address in the guest. Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> --- hw/pass-through.c | 8 ++-- hw/pass-through.h | 4 ++ hw/pt-graphics.c | 96 ++++++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 85 insertions(+), 23 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jean Guyader
2011-Dec-01 18:39 UTC
Re: [PATCH] qemu-xen: Intel GPU passthrough, fix OpRegion mapping.
On 27/11 04:23, Jean Guyader wrote:> > The OpRegion shouldn''t be mapped 1:1 because the address in the host > can''t be used in the guest directly. > > This patch traps read and write access to the opregion of the Intel > GPU config space (offset 0xfc). > > To work correctly this patch needs a change in hvmloader. > > HVMloader will allocate 2 pages for the OpRegion and write this address > on the config space of the Intel GPU. Qemu will trap and map the host > OpRegion to the guest. Any write to this offset after that won''t have > any effect. Any read of this config space offset will return the address > in the guest. >Ian/Allen, The pending change to hvmloader has been applied already to xen-unstable. Is the change alright with you? Jean
Ian Jackson
2011-Dec-01 18:42 UTC
Re: [PATCH] qemu-xen: Intel GPU passthrough, fix OpRegion mapping.
Jean Guyader writes ("[Xen-devel] [PATCH] qemu-xen: Intel GPU passthrough, fix OpRegion mapping."):> The OpRegion shouldn''t be mapped 1:1 because the address in the host > can''t be used in the guest directly.I confess that I don''t understand this area of the code at all. Would anyone like to comment on this patch ? Ian.
Stefano Stabellini
2011-Dec-02 11:53 UTC
Re: [PATCH] qemu-xen: Intel GPU passthrough, fix OpRegion mapping.
On Sun, 27 Nov 2011, Jean Guyader wrote:> The OpRegion shouldn''t be mapped 1:1 because the address in the host > can''t be used in the guest directly. > > This patch traps read and write access to the opregion of the Intel > GPU config space (offset 0xfc). > > To work correctly this patch needs a change in hvmloader. > > HVMloader will allocate 2 pages for the OpRegion and write this address > on the config space of the Intel GPU. Qemu will trap and map the host > OpRegion to the guest. Any write to this offset after that won''t have > any effect. Any read of this config space offset will return the address > in the guest. > > Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> > --- > hw/pass-through.c | 8 ++-- > hw/pass-through.h | 4 ++ > hw/pt-graphics.c | 96 ++++++++++++++++++++++++++++++++++++++++++---------- > 3 files changed, 85 insertions(+), 23 deletions(-) > > diff --git a/hw/pass-through.c b/hw/pass-through.c > index 919937f..976d28f 100644 > --- a/hw/pass-through.c > +++ b/hw/pass-through.c > @@ -1455,8 +1455,7 @@ out: > return index; > } > > -static void pt_pci_write_config(PCIDevice *d, uint32_t address, uint32_t val, > - int len) > +void pt_pci_write_config(PCIDevice *d, uint32_t address, uint32_t val, int len) > { > struct pt_dev *assigned_device = (struct pt_dev *)d; > struct pci_dev *pci_dev = assigned_device->pci_dev; > @@ -1637,7 +1636,7 @@ exit: > return; > } > > -static uint32_t pt_pci_read_config(PCIDevice *d, uint32_t address, int len) > +uint32_t pt_pci_read_config(PCIDevice *d, uint32_t address, int len) > { > struct pt_dev *assigned_device = (struct pt_dev *)d; > struct pci_dev *pci_dev = assigned_device->pci_dev; > @@ -4191,7 +4190,8 @@ static struct pt_dev * register_real_device(PCIBus *e_bus, > /* Register device */ > assigned_device = (struct pt_dev *) pci_register_device(e_bus, e_dev_name, > sizeof(struct pt_dev), e_devfn, > - pt_pci_read_config, pt_pci_write_config); > + gfx_passthru ? vgapt_pci_read_config : pt_pci_read_config, > + gfx_passthru ? vgapt_pci_write_config : pt_pci_write_config);Would it be possible to call the two vgapt_* functions from pt_pci_read/write_config? Maybe hooking them into pt_emu_reg_grp_tbl?> if ( assigned_device == NULL ) > { > PT_LOG("Error: couldn''t register real device\n"); > diff --git a/hw/pass-through.h b/hw/pass-through.h > index 884139c..c898c7c 100644 > --- a/hw/pass-through.h > +++ b/hw/pass-through.h > @@ -422,6 +422,10 @@ PCIBus *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, > uint16_t did, const char *name, uint16_t revision); > void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len); > uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len); > +void vgapt_pci_write_config(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len); > +uint32_t vgapt_pci_read_config(PCIDevice *pci_dev, uint32_t config_addr, int len); > +uint32_t pt_pci_read_config(PCIDevice *d, uint32_t address, int len); > +void pt_pci_write_config(PCIDevice *d, uint32_t address, uint32_t val, int len); > > #endif /* __PASSTHROUGH_H__ */ > > diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c > index fec7390..33cbff5 100644 > --- a/hw/pt-graphics.c > +++ b/hw/pt-graphics.c > @@ -13,6 +13,8 @@ > extern int gfx_passthru; > extern int igd_passthru; > > +static uint32_t igd_guest_opregion = 0; > + > static int pch_map_irq(PCIDevice *pci_dev, int irq_num) > { > PT_LOG("pch_map_irq called\n"); > @@ -37,6 +39,77 @@ void intel_pch_init(PCIBus *bus) > pch_map_irq, "intel_bridge_1f"); > } > > +static void igd_write_opregion(PCIDevice *pci_dev, uint32_t val, int len) > +{ > + struct pt_dev *real_dev = (struct pt_dev *)pci_dev; > + uint32_t host_opregion = 0; > + int ret; > + > + if ( igd_guest_opregion || len != 4 ) > + return;You should print a warning here.> + host_opregion = pt_pci_host_read(real_dev->pci_dev, > + PCI_INTEL_OPREGION, len); > + igd_guest_opregion = (val & ~0xfff) | (host_opregion & 0xfff); > + PT_LOG("Map OpRegion: %x -> %x\n", host_opregion, igd_guest_opregion); > + > + ret = xc_domain_memory_mapping(xc_handle, domid, > + igd_guest_opregion >> XC_PAGE_SHIFT, > + host_opregion >> XC_PAGE_SHIFT, > + 2, > + DPCI_ADD_MAPPING); > + > + if ( ret != 0 ) > + { > + PT_LOG("Error: Can''t map opregion\n"); > + igd_guest_opregion = 0; > + } > +} > + > +void vgapt_pci_write_config(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len) > +{ > +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS > + PT_LOG("vgapt_pci_write_config: %x:%x.%x: addr=%x len=%x val=%x\n", > + pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn), > + PCI_FUNC(pci_dev->devfn), config_addr, len, val); > +#endif > + > + if ( igd_passthru ) > + { > + switch ( config_addr ) > + { > + case 0xfc : /* Opregion */ > + igd_write_opregion(pci_dev, val, len); > + return; > + } > + }Shouldn''t you be calling igd_write_opregion only if vendor_id =PCI_VENDOR_ID_INTEL?> + pt_pci_write_config(pci_dev, config_addr, val, len); > +} > + > +uint32_t vgapt_pci_read_config(PCIDevice *pci_dev, uint32_t config_addr, int len) > +{ > + uint32_t val; > + > + val = pt_pci_read_config(pci_dev, config_addr, len); > + > + if ( igd_passthru ) > + { > + switch ( config_addr ) > + { > + case 0xfc: /* OpRegion */ > + if ( igd_guest_opregion ) > + val = igd_guest_opregion; > + break; > + } > + }Shouldn''t you call pt_pci_read_config only at the end, in case it is not igd_passthru or it is not config_addr == 0xfc?> +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS > + PT_LOG_DEV(pci_dev, "vgapt_pci_read_config: %x:%x.%x: addr=%x len=%x val=%x\n", > + config_addr, len, val); > +#endif > + return val; > +} > + > void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len) > { > struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0); > @@ -99,7 +172,6 @@ uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) > int register_vga_regions(struct pt_dev *real_device) > { > u16 vendor_id; > - int igd_opregion; > int ret = 0; > > if ( !gfx_passthru || real_device->pci_dev->device_class != 0x0300 ) > @@ -117,19 +189,6 @@ int register_vga_regions(struct pt_dev *real_device) > 0x20, > DPCI_ADD_MAPPING); > > - /* 1:1 map ASL Storage register value */ > - vendor_id = pt_pci_host_read(real_device->pci_dev, 0, 2); > - igd_opregion = pt_pci_host_read(real_device->pci_dev, PCI_INTEL_OPREGION, 4); > - if ( (vendor_id == PCI_VENDOR_ID_INTEL ) && igd_opregion ) > - { > - ret |= xc_domain_memory_mapping(xc_handle, domid, > - igd_opregion >> XC_PAGE_SHIFT, > - igd_opregion >> XC_PAGE_SHIFT, > - 2, > - DPCI_ADD_MAPPING); > - PT_LOG("register_vga: igd_opregion = %x\n", igd_opregion); > - } > - > if ( ret != 0 ) > PT_LOG("VGA region mapping failed\n"); > > @@ -141,7 +200,7 @@ int register_vga_regions(struct pt_dev *real_device) > */ > int unregister_vga_regions(struct pt_dev *real_device) > { > - u32 vendor_id, igd_opregion; > + u32 vendor_id; > int ret = 0; > > if ( !gfx_passthru || real_device->pci_dev->device_class != 0x0300 ) > @@ -160,12 +219,11 @@ int unregister_vga_regions(struct pt_dev *real_device) > DPCI_REMOVE_MAPPING); > > vendor_id = pt_pci_host_read(real_device->pci_dev, PCI_VENDOR_ID, 2); > - igd_opregion = pt_pci_host_read(real_device->pci_dev, PCI_INTEL_OPREGION, 4); > - if ( (vendor_id == PCI_VENDOR_ID_INTEL) && igd_opregion ) > + if ( (vendor_id == PCI_VENDOR_ID_INTEL) && igd_guest_opregion ) > { > ret |= xc_domain_memory_mapping(xc_handle, domid, > - igd_opregion >> XC_PAGE_SHIFT, > - igd_opregion >> XC_PAGE_SHIFT, > + igd_guest_opregion >> XC_PAGE_SHIFT, > + igd_guest_opregion >> XC_PAGE_SHIFT, > 2, > DPCI_REMOVE_MAPPING); > }
Konrad Rzeszutek Wilk
2011-Dec-12 16:32 UTC
Re: [PATCH] qemu-xen: Intel GPU passthrough, fix OpRegion mapping.
On Sun, Nov 27, 2011 at 04:23:26PM +0000, Jean Guyader wrote:> > The OpRegion shouldn''t be mapped 1:1 because the address in the host > can''t be used in the guest directly. > > This patch traps read and write access to the opregion of the Intel > GPU config space (offset 0xfc).> > To work correctly this patch needs a change in hvmloader. > > HVMloader will allocate 2 pages for the OpRegion and write this address > on the config space of the Intel GPU. Qemu will trap and map the host > OpRegion to the guest. Any write to this offset after that won''t have > any effect. Any read of this config space offset will return the address > in the guest. > > Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> > --- > hw/pass-through.c | 8 ++-- > hw/pass-through.h | 4 ++ > hw/pt-graphics.c | 96 ++++++++++++++++++++++++++++++++++++++++++---------- > 3 files changed, 85 insertions(+), 23 deletions(-) >> diff --git a/hw/pass-through.c b/hw/pass-through.c > index 919937f..976d28f 100644 > --- a/hw/pass-through.c > +++ b/hw/pass-through.c > @@ -1455,8 +1455,7 @@ out: > return index; > } > > -static void pt_pci_write_config(PCIDevice *d, uint32_t address, uint32_t val, > - int len) > +void pt_pci_write_config(PCIDevice *d, uint32_t address, uint32_t val, int len) > { > struct pt_dev *assigned_device = (struct pt_dev *)d; > struct pci_dev *pci_dev = assigned_device->pci_dev; > @@ -1637,7 +1636,7 @@ exit: > return; > } > > -static uint32_t pt_pci_read_config(PCIDevice *d, uint32_t address, int len) > +uint32_t pt_pci_read_config(PCIDevice *d, uint32_t address, int len) > { > struct pt_dev *assigned_device = (struct pt_dev *)d; > struct pci_dev *pci_dev = assigned_device->pci_dev; > @@ -4191,7 +4190,8 @@ static struct pt_dev * register_real_device(PCIBus *e_bus, > /* Register device */ > assigned_device = (struct pt_dev *) pci_register_device(e_bus, e_dev_name, > sizeof(struct pt_dev), e_devfn, > - pt_pci_read_config, pt_pci_write_config); > + gfx_passthru ? vgapt_pci_read_config : pt_pci_read_config, > + gfx_passthru ? vgapt_pci_write_config : pt_pci_write_config); > if ( assigned_device == NULL ) > { > PT_LOG("Error: couldn''t register real device\n"); > diff --git a/hw/pass-through.h b/hw/pass-through.h > index 884139c..c898c7c 100644 > --- a/hw/pass-through.h > +++ b/hw/pass-through.h > @@ -422,6 +422,10 @@ PCIBus *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, > uint16_t did, const char *name, uint16_t revision); > void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len); > uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len); > +void vgapt_pci_write_config(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len); > +uint32_t vgapt_pci_read_config(PCIDevice *pci_dev, uint32_t config_addr, int len); > +uint32_t pt_pci_read_config(PCIDevice *d, uint32_t address, int len); > +void pt_pci_write_config(PCIDevice *d, uint32_t address, uint32_t val, int len); > > #endif /* __PASSTHROUGH_H__ */ > > diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c > index fec7390..33cbff5 100644 > --- a/hw/pt-graphics.c > +++ b/hw/pt-graphics.c > @@ -13,6 +13,8 @@ > extern int gfx_passthru; > extern int igd_passthru; > > +static uint32_t igd_guest_opregion = 0; > + > static int pch_map_irq(PCIDevice *pci_dev, int irq_num) > { > PT_LOG("pch_map_irq called\n"); > @@ -37,6 +39,77 @@ void intel_pch_init(PCIBus *bus) > pch_map_irq, "intel_bridge_1f"); > } > > +static void igd_write_opregion(PCIDevice *pci_dev, uint32_t val, int len) > +{ > + struct pt_dev *real_dev = (struct pt_dev *)pci_dev; > + uint32_t host_opregion = 0; > + int ret; > + > + if ( igd_guest_opregion || len != 4 ) > + return; > + > + host_opregion = pt_pci_host_read(real_dev->pci_dev, > + PCI_INTEL_OPREGION, len);The tabs here are a bit weird.> + igd_guest_opregion = (val & ~0xfff) | (host_opregion & 0xfff); > + PT_LOG("Map OpRegion: %x -> %x\n", host_opregion, igd_guest_opregion); > + > + ret = xc_domain_memory_mapping(xc_handle, domid, > + igd_guest_opregion >> XC_PAGE_SHIFT, > + host_opregion >> XC_PAGE_SHIFT, > + 2, > + DPCI_ADD_MAPPING); > +Shouldn''t you unmap the older region first?> + if ( ret != 0 ) > + { > + PT_LOG("Error: Can''t map opregion\n"); > + igd_guest_opregion = 0; > + } > +} > + > +void vgapt_pci_write_config(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len) > +{ > +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS > + PT_LOG("vgapt_pci_write_config: %x:%x.%x: addr=%x len=%x val=%x\n", > + pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn), > + PCI_FUNC(pci_dev->devfn), config_addr, len, val); > +#endif > + > + if ( igd_passthru ) > + { > + switch ( config_addr ) > + { > + case 0xfc : /* Opregion */ > + igd_write_opregion(pci_dev, val, len); > + return;No default case? I would think the compiler would throw a fit for that.> + } > + } > + > + pt_pci_write_config(pci_dev, config_addr, val, len); > +} > + > +uint32_t vgapt_pci_read_config(PCIDevice *pci_dev, uint32_t config_addr, int len) > +{ > + uint32_t val; > + > + val = pt_pci_read_config(pci_dev, config_addr, len); > + > + if ( igd_passthru ) > + { > + switch ( config_addr ) > + { > + case 0xfc: /* OpRegion */ > + if ( igd_guest_opregion ) > + val = igd_guest_opregion; > + break; > + } > + } > +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS > + PT_LOG_DEV(pci_dev, "vgapt_pci_read_config: %x:%x.%x: addr=%x len=%x val=%x\n", > + config_addr, len, val); > +#endif > + return val; > +} > + > void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len) > { > struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0); > @@ -99,7 +172,6 @@ uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) > int register_vga_regions(struct pt_dev *real_device) > { > u16 vendor_id; > - int igd_opregion; > int ret = 0; > > if ( !gfx_passthru || real_device->pci_dev->device_class != 0x0300 ) > @@ -117,19 +189,6 @@ int register_vga_regions(struct pt_dev *real_device) > 0x20, > DPCI_ADD_MAPPING); > > - /* 1:1 map ASL Storage register value */ > - vendor_id = pt_pci_host_read(real_device->pci_dev, 0, 2); > - igd_opregion = pt_pci_host_read(real_device->pci_dev, PCI_INTEL_OPREGION, 4); > - if ( (vendor_id == PCI_VENDOR_ID_INTEL ) && igd_opregion ) > - { > - ret |= xc_domain_memory_mapping(xc_handle, domid, > - igd_opregion >> XC_PAGE_SHIFT, > - igd_opregion >> XC_PAGE_SHIFT, > - 2, > - DPCI_ADD_MAPPING); > - PT_LOG("register_vga: igd_opregion = %x\n", igd_opregion); > - } > - > if ( ret != 0 ) > PT_LOG("VGA region mapping failed\n"); > > @@ -141,7 +200,7 @@ int register_vga_regions(struct pt_dev *real_device) > */ > int unregister_vga_regions(struct pt_dev *real_device) > { > - u32 vendor_id, igd_opregion; > + u32 vendor_id; > int ret = 0; > > if ( !gfx_passthru || real_device->pci_dev->device_class != 0x0300 ) > @@ -160,12 +219,11 @@ int unregister_vga_regions(struct pt_dev *real_device) > DPCI_REMOVE_MAPPING); > > vendor_id = pt_pci_host_read(real_device->pci_dev, PCI_VENDOR_ID, 2); > - igd_opregion = pt_pci_host_read(real_device->pci_dev, PCI_INTEL_OPREGION, 4); > - if ( (vendor_id == PCI_VENDOR_ID_INTEL) && igd_opregion ) > + if ( (vendor_id == PCI_VENDOR_ID_INTEL) && igd_guest_opregion ) > { > ret |= xc_domain_memory_mapping(xc_handle, domid, > - igd_opregion >> XC_PAGE_SHIFT, > - igd_opregion >> XC_PAGE_SHIFT, > + igd_guest_opregion >> XC_PAGE_SHIFT, > + igd_guest_opregion >> XC_PAGE_SHIFT, > 2, > DPCI_REMOVE_MAPPING); > }> _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel
Jean Guyader
2012-Jan-12 12:37 UTC
Re: [PATCH] qemu-xen: Intel GPU passthrough, fix OpRegion mapping.
On 12 December 2011 16:32, Konrad Rzeszutek Wilk <konrad@darnok.org> wrote:> On Sun, Nov 27, 2011 at 04:23:26PM +0000, Jean Guyader wrote: >> >> The OpRegion shouldn''t be mapped 1:1 because the address in the host >> can''t be used in the guest directly. >> >> This patch traps read and write access to the opregion of the Intel >> GPU config space (offset 0xfc). > > >> >> To work correctly this patch needs a change in hvmloader. >> >> HVMloader will allocate 2 pages for the OpRegion and write this address >> on the config space of the Intel GPU. Qemu will trap and map the host >> OpRegion to the guest. Any write to this offset after that won''t have >> any effect. Any read of this config space offset will return the address >> in the guest. >> >> Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> >> --- >> hw/pass-through.c | 8 ++-- >> hw/pass-through.h | 4 ++ >> hw/pt-graphics.c | 96 ++++++++++++++++++++++++++++++++++++++++++---------- >> 3 files changed, 85 insertions(+), 23 deletions(-) >> > >> diff --git a/hw/pass-through.c b/hw/pass-through.c >> index 919937f..976d28f 100644 >> --- a/hw/pass-through.c >> +++ b/hw/pass-through.c >> @@ -1455,8 +1455,7 @@ out: >> return index; >> } >> >> -static void pt_pci_write_config(PCIDevice *d, uint32_t address, uint32_t val, >> - int len) >> +void pt_pci_write_config(PCIDevice *d, uint32_t address, uint32_t val, int len) >> { >> struct pt_dev *assigned_device = (struct pt_dev *)d; >> struct pci_dev *pci_dev = assigned_device->pci_dev; >> @@ -1637,7 +1636,7 @@ exit: >> return; >> } >> >> -static uint32_t pt_pci_read_config(PCIDevice *d, uint32_t address, int len) >> +uint32_t pt_pci_read_config(PCIDevice *d, uint32_t address, int len) >> { >> struct pt_dev *assigned_device = (struct pt_dev *)d; >> struct pci_dev *pci_dev = assigned_device->pci_dev; >> @@ -4191,7 +4190,8 @@ static struct pt_dev * register_real_device(PCIBus *e_bus, >> /* Register device */ >> assigned_device = (struct pt_dev *) pci_register_device(e_bus, e_dev_name, >> sizeof(struct pt_dev), e_devfn, >> - pt_pci_read_config, pt_pci_write_config); >> + gfx_passthru ? vgapt_pci_read_config : pt_pci_read_config, >> + gfx_passthru ? vgapt_pci_write_config : pt_pci_write_config); >> if ( assigned_device == NULL ) >> { >> PT_LOG("Error: couldn''t register real device\n"); >> diff --git a/hw/pass-through.h b/hw/pass-through.h >> index 884139c..c898c7c 100644 >> --- a/hw/pass-through.h >> +++ b/hw/pass-through.h >> @@ -422,6 +422,10 @@ PCIBus *intel_pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, >> uint16_t did, const char *name, uint16_t revision); >> void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len); >> uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len); >> +void vgapt_pci_write_config(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len); >> +uint32_t vgapt_pci_read_config(PCIDevice *pci_dev, uint32_t config_addr, int len); >> +uint32_t pt_pci_read_config(PCIDevice *d, uint32_t address, int len); >> +void pt_pci_write_config(PCIDevice *d, uint32_t address, uint32_t val, int len); >> >> #endif /* __PASSTHROUGH_H__ */ >> >> diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c >> index fec7390..33cbff5 100644 >> --- a/hw/pt-graphics.c >> +++ b/hw/pt-graphics.c >> @@ -13,6 +13,8 @@ >> extern int gfx_passthru; >> extern int igd_passthru; >> >> +static uint32_t igd_guest_opregion = 0; >> + >> static int pch_map_irq(PCIDevice *pci_dev, int irq_num) >> { >> PT_LOG("pch_map_irq called\n"); >> @@ -37,6 +39,77 @@ void intel_pch_init(PCIBus *bus) >> pch_map_irq, "intel_bridge_1f"); >> } >> >> +static void igd_write_opregion(PCIDevice *pci_dev, uint32_t val, int len) >> +{ >> + struct pt_dev *real_dev = (struct pt_dev *)pci_dev; >> + uint32_t host_opregion = 0; >> + int ret; >> + >> + if ( igd_guest_opregion || len != 4 ) >> + return; >> + >> + host_opregion = pt_pci_host_read(real_dev->pci_dev, >> + PCI_INTEL_OPREGION, len); > > The tabs here are a bit weird. >> + igd_guest_opregion = (val & ~0xfff) | (host_opregion & 0xfff); >> + PT_LOG("Map OpRegion: %x -> %x\n", host_opregion, igd_guest_opregion); >> + >> + ret = xc_domain_memory_mapping(xc_handle, domid, >> + igd_guest_opregion >> XC_PAGE_SHIFT, >> + host_opregion >> XC_PAGE_SHIFT, >> + 2, >> + DPCI_ADD_MAPPING); >> + > > Shouldn''t you unmap the older region first? >We don''t really need to. This region can''t be remapped. The trick I used here will only work once. Once the page has been mapped by hvmloader it will stay where it is. Jean>> + if ( ret != 0 ) >> + { >> + PT_LOG("Error: Can''t map opregion\n"); >> + igd_guest_opregion = 0; >> + } >> +} >> + >> +void vgapt_pci_write_config(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len) >> +{ >> +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS >> + PT_LOG("vgapt_pci_write_config: %x:%x.%x: addr=%x len=%x val=%x\n", >> + pci_bus_num(pci_dev->bus), PCI_SLOT(pci_dev->devfn), >> + PCI_FUNC(pci_dev->devfn), config_addr, len, val); >> +#endif >> + >> + if ( igd_passthru ) >> + { >> + switch ( config_addr ) >> + { >> + case 0xfc : /* Opregion */ >> + igd_write_opregion(pci_dev, val, len); >> + return; > > No default case? I would think the compiler would throw a fit for that. > >> + } >> + } >> + >> + pt_pci_write_config(pci_dev, config_addr, val, len); >> +} >> + >> +uint32_t vgapt_pci_read_config(PCIDevice *pci_dev, uint32_t config_addr, int len) >> +{ >> + uint32_t val; >> + >> + val = pt_pci_read_config(pci_dev, config_addr, len); >> + >> + if ( igd_passthru ) >> + { >> + switch ( config_addr ) >> + { >> + case 0xfc: /* OpRegion */ >> + if ( igd_guest_opregion ) >> + val = igd_guest_opregion; >> + break; >> + } >> + } >> +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS >> + PT_LOG_DEV(pci_dev, "vgapt_pci_read_config: %x:%x.%x: addr=%x len=%x val=%x\n", >> + config_addr, len, val); >> +#endif >> + return val; >> +} >> + >> void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int len) >> { >> struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0); >> @@ -99,7 +172,6 @@ uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) >> int register_vga_regions(struct pt_dev *real_device) >> { >> u16 vendor_id; >> - int igd_opregion; >> int ret = 0; >> >> if ( !gfx_passthru || real_device->pci_dev->device_class != 0x0300 ) >> @@ -117,19 +189,6 @@ int register_vga_regions(struct pt_dev *real_device) >> 0x20, >> DPCI_ADD_MAPPING); >> >> - /* 1:1 map ASL Storage register value */ >> - vendor_id = pt_pci_host_read(real_device->pci_dev, 0, 2); >> - igd_opregion = pt_pci_host_read(real_device->pci_dev, PCI_INTEL_OPREGION, 4); >> - if ( (vendor_id == PCI_VENDOR_ID_INTEL ) && igd_opregion ) >> - { >> - ret |= xc_domain_memory_mapping(xc_handle, domid, >> - igd_opregion >> XC_PAGE_SHIFT, >> - igd_opregion >> XC_PAGE_SHIFT, >> - 2, >> - DPCI_ADD_MAPPING); >> - PT_LOG("register_vga: igd_opregion = %x\n", igd_opregion); >> - } >> - >> if ( ret != 0 ) >> PT_LOG("VGA region mapping failed\n"); >> >> @@ -141,7 +200,7 @@ int register_vga_regions(struct pt_dev *real_device) >> */ >> int unregister_vga_regions(struct pt_dev *real_device) >> { >> - u32 vendor_id, igd_opregion; >> + u32 vendor_id; >> int ret = 0; >> >> if ( !gfx_passthru || real_device->pci_dev->device_class != 0x0300 ) >> @@ -160,12 +219,11 @@ int unregister_vga_regions(struct pt_dev *real_device) >> DPCI_REMOVE_MAPPING); >> >> vendor_id = pt_pci_host_read(real_device->pci_dev, PCI_VENDOR_ID, 2); >> - igd_opregion = pt_pci_host_read(real_device->pci_dev, PCI_INTEL_OPREGION, 4); >> - if ( (vendor_id == PCI_VENDOR_ID_INTEL) && igd_opregion ) >> + if ( (vendor_id == PCI_VENDOR_ID_INTEL) && igd_guest_opregion ) >> { >> ret |= xc_domain_memory_mapping(xc_handle, domid, >> - igd_opregion >> XC_PAGE_SHIFT, >> - igd_opregion >> XC_PAGE_SHIFT, >> + igd_guest_opregion >> XC_PAGE_SHIFT, >> + igd_guest_opregion >> XC_PAGE_SHIFT, >> 2, >> DPCI_REMOVE_MAPPING); >> } >
Maybe Matching Threads
- [PATCH] hvmloader / qemu-xen: Getting rid of resource conflict for OpRegion.
- PCI device assignment to guests (userspace)
- PCI device assignment to guests (userspace)
- "pt_iomul_init: Error: pt_iomul_init can't open file /dev/xen/pci_iomul: No such file or directory: 0x1:0x0.0x0". in qemu-dm-example.hvm.log
- Patch series for IGD passthrough