[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 05/37] target/i386: add core of new i386 decoder
From: |
Richard Henderson |
Subject: |
Re: [PATCH 05/37] target/i386: add core of new i386 decoder |
Date: |
Mon, 12 Sep 2022 10:27:58 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 |
On 9/12/22 00:03, Paolo Bonzini wrote:
+ case X86_TYPE_B: /* VEX.vvvv selects a GPR */
+ op->unit = X86_OP_INT;
+ op->n = s->vex_v;
+ break;
Could use a comment for where missing vex prefix is diagnosed.
I guess it's one of the "vexN" group markers in the insn table?
+ case X86_TYPE_S: /* reg selects a segment register */
+ op->unit = X86_OP_SEG;
+ goto get_reg;
+
+ goto get_reg;
Stray goto.
+
+ case X86_TYPE_V: /* reg in the modrm byte selects an XMM/YMM register */
+ if (decode->e.special == X86_SPECIAL_MMX &&
+ !(s->prefix & (PREFIX_DATA | PREFIX_REPZ | PREFIX_REPNZ))) {
+ case X86_TYPE_P: /* reg in the modrm byte selects an MMX register */
+ op->unit = X86_OP_MMX;
+ } else {
+ op->unit = X86_OP_SSE;
+ }
+ get_reg:
Nesting P into the if works, but it's ugly.
Better to separate it out as
case X86_TYPE_P:
op->unit = X86_OP_MMX;
goto get_reg;
+ case X86_TYPE_W: /* XMM/YMM modrm operand */
+ if (decode->e.special == X86_SPECIAL_MMX &&
+ !(s->prefix & (PREFIX_DATA | PREFIX_REPZ | PREFIX_REPNZ))) {
+ case X86_TYPE_Q: /* MMX modrm operand */
+ op->unit = X86_OP_MMX;
+ } else {
+ op->unit = X86_OP_SSE;
+ }
+ goto get_modrm;
Likewise.
+ case X86_TYPE_U: /* R/M in the modrm byte selects an XMM/YMM register */
+ if (decode->e.special == X86_SPECIAL_MMX &&
+ !(s->prefix & (PREFIX_DATA | PREFIX_REPZ | PREFIX_REPNZ))) {
+ case X86_TYPE_N: /* R/M in the modrm byte selects an MMX register */
+ op->unit = X86_OP_MMX;
+ } else {
+ op->unit = X86_OP_SSE;
+ }
+ goto get_modrm_reg;
Likewise.
+ case X86_TYPE_H: /* For AVX, VEX.vvvv selects an XMM/YMM register */
+ if ((s->prefix & PREFIX_VEX)) {
+ op->unit = X86_OP_SSE;
+ op->n = s->vex_v;
+ break;
Similar to X86_TYPE_B, should this diagnose error if missing VEX?
+ e X86_TYPE_J: /* Relative offset for a jump */
+ op->unit = X86_OP_IMM;
+ decode->immediate = insn_get_signed(env, s, op->ot);
Mailer damage?
+ decode->immediate += s->pc - s->cs_base;
Please consider
20220906100932.343523-1-richard.henderson@linaro.org/">https://lore.kernel.org/qemu-devel/20220906100932.343523-1-richard.henderson@linaro.org/
or at least the first half of the patch set, which rationalizes and consolidates the
handing of s->cs_base.
+ default:
+ abort();
g_assert_not_reached().
+static bool decode_insn(DisasContext *s, CPUX86State *env, X86DecodeFunc
decode_func,
+ X86DecodedInsn *decode)
+{
+ X86OpEntry *e = &decode->e;
+
+ decode_func(s, env, e, &decode->b);
+ while (e->is_decode) {
+ e->is_decode = false;
+ e->decode(s, env, e, &decode->b);
+ }
+
+ /* First compute size of operands in order to initialize s->rip_offset. */
+ if (e->op0 != X86_TYPE_None) {
+ if (!decode_op_size(s, e, e->s0, &decode->op[0].ot)) {
+ return false;
+ }
+ if (e->op0 == X86_TYPE_I) {
+ s->rip_offset += 1 << decode->op[0].ot;
+ }
+ }
+ if (e->op1 != X86_TYPE_None) {
+ if (!decode_op_size(s, e, e->s1, &decode->op[1].ot)) {
+ return false;
+ }
+ if (e->op1 == X86_TYPE_I) {
+ s->rip_offset += 1 << decode->op[1].ot;
+ }
+ }
+ if (e->op2 != X86_TYPE_None) {
+ if (!decode_op_size(s, e, e->s2, &decode->op[2].ot)) {
+ return false;
+ }
+ if (e->op2 == X86_TYPE_I) {
+ s->rip_offset += 1 << decode->op[2].ot;
+ }
+ }
+ if (e->op3 != X86_TYPE_None) {
+ assert(e->op3 == X86_TYPE_I && e->s3 == X86_SIZE_b);
+ s->rip_offset += 1;
+ }
+
+ if (e->op0 != X86_TYPE_None &&
+ !decode_op(s, env, decode, &decode->op[0], e->op0, decode->b)) {
+ return false;
+ }
+
+ if (e->op1 != X86_TYPE_None &&
+ !decode_op(s, env, decode, &decode->op[1], e->op1, decode->b)) {
+ return false;
+ }
+
+ if (e->op2 != X86_TYPE_None &&
+ !decode_op(s, env, decode, &decode->op[2], e->op2, decode->b)) {
+ return false;
+ }
+
+ if (e->op3 != X86_TYPE_None) {
+ decode->immediate = insn_get_signed(env, s, MO_8);
+ }
+
+ return true;
+}
+
+/* convert one instruction. s->base.is_jmp is set if the translation must
+ be stopped. Return the next pc value */
+static target_ulong disas_insn_new(DisasContext *s, CPUState *cpu, int b)
Note patch 2 from the cs_base cleanup above changes the return type from
disas_insn to bool.
+{
+ CPUX86State *env = cpu->env_ptr;
+ bool first = true;
+ X86DecodedInsn decode;
+ X86DecodeFunc decode_func = decode_root;
+
+#ifdef CONFIG_USER_ONLY
+ if (limit) { --limit; }
+#endif
+ s->has_modrm = false;
+#if 0
+ s->pc_start = s->pc = s->base.pc_next;
+ s->override = -1;
+#ifdef TARGET_X86_64
+ s->rex_w = false;
+ s->rex_r = 0;
+ s->rex_x = 0;
+ s->rex_b = 0;
+#endif
+ s->prefix = 0;
+ s->rip_offset = 0; /* for relative ip address */
+ s->vex_l = 0;
+ s->vex_v = 0;
+ if (sigsetjmp(s->jmpbuf, 0) != 0) {
+ gen_exception_gpf(s);
+ return s->pc;
+ }
Mainline has two longjmp error paths:
(1) insn too long: raise #GP,
(2) insn crosses page boundary, and isn't first in the TB:
undo processing and defer insn to next TB.
+static inline target_long insn_get_signed(CPUX86State *env, DisasContext *s,
MemOp ot)
No need for inline.
r~
- [RFC PATCH 00/37] target/i386: new decoder + AVX implementation, Paolo Bonzini, 2022/09/11
- [PATCH 02/37] target/i386: make ldo/sto operations consistent with ldq, Paolo Bonzini, 2022/09/11
- [PATCH 05/37] target/i386: add core of new i386 decoder, Paolo Bonzini, 2022/09/11
- [PATCH 01/37] target/i386: Define XMMReg and access macros, align ZMM registers, Paolo Bonzini, 2022/09/11
- [PATCH 03/37] target/i386: REPZ and REPNZ are mutually exclusive, Paolo Bonzini, 2022/09/11
- [PATCH 06/37] target/i386: add ALU load/writeback core, Paolo Bonzini, 2022/09/11
- [PATCH 07/37] target/i386: add CPUID[EAX=7, ECX=0].ECX to DisasContext, Paolo Bonzini, 2022/09/11
- [PATCH 08/37] target/i386: add CPUID feature checks to new decoder, Paolo Bonzini, 2022/09/11