[PATCH V2] mcheck, vmce: Allow vmce_amd_* functions to handle AMD thresolding MSRs

[PATCH V2] mcheck, vmce: Allow vmce_amd_* functions to handle AMD thresolding MSRs

[Aravind Gopalakrishnan]

vmce_amd_[rd|wr]msr functions can handle accesses to AMD thresholding
registers. But due to this statement here:
switch ( msr & (MSR_IA32_MC0_CTL | 3) )
we are wrongly masking off top two bits which meant the register
accesses never made it to vmce_amd_* functions.

Corrected this problem by modifying the mask in this patch to allow
AMD thresholding registers to fall to 'default' case which in turn
allows vmce_amd_* functions to handle access to the registers.

While at it, remove some clutter in the vmce_amd* functions. Retained
current policy of returning zero for reads and ignoring writes.

Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Reviewed-by: Christoph Egger <chegger@amazon.de>
---
 xen/arch/x86/cpu/mcheck/amd_f10.c |   41 ++++++-------------------------------
 xen/arch/x86/cpu/mcheck/vmce.c    |    4 ++--
 2 files changed, 8 insertions(+), 37 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/amd_f10.c b/xen/arch/x86/cpu/mcheck/amd_f10.c
index 61319dc..03797ab 100644
--- a/xen/arch/x86/cpu/mcheck/amd_f10.c
+++ b/xen/arch/x86/cpu/mcheck/amd_f10.c
@@ -105,43 +105,14 @@ enum mcheck_type amd_f10_mcheck_init(struct cpuinfo_x86 *c)
 /* amd specific MCA MSR */
 int vmce_amd_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
 {
-	switch (msr) {
-	case MSR_F10_MC4_MISC1: /* DRAM error type */
-		v->arch.vmce.bank[1].mci_misc = val; 
-		mce_printk(MCE_VERBOSE, "MCE: wr msr %#"PRIx64"\n", val);
-		break;
-	case MSR_F10_MC4_MISC2: /* Link error type */
-	case MSR_F10_MC4_MISC3: /* L3 cache error type */
-		/* ignore write: we do not emulate link and l3 cache errors
-		 * to the guest.
-		 */
-		mce_printk(MCE_VERBOSE, "MCE: wr msr %#"PRIx64"\n", val);
-		break;
-	default:
-		return 0;
-	}
-
-	return 1;
+    /* Do nothing as we don't emulate this MC bank currently */
+    mce_printk(MCE_VERBOSE, "MCE: wr msr %#"PRIx64"\n", val);
+    return 1;
 }
 
 int vmce_amd_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
 {
-	switch (msr) {
-	case MSR_F10_MC4_MISC1: /* DRAM error type */
-		*val = v->arch.vmce.bank[1].mci_misc;
-		mce_printk(MCE_VERBOSE, "MCE: rd msr %#"PRIx64"\n", *val);
-		break;
-	case MSR_F10_MC4_MISC2: /* Link error type */
-	case MSR_F10_MC4_MISC3: /* L3 cache error type */
-		/* we do not emulate link and l3 cache
-		 * errors to the guest.
-		 */
-		*val = 0;
-		mce_printk(MCE_VERBOSE, "MCE: rd msr %#"PRIx64"\n", *val);
-		break;
-	default:
-		return 0;
-	}
-
-	return 1;
+    /* Assign '0' as we don't emulate this MC bank currently */
+    *val = 0;
+    return 1;
 }
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index f6c35db..be9bb5e 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -107,7 +107,7 @@ static int bank_mce_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
 
     *val = 0;
 
-    switch ( msr & (MSR_IA32_MC0_CTL | 3) )
+    switch ( msr & (-MSR_IA32_MC0_CTL | 3) )
     {
     case MSR_IA32_MC0_CTL:
         /* stick all 1's to MCi_CTL */
@@ -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 & (-MSR_IA32_MC0_CTL | 3) )
     {
     case MSR_IA32_MC0_CTL:
         /*
-- 
1.7.9.5

Re: [PATCH V2] mcheck, vmce: Allow vmce_amd_* functions to handle AMD thresolding MSRs

[Jan Beulich]

>>> On 10.02.14 at 17:38, Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com> wrote:
> vmce_amd_[rd|wr]msr functions can handle accesses to AMD thresholding
> registers. But due to this statement here:
> switch ( msr & (MSR_IA32_MC0_CTL | 3) )
> we are wrongly masking off top two bits which meant the register
> accesses never made it to vmce_amd_* functions.
> 
> Corrected this problem by modifying the mask in this patch to allow
> AMD thresholding registers to fall to 'default' case which in turn
> allows vmce_amd_* functions to handle access to the registers.
> 
> While at it, remove some clutter in the vmce_amd* functions. Retained
> current policy of returning zero for reads and ignoring writes.
> 
> Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
> Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Reviewed-by: Christoph Egger <chegger@amazon.de>

Are these tags for _this_ version of the patch, or an earlier one?
The nature of the changes done on this latest round (which finally
looks good to me) would require you to drop all earlier acks and
reviews, unless some reviewing went on behind the scenes.

Jan

Re: [PATCH V2] mcheck, vmce: Allow vmce_amd_* functions to handle AMD thresolding MSRs

[Aravind Gopalakrishnan]

On Mon, Feb 10, 2014 at 05:01:14PM +0000, Jan Beulich wrote:
> >>> On 10.02.14 at 17:38, Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com> wrote:
> > Signed-off-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
> > Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > Reviewed-by: Christoph Egger <chegger@amazon.de>
> 
> Are these tags for _this_ version of the patch, or an earlier one?
> The nature of the changes done on this latest round (which finally
> looks good to me) would require you to drop all earlier acks and
> reviews, unless some reviewing went on behind the scenes.
> 

Ah. Nope. I assumed I had to retain the 'reviewed-by' lines as the patch is
related to the earlier ones I had sent out..

Sent a corrected one now.

-Aravind.