[PATCH v3 0/2] x86/mm: fix regression when booting with CET-SS

[PATCH v3 0/2] x86/mm: fix regression when booting with CET-SS

[Roger Pau Monne]

Hello,

Following series aim to fix a regression when booting with CET shadow
stacks enabled.  First patch is a pre-req non-functional change, second
patch is the actual fix.

Thanks, Roger.

Roger Pau Monne (2):
  x86/mm: rename FLUSH_FORCE_IPI to FLUSH_NO_ASSIST
  x86/flushtlb: remove flush_area check on system state

 xen/arch/x86/include/asm/flushtlb.h | 16 ++++++++--------
 xen/arch/x86/mm.c                   | 11 +++--------
 xen/arch/x86/smp.c                  |  5 ++++-
 3 files changed, 15 insertions(+), 17 deletions(-)

-- 
2.36.0

[PATCH v3 1/2] x86/mm: rename FLUSH_FORCE_IPI to FLUSH_NO_ASSIST

[Roger Pau Monne]

Rename the flag to better note that it's not actually forcing any IPIs
to be issued if none is required, but merely avoiding the usage of TLB
flush assistance (which itself can avoid the sending of IPIs to remote
processors).

No functional change expected.

Requested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v2:
 - New in this version.
---
 xen/arch/x86/include/asm/flushtlb.h | 16 ++++++++--------
 xen/arch/x86/mm.c                   |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/include/asm/flushtlb.h b/xen/arch/x86/include/asm/flushtlb.h
index 18777f1d4c..a461ee36ff 100644
--- a/xen/arch/x86/include/asm/flushtlb.h
+++ b/xen/arch/x86/include/asm/flushtlb.h
@@ -128,13 +128,12 @@ void switch_cr3_cr4(unsigned long cr3, unsigned long cr4);
 #endif
 #if defined(CONFIG_PV) || defined(CONFIG_SHADOW_PAGING)
 /*
- * Force an IPI to be sent. Note that adding this to the flags passed to
- * flush_area_mask will prevent using the assisted flush without having any
- * other side effect.
+ * Adding this to the flags passed to flush_area_mask will prevent using the
+ * assisted flush without having any other side effect.
  */
-# define FLUSH_FORCE_IPI 0x8000
+# define FLUSH_NO_ASSIST 0x8000
 #else
-# define FLUSH_FORCE_IPI 0
+# define FLUSH_NO_ASSIST 0
 #endif
 
 /* Flush local TLBs/caches. */
@@ -162,11 +161,12 @@ void flush_area_mask(const cpumask_t *, const void *va, unsigned int flags);
     flush_area_mask(mask, (const void *)(v), FLUSH_TLB|FLUSH_ORDER(0))
 
 /*
- * Make the common code TLB flush helper force use of an IPI in order to be
- * on the safe side. Note that not all calls from common code strictly require
+ * Make the common code TLB flush helper disallow the usage of any flush
+ * assistance in order to be on the safe side and interrupt remote processors
+ * requiring a flush. Note that not all calls from common code strictly require
  * this.
  */
-#define arch_flush_tlb_mask(mask) flush_mask(mask, FLUSH_TLB | FLUSH_FORCE_IPI)
+#define arch_flush_tlb_mask(mask) flush_mask(mask, FLUSH_TLB | FLUSH_NO_ASSIST)
 
 /* Flush all CPUs' TLBs */
 #define flush_tlb_all()                         \
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 72dbce43b1..bbb834c3fb 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2988,7 +2988,7 @@ static int _get_page_type(struct page_info *page, unsigned long type,
                     flush_mask(mask,
                                (x & PGT_type_mask) &&
                                (x & PGT_type_mask) <= PGT_root_page_table
-                               ? FLUSH_TLB | FLUSH_FORCE_IPI
+                               ? FLUSH_TLB | FLUSH_NO_ASSIST
                                : FLUSH_TLB);
                 }
 
-- 
2.36.0

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

[Roger Pau Monne]

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

Assertion 'local_irq_is_enabled()' failed at arch/x86/smp.c:265
----[ Xen-4.17.0-10.24-d  x86_64  debug=y  Not tainted ]----
CPU:    0
RIP:    e008:[<ffff82d040345300>] flush_area_mask+0x40/0x13e
[...]
Xen call trace:
   [<ffff82d040345300>] R flush_area_mask+0x40/0x13e
   [<ffff82d040338a40>] F modify_xen_mappings+0xc5/0x958
   [<ffff82d0404474f9>] F arch/x86/alternative.c#_alternative_instructions+0xb7/0xb9
   [<ffff82d0404476cc>] F alternative_branches+0xf/0x12
   [<ffff82d04044e37d>] F __start_xen+0x1ef4/0x2776
   [<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 of flush_area_all() in mm.c, 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().

While there also introduce an ASSERT to check that a vCPU state flush
is not issued for the local CPU only.

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>
---
Changes since v2:
 - Fix commit message.
 - Keep flush_area() in mm.c and reduce code churn.

Changes since v1:
 - Add an extra assert.
 - Rename flush_area() to flush_area_all().
---
 xen/arch/x86/mm.c  | 9 ++-------
 xen/arch/x86/smp.c | 5 ++++-
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index bbb834c3fb..038f71ecf4 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5070,13 +5070,8 @@ 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))
+/* flush_area_all() can be used prior to any other CPU being online.  */
+#define flush_area(v, f) flush_area_all((const void *)v, f)
 
 #define L3T_INIT(page) (page) = ZERO_BLOCK_PTR
 
