* [PATCH 0/3] Start fixing kvm-unit-tests for svm
@ 2021-06-14 10:08 Lara Lazier
2021-06-14 10:09 ` [PATCH 1/3] target/i386: Added consistency checks for VMRUN intercept and ASID Lara Lazier
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Lara Lazier @ 2021-06-14 10:08 UTC (permalink / raw)
To: qemu-devel; +Cc: Lara Lazier
Following the APM2 I added some checks to
resolve the following tests in kvm-unit-tests for svm:
* vmrun_intercept_check
* asid_zero
* sel_cr0_bug
* CR0 CD=0,NW=1: a0010011
* CR0 63:32: 180010011
* CR0 63:32: 1080010011
* CR0 63:32: 10080010011
* CR0 63:32: 100080010011
* CR0 63:32: 1000080010011
* CR0 63:32: 10000080010011
* CR0 63:32: 100000080010011
* CR0 63:32: 1000000080010011
Lara Lazier (3):
target/i386: Added consistency checks for VMRUN intercept and ASID
target/i386: Added consistency checks for CR0
target/i386: Added Intercept CR0 writes check
target/i386/cpu.h | 4 ++++
target/i386/svm.h | 3 +++
target/i386/tcg/sysemu/misc_helper.c | 9 +++++++++
target/i386/tcg/sysemu/svm_helper.c | 22 +++++++++++++++++++---
4 files changed, 35 insertions(+), 3 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] target/i386: Added consistency checks for VMRUN intercept and ASID
2021-06-14 10:08 [PATCH 0/3] Start fixing kvm-unit-tests for svm Lara Lazier
@ 2021-06-14 10:09 ` Lara Lazier
2021-06-15 12:24 ` Paolo Bonzini
2021-06-14 10:09 ` [PATCH 2/3] target/i386: Added consistency checks for CR0 Lara Lazier
2021-06-14 10:09 ` [PATCH 3/3] target/i386: Added Intercept CR0 writes check Lara Lazier
2 siblings, 1 reply; 5+ messages in thread
From: Lara Lazier @ 2021-06-14 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Lara Lazier
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(+)
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,
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] target/i386: Added consistency checks for CR0
2021-06-14 10:08 [PATCH 0/3] Start fixing kvm-unit-tests for svm Lara Lazier
2021-06-14 10:09 ` [PATCH 1/3] target/i386: Added consistency checks for VMRUN intercept and ASID Lara Lazier
@ 2021-06-14 10:09 ` Lara Lazier
2021-06-14 10:09 ` [PATCH 3/3] target/i386: Added Intercept CR0 writes check Lara Lazier
2 siblings, 0 replies; 5+ messages in thread
From: Lara Lazier @ 2021-06-14 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Lara Lazier
The combination of unset CD and set NW bit in CR0 is illegal.
CR0[63:32] are also reserved and need to be zero.
(AMD64 Architecture Programmer's Manual, V2, 15.5)
Signed-off-by: Lara Lazier <laramglazier@gmail.com>
---
target/i386/cpu.h | 2 ++
target/i386/svm.h | 1 +
target/i386/tcg/sysemu/svm_helper.c | 12 +++++++++---
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index ac3abea97c..46542513cc 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -224,6 +224,8 @@ typedef enum X86Seg {
#define CR0_NE_MASK (1U << 5)
#define CR0_WP_MASK (1U << 16)
#define CR0_AM_MASK (1U << 18)
+#define CR0_NW_MASK (1U << 29)
+#define CR0_CD_MASK (1U << 30)
#define CR0_PG_MASK (1U << 31)
#define CR4_VME_MASK (1U << 0)
diff --git a/target/i386/svm.h b/target/i386/svm.h
index 1c55d4f829..7a88906907 100644
--- a/target/i386/svm.h
+++ b/target/i386/svm.h
@@ -136,6 +136,7 @@
#define SVM_NPTEXIT_GPT (1ULL << 33)
#define SVM_VMRUN_INTERCEPT (1ULL << 32)
+#define SVM_CR0_RESERVED_MASK 0xffffffff00000000U
struct QEMU_PACKED vmcb_control_area {
uint16_t intercept_cr_read;
diff --git a/target/i386/tcg/sysemu/svm_helper.c b/target/i386/tcg/sysemu/svm_helper.c
index ff826fe11a..63aaa53490 100644
--- a/target/i386/tcg/sysemu/svm_helper.c
+++ b/target/i386/tcg/sysemu/svm_helper.c
@@ -73,6 +73,7 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
uint32_t event_inj;
uint32_t int_ctl;
uint32_t asid;
+ uint64_t new_cr0;
cpu_svm_check_intercept_param(env, SVM_EXIT_VMRUN, 0, GETPC());
@@ -192,13 +193,18 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
env->idt.limit = x86_ldl_phys(cs, env->vm_vmcb + offsetof(struct vmcb,
save.idtr.limit));
+ new_cr0 = x86_ldq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, save.cr0));
+ if (new_cr0 & SVM_CR0_RESERVED_MASK) {
+ cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
+ }
+ if ((new_cr0 & CR0_NW_MASK) && !(new_cr0 & CR0_CD_MASK)) {
+ cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
+ }
/* clear exit_info_2 so we behave like the real hardware */
x86_stq_phys(cs,
env->vm_vmcb + offsetof(struct vmcb, control.exit_info_2), 0);
- cpu_x86_update_cr0(env, x86_ldq_phys(cs,
- env->vm_vmcb + offsetof(struct vmcb,
- save.cr0)));
+ cpu_x86_update_cr0(env, new_cr0);
cpu_x86_update_cr4(env, x86_ldq_phys(cs,
env->vm_vmcb + offsetof(struct vmcb,
save.cr4)));
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] target/i386: Added Intercept CR0 writes check
2021-06-14 10:08 [PATCH 0/3] Start fixing kvm-unit-tests for svm Lara Lazier
2021-06-14 10:09 ` [PATCH 1/3] target/i386: Added consistency checks for VMRUN intercept and ASID Lara Lazier
2021-06-14 10:09 ` [PATCH 2/3] target/i386: Added consistency checks for CR0 Lara Lazier
@ 2021-06-14 10:09 ` Lara Lazier
2 siblings, 0 replies; 5+ messages in thread
From: Lara Lazier @ 2021-06-14 10:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Lara Lazier
When the selective CR0 write intercept is set, all writes to bits in
CR0 other than CR0.TS or CR0.MP cause a VMEXIT.
Signed-off-by: Lara Lazier <laramglazier@gmail.com>
---
target/i386/cpu.h | 2 ++
target/i386/tcg/sysemu/misc_helper.c | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 46542513cc..ff0ff97ca9 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -228,6 +228,8 @@ typedef enum X86Seg {
#define CR0_CD_MASK (1U << 30)
#define CR0_PG_MASK (1U << 31)
+#define INTERCEPT_SELECTIVE_CR0 (1ULL << 5)
+
#define CR4_VME_MASK (1U << 0)
#define CR4_PVI_MASK (1U << 1)
#define CR4_TSD_MASK (1U << 2)
diff --git a/target/i386/tcg/sysemu/misc_helper.c b/target/i386/tcg/sysemu/misc_helper.c
index 0cef2f1a4c..53117f47de 100644
--- a/target/i386/tcg/sysemu/misc_helper.c
+++ b/target/i386/tcg/sysemu/misc_helper.c
@@ -84,6 +84,15 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
{
switch (reg) {
case 0:
+ /*
+ * If we reach this point, the CR0 write intercept is disabled.
+ * But we could still exit if the hypervisor has requested the selective
+ * intercept for bits other than TS and MP
+ */
+ if ((env->intercept & INTERCEPT_SELECTIVE_CR0) &&
+ ((env->cr[0] ^ t0) & ~(CR0_TS_MASK | CR0_MP_MASK))) {
+ cpu_vmexit(env, SVM_EXIT_CR0_SEL_WRITE, 0, GETPC());
+ }
cpu_x86_update_cr0(env, t0);
break;
case 3:
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] target/i386: Added consistency checks for VMRUN intercept and ASID
2021-06-14 10:09 ` [PATCH 1/3] target/i386: Added consistency checks for VMRUN intercept and ASID Lara Lazier
@ 2021-06-15 12:24 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2021-06-15 12:24 UTC (permalink / raw)
To: Lara Lazier, qemu-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-15 12:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-14 10:08 [PATCH 0/3] Start fixing kvm-unit-tests for svm Lara Lazier
2021-06-14 10:09 ` [PATCH 1/3] target/i386: Added consistency checks for VMRUN intercept and ASID Lara Lazier
2021-06-15 12:24 ` Paolo Bonzini
2021-06-14 10:09 ` [PATCH 2/3] target/i386: Added consistency checks for CR0 Lara Lazier
2021-06-14 10:09 ` [PATCH 3/3] target/i386: Added Intercept CR0 writes check Lara Lazier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).