Mailing List Archive: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function

[PATCH 2 of 2] vpmu: Add a vpmu cpuid function

Jan 19, 2012, 5:54 AM

Post #1 of 11 (473 views)

tools/libxc/xc_cpuid_x86.c | 1 +
xen/arch/x86/hvm/vmx/vmx.c | 2 ++
xen/arch/x86/hvm/vmx/vpmu_core2.c | 38 ++++++++++++++++++++++++++++++++++++--
xen/arch/x86/hvm/vpmu.c | 25 ++++++++++++++++++-------
xen/include/asm-x86/hvm/vpmu.h | 6 ++++++
5 files changed, 63 insertions(+), 9 deletions(-)

The "Debug Store" cpuid flag in the Intel processors gets enabled in the libxc.
A new function call arch_vpmu_cpuid is added to the struct arch_vpmu_ops and for
Intel processors the function core2_vpmu_cpuid() is added.
The aim is that always a "struct arch_vpmu_ops" is accessible at least with
a cpuid function to switch off special PMU cpuid flags dynamically depending on
the boot variable opt_vpmu_enabled.

Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>

diff -r 7a82c2e2eb33 tools/libxc/xc_cpuid_x86.c
--- a/tools/libxc/xc_cpuid_x86.c Thu Jan 19 13:14:02 2012 +0100
+++ b/tools/libxc/xc_cpuid_x86.c Thu Jan 19 14:37:17 2012 +0100
@@ -343,6 +343,7 @@ static void xc_cpuid_hvm_policy(
bitmaskof(X86_FEATURE_CMOV) |
bitmaskof(X86_FEATURE_PAT) |
bitmaskof(X86_FEATURE_CLFLSH) |
+ bitmaskof(X86_FEATURE_DS) |
bitmaskof(X86_FEATURE_PSE36) |
bitmaskof(X86_FEATURE_MMX) |
bitmaskof(X86_FEATURE_FXSR) |
diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c Thu Jan 19 13:14:02 2012 +0100
+++ b/xen/arch/x86/hvm/vmx/vmx.c Thu Jan 19 14:37:17 2012 +0100
@@ -1603,6 +1603,8 @@ static void vmx_cpuid_intercept(
break;
}

+ vpmu_do_cpuid(input, eax, ebx, ecx, edx);
+
HVMTRACE_5D (CPUID, input, *eax, *ebx, *ecx, *edx);
}

diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vmx/vpmu_core2.c
--- a/xen/arch/x86/hvm/vmx/vpmu_core2.c Thu Jan 19 13:14:02 2012 +0100
+++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c Thu Jan 19 14:37:17 2012 +0100
@@ -598,6 +598,29 @@ static void core2_vpmu_destroy(struct vc
vpmu->flags &= ~VPMU_CONTEXT_ALLOCATED;
}

+/**
+ * core2_vpmu_cpuid - prepare special vpmu cpuid bits
+ * If emulation of vpmu is switched off, some bits are swtiched off, currently:
+ * - EAX[0x1].EAX[Bits 0-7]: PMC revision id.
+ * - EAX[0xa].EDX[Bit 21]: Debug Store
+ */
+#define bitmaskof(idx) (1U << ((idx) & 31))
+static void core2_vpmu_cpuid(unsigned int input,
+ unsigned int *eax, unsigned int *ebx,
+ unsigned int *ecx, unsigned int *edx)
+{
+ switch ( input )
+ {
+ case 0x1:
+ *edx &= ~bitmaskof(X86_FEATURE_DS); /* Debug Store not supported */
+ break;
+ case 0xa:
+ if ( !opt_vpmu_enabled )
+ *eax &= ~(unsigned int)0xff; /* Clear pmc version id. */
+ break;
+ }
+}
+
struct arch_vpmu_ops core2_vpmu_ops = {
.do_wrmsr = core2_vpmu_do_wrmsr,
.do_rdmsr = core2_vpmu_do_rdmsr,
@@ -605,7 +628,13 @@ struct arch_vpmu_ops core2_vpmu_ops = {
.arch_vpmu_initialise = core2_vpmu_initialise,
.arch_vpmu_destroy = core2_vpmu_destroy,
.arch_vpmu_save = core2_vpmu_save,
- .arch_vpmu_load = core2_vpmu_load
+ .arch_vpmu_load = core2_vpmu_load,
+ .arch_vpmu_cpuid = core2_vpmu_cpuid
+};
+
+/* Used if vpmu is disabled. */
+struct arch_vpmu_ops core2_vpmu_dis_ops = {
+ .arch_vpmu_cpuid = core2_vpmu_cpuid
};

int vmx_vpmu_initialize(struct vcpu *v)
@@ -615,7 +644,7 @@ int vmx_vpmu_initialize(struct vcpu *v)
__u8 cpu_model = current_cpu_data.x86_model;

if ( !opt_vpmu_enabled )
- return -EINVAL;
+ goto func_out;

if ( family == 6 )
{
@@ -635,6 +664,11 @@ int vmx_vpmu_initialize(struct vcpu *v)
printk("VPMU: Initialization failed. "
"Intel processor family %d model %d has not "
"been supported\n", family, cpu_model);
+
+func_out:
+
+ vpmu->arch_vpmu_ops = &core2_vpmu_dis_ops;
+
return -EINVAL;
}

diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vpmu.c
--- a/xen/arch/x86/hvm/vpmu.c Thu Jan 19 13:14:02 2012 +0100
+++ b/xen/arch/x86/hvm/vpmu.c Thu Jan 19 14:37:17 2012 +0100
@@ -39,7 +39,7 @@ int vpmu_do_wrmsr(unsigned int msr, uint
{
struct vpmu_struct *vpmu = vcpu_vpmu(current);

- if ( vpmu->arch_vpmu_ops )
+ if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
return 0;
}
@@ -48,7 +48,7 @@ int vpmu_do_rdmsr(unsigned int msr, uint
{
struct vpmu_struct *vpmu = vcpu_vpmu(current);

- if ( vpmu->arch_vpmu_ops )
+ if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_rdmsr )
return vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content);
return 0;
}
@@ -57,7 +57,7 @@ int vpmu_do_interrupt(struct cpu_user_re
{
struct vpmu_struct *vpmu = vcpu_vpmu(current);

- if ( vpmu->arch_vpmu_ops )
+ if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_interrupt )
return vpmu->arch_vpmu_ops->do_interrupt(regs);
return 0;
}
@@ -66,7 +66,7 @@ void vpmu_save(struct vcpu *v)
{
struct vpmu_struct *vpmu = vcpu_vpmu(v);

- if ( vpmu->arch_vpmu_ops )
+ if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_save )
vpmu->arch_vpmu_ops->arch_vpmu_save(v);
}

