| Message ID | 20210614100902.15860-2-laramglazier@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Start fixing kvm-unit-tests for svm | expand |
On 14/06/21 12:09, Lara Lazier wrote: > +#define SVM_VMRUN_INTERCEPT (1ULL << 32) > + > struct QEMU_PACKED vmcb_control_area { > uint16_t intercept_cr_read; > uint16_t intercept_cr_write; ... > + if (!(env->intercept & SVM_VMRUN_INTERCEPT)) { > + cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC()); > + } Hi Lara, as discussed in our weekly meeting, the only issue with these patch is a matter of aesthetics and maintainability more than functionality; namely, the duplication between SVM_VMRUN_INTERCEPT and SVM_EXIT_VMRUN, and likewise in patch 3 between INTERCEPT_SELECTIVE_CR0 and SVM_EXIT_CR0_SEL_WRITE. Showing them side by side also makes it apparent that the names are not consistent, but it's even better to avoid the duplication altogether if possible. In particular, one way to do so is to extract the intercept checks to a function that you can call like cpu_svm_has_intercept(env, SVM_EXIT_VMRUN) so that the function computes the right bit of the bitmap based on the second argument. Most of the code to do this is already in svm_helper.c's cpu_svm_check_intercept_param, which you're already familiar with. cpu_svm_check_intercept_param can also be modified to call the new cpu_svm_has_intercept. When your second version of the patches are ready, you can add the "-v2" argument to git format-patch and it will automatically start the subjects with "[PATCH v2 ...]" instead of just "[PATCH ...]". Paolo
diff --git a/target/i386/svm.h b/target/i386/svm.h index 87965e5bc2..1c55d4f829 100644 --- a/target/i386/svm.h +++ b/target/i386/svm.h @@ -135,6 +135,8 @@ #define SVM_NPTEXIT_GPA (1ULL << 32) #define SVM_NPTEXIT_GPT (1ULL << 33) +#define SVM_VMRUN_INTERCEPT (1ULL << 32) + struct QEMU_PACKED vmcb_control_area { uint16_t intercept_cr_read; uint16_t intercept_cr_write; diff --git a/target/i386/tcg/sysemu/svm_helper.c b/target/i386/tcg/sysemu/svm_helper.c index 9d671297cf..ff826fe11a 100644 --- a/target/i386/tcg/sysemu/svm_helper.c +++ b/target/i386/tcg/sysemu/svm_helper.c @@ -72,6 +72,7 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend) uint64_t nested_ctl; uint32_t event_inj; uint32_t int_ctl; + uint32_t asid; cpu_svm_check_intercept_param(env, SVM_EXIT_VMRUN, 0, GETPC()); @@ -154,9 +155,18 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend) nested_ctl = x86_ldq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, control.nested_ctl)); + asid = x86_ldq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, + control.asid)); env->nested_pg_mode = 0; + if (!(env->intercept & SVM_VMRUN_INTERCEPT)) { + cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC()); + } + if (asid == 0) { + cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC()); + } + if (nested_ctl & SVM_NPT_ENABLED) { env->nested_cr3 = x86_ldq_phys(cs, env->vm_vmcb + offsetof(struct vmcb,
Zero VMRUN intercept and ASID should cause an immediate VMEXIT during the consistency checks performed by VMRUN. (AMD64 Architecture Programmer's Manual, V2, 15.5) Signed-off-by: Lara Lazier <laramglazier@gmail.com> --- target/i386/svm.h | 2 ++ target/i386/tcg/sysemu/svm_helper.c | 10 ++++++++++ 2 files changed, 12 insertions(+)