Re: [Xen-devel] [PATCH v11 4/9] x86: detect and initialize Platform QoS Monitoring feature

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Monday, June 23, 2014 2:50 PM
> To: Xu, Dongxiao
> Cc: andrew.cooper3@xxxxxxxxxx; Ian.Campbell@xxxxxxxxxx;
> George.Dunlap@xxxxxxxxxxxxx; Ian.Jackson@xxxxxxxxxxxxx;
> stefano.stabellini@xxxxxxxxxxxxx; xen-devel@xxxxxxxxxxxxx;
> konrad.wilk@xxxxxxxxxx; dgdegra@xxxxxxxxxxxxx; keir@xxxxxxx
> Subject: RE: [PATCH v11 4/9] x86: detect and initialize Platform QoS 
> Monitoring
> feature
> 
> >>> On 23.06.14 at 08:38, <dongxiao.xu@xxxxxxxxx> wrote:
> >>  -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: Friday, June 20, 2014 11:04 PM
> >> To: Xu, Dongxiao
> >> Cc: andrew.cooper3@xxxxxxxxxx; Ian.Campbell@xxxxxxxxxx;
> >> George.Dunlap@xxxxxxxxxxxxx; Ian.Jackson@xxxxxxxxxxxxx;
> >> stefano.stabellini@xxxxxxxxxxxxx; xen-devel@xxxxxxxxxxxxx;
> >> konrad.wilk@xxxxxxxxxx; dgdegra@xxxxxxxxxxxxx; keir@xxxxxxx
> >> Subject: Re: [PATCH v11 4/9] x86: detect and initialize Platform QoS
> > Monitoring
> >> feature
> >>
> >> >>> On 20.06.14 at 16:31, <dongxiao.xu@xxxxxxxxx> wrote:
> >> > Detect platform QoS feature status and enumerate the resource types,
> >> > one of which is to monitor the L3 cache occupancy.
> >> >
> >> > Also introduce a Xen grub command line parameter to control the
> >> > QoS feature status.
> >>
> >> "grub"?
> >
> > Sorry, what's your point here? Not quite understand it...
> 
> I'm asking what the mentioning of "grub" here means. Xen does have
> command line parameters, but they can equally well be used when
> booting without GrUB, e.g. directly from UEFI. IOW saying "grub" in
> a statement like this is at best confusing, unless talk is of an option
> that's only affecting booting via GrUB (e.g. something affecting the
> handling of multiboot, albeit even in that case nothing dictates that
> only GrUB can implement this protocol).

Okay, got your point.

> 
> >> > +void __init init_platform_qos(void)
> >> > +{
> >> > +    if ( !opt_pqos )
> >> > +        return;
> >> > +
> >> > +    if ( opt_pqos_monitor && opt_rmid_max )
> >>
> >> Kind of pointless to split the two if()-s.
> >
> > The split is reserved for the following QoS features, such as CQE, etc.
> 
> Since that isn't part of this series, I'd suggest avoiding such odd
> looking constructs (unless you already have a follow-up series in the
> works, and a reasonably certain it'll make 4.5). Future patches can
> always split the one if() into multiple.

Okay.

Thanks,
Dongxiao

> 
> Jan


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