Re: [Xen-devel] [PATCH v6 5/7] x86: enable CQM monitoring for each domain RMID

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

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Monday, January 20, 2014 9:58 PM
> To: Xu, Dongxiao
> Cc: andrew.cooper3@xxxxxxxxxx; dario.faggioli@xxxxxxxxxx;
> Ian.Campbell@xxxxxxxxxx; Ian.Jackson@xxxxxxxxxxxxx;
> stefano.stabellini@xxxxxxxxxxxxx; xen-devel@xxxxxxxxxxxxx;
> konrad.wilk@xxxxxxxxxx; dgdegra@xxxxxxxxxxxxx; keir@xxxxxxx
> Subject: Re: [PATCH v6 5/7] x86: enable CQM monitoring for each domain RMID
> 
> >>> On 05.12.13 at 10:38, Dongxiao Xu <dongxiao.xu@xxxxxxxxx> wrote:
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -1366,6 +1366,8 @@ static void __context_switch(void)
> >      {
> >          memcpy(&p->arch.user_regs, stack_regs,
> CTXT_SWITCH_STACK_BYTES);
> >          vcpu_save_fpu(p);
> > +        if ( system_supports_cqm() )
> > +            cqm_assoc_rmid(0);
> >          p->arch.ctxt_switch_from(p);
> >      }
> >
> > @@ -1390,6 +1392,9 @@ static void __context_switch(void)
> >          }
> >          vcpu_restore_fpu_eager(n);
> >          n->arch.ctxt_switch_to(n);
> > +
> > +        if ( system_supports_cqm() && n->domain->arch.pqos_cqm_rmid >
> 0 )
> > +            cqm_assoc_rmid(n->domain->arch.pqos_cqm_rmid);
> >      }
> >
> >      gdt = !is_pv_32on64_vcpu(n) ? per_cpu(gdt_table, cpu) :
> 
> The two uses here clearly call for system_supports_cqm() to
> be an inline function (the more that the variable checked in that
> function is already global anyway).

Okay.

> 
> Further, cqm_assoc_rmid() being an RDMSR plus WRMSR, you
> surely will want to optimize the case of p's and n's RMIDs being
> identical. Or at the very least make sure you _never_ call that
> function if all domains run with RMID 0.

Okay.

> 
> > @@ -60,6 +60,8 @@ static void __init parse_pqos_param(char *s)
> >
> >  custom_param("pqos", parse_pqos_param);
> >
> > +static uint64_t rmid_mask;
> 
> __read_mostly?

Okay.

> 
> > +void cqm_assoc_rmid(unsigned int rmid)
> > +{
> > +    uint64_t val;
> > +
> > +    rdmsrl(MSR_IA32_PQR_ASSOC, val);
> > +    wrmsrl(MSR_IA32_PQR_ASSOC, (val & ~(rmid_mask)) | (rmid &
> rmid_mask));
> 
> Stray parentheses around a simple variable.

Okay.

Will reflect your suggestions in next version patch.

Thanks,
Dongxiao

> 
> Jan


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