All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ziqiao Kong <ziqiaokong@gmail.com>
To: QEMU Developers <qemu-devel@nongnu.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Eduardo Habkost <ehabkost@redhat.com>
Subject: Re: [PATCH v7 2/2] target/i386: Correct implementation for FCS, FIP,  FDS and FDP
Date: Fri, 25 Jun 2021 01:06:43 +0800	[thread overview]
Message-ID: <CAM0BWNCZLmAP-XV7TA_DTes+9X3-bARo4jhbP2nxxCUGCcoydQ@mail.gmail.com> (raw)
In-Reply-To: <CAM0BWNBdENZwQ-p3M3owrjCfw3u_=vZvnCceP82gAbQ123TqSw@mail.gmail.com>

Ping.

On Fri, Jun 11, 2021 at 10:32 PM Ziqiao Kong <ziqiaokong@gmail.com> wrote:
>
> Ping.
>
> On Fri, Jun 4, 2021 at 11:04 PM Ziqiao Kong <ziqiaokong@gmail.com> wrote:
> >
> > Ping.
> >
> > Sorry again for the previous duplicate emails.
> >
> > On Sun, May 30, 2021 at 11:05 PM Ziqiao Kong <ziqiaokong@gmail.com> wrote:
> > >
> > > Update FCS:FIP and FDS:FDP according to the Intel Manual Vol.1 8.1.8. Note that
> > > CPUID.(EAX=07H,ECX=0H):EBX[bit 13] is not implemented by design in this patch
> > > and will be added along with TCG features flag in a separate patch later.
> > >
> > > Signed-off-by: Ziqiao Kong <ziqiaokong@gmail.com>
> > > ---
> > > Sorry for the duplicate emails due to my bad network. The v7 has no
> > > difference from v6 and is sent just for clarification.
> > > Changes since v5:
> > > - Improve code indention in translate.c.
> > > Changes since v4:
> > > - Remove the dead code about CPUID_7_0_EBX_FCS_FDS.
> > > - Rewrite the commit message.
> > > ---
> > >  target/i386/cpu.h            |  2 ++
> > >  target/i386/tcg/fpu_helper.c | 32 +++++++++++--------------
> > >  target/i386/tcg/translate.c  | 45 +++++++++++++++++++++++++++++++++++-
> > >  3 files changed, 59 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > > index da72aa5228..147dadcce0 100644
> > > --- a/target/i386/cpu.h
> > > +++ b/target/i386/cpu.h
> > > @@ -1455,6 +1455,8 @@ typedef struct CPUX86State {
> > >      FPReg fpregs[8];
> > >      /* KVM-only so far */
> > >      uint16_t fpop;
> > > +    uint16_t fpcs;
> > > +    uint16_t fpds;
> > >      uint64_t fpip;
> > >      uint64_t fpdp;
> > >
> > > diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
> > > index 1b30f1bb73..d953f04bb5 100644
> > > --- a/target/i386/tcg/fpu_helper.c
> > > +++ b/target/i386/tcg/fpu_helper.c
> > > @@ -728,6 +728,10 @@ void helper_fninit(CPUX86State *env)
> > >  {
> > >      env->fpus = 0;
> > >      env->fpstt = 0;
> > > +    env->fpcs = 0;
> > > +    env->fpds = 0;
> > > +    env->fpip = 0;
> > > +    env->fpdp = 0;
> > >      cpu_set_fpuc(env, 0x37f);
> > >      env->fptags[0] = 1;
> > >      env->fptags[1] = 1;
> > > @@ -2357,19 +2361,19 @@ static void do_fstenv(CPUX86State *env, target_ulong ptr, int data32,
> > >          cpu_stl_data_ra(env, ptr, env->fpuc, retaddr);
> > >          cpu_stl_data_ra(env, ptr + 4, fpus, retaddr);
> > >          cpu_stl_data_ra(env, ptr + 8, fptag, retaddr);
> > > -        cpu_stl_data_ra(env, ptr + 12, 0, retaddr); /* fpip */
> > > -        cpu_stl_data_ra(env, ptr + 16, 0, retaddr); /* fpcs */
> > > -        cpu_stl_data_ra(env, ptr + 20, 0, retaddr); /* fpoo */
> > > -        cpu_stl_data_ra(env, ptr + 24, 0, retaddr); /* fpos */
> > > +        cpu_stl_data_ra(env, ptr + 12, env->fpip, retaddr); /* fpip */
> > > +        cpu_stl_data_ra(env, ptr + 16, env->fpcs, retaddr); /* fpcs */
> > > +        cpu_stl_data_ra(env, ptr + 20, env->fpdp, retaddr); /* fpoo */
> > > +        cpu_stl_data_ra(env, ptr + 24, env->fpds, retaddr); /* fpos */
> > >      } else {
> > >          /* 16 bit */
> > >          cpu_stw_data_ra(env, ptr, env->fpuc, retaddr);
> > >          cpu_stw_data_ra(env, ptr + 2, fpus, retaddr);
> > >          cpu_stw_data_ra(env, ptr + 4, fptag, retaddr);
> > > -        cpu_stw_data_ra(env, ptr + 6, 0, retaddr);
> > > -        cpu_stw_data_ra(env, ptr + 8, 0, retaddr);
> > > -        cpu_stw_data_ra(env, ptr + 10, 0, retaddr);
> > > -        cpu_stw_data_ra(env, ptr + 12, 0, retaddr);
> > > +        cpu_stw_data_ra(env, ptr + 6, env->fpip, retaddr);
> > > +        cpu_stw_data_ra(env, ptr + 8, env->fpcs, retaddr);
> > > +        cpu_stw_data_ra(env, ptr + 10, env->fpdp, retaddr);
> > > +        cpu_stw_data_ra(env, ptr + 12, env->fpds, retaddr);
> > >      }
> > >  }
> > >
> > > @@ -2436,17 +2440,7 @@ static void do_fsave(CPUX86State *env, target_ulong ptr, int data32,
> > >      }
> > >
> > >      /* fninit */
> > > -    env->fpus = 0;
> > > -    env->fpstt = 0;
> > > -    cpu_set_fpuc(env, 0x37f);
> > > -    env->fptags[0] = 1;
> > > -    env->fptags[1] = 1;
> > > -    env->fptags[2] = 1;
> > > -    env->fptags[3] = 1;
> > > -    env->fptags[4] = 1;
> > > -    env->fptags[5] = 1;
> > > -    env->fptags[6] = 1;
> > > -    env->fptags[7] = 1;
> > > +    helper_fninit(env);
> > >  }
> > >
> > >  void helper_fsave(CPUX86State *env, target_ulong ptr, int data32)
> > > diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> > > index 5c1b7b87c5..4c57ee5c26 100644
> > > --- a/target/i386/tcg/translate.c
> > > +++ b/target/i386/tcg/translate.c
> > > @@ -5930,6 +5930,11 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
> > >          /* floats */
> > >      case 0xd8 ... 0xdf:
> > >          {
> > > +            TCGv last_addr = tcg_temp_new();
> > > +            int last_seg;
> > > +            bool update_fdp = false;
> > > +            bool update_fip = true;
> > > +
> > >              if (s->flags & (HF_EM_MASK | HF_TS_MASK)) {
> > >                  /* if CR0.EM or CR0.TS are set, generate an FPU exception */
> > >                  /* XXX: what to do if illegal op ? */
> > > @@ -5942,7 +5947,14 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
> > >              op = ((b & 7) << 3) | ((modrm >> 3) & 7);
> > >              if (mod != 3) {
> > >                  /* memory op */
> > > -                gen_lea_modrm(env, s, modrm);
> > > +                AddressParts a = gen_lea_modrm_0(env, s, modrm);
> > > +                TCGv ea = gen_lea_modrm_1(s, a);
> > > +
> > > +                update_fdp = true;
> > > +                last_seg = a.def_seg;
> > > +                tcg_gen_mov_tl(last_addr, ea);
> > > +                gen_lea_v_seg(s, s->aflag, ea, a.def_seg, s->override);
> > > +
> > >                  switch (op) {
> > >                  case 0x00 ... 0x07: /* fxxxs */
> > >                  case 0x10 ... 0x17: /* fixxxl */
> > > @@ -6070,20 +6082,24 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
> > >                  case 0x0c: /* fldenv mem */
> > >                      gen_helper_fldenv(cpu_env, s->A0,
> > >                                        tcg_const_i32(dflag - 1));
> > > +                    update_fip = update_fdp = false;
> > >                      break;
> > >                  case 0x0d: /* fldcw mem */
> > >                      tcg_gen_qemu_ld_i32(s->tmp2_i32, s->A0,
> > >                                          s->mem_index, MO_LEUW);
> > >                      gen_helper_fldcw(cpu_env, s->tmp2_i32);
> > > +                    update_fip = update_fdp = false;
> > >                      break;
> > >                  case 0x0e: /* fnstenv mem */
> > >                      gen_helper_fstenv(cpu_env, s->A0,
> > >                                        tcg_const_i32(dflag - 1));
> > > +                    update_fip = update_fdp = false;
> > >                      break;
> > >                  case 0x0f: /* fnstcw mem */
> > >                      gen_helper_fnstcw(s->tmp2_i32, cpu_env);
> > >                      tcg_gen_qemu_st_i32(s->tmp2_i32, s->A0,
> > >                                          s->mem_index, MO_LEUW);
> > > +                    update_fip = update_fdp = false;
> > >                      break;
> > >                  case 0x1d: /* fldt mem */
> > >                      gen_helper_fldt_ST0(cpu_env, s->A0);
> > > @@ -6095,15 +6111,18 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
> > >                  case 0x2c: /* frstor mem */
> > >                      gen_helper_frstor(cpu_env, s->A0,
> > >                                        tcg_const_i32(dflag - 1));
> > > +                    update_fip = update_fdp = false;
> > >                      break;
> > >                  case 0x2e: /* fnsave mem */
> > >                      gen_helper_fsave(cpu_env, s->A0,
> > >                                       tcg_const_i32(dflag - 1));
> > > +                    update_fip = update_fdp = false;
> > >                      break;
> > >                  case 0x2f: /* fnstsw mem */
> > >                      gen_helper_fnstsw(s->tmp2_i32, cpu_env);
> > >                      tcg_gen_qemu_st_i32(s->tmp2_i32, s->A0,
> > >                                          s->mem_index, MO_LEUW);
> > > +                    update_fip = update_fdp = false;
> > >                      break;
> > >                  case 0x3c: /* fbld */
> > >                      gen_helper_fbld_ST0(cpu_env, s->A0);
> > > @@ -6146,6 +6165,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
> > >                      case 0: /* fnop */
> > >                          /* check exceptions (FreeBSD FPU probe) */
> > >                          gen_helper_fwait(cpu_env);
> > > +                        update_fip = update_fdp = false;
> > >                          break;
> > >                      default:
> > >                          goto unknown_op;
> > > @@ -6315,9 +6335,11 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
> > >                          break;
> > >                      case 2: /* fclex */
> > >                          gen_helper_fclex(cpu_env);
> > > +                        update_fip = update_fdp = false;
> > >                          break;
> > >                      case 3: /* fninit */
> > >                          gen_helper_fninit(cpu_env);
> > > +                        update_fip = update_fdp = false;
> > >                          break;
> > >                      case 4: /* fsetpm (287 only, just do nop here) */
> > >                          break;
> > > @@ -6438,6 +6460,27 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
> > >                      goto unknown_op;
> > >                  }
> > >              }
> > > +
> > > +            if (update_fip) {
> > > +                tcg_gen_ld32u_tl(s->T0, cpu_env,
> > > +                                 offsetof(CPUX86State, segs[R_CS].selector));
> > > +                tcg_gen_st16_tl(s->T0, cpu_env, offsetof(CPUX86State, fpcs));
> > > +
> > > +                tcg_gen_movi_tl(s->T0, pc_start - s->cs_base);
> > > +                tcg_gen_st_tl(s->T0, cpu_env, offsetof(CPUX86State, fpip));
> > > +            }
> > > +
> > > +            if (update_fdp) {
> > > +                if (s->override >= 0) {
> > > +                    last_seg = s->override;
> > > +                }
> > > +                tcg_gen_ld32u_tl(s->T0, cpu_env,
> > > +                                 offsetof(CPUX86State,
> > > +                                 segs[last_seg].selector));
> > > +                tcg_gen_st16_tl(s->T0, cpu_env, offsetof(CPUX86State, fpds));
> > > +
> > > +                tcg_gen_st_tl(last_addr, cpu_env, offsetof(CPUX86State, fpdp));
> > > +            }
> > >          }
> > >          break;
> > >          /************************/
> > > --
> > > 2.25.1
> > >


  reply	other threads:[~2021-06-24 17:07 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-30 15:01 [PATCH v7 1/2] target/i386: Trivial code motion and code style fix Ziqiao Kong
2021-05-30 15:01 ` [PATCH v7 2/2] target/i386: Correct implementation for FCS, FIP, FDS and FDP Ziqiao Kong
2021-06-04 15:04   ` Ziqiao Kong
2021-06-11 14:32     ` Ziqiao Kong
2021-06-24 17:06       ` Ziqiao Kong [this message]
2021-07-01 13:58         ` Ziqiao Kong
2021-07-06 14:20           ` Ziqiao Kong
2021-07-06 22:20   ` Eduardo Habkost
2021-07-07 21:08   ` Richard Henderson
2021-06-04 15:04 ` [PATCH v7 1/2] target/i386: Trivial code motion and code style fix Ziqiao Kong
2021-06-11 14:32   ` Ziqiao Kong
2021-06-24 17:06     ` Ziqiao Kong
2021-07-01 13:58       ` Ziqiao Kong
2021-07-06 14:20         ` Ziqiao Kong
2021-07-07 21:01 ` Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAM0BWNCZLmAP-XV7TA_DTes+9X3-bARo4jhbP2nxxCUGCcoydQ@mail.gmail.com \
    --to=ziqiaokong@gmail.com \
    --cc=ehabkost@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.