| Message ID | 20210412215611.110095-3-krish.sadhukhan@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: nSVM: Check addresses of MSR bitmap and IO bitmap tables on vmrun of nested guests | expand |
On 12/04/21 23:56, Krish Sadhukhan wrote: > nested_svm_vmrun() returns SVM_EXIT_ERR both when consistency checks for > MSRPM fail and when merging the MSRPM of vmcb12 with that of KVM fails. These > two failures are different in that the first one happens during consistency > checking while the second happens after consistency checking passes and after > guest mode switch is done. In order to differentiate between the two types of > error conditions, define an exit code that can be used to denote consistency > check failures. This new exit code is similar to what nVMX uses to denote > consistency check failures. For nSVM, we will use the highest bit in the high > part of the EXIT_CODE field. > > Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> The exit code is defined by AMD, we cannot change it. Paolo > --- > arch/x86/include/uapi/asm/svm.h | 1 + > arch/x86/kvm/svm/nested.c | 2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h > index 554f75fe013c..b0a6550a23f5 100644 > --- a/arch/x86/include/uapi/asm/svm.h > +++ b/arch/x86/include/uapi/asm/svm.h > @@ -111,6 +111,7 @@ > #define SVM_VMGEXIT_UNSUPPORTED_EVENT 0x8000ffff > > #define SVM_EXIT_ERR -1 > +#define SVM_CONSISTENCY_ERR 1 << 31 > > #define SVM_EXIT_REASONS \ > { SVM_EXIT_READ_CR0, "read_cr0" }, \ > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c > index 8453c898b68b..ae53ae46ebca 100644 > --- a/arch/x86/kvm/svm/nested.c > +++ b/arch/x86/kvm/svm/nested.c > @@ -606,7 +606,7 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu) > if (!nested_vmcb_valid_sregs(vcpu, &vmcb12->save) || > !nested_vmcb_check_controls(&svm->nested.ctl)) { > vmcb12->control.exit_code = SVM_EXIT_ERR; > - vmcb12->control.exit_code_hi = 0; > + vmcb12->control.exit_code_hi = SVM_CONSISTENCY_ERR; > vmcb12->control.exit_info_1 = 0; > vmcb12->control.exit_info_2 = 0; > goto out; >
On 4/17/21 7:17 AM, Paolo Bonzini wrote: > On 12/04/21 23:56, Krish Sadhukhan wrote: >> nested_svm_vmrun() returns SVM_EXIT_ERR both when consistency checks for >> MSRPM fail and when merging the MSRPM of vmcb12 with that of KVM >> fails. These >> two failures are different in that the first one happens during >> consistency >> checking while the second happens after consistency checking passes >> and after >> guest mode switch is done. In order to differentiate between the two >> types of >> error conditions, define an exit code that can be used to denote >> consistency >> check failures. This new exit code is similar to what nVMX uses to >> denote >> consistency check failures. For nSVM, we will use the highest bit in >> the high >> part of the EXIT_CODE field. >> >> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> > > The exit code is defined by AMD, we cannot change it. The reason why I thought of this is that SVM implementation uses only the lower half, as all AMD-defined exit code are handled therein only. Is this still going to cause an issue ? > > Paolo > >> --- >> arch/x86/include/uapi/asm/svm.h | 1 + >> arch/x86/kvm/svm/nested.c | 2 +- >> 2 files changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/include/uapi/asm/svm.h >> b/arch/x86/include/uapi/asm/svm.h >> index 554f75fe013c..b0a6550a23f5 100644 >> --- a/arch/x86/include/uapi/asm/svm.h >> +++ b/arch/x86/include/uapi/asm/svm.h >> @@ -111,6 +111,7 @@ >> #define SVM_VMGEXIT_UNSUPPORTED_EVENT 0x8000ffff >> #define SVM_EXIT_ERR -1 >> +#define SVM_CONSISTENCY_ERR 1 << 31 >> #define SVM_EXIT_REASONS \ >> { SVM_EXIT_READ_CR0, "read_cr0" }, \ >> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c >> index 8453c898b68b..ae53ae46ebca 100644 >> --- a/arch/x86/kvm/svm/nested.c >> +++ b/arch/x86/kvm/svm/nested.c >> @@ -606,7 +606,7 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu) >> if (!nested_vmcb_valid_sregs(vcpu, &vmcb12->save) || >> !nested_vmcb_check_controls(&svm->nested.ctl)) { >> vmcb12->control.exit_code = SVM_EXIT_ERR; >> - vmcb12->control.exit_code_hi = 0; >> + vmcb12->control.exit_code_hi = SVM_CONSISTENCY_ERR; >> vmcb12->control.exit_info_1 = 0; >> vmcb12->control.exit_info_2 = 0; >> goto out; >> >
On 19/04/21 19:57, Krish Sadhukhan wrote: > The reason why I thought of this is that SVM implementation uses only > the lower half, as all AMD-defined exit code are handled therein only. > Is this still going to cause an issue ? I would have to check what happens on bare metal, but VMEXIT_INVALID is defined as "-1", not "FFFFFFFFh", so I think it should use the high 32 bits (in which case KVM is wrong in not storing the high 32 bits). Paolo
On Mon, Apr 19, 2021 at 11:28 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 19/04/21 19:57, Krish Sadhukhan wrote: > > The reason why I thought of this is that SVM implementation uses only > > the lower half, as all AMD-defined exit code are handled therein only. > > Is this still going to cause an issue ? > > I would have to check what happens on bare metal, but VMEXIT_INVALID is > defined as "-1", not "FFFFFFFFh", so I think it should use the high 32 > bits (in which case KVM is wrong in not storing the high 32 bits). And VMEXIT_BUSY is -2.
diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h index 554f75fe013c..b0a6550a23f5 100644 --- a/arch/x86/include/uapi/asm/svm.h +++ b/arch/x86/include/uapi/asm/svm.h @@ -111,6 +111,7 @@ #define SVM_VMGEXIT_UNSUPPORTED_EVENT 0x8000ffff #define SVM_EXIT_ERR -1 +#define SVM_CONSISTENCY_ERR 1 << 31 #define SVM_EXIT_REASONS \ { SVM_EXIT_READ_CR0, "read_cr0" }, \ diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c index 8453c898b68b..ae53ae46ebca 100644 --- a/arch/x86/kvm/svm/nested.c +++ b/arch/x86/kvm/svm/nested.c @@ -606,7 +606,7 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu) if (!nested_vmcb_valid_sregs(vcpu, &vmcb12->save) || !nested_vmcb_check_controls(&svm->nested.ctl)) { vmcb12->control.exit_code = SVM_EXIT_ERR; - vmcb12->control.exit_code_hi = 0; + vmcb12->control.exit_code_hi = SVM_CONSISTENCY_ERR; vmcb12->control.exit_info_1 = 0; vmcb12->control.exit_info_2 = 0; goto out;
nested_svm_vmrun() returns SVM_EXIT_ERR both when consistency checks for MSRPM fail and when merging the MSRPM of vmcb12 with that of KVM fails. These two failures are different in that the first one happens during consistency checking while the second happens after consistency checking passes and after guest mode switch is done. In order to differentiate between the two types of error conditions, define an exit code that can be used to denote consistency check failures. This new exit code is similar to what nVMX uses to denote consistency check failures. For nSVM, we will use the highest bit in the high part of the EXIT_CODE field. Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> --- arch/x86/include/uapi/asm/svm.h | 1 + arch/x86/kvm/svm/nested.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)