Re: [PATCH v9 09/10] x86: use / "support" UDB

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

On 24/11/2025 3:01 pm, Jan Beulich wrote:
> With opcode D6h now firmly reserved as another #UD-raising one in 64-bit
> mode, use that instead of the two-byte UD2 for bug frame marking.
>
> While there also make the respective adjustment to the emulator.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Should we also switch {svm,vmx}_init_hypercall_page()?

The hypercall pages are 32/64-agnostic right now, and used by 32bit
guests.  UDB isn't safe to use in those cases.

> Furthermore x86_64/kexec_reloc.S also has two uses. Question is whether
> "tailcall" is being open-coded there, or whether that's deliberately not
> using the macro we have.

The code in kexec_reloc.S long predates the tailcall macro.  Also I now
regret calling it tailcall; terminalcall might be more accurate but it's
overly long.

Very counter-intuitively, Linux has plain KEXEC which is phrase as
"call", and KEXEC_JUMP which the ability for the invoked kernel to return.

For the regular cases, Linux invokes the new kernel by spilling a
register to the stack, and RET-ing to it.  For KEXEC_JUMP, it does call
the target kernel, after some games to set up a stack in the target image.

Either way, we can convert these two to plain JMPs.  One of the ud2's in
particular cannot be converted to UDB because of being in 32bit code.

I can do a patch if you'd like?

>
> One of the table entries in stub_selftest() uses UD1, yet not in quite
> an appropriate way: The 0x90 following it (presumably meant to be a NOP)
> really is a ModR/M byte, requiring a displacement to follow. Wouldn't we
> better adjust that (e.g. using 0xcc instead)?

Oh lovely...  That was my mistake in 2eb1132f7963, where I was
converting what looked to be a double ret.

Can't we use 0x00 so it doesn't look like an int3 either, and update the
comment to make it explicit that there's a ModRM byte?

> ---
> v9: New.
>
> --- a/xen/arch/x86/include/asm/bug.h
> +++ b/xen/arch/x86/include/asm/bug.h
> @@ -21,7 +21,7 @@
>  
>  #ifndef __ASSEMBLY__
>  
> -#define BUG_INSTR       "ud2"
> +#define BUG_INSTR       ".byte 0xd6" /* UDB */
>  #define BUG_ASM_CONST   "c"
>  
>  #else  /* !__ASSEMBLY__ */
> @@ -37,7 +37,7 @@
>          .error "Invalid BUGFRAME index"
>      .endif
>  
> -    .L\@ud: ud2a
> +    .L\@ud: .byte 0xd6 /* UDB */

add/remove: 0/0 grow/shrink: 1/520 up/down: 284/-1024 (-740)
Function                                     old     new   delta
x86_emulate                               144961  145245    +284
zap_low_mappings                              82      81      -1
xstate_set_init                              298     297      -1
...
paging_mark_dirty                            280     261     -19
csched_schedule                             3711    3691     -20
vmx_create_vmcs                             3740    3663     -77


I think this says quite a lot about how bad the vmread/write helpers
really are.

> --- a/xen/arch/x86/x86_emulate/decode.c
> +++ b/xen/arch/x86/x86_emulate/decode.c
> @@ -651,7 +651,7 @@ decode_onebyte(struct x86_emulate_state
>      case 0xce: /* into */
>      case 0xd4: /* aam */
>      case 0xd5: /* aad */
> -    case 0xd6: /* salc */
> +        /* 0xd6 (salc) omitted here, for #UD to be raised in 64-bit mode. */
>          s->not_64bit = true;
>          break;

Why does the not_64bit logic not suffice?  #UD is exactly what we do:

    generate_exception_if(state->not_64bit && mode_64bit(), X86_EXC_UD);


~Andrew