diff --git a/xen/arch/x86/smp.c b/xen/arch/x86/smp.c
index 0a02086966..b42603c351 100644
--- a/xen/arch/x86/smp.c
+++ b/xen/arch/x86/smp.c
@@ -262,7 +262,10 @@ 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_subset(mask, cpumask_of(cpu)));
+    /* Exclude use of FLUSH_VCPU_STATE for the local CPU. */
+    ASSERT(!cpumask_test_cpu(cpu, mask) || !(flags & FLUSH_VCPU_STATE));
 
     if ( (flags & ~(FLUSH_VCPU_STATE | FLUSH_ORDER_MASK)) &&
          cpumask_test_cpu(cpu, mask) )
-- 
2.36.0

Re: [PATCH v3 1/2] x86/mm: rename FLUSH_FORCE_IPI to FLUSH_NO_ASSIST

[Jan Beulich]

On 25.05.2022 10:13, Roger Pau Monne wrote:
> Rename the flag to better note that it's not actually forcing any IPIs
> to be issued if none is required, but merely avoiding the usage of TLB
> flush assistance (which itself can avoid the sending of IPIs to remote
> processors).
> 
> No functional change expected.
> 
> Requested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

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

[Jan Beulich]

On 25.05.2022 10:13, Roger Pau Monne wrote:
> Booting with Shadow Stacks leads to the following assert on a debug
> hypervisor:
> 
> Assertion 'local_irq_is_enabled()' failed at arch/x86/smp.c:265
> ----[ Xen-4.17.0-10.24-d  x86_64  debug=y  Not tainted ]----
> CPU:    0
> RIP:    e008:[<ffff82d040345300>] flush_area_mask+0x40/0x13e
> [...]
> Xen call trace:
>    [<ffff82d040345300>] R flush_area_mask+0x40/0x13e
>    [<ffff82d040338a40>] F modify_xen_mappings+0xc5/0x958
>    [<ffff82d0404474f9>] F arch/x86/alternative.c#_alternative_instructions+0xb7/0xb9
>    [<ffff82d0404476cc>] F alternative_branches+0xf/0x12
>    [<ffff82d04044e37d>] F __start_xen+0x1ef4/0x2776
>    [<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 of flush_area_all() in mm.c, 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().
> 
> While there also introduce an ASSERT to check that a vCPU state flush
> is not issued for the local CPU only.
> 
> 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>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with ...

> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5070,13 +5070,8 @@ 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))
> +/* flush_area_all() can be used prior to any other CPU being online.  */
> +#define flush_area(v, f) flush_area_all((const void *)v, f)

... v properly parenthesized here as the code is being touched anyway:
One less Misra-C violation. This surely can be done while committing.

Jan

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

[Roger Pau Monné]

On Wed, May 25, 2022 at 10:41:51AM +0200, Jan Beulich wrote:
> On 25.05.2022 10:13, Roger Pau Monne wrote:
> > Booting with Shadow Stacks leads to the following assert on a debug
> > hypervisor:
> > 
> > Assertion 'local_irq_is_enabled()' failed at arch/x86/smp.c:265
> > ----[ Xen-4.17.0-10.24-d  x86_64  debug=y  Not tainted ]----
> > CPU:    0
> > RIP:    e008:[<ffff82d040345300>] flush_area_mask+0x40/0x13e
> > [...]
> > Xen call trace:
> >    [<ffff82d040345300>] R flush_area_mask+0x40/0x13e
> >    [<ffff82d040338a40>] F modify_xen_mappings+0xc5/0x958
> >    [<ffff82d0404474f9>] F arch/x86/alternative.c#_alternative_instructions+0xb7/0xb9
> >    [<ffff82d0404476cc>] F alternative_branches+0xf/0x12
> >    [<ffff82d04044e37d>] F __start_xen+0x1ef4/0x2776
> >    [<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 of flush_area_all() in mm.c, 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().
> > 
> > While there also introduce an ASSERT to check that a vCPU state flush
> > is not issued for the local CPU only.
> > 
> > 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>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with ...
> 
> > --- a/xen/arch/x86/mm.c
> > +++ b/xen/arch/x86/mm.c
> > @@ -5070,13 +5070,8 @@ 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))
> > +/* flush_area_all() can be used prior to any other CPU being online.  */
> > +#define flush_area(v, f) flush_area_all((const void *)v, f)
> 
> ... v properly parenthesized here as the code is being touched anyway:
> One less Misra-C violation. This surely can be done while committing.

