Re: [PATCH v4 2/5] tests/x86: Introduce a userspace test harness for x86_decode_lite()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.10.2026 23:05, Andrew Cooper wrote:
> All the interesting behaviour is in insns.S.
> 
> There are 4 interesting cases; "not an instruction we tolerate", and one we do
> tolerate, split by no relation, disp8 or disp32.  The DECL()/END() macros

DYM "relocation" here?

> --- /dev/null
> +++ b/tools/tests/x86-decode-lite/Makefile
> @@ -0,0 +1,56 @@
> +XEN_ROOT = $(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +TARGET :=
> +
> +# Clang IAS doesn't like the convenience macros we use
> +$(call cc-option-add,CFLAGS,CC,-no-integrated-as)
> +
> +# Binutils around 2.30 have mutually exclusive expectations of instruction
> +# suffix validities compared to later versions.  Among the distro we test,
> +# this only excludes Ubuntu 18.04.

This is liable to go stale; 18.04's lifetime can't be all that much anymore.
I'd suggest to keep just the first sentence.

> --- /dev/null
> +++ b/tools/tests/x86-decode-lite/insns.S
> @@ -0,0 +1,711 @@
> [...]
> +sib:
> +        /* Mod=0, Reg=0, RM=4, SIB S=3, I=0, B {0..f} */
> +        _ add %al, (%rax, %rax, 8)
> +        _ add %al, (%rcx, %rax, 8)
> +        _ add %al, (%rdx, %rax, 8)
> +        _ add %al, (%rbx, %rax, 8)
> +        _ add %al, (%rsp, %rax, 8)
> +        _ add %al, (    , %rax, 8) /* "none", %rbp encoded with mod=1/2 */
> +        _ add %al, (%rsi, %rax, 8)
> +        _ add %al, (%rdi, %rax, 8)
> +        _ add %al, (%r8,  %rax, 8)
> +        _ add %al, (%r9,  %rax, 8)
> +        _ add %al, (%r10, %rax, 8)
> +        _ add %al, (%r11, %rax, 8)
> +        _ add %al, (%r12, %rax, 8)
> +        _ rex.b add %al,(,%rax, 8) /* "none", %r13 encoded with mod=1/2 */
> +        _ add %al, (%r14, %rax, 8)
> +        _ add %al, (%r15, %rax, 8)
> +
> +        /* Mod=1, Reg=0, RM=4, SIB S=3, I=0, B {0..f} */
> +        _ add %al, 0x01(%rax, %rax, 8)
> +        _ add %al, 0x01(%rcx, %rax, 8)
> +        _ add %al, 0x01(%rdx, %rax, 8)
> +        _ add %al, 0x01(%rbx, %rax, 8)
> +        _ add %al, 0x01(%rsp, %rax, 8)
> +        _ add %al, 0x01(%rbp, %rax, 8)
> +        _ add %al, 0x01(%rsi, %rax, 8)
> +        _ add %al, 0x01(%rdi, %rax, 8)
> +        _ add %al, 0x01(%r8,  %rax, 8)
> +        _ add %al, 0x01(%r9,  %rax, 8)
> +        _ add %al, 0x01(%r10, %rax, 8)
> +        _ add %al, 0x01(%r11, %rax, 8)
> +        _ add %al, 0x01(%r12, %rax, 8)
> +        _ add %al, 0x01(%r13, %rax, 8)
> +        _ add %al, 0x01(%r14, %rax, 8)
> +        _ add %al, 0x01(%r15, %rax, 8)
> +
> +        /* Mod=2, Reg=0, RM=4, SIB S=3, I=0, B {0..f} */
> +        _ add %al, 0x7f000001(%rax, %rax, 8)
> +        _ add %al, 0x7f000001(%rcx, %rax, 8)
> +        _ add %al, 0x7f000001(%rdx, %rax, 8)
> +        _ add %al, 0x7f000001(%rbx, %rax, 8)
> +        _ add %al, 0x7f000001(%rsp, %rax, 8)
> +        _ add %al, 0x7f000001(%rbp, %rax, 8)
> +        _ add %al, 0x7f000001(%rsi, %rax, 8)
> +        _ add %al, 0x7f000001(%rdi, %rax, 8)
> +        _ add %al, 0x7f000001(%r8,  %rax, 8)
> +        _ add %al, 0x7f000001(%r9,  %rax, 8)
> +        _ add %al, 0x7f000001(%r10, %rax, 8)
> +        _ add %al, 0x7f000001(%r11, %rax, 8)
> +        _ add %al, 0x7f000001(%r12, %rax, 8)
> +        _ add %al, 0x7f000001(%r13, %rax, 8)
> +        _ add %al, 0x7f000001(%r14, %rax, 8)
> +        _ add %al, 0x7f000001(%r15, %rax, 8)
> +
> +        /* Mod=0, Reg=0, RM=4, SIB S=3, I=4, B {0..f} */
> +        _ add %al, (%rax, %riz, 8)
> +        _ add %al, (%rcx, %riz, 8)
> +        _ add %al, (%rdx, %riz, 8)
> +        _ add %al, (%rbx, %riz, 8)
> +        _ add %al, (%rsp, %riz, 8)
> +        _ add %al, (    , %riz, 8) /* %rbp encoded with mod=1/2 */
> +        _ add %al, (%rsi, %riz, 8)
> +        _ add %al, (%rdi, %riz, 8)
> +        _ add %al, (%r8,  %riz, 8)
> +        _ add %al, (%r9,  %riz, 8)
> +        _ add %al, (%r10, %riz, 8)
> +        _ add %al, (%r11, %riz, 8)
> +        _ add %al, (%r12, %riz, 8)
> +        _ rex.b add %al,(,%riz, 8) /* %r13 encoded with mod=1/2 */
> +        _ add %al, (%r14, %riz, 8)
> +        _ add %al, (%r15, %riz, 8)
> +
> +        /* Mod=1, Reg=0, RM=4, SIB S=3, I=4, B {0..f} */
> +        _ add %al, 0x01(%rax, %riz, 8)
> +        _ add %al, 0x01(%rcx, %riz, 8)
> +        _ add %al, 0x01(%rdx, %riz, 8)
> +        _ add %al, 0x01(%rbx, %riz, 8)
> +        _ add %al, 0x01(%rsp, %riz, 8)
> +        _ add %al, 0x01(%rbp, %riz, 8)
> +        _ add %al, 0x01(%rsi, %riz, 8)
> +        _ add %al, 0x01(%rdi, %riz, 8)
> +        _ add %al, 0x01(%r8,  %riz, 8)
> +        _ add %al, 0x01(%r9,  %riz, 8)
> +        _ add %al, 0x01(%r10, %riz, 8)
> +        _ add %al, 0x01(%r11, %riz, 8)
> +        _ add %al, 0x01(%r12, %riz, 8)
> +        _ add %al, 0x01(%r13, %riz, 8)
> +        _ add %al, 0x01(%r14, %riz, 8)
> +        _ add %al, 0x01(%r15, %riz, 8)
> +
> +        /* Mod=2, Reg=0, RM=4, SIB S=3, I=4, B {0..f} */
> +        _ add %al, 0x7f000001(%rax, %riz, 8)
> +        _ add %al, 0x7f000001(%rcx, %riz, 8)
> +        _ add %al, 0x7f000001(%rdx, %riz, 8)
> +        _ add %al, 0x7f000001(%rbx, %riz, 8)
> +        _ add %al, 0x7f000001(%rsp, %riz, 8)
> +        _ add %al, 0x7f000001(%rbp, %riz, 8)
> +        _ add %al, 0x7f000001(%rsi, %riz, 8)
> +        _ add %al, 0x7f000001(%rdi, %riz, 8)
> +        _ add %al, 0x7f000001(%r8,  %riz, 8)
> +        _ add %al, 0x7f000001(%r9,  %riz, 8)
> +        _ add %al, 0x7f000001(%r10, %riz, 8)
> +        _ add %al, 0x7f000001(%r11, %riz, 8)
> +        _ add %al, 0x7f000001(%r12, %riz, 8)
> +        _ add %al, 0x7f000001(%r13, %riz, 8)
> +        _ add %al, 0x7f000001(%r14, %riz, 8)
> +        _ add %al, 0x7f000001(%r15, %riz, 8)
> +
> +
> +.macro alu_ops op

Nit: Can we avoid double blank lines, please?

> +onebyte_row_6x:
> +        /*pusha,popa,bound --> not supported */
> +        _ movsxd (%rax), %eax
> +        _ movslq (%rax), %rax

What about the (odd) 16-bit form? Decoding wise it's identical between AMD
and Intel.

> +        _ fs nop
> +        _ gs nop
> +        _ data16 nop
> +        /* addr32 --> not supported */
> +        _ pushq $0x7f000001
> +        _ pushw $0x7f01
> +        _ rex.w pushq $0x7f000001

Maybe also

        _ data16 rex.w pushq $0x7f000001

?

> +        _ imul $0x7f01, %ax, %ax
> +        _ imul $0x7f000001, %eax, %eax
> +        _ imul $0x7f000001, %rax, %rax
> +        _ pushq $0
> +        _ pushw $0
> +        _ rex.w pushq $0

Same here then.

> +onebyte_row_Fx:
> +        _ lock addb $0, (%rax)
> +        n "icebp" .byte 0xf1
> +        _ repne nop
> +        _ repe nop
> +        _ hlt
> +        _ cmc
> +        _ test $0, %cl /* Grp3, /0 has extra Imm{8,} */
> +        _ not %cl
> +        _ test $0x7f01, %cx
> +        _ not %cx
> +        _ test $0x7f000001, %ecx
> +        _ not %ecx
> +        _ test $0x7f000001, %rcx
> +        _ not %rcx
> +        _ clc
> +        _ stc
> +        _ cli
> +        _ sti
> +        _ cld
> +        _ std
> +        _ inc %cl /* Grp4 */
> +        _ dec %cl
> +        _ inc %ecx /* Grp5 */
> +        _ dec %ecx

Any reason you use register operands up to here, but then ...

> +        _ call *(%rax)

... a memory one here and ...

> +        _ lcall *(%rax)
> +        _ jmp *(%rax)

... here?

> +        _ ljmp *(%rax)
> +        _ push (%rax)

(I realize the assembler can't easily be told to encode this with a ModR/M
byte when used with a register operand.)

Again preferably with the above addressed (but explicitly also with some / all
left as is, except perhaps the double blank line one):
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

> +twobyte_row_2x:
> +        _ mov %cr0, %rax
> +        _ mov %dr0, %rax
> +        _ mov %rax, %cr0
> +        _ mov %rax, %dr0

I guess as we don't mean to use odd encodings of these (ModR/M.mod != 3),
there's no value in trying to test (and support) such.

> +twobyte_row_4x:
> +        _ cmovo (%rax), %eax
> +        _ cmovg (%rax), %eax
> +
> +/* twobyte_row_8x: --> Jcc disp32 */

Hmm, noticing only here: You do not foresee a need to support VMREAD/VMWRITE?

Jan


F