Re: [Xen-devel] [PATCH v4 09/12] x86: add scheduling support for Intel CAT

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/04/2015 10:18, Chao Peng wrote:
> On context switch, write the the domain's Class of Service(COS) to MSR
> IA32_PQR_ASSOC, to notify hardware to use the new COS.
>
> For performance reason, the socket number and COS mask for current cpu
> is also cached in the local per-CPU variable.
>
> Signed-off-by: Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>
> ---
> Changes in v2:
> * merge common scheduling changes into scheduling improvement patch.
> * use readable expr for psra->cos_mask.
> ---
>  xen/arch/x86/psr.c | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 5247bcd..046229d 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -37,6 +37,8 @@ struct psr_cat_socket_info {
>  
>  struct psr_assoc {
>      uint64_t val;
> +    unsigned int socket;

psr_assoc is per-cpu.  Why does it need to cache its own socket like this?

If it does, please reorder socket and cos_mask to avoid the padding space.

> +    uint64_t cos_mask;
>  };
>  
>  struct psr_cmt *__read_mostly psr_cmt;
> @@ -206,9 +208,22 @@ void psr_free_rmid(struct domain *d)
>  
>  static inline void psr_assoc_init(unsigned int cpu)
>  {
> +    unsigned int socket;
> +    struct psr_cat_socket_info *info;
>      struct psr_assoc *psra = &per_cpu(psr_assoc, cpu);
>  
> -    if ( psr_cmt_enabled() )
> +    if ( cat_socket_info )
> +    {
> +        socket = cpu_to_socket(cpu);
> +        psra->socket = socket;
> +
> +        info = cat_socket_info + socket;
> +        if ( info->enabled )
> +            psra->cos_mask = ((1ull << get_count_order(info->cos_max)) - 1)
> +                             << 32;
> +    }
> +
> +    if ( psr_cmt_enabled() || psra->cos_mask )
>          rdmsrl(MSR_IA32_PSR_ASSOC, psra->val);
>  }
>  
> @@ -217,6 +232,12 @@ static inline void psr_assoc_rmid(uint64_t *reg, 
> unsigned int rmid)
>      *reg = (*reg & ~rmid_mask) | (rmid & rmid_mask);
>  }
>  
> +static inline void psr_assoc_cos(uint64_t *reg, unsigned int cos,
> +                                 uint64_t cos_mask)
> +{
> +    *reg = (*reg & ~cos_mask) | (((uint64_t)cos << 32) & cos_mask);
> +}
> +
>  void psr_ctxt_switch_to(struct domain *d)
>  {
>      struct psr_assoc *psra = &this_cpu(psr_assoc);
> @@ -225,11 +246,21 @@ void psr_ctxt_switch_to(struct domain *d)
>      if ( psr_cmt_enabled() )
>          psr_assoc_rmid(&reg, d->arch.psr_rmid);
>  
> +    if ( psra->cos_mask )
> +    {
> +        if ( d->arch.psr_cos_ids )
> +            psr_assoc_cos(&reg, d->arch.psr_cos_ids[psra->socket],
> +                          psra->cos_mask);
> +        else
> +            psr_assoc_cos(&reg, 0, psra->cos_mask);
> +    }

This can be collapsed somewhat to

if ( psra->cos_mask )
    psr_assoc_cos(&reg,  d->arch.psr_cos_ids ?
d->arch.psr_cos_ids[psra->socket] : 0, psra->cos_mask);

> +
>      if ( reg != psra->val )
>      {
>          wrmsrl(MSR_IA32_PSR_ASSOC, reg);
>          psra->val = reg;
>      }
> +

Spurious whitespace change.

~Andrew

>  }
>  
>  static int get_cat_socket_info(unsigned int socket,


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