Re: [PATCH v1 03/25] xen/x86: complement PG_log_dirty wrapping

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.08.2025 11:47, Penny Zheng wrote:
> We have PG_log_dirty flag to wrap codes about paging log-dirty support.
> The main entry is paging_domctl(),

Yet paging_domctl() serves other paging modes, too. In fact for the shim
PG_SH_forced is actually a mode that could plausibly come into use (even
if not by way of a toolstack requesting it). It was previously indicated
that it was wrong for SHADOW_PAGING to be forced off in shim-exclusive
mode. No patch ever surfaced, but that may want sorting before any new
boundaries are to be drawn. In fact, ...

> --- a/xen/arch/x86/include/asm/paging.h
> +++ b/xen/arch/x86/include/asm/paging.h
> @@ -55,7 +55,7 @@
>  #define PG_translate   0
>  #define PG_external    0
>  #endif
> -#if defined(CONFIG_HVM) || !defined(CONFIG_PV_SHIM_EXCLUSIVE)
> +#if defined(CONFIG_HVM)
>  /* Enable log dirty mode */
>  #define PG_log_dirty   (XEN_DOMCTL_SHADOW_ENABLE_LOG_DIRTY << PG_mode_shift)

... the conditional here likely wants to become

#if defined(CONFIG_HVM) || defined(CONFIG_SHADOW_PAGING)

instead. This might actually be a far more isolated change then, still
moving you in the direction that you want (I think).

> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -189,6 +189,7 @@ out:
>      return rc;
>  }
>  
> +#if PG_log_dirty
>  /************************************************/
>  /*            HAP LOG DIRTY SUPPORT             */
>  /************************************************/
> @@ -254,6 +255,7 @@ static void cf_check hap_clean_dirty_bitmap(struct domain 
> *d)
>      p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_logdirty);
>      guest_flush_tlb_mask(d, d->dirty_cpumask);
>  }
> +#endif /* PG_log_dirty */

Please also don't go overboard with #ifdef-ary: Files built for HVM=y
only don't need these, as HVM=y implies non-zero PG_log_dirty. But maybe,
with the other change that I outlined, you won't even need all of this
anymore.

Jan