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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 03/12/13 08:47, Dongxiao Xu wrote:
> Detect platform QoS feature status and enumerate the resource types,
> one of which is to monitor the L3 cache occupancy.
>
> Also introduce a Xen grub command line parameter to control the
> QoS feature status.
>
> Signed-off-by: Jiongxi Li <jiongxi.li@xxxxxxxxx>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> ---
>  docs/misc/xen-command-line.markdown |    8 +++
>  xen/arch/x86/Makefile               |    1 +
>  xen/arch/x86/cpu/intel.c            |    6 ++
>  xen/arch/x86/pqos.c                 |  125 
> +++++++++++++++++++++++++++++++++++
>  xen/arch/x86/setup.c                |    3 +
>  xen/include/asm-x86/cpufeature.h    |    1 +
>  xen/include/asm-x86/pqos.h          |   42 ++++++++++++
>  7 files changed, 186 insertions(+)
>  create mode 100644 xen/arch/x86/pqos.c
>  create mode 100644 xen/include/asm-x86/pqos.h
>
> diff --git a/docs/misc/xen-command-line.markdown 
> b/docs/misc/xen-command-line.markdown
> index 15aa404..bdb5f44 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1037,3 +1037,11 @@ Use the x2apic physical apic driver.  The alternative 
> is the x2apic cluster driv
>  > Default: `true`
>  
>  Permit use of the `xsave/xrstor` instructions.
> +
> +### pqos
> +> `= <boolean>`
> +
> +> Default: `true`
> +
> +Configure platform QoS services.
> +For CQM feature, the default parameter is: pqos=1,cqm=1,cqm_max_rmid=255.

This should be in its correct alphabetic position.  It should also have
"(Intel)" in the title, and enumerate the options.

There isn't really a good example to follow.  Perhaps something like

+> `= List of ( <boolean> | cqm=<boolean> | cqm_max_rmid=<integer> )`


> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index d502bdf..54962e0 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -58,6 +58,7 @@ obj-y += crash.o
>  obj-y += tboot.o
>  obj-y += hpet.o
>  obj-y += xstate.o
> +obj-y += pqos.o
>  
>  obj-$(crash_debug) += gdbstub.o
>  
> diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
> index 27fe762..f0d83ea 100644
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -230,6 +230,12 @@ static void __devinit init_intel(struct cpuinfo_x86 *c)
>            ( c->cpuid_level >= 0x00000006 ) &&
>            ( cpuid_eax(0x00000006) & (1u<<2) ) )
>               set_bit(X86_FEATURE_ARAT, c->x86_capability);
> +
> +     /* Check platform QoS monitoring capability */
> +     if ((c->cpuid_level >= 0x00000007) &&
> +         (cpuid_ebx(0x00000007) & (1u<<12)))
> +             set_bit(X86_FEATURE_QOSM, c->x86_capability);
> +
>  }
>  
>  static struct cpu_dev intel_cpu_dev __cpuinitdata = {
> diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
> new file mode 100644
> index 0000000..b786ec2
> --- /dev/null
> +++ b/xen/arch/x86/pqos.c
> @@ -0,0 +1,125 @@
> +/*
> + * pqos.c: Platform QoS related service for guest.
> + *
> + * Copyright (c) 2013, 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.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program; if not, write to the Free Software Foundation, Inc., 59 
> Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + */
> +#include <asm/processor.h>
> +#include <xen/init.h>
> +#include <asm/pqos.h>
> +
> +static bool_t __initdata opt_pqos = 1;
> +static bool_t __initdata opt_cqm = 1;
> +static unsigned int __initdata opt_cqm_max_rmid = 255;
> +
> +static void __init parse_pqos_param(char *s)
> +{
> +    char *ss;
> +    char *val_str;
> +    unsigned long val;
> +
> +    do {
> +        ss = strchr(s, ',');
> +        if ( ss )
> +            *ss = '\0';
> +
> +        val_str = strchr(s, '=');
> +        if ( !val_str )
> +            val_str = s;
> +        else
> +            val_str += 1;
> +
> +        if ( !parse_bool(s) )
> +            opt_pqos = 0;
> +        else if ( !strncmp(s, "cqm=", 4) && !parse_bool(val_str) )
> +            opt_cqm = 0;
> +        else if ( !strncmp(s, "cqm_max_rmid=", 13) &&
> +                  (val = simple_strtoul(val_str, NULL, 0)) )
> +            opt_cqm_max_rmid = val;
> +
> +        s = ss + 1;
> +    } while ( ss );
> +}
> +custom_param("pqos", parse_pqos_param);

This currently won't do what you expect, and to get regular Xen boolean
behaviour, you need tobe a little more cunning.

Here is some completely untested code...

static void __init parse_pqos_param(char *s)
{
    char *ss;
    char *val_str;
    int val;

    do {
        val = !!strncmp(s, "no-", 3);
        if ( !val )
            s += 3;

        ss = strchr(s, ',');
        if ( ss )
            *ss = '\0';

        val_str = strchr(s, '=');

        if ( !parse_bool(s) )
            opt_pqos = 0;
        else if ( !strcmp(s, "cqm") )
        {
            if ( val_str && !parse_bool(val_str) )
                val = !val;
            opt_cqm = val;
        }
        else if ( val_str && !strcmp(s, "cqm_max_rmid") )
            opt_cqm_max_rmid = simple_strtoul(val_str, NULL, 0);

        s = ss + 1;
    } while ( ss );
}

Which should allow things like "pqos=no-cqm" to work as expected.

> +
> +struct pqos_cqm *cqm;
> +
> +static void __init init_cqm(void)
> +{
> +    unsigned int rmid;
> +    unsigned int eax, edx;

You have to deal with the case that opt_cqm_max_rmid is 0 if the user
has provided junk on the command line.

I would return early here in this case.

~Andrew

> +
> +    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) )
> +    {
> +        xfree(cqm);
> +        return;
> +    }
> +
> +    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 )
> +    {
> +        xfree(cqm);
> +        return;
> +    }
> +
> +    /* 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;
> +
> +    printk(XENLOG_INFO "Cache QoS Monitoring Enabled.\n");
> +}
> +
> +static void __init init_qos_monitor(void)
> +{
> +    unsigned int qm_features;
> +    unsigned int eax, ebx, ecx;
> +
> +    if ( !(boot_cpu_has(X86_FEATURE_QOSM)) )
> +        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 5bf4ee0..95418e4 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;
> @@ -1402,6 +1403,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..94d4f6e
> --- /dev/null
> +++ b/xen/include/asm-x86/pqos.h
> @@ -0,0 +1,42 @@
> +/*
> + * pqos.h: Platform QoS related service for guest.
> + *
> + * Copyright (c) 2013, 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.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program; if not, write to the Free Software Foundation, Inc., 59 
> Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + */
> +#ifndef ASM_PQOS_H
> +#define ASM_PQOS_H
> +
> +#include <public/xen.h>
> +
> +/* QoS Resource Type Enumeration */
> +#define QOS_MONITOR_TYPE_L3            0x2
> +
> +/* QoS Monitoring Event ID */
> +#define QOS_MONITOR_EVTID_L3           0x1
> +
> +struct pqos_cqm {
> +    unsigned int min_rmid;
> +    unsigned int max_rmid;
> +    unsigned int upscaling_factor;
> +    domid_t *rmid_to_dom;
> +};
> +extern struct pqos_cqm *cqm;
> +
> +void init_platform_qos(void);
> +
> +#endif


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