Re: [RFC v1 5/7] x86/hvm: Move INSTR_* constants to hvm.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.05.2026 12:12, Ross Lagerwall wrote:
> On 5/19/26 10:49 AM, Jan Beulich wrote:
>> On 18.05.2026 15:14, Ross Lagerwall wrote:
>>> --- a/xen/arch/x86/include/asm/hvm/hvm.h
>>> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
>>> @@ -851,6 +851,35 @@ static inline void hvm_sync_pir_to_irr(struct vcpu *v)
>>>           alternative_vcall(hvm_funcs.sync_pir_to_irr, v);
>>>   }
>>>   
>>> +/*
>>> + * Encoding for svm_get_insn_len().  We take X86EMUL_OPC() for the main
>>> + * opcode, shifted left to make room for the ModRM byte.
>>
>> With all of this moved, the comment wants adjusting, at the very least by
>> putting "e.g." in front of the function name.
>>
>>> + * The Grp7 instructions have their ModRM byte expressed in octal for 
>>> easier
>>> + * cross referencing with the opcode extension table.
>>> + */
>>> +#define INSTR_ENC(opc, modrm) (((opc) << 8) | (modrm))
>>> +
>>> +#define INSTR_PAUSE      INSTR_ENC(X86EMUL_OPC_F3(0, 0x90), 0)
>>> +#define INSTR_INT3       INSTR_ENC(X86EMUL_OPC(   0, 0xcc), 0)
>>> +#define INSTR_ICEBP      INSTR_ENC(X86EMUL_OPC(   0, 0xf1), 0)
>>> +#define INSTR_HLT        INSTR_ENC(X86EMUL_OPC(   0, 0xf4), 0)
>>> +#define INSTR_XSETBV     INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0321) /* 
>>> octal-ok */
>>> +#define INSTR_VMRUN      INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0330) /* 
>>> octal-ok */
>>> +#define INSTR_VMCALL     INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0331) /* 
>>> octal-ok */
>>> +#define INSTR_VMLOAD     INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0332) /* 
>>> octal-ok */
>>> +#define INSTR_VMSAVE     INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0333) /* 
>>> octal-ok */
>>> +#define INSTR_STGI       INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0334) /* 
>>> octal-ok */
>>> +#define INSTR_CLGI       INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0335) /* 
>>> octal-ok */
>>> +#define INSTR_INVLPGA    INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0337) /* 
>>> octal-ok */
>>> +#define INSTR_RDTSCP     INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0371) /* 
>>> octal-ok */
>>> +#define INSTR_INVD       INSTR_ENC(X86EMUL_OPC(0x0f, 0x08), 0)
>>> +#define INSTR_WBINVD     INSTR_ENC(X86EMUL_OPC(0x0f, 0x09), 0)
>>> +#define INSTR_WRMSR      INSTR_ENC(X86EMUL_OPC(0x0f, 0x30), 0)
>>> +#define INSTR_RDTSC      INSTR_ENC(X86EMUL_OPC(0x0f, 0x31), 0)
>>> +#define INSTR_RDMSR      INSTR_ENC(X86EMUL_OPC(0x0f, 0x32), 0)
>>> +#define INSTR_CPUID      INSTR_ENC(X86EMUL_OPC(0x0f, 0xa2), 0)
>>> +
>>>   #else  /* CONFIG_HVM */
>>
>> I further wonder whether putting this in hvm.h is a good idea. Is there
>> anything wrong with using a brand new header, e.g. instr-enc.h?
> 
> No objection to that. I do wonder though if using the instruction encoding 
> like
> this is the best way of passing through the instruction to the fast path in
> hvm_emulate_one_ctxt() since I think in some cases the instruction encoding
> may not match the actual instruction that triggered the VMEXIT.

Do you have an example? If so, that would indeed be at risk of being misleading
or actively confusing. (Of course INSTR_VMCALL wants renaming, as was already
suggested.)

Jan