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

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

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Monday, May 07, 2012 3:25 PM
> To: Hao, Xudong
> Cc: xen-devel; konrad.wilk@xxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH v2] Re-enable LTR/OBFF when device is owned
> by pciback
> 
> >>> On 06.05.12 at 10:10, "Hao, Xudong" <xudong.hao@xxxxxxxxx> wrote:
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> >>> On 04.05.12 at 09:49, "Hao, Xudong" <xudong.hao@xxxxxxxxx> wrote:
> >> I continue to disagree to doing this always, unconditionally, with
> >> hard-coded settings. If these features require explicit enabling,
> >> there likely is something that drivers unaware of them could have
> >> problems with - otherwise I would think these features would be
> >> always enabled when a device supports them, and no options
> >> would exist to control certain parameters.
> >>
> > I agree the driver control it, but it's difficult to let guest driver do it
> > till now(because of PCIe emulation by qemu issue).
> 
> Then why not think about how to address this properly?
> 
I means the patch is the most appropriate method now. 

When qemu can emulate PCIe, then we shall migrate the method to guest driver 
controlling.

> >> > +        /* 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");
> >>
> >> ... dev_err() here and below is certainly too severe, particularly for
> >> the -ENOTSUPP cases. And the spelling would need fixing, especially
> >> with it being broken exactly the same way a second time below.
> >>
> >
> > dev_alert should be ok.
> 
> dev_alert() is even higher severity, whereas I was hinting towards
> lowering it (and perhaps being completely silent upon encountering
> -ENOTSUPP), i.e. dev_warn() or even dev_notice(), .
> 
> > Thanks for the careful review of typo "enable", I'll
> > modify it in next version.
> 
> Plus the one in "Could".
> 
Thanks.

> Jan


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