| Message ID | 1460626109-24343-1-git-send-email-shuai.ruan@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2016-04-14 17:28+0800, Shuai Ruan: > When passthrough the Intel GPU Device (igd) to windows guest, > the idg driver of the guest will read three MSRs: > > MSR_NHM_TURBO_RATIO_LIMIT > MSR_PLATFORM_INFO > MSR_PKG_POWER_LIMIT > > Windows guest os will crash as kvm will inject a #GP into the guest > if that tries to access unhandled MSRs, so add this three MSRS to > the list of ignored MSRs. It's not a list of ignored MSRs, it's a list of MSRs that are emulated by returning 0. I don't think that these MSRs return 0, so we should try harder: MSR_NHM_TURBO_RATIO_LIMIT: We probaly have to return the Maximum Ratio Limit (without turbo) and the MSR value also depends on the amount of cores the guest has per package. Writes are problematic, but maybe we can #GP if the value doesn't equal what we begin with. MSR_PKG_POWER_LIMIT: returning "1 << 63" (just the lock bit) might be acceptable. Writes could then be emulated by doing nothing. MSR_PLATFORM_INFO: Intel changes it from family to family and there is no obvious overlap or default. If we picked 0 (any other fixed value), then the guest would have to know that 0 doesn't mean that MSR_PLATFORM_INFO returned 0, but that KVM doesn't emulate this MSR and the value cannot be used. This is very similar to handling a #GP in the guest, but also has a disadvantage, because KVM cannot say that MSR_PLATFORM_INFO is 0. Simple emulation is not possible. The main bug in KVM is that it allows the guest to think that it runs on CPU that isn't emulated: any CPU that has MSR_PLATFORM_INFO shouldn't be exposed in KVM. Incorrect use of CPU also is a userspace problem, so KVM could just pass this MSR for emulation to userspace. (Passing to userspace won't directly fix your problem, though) The main bug in igd is that it always accesses these MSRs, even if the CPU doesn't have them, like when running on kvm64 CPU. igd needs to avoid accesses on kvm64 even if KVM changes, so what about catching the #GP in igd to fix this bug? :) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14/04/2016 15:33, Radim Kr?má? wrote: > The main bug in KVM is that it allows the guest to think that it runs on > CPU that isn't emulated: any CPU that has MSR_PLATFORM_INFO shouldn't > be exposed in KVM. That's all of them. f/m/s values and the model name are a useful debugging tool. Considering that MSR_PLATFORM_INFO is hardly ever used, it doesn't seem to be a great compromise. The actual question is why the hell a graphics driver cares about it. Paolo > Incorrect use of CPU also is a userspace problem, so > KVM could just pass this MSR for emulation to userspace. > (Passing to userspace won't directly fix your problem, though) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2016-04-14 16:42+0200, Paolo Bonzini: > On 14/04/2016 15:33, Radim Kr?má? wrote: >> The main bug in KVM is that it allows the guest to think that it runs on >> CPU that isn't emulated: any CPU that has MSR_PLATFORM_INFO shouldn't >> be exposed in KVM. > > That's all of them. f/m/s values and the model name are a useful > debugging tool. Considering that MSR_PLATFORM_INFO is hardly ever used, > it doesn't seem to be a great compromise. I don't see that as a compromise. igd would fail even if we fixed the host side, so we'll have problems regardless of what we do. We have a bug, because certain v/f/m/s implies some features (MSRs, constant_tsc, ...) and those aren't emulated. I do agree that we don't want to fix the bug, either by whitelisting and emulating features that makes little sense in virt or by forcing guests to adopt new v/f/m/s (the latter option is more reasonable), because rare occurences of the bug take *much* less work to fix on the guest side. (The only part I'm concerned about is that we don't have a good excuse for some guest errors ...) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14/04/2016 18:29, Radim Kr?má? wrote: > I don't see that as a compromise. igd would fail even if we fixed the > host side, so we'll have problems regardless of what we do. Would it? I suppose that Shuai tested his patch. > We have a bug, because certain v/f/m/s implies some features (MSRs, > constant_tsc, ...) and those aren't emulated. > > I do agree that we don't want to fix the bug, either by whitelisting and > emulating features that makes little sense in virt or by forcing guests > to adopt new v/f/m/s (the latter option is more reasonable), Well, the Pentium was the last processor without MSRs. :) More code would break if you set f=5 than if you return a bogus value for MSR_PLATFORM_INFO. This is the compromise I was referring to. The only solution is to bug Intel to add CPUID bits even for non-architectural features. Then _if_ the CPUID bit is there you use f/m/s to find the details of the feature. Intel likes to get feedback from us and we did provide such feedback. The problem is that the 2-3 years that pass between giving feedback and getting our hands on the silicon. Paolo > because > rare occurences of the bug take *much* less work to fix on the guest > side. (The only part I'm concerned about is that we don't have a good > excuse for some guest errors ...) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2016-04-15 12:48+0200, Paolo Bonzini: > On 14/04/2016 18:29, Radim Kr?má? wrote: >> I don't see that as a compromise. igd would fail even if we fixed the >> host side, so we'll have problems regardless of what we do. > > Would it? I suppose that Shuai tested his patch. I meant a fix that would make KVM behave according to the spec, which would not help igd on kvm64 CPU model, because igd accesses MSRs that don't exist, so #GP is the only response. The patch does completely work around current igd issue, but we trade an obvious #GP for an unknown error when the guest acts as if MSR_PLATFORM_INFO was 0. >> We have a bug, because certain v/f/m/s implies some features (MSRs, >> constant_tsc, ...) and those aren't emulated. >> >> I do agree that we don't want to fix the bug, either by whitelisting and >> emulating features that makes little sense in virt or by forcing guests >> to adopt new v/f/m/s (the latter option is more reasonable), > > Well, the Pentium was the last processor without MSRs. :) More code > would break if you set f=5 than if you return a bogus value for > MSR_PLATFORM_INFO. True. The loss becomes less clear with f=15 (kvm64 and Pentium 4) that "solves" the igd bug by not providing MSR_PLATFORM_INFO ... Btw. do we emulate any feature of newer GenuineIntel/f/m/s? > This is the compromise I was referring to. Ah, thanks. I only saw a complete disadvantage for KVM. :) > The only solution is to bug Intel to add CPUID bits even for > non-architectural features. Then _if_ the CPUID bit is there you use > f/m/s to find the details of the feature. Intel likes to get feedback > from us and we did provide such feedback. The problem is that the 2-3 > years that pass between giving feedback and getting our hands on the > silicon. That is great! (CPUID-only feature detection would suit us more, but f/m/s will at least have a chance to regain trust under KVM.) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Shuai Ruan [mailto:shuai.ruan@linux.intel.com] > Sent: Tuesday, April 19, 2016 2:20 AM > To: Radim Kr?má? <rkrcmar@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com>; kvm@vger.kernel.org; Kay, Allen > M <allen.m.kay@intel.com> > Subject: Re: [PATCH] KVM: x86: Add three MSRs to the list of ignored MSRs > > On Fri, Apr 15, 2016 at 04:12:39PM +0200, Radim Kr?má? wrote: > > 2016-04-15 12:48+0200, Paolo Bonzini: > > > On 14/04/2016 18:29, Radim Kr?má? wrote: > Hi Paolo/Radim: > > Thanks for your review and suggestion. > After confirm with igd driver team, they will fix MSR problem in the driver. > Just to provide more clarification on this, the driver team is looking into putting try-except around MSR register accesses. However, the change will only be in future driver release (15.45 version). In the meantime, please use KVM ignore_msrs option when doing IGD passthrough as indicated by Alex. Allen
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 9b7798c..f00a58e 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2301,6 +2301,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) case MSR_IA32_LASTBRANCHTOIP: case MSR_IA32_LASTINTFROMIP: case MSR_IA32_LASTINTTOIP: + case MSR_NHM_TURBO_RATIO_LIMIT: + case MSR_PLATFORM_INFO: + case MSR_PKG_POWER_LIMIT: case MSR_K8_SYSCFG: case MSR_K8_TSEG_ADDR: case MSR_K8_TSEG_MASK:
When passthrough the Intel GPU Device (igd) to windows guest, the idg driver of the guest will read three MSRs: MSR_NHM_TURBO_RATIO_LIMIT MSR_PLATFORM_INFO MSR_PKG_POWER_LIMIT Windows guest os will crash as kvm will inject a #GP into the guest if that tries to access unhandled MSRs, so add this three MSRS to the list of ignored MSRs. Signed-off-by: Shuai Ruan <shuai.ruan@linux.intel.com> --- arch/x86/kvm/x86.c | 3 +++ 1 file changed, 3 insertions(+)