1. Patchew
  2. QEMU
  3. View series

[PATCH v2] target/i386: Added consistency checks for EFER

Lara Lazier posted 1 patch 2 years, 9 months ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210721152651.14683-3-laramglazier@gmail.com
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>
target/i386/cpu.h                   |  5 ++++
target/i386/tcg/sysemu/svm_helper.c | 40 +++++++++++++++++++++++++++++
2 files changed, 45 insertions(+)
[PATCH v2] target/i386: Added consistency checks for EFER
Posted by Lara Lazier 2 years, 9 months ago 
EFER.SVME has to be set, and EFER reserved bits 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                   |  5 ++++
 target/i386/tcg/sysemu/svm_helper.c | 40 +++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 5d98a4e7c0..0b3057bdb6 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -466,6 +466,11 @@ typedef enum X86Seg {
 #define MSR_EFER_SVME  (1 << 12)
 #define MSR_EFER_FFXSR (1 << 14)
 
+#define MSR_EFER_RESERVED\
+        (~(target_ulong)(MSR_EFER_SCE | MSR_EFER_LME\
+            | MSR_EFER_LMA | MSR_EFER_NXE | MSR_EFER_SVME\
+            | MSR_EFER_FFXSR))
+
 #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 00618cff23..b6df36d4e5 100644
--- a/target/i386/tcg/sysemu/svm_helper.c
+++ b/target/i386/tcg/sysemu/svm_helper.c
@@ -65,6 +65,42 @@ 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);
@@ -278,6 +314,10 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
     }
 #endif
 
+    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
Re: [PATCH v2] target/i386: Added consistency checks for EFER
Posted by Paolo Bonzini 2 years, 9 months ago 
On 21/07/21 17:26, Lara Lazier wrote:
> EFER.SVME has to be set, and EFER reserved bits 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                   |  5 ++++
>   target/i386/tcg/sysemu/svm_helper.c | 40 +++++++++++++++++++++++++++++
>   2 files changed, 45 insertions(+)
> 
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 5d98a4e7c0..0b3057bdb6 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -466,6 +466,11 @@ typedef enum X86Seg {
>   #define MSR_EFER_SVME  (1 << 12)
>   #define MSR_EFER_FFXSR (1 << 14)
>   
> +#define MSR_EFER_RESERVED\
> +        (~(target_ulong)(MSR_EFER_SCE | MSR_EFER_LME\
> +            | MSR_EFER_LMA | MSR_EFER_NXE | MSR_EFER_SVME\
> +            | MSR_EFER_FFXSR))
> +
>   #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 00618cff23..b6df36d4e5 100644
> --- a/target/i386/tcg/sysemu/svm_helper.c
> +++ b/target/i386/tcg/sysemu/svm_helper.c
> @@ -65,6 +65,42 @@ 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);
> @@ -278,6 +314,10 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
>       }
>   #endif
>   
> +    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:
> 

Queued all, thanks.  However I modified the CR4 one to use a static 
inline function instead of a macro (the KVM code you based it on reuses 
the code for both the host and the guest CPUID, but this is not the case 
in QEMU).

Paolo

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.