[PATCH] x86/shadow: don't use #if in unlikely() invocation

Jan Beulich posted 1 patch 4 days, 16 hours ago
Failed in applying to current master (apply log)
[PATCH] x86/shadow: don't use #if in unlikely() invocation
Posted by Jan Beulich 4 days, 16 hours ago
As per the standard this is UB, i.e. we're building on an extension in
the compilers we use (for gcc, see "The C Preprocessor" section
"Directives Within Macro Arguments"). Misra C:2012 rule 20.6 disallows
this altogether, though.

Combine the fix with some tidying: Fold the two level-1 checks, adjust in
particular comment style, and move the unlikely() use to a single inner
part of the conditional.

No functional change intended.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -634,35 +634,36 @@ _sh_propagate(struct vcpu *v,
         sflags |= _PAGE_UC;
     }
 
-    // protect guest page tables
-    //
-    if ( unlikely((level == 1)
-                  && sh_mfn_is_a_page_table(target_mfn)
-#if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC )
-                  /* Unless the page is out of sync and the guest is
-                     writing to it. */
-                  && !(mfn_oos_may_write(target_mfn)
-                       && (ft == ft_demand_write))
-#endif /* OOS */
-                  ) )
-        sflags &= ~_PAGE_RW;
-
-    /*
-     * shadow_mode_log_dirty support
-     *
-     * Only allow the guest write access to a page a) on a demand fault,
-     * or b) if the page is already marked as dirty.
-     *
-     * (We handle log-dirty entirely inside the shadow code, without using the
-     * p2m_ram_logdirty p2m type: only HAP uses that.)
-     */
-    if ( level == 1 && unlikely(paging_mode_log_dirty(d)) && !mmio_mfn )
+    if ( level == 1 )
     {
-        if ( ft & FETCH_TYPE_WRITE )
-            paging_mark_dirty(d, target_mfn);
-        else if ( (sflags & _PAGE_RW) &&
-                  !paging_mfn_is_dirty(d, target_mfn) )
+        /* Protect guest page tables. */
+        if ( unlikely(sh_mfn_is_a_page_table(target_mfn))
+#if SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC
+             /*
+              * Unless the page is out of sync and the guest is writing to it.
+              */
+             && (ft != ft_demand_write || !mfn_oos_may_write(target_mfn))
+#endif /* OOS */
+           )
             sflags &= ~_PAGE_RW;
+
+        /*
+         * shadow_mode_log_dirty support
+         *
+         * Only allow the guest write access to a page a) on a demand fault,
+         * or b) if the page is already marked as dirty.
+         *
+         * (We handle log-dirty entirely inside the shadow code, without using
+         * the p2m_ram_logdirty p2m type: only HAP uses that.)
+         */
+        if ( unlikely(paging_mode_log_dirty(d)) && !mmio_mfn )
+        {
+            if ( ft & FETCH_TYPE_WRITE )
+                paging_mark_dirty(d, target_mfn);
+            else if ( (sflags & _PAGE_RW) &&
+                      !paging_mfn_is_dirty(d, target_mfn) )
+                sflags &= ~_PAGE_RW;
+        }
     }
 
     // PV guests in 64-bit mode use two different page tables for user vs
Re: [PATCH] x86/shadow: don't use #if in unlikely() invocation
Posted by Andrew Cooper 4 days, 15 hours ago
On 26/02/2026 8:57 am, Jan Beulich wrote:
> As per the standard this is UB, i.e. we're building on an extension in
> the compilers we use (for gcc, see "The C Preprocessor" section
> "Directives Within Macro Arguments"). Misra C:2012 rule 20.6 disallows
> this altogether, though.
>
> Combine the fix with some tidying: Fold the two level-1 checks, adjust in
> particular comment style, and move the unlikely() use to a single inner
> part of the conditional.
>
> No functional change intended.
>
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>