| Message ID | 1403662641-28526-6-git-send-email-tiejun.chen@intel.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 25, 2014 at 10:17:21AM +0800, 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> > --- > v5: > > * Nothing is changed. > > v4: > > * Nothing is changed. > > 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 > XEN_.... please PCI_CAP_MAX should be fixed too. > 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 f3fbfed..fa341ad 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 0; > @@ -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), don't spread casts all around. Should be a last resort. > + 3, > + DPCI_REMOVE_MAPPING); > + if (ret) { > + return ret; > + } > + } > + > return 0; > } > > @@ -447,3 +462,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) { !igd_guest_opregion is shorter and does the same, > + 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); > + Clearly broken on BE. Maybe not important here but writing clean code is just as easy. uint8_t igd_host_opregion[4]; ... xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION, igd_host_opregion, sizeof igd_host_opregion); igd_guest_opregion = (val & ~0xfff) | (pci_get_word(igd_host_opregion) & 0xfff); 0xfff should be a macro too to avoid duplication. > + 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)); > +} > -- > 1.9.1
On 2014/6/25 15:13, Michael S. Tsirkin wrote: > On Wed, Jun 25, 2014 at 10:17:21AM +0800, Tiejun Chen wrote: [snip] >> 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 >> > > XEN_.... please > > PCI_CAP_MAX should be fixed too. They are specific to PCI, not XEN. Why should we add such a prefix? > > [snip] >> >> + 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), > > don't spread casts all around. > Should be a last resort. Okay. > >> + 3, >> + DPCI_REMOVE_MAPPING); >> + if (ret) { >> + return ret; >> + } >> + } >> + >> return 0; >> } >> >> @@ -447,3 +462,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) { > > !igd_guest_opregion is shorter and does the same, Okay. > >> + 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); >> + > > Clearly broken on BE. I still can't understand why we need to address this in BE case. > Maybe not important here but writing clean code is > just as easy. > uint8_t igd_host_opregion[4]; > > ... > > xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION, > igd_host_opregion, sizeof igd_host_opregion); > > igd_guest_opregion = (val & ~0xfff) | > (pci_get_word(igd_host_opregion) & 0xfff); > > 0xfff should be a macro too to avoid duplication. > Okay. Thanks Tiejun
On Fri, Jun 27, 2014 at 05:22:18PM +0800, Chen, Tiejun wrote: > On 2014/6/25 15:13, Michael S. Tsirkin wrote: > >On Wed, Jun 25, 2014 at 10:17:21AM +0800, Tiejun Chen wrote: > > [snip] > > >>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 > >> > > > >XEN_.... please > > > >PCI_CAP_MAX should be fixed too. > > They are specific to PCI, not XEN. They are? Where in the PCI spec does it say 48? Same for PCI_INTEL_OPREGION. > Why should we add such a prefix? So that people working on core pci do not have to worry about breaking your devices by adding a symbol in the global header. > > > > > > [snip] > > >> > >>+ 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), > > > >don't spread casts all around. > >Should be a last resort. > > Okay. > > > > >>+ 3, > >>+ DPCI_REMOVE_MAPPING); > >>+ if (ret) { > >>+ return ret; > >>+ } > >>+ } > >>+ > >> return 0; > >> } > >> > >>@@ -447,3 +462,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) { > > > >!igd_guest_opregion is shorter and does the same, > > Okay. > > > > >>+ 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); > >>+ > > > >Clearly broken on BE. > > I still can't understand why we need to address this in BE case. So code is clean and reusable. Copy and paste is a fact of life, you don't want people to inherit bugs. If some code absolutely must be LE specific, it needs a comment that explains this and cautions people against trying to use it elsewhere in QEMU. > >Maybe not important here but writing clean code is > >just as easy. > >uint8_t igd_host_opregion[4]; > > > >... > > > > xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION, > > igd_host_opregion, sizeof igd_host_opregion); > > > > igd_guest_opregion = (val & ~0xfff) | > > (pci_get_word(igd_host_opregion) & 0xfff); > > > >0xfff should be a macro too to avoid duplication. > > > > Okay. > > Thanks > Tiejun
On 2014/6/29 19:43, Michael S. Tsirkin wrote: > On Fri, Jun 27, 2014 at 05:22:18PM +0800, Chen, Tiejun wrote: >> On 2014/6/25 15:13, Michael S. Tsirkin wrote: >>> On Wed, Jun 25, 2014 at 10:17:21AM +0800, Tiejun Chen wrote: >> >> [snip] >> >>>> 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 >>>> >>> >>> XEN_.... please >>> >>> PCI_CAP_MAX should be fixed too. >> >> They are specific to PCI, not XEN. > > They are? Where in the PCI spec does it say 48? > Same for PCI_INTEL_OPREGION. > >> Why should we add such a prefix? > > So that people working on core pci do not have to worry about breaking > your devices by adding a symbol in the global header. Okay. > > >>> >>> >> >> [snip] >> >>>> >>>> + 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), >>> >>> don't spread casts all around. >>> Should be a last resort. >> >> Okay. >> >>> >>>> + 3, >>>> + DPCI_REMOVE_MAPPING); >>>> + if (ret) { >>>> + return ret; >>>> + } >>>> + } >>>> + >>>> return 0; >>>> } >>>> >>>> @@ -447,3 +462,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) { >>> >>> !igd_guest_opregion is shorter and does the same, >> >> Okay. >> >>> >>>> + 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); >>>> + >>> >>> Clearly broken on BE. >> >> I still can't understand why we need to address this in BE case. > > So code is clean and reusable. Copy and paste is a fact of life, > you don't want people to inherit bugs. Understood. > If some code absolutely must be LE specific, > it needs a comment that explains this and cautions > people against trying to use it elsewhere in QEMU. I think its fine enough to add a comment. Thanks Tiejun > > >>> Maybe not important here but writing clean code is >>> just as easy. >>> uint8_t igd_host_opregion[4]; >>> >>> ... >>> >>> xen_host_pci_get_block(&s->real_device, PCI_INTEL_OPREGION, >>> igd_host_opregion, sizeof igd_host_opregion); >>> >>> igd_guest_opregion = (val & ~0xfff) | >>> (pci_get_word(igd_host_opregion) & 0xfff); >>> >>> 0xfff should be a macro too to avoid duplication. >>> >> >> Okay. >> >> Thanks >> Tiejun >
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 f3fbfed..fa341ad 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 0; @@ -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; } @@ -447,3 +462,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)); +}