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