Re: [Xen-devel] [PATCH 4/4] x86/svm: Use named (bit)fields for task switch exit info

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.12.2019 10:43, Andrew Cooper wrote:
> Introduce vmcb.e1.* and vmcb.e2.* to provide names to fields in exitinfo{1,2}
> respectively.  Implement the task switch names for now, and clean up the
> TASK_SWITCH handler.

"e1" and "e2" look overly short - and hence possibly ambiguous -
to me. Make them perhaps "ei1" and "ei2"? Furthermore, seeing ...

> @@ -2795,19 +2792,12 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>          if ( insn_len < 0 && (insn_len = svm_get_task_switch_insn_len()) == 
> 0 )
>              goto crash_or_fault;
>  
> -        if ( (vmcb->exitinfo2 >> 36) & 1 )
> -            reason = TSW_iret;
> -        else if ( (vmcb->exitinfo2 >> 38) & 1 )
> -            reason = TSW_jmp;
> -        else
> -            reason = TSW_call_or_int;
> -        if ( (vmcb->exitinfo2 >> 44) & 1 )
> -            errcode = (uint32_t)vmcb->exitinfo2;
> -
> -        hvm_task_switch(vmcb->exitinfo1, reason, errcode, insn_len,
> -                        (vmcb->exitinfo2 & (1ul << 48)) ? X86_EFLAGS_RF : 0);
> +        hvm_task_switch(vmcb->e1.task_switch.sel,
> +                        vmcb->e2.task_switch.iret ? TSW_iret :
> +                        vmcb->e2.task_switch.jmp  ? TSW_jmp : 
> TSW_call_or_int,
> +                        vmcb->e2.task_switch.ev ? vmcb->e2.task_switch.ec : 
> -1,
> +                        insn_len, vmcb->e2.task_switch.rf ? X86_EFLAGS_RF : 
> 0);

... this, wouldn't it make sense to simply have "ei" covering both
parts, no longer making it a requirement to use (and hence look up)
the numeric suffixes at use sites?

> --- a/xen/include/asm-x86/hvm/svm/vmcb.h
> +++ b/xen/include/asm-x86/hvm/svm/vmcb.h
> @@ -418,8 +418,30 @@ struct vmcb_struct {
>      vintr_t _vintr;             /* offset 0x60 - cleanbit 3 */
>      u64 interrupt_shadow;       /* offset 0x68 */
>      u64 exitcode;               /* offset 0x70 */
> -    u64 exitinfo1;              /* offset 0x78 */
> -    u64 exitinfo2;              /* offset 0x80 */
> +    union {
> +        u64 exitinfo1;          /* offset 0x78 */

uint64_t (also below)?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel