|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [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@xxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |