msr: Use defines for bits of MSR_IA32_DEBUGCTLMSR instead of numbers

PATCH Superseded Dietmar Hahn 2 participants 2 messages Jan 31, 2012

lore

Share & tools

lore

Dietmar Hahn

author

2012-01-31 08:42 UTC #0 lore raw

PATCH 1/1 msr: Use defines for bits of MSR_IA32_DEBUGCTLMSR instead of numbers

# HG changeset patch
# Parent e2722b24dc0962de37215320b05d1bb7c4c42864

Use defines for bits of MSR_IA32_DEBUGCTLMSR instead of numbers.

Signed-off-by Dietmar Hahn <[email protected]>

xen/arch/x86/hvm/vmx/vmx.c +4 -3

diff -r e2722b24dc09 xen/arch/x86/hvm/vmx/vmx.c
@@ -1944,11 +1944,12 @@ static int vmx_msr_write_intercept(unsig
         break;
     case MSR_IA32_DEBUGCTLMSR: {
         int i, rc = 0;
-
-        if ( !msr_content || (msr_content & ~3) )
+        uint64_t allowed = MSR_IA32_DEBUGCTLMSR_LBR | MSR_IA32_DEBUGCTLMSR_BTF;
+
+        if ( !msr_content || (msr_content & ~allowed) )
             break;
 
-        if ( msr_content & 1 )
+        if ( msr_content & MSR_IA32_DEBUGCTLMSR_LBR )
         {
             const struct lbr_info *lbr = last_branch_msr_get();
             if ( lbr == NULL )

xen/include/asm-x86/msr-index.h +7 -1

diff -r e2722b24dc09 xen/include/asm-x86/msr-index.h
@@ -64,12 +64,18 @@
 #define MSR_MTRRfix4K_F8000		0x0000026f
 #define MSR_MTRRdefType			0x000002ff
 
+/* MSR_IA32_DEBUGCTLMSR bits: */
+#define _IA32_DEBUGCTLMSR_LBR		0 /* Last Branch Record */
+#define _IA32_DEBUGCTLMSR_BTF		1 /* Single Step on Branches */
+#define MSR_IA32_DEBUGCTLMSR_LBR	(1<<_IA32_DEBUGCTLMSR_LBR)
+#define MSR_IA32_DEBUGCTLMSR_BTF	(1<<_IA32_DEBUGCTLMSR_BTF)
+
 #define MSR_IA32_DEBUGCTLMSR		0x000001d9
 #define MSR_IA32_LASTBRANCHFROMIP	0x000001db
 #define MSR_IA32_LASTBRANCHTOIP		0x000001dc
 #define MSR_IA32_LASTINTFROMIP		0x000001dd
 #define MSR_IA32_LASTINTTOIP		0x000001de
- 
+
 #define MSR_IA32_MTRR_PHYSBASE0     0x00000200
 #define MSR_IA32_MTRR_PHYSMASK0     0x00000201
 #define MSR_IA32_MTRR_PHYSBASE1     0x00000202

Signature · 1 line

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

Jan Beulich

2012-01-31 10:23 UTC #1 lore raw

Quoted 5 lines

Quoted 1 line
Quoted 1 line · Dietmar Hahn
# HG changeset patch
# Parent e2722b24dc0962de37215320b05d1bb7c4c42864
Use defines for bits of MSR_IA32_DEBUGCTLMSR instead of numbers.

That would be nice, but only if done consistently everywhere (missing at least xen/arch/x86/traps.c:ler_enable(); I'm surprise SVM code doesn't appear to need any adjustment).

Quoted 26 lines

Signed-off-by Dietmar Hahn <[email protected]>

diff -r e2722b24dc09 xen/arch/x86/hvm/vmx/vmx.c

xen/arch/x86/hvm/vmx/vmx.c +4 -3

@@ -1944,11 +1944,12 @@ static int vmx_msr_write_intercept(unsig
         break;
     case MSR_IA32_DEBUGCTLMSR: {
         int i, rc = 0;
-
-        if ( !msr_content || (msr_content & ~3) )
+        uint64_t allowed = MSR_IA32_DEBUGCTLMSR_LBR | 
 MSR_IA32_DEBUGCTLMSR_BTF;
+
+        if ( !msr_content || (msr_content & ~allowed) )
             break;
 
-        if ( msr_content & 1 )
+        if ( msr_content & MSR_IA32_DEBUGCTLMSR_LBR )
         {
             const struct lbr_info *lbr = last_branch_msr_get();

             if ( lbr == NULL )

diff -r e2722b24dc09 xen/include/asm-x86/msr-index.h

xen/include/asm-x86/msr-index.h line 72

Quoted 9 lines

xen/include/asm-x86/msr-index.h +6 -0

@@ -64,12 +64,18 @@
 #define MSR_MTRRfix4K_F8000		0x0000026f
 #define MSR_MTRRdefType			0x000002ff
 
+/* MSR_IA32_DEBUGCTLMSR bits: */
+#define _IA32_DEBUGCTLMSR_LBR		0 /* Last Branch Record */
+#define _IA32_DEBUGCTLMSR_BTF		1 /* Single Step on Branches */
+#define MSR_IA32_DEBUGCTLMSR_LBR	(1<<_IA32_DEBUGCTLMSR_LBR)
+#define MSR_IA32_DEBUGCTLMSR_BTF	(1<<_IA32_DEBUGCTLMSR_BTF)
+

Please no MSR_ prefix on values not representing MSR indices.

Also I personally prefer to have individual field definitions of an MSR follow its index definition.

Jan

Quoted 10 lines

xen/include/asm-x86/msr-index.h +1 -1

 #define MSR_IA32_DEBUGCTLMSR		0x000001d9
 #define MSR_IA32_LASTBRANCHFROMIP	0x000001db
 #define MSR_IA32_LASTBRANCHTOIP		0x000001dc
 #define MSR_IA32_LASTINTFROMIP		0x000001dd
 #define MSR_IA32_LASTINTTOIP		0x000001de
- 
+
 #define MSR_IA32_MTRR_PHYSBASE0     0x00000200
 #define MSR_IA32_MTRR_PHYSMASK0     0x00000201
 #define MSR_IA32_MTRR_PHYSBASE1     0x00000202