Message ID | 1401097389-4255-6-git-send-email-tiejun.chen@intel.com |
---|---|
State | New |
Headers | show |
On Mon, 26 May 2014, Tiejun Chen 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). > > The original patch is from Jean Guyader <jean.guyader@eu.citrix.com> > > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com> > Cc: Jean Guyader <jean.guyader@eu.citrix.com> > --- > v3: > > * Fix some typos. > * Add more comments to make that readable. > * To unmap igd_opregion when call xen_pt_unregister_vga_regions(). > * Improve some return paths. > * We need to map 3 pages for opregion as hvmloader set. > * Force to convert igd_guest/host_opoegion as an unsigned long type > while calling xc_domain_memory_mapping(). > > v2: > > * We should return zero as an invalid address value while calling > igd_read_opregion(). > > hw/xen/xen_pt.h | 4 ++- > hw/xen/xen_pt_config_init.c | 50 ++++++++++++++++++++++++++++++++++- > hw/xen/xen_pt_graphics.c | 64 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 116 insertions(+), 2 deletions(-) > > diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h > index 507165c..25147cf 100644 > --- a/hw/xen/xen_pt.h > +++ b/hw/xen/xen_pt.h > @@ -63,7 +63,7 @@ typedef int (*xen_pt_conf_byte_read) > #define XEN_PT_BAR_UNMAPPED (-1) > > #define PCI_CAP_MAX 48 > - > +#define PCI_INTEL_OPREGION 0xfc > > typedef enum { > XEN_PT_GRP_TYPE_HARDWIRED = 0, /* 0 Hardwired reg group */ > @@ -306,5 +306,7 @@ int pci_create_pch(PCIBus *bus); > 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); > +uint32_t igd_read_opregion(XenPCIPassthroughState *s); > +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val); > > #endif /* !XEN_PT_H */ > diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c > index de9a20f..6eaaa9a 100644 > --- a/hw/xen/xen_pt_config_init.c > +++ b/hw/xen/xen_pt_config_init.c > @@ -575,6 +575,22 @@ static int xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s, > return 0; > } > > +static int xen_pt_intel_opregion_read(XenPCIPassthroughState *s, > + XenPTReg *cfg_entry, > + uint32_t *value, uint32_t valid_mask) > +{ > + *value = igd_read_opregion(s); > + return 0; > +} > + > +static int xen_pt_intel_opregion_write(XenPCIPassthroughState *s, > + XenPTReg *cfg_entry, uint32_t *value, > + uint32_t dev_value, uint32_t valid_mask) > +{ > + igd_write_opregion(s, *value); > + return 0; > +} > + > /* Header Type0 reg static information table */ > static XenPTRegInfo xen_pt_emu_reg_header0[] = { > /* Vendor ID reg */ > @@ -1440,6 +1456,20 @@ static XenPTRegInfo xen_pt_emu_reg_msix[] = { > }, > }; > > +static XenPTRegInfo xen_pt_emu_reg_igd_opregion[] = { > + /* Intel IGFX OpRegion reg */ > + { > + .offset = 0x0, > + .size = 4, > + .init_val = 0, > + .no_wb = 1, > + .u.dw.read = xen_pt_intel_opregion_read, > + .u.dw.write = xen_pt_intel_opregion_write, > + }, > + { > + .size = 0, > + }, > +}; > > /**************************** > * Capabilities > @@ -1677,6 +1707,14 @@ static const XenPTRegGroupInfo xen_pt_emu_reg_grps[] = { > .size_init = xen_pt_msix_size_init, > .emu_regs = xen_pt_emu_reg_msix, > }, > + /* Intel IGD Opregion group */ > + { > + .grp_id = PCI_INTEL_OPREGION, > + .grp_type = XEN_PT_GRP_TYPE_EMU, > + .grp_size = 0x4, > + .size_init = xen_pt_reg_grp_size_init, > + .emu_regs = xen_pt_emu_reg_igd_opregion, > + }, > { > .grp_size = 0, > }, > @@ -1806,7 +1844,8 @@ int xen_pt_config_init(XenPCIPassthroughState *s) > uint32_t reg_grp_offset = 0; > XenPTRegGroup *reg_grp_entry = NULL; > > - if (xen_pt_emu_reg_grps[i].grp_id != 0xFF) { > + if (xen_pt_emu_reg_grps[i].grp_id != 0xFF > + && xen_pt_emu_reg_grps[i].grp_id != PCI_INTEL_OPREGION) { > if (xen_pt_hide_dev_cap(&s->real_device, > xen_pt_emu_reg_grps[i].grp_id)) { > continue; > @@ -1819,6 +1858,15 @@ int xen_pt_config_init(XenPCIPassthroughState *s) > } > } > > + /* > + * By default we will trap up to 0x40 in the cfg space. > + * If an intel device is pass through we need to trap 0xfc, > + * therefore the size should be 0xff. > + */ > + if (xen_pt_emu_reg_grps[i].grp_id == PCI_INTEL_OPREGION) { > + reg_grp_offset = PCI_INTEL_OPREGION; > + } > + > reg_grp_entry = g_new0(XenPTRegGroup, 1); > QLIST_INIT(®_grp_entry->reg_tbl_list); > QLIST_INSERT_HEAD(&s->reg_grps, reg_grp_entry, entries); > diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c > index 72e30ac..ee6fec0 100644 > --- a/hw/xen/xen_pt_graphics.c > +++ b/hw/xen/xen_pt_graphics.c > @@ -6,6 +6,9 @@ > #include "hw/xen/xen_backend.h" > #include "hw/pci/pci_bus.h" > > +static unsigned long igd_guest_opregion; > +static unsigned long igd_host_opregion; > + > static int is_vga_passthrough(XenHostPCIDevice *dev) > { > return (xen_has_gfx_passthru > @@ -88,6 +91,7 @@ int xen_pt_register_vga_regions(XenHostPCIDevice *dev) > int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev) > { > int i = 0; > + int ret = 0; > > if (!is_vga_passthrough(dev)) { > return -1; > @@ -114,6 +118,17 @@ int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev) > } > } > > + if (igd_guest_opregion) { > + ret = xc_domain_memory_mapping(xen_xc, xen_domid, > + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT), > + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT), > + 3, > + DPCI_REMOVE_MAPPING); > + if (ret) { > + return ret; > + } > + } > + > return 0; > } > > @@ -452,3 +467,52 @@ err_out: > XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n"); > return -1; > } > + > +uint32_t igd_read_opregion(XenPCIPassthroughState *s) > +{ > + uint32_t val = 0; > + > + if (igd_guest_opregion == 0) { > + return val; Sorry for not having spotted it before, but isn't this supposed to return an error? The old code returns -1 in this case. > + } > + > + val = igd_guest_opregion; > + > + XEN_PT_LOG(&s->dev, "Read opregion val=%x\n", val); > + return val; > +} > + > +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val) > +{ > + int ret; > + > + if (igd_guest_opregion) { > + XEN_PT_LOG(&s->dev, "opregion register already been set, ignoring %x\n", > + val); > + return; > + } > + > + xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION, > + (uint8_t *)&igd_host_opregion, 4); > + igd_guest_opregion = (unsigned long)(val & ~0xfff) > + | (igd_host_opregion & 0xfff); > + > + ret = xc_domain_memory_mapping(xen_xc, xen_domid, > + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT), > + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT), > + 3, Why 3 pages instead of 2? > + DPCI_ADD_MAPPING); > + > + if (ret) { > + XEN_PT_ERR(&s->dev, "[%d]:Can't map IGD host opregion:0x%lx to" > + " guest opregion:0x%lx.\n", ret, > + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT), > + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT)); > + igd_guest_opregion = 0; > + return; > + } > + > + XEN_PT_LOG(&s->dev, "Map OpRegion: 0x%lx -> 0x%lx\n", > + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT), > + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT)); > +} > -- > 1.9.1 >
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index 507165c..25147cf 100644 --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -63,7 +63,7 @@ typedef int (*xen_pt_conf_byte_read) #define XEN_PT_BAR_UNMAPPED (-1) #define PCI_CAP_MAX 48 - +#define PCI_INTEL_OPREGION 0xfc typedef enum { XEN_PT_GRP_TYPE_HARDWIRED = 0, /* 0 Hardwired reg group */ @@ -306,5 +306,7 @@ int pci_create_pch(PCIBus *bus); 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); +uint32_t igd_read_opregion(XenPCIPassthroughState *s); +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val); #endif /* !XEN_PT_H */ diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c index de9a20f..6eaaa9a 100644 --- a/hw/xen/xen_pt_config_init.c +++ b/hw/xen/xen_pt_config_init.c @@ -575,6 +575,22 @@ static int xen_pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s, return 0; } +static int xen_pt_intel_opregion_read(XenPCIPassthroughState *s, + XenPTReg *cfg_entry, + uint32_t *value, uint32_t valid_mask) +{ + *value = igd_read_opregion(s); + return 0; +} + +static int xen_pt_intel_opregion_write(XenPCIPassthroughState *s, + XenPTReg *cfg_entry, uint32_t *value, + uint32_t dev_value, uint32_t valid_mask) +{ + igd_write_opregion(s, *value); + return 0; +} + /* Header Type0 reg static information table */ static XenPTRegInfo xen_pt_emu_reg_header0[] = { /* Vendor ID reg */ @@ -1440,6 +1456,20 @@ static XenPTRegInfo xen_pt_emu_reg_msix[] = { }, }; +static XenPTRegInfo xen_pt_emu_reg_igd_opregion[] = { + /* Intel IGFX OpRegion reg */ + { + .offset = 0x0, + .size = 4, + .init_val = 0, + .no_wb = 1, + .u.dw.read = xen_pt_intel_opregion_read, + .u.dw.write = xen_pt_intel_opregion_write, + }, + { + .size = 0, + }, +}; /**************************** * Capabilities @@ -1677,6 +1707,14 @@ static const XenPTRegGroupInfo xen_pt_emu_reg_grps[] = { .size_init = xen_pt_msix_size_init, .emu_regs = xen_pt_emu_reg_msix, }, + /* Intel IGD Opregion group */ + { + .grp_id = PCI_INTEL_OPREGION, + .grp_type = XEN_PT_GRP_TYPE_EMU, + .grp_size = 0x4, + .size_init = xen_pt_reg_grp_size_init, + .emu_regs = xen_pt_emu_reg_igd_opregion, + }, { .grp_size = 0, }, @@ -1806,7 +1844,8 @@ int xen_pt_config_init(XenPCIPassthroughState *s) uint32_t reg_grp_offset = 0; XenPTRegGroup *reg_grp_entry = NULL; - if (xen_pt_emu_reg_grps[i].grp_id != 0xFF) { + if (xen_pt_emu_reg_grps[i].grp_id != 0xFF + && xen_pt_emu_reg_grps[i].grp_id != PCI_INTEL_OPREGION) { if (xen_pt_hide_dev_cap(&s->real_device, xen_pt_emu_reg_grps[i].grp_id)) { continue; @@ -1819,6 +1858,15 @@ int xen_pt_config_init(XenPCIPassthroughState *s) } } + /* + * By default we will trap up to 0x40 in the cfg space. + * If an intel device is pass through we need to trap 0xfc, + * therefore the size should be 0xff. + */ + if (xen_pt_emu_reg_grps[i].grp_id == PCI_INTEL_OPREGION) { + reg_grp_offset = PCI_INTEL_OPREGION; + } + reg_grp_entry = g_new0(XenPTRegGroup, 1); QLIST_INIT(®_grp_entry->reg_tbl_list); QLIST_INSERT_HEAD(&s->reg_grps, reg_grp_entry, entries); diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c index 72e30ac..ee6fec0 100644 --- a/hw/xen/xen_pt_graphics.c +++ b/hw/xen/xen_pt_graphics.c @@ -6,6 +6,9 @@ #include "hw/xen/xen_backend.h" #include "hw/pci/pci_bus.h" +static unsigned long igd_guest_opregion; +static unsigned long igd_host_opregion; + static int is_vga_passthrough(XenHostPCIDevice *dev) { return (xen_has_gfx_passthru @@ -88,6 +91,7 @@ int xen_pt_register_vga_regions(XenHostPCIDevice *dev) int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev) { int i = 0; + int ret = 0; if (!is_vga_passthrough(dev)) { return -1; @@ -114,6 +118,17 @@ int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev) } } + if (igd_guest_opregion) { + ret = xc_domain_memory_mapping(xen_xc, xen_domid, + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT), + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT), + 3, + DPCI_REMOVE_MAPPING); + if (ret) { + return ret; + } + } + return 0; } @@ -452,3 +467,52 @@ err_out: XEN_PT_ERR(pci_dev, "Can't get pci_dev_host_bridge\n"); return -1; } + +uint32_t igd_read_opregion(XenPCIPassthroughState *s) +{ + uint32_t val = 0; + + if (igd_guest_opregion == 0) { + return val; + } + + val = igd_guest_opregion; + + XEN_PT_LOG(&s->dev, "Read opregion val=%x\n", val); + return val; +} + +void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val) +{ + int ret; + + if (igd_guest_opregion) { + XEN_PT_LOG(&s->dev, "opregion register already been set, ignoring %x\n", + val); + return; + } + + xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION, + (uint8_t *)&igd_host_opregion, 4); + igd_guest_opregion = (unsigned long)(val & ~0xfff) + | (igd_host_opregion & 0xfff); + + ret = xc_domain_memory_mapping(xen_xc, xen_domid, + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT), + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT), + 3, + DPCI_ADD_MAPPING); + + if (ret) { + XEN_PT_ERR(&s->dev, "[%d]:Can't map IGD host opregion:0x%lx to" + " guest opregion:0x%lx.\n", ret, + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT), + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT)); + igd_guest_opregion = 0; + return; + } + + XEN_PT_LOG(&s->dev, "Map OpRegion: 0x%lx -> 0x%lx\n", + (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT), + (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT)); +}