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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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).

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.

> @@ -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?

> +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.

Jan


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