[PATCH v15 09/10] x86/shadow: make forced (L1TF) mode enable properly preemptable

Jan Beulich <[email protected]> Tue, 28 Jul 2026 16:24:27 +0200
Newsgroups gmane.comp.emulators.xen.devel
Message-ID <[email protected]>
Like for log-dirty enable/disable, reduce the amount of work to do
underneath sh_new_mode() by calling shadow_blow_tables() (which is
properly preemptable) up front. shadow_set_allocation() possibly taking
long also is handled similar to log-dirty enable.

Since this code runs inside a tasklet, carrying out preemption isn't
straightforward; see the code comment.

Signed-off-by: Jan Beulich <[email protected]>
---
The single-CPU case could of course do with improvement. Plus even on
multi-CPU systems there will still be problems if there are enough
domains all being switched to shadow mode at roughly the same time.

Interaction with other shadow-op preemption is a little rough: Any
attempt to issue a shadow-op (other than get-allocation) will result in
-EBUSY while the operation here is still in progress.

Notes from discussion:
- if there weren't certain special situations (realtime scheduling requirements, null scheduler,
  vCPU pinning), simply handling softirqs _only_ in the tasklet might be okay
- instead of doing shadow_blow_tables() and shadow_set_allocation() from the tasklet, we may be
  able to arrange doing that in the context of the initiating vCPU (with all other vCPU-s paused),
  scheduling the tasklet only once done, and making sure we don't fully exit back to guest context
  in the process (but enough to allow scheduling to occur) [Could we have paravirt_ctxt_switch_to()
  schedule a softirq-tasklet for VMs in transitional state, which then does the work?]
---
v14: New.

--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -237,6 +237,7 @@ struct paging_domain {
         const struct domain *dom;
         unsigned int op;
         bool drop_allocation:1;
+        bool pv_l1tf_paused:1;
         union {
             struct {
                 unsigned long done:PADDR_BITS - PAGE_SHIFT;
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -18,6 +18,7 @@
 #include <xen/domain_page.h>
 #include <xen/guest_access.h>
 #include <xen/keyhandler.h>
+#include <xen/softirq.h>
 #include <asm/event.h>
 #include <asm/page.h>
 #include <asm/current.h>
@@ -2444,11 +2445,11 @@ static int shadow_one_bit_enable(struct
 
         /* Init the shadow memory allocation if the user hasn't done so */
         if ( shadow_set_allocation(d, 1,
-                                   mode & PG_log_dirty ? &preempted
-                                                       : NULL) != 0 )
+                                   mode & ~PG_SH_enable ? &preempted
+                                                        : NULL) != 0 )
         {
             shadow_set_allocation(d, 0,
-                                  mode & PG_log_dirty ? &preempted : NULL);
+                                  mode & ~PG_SH_enable ? &preempted : NULL);
             if ( !preempted )
                 return -ENOMEM;
             d->arch.paging.preempt.drop_allocation = true;
@@ -2785,22 +2786,80 @@ void shadow_audit_tables(struct vcpu *v)
 void cf_check pv_l1tf_tasklet(void *data)
 {
     struct domain *d = data;
+    int ret = 0;
 
-    domain_pause(d);
+    /* Lock-less read is okay: The field is only written inside this tasklet. */
+    if ( !d->arch.paging.preempt.pv_l1tf_paused )
+        domain_pause(d);
     paging_lock(d);
 
-    if ( !paging_mode_sh_forced(d) && !d->is_dying )
+    while ( !paging_mode_sh_forced(d) && !d->is_dying )
     {
-        int ret = shadow_one_bit_enable(d, PG_SH_forced);
+        bool preempted = false;
+        unsigned int cpu, next;
 
-        if ( ret )
+        if ( !d->arch.paging.preempt.dom )
+        {
+            d->arch.paging.preempt.dom = dom_xen;
+            d->arch.paging.preempt.drop_allocation = false;
+        }
+
+        if ( unlikely(d->arch.paging.preempt.dom != dom_xen) )
+        {
+            /* Wait for other continuation to finish. */
+        }
+        else if ( unlikely(d->arch.paging.preempt.drop_allocation) )
         {
-            printk(XENLOG_G_ERR "d%d Failed to enable PG_SH_forced: %d\n",
-                   d->domain_id, ret);
-            domain_crash(d);
+            ret = -ENOMEM;
+            shadow_set_allocation(d, 0, &preempted);
+            if ( !preempted )
+                break;
         }
+        else
+        {
+            /*
+             * Limit the amount of work to do from sh_detach_old_tables()
+             * (called from shadow_one_bit_enable() via sh_new_mode() ->
+             * sh_update_paging_modes()), such that it doesn't also need to
+             * deal with preemption checks.
+             */
+            shadow_blow_tables(d, &preempted);
+            if ( !preempted &&
+                 (ret = shadow_one_bit_enable(d, PG_SH_forced)) != -ERESTART )
+                break;
+        }
+
+        d->arch.paging.preempt.pv_l1tf_paused = true;
+        paging_unlock(d);
+
+        /*
+         * Crude "preemption" of a tasklet: To avoid hogging the local CPU,
+         * re-schedule the tasklet on another one. Unless of course there
+         * is none, in which case we will simply continue here.
+         */
+        cpu = smp_processor_id();
+        next = cpumask_cycle(cpu, &cpu_online_map);
+        if ( next != cpu )
+        {
+            tasklet_schedule_on_cpu(&d->arch.paging.shadow.pv_l1tf_tasklet,
+                                    next);
+            return;
+        }
+
+        process_pending_softirqs();
+        paging_lock(d);
+    }
+
+    if ( ret )
+    {
+        printk(XENLOG_G_ERR "%pd: Failed to enable PG_SH_forced: %d\n",
+               d, ret);
+        domain_crash(d);
     }
 
+    d->arch.paging.preempt.dom = NULL;
+    d->arch.paging.preempt.drop_allocation = false;
+    d->arch.paging.preempt.pv_l1tf_paused = false;
     paging_unlock(d);
     domain_unpause(d);
 }