Re: [PATCH] Limit MCG Cap

Re: [PATCH] Limit MCG Cap

[David Lively]

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

Hi Keir -
  Your suggestion to report 0 for reads of MSR_IA32_MCG_CAP seems to be 
working well on a wide variety of HVM guests (Windows & Linux, 32 & 
64bit).  I've attached a new patch.

Thanks,
Dave

This patch implements a suggestion of Keir's (in response to a patch of 
mine):

Intercept guest reads of MSR_IA32_MCG_CAP and report 0, indicating no 
machine check "units", which agrees more closely with Xen's 
super-minimal machine check architecture (just enough to allow Windows 
to run).

This fixes a bug that occurs when migrating a RHEL4-64bit guest to a 
host with fewer machine check units than the original host.  These host 
physical details shouldn't be leaking through to guests.

Signed-off-by: David Lively <dlively@virtualiron.com>


Keir Fraser wrote:
> Given that we don't properly virtualise/emulate machine check (we only set
> the feature bit because some versions of Windows demand it) can we get away
> with returning zero for reads of MCG_CAP?
>
>  -- Keir
>
> On 27/8/07 20:11, "Ben Guthro" <bguthro@virtualiron.com> wrote:
>
>   
>> Intercept guest reads of MSR_IA32_MCG_CAP and limit the number of memory banks
>> reported to one.
>> This prevents us from trying to read status of non-existent banks when
>> migrated to a machine
>> with fewer banks.
>>
>>     


[-- Attachment #2: xen-limit-mcg-cap.patch --]
[-- Type: text/x-patch, Size: 1007 bytes --]

diff -r 9f111c7d648f xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c	Tue Aug 28 22:40:01 2007 -0400
+++ b/xen/arch/x86/hvm/svm/svm.c	Tue Aug 28 22:40:01 2007 -0400
@@ -2058,6 +2058,9 @@ static inline void svm_do_msr_access(
              */
             msr_content = 0;
             break;
+        case MSR_IA32_MCG_CAP:
+            msr_content = 0;
+            break;
         default:
             if ( rdmsr_hypervisor_regs(ecx, &eax, &edx) ||
                  rdmsr_safe(ecx, eax, edx) == 0 )
diff -r 9f111c7d648f xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c	Tue Aug 28 22:40:01 2007 -0400
+++ b/xen/arch/x86/hvm/vmx/vmx.c	Tue Aug 28 22:40:01 2007 -0400
@@ -2554,6 +2554,9 @@ static inline int vmx_do_msr_read(struct
     case MSR_IA32_APICBASE:
         msr_content = vcpu_vlapic(v)->hw.apic_base_msr;
         break;
+    case MSR_IA32_MCG_CAP:
+        msr_content = 0;
+        break;
     default:
         if ( long_mode_do_msr_read(regs) )
             goto done;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

[PATCH] Limit MCG Cap

[Ben Guthro]

[-- Attachment #1: Type: text/plain, Size: 289 bytes --]

Intercept guest reads of MSR_IA32_MCG_CAP and limit the number of memory banks reported to one.
This prevents us from trying to read status of non-existent banks when migrated to a machine
with fewer banks.

Signed-off-by: Ben Guthro
Signed-off-by: David Lively <dlively@virtualiron.com>


[-- Attachment #2: xen-limit-mcg-cap.patch --]
[-- Type: text/x-patch, Size: 2162 bytes --]

diff -r 4eea7c9b3bf2 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c	Thu Aug 02 08:59:43 2007 -0400
+++ b/xen/arch/x86/hvm/svm/svm.c	Thu Aug 02 08:59:43 2007 -0400
@@ -2162,7 +2162,19 @@ static void svm_do_msr_access(
             /* No point in letting the guest see real MCEs */
             msr_content = 0;
             break;
-
+        case MSR_IA32_MCG_CAP:
+            if ( rdmsr_safe(ecx, regs->eax, regs->edx) == 0 ) {
+                /*
+                 * Report at most one memory bank so migration to a machine with
+                 * fewer banks doesn't cause guest GPFs when the guest tries to read the
+                 * (now, after migration) non-existent banks' MSRs.
+                 */
+                regs->eax = (regs->eax & ~0xFF) | !!(regs->eax & 0xFF);
+                goto done;
+            }
+            gdprintk(XENLOG_ERR, "%s: rdmsr_safe(MCG_CAP) failed!\n", __FUNCTION__);
+            svm_inject_exception(v, TRAP_gp_fault, 1, 0);
+            return;
         default:
             if ( rdmsr_hypervisor_regs(ecx, &eax, &edx) ||
                  rdmsr_safe(ecx, eax, edx) == 0 )
diff -r 4eea7c9b3bf2 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c	Thu Aug 02 08:59:43 2007 -0400
+++ b/xen/arch/x86/hvm/vmx/vmx.c	Thu Aug 02 09:14:06 2007 -0400
@@ -2754,6 +2754,19 @@ static int vmx_do_msr_read(struct cpu_us
         /* No point in letting the guest see real MCEs */
         msr_content = 0;
         break;
+    case MSR_IA32_MCG_CAP:
+        if ( rdmsr_safe(ecx, regs->eax, regs->edx) == 0 ) {
+            /*
+             * Report only a single memory bank so migration to a machine with
+             * fewer banks doesn't cause guest GPFs when the guest tries to read the
+             * (now, after migration) non-existent banks' MSRs.
+             */
+            regs->eax = (regs->eax & ~0xFF) | !!(regs->eax & 0xFF);
+            goto done;
+        }
+        gdprintk(XENLOG_ERR, "%s: rdmsr_safe(MCG_CAP) failed!\n", __FUNCTION__);
+        vmx_inject_hw_exception(v, TRAP_gp_fault, 0);
+        return 0;
     default:
         switch ( long_mode_do_msr_read(regs) )
         {

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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