Re: [PATCH] x86: Add Kconfig option for log-dirty tracking

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.02.2026 11:31, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -146,6 +146,7 @@ config XEN_IBT
>  config SHADOW_PAGING
>       bool "Shadow Paging"
>       default !PV_SHIM_EXCLUSIVE
> +     select LOG_DIRTY
>       depends on PV || HVM
>       help

Why would this be? IOW why would shadow imply log-dirty, but HAP wouldn't?

> @@ -166,6 +167,14 @@ config SHADOW_PAGING
>  config PAGING
>       def_bool HVM || SHADOW_PAGING
>  
> +config LOG_DIRTY

PAGING_LOG_DIRTY?

> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -220,15 +220,15 @@ long arch_do_domctl(
>      {
>  
>      case XEN_DOMCTL_shadow_op:
> -#ifdef CONFIG_PAGING
> +        ret = -EOPNOTSUPP;
> +        if ( !IS_ENABLED(CONFIG_LOG_DIRTY) )
> +            break;
> +
>          ret = paging_domctl(d, &domctl->u.shadow_op, u_domctl, 0);
>          if ( ret == -ERESTART )
>              return hypercall_create_continuation(
>                         __HYPERVISOR_paging_domctl_cont, "h", u_domctl);
>          copyback = true;
> -#else
> -        ret = -EOPNOTSUPP;
> -#endif
>          break;

Can a HVM-only hypervisor create any guests with this? I simply fail to
see how XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION would then make it through to
hap_domctl().

> --- a/xen/arch/x86/include/asm/domain.h
> +++ b/xen/arch/x86/include/asm/domain.h
> @@ -226,7 +226,9 @@ struct paging_domain {
>      unsigned int            p2m_pages;    /* number of pages allocated to 
> p2m */
>  
>      /* log dirty support */
> +#ifdef CONFIG_LOG_DIRTY
>      struct log_dirty_domain log_dirty;
> +#endif /* CONFIG_LOG_DIRTY */

Such an #ifdef can likely replace the comment? Or else the comment would
better also live inside the #ifdef?

> --- a/xen/arch/x86/include/asm/paging.h
> +++ b/xen/arch/x86/include/asm/paging.h
> @@ -55,12 +55,9 @@
>  #define PG_translate   0
>  #define PG_external    0
>  #endif
> -#if defined(CONFIG_PAGING) && !defined(CONFIG_PV_SHIM_EXCLUSIVE)
>  /* Enable log dirty mode */
> -#define PG_log_dirty   (XEN_DOMCTL_SHADOW_ENABLE_LOG_DIRTY << PG_mode_shift)
> -#else
> -#define PG_log_dirty   0
> -#endif
> +#define PG_log_dirty   IS_ENABLED(CONFIG_LOG_DIRTY) * \
> +                       (XEN_DOMCTL_SHADOW_ENABLE_LOG_DIRTY << PG_mode_shift)

Need wrapping in parentheses then.

> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -50,7 +50,7 @@ struct hap_dirty_vram {
>   * calling p2m_log_dirty_range(), which interrogates each vram
>   * page's p2m type looking for pages that have been made writable.
>   */
> -
> +#ifdef CONFIG_LOG_DIRTY

This wants to move further up.

> --- a/xen/include/hypercall-defs.c
> +++ b/xen/include/hypercall-defs.c
> @@ -194,7 +194,7 @@ dm_op(domid_t domid, unsigned int nr_bufs, 
> xen_dm_op_buf_t *bufs)
>  #ifdef CONFIG_SYSCTL
>  sysctl(xen_sysctl_t *u_sysctl)
>  #endif
> -#if defined(CONFIG_X86) && defined(CONFIG_PAGING) && 
> !defined(CONFIG_PV_SHIM_EXCLUSIVE)
> +#if defined(CONFIG_LOG_DIRTY)
>  paging_domctl_cont(xen_domctl_t *u_domctl)
>  #endif
>  #ifndef CONFIG_PV_SHIM_EXCLUSIVE
> @@ -292,7 +292,7 @@ dm_op                              compat   do       
> compat   do       do
>  hypfs_op                           do       do       do       do       do
>  #endif
>  mca                                do       do       -        -        -
> -#if defined(CONFIG_X86) && defined(CONFIG_PAGING) && 
> !defined(CONFIG_PV_SHIM_EXCLUSIVE)
> +#if defined(CONFIG_LOG_DIRTY)
>  paging_domctl_cont                 do       do       do       do       -
>  #endif

The CONFIG_X86 part of the checking wants to remain: Another port may also gain
a setting of this name, without necessarily having this auxiliary hypercall.

Jan