Patchwork target-i386: GPF on invalid MSRs

login
register
mail settings
Submitter Josh Triplett
Date May 26, 2011, 9:08 a.m.
Message ID <20110526090804.GA32647@leaf>
Download mbox | patch
Permalink /patch/97510/
State New
Headers show

Comments

Josh Triplett - May 26, 2011, 9:08 a.m.
qemu currently returns 0 for rdmsr on invalid MSRs, and ignores wrmsr on
invalid MSRs.  Real x86 processors GPF on invalid MSRs, which allows
software to detect unavailable MSRs.  Emulate this behavior correctly in
qemu.

Bug discovered via the BIOS Implementation Test Suite
<http://biosbits.org/>; fix tested the same way, for both 32-bit and
64-bit x86.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 op_helper.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Alexander Graf - May 26, 2011, 9:12 a.m.
On 26.05.2011, at 11:08, Josh Triplett wrote:

> qemu currently returns 0 for rdmsr on invalid MSRs, and ignores wrmsr on
> invalid MSRs.  Real x86 processors GPF on invalid MSRs, which allows
> software to detect unavailable MSRs.  Emulate this behavior correctly in
> qemu.
> 
> Bug discovered via the BIOS Implementation Test Suite
> <http://biosbits.org/>; fix tested the same way, for both 32-bit and
> 64-bit x86.

This would break a _lot_ of guests that work just fine today, as qemu doesn't handle all the necessary MSRs.


Alex
Josh Triplett - May 27, 2011, 3:13 p.m.
On Thu, May 26, 2011 at 11:12:12AM +0200, Alexander Graf wrote:
> On 26.05.2011, at 11:08, Josh Triplett wrote:
> > qemu currently returns 0 for rdmsr on invalid MSRs, and ignores wrmsr on
> > invalid MSRs.  Real x86 processors GPF on invalid MSRs, which allows
> > software to detect unavailable MSRs.  Emulate this behavior correctly in
> > qemu.
> > 
> > Bug discovered via the BIOS Implementation Test Suite
> > <http://biosbits.org/>; fix tested the same way, for both 32-bit and
> > 64-bit x86.
> 
> This would break a _lot_ of guests that work just fine today, as qemu doesn't handle all the necessary MSRs.

It also fixes guests that rely on the GPF to indicate the absence of an
MSR, and assume that the lack of GPF means the availability of that MSR.
Silently returning 0 for unknown MSRs means silent breakage.

What (buggy) guests expect to use random model-specific registers
without either handling GPFs or checking the CPU model first?

What MSRs do those guests expect that qemu doesn't currently implement?

If this represents a workaround for buggy guests, then may I add an
option to control this behavior?

- Josh Triplett
Alexander Graf - May 27, 2011, 3:16 p.m.
On 27.05.2011, at 17:13, Josh Triplett wrote:

> On Thu, May 26, 2011 at 11:12:12AM +0200, Alexander Graf wrote:
>> On 26.05.2011, at 11:08, Josh Triplett wrote:
>>> qemu currently returns 0 for rdmsr on invalid MSRs, and ignores wrmsr on
>>> invalid MSRs.  Real x86 processors GPF on invalid MSRs, which allows
>>> software to detect unavailable MSRs.  Emulate this behavior correctly in
>>> qemu.
>>> 
>>> Bug discovered via the BIOS Implementation Test Suite
>>> <http://biosbits.org/>; fix tested the same way, for both 32-bit and
>>> 64-bit x86.
>> 
>> This would break a _lot_ of guests that work just fine today, as qemu doesn't handle all the necessary MSRs.
> 
> It also fixes guests that rely on the GPF to indicate the absence of an
> MSR, and assume that the lack of GPF means the availability of that MSR.
> Silently returning 0 for unknown MSRs means silent breakage.

It's not about guests triggereing MSRs that they shouldn't. It's that qemu doesn't implement all MSRs that all the respective CPUs implement.

> 
> What (buggy) guests expect to use random model-specific registers
> without either handling GPFs or checking the CPU model first?

Mac OS X for example :). It even breaks on KVM today due to MSR checks.

> 
> What MSRs do those guests expect that qemu doesn't currently implement?
> 
> If this represents a workaround for buggy guests, then may I add an
> option to control this behavior?

I'm not against this change per-se, but it should definitely have an option to disable/enable it and you need to do very extensive testing to make sure that all MSRs for most OSs are actually handled.


Alex
Josh Triplett - May 27, 2011, 3:46 p.m.
On Fri, May 27, 2011 at 05:16:56PM +0200, Alexander Graf wrote:
> 
> On 27.05.2011, at 17:13, Josh Triplett wrote:
> 
> > On Thu, May 26, 2011 at 11:12:12AM +0200, Alexander Graf wrote:
> >> On 26.05.2011, at 11:08, Josh Triplett wrote:
> >>> qemu currently returns 0 for rdmsr on invalid MSRs, and ignores wrmsr on
> >>> invalid MSRs.  Real x86 processors GPF on invalid MSRs, which allows
> >>> software to detect unavailable MSRs.  Emulate this behavior correctly in
> >>> qemu.
> >>> 
> >>> Bug discovered via the BIOS Implementation Test Suite
> >>> <http://biosbits.org/>; fix tested the same way, for both 32-bit and
> >>> 64-bit x86.
> >> 
> >> This would break a _lot_ of guests that work just fine today, as qemu doesn't handle all the necessary MSRs.
> > 
> > It also fixes guests that rely on the GPF to indicate the absence of an
> > MSR, and assume that the lack of GPF means the availability of that MSR.
> > Silently returning 0 for unknown MSRs means silent breakage.
> 
> It's not about guests triggereing MSRs that they shouldn't. It's that qemu doesn't implement all MSRs that all the respective CPUs implement.
> 
> > What (buggy) guests expect to use random model-specific registers
> > without either handling GPFs or checking the CPU model first?
> 
> Mac OS X for example :). It even breaks on KVM today due to MSR checks.

Ah, of course, since they only run on their own hardware.  Fair enough.

> > What MSRs do those guests expect that qemu doesn't currently implement?
> > 
> > If this represents a workaround for buggy guests, then may I add an
> > option to control this behavior?
> 
> I'm not against this change per-se, but it should definitely have an option to disable/enable it and you need to do very extensive testing to make sure that all MSRs for most OSs are actually handled.

Fair enough.  Expect PATCHv2 with an option in the near future.

- Josh Triplett

Patch

diff -Naur a/target-i386/op_helper.c b/target-i386/op_helper.c
--- a/target-i386/op_helper.c	2011-02-07 15:13:34.000000000 -0800
+++ b/target-i386/op_helper.c	2011-05-26 00:08:49.608636117 -0700
@@ -3135,7 +3135,7 @@ 
                 env->mce_banks[offset] = val;
             break;
         }
-        /* XXX: exception ? */
+        raise_exception(EXCP0D_GPF);
         break;
     }
 }
@@ -3266,7 +3266,7 @@ 
             val = env->mce_banks[offset];
             break;
         }
-        /* XXX: exception ? */
+        raise_exception(EXCP0D_GPF);
         val = 0;
         break;
     }