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: Monday, May 07, 2012 9:54 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 Sun, May 06, 2012 at 07:35:47AM +0000, Hao, Xudong wrote:
> >
> > > -----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.
> 
> What if the driver isn't doing that?

Make it clear, here host side do not be considered, host has its own driver. 
The only thing is to make sure ltr/obff is enabled before assigning guest, so 
that benefit on power.

Since device is owned by pciback again when it un-assigned from guest, we need 
not disable explicitly.
  
> >
> > > > 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.
> 
> Is the max value defined somewhere? Can it be defined somewhere so that
> both KVM adn the Xen patches re-use the same #define?
> 

Since there is not real device with ltr/obff supported now, so we can't 
determine the platform's maximum supported latency. I think the best way is 
using the default value 0, so I'd like to remove value setting. (if device is 
reset, the value still is 0)

> >
> > > > +
> > > >         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