@@ -74,7 +74,7 @@ void vpmu_load(struct vcpu *v)
{
struct vpmu_struct *vpmu = vcpu_vpmu(v);

- if ( vpmu->arch_vpmu_ops )
+ if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_load )
vpmu->arch_vpmu_ops->arch_vpmu_load(v);
}

@@ -109,7 +109,8 @@ void vpmu_initialise(struct vcpu *v)
{
vpmu->flags = 0;
vpmu->context = NULL;
- vpmu->arch_vpmu_ops->arch_vpmu_initialise(v);
+ if ( vpmu->arch_vpmu_ops->arch_vpmu_initialise )
+ vpmu->arch_vpmu_ops->arch_vpmu_initialise(v);
}
}

@@ -117,7 +118,17 @@ void vpmu_destroy(struct vcpu *v)
{
struct vpmu_struct *vpmu = vcpu_vpmu(v);

- if ( vpmu->arch_vpmu_ops )
+ if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_destroy )
vpmu->arch_vpmu_ops->arch_vpmu_destroy(v);
}

+void vpmu_do_cpuid(unsigned int input,
+ unsigned int *eax, unsigned int *ebx,
+ unsigned int *ecx, unsigned int *edx)
+{
+ struct vpmu_struct *vpmu = vcpu_vpmu(current);
+
+ if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_cpuid)
+ vpmu->arch_vpmu_ops->arch_vpmu_cpuid(input, eax, ebx, ecx, edx);
+}
+
diff -r 7a82c2e2eb33 xen/include/asm-x86/hvm/vpmu.h
--- a/xen/include/asm-x86/hvm/vpmu.h Thu Jan 19 13:14:02 2012 +0100
+++ b/xen/include/asm-x86/hvm/vpmu.h Thu Jan 19 14:37:17 2012 +0100
@@ -56,6 +56,9 @@ struct arch_vpmu_ops {
void (*arch_vpmu_destroy)(struct vcpu *v);
void (*arch_vpmu_save)(struct vcpu *v);
void (*arch_vpmu_load)(struct vcpu *v);
+ void (*arch_vpmu_cpuid)(unsigned int input,
+ unsigned int *eax, unsigned int *ebx,
+ unsigned int *ecx, unsigned int *edx);
};

int vmx_vpmu_initialize(struct vcpu *v);
@@ -78,6 +81,9 @@ void vpmu_initialise(struct vcpu *v);
void vpmu_destroy(struct vcpu *v);
void vpmu_save(struct vcpu *v);
void vpmu_load(struct vcpu *v);
+void vpmu_do_cpuid(unsigned int input,
+ unsigned int *eax, unsigned int *ebx,
+ unsigned int *ecx, unsigned int *edx);

extern int acquire_pmu_ownership(int pmu_ownership);
extern void release_pmu_ownership(int pmu_ownership);

--
Company details: http://ts.fujitsu.com/imprint.html

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function [ In reply to ]

keir at xen

Jan 20, 2012, 2:29 AM

Post #2 of 11 (462 views)

Permalink

On 19/01/2012 13:54, "Dietmar Hahn" <dietmar.hahn@ts.fujitsu.com> wrote:

>
> tools/libxc/xc_cpuid_x86.c | 1 +
> xen/arch/x86/hvm/vmx/vmx.c | 2 ++
> xen/arch/x86/hvm/vmx/vpmu_core2.c | 38
> ++++++++++++++++++++++++++++++++++++--
> xen/arch/x86/hvm/vpmu.c | 25 ++++++++++++++++++-------
> xen/include/asm-x86/hvm/vpmu.h | 6 ++++++
> 5 files changed, 63 insertions(+), 9 deletions(-)
>
>
> The "Debug Store" cpuid flag in the Intel processors gets enabled in the
> libxc.
> A new function call arch_vpmu_cpuid is added to the struct arch_vpmu_ops and
> for
> Intel processors the function core2_vpmu_cpuid() is added.
> The aim is that always a "struct arch_vpmu_ops" is accessible at least with
> a cpuid function to switch off special PMU cpuid flags dynamically depending
> on
> the boot variable opt_vpmu_enabled.

Our CPUID configuration is done per-domain, and from
tools/libxc/xc_cpuid_x86.c. CPUID adjustments implemented within the
hypervisor are generally not acceptable without very good reason.

-- Keir

> Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
>
> diff -r 7a82c2e2eb33 tools/libxc/xc_cpuid_x86.c
> --- a/tools/libxc/xc_cpuid_x86.c Thu Jan 19 13:14:02 2012 +0100
> +++ b/tools/libxc/xc_cpuid_x86.c Thu Jan 19 14:37:17 2012 +0100
> @@ -343,6 +343,7 @@ static void xc_cpuid_hvm_policy(
> bitmaskof(X86_FEATURE_CMOV) |
> bitmaskof(X86_FEATURE_PAT) |
> bitmaskof(X86_FEATURE_CLFLSH) |
> + bitmaskof(X86_FEATURE_DS) |
> bitmaskof(X86_FEATURE_PSE36) |
> bitmaskof(X86_FEATURE_MMX) |
> bitmaskof(X86_FEATURE_FXSR) |
> diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vmx/vmx.c
> --- a/xen/arch/x86/hvm/vmx/vmx.c Thu Jan 19 13:14:02 2012 +0100
> +++ b/xen/arch/x86/hvm/vmx/vmx.c Thu Jan 19 14:37:17 2012 +0100
> @@ -1603,6 +1603,8 @@ static void vmx_cpuid_intercept(
> break;
> }
>
> + vpmu_do_cpuid(input, eax, ebx, ecx, edx);
> +
> HVMTRACE_5D (CPUID, input, *eax, *ebx, *ecx, *edx);
> }
>
> diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vmx/vpmu_core2.c
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c Thu Jan 19 13:14:02 2012 +0100
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c Thu Jan 19 14:37:17 2012 +0100
> @@ -598,6 +598,29 @@ static void core2_vpmu_destroy(struct vc
> vpmu->flags &= ~VPMU_CONTEXT_ALLOCATED;
> }
>
> +/**
> + * core2_vpmu_cpuid - prepare special vpmu cpuid bits
> + * If emulation of vpmu is switched off, some bits are swtiched off,
> currently:
> + * - EAX[0x1].EAX[Bits 0-7]: PMC revision id.
> + * - EAX[0xa].EDX[Bit 21]: Debug Store
> + */
> +#define bitmaskof(idx) (1U << ((idx) & 31))
> +static void core2_vpmu_cpuid(unsigned int input,
> + unsigned int *eax, unsigned int *ebx,
> + unsigned int *ecx, unsigned int *edx)
> +{
> + switch ( input )
> + {
> + case 0x1:
> + *edx &= ~bitmaskof(X86_FEATURE_DS); /* Debug Store not supported
> */
> + break;
> + case 0xa:
> + if ( !opt_vpmu_enabled )
> + *eax &= ~(unsigned int)0xff; /* Clear pmc version id. */
> + break;
> + }
> +}
> +
> struct arch_vpmu_ops core2_vpmu_ops = {
> .do_wrmsr = core2_vpmu_do_wrmsr,
> .do_rdmsr = core2_vpmu_do_rdmsr,
> @@ -605,7 +628,13 @@ struct arch_vpmu_ops core2_vpmu_ops = {
> .arch_vpmu_initialise = core2_vpmu_initialise,
> .arch_vpmu_destroy = core2_vpmu_destroy,
> .arch_vpmu_save = core2_vpmu_save,
> - .arch_vpmu_load = core2_vpmu_load
> + .arch_vpmu_load = core2_vpmu_load,
> + .arch_vpmu_cpuid = core2_vpmu_cpuid
> +};
> +
> +/* Used if vpmu is disabled. */
> +struct arch_vpmu_ops core2_vpmu_dis_ops = {
> + .arch_vpmu_cpuid = core2_vpmu_cpuid
> };
>
> int vmx_vpmu_initialize(struct vcpu *v)
> @@ -615,7 +644,7 @@ int vmx_vpmu_initialize(struct vcpu *v)
> __u8 cpu_model = current_cpu_data.x86_model;
>
> if ( !opt_vpmu_enabled )
> - return -EINVAL;
> + goto func_out;
>
> if ( family == 6 )
> {
> @@ -635,6 +664,11 @@ int vmx_vpmu_initialize(struct vcpu *v)
> printk("VPMU: Initialization failed. "
> "Intel processor family %d model %d has not "
> "been supported\n", family, cpu_model);
> +
> +func_out:
> +
> + vpmu->arch_vpmu_ops = &core2_vpmu_dis_ops;
> +
> return -EINVAL;
> }
>
> diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vpmu.c
> --- a/xen/arch/x86/hvm/vpmu.c Thu Jan 19 13:14:02 2012 +0100
> +++ b/xen/arch/x86/hvm/vpmu.c Thu Jan 19 14:37:17 2012 +0100
> @@ -39,7 +39,7 @@ int vpmu_do_wrmsr(unsigned int msr, uint
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>
> - if ( vpmu->arch_vpmu_ops )
> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
> return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
> return 0;
> }
> @@ -48,7 +48,7 @@ int vpmu_do_rdmsr(unsigned int msr, uint
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>
> - if ( vpmu->arch_vpmu_ops )
> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_rdmsr )
> return vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content);
> return 0;
> }
> @@ -57,7 +57,7 @@ int vpmu_do_interrupt(struct cpu_user_re
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>
> - if ( vpmu->arch_vpmu_ops )
> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_interrupt )
> return vpmu->arch_vpmu_ops->do_interrupt(regs);
> return 0;
> }
> @@ -66,7 +66,7 @@ void vpmu_save(struct vcpu *v)
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>
> - if ( vpmu->arch_vpmu_ops )
> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_save )
> vpmu->arch_vpmu_ops->arch_vpmu_save(v);
> }
>
> @@ -74,7 +74,7 @@ void vpmu_load(struct vcpu *v)
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>
> - if ( vpmu->arch_vpmu_ops )
> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_load )
> vpmu->arch_vpmu_ops->arch_vpmu_load(v);
> }
>
> @@ -109,7 +109,8 @@ void vpmu_initialise(struct vcpu *v)
> {
> vpmu->flags = 0;
> vpmu->context = NULL;
> - vpmu->arch_vpmu_ops->arch_vpmu_initialise(v);
> + if ( vpmu->arch_vpmu_ops->arch_vpmu_initialise )
> + vpmu->arch_vpmu_ops->arch_vpmu_initialise(v);
> }
> }
>
> @@ -117,7 +118,17 @@ void vpmu_destroy(struct vcpu *v)
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>
> - if ( vpmu->arch_vpmu_ops )
> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_destroy )
> vpmu->arch_vpmu_ops->arch_vpmu_destroy(v);
> }
>
> +void vpmu_do_cpuid(unsigned int input,
> + unsigned int *eax, unsigned int *ebx,
> + unsigned int *ecx, unsigned int *edx)
> +{
> + struct vpmu_struct *vpmu = vcpu_vpmu(current);
> +
> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_cpuid)
> + vpmu->arch_vpmu_ops->arch_vpmu_cpuid(input, eax, ebx, ecx, edx);
> +}
> +
> diff -r 7a82c2e2eb33 xen/include/asm-x86/hvm/vpmu.h
> --- a/xen/include/asm-x86/hvm/vpmu.h Thu Jan 19 13:14:02 2012 +0100
> +++ b/xen/include/asm-x86/hvm/vpmu.h Thu Jan 19 14:37:17 2012 +0100
> @@ -56,6 +56,9 @@ struct arch_vpmu_ops {
> void (*arch_vpmu_destroy)(struct vcpu *v);
> void (*arch_vpmu_save)(struct vcpu *v);
> void (*arch_vpmu_load)(struct vcpu *v);
> + void (*arch_vpmu_cpuid)(unsigned int input,
> + unsigned int *eax, unsigned int *ebx,
> + unsigned int *ecx, unsigned int *edx);
> };
>
> int vmx_vpmu_initialize(struct vcpu *v);
> @@ -78,6 +81,9 @@ void vpmu_initialise(struct vcpu *v);
> void vpmu_destroy(struct vcpu *v);
> void vpmu_save(struct vcpu *v);
> void vpmu_load(struct vcpu *v);
> +void vpmu_do_cpuid(unsigned int input,
> + unsigned int *eax, unsigned int *ebx,
> + unsigned int *ecx, unsigned int *edx);
>
> extern int acquire_pmu_ownership(int pmu_ownership);
> extern void release_pmu_ownership(int pmu_ownership);

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function [ In reply to ]

dietmar.hahn at ts

Jan 20, 2012, 2:49 AM

Post #3 of 11 (470 views)

Permalink

