[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] MCHECK, AMD: Fix code to allow calls to vmce_amd_rdmsr and vmce_amd_wrmsr




- switch ( msr & (MSR_IA32_MC0_CTL | 3) )
+    switch ( msr )

You can't drop the masking altogether here - that way you're
preventing banks other than bank 0 to be handled here.

When I drop the mask, I am allowing all MCA MSR's here.. It is when I apply the mask that I am allowing only MC0 bank MSR's alone. Here is an example: (All values in hexadecimal form)

(MSR_IA32_MC0_CTL | 3) = 0x403; Therefore -


Msr val = 400
val & 0x403 = 400

Msr val = 401
val & 0x403 = 401

Msr val = 402
val & 0x403 = 402

Msr val = 403
val & 0x403 = 403

Msr val = 404
val & 0x403 = 400

Msr val = 405
val & 0x403 = 401

Msr val = 406
val & 0x403 = 402

Msr val = 407
val & 0x403 = 403

Msr val = 413
val & 0x403 = 403

Msr val = c0000408
val & 0x403 = 400

Msr val = c0000409
val & 0x403 = 401

We need to route the MC4 MSR's (0x413 for DRAM errors, 0xc0000408 for Link errors, 0xc0000409 for L3 errors) to be handled by vmce_amd_wrmsr and vmce_amd_rdmsr. Since by removing the mask altogether, I am also allowing MSR's 0x404, 0x405, 0x406 and 0x407 (which is harmless still as they fall to 'default' in vmce_amd_rdmsr or
vmce_amd_wrmsr functions);


@@ -210,7 +210,7 @@ static int bank_mce_wrmsr(struct vcpu *v, uint32_t msr, 
uint64_t val)
      int ret = 1;
      unsigned int bank = (msr - MSR_IA32_MC0_CTL) / 4;
- switch ( msr & (MSR_IA32_MC0_CTL | 3) )
+    switch ( msr )
Same here.

--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1460,8 +1460,9 @@ static int svm_msr_read_intercept(unsigned int msr, 
uint64_t *msr_content)
          *msr_content = v->arch.hvm_svm.guest_sysenter_eip;
          break;
- case MSR_IA32_MCx_MISC(4): /* Threshold register */
This deletion isn't being explained at all in the description.

@@ -1659,8 +1660,9 @@ static int svm_msr_write_intercept(unsigned int msr, 
uint64_t msr_content)
          vpmu_do_wrmsr(msr, msr_content);
          break;
- case MSR_IA32_MCx_MISC(4): /* Threshold register */
Again, same thing here.

 I will explain this better in a V2 patch...
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -218,9 +218,9 @@
  #define AMD64_NB_CFG_CF8_EXT_ENABLE_BIT       46
/* AMD Family10h machine check MSRs */
-#define MSR_F10_MC4_MISC1              0xc0000408
-#define MSR_F10_MC4_MISC2              0xc0000409
-#define MSR_F10_MC4_MISC3              0xc000040A
+#define MSR_F10_MC4_MISC1              0x00000413
+#define MSR_F10_MC4_MISC2              0xc0000408
+#define MSR_F10_MC4_MISC3              0xc0000409
Fam10 BIOS and Kernel Developer's Guide disagrees with this.
Fam15 model 0x also doesn't list C0000413 (but indeed marks
C000040A [as well as subsequent ones] as reserved). Fam15
model 1x even lists everything from C0000409 onwards as
reserved.
It is MSR 0x413 and not 0xc0000413. MSR 0x413 *is* listed as DRAM
thresholding register.

And you are right about Link Thresholding (MSRC0000408) and
L3 Thresholding(MSRC0000409). One way to resolve this can be to add
quirks in 'mce_amd_quirks' structure and check for it before we emulate
Link/L3 thresholding MSR's to the guest.

Thoughts?

Thanks,
-Aravind.
So I'm afraid you need to start over.

Jan





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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.