thr3ads.net - Xen devel - [PATCH 1/2] [passthrough] Change init for pt_pci

If this information is useful, please help other people find it:
Share via:

Jean Guyader

2012-Jan-17 13:53 UTC

[PATCH 1/2] [passthrough] Change init for pt_pci_host return value.

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(-)



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jean Guyader

2012-Jan-17 13:53 UTC

head link

[PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge.

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(-)



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Stefano Stabellini

2012-Jan-17 14:59 UTC

head link

Re: [PATCH 1/2] [passthrough] Change init for pt_pci_host return value.

On Tue, 17 Jan 2012, Jean Guyader wrote:> With an init of -1 all the return value smaller than a double word
> will be prefixed with "f"s.
ack

Stefano Stabellini

2012-Jan-17 15:16 UTC

head link

Re: [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge.

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;
>  }
>

Jean Guyader

2012-Jan-17 16:34 UTC

head link

Re: [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge.

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

Jean Guyader

2012-Jan-17 16:40 UTC

head link

[PATCH 1/2] [passthrough] Change init for pt_pci_host return value.

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(-)



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Jean Guyader

2012-Jan-17 16:40 UTC

head link

[PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge.

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(-)



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Xen devel - Jan 2012 - [PATCH 1/2] [passthrough] Change init for pt_pci_host return value.

[PATCH 1/2] [passthrough] Change init for pt_pci_host return value.

[PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge.

Re: [PATCH 1/2] [passthrough] Change init for pt_pci_host return value.

Re: [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge.

Re: [PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge.

[PATCH 1/2] [passthrough] Change init for pt_pci_host return value.

[PATCH 2/2] intel gpu passthrough: Expose vendor specific pci cap on host bridge.