Am Freitag 20 Januar 2012, 10:29:41 schrieb Keir Fraser:
> On 19/01/2012 13:54, "Dietmar Hahn" <dietmar.hahn@ts.fujitsu.com> wrote:
>
> >
> > tools/libxc/xc_cpuid_x86.c | 1 +
> > xen/arch/x86/hvm/vmx/vmx.c | 2 ++
> > xen/arch/x86/hvm/vmx/vpmu_core2.c | 38
> > ++++++++++++++++++++++++++++++++++++--
> > xen/arch/x86/hvm/vpmu.c | 25 ++++++++++++++++++-------
> > xen/include/asm-x86/hvm/vpmu.h | 6 ++++++
> > 5 files changed, 63 insertions(+), 9 deletions(-)
> >
> >
> > The "Debug Store" cpuid flag in the Intel processors gets enabled in the
> > libxc.
> > A new function call arch_vpmu_cpuid is added to the struct arch_vpmu_ops and
> > for
> > Intel processors the function core2_vpmu_cpuid() is added.
> > The aim is that always a "struct arch_vpmu_ops" is accessible at least with
> > a cpuid function to switch off special PMU cpuid flags dynamically depending
> > on
> > the boot variable opt_vpmu_enabled.
>
> Our CPUID configuration is done per-domain, and from
> tools/libxc/xc_cpuid_x86.c. CPUID adjustments implemented within the
> hypervisor are generally not acceptable without very good reason.

Then a way is needed to have access to the opt_vpmu_enabled variable within the
hypervisor from the tools to decide the enabling of the flag (is there such a
way?) or the mechanism with the boot variable must be changed.
The opt_vpmu_enabled boot variable was introduced because of a PMU problem in
the Nehalem cpus leading sometimes to hypervisor crashes. But with the done
quirk we never had a crash anymore.
So maybe we can always switch on the vpmu stuff in the hypervisor and add a
flag in the domain configuration when somebody wants to do some performance
tests?

Dietmar.

