Re: [Xen-devel] [PATCH V4] Decouple SandyBridge quirk from VTd timeout

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Don Dugger [mailto:n0ano@xxxxxxxxx]
> Sent: Friday, December 05, 2014 2:13 PM
> 
> On Fri, Dec 05, 2014 at 05:30:52AM +0000, Tian, Kevin wrote:
> > > From: Donald D. Dugger
> > > Sent: Friday, November 21, 2014 1:12 PM
> > >
> > > Currently the quirk code for SandyBridge uses the VTd timeout value when
> > > writing to an IGD register.  This is the wrong timeout to use and, at
> > > 1000 msec., is also much too large.  This patch changes the quirk code
> > > to use a timeout that is specific to the IGD device and allows the user
> > > control of the timeout.
> > >
> > > Boolean settings for the boot parameter `snb_igd_quirk' keep their current
> > > meaning, enabling or disabling the quirk code with a timeout of 1000
> msec.
> > >
> > > In addition specifying `snb_igd_quirk=default' will enable the code and
> > > set the timeout to the theoretical maximum of 670 msec.  For finer
> control,
> > > specifying `snb_igd_quirk=n', where `n' is a decimal number, will enable
> > > the code and set the timeout to `n' msec.
> >
> > I'm OK with the patch except one comment. User usually thinks bool option
> > means default, but here an explicit 'default' actually means a different
> value.
> > It's a bit confused to me. How about changing 'default' to another more
> > meaningful word, e.g. 'max'? and you may still keep a 'snb_igd_quirk=
> > legacy' option, so all the possibilities are included in the assigned case, 
> > with
> > bool option alias to 'legacy'.
> 
> I don't see a need for a `legacy' option, merely enabling the option (which is
> what current users are doing) will select the current 1 sec. timeout.  If you
> don't like `default' (and I don't like `max' since that would select a value
> less that the current default) how about the string `cap' (the cap on the
> theoretical timeout) to select the 670 msec value?
> 

that looks fine to me. You can have my ACK for your next version. :-)

Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>


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