|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 1/5] x86/emul: Introduce x86_decode_lite()
> --- /dev/null
> +++ b/xen/arch/x86/x86_emulate/decode-lite.c
> @@ -0,0 +1,330 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifdef __XEN__
> +# include <xen/init.h>
> +# include <xen/livepatch.h>
> +#endif
> +
> +#include "private.h"
> +
> +#undef ModRM
> +
> +/*
> + * Bare minimum x86 instruction decoder to parse the alternative replacement
> + * instructions and locate the IP-relative references that may need updating.
> + *
> + * These are:
> + * - disp8/32 from near direct branches
> + * - RIP-relative memory references
> + *
> + * The following simplifications are used:
> + * - All code is 64bit, the instruction stream is well formed and safe to
> + * read.
> + * - Instruction groups and prefixes not used by Xen's current alternatives
> + * are not implemented in order to reduce the decode complexity.
> + * - Certain instructions are intentionally not recognised, when it is more
> + * likely for their presence to be an error than intentional.
> + *
> + * Inputs:
> + * @ip The position to start decoding from.
> + * @end End of the replacement block. Exceeding this is considered an
> error.
Why do you mention replacement blocks here? Are we entirely set on this
code not possibly gaining any purpose beyond the scanning of those?
> + * Returns: x86_decode_lite_t
> + * - On failure, length of 0.
> + * - On success, length > 0. For rel_sz > 0, rel points at the relative
> + * field in the instruction stream.
> + */
> +x86_decode_lite_t init_or_livepatch x86_decode_lite(void *ip, void *end)
Is there a reason the parameters can't be pointer-to-const? Hmm,
apparently for x86_decode_lite_t's "rel" field not be plaing void *, "ip"
needs to be this way as well. But not "end", I don't think.
> +{
> +#define Imm8 (1 << 0)
> +#define Imm (1 << 1)
> +#define Moffs (1 << 2)
> +#define Branch (1 << 5) /* Near direct branches, which have a displacement */
> +#define ModRM (1 << 6)
> +#define Known (1 << 7)
> +
> + static const uint8_t init_or_livepatch_const onebyte[256] = {
> +
> +#define ALU_OPS(x) \
> + [(x) + 0] = (Known|ModRM), \
> + [(x) + 1] = (Known|ModRM), \
> + [(x) + 2] = (Known|ModRM), \
> + [(x) + 3] = (Known|ModRM), \
> + [(x) + 4] = (Known|Imm8), \
> + [(x) + 5] = (Known|Imm)
> +
> + ALU_OPS(0x00) /* ADD */, ALU_OPS(0x08) /* OR */,
> + ALU_OPS(0x10) /* ADC */, ALU_OPS(0x18) /* SBB */,
> + ALU_OPS(0x20) /* AND */, ALU_OPS(0x28) /* SUB */,
> + ALU_OPS(0x30) /* XOR */, ALU_OPS(0x38) /* CMP */,
> +
> +#undef ALU_OPS
> +
> + [0x50 ... 0x5f] = (Known), /* PUSH/POP %reg */
> +
> + [0x62] = 0, /* BOUND, but also EVEX
> prefix, not implemented. */
> + [0x63] = (Known|ModRM), /* MOVSxd */
> +
> + [0x68] = (Known|Imm), /* PUSH $imm */
> + [0x69] = (Known|ModRM|Imm), /* IMUL $imm */
> + [0x6a] = (Known|Imm8), /* PUSH $imm8 */
> + [0x6b] = (Known|ModRM|Imm8), /* PUSH $imm8 */
> + [0x6c ... 0x6f] = (Known), /* INS/OUTS */
> + [0x70 ... 0x7f] = (Known|Branch|Imm8), /* Jcc disp8 */
> + [0x80] = (Known|ModRM|Imm8), /* Grp1 */
> + [0x81] = (Known|ModRM|Imm), /* Grp1 */
> +
> + [0x83] = (Known|ModRM|Imm8), /* Grp1 */
> + [0x84 ... 0x8e] = (Known|ModRM), /* TEST/XCHG/MOV/MOV-SREG/LEA
> */
> + [0x8f] = 0, /* Grp1A - POP but also XOP
> prefix, not implemented. */
POP doesn't look all that unlikely to be used in inline assembly, and
hence in alternatives. That said, of course using it with a memory
operand requires quite a bit of care. I don't see you excluding the
PUSH counterpart, though - being consistent for any such pairs would
seem somewhat desirable.
> + [0x90 ... 0x99] = (Known), /* NOP/XCHG %rAX/CLTQ/CQTO */
> +
> + [0x9b ... 0x9f] = (Known), /* FWAIT/PUSHF/POPF/SAHF/LAHF
> */
> + [0xa0 ... 0xa3] = (Known|Moffs), /* MOVABS */
> + [0xa4 ... 0xa7] = (Known), /* MOVS/CMPS */
> + [0xa8] = (Known|Imm8), /* TEST %al */
> + [0xa9] = (Known|Imm), /* TEST %rAX */
> + [0xaa ... 0xaf] = (Known), /* STOS/LODS/SCAS */
> + [0xb0 ... 0xb7] = (Known|Imm8), /* MOV $imm8, %reg */
> + [0xb8 ... 0xbf] = (Known|Imm), /* MOV $imm{16,32,64}, %reg */
> + [0xc0 ... 0xc1] = (Known|ModRM|Imm8), /* Grp2 (ROL..SAR $imm8,
> %reg) */
> +
> + [0xc3] = (Known), /* RET */
> + [0xc4 ... 0xc5] = 0, /* LES/LDS but also VEX
> prefixes, not implemented. */
This may bite us sooner or later, due to the VEX-encoded integer insns
that there are. Of course as long as we don't use this function on
compiled code, and as long as my "x86: allow Kconfig control over psABI
level" doesn't come close to going in, that's merely a theoretical
concern.
Same goes for not supporting the 3-byte opcodes, which also encode
certain integer insns.
> + [0xc6] = (Known|ModRM|Imm8), /* Grp11, Further ModRM
> decode */
> + [0xc7] = (Known|ModRM|Imm), /* Grp11, Further ModRM
> decode */
> +
> + [0xcb ... 0xcc] = (Known), /* LRET/INT3 */
> + [0xcd] = (Known|Imm8), /* INT $imm8 */
> +
> + [0xd0 ... 0xd3] = (Known|ModRM), /* Grp2 (ROL..SAR {$1,%cl},
> %reg) */
> +
> + [0xd6] = (Known), /* UDB */
I guess you consider XLAT, LOOP*, and J*CXZ as too odd to use in alternatives?
Decoding-wise they're rather easy to implement.
> + [0xe4 ... 0xe7] = (Known|Imm8), /* IN/OUT $imm8 */
> + [0xe8 ... 0xe9] = (Known|Branch|Imm), /* CALL/JMP disp32 */
> +
> + [0xeb] = (Known|Branch|Imm8), /* JMP disp8 */
> + [0xec ... 0xef] = (Known), /* IN/OUT %dx */
> +
> + [0xf1] = (Known), /* ICEBP */
> +
> + [0xf4] = (Known), /* HLT */
> + [0xf5] = (Known), /* CMC */
> + [0xf6 ... 0xf7] = (Known|ModRM), /* Grp3, Further ModRM decode
> */
> + [0xf8 ... 0xfd] = (Known), /* CLC ... STD */
> + [0xfe ... 0xff] = (Known|ModRM), /* Grp4 */
> + };
> + static const uint8_t init_or_livepatch_const twobyte[256] = {
> + [0x00 ... 0x03] = (Known|ModRM), /* Grp6/Grp7/LAR/LSL */
Leaving out INVD is surely find, but WBINVD?
> + [0x0b] = (Known), /* UD2 */
> +
> + [0x18 ... 0x1f] = (Known|ModRM), /* Grp16 (Hint Nop) */
> + [0x20 ... 0x23] = (Known|ModRM), /* MOV %cr/%dr */
> +
> + [0x30 ... 0x33] = (Known), /* WRMSR/RDTSC/RDMSR/RDPMC */
> +
> + [0x40 ... 0x4f] = (Known|ModRM), /* CMOVcc */
> +
> + [0x80 ... 0x8f] = (Known|Branch|Imm), /* Jcc disp32 */
> + [0x90 ... 0x9f] = (Known|ModRM), /* SETcc */
> +
> + [0xa0 ... 0xa2] = (Known), /* PUSH/POP %fs/CPUID */
> + [0xa3] = (Known|ModRM), /* BT */
> + [0xa4] = (Known|ModRM|Imm8), /* SHLD $imm8 */
> + [0xa5] = (Known|ModRM), /* SHLD %cl */
> +
> + [0xa8 ... 0xa9] = (Known), /* PUSH/POP %gs */
> +
> + [0xab] = (Known|ModRM), /* BTS */
> + [0xac] = (Known|ModRM|Imm8), /* SHRD $imm8 */
> + [0xad ... 0xaf] = (Known|ModRM), /* SHRD %cl/Grp15/IMUL */
> +
> + [0xb0 ... 0xb9] = (Known|ModRM), /*
> CMPXCHG/LSS/BTR/LFS/LGS/MOVZxx/POPCNT/UD1 */
> + [0xba] = (Known|ModRM|Imm8), /* Grp8 */
> + [0xbb ... 0xbf] = (Known|ModRM), /* BTC/BSF/BSR/MOVSX */
> + [0xc0 ... 0xc1] = (Known|ModRM), /* XADD */
What about MOVNTI?
> + [0xc7] = (Known|ModRM), /* Grp9 */
> + [0xc8 ... 0xcf] = (Known), /* BSWAP */
> + };
What about UD0?
> + void *start = ip, *rel = NULL;
> + unsigned int opc, rel_sz = 0;
> + uint8_t b, d, rex = 0, osize = 4;
> +
> +#define OPC_TWOBYTE (1 << 8)
> +
> + /* Mutates IP, uses END. */
> +#define FETCH(ty) \
> + ({ \
> + ty _val; \
> + \
> + if ( (ip + sizeof(ty)) > end ) \
> + goto overrun; \
> + _val = *(ty *)ip; \
> + ip += sizeof(ty); \
> + _val; \
> + })
> +
> + for ( ;; ) /* Prefixes */
> + {
> + switch ( b = FETCH(uint8_t) )
> + {
> + case 0x26: /* ES override */
> + case 0x2e: /* CS override */
> + case 0x36: /* DS override */
> + case 0x3e: /* SS override */
> + case 0x64: /* FS override */
> + case 0x65: /* GS override */
> + case 0xf0: /* LOCK */
> + case 0xf2: /* REPNE */
> + case 0xf3: /* REP */
> + break;
> +
> + case 0x66: /* Operand size override */
> + osize = 2;
> + break;
> +
> + /* case 0x67: Address size override, not implemented */
> +
> + case 0x40 ... 0x4f: /* REX */
> + rex = b;
> + continue;
> +
> + default:
> + goto prefixes_done;
> + }
> + rex = 0; /* REX cancelled by subsequent legacy prefix. */
> + }
> + prefixes_done:
> +
> + if ( rex & REX_W )
> + osize = 8;
> +
> + /* Fetch the main opcode byte(s) */
> + if ( b == 0x0f )
> + {
> + b = FETCH(uint8_t);
> + opc = OPC_TWOBYTE | b;
> +
> + d = twobyte[b];
> + }
> + else
> + {
> + opc = b;
> + d = onebyte[b];
> + }
> +
> + if ( unlikely(!(d & Known)) )
> + goto unknown;
> +
> + if ( d & ModRM )
> + {
> + uint8_t modrm = FETCH(uint8_t);
> + uint8_t mod = modrm >> 6;
> + uint8_t reg = (modrm >> 3) & 7;
> + uint8_t rm = modrm & 7;
> +
> + /* ModRM/SIB decode */
> + if ( mod == 0 && rm == 5 ) /* RIP relative */
> + {
> + rel = ip;
> + rel_sz = 4;
> + FETCH(int32_t);
FETCH() here but ...
> + }
> + else if ( mod != 3 && rm == 4 ) /* SIB */
> + {
> + uint8_t sib = FETCH(uint8_t);
> + uint8_t base = sib & 7;
> +
> + if ( mod == 0 && base == 5 )
> + goto disp32;
... goto here?
> + }
> +
> + if ( mod == 1 ) /* disp8 */
> + FETCH(int8_t);
> + else if ( mod == 2 ) /* disp32 */
> + {
> + disp32:
> + FETCH(int32_t);
> + }
In several cases the FETCH()ed value isn't used. Compilers as well as Eclair
(and alike) are happy with that? And compilers also manage to eliminate the
memory accesses then?
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -835,4 +835,18 @@ static inline void x86_emul_reset_event(struct
> x86_emulate_ctxt *ctxt)
> ctxt->event = (struct x86_event){};
> }
>
> +/*
> + * x86_decode_lite(). Very minimal decoder for managing alternatives.
> + *
> + * @len is 0 on error, or nonzero on success. If the instruction has a
> + * relative field, @rel_sz is nonzero, and @rel points at the field.
> + */
> +typedef struct {
> + uint8_t len;
> + uint8_t rel_sz; /* bytes: 0, 1 or 4 */
Perhaps use bitfields in favor of fixed-width integers, seeing what
./CODING_STYLE says?
Jan
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |