Message ID | 20181219110613.24459-1-vkuznets@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM: x86: svm: report MSR_IA32_MCG_EXT_CTL as unsupported | expand |
2018-12-19 12:06+0100, Vitaly Kuznetsov: > AMD doesn't seem to implement MSR_IA32_MCG_EXT_CTL and svm code in kvm > knows nothing about it, however, this MSR is among emulated_msrs and > thus returned with KVM_GET_MSR_INDEX_LIST. The consequent KVM_GET_MSRS, > of course, fails. > > Report the MSR as unsupported to not confuse userspace. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/x86/kvm/svm.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 2acb42b74a51..dfdf7d0b7f88 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -5845,6 +5845,13 @@ static bool svm_cpu_has_accelerated_tpr(void) > > static bool svm_has_emulated_msr(int index) > { > + switch (index) { > + case MSR_IA32_MCG_EXT_CTL: > + return false; > + default: > + break; Queued, thanks. Btw, I would prefer this without the default: break; as I don't think we'll ever add something there.
Radim Krčmář <rkrcmar@redhat.com> writes: > 2018-12-19 12:06+0100, Vitaly Kuznetsov: >> AMD doesn't seem to implement MSR_IA32_MCG_EXT_CTL and svm code in kvm >> knows nothing about it, however, this MSR is among emulated_msrs and >> thus returned with KVM_GET_MSR_INDEX_LIST. The consequent KVM_GET_MSRS, >> of course, fails. >> >> Report the MSR as unsupported to not confuse userspace. >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> --- >> arch/x86/kvm/svm.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index 2acb42b74a51..dfdf7d0b7f88 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -5845,6 +5845,13 @@ static bool svm_cpu_has_accelerated_tpr(void) >> >> static bool svm_has_emulated_msr(int index) >> { >> + switch (index) { >> + case MSR_IA32_MCG_EXT_CTL: >> + return false; >> + default: >> + break; > > Queued, thanks. > > Btw, I would prefer this without the > > default: break; > > as I don't think we'll ever add something there. "640K ought to be enough for anybody" :-) But in case you commit it as return index != MSR_IA32_MCG_EXT_CTL; I won't object. Thanks,
On 21/12/18 11:06, Vitaly Kuznetsov wrote: >> Btw, I would prefer this without the >> >> default: break; >> >> as I don't think we'll ever add something there. > > "640K ought to be enough for anybody" :-) > > But in case you commit it as > > return index != MSR_IA32_MCG_EXT_CTL; > > I won't object. Thanks, Radim committed it as it, but I think he did want the switch statement (hey, vmx_has_emulated_msr has *two* cases!). It's just the "default: break;" that is unnecessary or could be "default: return true;". Paolo
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 2acb42b74a51..dfdf7d0b7f88 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -5845,6 +5845,13 @@ static bool svm_cpu_has_accelerated_tpr(void) static bool svm_has_emulated_msr(int index) { + switch (index) { + case MSR_IA32_MCG_EXT_CTL: + return false; + default: + break; + } + return true; }
AMD doesn't seem to implement MSR_IA32_MCG_EXT_CTL and svm code in kvm knows nothing about it, however, this MSR is among emulated_msrs and thus returned with KVM_GET_MSR_INDEX_LIST. The consequent KVM_GET_MSRS, of course, fails. Report the MSR as unsupported to not confuse userspace. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- arch/x86/kvm/svm.c | 7 +++++++ 1 file changed, 7 insertions(+)