Indeed.  I had my addition properly parenthesized, but forgot to do it
here when moving the line.

Thanks, Roger.

Re: [PATCH v3 1/2] x86/mm: rename FLUSH_FORCE_IPI to FLUSH_NO_ASSIST

[Andrew Cooper]

On 25/05/2022 09:13, Roger Pau Monne wrote:
> Rename the flag to better note that it's not actually forcing any IPIs
> to be issued if none is required, but merely avoiding the usage of TLB
> flush assistance (which itself can avoid the sending of IPIs to remote
> processors).
>
> No functional change expected.
>
> Requested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Changes since v2:
>  - New in this version.

:(  This needs reverting.

It is specific to IPIs, because of our current choice of algorithm for
freeing pagetables.

"no assist" excludes ipi-helper hypercalls which invoke
INVALIDATE_TLB_VECTOR.  Such hypercalls do exist and specifically would
be improvement that we ought to use.

Furthermore, we do want to work around the limitation that created
FLUSH_FORCE_IPI, because we absolutely do want to be able to use
hypercalls/remote TLB flushing capabilities when available.

I accept that FORCE_IPI perhaps isn't a perfect name, but it's a whole
lot less bad than NO_ASSIST.

~Andrew

Re: [PATCH v3 1/2] x86/mm: rename FLUSH_FORCE_IPI to FLUSH_NO_ASSIST

[Jan Beulich]

On 25.05.2022 12:52, Andrew Cooper wrote:
> On 25/05/2022 09:13, Roger Pau Monne wrote:
>> Rename the flag to better note that it's not actually forcing any IPIs
>> to be issued if none is required, but merely avoiding the usage of TLB
>> flush assistance (which itself can avoid the sending of IPIs to remote
>> processors).
>>
>> No functional change expected.
>>
>> Requested-by: Jan Beulich <jbeulich@suse.com>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> ---
>> Changes since v2:
>>  - New in this version.
> 
> :(  This needs reverting.
> 
> It is specific to IPIs, because of our current choice of algorithm for
> freeing pagetables.
> 
> "no assist" excludes ipi-helper hypercalls which invoke
> INVALIDATE_TLB_VECTOR.  Such hypercalls do exist and specifically would
> be improvement that we ought to use.
> 
> Furthermore, we do want to work around the limitation that created
> FLUSH_FORCE_IPI, because we absolutely do want to be able to use
> hypercalls/remote TLB flushing capabilities when available.
> 
> I accept that FORCE_IPI perhaps isn't a perfect name, but it's a whole
> lot less bad than NO_ASSIST.

But FORCE_IPI has caused actual confusion while reviewing patch 2.
If NO_ASSIST doesn't suit you and FORCE_IPI is also wrong, can you
suggest a better name fitting both aspects?

Jan

Re: [PATCH v3 1/2] x86/mm: rename FLUSH_FORCE_IPI to FLUSH_NO_ASSIST

[Andrew Cooper]

On 25/05/2022 12:14, Jan Beulich wrote:
> On 25.05.2022 12:52, Andrew Cooper wrote:
>> On 25/05/2022 09:13, Roger Pau Monne wrote:
>>> Rename the flag to better note that it's not actually forcing any IPIs
>>> to be issued if none is required, but merely avoiding the usage of TLB
>>> flush assistance (which itself can avoid the sending of IPIs to remote
>>> processors).
>>>
>>> No functional change expected.
>>>
>>> Requested-by: Jan Beulich <jbeulich@suse.com>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> ---
>>> Changes since v2:
>>>  - New in this version.
>> :(  This needs reverting.
>>
>> It is specific to IPIs, because of our current choice of algorithm for
>> freeing pagetables.
>>
>> "no assist" excludes ipi-helper hypercalls which invoke
>> INVALIDATE_TLB_VECTOR.  Such hypercalls do exist and specifically would
>> be improvement that we ought to use.
>>
>> Furthermore, we do want to work around the limitation that created
>> FLUSH_FORCE_IPI, because we absolutely do want to be able to use
>> hypercalls/remote TLB flushing capabilities when available.
>>
>> I accept that FORCE_IPI perhaps isn't a perfect name, but it's a whole
>> lot less bad than NO_ASSIST.
> But FORCE_IPI has caused actual confusion while reviewing patch 2.
> If NO_ASSIST doesn't suit you and FORCE_IPI is also wrong, can you
> suggest a better name fitting both aspects?

I don't actually agree that FORCE_IPI is unclear to begin with.

The safety property required is "if you need to contact a remote CPU, it
must be by IPI to interlock with Xen's #PF handler".

FORCE_IPI is very close to meaning this.  If anything else is unclear,
it can be clarified in the adjacent comment.

~Andrew

Re: [PATCH v3 1/2] x86/mm: rename FLUSH_FORCE_IPI to FLUSH_NO_ASSIST

[Roger Pau Monné]

On Wed, May 25, 2022 at 10:52:48AM +0000, Andrew Cooper wrote:
> On 25/05/2022 09:13, Roger Pau Monne wrote:
> > Rename the flag to better note that it's not actually forcing any IPIs
> > to be issued if none is required, but merely avoiding the usage of TLB
> > flush assistance (which itself can avoid the sending of IPIs to remote
> > processors).
> >
> > No functional change expected.
> >
> > Requested-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Changes since v2:
> >  - New in this version.
> 
> :(  This needs reverting.
> 
> It is specific to IPIs, because of our current choice of algorithm for
> freeing pagetables.
> 
> "no assist" excludes ipi-helper hypercalls which invoke
> INVALIDATE_TLB_VECTOR.  Such hypercalls do exist and specifically would
> be improvement that we ought to use.

So the improvement of that mechanism is that you can pass a cpumask
parameter to an hypercall in order to avoid having to issue repeated
wrmsrs (or APIC MMIO accesses) to send IPIs to different vCPUs?

But that could be seen as generic assistance for triggering the
execution of remote IDT handlers, and as such won't be restricted by
the NO_ASSIST flag (also because it would be implemented in
send_IPI_mask() rather than flush_area_mask() IMO).

> Furthermore, we do want to work around the limitation that created
> FLUSH_FORCE_IPI, because we absolutely do want to be able to use
> hypercalls/remote TLB flushing capabilities when available.

I agree, we need to get rid of FLUSH_FORCE_IPI.

> I accept that FORCE_IPI perhaps isn't a perfect name, but it's a whole
> lot less bad than NO_ASSIST.

I'm happy for you and Jan to decide on a different name that you both
agree.

Thanks, Roger.

Re: [PATCH v3 1/2] x86/mm: rename FLUSH_FORCE_IPI to FLUSH_NO_ASSIST

[Jan Beulich]

On 25.05.2022 16:33, Andrew Cooper wrote:
> On 25/05/2022 12:14, Jan Beulich wrote:
>> On 25.05.2022 12:52, Andrew Cooper wrote:
>>> On 25/05/2022 09:13, Roger Pau Monne wrote:
>>>> Rename the flag to better note that it's not actually forcing any IPIs
>>>> to be issued if none is required, but merely avoiding the usage of TLB
>>>> flush assistance (which itself can avoid the sending of IPIs to remote
>>>> processors).
>>>>
>>>> No functional change expected.
>>>>
>>>> Requested-by: Jan Beulich <jbeulich@suse.com>
>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>>> ---
>>>> Changes since v2:
>>>>  - New in this version.
>>> :(  This needs reverting.
>>>
>>> It is specific to IPIs, because of our current choice of algorithm for
>>> freeing pagetables.
>>>
>>> "no assist" excludes ipi-helper hypercalls which invoke
>>> INVALIDATE_TLB_VECTOR.  Such hypercalls do exist and specifically would
>>> be improvement that we ought to use.
>>>
>>> Furthermore, we do want to work around the limitation that created
>>> FLUSH_FORCE_IPI, because we absolutely do want to be able to use
>>> hypercalls/remote TLB flushing capabilities when available.
>>>
>>> I accept that FORCE_IPI perhaps isn't a perfect name, but it's a whole
>>> lot less bad than NO_ASSIST.
>> But FORCE_IPI has caused actual confusion while reviewing patch 2.
>> If NO_ASSIST doesn't suit you and FORCE_IPI is also wrong, can you
>> suggest a better name fitting both aspects?
> 
> I don't actually agree that FORCE_IPI is unclear to begin with.

You did see the earlier communication with Roger, didn't you? To
me the name pretty clearly suggests "always IPI" (hence "force"),
i.e. ...

> The safety property required is "if you need to contact a remote CPU, it
> must be by IPI to interlock with Xen's #PF handler".

... not in any way limited to remote CPUs. Yet patch 2 is about
cases where things are safe because no IPI will be needed (not
even a self-IPI).

> FORCE_IPI is very close to meaning this.  If anything else is unclear,
> it can be clarified in the adjacent comment.

I'm afraid I don't think a comment is what would help here.

Jan