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.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
xen/arch/x86/hvm/svm/svm.c | 22 ++++++----------------
xen/include/asm-x86/hvm/svm/vmcb.h | 26 ++++++++++++++++++++++++--
2 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 263ae03bfd..6c68bcee59 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2746,10 +2746,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
svm_vmexit_do_invalidate_cache(regs, exit_reason == VMEXIT_INVD);
break;
- case VMEXIT_TASK_SWITCH: {
- enum hvm_task_switch_reason reason;
- int32_t errcode = -1;
-
+ case VMEXIT_TASK_SWITCH:
/*
* All TASK_SWITCH intercepts have fault-like semantics. NRIP is
* never provided, even for instruction-induced task switches, but we
@@ -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);
break;
- }
case VMEXIT_CPUID:
if ( (insn_len = svm_get_insn_len(v, INSTR_CPUID)) == 0 )
diff --git a/xen/include/asm-x86/hvm/svm/vmcb.h b/xen/include/asm-x86/hvm/svm/vmcb.h
index fc67a88660..02b5e86b49 100644
--- 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 */
+ union {
+ struct {
+ uint16_t sel;
+ } task_switch;
+ } e1;
+ };
+ union {
+ u64 exitinfo2; /* offset 0x80 */
+ union {
+ struct {
+ uint32_t ec;
+ uint32_t :4;
+ bool iret:1;
+ uint32_t :1;
+ bool jmp:1;
+ uint32_t :5;
+ bool ev:1;
+ uint32_t :3;
+ bool rf:1;
+ } task_switch;
+ } e2;
+ };
intinfo_t exitintinfo; /* offset 0x88 */
u64 _np_enable; /* offset 0x90 - cleanbit 4 */
u64 res08[2];
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-develOn 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 04/12/2019 10:24, Jan Beulich wrote:
> 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"?
Written on their own like that perhaps, but the ei[12] versions are
equally ambiguous.
However, they are only ever used with vmcb-> in code, so there is no
issue with ambiguity.
> 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?
Net delta is:
diff --git a/xen/include/asm-x86/hvm/svm/vmcb.h
b/xen/include/asm-x86/hvm/svm/vmcb.h
index 02b5e86b49..864618ccf9 100644
--- a/xen/include/asm-x86/hvm/svm/vmcb.h
+++ b/xen/include/asm-x86/hvm/svm/vmcb.h
@@ -419,17 +419,15 @@ struct vmcb_struct {
u64 interrupt_shadow; /* offset 0x68 */
u64 exitcode; /* offset 0x70 */
union {
- u64 exitinfo1; /* offset 0x78 */
+ struct {
+ uint64_t exitinfo1; /* offset 0x78 */
+ uint64_t exitinfo2; /* offset 0x80 */
+ };
union {
struct {
uint16_t sel;
- } task_switch;
- } e1;
- };
- union {
- u64 exitinfo2; /* offset 0x80 */
- union {
- struct {
+ uint64_t :48;
+
uint32_t ec;
uint32_t :4;
bool iret:1;
@@ -440,7 +438,7 @@ struct vmcb_struct {
uint32_t :3;
bool rf:1;
} task_switch;
- } e2;
+ } ei;
};
intinfo_t exitintinfo; /* offset 0x88 */
u64 _np_enable; /* offset 0x90 - cleanbit 4 */
LGTM.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 04.12.2019 21:04, Andrew Cooper wrote:
> Net delta is:
>
> diff --git a/xen/include/asm-x86/hvm/svm/vmcb.h
> b/xen/include/asm-x86/hvm/svm/vmcb.h
> index 02b5e86b49..864618ccf9 100644
> --- a/xen/include/asm-x86/hvm/svm/vmcb.h
> +++ b/xen/include/asm-x86/hvm/svm/vmcb.h
> @@ -419,17 +419,15 @@ struct vmcb_struct {
> u64 interrupt_shadow; /* offset 0x68 */
> u64 exitcode; /* offset 0x70 */
> union {
> - u64 exitinfo1; /* offset 0x78 */
> + struct {
> + uint64_t exitinfo1; /* offset 0x78 */
> + uint64_t exitinfo2; /* offset 0x80 */
> + };
> union {
> struct {
> uint16_t sel;
> - } task_switch;
> - } e1;
> - };
> - union {
> - u64 exitinfo2; /* offset 0x80 */
> - union {
> - struct {
> + uint64_t :48;
> +
> uint32_t ec;
> uint32_t :4;
> bool iret:1;
> @@ -440,7 +438,7 @@ struct vmcb_struct {
> uint32_t :3;
> bool rf:1;
> } task_switch;
> - } e2;
> + } ei;
> };
> intinfo_t exitintinfo; /* offset 0x88 */
> u64 _np_enable; /* offset 0x90 - cleanbit 4 */
>
> LGTM.
And the result then
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2026 Red Hat, Inc.