| Message ID | 20110526090804.GA32647@leaf |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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; }
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(-)