* [PATCH] x86/vmx: Partially revert "x86/vmx: implement Notify VM Exit"
@ 2023-01-18 19:36 Andrew Cooper
2023-01-19 5:52 ` Tian, Kevin
0 siblings, 1 reply; 2+ messages in thread
From: Andrew Cooper @ 2023-01-18 19:36 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu, Kevin Tian
The original patch tried to do two things - implement VMNotify, and
re-optimise VT-x to not intercept #DB/#AC by default.
The second part is buggy in multiple ways. Both GDBSX and Introspection need
to conditionally intercept #DB, which was not accounted for. Also, #DB
interception has nothing at all to do with cpu_has_monitor_trap_flag.
Revert the second half, leaving #DB/#AC intercepted unilaterally, but with
VMNotify active by default when available.
Fixes: 573279cde1c4 ("x86/vmx: implement Notify VM Exit")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Kevin Tian <kevin.tian@intel.com>
#DB/#AC are not fastpaths in the slightest. This perf optimisation can be
reworked at some point later with rather more care and testing.
It's *really* not as urgent as getting VMNotify active by default.
---
xen/arch/x86/hvm/vmx/vmcs.c | 11 ++---------
xen/arch/x86/hvm/vmx/vmx.c | 16 ++--------------
2 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 8992f4e0aeb2..7d8bfeb53982 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1296,17 +1296,10 @@ static int construct_vmcs(struct vcpu *v)
v->arch.hvm.vmx.exception_bitmap = HVM_TRAP_MASK
| (paging_mode_hap(d) ? 0 : (1U << TRAP_page_fault))
| (v->arch.fully_eager_fpu ? 0 : (1U << TRAP_no_device));
+
if ( cpu_has_vmx_notify_vm_exiting )
- {
__vmwrite(NOTIFY_WINDOW, vm_notify_window);
- /*
- * Disable #AC and #DB interception: by using VM Notify Xen is
- * guaranteed to get a VM exit even if the guest manages to lock the
- * CPU.
- */
- v->arch.hvm.vmx.exception_bitmap &= ~((1U << TRAP_debug) |
- (1U << TRAP_alignment_check));
- }
+
vmx_update_exception_bitmap(v);
v->arch.hvm.guest_cr[0] = X86_CR0_PE | X86_CR0_ET;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 15a07933ee5d..2e2ab0ac0e26 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1564,19 +1564,10 @@ static void cf_check vmx_update_host_cr3(struct vcpu *v)
void vmx_update_debug_state(struct vcpu *v)
{
- unsigned int mask = 1u << TRAP_int3;
-
- if ( !cpu_has_monitor_trap_flag && cpu_has_vmx_notify_vm_exiting )
- /*
- * Only allow toggling TRAP_debug if notify VM exit is enabled, as
- * unconditionally setting TRAP_debug is part of the XSA-156 fix.
- */
- mask |= 1u << TRAP_debug;
-
if ( v->arch.hvm.debug_state_latch )
- v->arch.hvm.vmx.exception_bitmap |= mask;
+ v->arch.hvm.vmx.exception_bitmap |= 1U << TRAP_int3;
else
- v->arch.hvm.vmx.exception_bitmap &= ~mask;
+ v->arch.hvm.vmx.exception_bitmap &= ~(1U << TRAP_int3);
vmx_vmcs_enter(v);
vmx_update_exception_bitmap(v);
@@ -4192,9 +4183,6 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
switch ( vector )
{
case TRAP_debug:
- if ( cpu_has_monitor_trap_flag && cpu_has_vmx_notify_vm_exiting )
- goto exit_and_crash;
-
/*
* Updates DR6 where debugger can peek (See 3B 23.2.1,
* Table 23-1, "Exit Qualification for Debug Exceptions").
--
2.11.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* RE: [PATCH] x86/vmx: Partially revert "x86/vmx: implement Notify VM Exit"
2023-01-18 19:36 [PATCH] x86/vmx: Partially revert "x86/vmx: implement Notify VM Exit" Andrew Cooper
@ 2023-01-19 5:52 ` Tian, Kevin
0 siblings, 0 replies; 2+ messages in thread
From: Tian, Kevin @ 2023-01-19 5:52 UTC (permalink / raw)
To: andrew.cooper3, Xen-devel
Cc: andrew.cooper3, Beulich, Jan, Pau Monné, Roger, Wei Liu
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> Sent: Thursday, January 19, 2023 3:37 AM
>
> The original patch tried to do two things - implement VMNotify, and
> re-optimise VT-x to not intercept #DB/#AC by default.
>
> The second part is buggy in multiple ways. Both GDBSX and Introspection
> need
> to conditionally intercept #DB, which was not accounted for. Also, #DB
> interception has nothing at all to do with cpu_has_monitor_trap_flag.
>
> Revert the second half, leaving #DB/#AC intercepted unilaterally, but with
> VMNotify active by default when available.
>
> Fixes: 573279cde1c4 ("x86/vmx: implement Notify VM Exit")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-01-19 5:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-18 19:36 [PATCH] x86/vmx: Partially revert "x86/vmx: implement Notify VM Exit" Andrew Cooper
2023-01-19 5:52 ` Tian, Kevin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).