From: Roger Pau Monne <roger.pau@citrix.com>
To: xen-devel@lists.xenproject.org
Cc: Roger Pau Monne <roger.pau@citrix.com>,
Jan Beulich <jbeulich@suse.com>,
Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>
Subject: [PATCH] x86/flushtlb: remove flush_area check on system state
Date: Mon, 16 May 2022 16:31:16 +0200 [thread overview]
Message-ID: <20220516143116.28602-1-roger.pau@citrix.com> (raw)
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@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
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
next reply other threads:[~2022-05-16 14:33 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-16 14:31 Roger Pau Monne [this message]
2022-05-16 14:47 ` [PATCH] x86/flushtlb: remove flush_area check on system state Andrew Cooper
2022-05-18 8:49 ` Jan Beulich
2022-05-23 14:37 ` Roger Pau Monné
2022-05-23 15:13 ` Jan Beulich
2022-05-23 16:24 ` Roger Pau Monné
2022-05-24 10:48 ` Roger Pau Monné
2022-05-24 7:32 ` Roger Pau Monné
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220516143116.28602-1-roger.pau@citrix.com \
--to=roger.pau@citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=jbeulich@suse.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.