[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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?

>> > +  /* 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".

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.