Re: [PATCH 3/5] x86/emul: Add a pending_dbg field to x86_emulate_ctxt.retire

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/09/2023 3:24 pm, Jinoh Kang wrote:
> On 9/15/23 21:20, Jinoh Kang wrote:
>> On 9/13/23 08:21, Andrew Cooper wrote:
>>> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h 
>>> b/xen/arch/x86/x86_emulate/x86_emulate.h
>>> index 698750267a90..f0e74d23c378 100644
>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
>>> @@ -588,15 +588,26 @@ struct x86_emulate_ctxt
>>>      /* Canonical opcode (see below) (valid only on X86EMUL_OKAY). */
>>>      unsigned int opcode;
>>>  
>>> -    /* Retirement state, set by the emulator (valid only on X86EMUL_OKAY). 
>>> */
>>> +    /*
>>> +     * Retirement state, set by the emulator (valid only on 
>>> X86EMUL_OKAY/DONE).
>>> +     *
>>> +     * TODO: all this state should be input/output from the VMCS 
>>> PENDING_DBG,
>>> +     * INTERRUPTIBILITY and ACTIVITIY fields.
>>> +     */
>>>      union {
>>> -        uint8_t raw;
>>> +        unsigned long raw;
>> Minor nit: this should be uint64_t for clarity.  Otherwise, it's not at all
>> clear that the raw field covers the entire union, unless you remind myself
>> that Xen does not support 32-bit host.
> you remind yourself*.  What a weird typo to make :-(

For better or worse, this is form preferred by the Xen coding style.

We deleted the 32bit build of the Xen more than a decade ago, and have
been 64bit-only ever since.

~Andrew