[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] x86/flushtlb: remove flush_area check on system state


  • To: xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
  • Date: Mon, 16 May 2022 16:31:16 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=3z7cp0g1R/YVtPHI0JJ7+lHogu2k/5+yi01WV1RPyZY=; b=FuneRbeUOnSlYZWxS5wl4JzPTLRuB8OclW03jS+OqDQv3qRSOC1aMk7k80QduKv6zpdFwRgOEeD7iweUigfHPcYEK2PjfvZvDaTUNBIge1hPdCbn60mE81JgICZ83FUOQx6xIMQuoGcSZg7pG3WF/G3TySoB4tsm0WTxW0pTgEPFNtShvuMm5G9x5VtuLWyevXeBTgWwC0JfMdBPA6CECWhnUwZw0oAMex27hYhYKIohPUZopw/PafNTZx2vQEsSzHEhh9d9eywHJKj7mMtj1I1yHp9Rb3zc0M70r6NHWfTFY/1U6Kn5LP06D6nLVz+L2taDUtqmVbDltIFsiW5Pgw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=U5R/AhEzvjGD/pTTvXqv1fufEiOpUxny6N1jPjbbdec1fTWeaFNwN//ePBIaGU8pnwWqkzNGC9yjK20iVzqawsfULJ642JEHf/0boxNa4boVTZiXbnzd2kN8gUnjkOvsW8sfEAjH5b/mjnuA1QqutMw8B3QTl2cu4IIjGudP3zxXoJh4ly3x3WYSmfMuszau0bUJ3N9zivJ3470U+81fCTWpb0MdRfaRcWuCOknZPs0w0P+pL8x3MgIzir9q7KiYE+6gZLM+VThvVB3QIKIdi1dn5LkVvkPhJsQDQBHLmj+N2/ypzpkuexf0y1SLfBEvO0ly8GbZEOz8By4WRnrkeg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Mon, 16 May 2022 14:33:31 +0000
  • Ironport-data: A9a23:ON5eO67x15flxBiQOBUC4QxRtE7GchMFZxGqfqrLsTDasY5as4F+v mROD2GPPveJa2DyeNxzb9/i/EsHvMXWxoRqGVFkrH8yHi5G8cbLO4+Ufxz6V8+wwmwvb67FA +E2MISowBUcFyeEzvuVGuG96yE6j8lkf5KkYAL+EnkZqTRMFWFw0HqPp8Zj2tQy2YXgXVvU0 T/Pi5a31GGNimYc3l08s8pvmDs31BglkGpF1rCWTakjUG72zxH5PrpGTU2CByKQrr1vNvy7X 47+IISRpQs1yfuP5uSNyd4XemVSKlLb0JPnZnB+A8BOiTAazsA+PzpS2FPxpi67hh3Q9+2dx umhurS7TxskMvfyk9hCUhtAOn9PZbxXxq/+dC3XXcy7lyUqclPK6tA3VAQTAtdd/ex6R2ZT6 fYfNTYBKAiZgP67y666Te8qgdk/KM7sP8UUvXQIITPxVK56B8ycBfiXo4YAgF/chegXdRraT 9AeZjd1KgzJfjVEO0sNCYJ4l+Ct7pX6W2IA9wLI+PRqi4TV5E9owr7jL4PzQ5+1e9wPlBy6v Vv4znusV3n2M/Tak1Jp6EmEluLJ2C/2Ro8WPLm57eJxxk2ewHQJDx8bXkf9puO24ma8Ud9CL 00f+gI1sLM/skesS7HVQBmQsHOC+BkGVLJt//YS7QiMzu/R/FyfD21dFjpZMoV+6IkxWCAg0 UKPk5XxHztzvbaJSHWbsLCJsTe1PitTJmgHDcMZcTY4DxDYiNlbpnryohxLSfLs5jEpMVkcG wy3kRU=
  • Ironport-hdrordr: A9a23:GIW2G66T+KC4Ym9F6APXwAzXdLJyesId70hD6qm+c3Nom7+j5q eTdZUgpH3JYVMqM03I9urgBEDtex7hHP1OkOos1NWZPDUO0VHAROtfBODZrQEIbheOktK1u5 0NT0BaYOeAdCkD/L2KmnjELz4CqOP3j5xBvo/lvgtQpHlRGsRdBzUQMHfkLqVBLDM2dKbQI/ Knl7p6jivlcm8QasSmAj0ARebCqrTw5fTbXSI=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Booting with Shadow Stacks leads to the following assert on a debug
hypervisor:

