Re: [Xen-devel] [PATCH v3 2/3] x86/svm: Drop enum instruction_index and simplify svm_get_insn_len()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.12.18 at 12:37, <andrew.cooper3@xxxxxxxxxx> wrote:
> Passing a 32-bit integer index into an array with entries containing less than
> 32 bits of data is wasteful, and creates an unnecessary error condition of
> passing an out-of-range index.
> 
> The width of the X86EMUL_OPC() encoding is at most 24 bits, which leaves room
> for a modrm byte.

That's true for the 0x0f-prefix-space insns (and it's just 20 bits in that
case), but going this route we'd paint ourselves into a corner if we'd
ever have to add 0x0f38-, 0x0f3a-, or 0x8f0?-prefix-space insns.

Furthermore someone adjusting the encoding layout in x86_emulate.h
is very unlikely to notice breakage here until trying the resulting
binary - I strongly think some BUILD_BUG_ON() should be added to
make this apparent at build time.

> --- a/xen/include/asm-x86/hvm/svm/emulate.h
> +++ b/xen/include/asm-x86/hvm/svm/emulate.h
> @@ -19,33 +19,36 @@
>  #ifndef __ASM_X86_HVM_SVM_EMULATE_H__
>  #define __ASM_X86_HVM_SVM_EMULATE_H__
>  
> -/* Enumerate some standard instructions that we support */
> -enum instruction_index {
> -    INSTR_INVD,
> -    INSTR_WBINVD,
> -    INSTR_CPUID,
> -    INSTR_RDMSR,
> -    INSTR_WRMSR,
> -    INSTR_VMCALL,
> -    INSTR_HLT,
> -    INSTR_INT3,
> -    INSTR_RDTSC,
> -    INSTR_RDTSCP,
> -    INSTR_PAUSE,
> -    INSTR_XSETBV,
> -    INSTR_VMRUN,
> -    INSTR_VMLOAD,
> -    INSTR_VMSAVE,
> -    INSTR_STGI,
> -    INSTR_CLGI,
> -    INSTR_INVLPGA,
> -    INSTR_ICEBP,
> -    INSTR_MAX_COUNT /* Must be last - Number of instructions supported */
> -};
> +/*
> + * Encoding for svm_get_insn_len().  We take X86EMUL_OPC() for the main
> + * opcode, shifted left to make room for the ModRM byte.
> + */
> +#define INSTR_ENC(opc, modrm) (((unsigned int)(opc) << 8) | (modrm))

I can't seem to figure what good the cast does.

> +#define MODRM(mod, reg, rm) (((mod) << 6) | ((reg) << 3) | rm)

"rm" also wants to be parenthesized (or neither "mod" nor "reg" would
need to be).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel