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

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: Wednesday, November 20, 2013 7:12 PM
> To: Xu, Dongxiao
> Cc: xen-devel@xxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH 1/8] x86: detect and initialize Cache QoS
> Monitoring feature
> 
> On 20/11/13 03:27, dongxiao.xu@xxxxxxxxx wrote:
> > From: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> >
> > 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 globally.
> >
> > Signed-off-by: Jiongxi Li <jiongxi.li@xxxxxxxxx>
> > Signed-off-by: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> > ---
> >  xen/arch/x86/Makefile            |    1 +
> >  xen/arch/x86/cpu/intel.c         |    6 +++
> >  xen/arch/x86/pqos.c              |   89
> ++++++++++++++++++++++++++++++++++++++
> >  xen/arch/x86/setup.c             |    3 ++
> >  xen/include/asm-x86/cpufeature.h |    1 +
> >  xen/include/asm-x86/pqos.h       |   37 ++++++++++++++++
> >  6 files changed, 137 insertions(+)
> >  create mode 100644 xen/arch/x86/pqos.c
> >  create mode 100644 xen/include/asm-x86/pqos.h
> >
> > 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..617cce1
> > --- /dev/null
> > +++ b/xen/arch/x86/pqos.c
> > @@ -0,0 +1,89 @@
> > +/*
> > + * 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 pqos_enabled = 1;
> > +boolean_param("pqos", pqos_enabled);
> > +
> > +unsigned int cqm_res_count = 0;
> > +unsigned int cqm_upscaling_factor = 0;
> > +bool_t cqm_enabled = 0;
> > +struct cqm_res_struct *cqm_res_array = NULL;
> > +
> > +static void init_cqm(void)
> 
> __init

OK.

> 
> > +{
> > +    unsigned int eax, edx;
> > +    unsigned int max_cqm_rmid;
> > +
> > +    cpuid_count(0xf, 1, &eax, &cqm_upscaling_factor, &max_cqm_rmid,
> &edx);
> > +    if ( !(edx & QOS_MONITOR_EVTID_L3) )
> > +        return;
> > +
> > +    cqm_res_count = max_cqm_rmid + 1;
> > +
> > +    cqm_res_array = xzalloc_array(struct cqm_res_struct, cqm_res_count);
> > +    if ( !cqm_res_array )
> > +    {
> > +        cqm_res_count = 0;
> > +        return;
> > +    }
> > +
> > +    cqm_enabled = 1;
> > +
> > +    /* Reserve RMID 0 for all domains not being monitored */
> > +    cqm_res_array[0].inuse = 1;
> > +
> > +    printk("Cache QoS Monitoring Enabled.\n");
> 
> This should probably be a XENLOG_INFO, and we tend to omit full stops.

OK.

> 
> > +}
> > +
> > +static void init_qos_monitor(void)
> 
> __init

OK.

> 
> > +{
> > +    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 ( qm_features & QOS_MONITOR_TYPE_L3 )
> > +        init_cqm();
> > +}
> > +
> > +void init_platform_qos(void)
> 
> __init

OK.

> 
> Are there expected to be future platform qos monitoring abilities?

Yes.

> 
> > +{
> > +    if ( !pqos_enabled )
> > +        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..2184ea9
> > --- /dev/null
> > +++ b/xen/include/asm-x86/pqos.h
> > @@ -0,0 +1,37 @@
> > +/*
> > + * 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
> > +
> > +/* QoS Resource Type Enumeration */
> > +#define QOS_MONITOR_TYPE_L3            0x2
> > +
> > +/* QoS Monitoring Event ID */
> > +#define QOS_MONITOR_EVTID_L3           0x1
> > +
> > +struct cqm_res_struct {
> > +    bool_t    inuse;
> > +    uint16_t  domain_id;
> 
> domid_t please.  Also, reversing the order of the parameters would be a
> good idea, packing-wise.

OK.

Will reflect above suggestions in next version patches.

Thanks,
Dongxiao

> 
> ~Andrew
> 
> > +};
> > +
> > +void init_platform_qos(void);
> > +
> > +#endif


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