| Message ID | 20210722123018.260035-1-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Check the right feature bit for MSR_KVM_ASYNC_PF_ACK access | expand |
On Thu, 2021-07-22 at 14:30 +0200, Vitaly Kuznetsov wrote: > MSR_KVM_ASYNC_PF_ACK MSR is part of interrupt based asynchronous page fault > interface and not the original (deprecated) KVM_FEATURE_ASYNC_PF. This is > stated in Documentation/virt/kvm/msr.rst. > > Fixes: 66570e966dd9 ("kvm: x86: only provide PV features if enabled in guest's CPUID") > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/x86/kvm/x86.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index d715ae9f9108..88ff7a1af198 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3406,7 +3406,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > return 1; > break; > case MSR_KVM_ASYNC_PF_ACK: > - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) > + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) > return 1; > if (data & 0x1) { > vcpu->arch.apf.pageready_pending = false; > @@ -3745,7 +3745,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > msr_info->data = vcpu->arch.apf.msr_int_val; > break; > case MSR_KVM_ASYNC_PF_ACK: > - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) > + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) > return 1; > > msr_info->data = 0; Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky
On Thu, Jul 22, 2021 at 5:39 AM Maxim Levitsky <mlevitsk@redhat.com> wrote: > > On Thu, 2021-07-22 at 14:30 +0200, Vitaly Kuznetsov wrote: > > MSR_KVM_ASYNC_PF_ACK MSR is part of interrupt based asynchronous page fault > > interface and not the original (deprecated) KVM_FEATURE_ASYNC_PF. This is > > stated in Documentation/virt/kvm/msr.rst. > > > > Fixes: 66570e966dd9 ("kvm: x86: only provide PV features if enabled in guest's CPUID") > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > > --- > > arch/x86/kvm/x86.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index d715ae9f9108..88ff7a1af198 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -3406,7 +3406,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > > return 1; > > break; > > case MSR_KVM_ASYNC_PF_ACK: > > - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) > > + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) > > return 1; > > if (data & 0x1) { > > vcpu->arch.apf.pageready_pending = false; > > @@ -3745,7 +3745,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > > msr_info->data = vcpu->arch.apf.msr_int_val; > > break; > > case MSR_KVM_ASYNC_PF_ACK: > > - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) > > + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) > > return 1; > > > > msr_info->data = 0; > > Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> > > Best regards, > Maxim Levitsky > Reviewed-by: Oliver Upton <oupton@google.com> -- Thanks, Oliver
On Thu, Jul 22, 2021, Vitaly Kuznetsov wrote: > MSR_KVM_ASYNC_PF_ACK MSR is part of interrupt based asynchronous page fault > interface and not the original (deprecated) KVM_FEATURE_ASYNC_PF. This is > stated in Documentation/virt/kvm/msr.rst. > > Fixes: 66570e966dd9 ("kvm: x86: only provide PV features if enabled in guest's CPUID") > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/x86/kvm/x86.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index d715ae9f9108..88ff7a1af198 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3406,7 +3406,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > return 1; > break; > case MSR_KVM_ASYNC_PF_ACK: > - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) > + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) Do we want to require both, or do we want to let userspace be stupid? > return 1; > if (data & 0x1) { > vcpu->arch.apf.pageready_pending = false; > @@ -3745,7 +3745,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > msr_info->data = vcpu->arch.apf.msr_int_val; > break; > case MSR_KVM_ASYNC_PF_ACK: > - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) > + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) > return 1; > > msr_info->data = 0; > -- > 2.31.1 >
Sean Christopherson <seanjc@google.com> writes: > On Thu, Jul 22, 2021, Vitaly Kuznetsov wrote: >> MSR_KVM_ASYNC_PF_ACK MSR is part of interrupt based asynchronous page fault >> interface and not the original (deprecated) KVM_FEATURE_ASYNC_PF. This is >> stated in Documentation/virt/kvm/msr.rst. >> >> Fixes: 66570e966dd9 ("kvm: x86: only provide PV features if enabled in guest's CPUID") >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> --- >> arch/x86/kvm/x86.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index d715ae9f9108..88ff7a1af198 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -3406,7 +3406,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> return 1; >> break; >> case MSR_KVM_ASYNC_PF_ACK: >> - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) >> + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) > > Do we want to require both, or do we want to let userspace be stupid? > It's OK to be stupid :-) Thinking more about it, I'd suggest we go the other way around: allow access to MSR_KVM_ASYNC_PF_EN when either KVM_FEATURE_ASYNC_PF or KVM_FEATURE_ASYNC_PF_INT are present. This will allow to eventually deprecate KVM_FEATURE_ASYNC_PF completely and switch to KVM_FEATURE_ASYNC_PF_INT exclusively. >> return 1; >> if (data & 0x1) { >> vcpu->arch.apf.pageready_pending = false; >> @@ -3745,7 +3745,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> msr_info->data = vcpu->arch.apf.msr_int_val; >> break; >> case MSR_KVM_ASYNC_PF_ACK: >> - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) >> + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) >> return 1; >> >> msr_info->data = 0; >> -- >> 2.31.1 >> >
On 22/07/21 14:30, Vitaly Kuznetsov wrote: > MSR_KVM_ASYNC_PF_ACK MSR is part of interrupt based asynchronous page fault > interface and not the original (deprecated) KVM_FEATURE_ASYNC_PF. This is > stated in Documentation/virt/kvm/msr.rst. > > Fixes: 66570e966dd9 ("kvm: x86: only provide PV features if enabled in guest's CPUID") > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/x86/kvm/x86.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index d715ae9f9108..88ff7a1af198 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3406,7 +3406,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > return 1; > break; > case MSR_KVM_ASYNC_PF_ACK: > - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) > + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) > return 1; > if (data & 0x1) { > vcpu->arch.apf.pageready_pending = false; > @@ -3745,7 +3745,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > msr_info->data = vcpu->arch.apf.msr_int_val; > break; > case MSR_KVM_ASYNC_PF_ACK: > - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) > + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) > return 1; > > msr_info->data = 0; > Queued, thanks. Paolo
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index d715ae9f9108..88ff7a1af198 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3406,7 +3406,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) return 1; break; case MSR_KVM_ASYNC_PF_ACK: - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) return 1; if (data & 0x1) { vcpu->arch.apf.pageready_pending = false; @@ -3745,7 +3745,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) msr_info->data = vcpu->arch.apf.msr_int_val; break; case MSR_KVM_ASYNC_PF_ACK: - if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF)) + if (!guest_pv_has(vcpu, KVM_FEATURE_ASYNC_PF_INT)) return 1; msr_info->data = 0;
MSR_KVM_ASYNC_PF_ACK MSR is part of interrupt based asynchronous page fault interface and not the original (deprecated) KVM_FEATURE_ASYNC_PF. This is stated in Documentation/virt/kvm/msr.rst. Fixes: 66570e966dd9 ("kvm: x86: only provide PV features if enabled in guest's CPUID") Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- arch/x86/kvm/x86.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)