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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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. > > Signed-off-by: Ben Guthro > Signed-off-by: David Lively <dlively@virtualiron.com> > > 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) ) > { > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
I don''t see why not, now that you mention it. I''ll give it a try ... Dave 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. >> >> Signed-off-by: Ben Guthro >> Signed-off-by: David Lively <dlively@virtualiron.com> >> >> 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) ) >> { >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel >> > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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. >> >>_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel