|
[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 12 December 2011 16:32, Konrad Rzeszutek Wilk <konrad@xxxxxxxxxx> 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@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?
>
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);
>> }
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |