Re: [PATCH 2/4] target/i386: Added DR6 and DR7 consistency checks

[Paolo Bonzini]

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))) 
> {