Re: [Qemu-devel] [PATCH] x86: Fix exceptions for fxsave/fxrstor

[Aurelien Jarno]

On Fri, Oct 02, 2009 at 10:28:57PM +0200, Kevin Wolf wrote:
> This patch corrects the following aspects of exception generation in
> fxsave/fxrstor:
> 
> * Generate #GP if the operand is not aligned to a 16 byte boundary

Agreed.

> * Generate #UD if the LOCK prefix is used

Agreed.

> * For CR0.EM = 1 #NM is generated, not #UD

This does not match the Intel manual:

| #NM If CR0.TS[bit 3] = 1.

| #UD If CR0.EM[bit 2] = 1.
|     If CPUID.01H:EDX.FXSR[bit 24] = 0.
|     If the LOCK prefix is used.

 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  target-i386/op_helper.c |   10 ++++++++++
>  target-i386/translate.c |    8 ++++----
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
> index 33d44b0..3f05532 100644
> --- a/target-i386/op_helper.c
> +++ b/target-i386/op_helper.c
> @@ -4338,6 +4338,11 @@ void helper_fxsave(target_ulong ptr, int data64)
>      CPU86_LDouble tmp;
>      target_ulong addr;
>  
> +    /* The operand must be 16 byte aligned */
> +    if (ptr & 0xf) {
> +        raise_exception(EXCP0D_GPF);
> +    }
> +
>      fpus = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
>      fptag = 0;
>      for(i = 0; i < 8; i++) {
> @@ -4394,6 +4399,11 @@ void helper_fxrstor(target_ulong ptr, int data64)
>      CPU86_LDouble tmp;
>      target_ulong addr;
>  
> +    /* The operand must be 16 byte aligned */
> +    if (ptr & 0xf) {
> +        raise_exception(EXCP0D_GPF);
> +    }
> +
>      env->fpuc = lduw(ptr);
>      fpus = lduw(ptr + 2);
>      fptag = lduw(ptr + 4);
> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index 5b11d7f..9af2eed 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -7436,9 +7436,9 @@ static target_ulong disas_insn(DisasContext *s, 
> target_ulong pc_start)
>          switch(op) {
>          case 0: /* fxsave */
>              if (mod == 3 || !(s->cpuid_features & CPUID_FXSR) ||
> -                (s->flags & HF_EM_MASK))
> +                (s->prefix & PREFIX_LOCK))
>                  goto illegal_op;
> -            if (s->flags & HF_TS_MASK) {
> +            if ((s->flags & HF_EM_MASK) || (s->flags & HF_TS_MASK)) {
>                  gen_exception(s, EXCP07_PREX, pc_start - s->cs_base);
>                  break;
>              }
> @@ -7450,9 +7450,9 @@ static target_ulong disas_insn(DisasContext *s, 
> target_ulong pc_start)
>              break;
>          case 1: /* fxrstor */
>              if (mod == 3 || !(s->cpuid_features & CPUID_FXSR) ||
> -                (s->flags & HF_EM_MASK))
> +                (s->prefix & PREFIX_LOCK))
>                  goto illegal_op;
> -            if (s->flags & HF_TS_MASK) {
> +            if ((s->flags & HF_EM_MASK) || (s->flags & HF_TS_MASK)) {
>                  gen_exception(s, EXCP07_PREX, pc_start - s->cs_base);
>                  break;
>              }
> -- 
> 1.6.0.2
> 
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net