[PATCH v2 2/3] x86/vmx: Fix IRQ handling for EXIT_REASON_INIT

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Reima ISHII" <ishiir@g.ecc.u-tokyo.ac.jp>,
	"Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Jun Nakajima" <jun.nakajima@intel.com>,
	"Kevin Tian" <kevin.tian@intel.com>,
	"Tamas K Lengyel" <tamas@tklengyel.com>,
	"Takahiro Shinagawa" <shina@ecc.u-tokyo.ac.jp>,
	"George Dunlap" <george.dunlap@citrix.com>
Subject: [PATCH v2 2/3] x86/vmx: Fix IRQ handling for EXIT_REASON_INIT
Date: Thu, 11 Jan 2024 23:13:22 +0000	[thread overview]
Message-ID: <20240111231323.4043461-3-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20240111231323.4043461-1-andrew.cooper3@citrix.com>

When receiving an INIT, a prior bugfix tried to ignore the INIT and continue
onwards.

Unfortunately it's not safe to return at that point in vmx_vmexit_handler().
Just out of context in the first hunk is a local_irqs_enabled() which is
depended-upon by the return-to-guest path, causing the following checklock
failure in debug builds:

  (XEN) Error: INIT received - ignoring
  (XEN) CHECKLOCK FAILURE: prev irqsafe: 0, curr irqsafe 1
  (XEN) Xen BUG at common/spinlock.c:132
  (XEN) ----[ Xen-4.19-unstable  x86_64  debug=y  Tainted:     H  ]----
  ...
  (XEN) Xen call trace:
  (XEN)    [<ffff82d040238e10>] R check_lock+0xcd/0xe1
  (XEN)    [<ffff82d040238fe3>] F _spin_lock+0x1b/0x60
  (XEN)    [<ffff82d0402ed6a8>] F pt_update_irq+0x32/0x3bb
  (XEN)    [<ffff82d0402b9632>] F vmx_intr_assist+0x3b/0x51d
  (XEN)    [<ffff82d040206447>] F vmx_asm_vmexit_handler+0xf7/0x210

Luckily, this is benign in release builds.  Accidentally having IRQs disabled
when trying to take an IRQs-on lock isn't a deadlock-vulnerable pattern.

Drop the problematic early return.  In hindsight, it's wrong to skip other
normal VMExit steps.

Fixes: b1f11273d5a7 ("x86/vmx: Don't spuriously crash the domain when INIT is received")
Reported-by: Reima ISHII <ishiir@g.ecc.u-tokyo.ac.jp>
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: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Tamas K Lengyel <tamas@tklengyel.com>
CC: Reima Ishii <ishiir@g.ecc.u-tokyo.ac.jp>
CC: Takahiro Shinagawa <shina@ecc.u-tokyo.ac.jp>
CC: George Dunlap <george.dunlap@citrix.com>

With this patch in place, the INIT is ignored and the guest continues:

  (XEN) HVM1 restore: CPU 0
  (d1) --- Xen Test Framework ---
  (d1) Environment: HVM 64bit (Long mode 4 levels)
  (XEN) Error: INIT received - ignoring
  (d1) Test result: SUCCESS

v2:
 * Leave the printk() early, and skip in the main block.
---
 xen/arch/x86/hvm/vmx/vmx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 829c27f1fa33..a85394232a23 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4090,7 +4090,7 @@ void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs)
 
     case EXIT_REASON_INIT:
         printk(XENLOG_ERR "Error: INIT received - ignoring\n");
-        return; /* Renter the guest without further processing */
+        break;
     }
 
     /* Now enable interrupts so it's safe to take locks. */
@@ -4692,6 +4692,7 @@ void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs)
         break;
 
     case EXIT_REASON_EXTERNAL_INTERRUPT:  /* Handled earlier */
+    case EXIT_REASON_INIT:                /* Handled earlier */
     case EXIT_REASON_TPR_BELOW_THRESHOLD: /* Handled later in vmx_intr_assist() */
     case EXIT_REASON_BUS_LOCK:            /* Nothing to do (rate-limit only) */
         break;
-- 
2.30.2


