Re: [Xen-devel] [PATCH v9 1/6] x86: detect and initialize Cache QoS Monitoring feature

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.02.14 at 07:32, Dongxiao Xu <dongxiao.xu@xxxxxxxxx> wrote:
> +struct pqos_cqm __read_mostly *cqm = NULL;

Misplaced __read_mostly (belongs after the *).

> +static void __init init_cqm(void)
> +{
> +    unsigned int rmid;
> +    unsigned int eax, edx;
> +    unsigned int cqm_pages;
> +    unsigned int i;
> +
> +    if ( !opt_cqm_max_rmid )
> +        return;
> +
> +    cqm = xzalloc(struct pqos_cqm);
> +    if ( !cqm )
> +        return;
> +
> +    cpuid_count(0xf, 1, &eax, &cqm->upscaling_factor, &cqm->max_rmid, &edx);
> +    if ( !(edx & QOS_MONITOR_EVTID_L3) )
> +        goto out;
> +
> +    cqm->min_rmid = 1;
> +    cqm->max_rmid = min(opt_cqm_max_rmid, cqm->max_rmid);
> +
> +    cqm->rmid_to_dom = xmalloc_array(domid_t, cqm->max_rmid + 1);
> +    if ( !cqm->rmid_to_dom )
> +        goto out;
> +
> +    /* Reserve RMID 0 for all domains not being monitored */
> +    cqm->rmid_to_dom[0] = DOMID_XEN;
> +    for ( rmid = cqm->min_rmid; rmid <= cqm->max_rmid; rmid++ )
> +        cqm->rmid_to_dom[rmid] = DOMID_INVALID;
> +
> +    /* Allocate CQM buffer size in initialization stage */
> +    cqm_pages = ((cqm->max_rmid + 1) * sizeof(domid_t) +
> +                (cqm->max_rmid + 1) * sizeof(uint64_t) * NR_CPUS)/

Does this really need to be NR_CPUS (rather than nr_cpu_ids)?

> +                PAGE_SIZE + 1;
> +    cqm->buffer_size = cqm_pages * PAGE_SIZE;
> +
> +    cqm->buffer = alloc_xenheap_pages(get_order_from_pages(cqm_pages), 0);

And does the allocation really need to be physically contiguous?
If so - did you calculate how much more memory you allocate
(due to the rounding up to the next power of 2), to decide
whether it's worthwhile freeing the unused portion?

> +    if ( !cqm->buffer )
> +    {
> +        xfree(cqm->rmid_to_dom);
> +        goto out;
> +    }
> +    memset(cqm->buffer, 0, cqm->buffer_size);
> +
> +    for ( i = 0; i < cqm_pages; i++ )
> +        share_xen_page_with_privileged_guests(
> +            virt_to_page((void *)((unsigned long)cqm->buffer + i * 
> PAGE_SIZE)),

virt_to_page((void *)cqm->buffer + i * PAGE_SIZE)

> +static void __init init_qos_monitor(void)
> +{
> +    unsigned int qm_features;
> +    unsigned int eax, ebx, ecx;
> +
> +    if ( !(boot_cpu_has(X86_FEATURE_QOSM)) )

Pointless pair of parentheses.

> +        return;
> +
> +    cpuid_count(0xf, 0, &eax, &ebx, &ecx, &qm_features);
> +
> +    if ( opt_cqm && (qm_features & QOS_MONITOR_TYPE_L3) )
> +        init_cqm();
> +}
> +
> +void __init init_platform_qos(void)
> +{
> +    if ( !opt_pqos )
> +        return;
> +
> +    init_qos_monitor();
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index b49256d..639528f 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -48,6 +48,7 @@
>  #include <asm/setup.h>
>  #include <xen/cpu.h>
>  #include <asm/nmi.h>
> +#include <asm/pqos.h>
>  
>  /* opt_nosmp: If true, secondary processors are ignored. */
>  static bool_t __initdata opt_nosmp;
> @@ -1419,6 +1420,8 @@ void __init __start_xen(unsigned long mbi_p)
>  
>      domain_unpause_by_systemcontroller(dom0);
>  
> +    init_platform_qos();
> +
>      reset_stack_and_jump(init_done);
>  }
>  
> diff --git a/xen/include/asm-x86/cpufeature.h 
> b/xen/include/asm-x86/cpufeature.h
> index 1cfaf94..ca59668 100644
> --- a/xen/include/asm-x86/cpufeature.h
> +++ b/xen/include/asm-x86/cpufeature.h
> @@ -147,6 +147,7 @@
>  #define X86_FEATURE_ERMS     (7*32+ 9) /* Enhanced REP MOVSB/STOSB */
>  #define X86_FEATURE_INVPCID  (7*32+10) /* Invalidate Process Context ID */
>  #define X86_FEATURE_RTM      (7*32+11) /* Restricted Transactional Memory */
> +#define X86_FEATURE_QOSM     (7*32+12) /* Platform QoS monitoring capability 
> */
>  #define X86_FEATURE_NO_FPU_SEL       (7*32+13) /* FPU CS/DS stored as zero */
>  #define X86_FEATURE_SMAP     (7*32+20) /* Supervisor Mode Access Prevention 
> */
>  
> diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
> new file mode 100644
> index 0000000..0a8065c
> --- /dev/null
> +++ b/xen/include/asm-x86/pqos.h
> @@ -0,0 +1,43 @@
> +/*
> + * pqos.h: Platform QoS related service for guest.
> + *
> + * Copyright (c) 2014, Intel Corporation
> + * Author: Jiongxi Li  <jiongxi.li@xxxxxxxxx>
> + * Author: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> for
> + * more details.
> + */
> +#ifndef ASM_PQOS_H
> +#define ASM_PQOS_H
> +
> +#include <public/xen.h>
> +#include <xen/spinlock.h>
> +
> +/* QoS Resource Type Enumeration */
> +#define QOS_MONITOR_TYPE_L3            0x2
> +
> +/* QoS Monitoring Event ID */
> +#define QOS_MONITOR_EVTID_L3           0x1
> +
> +struct pqos_cqm {
> +    spinlock_t cqm_lock;
> +    uint64_t *buffer;
> +    unsigned int min_rmid;
> +    unsigned int max_rmid;
> +    unsigned int used_rmid;
> +    unsigned int upscaling_factor;
> +    unsigned int buffer_size;
> +    domid_t *rmid_to_dom;
> +};
> +extern struct pqos_cqm *cqm;
> +
> +void init_platform_qos(void);
> +
> +#endif
> -- 
> 1.7.9.5



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