(XEN) [   11.625166] Assertion 'local_irq_is_enabled()' failed at 
arch/x86/smp.c:265
(XEN) [   11.629410] ----[ Xen-4.17.0-10.24-d  x86_64  debug=y  Not tainted 
]----
(XEN) [   11.633679] CPU:    0
(XEN) [   11.637834] RIP:    e008:[<ffff82d040345300>] 
flush_area_mask+0x40/0x13e
[...]
(XEN) [   11.806158] Xen call trace:
(XEN) [   11.811255]    [<ffff82d040345300>] R flush_area_mask+0x40/0x13e
(XEN) [   11.816459]    [<ffff82d040338a40>] F modify_xen_mappings+0xc5/0x958
(XEN) [   11.821689]    [<ffff82d0404474f9>] F 
arch/x86/alternative.c#_alternative_instructions+0xb7/0xb9
(XEN) [   11.827053]    [<ffff82d0404476cc>] F alternative_branches+0xf/0x12
(XEN) [   11.832416]    [<ffff82d04044e37d>] F __start_xen+0x1ef4/0x2776
(XEN) [   11.837809]    [<ffff82d040203344>] F __high_start+0x94/0xa0


This is due to SYS_STATE_smp_boot being set before calling
alternative_branches(), and the flush in modify_xen_mappings() then
using flush_area_all() with interrupts disabled.  Note that
alternative_branches() is called before APs are started, so the flush
must be a local one (and indeed the cpumask passed to
flush_area_mask() just contains one CPU).

Take the opportunity to simplify a bit the logic and make flush_area()
an alias for flush_area_mask(&cpu_online_map...), taking into account
that cpu_online_map just contains the BSP before APs are started.
This requires widening the assert in flush_area_mask() to allow
being called with interrupts disabled as long as it's strictly a local
only flush.

The overall result is that a conditional can be removed from
flush_area().

Fixes: (78e072bc37 'x86/mm: avoid inadvertently degrading a TLB flush to local 
only')
Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/arch/x86/include/asm/flushtlb.h | 3 ++-
 xen/arch/x86/mm.c                   | 8 --------
 xen/arch/x86/smp.c                  | 3 ++-
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/include/asm/flushtlb.h 
b/xen/arch/x86/include/asm/flushtlb.h
index 18777f1d4c..a97b3a2327 100644
--- a/xen/arch/x86/include/asm/flushtlb.h
+++ b/xen/arch/x86/include/asm/flushtlb.h
@@ -146,7 +146,8 @@ void flush_area_mask(const cpumask_t *, const void *va, 
unsigned int flags);
 #define flush_mask(mask, flags) flush_area_mask(mask, NULL, flags)
 
 /* Flush all CPUs' TLBs/caches */
-#define flush_area_all(va, flags) flush_area_mask(&cpu_online_map, va, flags)
+#define flush_area(va, flags) \
+    flush_area_mask(&cpu_online_map, (const void *)(va), flags)
 #define flush_all(flags) flush_mask(&cpu_online_map, flags)
 
 /* Flush local TLBs */
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 67c0427963..45235c5aa6 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5066,14 +5066,6 @@ l1_pgentry_t *virt_to_xen_l1e(unsigned long v)
 #define l1f_to_lNf(f) (((f) & _PAGE_PRESENT) ? ((f) |  _PAGE_PSE) : (f))
 #define lNf_to_l1f(f) (((f) & _PAGE_PRESENT) ? ((f) & ~_PAGE_PSE) : (f))
 
-/*
- * map_pages_to_xen() can be called early in boot before any other
- * CPUs are online. Use flush_area_local() in this case.
- */
-#define flush_area(v,f) (system_state < SYS_STATE_smp_boot ?    \
-                         flush_area_local((const void *)v, f) : \
-                         flush_area_all((const void *)v, f))
-
 #define L3T_INIT(page) (page) = ZERO_BLOCK_PTR
 
 #define L3T_LOCK(page)        \
diff --git a/xen/arch/x86/smp.c b/xen/arch/x86/smp.c
index 0a02086966..2f4e98cec9 100644
--- a/xen/arch/x86/smp.c
+++ b/xen/arch/x86/smp.c
@@ -262,7 +262,8 @@ void flush_area_mask(const cpumask_t *mask, const void *va, 
unsigned int flags)
 {
     unsigned int cpu = smp_processor_id();
 
-    ASSERT(local_irq_is_enabled());
+    /* Local flushes can be performed with interrupts disabled. */
+    ASSERT(local_irq_is_enabled() || cpumask_equal(mask, cpumask_of(cpu)));
 
     if ( (flags & ~(FLUSH_VCPU_STATE | FLUSH_ORDER_MASK)) &&
          cpumask_test_cpu(cpu, mask) )
-- 
2.36.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.