* [PATCH 0/4] VMX: DebugCtl MSR related adjustments
@ 2014-08-12 9:08 Jan Beulich
2014-08-12 9:14 ` [PATCH 1/4] VMX: fix DebugCtl MSR clearing Jan Beulich
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Jan Beulich @ 2014-08-12 9:08 UTC (permalink / raw)
To: xen-devel; +Cc: Kevin Tian, Eddie Dong, Jun Nakajima
1: fix DebugCtl MSR clearing
2: vPMU: fix DebugCtl MSR handling
3: allow RTM advanced debugging to be used by guests
4: vPMU: reduce core2_vpmu_initialise() verbosity
Signed-off-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/4] VMX: fix DebugCtl MSR clearing
2014-08-12 9:08 [PATCH 0/4] VMX: DebugCtl MSR related adjustments Jan Beulich
@ 2014-08-12 9:14 ` Jan Beulich
2014-08-12 9:37 ` Andrew Cooper
2014-08-13 18:48 ` Tian, Kevin
2014-08-12 9:15 ` [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling Jan Beulich
` (2 subsequent siblings)
3 siblings, 2 replies; 17+ messages in thread
From: Jan Beulich @ 2014-08-12 9:14 UTC (permalink / raw)
To: xen-devel; +Cc: Kevin Tian, Eddie Dong, Jun Nakajima
[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]
The previous shortcut was wrong, as it bypassed the necessary vmwrite:
All we really want to avoid if the guest writes zero is to add the MSR
to the host-load list.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2246,8 +2246,6 @@ static int vmx_msr_write_intercept(unsig
int i, rc = 0;
uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
- if ( !msr_content )
- break;
if ( msr_content & ~supported )
{
/* Perhaps some other bits are supported in vpmu. */
@@ -2267,12 +2265,10 @@ static int vmx_msr_write_intercept(unsig
}
if ( (rc < 0) ||
- (vmx_add_host_load_msr(msr) < 0) )
+ (msr_content && (vmx_add_host_load_msr(msr) < 0)) )
hvm_inject_hw_exception(TRAP_machine_check, 0);
else
- {
__vmwrite(GUEST_IA32_DEBUGCTL, msr_content);
- }
break;
}
[-- Attachment #2: VMX-debuctl-clearing.patch --]
[-- Type: text/plain, Size: 1075 bytes --]
VMX: fix DebugCtl MSR clearing
The previous shortcut was wrong, as it bypassed the necessary vmwrite:
All we really want to avoid if the guest writes zero is to add the MSR
to the host-load list.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2246,8 +2246,6 @@ static int vmx_msr_write_intercept(unsig
int i, rc = 0;
uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
- if ( !msr_content )
- break;
if ( msr_content & ~supported )
{
/* Perhaps some other bits are supported in vpmu. */
@@ -2267,12 +2265,10 @@ static int vmx_msr_write_intercept(unsig
}
if ( (rc < 0) ||
- (vmx_add_host_load_msr(msr) < 0) )
+ (msr_content && (vmx_add_host_load_msr(msr) < 0)) )
hvm_inject_hw_exception(TRAP_machine_check, 0);
else
- {
__vmwrite(GUEST_IA32_DEBUGCTL, msr_content);
- }
break;
}
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling
2014-08-12 9:08 [PATCH 0/4] VMX: DebugCtl MSR related adjustments Jan Beulich
2014-08-12 9:14 ` [PATCH 1/4] VMX: fix DebugCtl MSR clearing Jan Beulich
@ 2014-08-12 9:15 ` Jan Beulich
2014-08-12 9:44 ` Andrew Cooper
` (2 more replies)
2014-08-12 9:17 ` [PATCH 3/4] VMX: allow RTM advanced debugging to be used by guests Jan Beulich
2014-08-12 9:18 ` [PATCH 4/4] VMX/vPMU: reduce core2_vpmu_initialise() verbosity Jan Beulich
3 siblings, 3 replies; 17+ messages in thread
From: Jan Beulich @ 2014-08-12 9:15 UTC (permalink / raw)
To: xen-devel; +Cc: Kevin Tian, Eddie Dong, Jun Nakajima
[-- Attachment #1: Type: text/plain, Size: 5696 bytes --]
- writes with one of the vPMU-supported flags and some unsupported one
set got accepted, leading to a VM entry failure
- writes with one of the vPMU-supported flags set but the Debug Store
feature unavailable produced a #GP (other than other writes to this
MSR with unsupported bits set)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1789,7 +1789,7 @@ static int svm_msr_write_intercept(unsig
case MSR_AMD_FAM15H_EVNTSEL3:
case MSR_AMD_FAM15H_EVNTSEL4:
case MSR_AMD_FAM15H_EVNTSEL5:
- vpmu_do_wrmsr(msr, msr_content);
+ vpmu_do_wrmsr(msr, msr_content, 0);
break;
case MSR_IA32_MCx_MISC(4): /* Threshold register */
--- a/xen/arch/x86/hvm/svm/vpmu.c
+++ b/xen/arch/x86/hvm/svm/vpmu.c
@@ -278,11 +278,14 @@ static void context_update(unsigned int
}
}
-static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
+static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
+ uint64_t supported)
{
struct vcpu *v = current;
struct vpmu_struct *vpmu = vcpu_vpmu(v);
+ ASSERT(!supported);
+
/* For all counters, enable guest only mode for HVM guest */
if ( (get_pmu_reg_type(msr) == MSR_TYPE_CTRL) &&
!(is_guest_mode(msr_content)) )
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2249,7 +2249,7 @@ static int vmx_msr_write_intercept(unsig
if ( msr_content & ~supported )
{
/* Perhaps some other bits are supported in vpmu. */
- if ( !vpmu_do_wrmsr(msr, msr_content) )
+ if ( !vpmu_do_wrmsr(msr, msr_content, supported) )
break;
}
if ( msr_content & IA32_DEBUGCTLMSR_LBR )
@@ -2278,7 +2278,7 @@ static int vmx_msr_write_intercept(unsig
goto gp_fault;
break;
default:
- if ( vpmu_do_wrmsr(msr, msr_content) )
+ if ( vpmu_do_wrmsr(msr, msr_content, 0) )
return X86EMUL_OKAY;
if ( passive_domain_do_wrmsr(msr, msr_content) )
return X86EMUL_OKAY;
--- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
+++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
@@ -454,7 +454,8 @@ static int core2_vpmu_msr_common_check(u
return 1;
}
-static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
+static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
+ uint64_t supported)
{
u64 global_ctrl, non_global_ctrl;
char pmu_enable = 0;
@@ -469,24 +470,26 @@ static int core2_vpmu_do_wrmsr(unsigned
/* Special handling for BTS */
if ( msr == MSR_IA32_DEBUGCTLMSR )
{
- uint64_t supported = IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
- IA32_DEBUGCTLMSR_BTINT;
+ supported |= IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
+ IA32_DEBUGCTLMSR_BTINT;
if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS |
IA32_DEBUGCTLMSR_BTS_OFF_USR;
- if ( msr_content & supported )
- {
- if ( vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
- return 1;
- gdprintk(XENLOG_WARNING, "Debug Store is not supported on this cpu\n");
- hvm_inject_hw_exception(TRAP_gp_fault, 0);
- return 0;
- }
+ if ( !(msr_content & ~supported) &&
+ vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
+ return 1;
+ if ( (msr_content & supported) &&
+ !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
+ printk(XENLOG_G_WARNING
+ "%pv: Debug Store unsupported on this CPU\n",
+ current);
}
return 0;
}
+ ASSERT(!supported);
+
core2_vpmu_cxt = vpmu->context;
switch ( msr )
{
--- a/xen/arch/x86/hvm/vpmu.c
+++ b/xen/arch/x86/hvm/vpmu.c
@@ -64,12 +64,12 @@ static void __init parse_vpmu_param(char
}
}
-int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
+int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported)
{
struct vpmu_struct *vpmu = vcpu_vpmu(current);
if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
- return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
+ return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content, supported);
return 0;
}
--- a/xen/include/asm-x86/hvm/vpmu.h
+++ b/xen/include/asm-x86/hvm/vpmu.h
@@ -45,7 +45,8 @@
/* Arch specific operations shared by all vpmus */
struct arch_vpmu_ops {
- int (*do_wrmsr)(unsigned int msr, uint64_t msr_content);
+ int (*do_wrmsr)(unsigned int msr, uint64_t msr_content,
+ uint64_t supported);
int (*do_rdmsr)(unsigned int msr, uint64_t *msr_content);
int (*do_interrupt)(struct cpu_user_regs *regs);
void (*do_cpuid)(unsigned int input,
@@ -86,7 +87,7 @@ struct vpmu_struct {
#define vpmu_is_set(_vpmu, _x) ((_vpmu)->flags & (_x))
#define vpmu_clear(_vpmu) ((_vpmu)->flags = 0)
-int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content);
+int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported);
int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content);
int vpmu_do_interrupt(struct cpu_user_regs *regs);
void vpmu_do_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
[-- Attachment #2: VMX-vPMU-debugctl.patch --]
[-- Type: text/plain, Size: 5731 bytes --]
VMX/vPMU: fix DebugCtl MSR handling
- writes with one of the vPMU-supported flags and some unsupported one
set got accepted, leading to a VM entry failure
- writes with one of the vPMU-supported flags set but the Debug Store
feature unavailable produced a #GP (other than other writes to this
MSR with unsupported bits set)
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1789,7 +1789,7 @@ static int svm_msr_write_intercept(unsig
case MSR_AMD_FAM15H_EVNTSEL3:
case MSR_AMD_FAM15H_EVNTSEL4:
case MSR_AMD_FAM15H_EVNTSEL5:
- vpmu_do_wrmsr(msr, msr_content);
+ vpmu_do_wrmsr(msr, msr_content, 0);
break;
case MSR_IA32_MCx_MISC(4): /* Threshold register */
--- a/xen/arch/x86/hvm/svm/vpmu.c
+++ b/xen/arch/x86/hvm/svm/vpmu.c
@@ -278,11 +278,14 @@ static void context_update(unsigned int
}
}
-static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
+static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
+ uint64_t supported)
{
struct vcpu *v = current;
struct vpmu_struct *vpmu = vcpu_vpmu(v);
+ ASSERT(!supported);
+
/* For all counters, enable guest only mode for HVM guest */
if ( (get_pmu_reg_type(msr) == MSR_TYPE_CTRL) &&
!(is_guest_mode(msr_content)) )
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2249,7 +2249,7 @@ static int vmx_msr_write_intercept(unsig
if ( msr_content & ~supported )
{
/* Perhaps some other bits are supported in vpmu. */
- if ( !vpmu_do_wrmsr(msr, msr_content) )
+ if ( !vpmu_do_wrmsr(msr, msr_content, supported) )
break;
}
if ( msr_content & IA32_DEBUGCTLMSR_LBR )
@@ -2278,7 +2278,7 @@ static int vmx_msr_write_intercept(unsig
goto gp_fault;
break;
default:
- if ( vpmu_do_wrmsr(msr, msr_content) )
+ if ( vpmu_do_wrmsr(msr, msr_content, 0) )
return X86EMUL_OKAY;
if ( passive_domain_do_wrmsr(msr, msr_content) )
return X86EMUL_OKAY;
--- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
+++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
@@ -454,7 +454,8 @@ static int core2_vpmu_msr_common_check(u
return 1;
}
-static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
+static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
+ uint64_t supported)
{
u64 global_ctrl, non_global_ctrl;
char pmu_enable = 0;
@@ -469,24 +470,26 @@ static int core2_vpmu_do_wrmsr(unsigned
/* Special handling for BTS */
if ( msr == MSR_IA32_DEBUGCTLMSR )
{
- uint64_t supported = IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
- IA32_DEBUGCTLMSR_BTINT;
+ supported |= IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
+ IA32_DEBUGCTLMSR_BTINT;
if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS |
IA32_DEBUGCTLMSR_BTS_OFF_USR;
- if ( msr_content & supported )
- {
- if ( vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
- return 1;
- gdprintk(XENLOG_WARNING, "Debug Store is not supported on this cpu\n");
- hvm_inject_hw_exception(TRAP_gp_fault, 0);
- return 0;
- }
+ if ( !(msr_content & ~supported) &&
+ vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
+ return 1;
+ if ( (msr_content & supported) &&
+ !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
+ printk(XENLOG_G_WARNING
+ "%pv: Debug Store unsupported on this CPU\n",
+ current);
}
return 0;
}
+ ASSERT(!supported);
+
core2_vpmu_cxt = vpmu->context;
switch ( msr )
{
--- a/xen/arch/x86/hvm/vpmu.c
+++ b/xen/arch/x86/hvm/vpmu.c
@@ -64,12 +64,12 @@ static void __init parse_vpmu_param(char
}
}
-int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
+int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported)
{
struct vpmu_struct *vpmu = vcpu_vpmu(current);
if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
- return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
+ return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content, supported);
return 0;
}
--- a/xen/include/asm-x86/hvm/vpmu.h
+++ b/xen/include/asm-x86/hvm/vpmu.h
@@ -45,7 +45,8 @@
/* Arch specific operations shared by all vpmus */
struct arch_vpmu_ops {
- int (*do_wrmsr)(unsigned int msr, uint64_t msr_content);
+ int (*do_wrmsr)(unsigned int msr, uint64_t msr_content,
+ uint64_t supported);
int (*do_rdmsr)(unsigned int msr, uint64_t *msr_content);
int (*do_interrupt)(struct cpu_user_regs *regs);
void (*do_cpuid)(unsigned int input,
@@ -86,7 +87,7 @@ struct vpmu_struct {
#define vpmu_is_set(_vpmu, _x) ((_vpmu)->flags & (_x))
#define vpmu_clear(_vpmu) ((_vpmu)->flags = 0)
-int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content);
+int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported);
int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content);
int vpmu_do_interrupt(struct cpu_user_regs *regs);
void vpmu_do_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4] VMX: allow RTM advanced debugging to be used by guests
2014-08-12 9:08 [PATCH 0/4] VMX: DebugCtl MSR related adjustments Jan Beulich
2014-08-12 9:14 ` [PATCH 1/4] VMX: fix DebugCtl MSR clearing Jan Beulich
2014-08-12 9:15 ` [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling Jan Beulich
@ 2014-08-12 9:17 ` Jan Beulich
2014-08-12 10:08 ` Andrew Cooper
2014-08-13 18:52 ` Tian, Kevin
2014-08-12 9:18 ` [PATCH 4/4] VMX/vPMU: reduce core2_vpmu_initialise() verbosity Jan Beulich
3 siblings, 2 replies; 17+ messages in thread
From: Jan Beulich @ 2014-08-12 9:17 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser, Kevin Tian, Eddie Dong, Jun Nakajima
[-- Attachment #1: Type: text/plain, Size: 2186 bytes --]
All that is needed here is allowing the respective DebugCtl MSR bit to
be set by the guest.
At once - even if PV guests can't currently use it due to missing
DebugCtl MSR virtualization - make the respective adjustments to
debugreg.h.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2246,6 +2246,8 @@ static int vmx_msr_write_intercept(unsig
int i, rc = 0;
uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
+ if ( boot_cpu_has(X86_FEATURE_RTM) )
+ supported |= IA32_DEBUGCTLMSR_RTM;
if ( msr_content & ~supported )
{
/* Perhaps some other bits are supported in vpmu. */
--- a/xen/include/asm-x86/debugreg.h
+++ b/xen/include/asm-x86/debugreg.h
@@ -20,6 +20,7 @@
#define DR_TRAP3 (0x8) /* db3 */
#define DR_STEP (0x4000) /* single-step */
#define DR_SWITCH (0x8000) /* task switch */
+#define DR_NOT_RTM (0x10000) /* clear: #BP inside RTM region */
/* Now define a bunch of things for manipulating the control register.
The top two bytes of the control register consist of 4 fields of 4
@@ -62,6 +63,7 @@
#define DR_CONTROL_RESERVED_ONE (0x00000400ul) /* Reserved, read as one */
#define DR_LOCAL_EXACT_ENABLE (0x00000100ul) /* Local exact enable */
#define DR_GLOBAL_EXACT_ENABLE (0x00000200ul) /* Global exact enable */
+#define DR_RTM_ENABLE (0x00000800ul) /* RTM debugging enable */
#define DR_GENERAL_DETECT (0x00002000ul) /* General detect enable */
#define write_debugreg(reg, val) do { \
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -79,6 +79,7 @@
#define IA32_DEBUGCTLMSR_BTINT (1<<8) /* Branch Trace Interrupt */
#define IA32_DEBUGCTLMSR_BTS_OFF_OS (1<<9) /* BTS off if CPL 0 */
#define IA32_DEBUGCTLMSR_BTS_OFF_USR (1<<10) /* BTS off if CPL > 0 */
+#define IA32_DEBUGCTLMSR_RTM (1<<15) /* RTM debugging enable */
#define MSR_IA32_LASTBRANCHFROMIP 0x000001db
#define MSR_IA32_LASTBRANCHTOIP 0x000001dc
[-- Attachment #2: VMX-RTM-debugging.patch --]
[-- Type: text/plain, Size: 2238 bytes --]
VMX: allow RTM advanced debugging to be used by guests
All that is needed here is allowing the respective DebugCtl MSR bit to
be set by the guest.
At once - even if PV guests can't currently use it due to missing
DebugCtl MSR virtualization - make the respective adjustments to
debugreg.h.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2246,6 +2246,8 @@ static int vmx_msr_write_intercept(unsig
int i, rc = 0;
uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
+ if ( boot_cpu_has(X86_FEATURE_RTM) )
+ supported |= IA32_DEBUGCTLMSR_RTM;
if ( msr_content & ~supported )
{
/* Perhaps some other bits are supported in vpmu. */
--- a/xen/include/asm-x86/debugreg.h
+++ b/xen/include/asm-x86/debugreg.h
@@ -20,6 +20,7 @@
#define DR_TRAP3 (0x8) /* db3 */
#define DR_STEP (0x4000) /* single-step */
#define DR_SWITCH (0x8000) /* task switch */
+#define DR_NOT_RTM (0x10000) /* clear: #BP inside RTM region */
/* Now define a bunch of things for manipulating the control register.
The top two bytes of the control register consist of 4 fields of 4
@@ -62,6 +63,7 @@
#define DR_CONTROL_RESERVED_ONE (0x00000400ul) /* Reserved, read as one */
#define DR_LOCAL_EXACT_ENABLE (0x00000100ul) /* Local exact enable */
#define DR_GLOBAL_EXACT_ENABLE (0x00000200ul) /* Global exact enable */
+#define DR_RTM_ENABLE (0x00000800ul) /* RTM debugging enable */
#define DR_GENERAL_DETECT (0x00002000ul) /* General detect enable */
#define write_debugreg(reg, val) do { \
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -79,6 +79,7 @@
#define IA32_DEBUGCTLMSR_BTINT (1<<8) /* Branch Trace Interrupt */
#define IA32_DEBUGCTLMSR_BTS_OFF_OS (1<<9) /* BTS off if CPL 0 */
#define IA32_DEBUGCTLMSR_BTS_OFF_USR (1<<10) /* BTS off if CPL > 0 */
+#define IA32_DEBUGCTLMSR_RTM (1<<15) /* RTM debugging enable */
#define MSR_IA32_LASTBRANCHFROMIP 0x000001db
#define MSR_IA32_LASTBRANCHTOIP 0x000001dc
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/4] VMX/vPMU: reduce core2_vpmu_initialise() verbosity
2014-08-12 9:08 [PATCH 0/4] VMX: DebugCtl MSR related adjustments Jan Beulich
` (2 preceding siblings ...)
2014-08-12 9:17 ` [PATCH 3/4] VMX: allow RTM advanced debugging to be used by guests Jan Beulich
@ 2014-08-12 9:18 ` Jan Beulich
2014-08-12 9:29 ` Andrew Cooper
2014-08-13 18:53 ` Tian, Kevin
3 siblings, 2 replies; 17+ messages in thread
From: Jan Beulich @ 2014-08-12 9:18 UTC (permalink / raw)
To: xen-devel; +Cc: Kevin Tian, Eddie Dong, Jun Nakajima
[-- Attachment #1: Type: text/plain, Size: 2704 bytes --]
No need to print these messages for each vCPU, even more, no need to
print them for each domain - they all depend on CPU features that are
either there or not.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
+++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
@@ -764,19 +764,19 @@ static int core2_vpmu_initialise(struct
{
struct vpmu_struct *vpmu = vcpu_vpmu(v);
u64 msr_content;
- struct cpuinfo_x86 *c = ¤t_cpu_data;
+ static bool_t ds_warned;
if ( !(vpmu_flags & VPMU_BOOT_BTS) )
goto func_out;
/* Check the 'Debug Store' feature in the CPUID.EAX[1]:EDX[21] */
- if ( cpu_has(c, X86_FEATURE_DS) )
+ while ( boot_cpu_has(X86_FEATURE_DS) )
{
- if ( !cpu_has(c, X86_FEATURE_DTES64) )
+ if ( !boot_cpu_has(X86_FEATURE_DTES64) )
{
- printk(XENLOG_G_WARNING "CPU doesn't support 64-bit DS Area"
- " - Debug Store disabled for %pv\n",
- v);
- goto func_out;
+ if ( !ds_warned )
+ printk(XENLOG_G_WARNING "CPU doesn't support 64-bit DS Area"
+ " - Debug Store disabled for guests\n");
+ break;
}
vpmu_set(vpmu, VPMU_CPU_HAS_DS);
rdmsrl(MSR_IA32_MISC_ENABLE, msr_content);
@@ -784,14 +784,16 @@ static int core2_vpmu_initialise(struct
{
/* If BTS_UNAVAIL is set reset the DS feature. */
vpmu_reset(vpmu, VPMU_CPU_HAS_DS);
- printk(XENLOG_G_WARNING "CPU has set BTS_UNAVAIL"
- " - Debug Store disabled for %pv\n",
- v);
+ if ( !ds_warned )
+ printk(XENLOG_G_WARNING "CPU has set BTS_UNAVAIL"
+ " - Debug Store disabled for guests\n");
+ break;
}
- else
+
+ vpmu_set(vpmu, VPMU_CPU_HAS_BTS);
+ if ( !ds_warned )
{
- vpmu_set(vpmu, VPMU_CPU_HAS_BTS);
- if ( !cpu_has(c, X86_FEATURE_DSCPL) )
+ if ( !boot_cpu_has(X86_FEATURE_DSCPL) )
printk(XENLOG_G_INFO
"vpmu: CPU doesn't support CPL-Qualified BTS\n");
printk("******************************************************\n");
@@ -803,8 +805,10 @@ static int core2_vpmu_initialise(struct
printk("** It is NOT recommended for production use! **\n");
printk("******************************************************\n");
}
+ break;
}
-func_out:
+ ds_warned = 1;
+ func_out:
check_pmc_quirk();
return 0;
}
[-- Attachment #2: VMX-vPMU-verbosity.patch --]
[-- Type: text/plain, Size: 2752 bytes --]
VMX/vPMU: reduce core2_vpmu_initialise() verbosity
No need to print these messages for each vCPU, even more, no need to
print them for each domain - they all depend on CPU features that are
either there or not.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
+++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
@@ -764,19 +764,19 @@ static int core2_vpmu_initialise(struct
{
struct vpmu_struct *vpmu = vcpu_vpmu(v);
u64 msr_content;
- struct cpuinfo_x86 *c = ¤t_cpu_data;
+ static bool_t ds_warned;
if ( !(vpmu_flags & VPMU_BOOT_BTS) )
goto func_out;
/* Check the 'Debug Store' feature in the CPUID.EAX[1]:EDX[21] */
- if ( cpu_has(c, X86_FEATURE_DS) )
+ while ( boot_cpu_has(X86_FEATURE_DS) )
{
- if ( !cpu_has(c, X86_FEATURE_DTES64) )
+ if ( !boot_cpu_has(X86_FEATURE_DTES64) )
{
- printk(XENLOG_G_WARNING "CPU doesn't support 64-bit DS Area"
- " - Debug Store disabled for %pv\n",
- v);
- goto func_out;
+ if ( !ds_warned )
+ printk(XENLOG_G_WARNING "CPU doesn't support 64-bit DS Area"
+ " - Debug Store disabled for guests\n");
+ break;
}
vpmu_set(vpmu, VPMU_CPU_HAS_DS);
rdmsrl(MSR_IA32_MISC_ENABLE, msr_content);
@@ -784,14 +784,16 @@ static int core2_vpmu_initialise(struct
{
/* If BTS_UNAVAIL is set reset the DS feature. */
vpmu_reset(vpmu, VPMU_CPU_HAS_DS);
- printk(XENLOG_G_WARNING "CPU has set BTS_UNAVAIL"
- " - Debug Store disabled for %pv\n",
- v);
+ if ( !ds_warned )
+ printk(XENLOG_G_WARNING "CPU has set BTS_UNAVAIL"
+ " - Debug Store disabled for guests\n");
+ break;
}
- else
+
+ vpmu_set(vpmu, VPMU_CPU_HAS_BTS);
+ if ( !ds_warned )
{
- vpmu_set(vpmu, VPMU_CPU_HAS_BTS);
- if ( !cpu_has(c, X86_FEATURE_DSCPL) )
+ if ( !boot_cpu_has(X86_FEATURE_DSCPL) )
printk(XENLOG_G_INFO
"vpmu: CPU doesn't support CPL-Qualified BTS\n");
printk("******************************************************\n");
@@ -803,8 +805,10 @@ static int core2_vpmu_initialise(struct
printk("** It is NOT recommended for production use! **\n");
printk("******************************************************\n");
}
+ break;
}
-func_out:
+ ds_warned = 1;
+ func_out:
check_pmc_quirk();
return 0;
}
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] VMX/vPMU: reduce core2_vpmu_initialise() verbosity
2014-08-12 9:18 ` [PATCH 4/4] VMX/vPMU: reduce core2_vpmu_initialise() verbosity Jan Beulich
@ 2014-08-12 9:29 ` Andrew Cooper
2014-08-13 18:53 ` Tian, Kevin
1 sibling, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2014-08-12 9:29 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Kevin Tian, Eddie Dong, Jun Nakajima
[-- Attachment #1.1: Type: text/plain, Size: 3006 bytes --]
On 12/08/14 10:18, Jan Beulich wrote:
> No need to print these messages for each vCPU, even more, no need to
> print them for each domain - they all depend on CPU features that are
> either there or not.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> @@ -764,19 +764,19 @@ static int core2_vpmu_initialise(struct
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> u64 msr_content;
> - struct cpuinfo_x86 *c = ¤t_cpu_data;
> + static bool_t ds_warned;
>
> if ( !(vpmu_flags & VPMU_BOOT_BTS) )
> goto func_out;
> /* Check the 'Debug Store' feature in the CPUID.EAX[1]:EDX[21] */
> - if ( cpu_has(c, X86_FEATURE_DS) )
> + while ( boot_cpu_has(X86_FEATURE_DS) )
> {
> - if ( !cpu_has(c, X86_FEATURE_DTES64) )
> + if ( !boot_cpu_has(X86_FEATURE_DTES64) )
> {
> - printk(XENLOG_G_WARNING "CPU doesn't support 64-bit DS Area"
> - " - Debug Store disabled for %pv\n",
> - v);
> - goto func_out;
> + if ( !ds_warned )
> + printk(XENLOG_G_WARNING "CPU doesn't support 64-bit DS Area"
> + " - Debug Store disabled for guests\n");
> + break;
> }
> vpmu_set(vpmu, VPMU_CPU_HAS_DS);
> rdmsrl(MSR_IA32_MISC_ENABLE, msr_content);
> @@ -784,14 +784,16 @@ static int core2_vpmu_initialise(struct
> {
> /* If BTS_UNAVAIL is set reset the DS feature. */
> vpmu_reset(vpmu, VPMU_CPU_HAS_DS);
> - printk(XENLOG_G_WARNING "CPU has set BTS_UNAVAIL"
> - " - Debug Store disabled for %pv\n",
> - v);
> + if ( !ds_warned )
> + printk(XENLOG_G_WARNING "CPU has set BTS_UNAVAIL"
> + " - Debug Store disabled for guests\n");
> + break;
> }
> - else
> +
> + vpmu_set(vpmu, VPMU_CPU_HAS_BTS);
> + if ( !ds_warned )
> {
> - vpmu_set(vpmu, VPMU_CPU_HAS_BTS);
> - if ( !cpu_has(c, X86_FEATURE_DSCPL) )
> + if ( !boot_cpu_has(X86_FEATURE_DSCPL) )
> printk(XENLOG_G_INFO
> "vpmu: CPU doesn't support CPL-Qualified BTS\n");
> printk("******************************************************\n");
> @@ -803,8 +805,10 @@ static int core2_vpmu_initialise(struct
> printk("** It is NOT recommended for production use! **\n");
> printk("******************************************************\n");
> }
> + break;
> }
> -func_out:
> + ds_warned = 1;
> + func_out:
> check_pmc_quirk();
> return 0;
> }
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 3798 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] VMX: fix DebugCtl MSR clearing
2014-08-12 9:14 ` [PATCH 1/4] VMX: fix DebugCtl MSR clearing Jan Beulich
@ 2014-08-12 9:37 ` Andrew Cooper
2014-08-13 18:48 ` Tian, Kevin
1 sibling, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2014-08-12 9:37 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Kevin Tian, Eddie Dong, Jun Nakajima
[-- Attachment #1.1: Type: text/plain, Size: 1310 bytes --]
On 12/08/14 10:14, Jan Beulich wrote:
> The previous shortcut was wrong, as it bypassed the necessary vmwrite:
> All we really want to avoid if the guest writes zero is to add the MSR
> to the host-load list.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2246,8 +2246,6 @@ static int vmx_msr_write_intercept(unsig
> int i, rc = 0;
> uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
>
> - if ( !msr_content )
> - break;
> if ( msr_content & ~supported )
> {
> /* Perhaps some other bits are supported in vpmu. */
> @@ -2267,12 +2265,10 @@ static int vmx_msr_write_intercept(unsig
> }
>
> if ( (rc < 0) ||
> - (vmx_add_host_load_msr(msr) < 0) )
> + (msr_content && (vmx_add_host_load_msr(msr) < 0)) )
> hvm_inject_hw_exception(TRAP_machine_check, 0);
> else
> - {
> __vmwrite(GUEST_IA32_DEBUGCTL, msr_content);
> - }
>
> break;
> }
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 2193 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling
2014-08-12 9:15 ` [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling Jan Beulich
@ 2014-08-12 9:44 ` Andrew Cooper
2014-08-12 9:51 ` Jan Beulich
2014-08-12 17:47 ` Boris Ostrovsky
2014-08-13 18:52 ` Tian, Kevin
2 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2014-08-12 9:44 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Kevin Tian, Eddie Dong, Jun Nakajima
[-- Attachment #1.1: Type: text/plain, Size: 6184 bytes --]
On 12/08/14 10:15, Jan Beulich wrote:
> - writes with one of the vPMU-supported flags and some unsupported one
> set got accepted, leading to a VM entry failure
> - writes with one of the vPMU-supported flags set but the Debug Store
> feature unavailable produced a #GP (other than other writes to this
> MSR with unsupported bits set)
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1789,7 +1789,7 @@ static int svm_msr_write_intercept(unsig
> case MSR_AMD_FAM15H_EVNTSEL3:
> case MSR_AMD_FAM15H_EVNTSEL4:
> case MSR_AMD_FAM15H_EVNTSEL5:
> - vpmu_do_wrmsr(msr, msr_content);
> + vpmu_do_wrmsr(msr, msr_content, 0);
> break;
>
> case MSR_IA32_MCx_MISC(4): /* Threshold register */
> --- a/xen/arch/x86/hvm/svm/vpmu.c
> +++ b/xen/arch/x86/hvm/svm/vpmu.c
> @@ -278,11 +278,14 @@ static void context_update(unsigned int
> }
> }
>
> -static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> +static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
> + uint64_t supported)
> {
> struct vcpu *v = current;
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>
> + ASSERT(!supported);
> +
> /* For all counters, enable guest only mode for HVM guest */
> if ( (get_pmu_reg_type(msr) == MSR_TYPE_CTRL) &&
> !(is_guest_mode(msr_content)) )
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2249,7 +2249,7 @@ static int vmx_msr_write_intercept(unsig
> if ( msr_content & ~supported )
> {
> /* Perhaps some other bits are supported in vpmu. */
> - if ( !vpmu_do_wrmsr(msr, msr_content) )
> + if ( !vpmu_do_wrmsr(msr, msr_content, supported) )
> break;
> }
> if ( msr_content & IA32_DEBUGCTLMSR_LBR )
> @@ -2278,7 +2278,7 @@ static int vmx_msr_write_intercept(unsig
> goto gp_fault;
> break;
> default:
> - if ( vpmu_do_wrmsr(msr, msr_content) )
> + if ( vpmu_do_wrmsr(msr, msr_content, 0) )
> return X86EMUL_OKAY;
> if ( passive_domain_do_wrmsr(msr, msr_content) )
> return X86EMUL_OKAY;
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> @@ -454,7 +454,8 @@ static int core2_vpmu_msr_common_check(u
> return 1;
> }
>
> -static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> +static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
> + uint64_t supported)
> {
> u64 global_ctrl, non_global_ctrl;
> char pmu_enable = 0;
> @@ -469,24 +470,26 @@ static int core2_vpmu_do_wrmsr(unsigned
> /* Special handling for BTS */
> if ( msr == MSR_IA32_DEBUGCTLMSR )
> {
> - uint64_t supported = IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
> - IA32_DEBUGCTLMSR_BTINT;
> + supported |= IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
> + IA32_DEBUGCTLMSR_BTINT;
>
> if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
> supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS |
> IA32_DEBUGCTLMSR_BTS_OFF_USR;
> - if ( msr_content & supported )
> - {
> - if ( vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
> - return 1;
> - gdprintk(XENLOG_WARNING, "Debug Store is not supported on this cpu\n");
> - hvm_inject_hw_exception(TRAP_gp_fault, 0);
> - return 0;
> - }
> + if ( !(msr_content & ~supported) &&
> + vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
> + return 1;
> + if ( (msr_content & supported) &&
> + !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
> + printk(XENLOG_G_WARNING
> + "%pv: Debug Store unsupported on this CPU\n",
> + current);
> }
> return 0;
> }
>
> + ASSERT(!supported);
> +
> core2_vpmu_cxt = vpmu->context;
> switch ( msr )
> {
> --- a/xen/arch/x86/hvm/vpmu.c
> +++ b/xen/arch/x86/hvm/vpmu.c
> @@ -64,12 +64,12 @@ static void __init parse_vpmu_param(char
> }
> }
>
> -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> +int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported)
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>
> if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
> - return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
> + return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content, supported);
> return 0;
> }
>
> --- a/xen/include/asm-x86/hvm/vpmu.h
> +++ b/xen/include/asm-x86/hvm/vpmu.h
> @@ -45,7 +45,8 @@
>
> /* Arch specific operations shared by all vpmus */
> struct arch_vpmu_ops {
> - int (*do_wrmsr)(unsigned int msr, uint64_t msr_content);
> + int (*do_wrmsr)(unsigned int msr, uint64_t msr_content,
> + uint64_t supported);
> int (*do_rdmsr)(unsigned int msr, uint64_t *msr_content);
> int (*do_interrupt)(struct cpu_user_regs *regs);
> void (*do_cpuid)(unsigned int input,
> @@ -86,7 +87,7 @@ struct vpmu_struct {
> #define vpmu_is_set(_vpmu, _x) ((_vpmu)->flags & (_x))
> #define vpmu_clear(_vpmu) ((_vpmu)->flags = 0)
>
> -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content);
> +int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported);
It might, for clarity sake, be preferable to name the new parameter
"supported_bits". At the moment, "supported" could equally well refer
to the MSR itself.
~Andrew
> int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content);
> int vpmu_do_interrupt(struct cpu_user_regs *regs);
> void vpmu_do_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 6857 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling
2014-08-12 9:44 ` Andrew Cooper
@ 2014-08-12 9:51 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2014-08-12 9:51 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Kevin Tian, Eddie Dong, JunNakajima
>>> On 12.08.14 at 11:44, <andrew.cooper3@citrix.com> wrote:
> On 12/08/14 10:15, Jan Beulich wrote:
>> - writes with one of the vPMU-supported flags and some unsupported one
>> set got accepted, leading to a VM entry failure
>> - writes with one of the vPMU-supported flags set but the Debug Store
>> feature unavailable produced a #GP (other than other writes to this
>> MSR with unsupported bits set)
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -1789,7 +1789,7 @@ static int svm_msr_write_intercept(unsig
>> case MSR_AMD_FAM15H_EVNTSEL3:
>> case MSR_AMD_FAM15H_EVNTSEL4:
>> case MSR_AMD_FAM15H_EVNTSEL5:
>> - vpmu_do_wrmsr(msr, msr_content);
>> + vpmu_do_wrmsr(msr, msr_content, 0);
>> break;
>>
>> case MSR_IA32_MCx_MISC(4): /* Threshold register */
>> --- a/xen/arch/x86/hvm/svm/vpmu.c
>> +++ b/xen/arch/x86/hvm/svm/vpmu.c
>> @@ -278,11 +278,14 @@ static void context_update(unsigned int
>> }
>> }
>>
>> -static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
>> +static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
>> + uint64_t supported)
>> {
>> struct vcpu *v = current;
>> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>>
>> + ASSERT(!supported);
>> +
>> /* For all counters, enable guest only mode for HVM guest */
>> if ( (get_pmu_reg_type(msr) == MSR_TYPE_CTRL) &&
>> !(is_guest_mode(msr_content)) )
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -2249,7 +2249,7 @@ static int vmx_msr_write_intercept(unsig
>> if ( msr_content & ~supported )
>> {
>> /* Perhaps some other bits are supported in vpmu. */
>> - if ( !vpmu_do_wrmsr(msr, msr_content) )
>> + if ( !vpmu_do_wrmsr(msr, msr_content, supported) )
>> break;
>> }
>> if ( msr_content & IA32_DEBUGCTLMSR_LBR )
>> @@ -2278,7 +2278,7 @@ static int vmx_msr_write_intercept(unsig
>> goto gp_fault;
>> break;
>> default:
>> - if ( vpmu_do_wrmsr(msr, msr_content) )
>> + if ( vpmu_do_wrmsr(msr, msr_content, 0) )
>> return X86EMUL_OKAY;
>> if ( passive_domain_do_wrmsr(msr, msr_content) )
>> return X86EMUL_OKAY;
>> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
>> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
>> @@ -454,7 +454,8 @@ static int core2_vpmu_msr_common_check(u
>> return 1;
>> }
>>
>> -static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
>> +static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
>> + uint64_t supported)
>> {
>> u64 global_ctrl, non_global_ctrl;
>> char pmu_enable = 0;
>> @@ -469,24 +470,26 @@ static int core2_vpmu_do_wrmsr(unsigned
>> /* Special handling for BTS */
>> if ( msr == MSR_IA32_DEBUGCTLMSR )
>> {
>> - uint64_t supported = IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS
> |
>> - IA32_DEBUGCTLMSR_BTINT;
>> + supported |= IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
>> + IA32_DEBUGCTLMSR_BTINT;
>>
>> if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
>> supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS |
>> IA32_DEBUGCTLMSR_BTS_OFF_USR;
>> - if ( msr_content & supported )
>> - {
>> - if ( vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
>> - return 1;
>> - gdprintk(XENLOG_WARNING, "Debug Store is not supported on
> this cpu\n");
>> - hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> - return 0;
>> - }
>> + if ( !(msr_content & ~supported) &&
>> + vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
>> + return 1;
>> + if ( (msr_content & supported) &&
>> + !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
>> + printk(XENLOG_G_WARNING
>> + "%pv: Debug Store unsupported on this CPU\n",
>> + current);
>> }
>> return 0;
>> }
>>
>> + ASSERT(!supported);
>> +
>> core2_vpmu_cxt = vpmu->context;
>> switch ( msr )
>> {
>> --- a/xen/arch/x86/hvm/vpmu.c
>> +++ b/xen/arch/x86/hvm/vpmu.c
>> @@ -64,12 +64,12 @@ static void __init parse_vpmu_param(char
>> }
>> }
>>
>> -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
>> +int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t
> supported)
>> {
>> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>>
>> if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
>> - return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
>> + return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content, supported);
>> return 0;
>> }
>>
>> --- a/xen/include/asm-x86/hvm/vpmu.h
>> +++ b/xen/include/asm-x86/hvm/vpmu.h
>> @@ -45,7 +45,8 @@
>>
>> /* Arch specific operations shared by all vpmus */
>> struct arch_vpmu_ops {
>> - int (*do_wrmsr)(unsigned int msr, uint64_t msr_content);
>> + int (*do_wrmsr)(unsigned int msr, uint64_t msr_content,
>> + uint64_t supported);
>> int (*do_rdmsr)(unsigned int msr, uint64_t *msr_content);
>> int (*do_interrupt)(struct cpu_user_regs *regs);
>> void (*do_cpuid)(unsigned int input,
>> @@ -86,7 +87,7 @@ struct vpmu_struct {
>> #define vpmu_is_set(_vpmu, _x) ((_vpmu)->flags & (_x))
>> #define vpmu_clear(_vpmu) ((_vpmu)->flags = 0)
>>
>> -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content);
>> +int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t
> supported);
>
> It might, for clarity sake, be preferable to name the new parameter
> "supported_bits". At the moment, "supported" could equally well refer
> to the MSR itself.
Not really, with it being of type uint64_t. Nor does the context
really suggest a meaning like what you imply.
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] VMX: allow RTM advanced debugging to be used by guests
2014-08-12 9:17 ` [PATCH 3/4] VMX: allow RTM advanced debugging to be used by guests Jan Beulich
@ 2014-08-12 10:08 ` Andrew Cooper
2014-08-12 10:40 ` Jan Beulich
2014-08-13 18:52 ` Tian, Kevin
1 sibling, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2014-08-12 10:08 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Eddie Dong, Kevin Tian, Keir Fraser, Jun Nakajima
[-- Attachment #1.1: Type: text/plain, Size: 2554 bytes --]
On 12/08/14 10:17, Jan Beulich wrote:
> All that is needed here is allowing the respective DebugCtl MSR bit to
> be set by the guest.
>
> At once - even if PV guests can't currently use it due to missing
> DebugCtl MSR virtualization - make the respective adjustments to
> debugreg.h.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2246,6 +2246,8 @@ static int vmx_msr_write_intercept(unsig
> int i, rc = 0;
> uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
>
> + if ( boot_cpu_has(X86_FEATURE_RTM) )
> + supported |= IA32_DEBUGCTLMSR_RTM;
This supported bitmask is runtime constant. Is it worth precalculating
it, rather than reevaluating each time DEBUGCTL is written to?
~Andrew
> if ( msr_content & ~supported )
> {
> /* Perhaps some other bits are supported in vpmu. */
> --- a/xen/include/asm-x86/debugreg.h
> +++ b/xen/include/asm-x86/debugreg.h
> @@ -20,6 +20,7 @@
> #define DR_TRAP3 (0x8) /* db3 */
> #define DR_STEP (0x4000) /* single-step */
> #define DR_SWITCH (0x8000) /* task switch */
> +#define DR_NOT_RTM (0x10000) /* clear: #BP inside RTM region */
>
> /* Now define a bunch of things for manipulating the control register.
> The top two bytes of the control register consist of 4 fields of 4
> @@ -62,6 +63,7 @@
> #define DR_CONTROL_RESERVED_ONE (0x00000400ul) /* Reserved, read as one */
> #define DR_LOCAL_EXACT_ENABLE (0x00000100ul) /* Local exact enable */
> #define DR_GLOBAL_EXACT_ENABLE (0x00000200ul) /* Global exact enable */
> +#define DR_RTM_ENABLE (0x00000800ul) /* RTM debugging enable */
> #define DR_GENERAL_DETECT (0x00002000ul) /* General detect enable */
>
> #define write_debugreg(reg, val) do { \
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -79,6 +79,7 @@
> #define IA32_DEBUGCTLMSR_BTINT (1<<8) /* Branch Trace Interrupt */
> #define IA32_DEBUGCTLMSR_BTS_OFF_OS (1<<9) /* BTS off if CPL 0 */
> #define IA32_DEBUGCTLMSR_BTS_OFF_USR (1<<10) /* BTS off if CPL > 0 */
> +#define IA32_DEBUGCTLMSR_RTM (1<<15) /* RTM debugging enable */
>
> #define MSR_IA32_LASTBRANCHFROMIP 0x000001db
> #define MSR_IA32_LASTBRANCHTOIP 0x000001dc
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 3360 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] VMX: allow RTM advanced debugging to be used by guests
2014-08-12 10:08 ` Andrew Cooper
@ 2014-08-12 10:40 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2014-08-12 10:40 UTC (permalink / raw)
To: Andrew Cooper
Cc: xen-devel, Kevin Tian, Keir Fraser, Eddie Dong, Jun Nakajima
>>> On 12.08.14 at 12:08, <andrew.cooper3@citrix.com> wrote:
> On 12/08/14 10:17, Jan Beulich wrote:
>> All that is needed here is allowing the respective DebugCtl MSR bit to
>> be set by the guest.
>>
>> At once - even if PV guests can't currently use it due to missing
>> DebugCtl MSR virtualization - make the respective adjustments to
>> debugreg.h.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -2246,6 +2246,8 @@ static int vmx_msr_write_intercept(unsig
>> int i, rc = 0;
>> uint64_t supported = IA32_DEBUGCTLMSR_LBR | IA32_DEBUGCTLMSR_BTF;
>>
>> + if ( boot_cpu_has(X86_FEATURE_RTM) )
>> + supported |= IA32_DEBUGCTLMSR_RTM;
>
> This supported bitmask is runtime constant. Is it worth precalculating
> it, rather than reevaluating each time DEBUGCTL is written to?
If the calculation was expensive ...
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling
2014-08-12 9:15 ` [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling Jan Beulich
2014-08-12 9:44 ` Andrew Cooper
@ 2014-08-12 17:47 ` Boris Ostrovsky
2014-08-14 17:14 ` Jan Beulich
2014-08-13 18:52 ` Tian, Kevin
2 siblings, 1 reply; 17+ messages in thread
From: Boris Ostrovsky @ 2014-08-12 17:47 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Kevin Tian, Eddie Dong, Jun Nakajima
On 08/12/2014 05:15 AM, Jan Beulich wrote:
> - writes with one of the vPMU-supported flags and some unsupported one
> set got accepted, leading to a VM entry failure
> - writes with one of the vPMU-supported flags set but the Debug Store
> feature unavailable produced a #GP (other than other writes to this
> MSR with unsupported bits set)
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1789,7 +1789,7 @@ static int svm_msr_write_intercept(unsig
> case MSR_AMD_FAM15H_EVNTSEL3:
> case MSR_AMD_FAM15H_EVNTSEL4:
> case MSR_AMD_FAM15H_EVNTSEL5:
> - vpmu_do_wrmsr(msr, msr_content);
> + vpmu_do_wrmsr(msr, msr_content, 0);
> break;
>
> case MSR_IA32_MCx_MISC(4): /* Threshold register */
> --- a/xen/arch/x86/hvm/svm/vpmu.c
> +++ b/xen/arch/x86/hvm/svm/vpmu.c
> @@ -278,11 +278,14 @@ static void context_update(unsigned int
> }
> }
>
> -static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> +static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
> + uint64_t supported)
> {
> struct vcpu *v = current;
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>
> + ASSERT(!supported);
> +
> /* For all counters, enable guest only mode for HVM guest */
> if ( (get_pmu_reg_type(msr) == MSR_TYPE_CTRL) &&
> !(is_guest_mode(msr_content)) )
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2249,7 +2249,7 @@ static int vmx_msr_write_intercept(unsig
> if ( msr_content & ~supported )
> {
> /* Perhaps some other bits are supported in vpmu. */
> - if ( !vpmu_do_wrmsr(msr, msr_content) )
> + if ( !vpmu_do_wrmsr(msr, msr_content, supported) )
> break;
> }
> if ( msr_content & IA32_DEBUGCTLMSR_LBR )
> @@ -2278,7 +2278,7 @@ static int vmx_msr_write_intercept(unsig
> goto gp_fault;
> break;
> default:
> - if ( vpmu_do_wrmsr(msr, msr_content) )
> + if ( vpmu_do_wrmsr(msr, msr_content, 0) )
> return X86EMUL_OKAY;
> if ( passive_domain_do_wrmsr(msr, msr_content) )
> return X86EMUL_OKAY;
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> @@ -454,7 +454,8 @@ static int core2_vpmu_msr_common_check(u
> return 1;
> }
>
> -static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> +static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
> + uint64_t supported)
> {
> u64 global_ctrl, non_global_ctrl;
> char pmu_enable = 0;
> @@ -469,24 +470,26 @@ static int core2_vpmu_do_wrmsr(unsigned
> /* Special handling for BTS */
> if ( msr == MSR_IA32_DEBUGCTLMSR )
> {
> - uint64_t supported = IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
> - IA32_DEBUGCTLMSR_BTINT;
> + supported |= IA32_DEBUGCTLMSR_TR | IA32_DEBUGCTLMSR_BTS |
> + IA32_DEBUGCTLMSR_BTINT;
>
> if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
> supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS |
> IA32_DEBUGCTLMSR_BTS_OFF_USR;
> - if ( msr_content & supported )
> - {
> - if ( vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
> - return 1;
> - gdprintk(XENLOG_WARNING, "Debug Store is not supported on this cpu\n");
> - hvm_inject_hw_exception(TRAP_gp_fault, 0);
> - return 0;
> - }
> + if ( !(msr_content & ~supported) &&
> + vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
> + return 1;
> + if ( (msr_content & supported) &&
> + !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
> + printk(XENLOG_G_WARNING
> + "%pv: Debug Store unsupported on this CPU\n",
> + current);
Can we move this if statement out of VPMU code into
vmx_msr_write_intercept()? There is not a whole lot of VPMU-specific
logic here and I am not sure I see much reason to go through
vendor-independent code (vpmu.c) to do something that is very much
Intel-specific.
You will need to start using vpmu_is_set() there but the upside is that
there is no need to introduce another parameter that does nothing on AMD
(and even on Intel is useful only for this particular register)
Or maybe add another Intel-specific routine in vpmu_core2.c to handle
this register?
-boris
> }
> return 0;
> }
>
> + ASSERT(!supported);
> +
> core2_vpmu_cxt = vpmu->context;
> switch ( msr )
> {
> --- a/xen/arch/x86/hvm/vpmu.c
> +++ b/xen/arch/x86/hvm/vpmu.c
> @@ -64,12 +64,12 @@ static void __init parse_vpmu_param(char
> }
> }
>
> -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> +int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported)
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>
> if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
> - return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
> + return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content, supported);
> return 0;
> }
>
> --- a/xen/include/asm-x86/hvm/vpmu.h
> +++ b/xen/include/asm-x86/hvm/vpmu.h
> @@ -45,7 +45,8 @@
>
> /* Arch specific operations shared by all vpmus */
> struct arch_vpmu_ops {
> - int (*do_wrmsr)(unsigned int msr, uint64_t msr_content);
> + int (*do_wrmsr)(unsigned int msr, uint64_t msr_content,
> + uint64_t supported);
> int (*do_rdmsr)(unsigned int msr, uint64_t *msr_content);
> int (*do_interrupt)(struct cpu_user_regs *regs);
> void (*do_cpuid)(unsigned int input,
> @@ -86,7 +87,7 @@ struct vpmu_struct {
> #define vpmu_is_set(_vpmu, _x) ((_vpmu)->flags & (_x))
> #define vpmu_clear(_vpmu) ((_vpmu)->flags = 0)
>
> -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content);
> +int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported);
> int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content);
> int vpmu_do_interrupt(struct cpu_user_regs *regs);
> void vpmu_do_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] VMX: fix DebugCtl MSR clearing
2014-08-12 9:14 ` [PATCH 1/4] VMX: fix DebugCtl MSR clearing Jan Beulich
2014-08-12 9:37 ` Andrew Cooper
@ 2014-08-13 18:48 ` Tian, Kevin
1 sibling, 0 replies; 17+ messages in thread
From: Tian, Kevin @ 2014-08-13 18:48 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Dong, Eddie, Nakajima, Jun
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Tuesday, August 12, 2014 2:14 AM
>
> The previous shortcut was wrong, as it bypassed the necessary vmwrite:
> All we really want to avoid if the guest writes zero is to add the MSR
> to the host-load list.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
>
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2246,8 +2246,6 @@ static int vmx_msr_write_intercept(unsig
> int i, rc = 0;
> uint64_t supported = IA32_DEBUGCTLMSR_LBR |
> IA32_DEBUGCTLMSR_BTF;
>
> - if ( !msr_content )
> - break;
> if ( msr_content & ~supported )
> {
> /* Perhaps some other bits are supported in vpmu. */
> @@ -2267,12 +2265,10 @@ static int vmx_msr_write_intercept(unsig
> }
>
> if ( (rc < 0) ||
> - (vmx_add_host_load_msr(msr) < 0) )
> + (msr_content && (vmx_add_host_load_msr(msr) < 0)) )
> hvm_inject_hw_exception(TRAP_machine_check, 0);
> else
> - {
> __vmwrite(GUEST_IA32_DEBUGCTL, msr_content);
> - }
>
> break;
> }
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling
2014-08-12 9:15 ` [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling Jan Beulich
2014-08-12 9:44 ` Andrew Cooper
2014-08-12 17:47 ` Boris Ostrovsky
@ 2014-08-13 18:52 ` Tian, Kevin
2 siblings, 0 replies; 17+ messages in thread
From: Tian, Kevin @ 2014-08-13 18:52 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Dong, Eddie, Nakajima, Jun
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Tuesday, August 12, 2014 2:15 AM
>
> - writes with one of the vPMU-supported flags and some unsupported one
> set got accepted, leading to a VM entry failure
> - writes with one of the vPMU-supported flags set but the Debug Store
> feature unavailable produced a #GP (other than other writes to this
> MSR with unsupported bits set)
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
>
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1789,7 +1789,7 @@ static int svm_msr_write_intercept(unsig
> case MSR_AMD_FAM15H_EVNTSEL3:
> case MSR_AMD_FAM15H_EVNTSEL4:
> case MSR_AMD_FAM15H_EVNTSEL5:
> - vpmu_do_wrmsr(msr, msr_content);
> + vpmu_do_wrmsr(msr, msr_content, 0);
> break;
>
> case MSR_IA32_MCx_MISC(4): /* Threshold register */
> --- a/xen/arch/x86/hvm/svm/vpmu.c
> +++ b/xen/arch/x86/hvm/svm/vpmu.c
> @@ -278,11 +278,14 @@ static void context_update(unsigned int
> }
> }
>
> -static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> +static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
> + uint64_t supported)
> {
> struct vcpu *v = current;
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
>
> + ASSERT(!supported);
> +
> /* For all counters, enable guest only mode for HVM guest */
> if ( (get_pmu_reg_type(msr) == MSR_TYPE_CTRL) &&
> !(is_guest_mode(msr_content)) )
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2249,7 +2249,7 @@ static int vmx_msr_write_intercept(unsig
> if ( msr_content & ~supported )
> {
> /* Perhaps some other bits are supported in vpmu. */
> - if ( !vpmu_do_wrmsr(msr, msr_content) )
> + if ( !vpmu_do_wrmsr(msr, msr_content, supported) )
> break;
> }
> if ( msr_content & IA32_DEBUGCTLMSR_LBR )
> @@ -2278,7 +2278,7 @@ static int vmx_msr_write_intercept(unsig
> goto gp_fault;
> break;
> default:
> - if ( vpmu_do_wrmsr(msr, msr_content) )
> + if ( vpmu_do_wrmsr(msr, msr_content, 0) )
> return X86EMUL_OKAY;
> if ( passive_domain_do_wrmsr(msr, msr_content) )
> return X86EMUL_OKAY;
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> @@ -454,7 +454,8 @@ static int core2_vpmu_msr_common_check(u
> return 1;
> }
>
> -static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> +static int core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
> + uint64_t supported)
> {
> u64 global_ctrl, non_global_ctrl;
> char pmu_enable = 0;
> @@ -469,24 +470,26 @@ static int core2_vpmu_do_wrmsr(unsigned
> /* Special handling for BTS */
> if ( msr == MSR_IA32_DEBUGCTLMSR )
> {
> - uint64_t supported = IA32_DEBUGCTLMSR_TR |
> IA32_DEBUGCTLMSR_BTS |
> - IA32_DEBUGCTLMSR_BTINT;
> + supported |= IA32_DEBUGCTLMSR_TR |
> IA32_DEBUGCTLMSR_BTS |
> + IA32_DEBUGCTLMSR_BTINT;
>
> if ( cpu_has(¤t_cpu_data, X86_FEATURE_DSCPL) )
> supported |= IA32_DEBUGCTLMSR_BTS_OFF_OS |
> IA32_DEBUGCTLMSR_BTS_OFF_USR;
> - if ( msr_content & supported )
> - {
> - if ( vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
> - return 1;
> - gdprintk(XENLOG_WARNING, "Debug Store is not
> supported on this cpu\n");
> - hvm_inject_hw_exception(TRAP_gp_fault, 0);
> - return 0;
> - }
> + if ( !(msr_content & ~supported) &&
> + vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
> + return 1;
> + if ( (msr_content & supported) &&
> + !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
> + printk(XENLOG_G_WARNING
> + "%pv: Debug Store unsupported on this CPU\n",
> + current);
> }
> return 0;
> }
>
> + ASSERT(!supported);
> +
> core2_vpmu_cxt = vpmu->context;
> switch ( msr )
> {
> --- a/xen/arch/x86/hvm/vpmu.c
> +++ b/xen/arch/x86/hvm/vpmu.c
> @@ -64,12 +64,12 @@ static void __init parse_vpmu_param(char
> }
> }
>
> -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
> +int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t
> supported)
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(current);
>
> if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
> - return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content);
> + return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content,
> supported);
> return 0;
> }
>
> --- a/xen/include/asm-x86/hvm/vpmu.h
> +++ b/xen/include/asm-x86/hvm/vpmu.h
> @@ -45,7 +45,8 @@
>
> /* Arch specific operations shared by all vpmus */
> struct arch_vpmu_ops {
> - int (*do_wrmsr)(unsigned int msr, uint64_t msr_content);
> + int (*do_wrmsr)(unsigned int msr, uint64_t msr_content,
> + uint64_t supported);
> int (*do_rdmsr)(unsigned int msr, uint64_t *msr_content);
> int (*do_interrupt)(struct cpu_user_regs *regs);
> void (*do_cpuid)(unsigned int input,
> @@ -86,7 +87,7 @@ struct vpmu_struct {
> #define vpmu_is_set(_vpmu, _x) ((_vpmu)->flags & (_x))
> #define vpmu_clear(_vpmu) ((_vpmu)->flags = 0)
>
> -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content);
> +int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t
> supported);
> int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content);
> int vpmu_do_interrupt(struct cpu_user_regs *regs);
> void vpmu_do_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] VMX: allow RTM advanced debugging to be used by guests
2014-08-12 9:17 ` [PATCH 3/4] VMX: allow RTM advanced debugging to be used by guests Jan Beulich
2014-08-12 10:08 ` Andrew Cooper
@ 2014-08-13 18:52 ` Tian, Kevin
1 sibling, 0 replies; 17+ messages in thread
From: Tian, Kevin @ 2014-08-13 18:52 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Keir Fraser, Dong, Eddie, Nakajima, Jun
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Tuesday, August 12, 2014 2:17 AM
>
> All that is needed here is allowing the respective DebugCtl MSR bit to
> be set by the guest.
>
> At once - even if PV guests can't currently use it due to missing
> DebugCtl MSR virtualization - make the respective adjustments to
> debugreg.h.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
>
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2246,6 +2246,8 @@ static int vmx_msr_write_intercept(unsig
> int i, rc = 0;
> uint64_t supported = IA32_DEBUGCTLMSR_LBR |
> IA32_DEBUGCTLMSR_BTF;
>
> + if ( boot_cpu_has(X86_FEATURE_RTM) )
> + supported |= IA32_DEBUGCTLMSR_RTM;
> if ( msr_content & ~supported )
> {
> /* Perhaps some other bits are supported in vpmu. */
> --- a/xen/include/asm-x86/debugreg.h
> +++ b/xen/include/asm-x86/debugreg.h
> @@ -20,6 +20,7 @@
> #define DR_TRAP3 (0x8) /* db3 */
> #define DR_STEP (0x4000) /* single-step */
> #define DR_SWITCH (0x8000) /* task switch */
> +#define DR_NOT_RTM (0x10000) /* clear: #BP inside RTM
> region */
>
> /* Now define a bunch of things for manipulating the control register.
> The top two bytes of the control register consist of 4 fields of 4
> @@ -62,6 +63,7 @@
> #define DR_CONTROL_RESERVED_ONE (0x00000400ul) /* Reserved, read
> as one */
> #define DR_LOCAL_EXACT_ENABLE (0x00000100ul) /* Local exact
> enable */
> #define DR_GLOBAL_EXACT_ENABLE (0x00000200ul) /* Global exact
> enable */
> +#define DR_RTM_ENABLE (0x00000800ul) /* RTM debugging
> enable */
> #define DR_GENERAL_DETECT (0x00002000ul) /* General detect
> enable */
>
> #define write_debugreg(reg, val) do { \
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -79,6 +79,7 @@
> #define IA32_DEBUGCTLMSR_BTINT (1<<8) /* Branch Trace Interrupt */
> #define IA32_DEBUGCTLMSR_BTS_OFF_OS (1<<9) /* BTS off if CPL 0 */
> #define IA32_DEBUGCTLMSR_BTS_OFF_USR (1<<10) /* BTS off if CPL > 0 */
> +#define IA32_DEBUGCTLMSR_RTM (1<<15) /* RTM debugging enable
> */
>
> #define MSR_IA32_LASTBRANCHFROMIP 0x000001db
> #define MSR_IA32_LASTBRANCHTOIP 0x000001dc
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] VMX/vPMU: reduce core2_vpmu_initialise() verbosity
2014-08-12 9:18 ` [PATCH 4/4] VMX/vPMU: reduce core2_vpmu_initialise() verbosity Jan Beulich
2014-08-12 9:29 ` Andrew Cooper
@ 2014-08-13 18:53 ` Tian, Kevin
1 sibling, 0 replies; 17+ messages in thread
From: Tian, Kevin @ 2014-08-13 18:53 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Dong, Eddie, Nakajima, Jun
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Tuesday, August 12, 2014 2:19 AM
>
> No need to print these messages for each vCPU, even more, no need to
> print them for each domain - they all depend on CPU features that are
> either there or not.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
>
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c
> @@ -764,19 +764,19 @@ static int core2_vpmu_initialise(struct
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> u64 msr_content;
> - struct cpuinfo_x86 *c = ¤t_cpu_data;
> + static bool_t ds_warned;
>
> if ( !(vpmu_flags & VPMU_BOOT_BTS) )
> goto func_out;
> /* Check the 'Debug Store' feature in the CPUID.EAX[1]:EDX[21] */
> - if ( cpu_has(c, X86_FEATURE_DS) )
> + while ( boot_cpu_has(X86_FEATURE_DS) )
> {
> - if ( !cpu_has(c, X86_FEATURE_DTES64) )
> + if ( !boot_cpu_has(X86_FEATURE_DTES64) )
> {
> - printk(XENLOG_G_WARNING "CPU doesn't support 64-bit DS
> Area"
> - " - Debug Store disabled for %pv\n",
> - v);
> - goto func_out;
> + if ( !ds_warned )
> + printk(XENLOG_G_WARNING "CPU doesn't support 64-bit
> DS Area"
> + " - Debug Store disabled for guests\n");
> + break;
> }
> vpmu_set(vpmu, VPMU_CPU_HAS_DS);
> rdmsrl(MSR_IA32_MISC_ENABLE, msr_content);
> @@ -784,14 +784,16 @@ static int core2_vpmu_initialise(struct
> {
> /* If BTS_UNAVAIL is set reset the DS feature. */
> vpmu_reset(vpmu, VPMU_CPU_HAS_DS);
> - printk(XENLOG_G_WARNING "CPU has set BTS_UNAVAIL"
> - " - Debug Store disabled for %pv\n",
> - v);
> + if ( !ds_warned )
> + printk(XENLOG_G_WARNING "CPU has set BTS_UNAVAIL"
> + " - Debug Store disabled for guests\n");
> + break;
> }
> - else
> +
> + vpmu_set(vpmu, VPMU_CPU_HAS_BTS);
> + if ( !ds_warned )
> {
> - vpmu_set(vpmu, VPMU_CPU_HAS_BTS);
> - if ( !cpu_has(c, X86_FEATURE_DSCPL) )
> + if ( !boot_cpu_has(X86_FEATURE_DSCPL) )
> printk(XENLOG_G_INFO
> "vpmu: CPU doesn't support CPL-Qualified
> BTS\n");
>
> printk("******************************************************\n");
> @@ -803,8 +805,10 @@ static int core2_vpmu_initialise(struct
> printk("** It is NOT recommended for production use!
> **\n");
>
> printk("******************************************************\n");
> }
> + break;
> }
> -func_out:
> + ds_warned = 1;
> + func_out:
> check_pmc_quirk();
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling
2014-08-12 17:47 ` Boris Ostrovsky
@ 2014-08-14 17:14 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2014-08-14 17:14 UTC (permalink / raw)
To: xen-devel, Boris Ostrovsky; +Cc: Kevin Tian, Eddie Dong, Jun Nakajima
>>> On 12.08.14 at 19:47, <boris.ostrovsky@oracle.com> wrote:
> On 08/12/2014 05:15 AM, Jan Beulich wrote:
>> - if ( msr_content & supported )
>> - {
>> - if ( vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
>> - return 1;
>> - gdprintk(XENLOG_WARNING, "Debug Store is not supported on this cpu\n");
>> - hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> - return 0;
>> - }
>> + if ( !(msr_content & ~supported) &&
>> + vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
>> + return 1;
>> + if ( (msr_content & supported) &&
>> + !vpmu_is_set(vpmu, VPMU_CPU_HAS_BTS) )
>> + printk(XENLOG_G_WARNING
>> + "%pv: Debug Store unsupported on this CPU\n",
>> + current);
>
> Can we move this if statement out of VPMU code into
> vmx_msr_write_intercept()? There is not a whole lot of VPMU-specific
> logic here and I am not sure I see much reason to go through
> vendor-independent code (vpmu.c) to do something that is very much
> Intel-specific.
I'd rather not (at least not in this patch), not the least that I can
see the "supported" function parameter to potentially be useful
elsewhere when use of certain bits within an MSR is split across
components.
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-08-14 17:14 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-12 9:08 [PATCH 0/4] VMX: DebugCtl MSR related adjustments Jan Beulich
2014-08-12 9:14 ` [PATCH 1/4] VMX: fix DebugCtl MSR clearing Jan Beulich
2014-08-12 9:37 ` Andrew Cooper
2014-08-13 18:48 ` Tian, Kevin
2014-08-12 9:15 ` [PATCH 2/4] VMX/vPMU: fix DebugCtl MSR handling Jan Beulich
2014-08-12 9:44 ` Andrew Cooper
2014-08-12 9:51 ` Jan Beulich
2014-08-12 17:47 ` Boris Ostrovsky
2014-08-14 17:14 ` Jan Beulich
2014-08-13 18:52 ` Tian, Kevin
2014-08-12 9:17 ` [PATCH 3/4] VMX: allow RTM advanced debugging to be used by guests Jan Beulich
2014-08-12 10:08 ` Andrew Cooper
2014-08-12 10:40 ` Jan Beulich
2014-08-13 18:52 ` Tian, Kevin
2014-08-12 9:18 ` [PATCH 4/4] VMX/vPMU: reduce core2_vpmu_initialise() verbosity Jan Beulich
2014-08-12 9:29 ` Andrew Cooper
2014-08-13 18:53 ` Tian, Kevin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.