>
> -- Keir
>
> > Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
> >
> > diff -r 7a82c2e2eb33 tools/libxc/xc_cpuid_x86.c
> > --- a/tools/libxc/xc_cpuid_x86.c Thu Jan 19 13:14:02 2012 +0100
> > +++ b/tools/libxc/xc_cpuid_x86.c Thu Jan 19 14:37:17 2012 +0100
> > @@ -343,6 +343,7 @@ static void xc_cpuid_hvm_policy(
> > bitmaskof(X86_FEATURE_CMOV) |
> > bitmaskof(X86_FEATURE_PAT) |
> > bitmaskof(X86_FEATURE_CLFLSH) |
> > + bitmaskof(X86_FEATURE_DS) |
> > bitmaskof(X86_FEATURE_PSE36) |
> > bitmaskof(X86_FEATURE_MMX) |
> > bitmaskof(X86_FEATURE_FXSR) |
> > diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vmx/vmx.c
> > --- a/xen/arch/x86/hvm/vmx/vmx.c Thu Jan 19 13:14:02 2012 +0100
> > +++ b/xen/arch/x86/hvm/vmx/vmx.c Thu Jan 19 14:37:17 2012 +0100
> > @@ -1603,6 +1603,8 @@ static void vmx_cpuid_intercept(
> > break;
> > }
> >
> > + vpmu_do_cpuid(input, eax, ebx, ecx, edx);
> > +
> > HVMTRACE_5D (CPUID, input, *eax, *ebx, *ecx, *edx);
> > }
> >
> > diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vmx/vpmu_core2.c
> > --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c Thu Jan 19 13:14:02 2012 +0100
> > +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c Thu Jan 19 14:37:17 2012 +0100
> > @@ -598,6 +598,29 @@ static void core2_vpmu_destroy(struct vc
> > vpmu->flags &= ~VPMU_CONTEXT_ALLOCATED;
> > }
> >
> > +/**
> > + * core2_vpmu_cpuid - prepare special vpmu cpuid bits
> > + * If emulation of vpmu is switched off, some bits are swtiched off,
> > currently:
> > + * - EAX[0x1].EAX[Bits 0-7]: PMC revision id.
> > + * - EAX[0xa].EDX[Bit 21]: Debug Store
> > + */
> > +#define bitmaskof(idx) (1U << ((idx) & 31))
> > +static void core2_vpmu_cpuid(unsigned int input,
> > + unsigned int *eax, unsigned int *ebx,
> > + unsigned int *ecx, unsigned int *edx)
> > +{
> > + switch ( input )
> > + {
> > + case 0x1:
> > + *edx &= ~bitmaskof(X86_FEATURE_DS); /* Debug Store not supported
> > */
> > + break;
> > + case 0xa:
> > + if ( !opt_vpmu_enabled )
> > + *eax &= ~(unsigned int)0xff; /* Clear pmc version id. */
> > + break;
> > + }
> > +}
> > +
> > struct arch_vpmu_ops core2_vpmu_ops = {
> > .do_wrmsr = core2_vpmu_do_wrmsr,
> > .do_rdmsr = core2_vpmu_do_rdmsr,
> > @@ -605,7 +628,13 @@ struct arch_vpmu_ops core2_vpmu_ops = {
> > .arch_vpmu_initialise = core2_vpmu_initialise,
> > .arch_vpmu_destroy = core2_vpmu_destroy,
> > .arch_vpmu_save = core2_vpmu_save,
> > - .arch_vpmu_load = core2_vpmu_load
> > + .arch_vpmu_load = core2_vpmu_load,
> > + .arch_vpmu_cpuid = core2_vpmu_cpuid
> > +};
> > +
> > +/* Used if vpmu is disabled. */
> > +struct arch_vpmu_ops core2_vpmu_dis_ops = {
> > + .arch_vpmu_cpuid = core2_vpmu_cpuid
> > };
> >
> > int vmx_vpmu_initialize(struct vcpu *v)
> > @@ -615,7 +644,7 @@ int vmx_vpmu_initialize(struct vcpu *v)
> > __u8 cpu_model = current_cpu_data.x86_model;
> >
> > if ( !opt_vpmu_enabled )
> > - return -EINVAL;
> > + goto func_out;
> >
> > if ( family == 6 )
> > {
> > @@ -635,6 +664,11 @@ int vmx_vpmu_initialize(struct vcpu *v)
> > printk("VPMU: Initialization failed. "
> > "Intel processor family %d model %d has not "
> > "been supported\n", family, cpu_model);
> > +
> > +func_out:
> > +
> > + vpmu->arch_vpmu_ops = &core2_vpmu_dis_ops;
> > +
> > return -EINVAL;
> > }
> >
> > diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vpmu.c
> > --- a/xen/arch/x86/hvm/vpmu.c Thu Jan 19 13:14:02 2012 +0100
> > +++ b/xen/arch/x86/hvm/vpmu.c Thu Jan 19 14:37:17 2012 +0100
> > @@ -39,7 +39,7 @@ int vpmu_do_wrmsr(unsigned int msr, uint
> > {
> > struct vpmu_struct *vpmu = vcpu_vpmu(current);
> >
> > - if ( vpmu->arch_vpmu_ops )
> > + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
> > return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
> > return 0;
> > }
> > @@ -48,7 +48,7 @@ int vpmu_do_rdmsr(unsigned int msr, uint
> > {
> > struct vpmu_struct *vpmu = vcpu_vpmu(current);
> >
> > - if ( vpmu->arch_vpmu_ops )
> > + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_rdmsr )
> > return vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content);
> > return 0;
> > }
> > @@ -57,7 +57,7 @@ int vpmu_do_interrupt(struct cpu_user_re
> > {
> > struct vpmu_struct *vpmu = vcpu_vpmu(current);
> >
> > - if ( vpmu->arch_vpmu_ops )
> > + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_interrupt )
> > return vpmu->arch_vpmu_ops->do_interrupt(regs);
> > return 0;
> > }
> > @@ -66,7 +66,7 @@ void vpmu_save(struct vcpu *v)
> > {
> > struct vpmu_struct *vpmu = vcpu_vpmu(v);
> >
> > - if ( vpmu->arch_vpmu_ops )
> > + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_save )
> > vpmu->arch_vpmu_ops->arch_vpmu_save(v);
> > }
> >
> > @@ -74,7 +74,7 @@ void vpmu_load(struct vcpu *v)
> > {
> > struct vpmu_struct *vpmu = vcpu_vpmu(v);
> >
> > - if ( vpmu->arch_vpmu_ops )
> > + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_load )
> > vpmu->arch_vpmu_ops->arch_vpmu_load(v);
> > }
> >
> > @@ -109,7 +109,8 @@ void vpmu_initialise(struct vcpu *v)
> > {
> > vpmu->flags = 0;
> > vpmu->context = NULL;
> > - vpmu->arch_vpmu_ops->arch_vpmu_initialise(v);
> > + if ( vpmu->arch_vpmu_ops->arch_vpmu_initialise )
> > + vpmu->arch_vpmu_ops->arch_vpmu_initialise(v);
> > }
> > }
> >
> > @@ -117,7 +118,17 @@ void vpmu_destroy(struct vcpu *v)
> > {
> > struct vpmu_struct *vpmu = vcpu_vpmu(v);
> >
> > - if ( vpmu->arch_vpmu_ops )
> > + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_destroy )
> > vpmu->arch_vpmu_ops->arch_vpmu_destroy(v);
> > }
> >
> > +void vpmu_do_cpuid(unsigned int input,
> > + unsigned int *eax, unsigned int *ebx,
> > + unsigned int *ecx, unsigned int *edx)
> > +{
> > + struct vpmu_struct *vpmu = vcpu_vpmu(current);
> > +
> > + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_cpuid)
> > + vpmu->arch_vpmu_ops->arch_vpmu_cpuid(input, eax, ebx, ecx, edx);
> > +}
> > +
> > diff -r 7a82c2e2eb33 xen/include/asm-x86/hvm/vpmu.h
> > --- a/xen/include/asm-x86/hvm/vpmu.h Thu Jan 19 13:14:02 2012 +0100
> > +++ b/xen/include/asm-x86/hvm/vpmu.h Thu Jan 19 14:37:17 2012 +0100
> > @@ -56,6 +56,9 @@ struct arch_vpmu_ops {
> > void (*arch_vpmu_destroy)(struct vcpu *v);
> > void (*arch_vpmu_save)(struct vcpu *v);
> > void (*arch_vpmu_load)(struct vcpu *v);
> > + void (*arch_vpmu_cpuid)(unsigned int input,
> > + unsigned int *eax, unsigned int *ebx,
> > + unsigned int *ecx, unsigned int *edx);
> > };
> >
> > int vmx_vpmu_initialize(struct vcpu *v);
> > @@ -78,6 +81,9 @@ void vpmu_initialise(struct vcpu *v);
> > void vpmu_destroy(struct vcpu *v);
> > void vpmu_save(struct vcpu *v);
> > void vpmu_load(struct vcpu *v);
> > +void vpmu_do_cpuid(unsigned int input,
> > + unsigned int *eax, unsigned int *ebx,
> > + unsigned int *ecx, unsigned int *edx);
> >
> > extern int acquire_pmu_ownership(int pmu_ownership);
> > extern void release_pmu_ownership(int pmu_ownership);
>
>
>
--
Company details: http://ts.fujitsu.com/imprint.html

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function [ In reply to ]

keir at xen

Jan 20, 2012, 2:50 AM

Post #4 of 11 (482 views)

Permalink

On 20/01/2012 10:29, "Keir Fraser" <keir@xen.org> wrote:

>> The "Debug Store" cpuid flag in the Intel processors gets enabled in the
>> libxc.
>> A new function call arch_vpmu_cpuid is added to the struct arch_vpmu_ops and
>> for
>> Intel processors the function core2_vpmu_cpuid() is added.
>> The aim is that always a "struct arch_vpmu_ops" is accessible at least with
>> a cpuid function to switch off special PMU cpuid flags dynamically depending
>> on
>> the boot variable opt_vpmu_enabled.
>
> Our CPUID configuration is done per-domain, and from
> tools/libxc/xc_cpuid_x86.c. CPUID adjustments implemented within the
> hypervisor are generally not acceptable without very good reason.

I'll preempt you saying that you need to depend on opt_vpmu_enabled by
saying you should get rid of that hacky global configuration flag, and allow
VPMU to be properly enabled per domain, via the toolstack. Then
configuration via libxc/xc_cpuid_x86.c will fall naturally into place.
Perhaps you can allow configuration straightforwardly via the existing
libxl_cpuid.c mechanism, or perhaps you need a higher-level config option
than that, and then cook it down into CPUID fiddling from within libxl, or
libxc. One of the toolstack maintainers (Ian Jackson for example) may have
better ideas than me on that.

Also, there are AMD- and Intel-specific sections to xc_cpuid_x86.c for HVM
guests -- {amd,intel}_xc_cpuid_policy(). You can use those to set flags
differently for the two vendors.

