Re: [PATCH v13 07/10] x86: enable QoS monitoring for each domain RMID

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

On 04/08/14 03:17, Dongxiao Xu wrote:
> If the QoS monitoring 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: Dongxiao Xu <dongxiao.xu@intel.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> ---
>  xen/arch/x86/domain.c           |  5 +++++
>  xen/arch/x86/pqos.c             | 26 ++++++++++++++++++++++++++
>  xen/include/asm-x86/msr-index.h |  3 +++
>  xen/include/asm-x86/pqos.h      |  1 +
>  4 files changed, 35 insertions(+)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index f8e0e33..bb9a4d2 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1398,6 +1398,8 @@ static void __context_switch(void)
>      {
>          memcpy(&p->arch.user_regs, stack_regs, CTXT_SWITCH_STACK_BYTES);
>          vcpu_save_fpu(p);
> +        if ( pqos_monitor_enabled() && pqosm->rmid_inuse )
> +            pqos_monitor_assoc_rmid(0);
>          p->arch.ctxt_switch_from(p);
>      }
>  
> @@ -1422,6 +1424,9 @@ static void __context_switch(void)
>          }
>          vcpu_restore_fpu_eager(n);
>          n->arch.ctxt_switch_to(n);
> +
> +        if ( pqos_monitor_enabled() && n->domain->arch.pqos_rmid > 0 )
> +            pqos_monitor_assoc_rmid(n->domain->arch.pqos_rmid);
>      }
>  
>      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 97b527f..46baaa5 100644
> --- a/xen/arch/x86/pqos.c
> +++ b/xen/arch/x86/pqos.c
> @@ -18,10 +18,16 @@
>  #include <xen/sched.h>
>  #include <asm/pqos.h>
>  
> +struct pqr_assoc {
> +    uint64_t val;
> +    bool_t initialized;
> +};
> +
>  struct pqos_monitor *__read_mostly pqosm = NULL;
>  static bool_t __initdata opt_pqos = 0;
>  static bool_t __initdata opt_pqos_monitor = 1;
>  static unsigned int __initdata opt_rmid_max = 255;
> +static DEFINE_PER_CPU(struct pqr_assoc, pqr_assoc);
>  
>  static void __init parse_pqos_param(char *s)
>  {
> @@ -157,6 +163,26 @@ void pqos_monitor_free_rmid(struct domain *d)
>      pqosm->rmid_inuse--;
>  }
>  
> +void pqos_monitor_assoc_rmid(unsigned int rmid)
> +{
> +    uint64_t val;
> +    uint64_t new_val;
> +
> +    if ( !this_cpu(pqr_assoc).initialized )

this_cpu() is specifically unoptimisable by the compiler.  Pull
prq_assoc out once at the start by pointer.

struct prq_assoc *pqr = &this_cpu(pqr_assoc);

> +    {
> +        rdmsrl(MSR_IA32_PQR_ASSOC, this_cpu(pqr_assoc).val);
> +        this_cpu(pqr_assoc).initialized = 1;
> +    }
> +    val = this_cpu(pqr_assoc).val;
> +
> +    new_val = (val & ~pqosm->rmid_mask) | (rmid & pqosm->rmid_mask);
> +    if ( val != new_val )
> +    {
> +        wrmsrl(MSR_IA32_PQR_ASSOC, new_val);
> +        this_cpu(pqr_assoc).val = new_val;
> +    }
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> index 70a8201..f894dfa 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -489,4 +489,7 @@
>  /* Geode defined MSRs */
>  #define MSR_GEODE_BUSCONT_CONF0		0x00001900
>  
> +/* Platform QoS MSRs */
> +#define MSR_IA32_PQR_ASSOC             0x00000c8f
> +

Tabs please, like the rest of the file.

~Andrew

>  #endif /* __ASM_MSR_INDEX_H */
> diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
> index b45c991..48978ea 100644
> --- a/xen/include/asm-x86/pqos.h
> +++ b/xen/include/asm-x86/pqos.h
> @@ -50,6 +50,7 @@ static inline bool_t pqos_monitor_enabled(void)
>  void init_platform_qos(void);
>  int pqos_monitor_alloc_rmid(struct domain *d);
>  void pqos_monitor_free_rmid(struct domain *d);
> +void pqos_monitor_assoc_rmid(unsigned int rmid);
>  
>  #endif /* __ASM_PQOS_H__ */
>