Re: [Xen-devel] [PATCH v2] Re-enable LTR/OBFF when device is owned by pciback

["Hao, Xudong" <xudong.hao@xxxxxxxxx>]

> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@xxxxxxxxxx]
> Sent: Friday, May 04, 2012 9:21 PM
> To: Hao, Xudong
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH v2] Re-enable LTR/OBFF when device is owned
> by pciback
> 
> On Fri, May 04, 2012 at 07:49:21AM +0000, Hao, Xudong wrote:
> > When PCIE device which has LTR/OBFF capabilities is owned by pciback,
> LTR/OBFF feature may be disabled. This patch re-enable LTR and OBFF, so that
> guest with device assigned can be benefitted.
> >
> > Changes from v1:
> > - put the variable definition at the start of function
> > - add error log report
> >
> 
> Don't you need to disable this when the device is un-assigned from the guest?
> 

I don't think this need to be disabled when device is un-assigned from guest. 
If host want to use device again, the host device driver need re-load, so 
whether disabling ltr/obff is up to host device driver.

> > Signed-off-by: Xudong Hao <xudong.hao@xxxxxxxxx>
> >
> > diff --git a/drivers/xen/xen-pciback/pci_stub.c
> b/drivers/xen/xen-pciback/pci_stub.c
> > index 097e536..74fbf23 100644
> > --- a/drivers/xen/xen-pciback/pci_stub.c
> > +++ b/drivers/xen/xen-pciback/pci_stub.c
> > @@ -313,6 +313,10 @@ static int __devinit pcistub_init_device(struct
> pci_dev *dev)
> >     struct xen_pcibk_dev_data *dev_data;
> >     int err = 0;
> >
> > +   /* set default value */
> > +   unsigned long type = PCI_EXP_OBFF_SIGNAL_ALWAYS;
> > +   int snoop_lat_ns = 1024, nosnoop_lat_ns = 1024;
> 
> Why these values? Is there a way to fetch optimal values?

These values is the max value that the register supported. I've no idea to get 
an optimal value, here it just be set max value as default.

> > +
> >     dev_dbg(&dev->dev, "initializing...\n");
> >
> >     /* The PCI backend is not intended to be a module (or to work with
> > @@ -369,6 +373,28 @@ static int __devinit pcistub_init_device(struct
> pci_dev *dev)
> >     dev_dbg(&dev->dev, "reset device\n");
> >     xen_pcibk_reset_device(dev);
> >
> > +   /* Enable LTR and OBFF before do device assignment */
> > +   /* LTR(Latency tolerance reporting) allows devices to send
> > +    * messages to the root complex indicating their latency tolerance
> > +    * for snooped & unsnooped memory transactions.
> > +    */
> > +   err = pci_enable_ltr(dev);
> > +   if (err)
> > +           dev_err(&dev->dev, "Counld not enalbe LTR for device!\n");
> > +
> > +   err = pci_set_ltr(dev, snoop_lat_ns, nosnoop_lat_ns);
> > +   if (err)
> > +           dev_err(&dev->dev, "Set LTR latency values failed.\n");
> > +
> > +   /* OBFF (optimized buffer flush/fill), where supported, can help
> > +    * improve energy efficiency by giving devices information about
> > +    * when interrupts and other activity will have a reduced power
> > +    * impact.
> > +    */
> > +   err = pci_enable_obff(dev, type);
> > +   if (err)
> > +           dev_err(&dev->dev, "Counld not enalbe OBFF for device!\n");
> > +
> >     dev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
> >     return 0;
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel