Re: [PATCH v4 06/12] x86: add COS information for each domain

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

On 09/04/2015 10:18, Chao Peng wrote:
> In Xen's implementation, the CAT enforcement granularity is per domain.
> Due to the length of CBM and the number of COS may be socket-different,
> each domain has COS ID for each socket. The domain get COS=0 by default
> and at runtime its COS is then allocated dynamically when user specifies
> a CBM for the domain.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
>  xen/arch/x86/domain.c        |  6 +++++-
>  xen/arch/x86/psr.c           | 42 ++++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-x86/domain.h |  5 ++++-
>  xen/include/asm-x86/psr.h    |  3 +++
>  4 files changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 695a2eb..129d42f 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -616,6 +616,9 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
>          /* 64-bit PV guest by default. */
>          d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 0;
>  
> +    if ( (rc = psr_domain_init(d)) != 0 )
> +        goto fail;
> +
>      /* initialize default tsc behavior in case tools don't */
>      tsc_set_info(d, TSC_MODE_DEFAULT, 0UL, 0, 0);
>      spin_lock_init(&d->arch.vtsc_lock);
> @@ -634,6 +637,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
>      free_perdomain_mappings(d);
>      if ( is_pv_domain(d) )
>          free_xenheap_page(d->arch.pv_domain.gdt_ldt_l1tab);
> +    psr_domain_free(d);
>      return rc;
>  }
>  
> @@ -657,7 +661,7 @@ void arch_domain_destroy(struct domain *d)
>      free_xenheap_page(d->shared_info);
>      cleanup_domain_irq_mapping(d);
>  
> -    psr_free_rmid(d);
> +    psr_domain_free(d);
>  }
>  
>  void arch_domain_shutdown(struct domain *d)
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 7de2504..51faa70 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -221,6 +221,48 @@ void psr_ctxt_switch_to(struct domain *d)
>      }
>  }
>  
> +/* Called with domain lock held, no psr specific lock needed */
> +static void psr_free_cos(struct domain *d)
> +{
> +    unsigned int socket;
> +    unsigned int cos;
> +
> +    if( !d->arch.psr_cos_ids )
> +        return;
> +
> +    for ( socket = 0; socket < nr_sockets; socket++ )
> +    {
> +        if ( !cat_socket_info[socket].enabled )
> +            continue;
> +
> +        if ( (cos = d->arch.psr_cos_ids[socket]) == 0 )
> +            continue;
> +
> +        cat_socket_info[socket].cos_cbm_map[cos].ref--;
> +    }
> +
> +    xfree(d->arch.psr_cos_ids);
> +    d->arch.psr_cos_ids = NULL;
> +}
> +
> +int psr_domain_init(struct domain *d)
> +{
> +    if ( cat_socket_info )
> +    {
> +        d->arch.psr_cos_ids = xzalloc_array(unsigned int, nr_sockets);

It is perhaps worth leaving a comment in patch 3 stating that nr_sockets
must never change after domains have been created.

That, or whomever implements CAT/socket hotplug support has to fix this
issue.  (I think the code is fine to leave in its current state, but
nr_sockets changing under the feet of Xen will be a subtle bug for
someone to track down)

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> +        if ( !d->arch.psr_cos_ids )
> +            return -ENOMEM;
> +    }
> +
> +    return 0;
> +}
> +
> +void psr_domain_free(struct domain *d)
> +{
> +    psr_free_rmid(d);
> +    psr_free_cos(d);
> +}
> +
>  static void cat_cpu_init(unsigned int cpu)
>  {
>      unsigned int eax, ebx, ecx, edx;
> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
> index 9cdffa8..9c4d0e6 100644
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -333,7 +333,10 @@ struct arch_domain
>      struct e820entry *e820;
>      unsigned int nr_e820;
>  
> -    unsigned int psr_rmid; /* RMID assigned to the domain for CMT */
> +    /* RMID assigned to the domain for CMT */
> +    unsigned int psr_rmid;
> +    /* COS assigned to the domain for each socket */
> +    unsigned int *psr_cos_ids;
>  
>      /* Shared page for notifying that explicit PIRQ EOI is required. */
>      unsigned long *pirq_eoi_map;
> diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
> index 3bc5496..45392bf 100644
> --- a/xen/include/asm-x86/psr.h
> +++ b/xen/include/asm-x86/psr.h
> @@ -52,6 +52,9 @@ void psr_free_rmid(struct domain *d);
>  
>  void psr_ctxt_switch_to(struct domain *d);
>  
> +int psr_domain_init(struct domain *d);
> +void psr_domain_free(struct domain *d);
> +
>  #endif /* __ASM_PSR_H__ */
>  
>  /*