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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 07/01/2019 10:30, Jan Beulich wrote:
>>>> On 31.12.18 at 12:37, <andrew.cooper3@citrix.com> 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.

We only need constants here for instructions which have intercepts.  I
doubt any SIMD instructions are going to enter that category, but we can
always reconsider the encoding scheme (probably to unsigned long) if
this becomes an issue.

> 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.

It turns out that BUILD_BUG_ON() doesn't work, because the macro
truncates at 32 bits due to types.

Using

#define INSTR_PAUSE       INSTR_ENC(X86EMUL_OPC_F3(0l, 0x90), 0)

i.e. using a long constant for for the ext field does end up triggering
-Woverflow when I artificially set a high bit inside X86EMUL_OPC() 
However, this isn't viable protection, as internal changes in
X86EMUL_OPC() would render it useless.


Given that a build time check is proving complicated, and that encoding
errors will be blindingly obvious in debug builds (as the diagnostics
will trigger and we'll hand #GP to the guest), and that it is unlikely
that we're going to vastly change the encoding scheme, I think it will
be fine to go without a build-time check.

~Andrew

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