> -- Keir
>
>> Signed-off-by: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
>>
>> diff -r 7a82c2e2eb33 tools/libxc/xc_cpuid_x86.c
>> --- a/tools/libxc/xc_cpuid_x86.c Thu Jan 19 13:14:02 2012 +0100
>> +++ b/tools/libxc/xc_cpuid_x86.c Thu Jan 19 14:37:17 2012 +0100
>> @@ -343,6 +343,7 @@ static void xc_cpuid_hvm_policy(
>> bitmaskof(X86_FEATURE_CMOV) |
>> bitmaskof(X86_FEATURE_PAT) |
>> bitmaskof(X86_FEATURE_CLFLSH) |
>> + bitmaskof(X86_FEATURE_DS) |
>> bitmaskof(X86_FEATURE_PSE36) |
>> bitmaskof(X86_FEATURE_MMX) |
>> bitmaskof(X86_FEATURE_FXSR) |
>> diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vmx/vmx.c
>> --- a/xen/arch/x86/hvm/vmx/vmx.c Thu Jan 19 13:14:02 2012 +0100
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c Thu Jan 19 14:37:17 2012 +0100
>> @@ -1603,6 +1603,8 @@ static void vmx_cpuid_intercept(
>> break;
>> }
>>
>> + vpmu_do_cpuid(input, eax, ebx, ecx, edx);
>> +
>> HVMTRACE_5D (CPUID, input, *eax, *ebx, *ecx, *edx);
>> }
>>
>> diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vmx/vpmu_core2.c
>> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c Thu Jan 19 13:14:02 2012 +0100
>> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c Thu Jan 19 14:37:17 2012 +0100
>> @@ -598,6 +598,29 @@ static void core2_vpmu_destroy(struct vc
>> vpmu->flags &= ~VPMU_CONTEXT_ALLOCATED;
>> }
>>
>> +/**
>> + * core2_vpmu_cpuid - prepare special vpmu cpuid bits
>> + * If emulation of vpmu is switched off, some bits are swtiched off,
>> currently:
>> + * - EAX[0x1].EAX[Bits 0-7]: PMC revision id.
>> + * - EAX[0xa].EDX[Bit 21]: Debug Store
>> + */
>> +#define bitmaskof(idx) (1U << ((idx) & 31))
>> +static void core2_vpmu_cpuid(unsigned int input,
>> + unsigned int *eax, unsigned int *ebx,
>> + unsigned int *ecx, unsigned int *edx)
>> +{
>> + switch ( input )
>> + {
>> + case 0x1:
>> + *edx &= ~bitmaskof(X86_FEATURE_DS); /* Debug Store not supported
>> */
>> + break;
>> + case 0xa:
>> + if ( !opt_vpmu_enabled )
>> + *eax &= ~(unsigned int)0xff; /* Clear pmc version id. */
>> + break;
>> + }
>> +}
>> +
>> struct arch_vpmu_ops core2_vpmu_ops = {
>> .do_wrmsr = core2_vpmu_do_wrmsr,
>> .do_rdmsr = core2_vpmu_do_rdmsr,
>> @@ -605,7 +628,13 @@ struct arch_vpmu_ops core2_vpmu_ops = {
>> .arch_vpmu_initialise = core2_vpmu_initialise,
>> .arch_vpmu_destroy = core2_vpmu_destroy,
>> .arch_vpmu_save = core2_vpmu_save,
>> - .arch_vpmu_load = core2_vpmu_load
>> + .arch_vpmu_load = core2_vpmu_load,
>> + .arch_vpmu_cpuid = core2_vpmu_cpuid
>> +};
>> +
>> +/* Used if vpmu is disabled. */
>> +struct arch_vpmu_ops core2_vpmu_dis_ops = {
>> + .arch_vpmu_cpuid = core2_vpmu_cpuid
>> };
>>
>> int vmx_vpmu_initialize(struct vcpu *v)
>> @@ -615,7 +644,7 @@ int vmx_vpmu_initialize(struct vcpu *v)
>> __u8 cpu_model = current_cpu_data.x86_model;
>>
>> if ( !opt_vpmu_enabled )
>> - return -EINVAL;
>> + goto func_out;
>>
>> if ( family == 6 )
>> {
>> @@ -635,6 +664,11 @@ int vmx_vpmu_initialize(struct vcpu *v)
>> printk("VPMU: Initialization failed. "
>> "Intel processor family %d model %d has not "
>> "been supported\n", family, cpu_model);
>> +
>> +func_out:
>> +
>> + vpmu->arch_vpmu_ops = &core2_vpmu_dis_ops;
>> +
>> return -EINVAL;
>> }
>>
>> diff -r 7a82c2e2eb33 xen/arch/x86/hvm/vpmu.c
>> --- a/xen/arch/x86/hvm/vpmu.c Thu Jan 19 13:14:02 2012 +0100
>> +++ b/xen/arch/x86/hvm/vpmu.c Thu Jan 19 14:37:17 2012 +0100
>> @@ -39,7 +39,7 @@ int vpmu_do_wrmsr(unsigned int msr, uint
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>>
>> - if ( vpmu->arch_vpmu_ops )
>> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
>> return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
>> return 0;
>> }
>> @@ -48,7 +48,7 @@ int vpmu_do_rdmsr(unsigned int msr, uint
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>>
>> - if ( vpmu->arch_vpmu_ops )
>> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_rdmsr )
>> return vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content);
>> return 0;
>> }
>> @@ -57,7 +57,7 @@ int vpmu_do_interrupt(struct cpu_user_re
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>>
>> - if ( vpmu->arch_vpmu_ops )
>> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_interrupt )
>> return vpmu->arch_vpmu_ops->do_interrupt(regs);
>> return 0;
>> }
>> @@ -66,7 +66,7 @@ void vpmu_save(struct vcpu *v)
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>>
>> - if ( vpmu->arch_vpmu_ops )
>> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_save )
>> vpmu->arch_vpmu_ops->arch_vpmu_save(v);
>> }
>>
>> @@ -74,7 +74,7 @@ void vpmu_load(struct vcpu *v)
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>>
>> - if ( vpmu->arch_vpmu_ops )
>> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_load )
>> vpmu->arch_vpmu_ops->arch_vpmu_load(v);
>> }
>>
>> @@ -109,7 +109,8 @@ void vpmu_initialise(struct vcpu *v)
>> {
>> vpmu->flags = 0;
>> vpmu->context = NULL;
>> - vpmu->arch_vpmu_ops->arch_vpmu_initialise(v);
>> + if ( vpmu->arch_vpmu_ops->arch_vpmu_initialise )
>> + vpmu->arch_vpmu_ops->arch_vpmu_initialise(v);
>> }
>> }
>>
>> @@ -117,7 +118,17 @@ void vpmu_destroy(struct vcpu *v)
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>>
>> - if ( vpmu->arch_vpmu_ops )
>> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_destroy )
>> vpmu->arch_vpmu_ops->arch_vpmu_destroy(v);
>> }
>>
>> +void vpmu_do_cpuid(unsigned int input,
>> + unsigned int *eax, unsigned int *ebx,
>> + unsigned int *ecx, unsigned int *edx)
>> +{
>> + struct vpmu_struct *vpmu = vcpu_vpmu(current);
>> +
>> + if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_cpuid)
>> + vpmu->arch_vpmu_ops->arch_vpmu_cpuid(input, eax, ebx, ecx, edx);
>> +}
>> +
>> diff -r 7a82c2e2eb33 xen/include/asm-x86/hvm/vpmu.h
>> --- a/xen/include/asm-x86/hvm/vpmu.h Thu Jan 19 13:14:02 2012 +0100
>> +++ b/xen/include/asm-x86/hvm/vpmu.h Thu Jan 19 14:37:17 2012 +0100
>> @@ -56,6 +56,9 @@ struct arch_vpmu_ops {
>> void (*arch_vpmu_destroy)(struct vcpu *v);
>> void (*arch_vpmu_save)(struct vcpu *v);
>> void (*arch_vpmu_load)(struct vcpu *v);
>> + void (*arch_vpmu_cpuid)(unsigned int input,
>> + unsigned int *eax, unsigned int *ebx,
>> + unsigned int *ecx, unsigned int *edx);
>> };
>>
>> int vmx_vpmu_initialize(struct vcpu *v);
>> @@ -78,6 +81,9 @@ void vpmu_initialise(struct vcpu *v);
>> void vpmu_destroy(struct vcpu *v);
>> void vpmu_save(struct vcpu *v);
>> void vpmu_load(struct vcpu *v);
>> +void vpmu_do_cpuid(unsigned int input,
>> + unsigned int *eax, unsigned int *ebx,
>> + unsigned int *ecx, unsigned int *edx);
>>
>> extern int acquire_pmu_ownership(int pmu_ownership);
>> extern void release_pmu_ownership(int pmu_ownership);
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function [ In reply to ]

keir at xen

Jan 20, 2012, 2:54 AM

Post #5 of 11 (463 views)

Permalink

On 20/01/2012 10:49, "Dietmar Hahn" <dietmar.hahn@ts.fujitsu.com> wrote:

>> Our CPUID configuration is done per-domain, and from
>> tools/libxc/xc_cpuid_x86.c. CPUID adjustments implemented within the
>> hypervisor are generally not acceptable without very good reason.
>
> Then a way is needed to have access to the opt_vpmu_enabled variable within
> the
> hypervisor from the tools to decide the enabling of the flag (is there such a
> way?) or the mechanism with the boot variable must be changed.
> The opt_vpmu_enabled boot variable was introduced because of a PMU problem in
> the Nehalem cpus leading sometimes to hypervisor crashes. But with the done
> quirk we never had a crash anymore.
> So maybe we can always switch on the vpmu stuff in the hypervisor and add a
> flag in the domain configuration when somebody wants to do some performance
> tests?

Yes!

It's obviously an option of fairly narrow interest. If someone tries to
enable the per-domain option on a CPU which has problems, you can fail the
domain creation, or print a warning in the hypervisor log, or whatever. Any
sensible option in that respect is fine by me!

-- Keir

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function [ In reply to ]

dietmar.hahn at ts

Jan 20, 2012, 4:40 AM

Post #6 of 11 (470 views)

Permalink

Hi Ian,

Am Freitag 20 Januar 2012, 10:54:44 schrieb Keir Fraser:
> On 20/01/2012 10:49, "Dietmar Hahn" <dietmar.hahn@ts.fujitsu.com> wrote:
>
> >> Our CPUID configuration is done per-domain, and from
> >> tools/libxc/xc_cpuid_x86.c. CPUID adjustments implemented within the
> >> hypervisor are generally not acceptable without very good reason.
> >
> > Then a way is needed to have access to the opt_vpmu_enabled variable within
> > the
> > hypervisor from the tools to decide the enabling of the flag (is there such a
> > way?) or the mechanism with the boot variable must be changed.
> > The opt_vpmu_enabled boot variable was introduced because of a PMU problem in
> > the Nehalem cpus leading sometimes to hypervisor crashes. But with the done
> > quirk we never had a crash anymore.
> > So maybe we can always switch on the vpmu stuff in the hypervisor and add a
> > flag in the domain configuration when somebody wants to do some performance
> > tests?
>
> Yes!
>
> It's obviously an option of fairly narrow interest. If someone tries to
> enable the per-domain option on a CPU which has problems, you can fail the
> domain creation, or print a warning in the hypervisor log, or whatever. Any
> sensible option in that respect is fine by me!

What is the best solution for this?
A domain specific configuration option is needed (vpmu?) which is usable in
libxc/xc_cpuid_x86.c to select/deselect special vpmu bits in the cpuid command.
Can you point me to an proper example?
Thanks.

Dietmar.

--
Company details: http://ts.fujitsu.com/imprint.html

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function [ In reply to ]

Ian.Campbell at citrix

Jan 20, 2012, 4:45 AM

Post #7 of 11 (469 views)

Permalink

On Fri, 2012-01-20 at 12:40 +0000, Dietmar Hahn wrote:
> Hi Ian,
>
> Am Freitag 20 Januar 2012, 10:54:44 schrieb Keir Fraser:
> > On 20/01/2012 10:49, "Dietmar Hahn" <dietmar.hahn@ts.fujitsu.com> wrote:
> >
> > >> Our CPUID configuration is done per-domain, and from
> > >> tools/libxc/xc_cpuid_x86.c. CPUID adjustments implemented within the
> > >> hypervisor are generally not acceptable without very good reason.
> > >
> > > Then a way is needed to have access to the opt_vpmu_enabled variable within
> > > the
> > > hypervisor from the tools to decide the enabling of the flag (is there such a
> > > way?) or the mechanism with the boot variable must be changed.
> > > The opt_vpmu_enabled boot variable was introduced because of a PMU problem in
> > > the Nehalem cpus leading sometimes to hypervisor crashes. But with the done
> > > quirk we never had a crash anymore.
> > > So maybe we can always switch on the vpmu stuff in the hypervisor and add a
> > > flag in the domain configuration when somebody wants to do some performance
> > > tests?
> >
> > Yes!
> >
> > It's obviously an option of fairly narrow interest. If someone tries to
> > enable the per-domain option on a CPU which has problems, you can fail the
> > domain creation, or print a warning in the hypervisor log, or whatever. Any
> > sensible option in that respect is fine by me!
>
> What is the best solution for this?
> A domain specific configuration option is needed (vpmu?) which is usable in
> libxc/xc_cpuid_x86.c to select/deselect special vpmu bits in the cpuid command.
> Can you point me to an proper example?

