* [PATCH 1/2] [passthrough] Change init for pt_pci_host return value. @ 2012-01-17 16:40 Jean Guyader 2012-01-17 16:40 ` [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge Jean Guyader 0 siblings, 1 reply; 5+ messages in thread From: Jean Guyader @ 2012-01-17 16:40 UTC (permalink / raw) To: xen-devel; +Cc: Ian.Jackson, djmagee, allen.m.kay, Jean Guyader, Ross.Philipson [-- Attachment #1: Type: text/plain, Size: 238 bytes --] With an init of -1 all the return value smaller than a double word will be prefixed with "f"s. Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> --- hw/pass-through.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-passthrough-Change-init-for-pt_pci_host-return-value.patch --] [-- Type: text/x-patch; name="0001-passthrough-Change-init-for-pt_pci_host-return-value.patch", Size: 396 bytes --] diff --git a/hw/pass-through.c b/hw/pass-through.c index dbe8804..6a77657 100644 --- a/hw/pass-through.c +++ b/hw/pass-through.c @@ -2085,7 +2085,7 @@ struct pci_dev *pt_pci_get_dev(int bus, int dev, int fn) u32 pt_pci_host_read(struct pci_dev *pci_dev, u32 addr, int len) { - u32 val = -1; + u32 val = 0; pci_access_init(); pci_read_block(pci_dev, addr, (u8 *) &val, len); [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge. 2012-01-17 16:40 [PATCH 1/2] [passthrough] Change init for pt_pci_host return value Jean Guyader @ 2012-01-17 16:40 ` Jean Guyader 0 siblings, 0 replies; 5+ messages in thread From: Jean Guyader @ 2012-01-17 16:40 UTC (permalink / raw) To: xen-devel; +Cc: Ian.Jackson, djmagee, allen.m.kay, Jean Guyader, Ross.Philipson [-- Attachment #1: Type: text/plain, Size: 351 bytes --] Some versions of the Windows Intel GPU driver expect the vendor PCI capability to be there on the host bridge config space when passing through a Intel GPU. Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> --- hw/pt-graphics.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 54 insertions(+), 5 deletions(-) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0002-intel-gpu-passthrough-Expose-vendor-specific-pci-cap.patch --] [-- Type: text/x-patch; name="0002-intel-gpu-passthrough-Expose-vendor-specific-pci-cap.patch", Size: 2612 bytes --] diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c index fec7390..55cbf75 100644 --- a/hw/pt-graphics.c +++ b/hw/pt-graphics.c @@ -60,6 +60,53 @@ void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int l } } +#define PCI_INTEL_VENDOR_CAP 0x34 +#define PCI_INTEL_VENDOR_CAP_TYPE 0x09 +/* + * This function returns 0 is the value hasn't been + * updated. That mean the offset doesn't anything to + * do with the vendor capability. + */ +static uint32_t igd_pci_read_vendor_cap(PCIDevice *pci_dev, uint32_t config_addr, int len, + uint32_t *val) +{ + struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0); + uint32_t vendor_cap = 0; + uint32_t cap_type = 0; + uint32_t cap_size = 0; + + vendor_cap = pt_pci_host_read(pci_dev_host_bridge, PCI_INTEL_VENDOR_CAP, 1); + if (!vendor_cap) + return 0; + + cap_type = pt_pci_host_read(pci_dev_host_bridge, vendor_cap, 1); + if (cap_type != PCI_INTEL_VENDOR_CAP_TYPE) + return 0; + + if (config_addr == PCI_INTEL_VENDOR_CAP) + { + *val = vendor_cap; + return 1; + } + + /* Remove the next capability link */ + if (config_addr == vendor_cap + 1) + { + *val = 0; + return 1; + } + + cap_size = pt_pci_host_read(pci_dev_host_bridge, vendor_cap + 2, 1); + if (config_addr >= vendor_cap && + config_addr + len < vendor_cap + cap_size) + { + *val = pt_pci_host_read(pci_dev_host_bridge, config_addr, len); + return 1; + } + + return 0; +} + uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) { struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0); @@ -82,14 +129,16 @@ uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) case 0xa4: /* SNB: graphics base of stolen memory */ case 0xa8: /* SNB: base of GTT stolen memory */ val = pt_pci_host_read(pci_dev_host_bridge, config_addr, len); -#ifdef PT_DEBUG_PCI_CONFIG_ACCESS - PT_LOG_DEV(pci_dev, "addr=%x len=%x val=%x\n", - config_addr, len, val); -#endif break; default: - val = pci_default_read_config(pci_dev, config_addr, len); + if (!igd_pci_read_vendor_cap(pci_dev, config_addr, len, &val)) + val = pci_default_read_config(pci_dev, config_addr, len); + } +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS + PT_LOG_DEV(pci_dev, "addr=%x len=%x val=%x\n", + config_addr, len, val); +#endif return val; } [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 1/2] [passthrough] Change init for pt_pci_host return value. @ 2012-01-17 13:53 Jean Guyader 2012-01-17 13:53 ` [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge Jean Guyader 0 siblings, 1 reply; 5+ messages in thread From: Jean Guyader @ 2012-01-17 13:53 UTC (permalink / raw) To: xen-devel; +Cc: Ian.Jackson, djmagee, allen.m.kay, Jean Guyader, Ross.Philipson [-- Attachment #1: Type: text/plain, Size: 238 bytes --] With an init of -1 all the return value smaller than a double word will be prefixed with "f"s. Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> --- hw/pass-through.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-passthrough-Change-init-for-pt_pci_host-return-value.patch --] [-- Type: text/x-patch; name="0001-passthrough-Change-init-for-pt_pci_host-return-value.patch", Size: 396 bytes --] diff --git a/hw/pass-through.c b/hw/pass-through.c index dbe8804..6a77657 100644 --- a/hw/pass-through.c +++ b/hw/pass-through.c @@ -2085,7 +2085,7 @@ struct pci_dev *pt_pci_get_dev(int bus, int dev, int fn) u32 pt_pci_host_read(struct pci_dev *pci_dev, u32 addr, int len) { - u32 val = -1; + u32 val = 0; pci_access_init(); pci_read_block(pci_dev, addr, (u8 *) &val, len); [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge. 2012-01-17 13:53 [PATCH 1/2] [passthrough] Change init for pt_pci_host return value Jean Guyader @ 2012-01-17 13:53 ` Jean Guyader 2012-01-17 15:16 ` Stefano Stabellini 0 siblings, 1 reply; 5+ messages in thread From: Jean Guyader @ 2012-01-17 13:53 UTC (permalink / raw) To: xen-devel; +Cc: Ian.Jackson, djmagee, allen.m.kay, Jean Guyader, Ross.Philipson [-- Attachment #1: Type: text/plain, Size: 450 bytes --] Some versions of the Windows Intel GPU driver expect the vendor PCI capability to be there on the host bridge config space when passing through a Intel GPU. From: Ross Philipson <Ross.Philipson@citrix.com> Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> Acked-by: Ross Philipson <Ross.Philipson@citrix.com> --- hw/pt-graphics.c | 49 ++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 44 insertions(+), 5 deletions(-) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0002-intel-gpu-passthrough-Expose-vendor-specific-pci-cap.patch --] [-- Type: text/x-patch; name="0002-intel-gpu-passthrough-Expose-vendor-specific-pci-cap.patch", Size: 2478 bytes --] diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c index fec7390..25e28ff 100644 --- a/hw/pt-graphics.c +++ b/hw/pt-graphics.c @@ -60,6 +60,42 @@ void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int l } } +#define PCI_INTEL_VENDOR_CAP 0x34 +#define PCI_INTEL_VENDOR_CAP_TYPE 0x09 +static uint32_t igd_pci_read_vendor_cap(PCIDevice *pci_dev, uint32_t config_addr, int len, + uint32_t val) +{ + struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0); + uint32_t vendor_cap = 0; + uint32_t cap_type = 0; + uint32_t cap_size = 0; + + vendor_cap = pt_pci_host_read(pci_dev_host_bridge, PCI_INTEL_VENDOR_CAP, 1); + if (!vendor_cap) + return -1; + + cap_type = pt_pci_host_read(pci_dev_host_bridge, vendor_cap, 1); + if (cap_type != PCI_INTEL_VENDOR_CAP_TYPE) + return -1; + + if (config_addr == PCI_INTEL_VENDOR_CAP) + return vendor_cap; + + /* Remove the next capability link */ + if (config_addr == vendor_cap + 1) + return 0; + + cap_size = pt_pci_host_read(pci_dev_host_bridge, vendor_cap + 2, 1); + if (config_addr >= vendor_cap && + config_addr + len < vendor_cap + cap_size) + { + return pt_pci_host_read(pci_dev_host_bridge, config_addr, len); + } + + /* -1, this function doesn't deal with this config space offset */ + return -1; +} + uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) { struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0); @@ -82,14 +118,17 @@ uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) case 0xa4: /* SNB: graphics base of stolen memory */ case 0xa8: /* SNB: base of GTT stolen memory */ val = pt_pci_host_read(pci_dev_host_bridge, config_addr, len); -#ifdef PT_DEBUG_PCI_CONFIG_ACCESS - PT_LOG_DEV(pci_dev, "addr=%x len=%x val=%x\n", - config_addr, len, val); -#endif break; default: - val = pci_default_read_config(pci_dev, config_addr, len); + val = igd_pci_read_vendor_cap(pci_dev, config_addr, len, val); + if (val == -1) + val = pci_default_read_config(pci_dev, config_addr, len); + } +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS + PT_LOG_DEV(pci_dev, "addr=%x len=%x val=%x\n", + config_addr, len, val); +#endif return val; } [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge. 2012-01-17 13:53 ` [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge Jean Guyader @ 2012-01-17 15:16 ` Stefano Stabellini 2012-01-17 16:34 ` Jean Guyader 0 siblings, 1 reply; 5+ messages in thread From: Stefano Stabellini @ 2012-01-17 15:16 UTC (permalink / raw) To: Jean Guyader Cc: xen-devel@lists.xensource.com, Ian Jackson, allen.m.kay@intel.com, Jean Guyader, djmagee@mageenet.net, Ross Philipson On Tue, 17 Jan 2012, Jean Guyader wrote: > Some versions of the Windows Intel GPU driver expect the vendor > PCI capability to be there on the host bridge config space when > passing through a Intel GPU. > > From: Ross Philipson <Ross.Philipson@citrix.com> > Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> > Acked-by: Ross Philipson <Ross.Philipson@citrix.com> > > --- > hw/pt-graphics.c | 49 ++++++++++++++++++++++++++++++++++++++++++++----- > 1 files changed, 44 insertions(+), 5 deletions(-) > > > diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c > index fec7390..25e28ff 100644 > --- a/hw/pt-graphics.c > +++ b/hw/pt-graphics.c > @@ -60,6 +60,42 @@ void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int l > } > } > > +#define PCI_INTEL_VENDOR_CAP 0x34 > +#define PCI_INTEL_VENDOR_CAP_TYPE 0x09 > +static uint32_t igd_pci_read_vendor_cap(PCIDevice *pci_dev, uint32_t config_addr, int len, > + uint32_t val) > +{ > + struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0); > + uint32_t vendor_cap = 0; > + uint32_t cap_type = 0; > + uint32_t cap_size = 0; > + > + vendor_cap = pt_pci_host_read(pci_dev_host_bridge, PCI_INTEL_VENDOR_CAP, 1); > + if (!vendor_cap) > + return -1; > + > + cap_type = pt_pci_host_read(pci_dev_host_bridge, vendor_cap, 1); > + if (cap_type != PCI_INTEL_VENDOR_CAP_TYPE) > + return -1; > + > + if (config_addr == PCI_INTEL_VENDOR_CAP) > + return vendor_cap; > + > + /* Remove the next capability link */ > + if (config_addr == vendor_cap + 1) > + return 0; > + > + cap_size = pt_pci_host_read(pci_dev_host_bridge, vendor_cap + 2, 1); > + if (config_addr >= vendor_cap && > + config_addr + len < vendor_cap + cap_size) > + { > + return pt_pci_host_read(pci_dev_host_bridge, config_addr, len); > + } > + > + /* -1, this function doesn't deal with this config space offset */ > + return -1; > +} You are passing val to igd_pci_read_vendor_cap without actually using it. Also you are returning -1 from a function that returns uint32_t. I would change the prototype of igd_pci_read_vendor_cap to: static int igd_pci_read_vendor_cap(PCIDevice *pci_dev, uint32_t config_addr, int len, uint32_t *val) return only 0 or error and set *val to the correct value. > uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) > { > struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0); > @@ -82,14 +118,17 @@ uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len) > case 0xa4: /* SNB: graphics base of stolen memory */ > case 0xa8: /* SNB: base of GTT stolen memory */ > val = pt_pci_host_read(pci_dev_host_bridge, config_addr, len); > -#ifdef PT_DEBUG_PCI_CONFIG_ACCESS > - PT_LOG_DEV(pci_dev, "addr=%x len=%x val=%x\n", > - config_addr, len, val); > -#endif > break; > default: > - val = pci_default_read_config(pci_dev, config_addr, len); > + val = igd_pci_read_vendor_cap(pci_dev, config_addr, len, val); > + if (val == -1) > + val = pci_default_read_config(pci_dev, config_addr, len); > + > } > +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS > + PT_LOG_DEV(pci_dev, "addr=%x len=%x val=%x\n", > + config_addr, len, val); > +#endif > return val; > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge. 2012-01-17 15:16 ` Stefano Stabellini @ 2012-01-17 16:34 ` Jean Guyader 0 siblings, 0 replies; 5+ messages in thread From: Jean Guyader @ 2012-01-17 16:34 UTC (permalink / raw) To: Stefano Stabellini Cc: xen-devel@lists.xensource.com, Ian Jackson, allen.m.kay@intel.com, Jean Guyader, djmagee@mageenet.net, Ross Philipson On 17/01 03:16, Stefano Stabellini wrote: > On Tue, 17 Jan 2012, Jean Guyader wrote: > > Some versions of the Windows Intel GPU driver expect the vendor > > PCI capability to be there on the host bridge config space when > > passing through a Intel GPU. > > > > From: Ross Philipson <Ross.Philipson@citrix.com> > > Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> > > Acked-by: Ross Philipson <Ross.Philipson@citrix.com> > > > > --- > > hw/pt-graphics.c | 49 ++++++++++++++++++++++++++++++++++++++++++++----- > > 1 files changed, 44 insertions(+), 5 deletions(-) > > > > > > diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c > > index fec7390..25e28ff 100644 > > --- a/hw/pt-graphics.c > > +++ b/hw/pt-graphics.c > > @@ -60,6 +60,42 @@ void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, int l > > } > > } > > > > +#define PCI_INTEL_VENDOR_CAP 0x34 > > +#define PCI_INTEL_VENDOR_CAP_TYPE 0x09 > > +static uint32_t igd_pci_read_vendor_cap(PCIDevice *pci_dev, uint32_t config_addr, int len, > > + uint32_t val) > > +{ > > + struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0); > > + uint32_t vendor_cap = 0; > > + uint32_t cap_type = 0; > > + uint32_t cap_size = 0; > > + > > + vendor_cap = pt_pci_host_read(pci_dev_host_bridge, PCI_INTEL_VENDOR_CAP, 1); > > + if (!vendor_cap) > > + return -1; > > + > > + cap_type = pt_pci_host_read(pci_dev_host_bridge, vendor_cap, 1); > > + if (cap_type != PCI_INTEL_VENDOR_CAP_TYPE) > > + return -1; > > + > > + if (config_addr == PCI_INTEL_VENDOR_CAP) > > + return vendor_cap; > > + > > + /* Remove the next capability link */ > > + if (config_addr == vendor_cap + 1) > > + return 0; > > + > > + cap_size = pt_pci_host_read(pci_dev_host_bridge, vendor_cap + 2, 1); > > + if (config_addr >= vendor_cap && > > + config_addr + len < vendor_cap + cap_size) > > + { > > + return pt_pci_host_read(pci_dev_host_bridge, config_addr, len); > > + } > > + > > + /* -1, this function doesn't deal with this config space offset */ > > + return -1; > > +} > > You are passing val to igd_pci_read_vendor_cap without actually using > it. > Also you are returning -1 from a function that returns uint32_t. > > I would change the prototype of igd_pci_read_vendor_cap to: > > static int igd_pci_read_vendor_cap(PCIDevice *pci_dev, uint32_t config_addr, int len, > uint32_t *val) > > return only 0 or error and set *val to the correct value. > > > Thanks for the review. I'll update the patch and send a new series. Jean ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-01-17 16:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-01-17 16:40 [PATCH 1/2] [passthrough] Change init for pt_pci_host return value Jean Guyader 2012-01-17 16:40 ` [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge Jean Guyader -- strict thread matches above, loose matches on Subject: below -- 2012-01-17 13:53 [PATCH 1/2] [passthrough] Change init for pt_pci_host return value Jean Guyader 2012-01-17 13:53 ` [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge Jean Guyader 2012-01-17 15:16 ` Stefano Stabellini 2012-01-17 16:34 ` Jean Guyader
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).