Re: [PATCH v7 2/2] target/i386: Correct implementation for FCS, FIP, FDS and FDP

[Richard Henderson]

On 5/30/21 8:01 AM, Ziqiao Kong wrote:
      /* 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);

Directly calling a function named "helper" is bad practice.  Those should only be called 
from TCG generated code.  In this case it happens to be ok, since helper_fninit does not 
use GETPC(), but really we should break out a "do_fninit" function to share.


      case 0xd8 ... 0xdf:
          {
+            TCGv last_addr = tcg_temp_new();
+            int last_seg;
+            bool update_fdp = false;

These belong...

@@ -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);

... here, within the section that deals with memory operands.

+            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));
+            }

Similarly the update_fdp test belongs at the end of the memory op block.


r~