Can't this already be done via the cpuid domain option given the correct
runes? Maybe with an addition to the table in libxl_cpuid_parse_config?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function [ In reply to ]

keir.xen at gmail

Jan 20, 2012, 5:33 AM

Post #8 of 11 (466 views)

Permalink

On 20/01/2012 12:45, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:

>>> It's obviously an option of fairly narrow interest. If someone tries to
>>> enable the per-domain option on a CPU which has problems, you can fail the
>>> domain creation, or print a warning in the hypervisor log, or whatever. Any
>>> sensible option in that respect is fine by me!
>>
>> What is the best solution for this?
>> A domain specific configuration option is needed (vpmu?) which is usable in
>> libxc/xc_cpuid_x86.c to select/deselect special vpmu bits in the cpuid
>> command.
>> Can you point me to an proper example?
>
> Can't this already be done via the cpuid domain option given the correct
> runes? Maybe with an addition to the table in libxl_cpuid_parse_config?

Yes that's probably the way to do it. If the resulting required
configuration runes are too cryptic or vendor-specific, it may make sense to
have the libxl cpuid logic consume a 'vpmu' option which it then turns into
a set of lower-level cpuid settings to eventually pass down to the code in
libxc/xc_cpuid.

It's a trifle messy I will admit. Arguably the 'default policy' bits of
xc_cpuid_x86.c would better belong in libxl these days, where we would have
better access to a domain's configuration state. As it is, we may end up
with a spread of default policy across Xen (for dom0), libxc, and libxl.

-- Keir

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function [ In reply to ]

Ian.Campbell at citrix

Jan 20, 2012, 5:37 AM

Post #9 of 11 (473 views)

Permalink

On Fri, 2012-01-20 at 13:33 +0000, Keir Fraser wrote:
> On 20/01/2012 12:45, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:
>
> >>> It's obviously an option of fairly narrow interest. If someone tries to
> >>> enable the per-domain option on a CPU which has problems, you can fail the
> >>> domain creation, or print a warning in the hypervisor log, or whatever. Any
> >>> sensible option in that respect is fine by me!
> >>
> >> What is the best solution for this?
> >> A domain specific configuration option is needed (vpmu?) which is usable in
> >> libxc/xc_cpuid_x86.c to select/deselect special vpmu bits in the cpuid
> >> command.
> >> Can you point me to an proper example?
> >
> > Can't this already be done via the cpuid domain option given the correct
> > runes? Maybe with an addition to the table in libxl_cpuid_parse_config?
>
> Yes that's probably the way to do it. If the resulting required
> configuration runes are too cryptic or vendor-specific, it may make sense to
> have the libxl cpuid logic consume a 'vpmu' option which it then turns into
> a set of lower-level cpuid settings to eventually pass down to the code in
> libxc/xc_cpuid.
>
> It's a trifle messy I will admit. Arguably the 'default policy' bits of
> xc_cpuid_x86.c would better belong in libxl these days, where we would have
> better access to a domain's configuration state. As it is, we may end up
> with a spread of default policy across Xen (for dom0), libxc, and libxl.

Plus a bunch of ad-hoc stuff which predates the cpuid bit-fiddling
support but is reflected in the cpuid (pae, apic, acpi etc).

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function [ In reply to ]

keir.xen at gmail

Jan 20, 2012, 5:47 AM

Post #10 of 11 (465 views)

Permalink

On 20/01/2012 13:37, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:

>> Yes that's probably the way to do it. If the resulting required
>> configuration runes are too cryptic or vendor-specific, it may make sense to
>> have the libxl cpuid logic consume a 'vpmu' option which it then turns into
>> a set of lower-level cpuid settings to eventually pass down to the code in
>> libxc/xc_cpuid.
>>
>> It's a trifle messy I will admit. Arguably the 'default policy' bits of
>> xc_cpuid_x86.c would better belong in libxl these days, where we would have
>> better access to a domain's configuration state. As it is, we may end up
>> with a spread of default policy across Xen (for dom0), libxc, and libxl.
>
> Plus a bunch of ad-hoc stuff which predates the cpuid bit-fiddling
> support but is reflected in the cpuid (pae, apic, acpi etc).

Well, this is done in libxc now, so I think I included it in my list. ;) But
it would be cleaner done in libxl.

I don't think anyone will be straining their arm to volunteer for this
cleanup, but we shouldn't be shy about putting new policy stuff in libxl if
it makes best sense to put it there.

-- Keir

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 2 of 2] vpmu: Add a vpmu cpuid function [ In reply to ]

dietmar.hahn at ts

Jan 20, 2012, 6:08 AM

Post #11 of 11 (470 views)

Permalink

Am Freitag 20 Januar 2012, 13:47:24 schrieb Keir Fraser:
> On 20/01/2012 13:37, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:
>
> >> Yes that's probably the way to do it. If the resulting required
> >> configuration runes are too cryptic or vendor-specific, it may make sense to
> >> have the libxl cpuid logic consume a 'vpmu' option which it then turns into
> >> a set of lower-level cpuid settings to eventually pass down to the code in
> >> libxc/xc_cpuid.
> >>
> >> It's a trifle messy I will admit. Arguably the 'default policy' bits of
> >> xc_cpuid_x86.c would better belong in libxl these days, where we would have
> >> better access to a domain's configuration state. As it is, we may end up
> >> with a spread of default policy across Xen (for dom0), libxc, and libxl.
> >
> > Plus a bunch of ad-hoc stuff which predates the cpuid bit-fiddling
> > support but is reflected in the cpuid (pae, apic, acpi etc).
>
> Well, this is done in libxc now, so I think I included it in my list. ;) But
> it would be cleaner done in libxl.
>
> I don't think anyone will be straining their arm to volunteer for this
> cleanup, but we shouldn't be shy about putting new policy stuff in libxl if
> it makes best sense to put it there.

Thanks for the hints. For the first time I'll go through the source and try to
understand what you are talking about.

Dietmar.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel