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

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


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 28 Jul 2026 16:24:27 +0200
  • Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=google header.d=suse.com header.i="@suse.com" header.h="Content-Transfer-Encoding:Content-Type:In-Reply-To:Autocrypt:Content-Language:References:Cc:To:From:Subject:User-Agent:MIME-Version:Date:Message-ID"
  • Autocrypt: addr=jbeulich@xxxxxxxx; keydata= xsDiBFk3nEQRBADAEaSw6zC/EJkiwGPXbWtPxl2xCdSoeepS07jW8UgcHNurfHvUzogEq5xk hu507c3BarVjyWCJOylMNR98Yd8VqD9UfmX0Hb8/BrA+Hl6/DB/eqGptrf4BSRwcZQM32aZK 7Pj2XbGWIUrZrd70x1eAP9QE3P79Y2oLrsCgbZJfEwCgvz9JjGmQqQkRiTVzlZVCJYcyGGsD /0tbFCzD2h20ahe8rC1gbb3K3qk+LpBtvjBu1RY9drYk0NymiGbJWZgab6t1jM7sk2vuf0Py O9Hf9XBmK0uE9IgMaiCpc32XV9oASz6UJebwkX+zF2jG5I1BfnO9g7KlotcA/v5ClMjgo6Gl MDY4HxoSRu3i1cqqSDtVlt+AOVBJBACrZcnHAUSuCXBPy0jOlBhxPqRWv6ND4c9PH1xjQ3NP nxJuMBS8rnNg22uyfAgmBKNLpLgAGVRMZGaGoJObGf72s6TeIqKJo/LtggAS9qAUiuKVnygo 3wjfkS9A3DRO+SpU7JqWdsveeIQyeyEJ/8PTowmSQLakF+3fote9ybzd880fSmFuIEJldWxp Y2ggPGpiZXVsaWNoQHN1c2UuY29tPsJgBBMRAgAgBQJZN5xEAhsDBgsJCAcDAgQVAggDBBYC AwECHgECF4AACgkQoDSui/t3IH4J+wCfQ5jHdEjCRHj23O/5ttg9r9OIruwAn3103WUITZee e7Sbg12UgcQ5lv7SzsFNBFk3nEQQCACCuTjCjFOUdi5Nm244F+78kLghRcin/awv+IrTcIWF hUpSs1Y91iQQ7KItirz5uwCPlwejSJDQJLIS+QtJHaXDXeV6NI0Uef1hP20+y8qydDiVkv6l IreXjTb7DvksRgJNvCkWtYnlS3mYvQ9NzS9PhyALWbXnH6sIJd2O9lKS1Mrfq+y0IXCP10eS FFGg+Av3IQeFatkJAyju0PPthyTqxSI4lZYuJVPknzgaeuJv/2NccrPvmeDg6Coe7ZIeQ8Yj t0ARxu2xytAkkLCel1Lz1WLmwLstV30g80nkgZf/wr+/BXJW/oIvRlonUkxv+IbBM3dX2OV8 AmRv1ySWPTP7AAMFB/9PQK/VtlNUJvg8GXj9ootzrteGfVZVVT4XBJkfwBcpC/XcPzldjv+3 HYudvpdNK3lLujXeA5fLOH+Z/G9WBc5pFVSMocI71I8bT8lIAzreg0WvkWg5V2WZsUMlnDL9 mpwIGFhlbM3gfDMs7MPMu8YQRFVdUvtSpaAs8OFfGQ0ia3LGZcjA6Ik2+xcqscEJzNH+qh8V m5jjp28yZgaqTaRbg3M/+MTbMpicpZuqF4rnB0AQD12/3BNWDR6bmh+EkYSMcEIpQmBM51qM EKYTQGybRCjpnKHGOxG0rfFY1085mBDZCH5Kx0cl0HVJuQKC+dV2ZY5AqjcKwAxpE75MLFkr wkkEGBECAAkFAlk3nEQCGwwACgkQoDSui/t3IH7nnwCfcJWUDUFKdCsBH/E5d+0ZnMQi+G0A nAuWpQkjM1ASeQwSHEeAWPgskBQL
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>
  • Delivery-date: Tue, 28 Jul 2026 14:24:32 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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 <jbeulich@xxxxxxxx>
---
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);
 }




 


Rackspace

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