Re: [PATCH v2 8/8] x86: enable CQM monitoring for each domain RMID

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> From: Dongxiao Xu <dongxiao.xu@intel.com>
>
> If the CQM service is attached to a domain, its related RMID will be set
> to hardware for monitoring when the domain's vcpu is scheduled in. When
> the domain's vcpu is scheduled out, RMID 0 (system reserved) will be set
> for monitoring.
>
> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> ---
>  xen/arch/x86/domain.c           |    5 +++++
>  xen/arch/x86/pqos.c             |   21 ++++++++++++++++++++-
>  xen/include/asm-x86/msr-index.h |    1 +
>  xen/include/asm-x86/pqos.h      |    1 +
>  4 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 9725649..1eda0ab 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1372,6 +1372,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);

So actions from the idle domain are accounted against the previously
scheduled vcpu?

>          p->arch.ctxt_switch_from(p);
>      }
>  
> @@ -1396,6 +1398,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 )

n->domain->arch.pqos_cqm_rmid can only be greater than 0 if the system
already supports cqm()

> +            cqm_assoc_rmid(n->domain->arch.pqos_cqm_rmid);

What happens to subsequent Xen accesses before returning to the guest?

What happens for Xen accesses in interrupt handlers?

>      }
>  
>      gdt = !is_pv_32on64_vcpu(n) ? per_cpu(gdt_table, cpu) :
> diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
> index d95784c..0cec172 100644
> --- a/xen/arch/x86/pqos.c
> +++ b/xen/arch/x86/pqos.c
> @@ -27,6 +27,7 @@
>  #include <asm/pqos.h>
>  
>  static bool_t pqos_enabled = 1;
> +static unsigned int rmid_bitwidth;
>  boolean_param("pqos", pqos_enabled);
>  
>  static void read_qm_data(void *arg)
> @@ -43,6 +44,14 @@ static void get_generic_qm_info(struct qm_element *qm_element)
>      on_selected_cpus(cpumask_of(cpu), read_qm_data, qm_element, 1);
>  }
>  
> +void cqm_assoc_rmid(uint16_t rmid)
> +{
> +    uint64_t val;
> +    uint64_t rmid_mask = ~(~0ull << rmid_bitwidth);
> +    rdmsrl(MSR_IA32_PQR_ASSOC, val);
> +    wrmsrl(MSR_IA32_PQR_ASSOC, (val & ~(rmid_mask)) | (rmid & rmid_mask));
> +}
> +
>  unsigned int cqm_res_count = 0;
>  unsigned int cqm_upscaling_factor = 0;
>  bool_t cqm_enabled = 0;
> @@ -78,13 +87,23 @@ static void __init init_cqm(void)
>  static void __init init_qos_monitor(void)
>  {
>      unsigned int qm_features;
> -    unsigned int eax, ebx, ecx;
> +    unsigned int eax, ebx, ecx, i = 0;
>  
>      if ( !(boot_cpu_has(X86_FEATURE_QOSM)) )
>          return;
>  
>      cpuid_count(0xf, 0, &eax, &ebx, &ecx, &qm_features);
>  
> +    while (1)
> +    {
> +        if ( (1 << i) - 1 >= ebx )
> +        {
> +            rmid_bitwidth = i;
> +            break;
> +        }
> +        i++;
> +    }

Is this some approximation a logarithm ?

Would it not be better to store rmid_mask (which would appear to be ebx)
rather than recalculate it from the constant rmid_bitwidth every time
cqm_assoc_rmid() is called ?

> +
>      if ( qm_features & QOS_MONITOR_TYPE_L3 )
>          init_cqm();
>  }
> diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> index 46ef165..45f4918 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -491,5 +491,6 @@
>  /* Platform QoS register */
>  #define MSR_IA32_QOSEVTSEL             0x00000c8d
>  #define MSR_IA32_QMC                   0x00000c8e
> +#define MSR_IA32_PQR_ASSOC             0x00000c8f
>  
>  #endif /* __ASM_MSR_INDEX_H */
> diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
> index 5c86c5d..2bb983b 100644
> --- a/xen/include/asm-x86/pqos.h
> +++ b/xen/include/asm-x86/pqos.h
> @@ -51,5 +51,6 @@ unsigned int get_cqm_count(void);
>  unsigned int get_cqm_avail(void);
>  void get_cqm_info(uint32_t rmid, cpumask_t cpu_cqmdata_map,
>                    struct xen_domctl_getdomcqminfo *info);
> +void cqm_assoc_rmid(uint16_t rmid);

rmid is inconsistently an int, u32 and u16 across this patch series. 
Can this be cleaned up.

~Andrew

>  
>  #endif