Add a function to map a specific bar into a pt_dev. Add a function that gets called everytime the bar of a pass through device gets remap. Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> --- hw/pass-through.c | 34 ++++++++++++++++++++++++++++++++++ hw/pass-through.h | 3 +++ hw/pt-graphics.c | 7 +++++++ 3 files changed, 44 insertions(+), 0 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Reset Intel GPU fences when the domain starts (first mapping of Bar0). Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> --- hw/pt-graphics.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 133 insertions(+), 0 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2012-Feb-01 14:47 UTC
Re: [PATCH 1/2] qemu-xen: Pass through, utility functions
On Tue, 31 Jan 2012, Jean Guyader wrote:> Add a function to map a specific bar into a pt_dev. > > Add a function that gets called everytime the bar of a pass > through device gets remap. > > Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> > --- > hw/pass-through.c | 34 ++++++++++++++++++++++++++++++++++ > hw/pass-through.h | 3 +++ > hw/pt-graphics.c | 7 +++++++ > 3 files changed, 44 insertions(+), 0 deletions(-)Could you please send inline patches in the future?> diff --git a/hw/pass-through.c b/hw/pass-through.c > index dbe8804..1bdb223 100644 > --- a/hw/pass-through.c > +++ b/hw/pass-through.c > @@ -93,6 +93,7 @@ > #include <unistd.h> > #include <sys/ioctl.h> > #include <assert.h> > +#include <sys/mman.h> > > extern int gfx_passthru; > int igd_passthru = 0; > @@ -1155,6 +1156,9 @@ static void pt_iomem_map(PCIDevice *d, int i, uint32_t e_phys, uint32_t e_size, > if ( e_size == 0 ) > return; > > + if (assigned_device->pci_dev->device_class == 0x0300) > + pt_graphic_bar_remap(assigned_device, i, first_map, DPCI_ADD_MAPPING); > + > if ( !first_map && old_ebase != -1 ) > { > if ( has_msix_mapping(assigned_device, i) )Wouldn''t it be better if we move this into _pt_iomem_helper?> @@ -1969,6 +1973,9 @@ static void pt_unregister_regions(struct pt_dev *assigned_device) > if ( type == PCI_ADDRESS_SPACE_MEM || > type == PCI_ADDRESS_SPACE_MEM_PREFETCH ) > { > + if (assigned_device->pci_dev->device_class == 0x0300) > + pt_graphic_bar_remap(assigned_device, i, 0, DPCI_REMOVE_MAPPING); > + > ret = xc_domain_memory_mapping(xc_handle, domid, > assigned_device->bases[i].e_physbase >> XC_PAGE_SHIFT, > assigned_device->bases[i].access.maddr >> XC_PAGE_SHIFT, > > @@ -2101,6 +2108,33 @@ int pt_pci_host_write(struct pci_dev *pci_dev, u32 addr, u32 val, int len) > return ret; > } > > +int pt_pci_host_map_bar(struct pt_dev *pt_dev, int bar) > +{ > + int fd; > + struct stat s; > + uint8_t *map; > + > + char filename[1024]; > + > + snprintf(filename, 1024, "/sys/bus/pci/devices/0000:%02x:%02x.%01x/resource%d", > + pt_dev->pci_dev->bus, > + pt_dev->pci_dev->dev, > + pt_dev->pci_dev->func, > + bar); > + fd = open(filename, O_RDWR | O_SYNC); > + if (fd < 0) > + return fd; > + fstat(fd, &s); > + > + map = mmap(0, s.st_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); > + if (map != MAP_FAILED) > + { > + pt_dev->bases[bar].map = map; > + return 0; > + } > + return errno; > +} > + > /* parse BAR */ > static int pt_bar_reg_parse( > struct pt_dev *ptdev, struct pt_reg_info_tbl *reg) > diff --git a/hw/pass-through.h b/hw/pass-through.h > index 884139c..26e6ff1 100644 > --- a/hw/pass-through.h > +++ b/hw/pass-through.h > @@ -170,6 +170,7 @@ struct pt_region { > uint64_t pio_base; > uint64_t u; > } access; > + uint8_t *map; > }; > > struct pt_msi_info { > @@ -414,6 +415,7 @@ uint8_t pci_intx(struct pt_dev *ptdev); > struct pci_dev *pt_pci_get_dev(int bus, int dev, int func); > u32 pt_pci_host_read(struct pci_dev *pci_dev, u32 addr, int len); > int pt_pci_host_write(struct pci_dev *pci_dev, u32 addr, u32 val, int len); > +int pt_pci_host_map_bar(struct pt_dev *pt_dev, int bar); > void intel_pch_init(PCIBus *bus); > int register_vga_regions(struct pt_dev *real_device); > int unregister_vga_regions(struct pt_dev *real_device); > @@ -422,6 +424,7 @@ 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 pt_graphic_bar_remap(struct pt_dev *real_device, int bar, int first_map, int map);rename "int map" to "int op" for clarity> > #endif /* __PASSTHROUGH_H__ */ > > diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c > index fec7390..5d5e5da 100644 > --- a/hw/pt-graphics.c > +++ b/hw/pt-graphics.c > @@ -94,6 +94,13 @@ uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) > } > > /* > + * Callback whenever a bar get remapped > + */ > +void pt_graphic_bar_remap(struct pt_dev *real_device, int bar, int first_map, int map) > +{ > +} > + > +/* > * register VGA resources for the domain with assigned gfx > */ > int register_vga_regions(struct pt_dev *real_device)
On Tue, 31 Jan 2012, Jean Guyader wrote:> > Reset Intel GPU fences when the domain starts (first mapping > of Bar0). > > Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> > --- > hw/pt-graphics.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 133 insertions(+), 0 deletions(-) >inline patches please> diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c > index 5d5e5da..7403abe 100644 > --- a/hw/pt-graphics.c > +++ b/hw/pt-graphics.c > @@ -13,6 +13,31 @@ > extern int gfx_passthru; > extern int igd_passthru; > > +#define IGFX_CANTIGA 0x10 > +#define IGFX_IRONLAKE 0x20 > +#define IGFX_IBEXPEAK 0x30 > +#define IGFX_COUGARPOINT 0x40 > + > +struct igfx_chip > +{ > + int chip; > + uint16_t device_id; > +}; > + > +struct igfx_chip igfx_chips[] > +{ > + {IGFX_IRONLAKE, 0x0042}, > + {IGFX_IRONLAKE, 0x0046}, > + {IGFX_CANTIGA, 0x20e4}, > + {IGFX_CANTIGA, 0x2a42}, > + {IGFX_CANTIGA, 0x2e12}, > + {IGFX_COUGARPOINT, 0x0152}, > + {IGFX_COUGARPOINT, 0x0112}, > + {IGFX_COUGARPOINT, 0x0116}, > + {IGFX_COUGARPOINT, 0x0126}, > + {0, 0} > +}; > + > static int pch_map_irq(PCIDevice *pci_dev, int irq_num) > { > PT_LOG("pch_map_irq called\n"); > @@ -37,6 +62,98 @@ void intel_pch_init(PCIBus *bus) > pch_map_irq, "intel_bridge_1f"); > } > > +static int igd_get_chip(struct pt_dev *p) > +{ > + int i; > + int chip = 0; > + int devid = p->pci_dev->device_id; > + > + for (i = 0; igfx_chips[i].chip; i++) > + if (devid == igfx_chips[i].device_id) > + { > + chip = igfx_chips[i].chip; > + break; > + } > + > + if (!chip) > + { > + if (devid & 0x2000) > + chip = IGFX_CANTIGA; > + else if (devid & 0x100) > + chip = IGFX_COUGARPOINT; > + else > + chip = IGFX_IRONLAKE; > + PT_LOG("GUESS FOR CHIP 0x%04x as type %x", devid, chip); > + } > + return chip; > +} > + > + > +static uint32_t igd_mmio_read(struct pt_dev *p, uint32_t addr, uint8_t size) > +{ > + uint8_t *map = p->bases[0].map; > + uint32_t ret; > + > + switch (size) > + { > + case 1: > + ret = *(volatile uint8_t *)(map + addr); > + break; > + case 2: > + ret = *(volatile uint16_t *)(map + addr); > + break; > + case 4: > + ret = *(volatile uint32_t *)(map + addr); > + break; > + default: > + PT_LOG("igd_do_mmio: Unknown size %d\n", size); > + } > + return ret; > +}igd_mmio_read is currently unused> +static void igd_mmio_write(struct pt_dev *p, uint32_t addr, uint32_t val, > + uint8_t size) > +{ > + uint8_t *map = p->bases[0].map; > + > + switch (size) > + { > + case 1: > + *(volatile uint8_t *)(map + addr) = (uint8_t)val; > + break; > + case 2: > + *(volatile uint16_t *)(map + addr) = (uint16_t)val; > + break; > + case 4: > + *(volatile uint32_t *)(map + addr) = (uint32_t)val; > + break; > + default: > + PT_LOG("igd_do_mmio: Unknown size %d\n", size); > + } > +} > + > +static void igd_reset_fences(struct pt_dev *pt_dev) > +{ > + int i = 0; > + uint32_t fence_addr; > + > + switch (igd_get_chip(pt_dev)) > + { > + case IGFX_CANTIGA: > + case IGFX_IRONLAKE: > + case IGFX_IBEXPEAK: > + fence_addr = 0x3000; > + case IGFX_COUGARPOINT: > + fence_addr = 0x100000; > + } > + > + for (i = 0; i < 16; i++) > + { > + igd_mmio_write(pt_dev, fence_addr + (i << 4), 0, 4); > + igd_mmio_write(pt_dev, fence_addr + (i << 4) + 4, 0, 4); > + } > +} > + > 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); > @@ -98,6 +215,16 @@ uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) > */ > void pt_graphic_bar_remap(struct pt_dev *real_device, int bar, int first_map, int map) > { > + /* > + * Reset the fence register on the first remap > + * of Bar0 for a Intel GPU > + */ > + if (real_device->pci_dev->device_class == 0x0300 && > + real_device->pci_dev->device_id == PCI_VENDOR_ID_INTEL && > + bar == 0 && first_map && map == DPCI_ADD_MAPPING) > + { > + igd_reset_fences(real_device); > + } > }coding style, see http://git.savannah.gnu.org/cgit/qemu.git/tree/CODING_STYLE> /* > @@ -137,6 +264,12 @@ int register_vga_regions(struct pt_dev *real_device) > PT_LOG("register_vga: igd_opregion = %x\n", igd_opregion); > } > > + if (vendor_id == PCI_VENDOR_ID_INTEL) > + { > + if (pt_pci_host_map_bar(real_device, 0) != 0) > + PT_LOG("Can''t map Intel Bar 0\n"); > + }How fatal is this error? Maybe we need to return an error from register_vga_regions and propagate it upward?
Jean Guyader
2012-Feb-01 15:26 UTC
Re: [PATCH 1/2] qemu-xen: Pass through, utility functions
On 01/02 02:47, Stefano Stabellini wrote:> On Tue, 31 Jan 2012, Jean Guyader wrote: > > Add a function to map a specific bar into a pt_dev. > > > > Add a function that gets called everytime the bar of a pass > > through device gets remap. > > > > Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> > > --- > > hw/pass-through.c | 34 ++++++++++++++++++++++++++++++++++ > > hw/pass-through.h | 3 +++ > > hw/pt-graphics.c | 7 +++++++ > > 3 files changed, 44 insertions(+), 0 deletions(-) > > Could you please send inline patches in the future? > > > > diff --git a/hw/pass-through.c b/hw/pass-through.c > > index dbe8804..1bdb223 100644 > > --- a/hw/pass-through.c > > +++ b/hw/pass-through.c > > @@ -93,6 +93,7 @@ > > #include <unistd.h> > > #include <sys/ioctl.h> > > #include <assert.h> > > +#include <sys/mman.h> > > > > extern int gfx_passthru; > > int igd_passthru = 0; > > @@ -1155,6 +1156,9 @@ static void pt_iomem_map(PCIDevice *d, int i, uint32_t e_phys, uint32_t e_size, > > if ( e_size == 0 ) > > return; > > > > + if (assigned_device->pci_dev->device_class == 0x0300) > > + pt_graphic_bar_remap(assigned_device, i, first_map, DPCI_ADD_MAPPING); > > + > > if ( !first_map && old_ebase != -1 ) > > { > > if ( has_msix_mapping(assigned_device, i) ) > > Wouldn''t it be better if we move this into _pt_iomem_helper? > >It will be equivalent, but at least it will make the current pt_ioemu_map function smaller which isn''t a bad thing. Jean
On 01/02 02:52, Stefano Stabellini wrote:> On Tue, 31 Jan 2012, Jean Guyader wrote: > > > > Reset Intel GPU fences when the domain starts (first mapping > > of Bar0). > > > > Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> > > --- > > hw/pt-graphics.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 files changed, 133 insertions(+), 0 deletions(-) > > > inline patches please > > > > diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c > > index 5d5e5da..7403abe 100644 > > --- a/hw/pt-graphics.c > > +++ b/hw/pt-graphics.c > > @@ -13,6 +13,31 @@ > > extern int gfx_passthru; > > extern int igd_passthru; > > > > +#define IGFX_CANTIGA 0x10 > > +#define IGFX_IRONLAKE 0x20 > > +#define IGFX_IBEXPEAK 0x30 > > +#define IGFX_COUGARPOINT 0x40 > > + > > +struct igfx_chip > > +{ > > + int chip; > > + uint16_t device_id; > > +}; > > + > > +struct igfx_chip igfx_chips[] > > +{ > > + {IGFX_IRONLAKE, 0x0042}, > > + {IGFX_IRONLAKE, 0x0046}, > > + {IGFX_CANTIGA, 0x20e4}, > > + {IGFX_CANTIGA, 0x2a42}, > > + {IGFX_CANTIGA, 0x2e12}, > > + {IGFX_COUGARPOINT, 0x0152}, > > + {IGFX_COUGARPOINT, 0x0112}, > > + {IGFX_COUGARPOINT, 0x0116}, > > + {IGFX_COUGARPOINT, 0x0126}, > > + {0, 0} > > +}; > > + > > static int pch_map_irq(PCIDevice *pci_dev, int irq_num) > > { > > PT_LOG("pch_map_irq called\n"); > > @@ -37,6 +62,98 @@ void intel_pch_init(PCIBus *bus) > > pch_map_irq, "intel_bridge_1f"); > > } > > > > +static int igd_get_chip(struct pt_dev *p) > > +{ > > + int i; > > + int chip = 0; > > + int devid = p->pci_dev->device_id; > > + > > + for (i = 0; igfx_chips[i].chip; i++) > > + if (devid == igfx_chips[i].device_id) > > + { > > + chip = igfx_chips[i].chip; > > + break; > > + } > > + > > + if (!chip) > > + { > > + if (devid & 0x2000) > > + chip = IGFX_CANTIGA; > > + else if (devid & 0x100) > > + chip = IGFX_COUGARPOINT; > > + else > > + chip = IGFX_IRONLAKE; > > + PT_LOG("GUESS FOR CHIP 0x%04x as type %x", devid, chip); > > + } > > + return chip; > > +} > > + > > + > > +static uint32_t igd_mmio_read(struct pt_dev *p, uint32_t addr, uint8_t size) > > +{ > > + uint8_t *map = p->bases[0].map; > > + uint32_t ret; > > + > > + switch (size) > > + { > > + case 1: > > + ret = *(volatile uint8_t *)(map + addr); > > + break; > > + case 2: > > + ret = *(volatile uint16_t *)(map + addr); > > + break; > > + case 4: > > + ret = *(volatile uint32_t *)(map + addr); > > + break; > > + default: > > + PT_LOG("igd_do_mmio: Unknown size %d\n", size); > > + } > > + return ret; > > +} > > igd_mmio_read is currently unused > > > > +static void igd_mmio_write(struct pt_dev *p, uint32_t addr, uint32_t val, > > + uint8_t size) > > +{ > > + uint8_t *map = p->bases[0].map; > > + > > + switch (size) > > + { > > + case 1: > > + *(volatile uint8_t *)(map + addr) = (uint8_t)val; > > + break; > > + case 2: > > + *(volatile uint16_t *)(map + addr) = (uint16_t)val; > > + break; > > + case 4: > > + *(volatile uint32_t *)(map + addr) = (uint32_t)val; > > + break; > > + default: > > + PT_LOG("igd_do_mmio: Unknown size %d\n", size); > > + } > > +} > > + > > +static void igd_reset_fences(struct pt_dev *pt_dev) > > +{ > > + int i = 0; > > + uint32_t fence_addr; > > + > > + switch (igd_get_chip(pt_dev)) > > + { > > + case IGFX_CANTIGA: > > + case IGFX_IRONLAKE: > > + case IGFX_IBEXPEAK: > > + fence_addr = 0x3000; > > + case IGFX_COUGARPOINT: > > + fence_addr = 0x100000; > > + } > > + > > + for (i = 0; i < 16; i++) > > + { > > + igd_mmio_write(pt_dev, fence_addr + (i << 4), 0, 4); > > + igd_mmio_write(pt_dev, fence_addr + (i << 4) + 4, 0, 4); > > + } > > +} > > + > > 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); > > @@ -98,6 +215,16 @@ uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) > > */ > > void pt_graphic_bar_remap(struct pt_dev *real_device, int bar, int first_map, int map) > > { > > + /* > > + * Reset the fence register on the first remap > > + * of Bar0 for a Intel GPU > > + */ > > + if (real_device->pci_dev->device_class == 0x0300 && > > + real_device->pci_dev->device_id == PCI_VENDOR_ID_INTEL && > > + bar == 0 && first_map && map == DPCI_ADD_MAPPING) > > + { > > + igd_reset_fences(real_device); > > + } > > } > > coding style, see http://git.savannah.gnu.org/cgit/qemu.git/tree/CODING_STYLE > > > > /* > > @@ -137,6 +264,12 @@ int register_vga_regions(struct pt_dev *real_device) > > PT_LOG("register_vga: igd_opregion = %x\n", igd_opregion); > > } > > > > + if (vendor_id == PCI_VENDOR_ID_INTEL) > > + { > > + if (pt_pci_host_map_bar(real_device, 0) != 0) > > + PT_LOG("Can''t map Intel Bar 0\n"); > > + } > > How fatal is this error? Maybe we need to return an error from > register_vga_regions and propagate it upward?It''s not actually fatal. If you don''t reset the fences the VESA buffer in the guest will look corrupted but appart for that it will work fine. So I think I will just make my code silent if the map isn''t mapped. Jean
Hi Jean,
I have following questions about the patch:
1) How is the new function pt_pci_host_map_bar() used? I see function
definition but do not see where it is called.
2) What is the purpose of pt_graphic_bar_remap()? It seems to be defined as
NULL as shown below.
/*
+ * Callback whenever a bar get remapped
+ */
+void pt_graphic_bar_remap(struct pt_dev *real_device, int bar, int first_map,
int map)
+{
+}
Allen
-----Original Message-----
From: Jean Guyader [mailto:jean.guyader@eu.citrix.com]
Sent: Tuesday, January 31, 2012 10:40 AM
To: xen-devel@lists.xensource.com
Cc: Kay, Allen M; Ian.Jackson@eu.citrix.com; Jean Guyader
Subject: [PATCH 2/2] qemu-xen: Intel GPU passthrough
Reset Intel GPU fences when the domain starts (first mapping of Bar0).
Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
hw/pt-graphics.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 133 insertions(+), 0 deletions(-)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel