|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 1/8] x86: detect and initialize Cache QoS Monitoring feature
On 21/11/13 07:20, 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..e6ab416
> --- /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;
I cant see anywhere in the series where this is used outside of an init
function, in which case it should be __initdata.
> +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 init_cqm(void)
> +{
> + 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;
> +
Range check on cqm_res_count ? If max_cqm_rmid ends up as -1 from the
cpuid, we will allocate a 0 length array and crash later when reserving
RMID 0
> + 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(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 ( qm_features & QOS_MONITOR_TYPE_L3 )
> + init_cqm();
> +}
> +
> +void __init init_platform_qos(void)
> +{
> + 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();
> +
Does this want to be slightly earlier during startup? Does it make sense
to have dom0 monitored? Is it possible to monitor Xen's L3 usage as well?
> 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..934d68a
> --- /dev/null
> +++ b/xen/include/asm-x86/pqos.h
It might make sense to split this file. The QOS_* defines could be
argued as asm-x86, (perhaps asm-x86/qpos-defines.h)
However, the cqm_res_struct and function declaration are very certainly
not asm code, and would probably better live in arch/x86/qpos.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 {
This name is redundant with the extra "_struct" at the end. How about
cqm_res or cqm_resource ?
> + domid_t domain_id;
You will need to include one of the types headers.
> + bool_t inuse;
> +};
> +
> +void init_platform_qos(void);
> +
> +#endif
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |