* [PATCH 1/4] target/i386: Added MSRPM and IOPM size check
2021-07-05 8:17 [PATCH 0/4] target/i386: Continuing fixing kvm-unit-tests for svm Lara Lazier
@ 2021-07-05 8:17 ` Lara Lazier
2021-07-06 16:01 ` Paolo Bonzini
2021-07-05 8:18 ` [PATCH 2/4] target/i386: Added DR6 and DR7 consistency checks Lara Lazier
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Lara Lazier @ 2021-07-05 8:17 UTC (permalink / raw)
To: qemu-devel; +Cc: Lara Lazier
The address of the last entry in the MSRPM and
in the IOPM must be smaller than the largest physical address.
(APM2 15.10-15.11)
Signed-off-by: Lara Lazier <laramglazier@gmail.com>
---
target/i386/svm.h | 3 +++
target/i386/tcg/sysemu/svm_helper.c | 15 +++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/target/i386/svm.h b/target/i386/svm.h
index 5098733053..adc058dc76 100644
--- a/target/i386/svm.h
+++ b/target/i386/svm.h
@@ -137,6 +137,9 @@
#define SVM_CR0_RESERVED_MASK 0xffffffff00000000U
+#define SVM_MSRPM_SIZE (1ULL << 13)
+#define SVM_IOPM_SIZE ((1ULL << 13) + 1)
+
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 1c2dbc1862..fa701829e5 100644
--- a/target/i386/tcg/sysemu/svm_helper.c
+++ b/target/i386/tcg/sysemu/svm_helper.c
@@ -68,6 +68,7 @@ static inline void svm_load_seg_cache(CPUX86State *env, hwaddr addr,
void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
{
CPUState *cs = env_cpu(env);
+ X86CPU *cpu = env_archcpu(env);
target_ulong addr;
uint64_t nested_ctl;
uint32_t event_inj;
@@ -159,6 +160,20 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
asid = x86_ldq_phys(cs, env->vm_vmcb + offsetof(struct vmcb,
control.asid));
+ uint64_t msrpm_base_pa = x86_ldq_phys(cs, env->vm_vmcb +
+ offsetof(struct vmcb,
+ control.msrpm_base_pa));
+ uint64_t iopm_base_pa = x86_ldq_phys(cs, env->vm_vmcb +
+ offsetof(struct vmcb, control.iopm_base_pa));
+
+ if ((msrpm_base_pa & ~0xfff) >= (1ull << cpu->phys_bits) - SVM_MSRPM_SIZE) {
+ cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
+ }
+
+ if ((iopm_base_pa & ~0xfff) >= (1ull << cpu->phys_bits) - SVM_IOPM_SIZE) {
+ cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
+ }
+
env->nested_pg_mode = 0;
if (!cpu_svm_has_intercept(env, SVM_EXIT_VMRUN)) {
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/4] target/i386: Added MSRPM and IOPM size check
2021-07-05 8:17 ` [PATCH 1/4] target/i386: Added MSRPM and IOPM size check Lara Lazier
@ 2021-07-06 16:01 ` Paolo Bonzini
0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2021-07-06 16:01 UTC (permalink / raw)
To: Lara Lazier, qemu-devel
On 05/07/21 10:17, Lara Lazier wrote:
> The address of the last entry in the MSRPM and
> in the IOPM must be smaller than the largest physical address.
> (APM2 15.10-15.11)
>
> Signed-off-by: Lara Lazier <laramglazier@gmail.com>
> ---
> target/i386/svm.h | 3 +++
> target/i386/tcg/sysemu/svm_helper.c | 15 +++++++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/target/i386/svm.h b/target/i386/svm.h
> index 5098733053..adc058dc76 100644
> --- a/target/i386/svm.h
> +++ b/target/i386/svm.h
> @@ -137,6 +137,9 @@
>
> #define SVM_CR0_RESERVED_MASK 0xffffffff00000000U
>
> +#define SVM_MSRPM_SIZE (1ULL << 13)
> +#define SVM_IOPM_SIZE ((1ULL << 13) + 1)
> +
> 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 1c2dbc1862..fa701829e5 100644
> --- a/target/i386/tcg/sysemu/svm_helper.c
> +++ b/target/i386/tcg/sysemu/svm_helper.c
> @@ -68,6 +68,7 @@ static inline void svm_load_seg_cache(CPUX86State *env, hwaddr addr,
> void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
> {
> CPUState *cs = env_cpu(env);
> + X86CPU *cpu = env_archcpu(env);
> target_ulong addr;
> uint64_t nested_ctl;
> uint32_t event_inj;
> @@ -159,6 +160,20 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
> asid = x86_ldq_phys(cs, env->vm_vmcb + offsetof(struct vmcb,
> control.asid));
>
> + uint64_t msrpm_base_pa = x86_ldq_phys(cs, env->vm_vmcb +
> + offsetof(struct vmcb,
> + control.msrpm_base_pa));
> + uint64_t iopm_base_pa = x86_ldq_phys(cs, env->vm_vmcb +
> + offsetof(struct vmcb, control.iopm_base_pa));
> +
> + if ((msrpm_base_pa & ~0xfff) >= (1ull << cpu->phys_bits) - SVM_MSRPM_SIZE) {
> + cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
> + }
> +
> + if ((iopm_base_pa & ~0xfff) >= (1ull << cpu->phys_bits) - SVM_IOPM_SIZE) {
> + cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
> + }
> +
> env->nested_pg_mode = 0;
>
> if (!cpu_svm_has_intercept(env, SVM_EXIT_VMRUN)) {
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/4] target/i386: Added DR6 and DR7 consistency checks
2021-07-05 8:17 [PATCH 0/4] target/i386: Continuing fixing kvm-unit-tests for svm Lara Lazier
2021-07-05 8:17 ` [PATCH 1/4] target/i386: Added MSRPM and IOPM size check Lara Lazier
@ 2021-07-05 8:18 ` Lara Lazier
2021-07-06 16:02 ` Paolo Bonzini
2021-07-05 8:18 ` [PATCH 3/4] target/i386: Added consistency checks for EFER Lara Lazier
2021-07-05 8:18 ` [PATCH 4/4] target/i386: Added VMRUN consistency checks for CR3 and CR4 Lara Lazier
3 siblings, 1 reply; 9+ messages in thread
From: Lara Lazier @ 2021-07-05 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Lara Lazier
DR6[63:32] and DR7[63:32] are reserved and need to be zero.
(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 | 8 +++++++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/target/i386/svm.h b/target/i386/svm.h
index adc058dc76..e54670ef12 100644
--- a/target/i386/svm.h
+++ b/target/i386/svm.h
@@ -140,6 +140,8 @@
#define SVM_MSRPM_SIZE (1ULL << 13)
#define SVM_IOPM_SIZE ((1ULL << 13) + 1)
+#define SVM_DR_RESERVED_MASK 0xffffffff00000000U
+
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 fa701829e5..276c240f70 100644
--- a/target/i386/tcg/sysemu/svm_helper.c
+++ b/target/i386/tcg/sysemu/svm_helper.c
@@ -269,7 +269,13 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
env->dr[6] = x86_ldq_phys(cs,
env->vm_vmcb + offsetof(struct vmcb, save.dr6));
- /* FIXME: guest state consistency checks */
+ if (env->dr[6] & SVM_DR_RESERVED_MASK) {
+ cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
+ }
+
+ if (env->dr[7] & SVM_DR_RESERVED_MASK) {
+ cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
+ }
switch (x86_ldub_phys(cs,
env->vm_vmcb + offsetof(struct vmcb, control.tlb_ctl))) {
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/4] target/i386: Added DR6 and DR7 consistency checks
2021-07-05 8:18 ` [PATCH 2/4] target/i386: Added DR6 and DR7 consistency checks Lara Lazier
@ 2021-07-06 16:02 ` Paolo Bonzini
0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2021-07-06 16:02 UTC (permalink / raw)
To: Lara Lazier, qemu-devel
On 05/07/21 10:18, Lara Lazier wrote:
> DR6[63:32] and DR7[63:32] are reserved and need to be zero.
> (AMD64 Architecture Programmer's Manual, V2, 15.5)
>
> Signed-off-by: Lara Lazier <laramglazier@gmail.com>
The checks need to be disabled on 32-bit builds where env->dr[6] and
env->dr[7] is a 32-bit value (compilers or static analyzers would
complain about always-zero conditions) but otherwise the patch is fine.
When you fetch from the QEMU repository and rebase, you will be able
to use my changes just with "git rebase --skip".
Queued, thanks.
Paolo
> ---
> target/i386/svm.h | 2 ++
> target/i386/tcg/sysemu/svm_helper.c | 8 +++++++-
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/svm.h b/target/i386/svm.h
> index adc058dc76..e54670ef12 100644
> --- a/target/i386/svm.h
> +++ b/target/i386/svm.h
> @@ -140,6 +140,8 @@
> #define SVM_MSRPM_SIZE (1ULL << 13)
> #define SVM_IOPM_SIZE ((1ULL << 13) + 1)
>
> +#define SVM_DR_RESERVED_MASK 0xffffffff00000000U
> +
> 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 fa701829e5..276c240f70 100644
> --- a/target/i386/tcg/sysemu/svm_helper.c
> +++ b/target/i386/tcg/sysemu/svm_helper.c
> @@ -269,7 +269,13 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
> env->dr[6] = x86_ldq_phys(cs,
> env->vm_vmcb + offsetof(struct vmcb, save.dr6));
>
> - /* FIXME: guest state consistency checks */
> + if (env->dr[6] & SVM_DR_RESERVED_MASK) {
> + cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
> + }
> +
> + if (env->dr[7] & SVM_DR_RESERVED_MASK) {
> + cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
> + }
>
> switch (x86_ldub_phys(cs,
> env->vm_vmcb + offsetof(struct vmcb, control.tlb_ctl))) {
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] target/i386: Added consistency checks for EFER
2021-07-05 8:17 [PATCH 0/4] target/i386: Continuing fixing kvm-unit-tests for svm Lara Lazier
2021-07-05 8:17 ` [PATCH 1/4] target/i386: Added MSRPM and IOPM size check Lara Lazier
2021-07-05 8:18 ` [PATCH 2/4] target/i386: Added DR6 and DR7 consistency checks Lara Lazier
@ 2021-07-05 8:18 ` Lara Lazier
2021-07-06 16:48 ` Paolo Bonzini
2021-07-05 8:18 ` [PATCH 4/4] target/i386: Added VMRUN consistency checks for CR3 and CR4 Lara Lazier
3 siblings, 1 reply; 9+ messages in thread
From: Lara Lazier @ 2021-07-05 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Lara Lazier
EFER.SVME has to be set, and EFER[63:16], EFER[9], EFER[7:5]
are reserved and must be zero.
In Addition the combinations
* EFER.LMA or EFER.LME is non-zero and the processor does not support LM
* non-zero EFER.LME and CR0.PG and zero CR4.PAE
* non-zero EFER.LME and CR0.PG and zero CR0.PE
* non-zero EFER.LME, CR0.PG, CR4.PAE, CS.L and CS.D
are all invalid.
(AMD64 Architecture Programmer's Manual, V2, 15.5)
Signed-off-by: Lara Lazier <laramglazier@gmail.com>
---
target/i386/cpu.h | 2 ++
target/i386/tcg/sysemu/svm_helper.c | 39 +++++++++++++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index f7fa5870b1..f5280b2951 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -464,6 +464,8 @@ typedef enum X86Seg {
#define MSR_EFER_SVME (1 << 12)
#define MSR_EFER_FFXSR (1 << 14)
+#define MSR_EFER_RESERVED 0xffffffffffff02e0
+
#define MSR_STAR 0xc0000081
#define MSR_LSTAR 0xc0000082
#define MSR_CSTAR 0xc0000083
diff --git a/target/i386/tcg/sysemu/svm_helper.c b/target/i386/tcg/sysemu/svm_helper.c
index 276c240f70..d652d6f9da 100644
--- a/target/i386/tcg/sysemu/svm_helper.c
+++ b/target/i386/tcg/sysemu/svm_helper.c
@@ -65,6 +65,41 @@ static inline void svm_load_seg_cache(CPUX86State *env, hwaddr addr,
sc->base, sc->limit, sc->flags);
}
+static inline bool is_efer_invalid_state (CPUX86State *env)
+{
+ if (!(env->efer & MSR_EFER_SVME)) {
+ return true;
+ }
+
+ if (env->efer & MSR_EFER_RESERVED) {
+ return true;
+ }
+
+ if ((env->efer & (MSR_EFER_LMA | MSR_EFER_LME)) &&
+ !(env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM)) {
+ return true;
+ }
+
+ if ((env->efer & MSR_EFER_LME) && (env->cr[0] & CR0_PG_MASK)
+ && !(env->cr[4] & CR4_PAE_MASK)) {
+ return true;
+ }
+
+ if ((env->efer & MSR_EFER_LME) && (env->cr[0] & CR0_PG_MASK)
+ && !(env->cr[0] & CR0_PE_MASK)) {
+ return true;
+ }
+
+ if ((env->efer & MSR_EFER_LME) && (env->cr[0] & CR0_PG_MASK)
+ && (env->cr[4] & CR4_PAE_MASK)
+ && (env->segs[R_CS].flags & DESC_L_MASK)
+ && (env->segs[R_CS].flags & DESC_B_MASK)) {
+ return true;
+ }
+
+ return false;
+}
+
void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
{
CPUState *cs = env_cpu(env);
@@ -277,6 +312,10 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
}
+ if (is_efer_invalid_state(env)) {
+ cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
+ }
+
switch (x86_ldub_phys(cs,
env->vm_vmcb + offsetof(struct vmcb, control.tlb_ctl))) {
case TLB_CONTROL_DO_NOTHING:
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] target/i386: Added consistency checks for EFER
2021-07-05 8:18 ` [PATCH 3/4] target/i386: Added consistency checks for EFER Lara Lazier
@ 2021-07-06 16:48 ` Paolo Bonzini
0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2021-07-06 16:48 UTC (permalink / raw)
To: Lara Lazier, qemu-devel
> EFER.SVME has to be set, and EFER[63:16], EFER[9], EFER[7:5]
> are reserved and must be zero.
My version of the manual says "any MBZ [must-be-zero] bit of EFER is
set", so that would be 7:1 (not 7:5), 9 and 63:16. In QEMU bits 13 and
15 are also unimplemented and thus must-be-zero.
On 05/07/21 10:18, Lara Lazier wrote:
> +#define MSR_EFER_RESERVED 0xffffffffffff02e0
> +
This has the same issue with 32-bit compilation, except in this case the
check *is* needed on 32-bit builds just without bits 63:32 set.
The obvious way here would be to add a #ifdef, but that's less
maintainable than the slightly ugly:
#define MSR_EFER_RESERVED ((target_ulong)(int)0xffff02e0u)
(where I was too lazy to compute the right mask for the bits I listed
above...).
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] target/i386: Added VMRUN consistency checks for CR3 and CR4
2021-07-05 8:17 [PATCH 0/4] target/i386: Continuing fixing kvm-unit-tests for svm Lara Lazier
` (2 preceding siblings ...)
2021-07-05 8:18 ` [PATCH 3/4] target/i386: Added consistency checks for EFER Lara Lazier
@ 2021-07-05 8:18 ` Lara Lazier
2021-07-06 16:52 ` Paolo Bonzini
3 siblings, 1 reply; 9+ messages in thread
From: Lara Lazier @ 2021-07-05 8:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Lara Lazier
All MBZ bits in CR3 and CR4 must be zero. (APM2 15.5)
Added reserved bitmask for CR4 and added checks in both
helper_vmrun and helper_write_crN.
Signed-off-by: Lara Lazier <laramglazier@gmail.com>
---
target/i386/cpu.h | 29 ++++++++++++++++++++++++++++
target/i386/tcg/sysemu/misc_helper.c | 6 ++++++
target/i386/tcg/sysemu/svm_helper.c | 18 +++++++++++------
3 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index f5280b2951..0368551a66 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -240,6 +240,7 @@ typedef enum X86Seg {
#define CR4_OSFXSR_SHIFT 9
#define CR4_OSFXSR_MASK (1U << CR4_OSFXSR_SHIFT)
#define CR4_OSXMMEXCPT_MASK (1U << 10)
+#define CR4_UMIP_MASK (1U << 11)
#define CR4_LA57_MASK (1U << 12)
#define CR4_VMXE_MASK (1U << 13)
#define CR4_SMXE_MASK (1U << 14)
@@ -251,6 +252,34 @@ typedef enum X86Seg {
#define CR4_PKE_MASK (1U << 22)
#define CR4_PKS_MASK (1U << 24)
+#define CR4_RESERVED_MASK \
+(~(unsigned long)(CR4_VME_MASK | CR4_PVI_MASK | CR4_TSD_MASK \
+ | CR4_DE_MASK | CR4_PSE_MASK | CR4_PAE_MASK \
+ | CR4_MCE_MASK | CR4_PGE_MASK | CR4_PCE_MASK \
+ | CR4_OSFXSR_MASK | CR4_OSXMMEXCPT_MASK |CR4_UMIP_MASK \
+ | CR4_FSGSBASE_MASK | CR4_PCIDE_MASK | CR4_OSXSAVE_MASK \
+ | CR4_SMEP_MASK | CR4_SMAP_MASK | CR4_PKE_MASK))
+
+#define cr4_reserved_bits(env) \
+({ \
+ uint64_t __reserved_bits = CR4_RESERVED_MASK; \
+ if (!env->features[FEAT_XSAVE]) \
+ __reserved_bits |= CR4_OSXSAVE_MASK; \
+ if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_SMEP)) \
+ __reserved_bits |= CR4_SMEP_MASK; \
+ if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_SMAP)) \
+ __reserved_bits |= CR4_SMAP_MASK; \
+ if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_FSGSBASE)) \
+ __reserved_bits |= CR4_FSGSBASE_MASK; \
+ if (!(env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_PKU)) \
+ __reserved_bits |= CR4_PKE_MASK; \
+ if (!(env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_LA57)) \
+ __reserved_bits |= CR4_LA57_MASK; \
+ if (!(env->features[FEAT_7_0_ECX] & CPUID_7_0_ECX_UMIP)) \
+ __reserved_bits |= CR4_UMIP_MASK; \
+ __reserved_bits; \
+})
+
#define DR6_BD (1 << 13)
#define DR6_BS (1 << 14)
#define DR6_BT (1 << 15)
diff --git a/target/i386/tcg/sysemu/misc_helper.c b/target/i386/tcg/sysemu/misc_helper.c
index db0d8a9d79..cdb4826987 100644
--- a/target/i386/tcg/sysemu/misc_helper.c
+++ b/target/i386/tcg/sysemu/misc_helper.c
@@ -96,9 +96,15 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
cpu_x86_update_cr0(env, t0);
break;
case 3:
+ if (t0 & ((~0UL) << env_archcpu(env)->phys_bits)) {
+ cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
+ }
cpu_x86_update_cr3(env, t0);
break;
case 4:
+ if (t0 & cr4_reserved_bits(env)) {
+ cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
+ }
if (((t0 ^ env->cr[4]) & CR4_LA57_MASK) &&
(env->hflags & HF_CS64_MASK)) {
raise_exception_ra(env, EXCP0D_GPF, GETPC());
diff --git a/target/i386/tcg/sysemu/svm_helper.c b/target/i386/tcg/sysemu/svm_helper.c
index d652d6f9da..e3a4fa74d2 100644
--- a/target/i386/tcg/sysemu/svm_helper.c
+++ b/target/i386/tcg/sysemu/svm_helper.c
@@ -110,6 +110,8 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
uint32_t int_ctl;
uint32_t asid;
uint64_t new_cr0;
+ uint64_t new_cr3;
+ uint64_t new_cr4;
cpu_svm_check_intercept_param(env, SVM_EXIT_VMRUN, 0, GETPC());
@@ -250,17 +252,21 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
if ((new_cr0 & CR0_NW_MASK) && !(new_cr0 & CR0_CD_MASK)) {
cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
}
+ new_cr3 = x86_ldq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, save.cr3));
+ if (new_cr3 & ((~0UL) << cpu->phys_bits)) {
+ cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC());
+ }
+ new_cr4 = x86_ldq_phys(cs, env->vm_vmcb + offsetof(struct vmcb, save.cr4));
+ if (new_cr4 & cr4_reserved_bits(env)) {
+ 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, new_cr0);
- cpu_x86_update_cr4(env, x86_ldq_phys(cs,
- env->vm_vmcb + offsetof(struct vmcb,
- save.cr4)));
- cpu_x86_update_cr3(env, x86_ldq_phys(cs,
- env->vm_vmcb + offsetof(struct vmcb,
- save.cr3)));
+ cpu_x86_update_cr4(env, new_cr4);
+ cpu_x86_update_cr3(env, new_cr3);
env->cr[2] = x86_ldq_phys(cs,
env->vm_vmcb + offsetof(struct vmcb, save.cr2));
int_ctl = x86_ldl_phys(cs,
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] target/i386: Added VMRUN consistency checks for CR3 and CR4
2021-07-05 8:18 ` [PATCH 4/4] target/i386: Added VMRUN consistency checks for CR3 and CR4 Lara Lazier
@ 2021-07-06 16:52 ` Paolo Bonzini
0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2021-07-06 16:52 UTC (permalink / raw)
To: Lara Lazier, qemu-devel
On 05/07/21 10:18, Lara Lazier wrote:
> +#define CR4_RESERVED_MASK \
> +(~(unsigned long)(CR4_VME_MASK | CR4_PVI_MASK | CR4_TSD_MASK \
> + | CR4_DE_MASK | CR4_PSE_MASK | CR4_PAE_MASK \
> + | CR4_MCE_MASK | CR4_PGE_MASK | CR4_PCE_MASK \
> + | CR4_OSFXSR_MASK | CR4_OSXMMEXCPT_MASK |CR4_UMIP_MASK \
> + | CR4_FSGSBASE_MASK | CR4_PCIDE_MASK | CR4_OSXSAVE_MASK \
> + | CR4_SMEP_MASK | CR4_SMAP_MASK | CR4_PKE_MASK))
> +
This ~ trick could also be useful for EFER, very nice!
Just a couple changes required:
1) CR4_PKS_MASK is missing here and in cr4_reserved_bits (TCG supports
it but KVM does not)
2) the cast should be to target_ulong (to cover the case of 32-bit host
and 64-bit emulated processor)
In addition, as discussed on our weekly call CR3 checks are not complete
so it's probably best to focus on CR4 for this patch and split CR3 to a
different one.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread