* [PATCH] x86: Add Kconfig option for log-dirty tracking
@ 2026-02-09 10:31 Alejandro Vallejo
2026-02-09 14:48 ` Jan Beulich
2026-02-09 15:44 ` Roger Pau Monné
0 siblings, 2 replies; 9+ messages in thread
From: Alejandro Vallejo @ 2026-02-09 10:31 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Anthony PERARD, Michal Orzel, Julien Grall,
Stefano Stabellini
Creates a CONFIG_LOG_DIRTY Kconfig option with the following effects
when disabled:
* paging_domctl{,_cont} return -EOPNOTSUPP (XEN_DOMCTL_shadow_op).
* VRAM tracking via DMOP returns EOPNOTSUPP.
And compiles out all log-dirty tracking infra.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
RFC for the Kconfig help message.
---
xen/arch/x86/Kconfig | 9 +++++++++
xen/arch/x86/domctl.c | 8 ++++----
xen/arch/x86/hvm/dm.c | 3 +++
xen/arch/x86/include/asm/domain.h | 2 ++
xen/arch/x86/include/asm/p2m.h | 2 ++
xen/arch/x86/include/asm/paging.h | 8 +++-----
xen/arch/x86/mm/hap/hap.c | 22 +++++++++++++---------
xen/arch/x86/mm/p2m.c | 2 ++
xen/arch/x86/mm/paging.c | 2 ++
xen/include/hypercall-defs.c | 4 ++--
10 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 61f58aa829..fbf044aa4d 100644
--- 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
@@ -166,6 +167,14 @@ config SHADOW_PAGING
config PAGING
def_bool HVM || SHADOW_PAGING
+config LOG_DIRTY
+ bool "Log-dirty page tracking" if EXPERT
+ depends on PAGING
+ default !PV_SHIM_EXCLUSIVE
+ help
+ Enable log-dirty infrastructure so Xen can track domain memory writes and
+ the dirty state of VRAM for device models and live migrations.
+
config BIGMEM
bool "big memory support"
default n
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index d9521808dc..61d43a21d0 100644
--- 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;
case XEN_DOMCTL_ioport_permission:
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index 3b53471af0..94216aecc2 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -48,6 +48,9 @@ static int track_dirty_vram(struct domain *d, xen_pfn_t first_pfn,
unsigned int nr_frames,
const struct xen_dm_op_buf *buf)
{
+ if ( !IS_ENABLED(CONFIG_LOG_DIRTY) )
+ return -EOPNOTSUPP;
+
if ( nr_frames > (GB(1) >> PAGE_SHIFT) )
return -EINVAL;
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index 94b0cf7f1d..f09c13909f 100644
--- 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 */
/* preemption handling */
struct {
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 9016e88411..3c2dcacfa5 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -253,9 +253,11 @@ struct p2m_domain {
bool *sve);
int (*recalc)(struct p2m_domain *p2m,
unsigned long gfn);
+#ifdef CONFIG_LOG_DIRTY
void (*enable_hardware_log_dirty)(struct p2m_domain *p2m);
void (*disable_hardware_log_dirty)(struct p2m_domain *p2m);
void (*flush_hardware_cached_dirty)(struct p2m_domain *p2m);
+#endif /* CONFIG_LOG_DIRTY */
void (*change_entry_type_global)(struct p2m_domain *p2m,
p2m_type_t ot,
p2m_type_t nt);
diff --git a/xen/arch/x86/include/asm/paging.h b/xen/arch/x86/include/asm/paging.h
index 291ab386e8..980cdfa455 100644
--- 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)
/* All paging modes. */
#define PG_MASK (PG_refcounts | PG_log_dirty | PG_translate | PG_external)
@@ -174,6 +171,7 @@ static inline void paging_log_dirty_init(struct domain *d,
const struct log_dirty_ops *ops) {}
static inline void paging_mark_dirty(struct domain *d, mfn_t gmfn) {}
static inline void paging_mark_pfn_dirty(struct domain *d, pfn_t pfn) {}
+static inline void paging_mark_pfn_clean(struct domain *d, pfn_t pfn) {}
static inline bool paging_mfn_is_dirty(struct domain *d, mfn_t gmfn) { return false; }
#endif /* PG_log_dirty */
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index a337752bf4..21672db011 100644
--- 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
int hap_track_dirty_vram(struct domain *d,
unsigned long begin_pfn,
unsigned int nr_frames,
@@ -161,6 +161,7 @@ out:
return rc;
}
+#endif /* CONFIG_LOG_DIRTY */
/************************************************/
/* HAP LOG DIRTY SUPPORT */
@@ -440,14 +441,17 @@ static bool cf_check flush_tlb(const unsigned long *vcpu_bitmap);
void hap_domain_init(struct domain *d)
{
- static const struct log_dirty_ops hap_ops = {
- .enable = hap_enable_log_dirty,
- .disable = hap_disable_log_dirty,
- .clean = hap_clean_dirty_bitmap,
- };
-
- /* Use HAP logdirty mechanism. */
- paging_log_dirty_init(d, &hap_ops);
+ if ( IS_ENABLED(CONFIG_LOG_DIRTY) )
+ {
+ static const struct log_dirty_ops hap_ops = {
+ .enable = hap_enable_log_dirty,
+ .disable = hap_disable_log_dirty,
+ .clean = hap_clean_dirty_bitmap,
+ };
+
+ /* Use HAP logdirty mechanism. */
+ paging_log_dirty_init(d, &hap_ops);
+ }
d->arch.paging.update_paging_modes = hap_update_paging_modes;
d->arch.paging.flush_tlb = flush_tlb;
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index e915da26a8..373382c28c 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -236,6 +236,7 @@ struct ioreq_server *p2m_get_ioreq_server(struct domain *d,
return s;
}
+#ifdef CONFIG_LOG_DIRTY
void p2m_enable_hardware_log_dirty(struct domain *d)
{
struct p2m_domain *p2m = p2m_get_hostp2m(d);
@@ -263,6 +264,7 @@ void p2m_flush_hardware_cached_dirty(struct domain *d)
p2m_unlock(p2m);
}
}
+#endif /* CONFIG_LOG_DIRTY */
/*
* Force a synchronous P2M TLB flush if a deferred flush is pending.
diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index 2396f81ad5..13ee137db9 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -623,10 +623,12 @@ int paging_domain_init(struct domain *d)
INIT_PAGE_LIST_HEAD(&d->arch.paging.freelist);
mm_lock_init(&d->arch.paging.lock);
+#ifdef CONFIG_LOG_DIRTY
/* This must be initialized separately from the rest of the
* log-dirty init code as that can be called more than once and we
* don't want to leak any active log-dirty bitmaps */
d->arch.paging.log_dirty.top = INVALID_MFN;
+#endif /* CONFIG_LOG_DIRTY */
/*
* Shadow pagetables are the default, but we will use
diff --git a/xen/include/hypercall-defs.c b/xen/include/hypercall-defs.c
index 63755bb8df..be7ed832be 100644
--- 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
base-commit: 1ee8b11c1106dca6b8fe0d54c0e79146bb6545f0
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Add Kconfig option for log-dirty tracking
2026-02-09 10:31 [PATCH] x86: Add Kconfig option for log-dirty tracking Alejandro Vallejo
@ 2026-02-09 14:48 ` Jan Beulich
2026-02-09 15:24 ` Alejandro Vallejo
2026-02-09 15:44 ` Roger Pau Monné
1 sibling, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2026-02-09 14:48 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, Michal Orzel,
Julien Grall, Stefano Stabellini, xen-devel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Add Kconfig option for log-dirty tracking
2026-02-09 14:48 ` Jan Beulich
@ 2026-02-09 15:24 ` Alejandro Vallejo
2026-02-09 15:55 ` Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: Alejandro Vallejo @ 2026-02-09 15:24 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, Michal Orzel,
Julien Grall, Stefano Stabellini, xen-devel
On Mon Feb 9, 2026 at 3:48 PM CET, Jan Beulich wrote:
> 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?
The logic is rather opaque. I admit I'm a bit fuzzy on the uses of logdirty.
I know what it's for and I could navigate the code if a problem arose, but I'm
less clear about which other elements of the hypervisor rely on it (pod? nsvm?
vvmx? shadow? hap?).
If it's strictly toolstack/DM-driven maybe it'd be more apt to have a separate
LIVE_MIGRATION and SAVE_RESTORE configs where LM selects SAVE_RESTORE, which
selects LOG_DIRTY. That's also improve some defaults auto-downgraded from the
max policy just in case a VM is migrated.
>
>> @@ -166,6 +167,14 @@ config SHADOW_PAGING
>> config PAGING
>> def_bool HVM || SHADOW_PAGING
>>
>> +config LOG_DIRTY
>
> PAGING_LOG_DIRTY?
sure.
>
>> --- 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().
xl doesn't seem to call it at all. hap_set_allocation() is implicitly called
through paging_enable() -> hap_enable() -> hap_set_allocation()
>
>> --- 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?
true.
>
>> --- 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.
true.
>
>> --- 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.
sure
>
>> --- 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.
Hmmm. Makes sense.
>
> Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Add Kconfig option for log-dirty tracking
2026-02-09 10:31 [PATCH] x86: Add Kconfig option for log-dirty tracking Alejandro Vallejo
2026-02-09 14:48 ` Jan Beulich
@ 2026-02-09 15:44 ` Roger Pau Monné
2026-02-09 16:00 ` Jan Beulich
1 sibling, 1 reply; 9+ messages in thread
From: Roger Pau Monné @ 2026-02-09 15:44 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: xen-devel, Jan Beulich, Andrew Cooper, Anthony PERARD,
Michal Orzel, Julien Grall, Stefano Stabellini
On Mon, Feb 09, 2026 at 11:31:15AM +0100, Alejandro Vallejo wrote:
> Creates a CONFIG_LOG_DIRTY Kconfig option with the following effects
> when disabled:
>
> * paging_domctl{,_cont} return -EOPNOTSUPP (XEN_DOMCTL_shadow_op).
> * VRAM tracking via DMOP returns EOPNOTSUPP.
>
> And compiles out all log-dirty tracking infra.
>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
> RFC for the Kconfig help message.
> ---
> xen/arch/x86/Kconfig | 9 +++++++++
> xen/arch/x86/domctl.c | 8 ++++----
> xen/arch/x86/hvm/dm.c | 3 +++
> xen/arch/x86/include/asm/domain.h | 2 ++
> xen/arch/x86/include/asm/p2m.h | 2 ++
> xen/arch/x86/include/asm/paging.h | 8 +++-----
> xen/arch/x86/mm/hap/hap.c | 22 +++++++++++++---------
> xen/arch/x86/mm/p2m.c | 2 ++
> xen/arch/x86/mm/paging.c | 2 ++
> xen/include/hypercall-defs.c | 4 ++--
> 10 files changed, 42 insertions(+), 20 deletions(-)
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 61f58aa829..fbf044aa4d 100644
> --- 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
I think I'm missing something, why this dependency of shadow on log
dirty? Isn't HAP equally dependent on it?
Oh, I see, You adjust HAP code to deal with the !CONFIG_LOG_DIRTY
scenario, but not the SHADOW counterpart, that still has a hard
dependency on LOG_DIRTY being enabled.
> depends on PV || HVM
> help
>
> @@ -166,6 +167,14 @@ config SHADOW_PAGING
> config PAGING
> def_bool HVM || SHADOW_PAGING
>
> +config LOG_DIRTY
> + bool "Log-dirty page tracking" if EXPERT
> + depends on PAGING
> + default !PV_SHIM_EXCLUSIVE
> + help
> + Enable log-dirty infrastructure so Xen can track domain memory writes and
> + the dirty state of VRAM for device models and live migrations.
> +
> config BIGMEM
> bool "big memory support"
> default n
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index d9521808dc..61d43a21d0 100644
> --- 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;
>
> case XEN_DOMCTL_ioport_permission:
> diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
> index 3b53471af0..94216aecc2 100644
> --- a/xen/arch/x86/hvm/dm.c
> +++ b/xen/arch/x86/hvm/dm.c
> @@ -48,6 +48,9 @@ static int track_dirty_vram(struct domain *d, xen_pfn_t first_pfn,
> unsigned int nr_frames,
> const struct xen_dm_op_buf *buf)
> {
> + if ( !IS_ENABLED(CONFIG_LOG_DIRTY) )
> + return -EOPNOTSUPP;
> +
> if ( nr_frames > (GB(1) >> PAGE_SHIFT) )
> return -EINVAL;
>
> diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
> index 94b0cf7f1d..f09c13909f 100644
> --- 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 */
Can also ifdef out the declaration of the log_dirty_domain struct, and
just provide a forward pointer declaration for it? Would that satisfy
the paging_log_dirty_init() prototype? Hm, I guess that won't work
with the usage in hap_domain_init().
> /* preemption handling */
> struct {
> diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
> index 9016e88411..3c2dcacfa5 100644
> --- a/xen/arch/x86/include/asm/p2m.h
> +++ b/xen/arch/x86/include/asm/p2m.h
> @@ -253,9 +253,11 @@ struct p2m_domain {
> bool *sve);
> int (*recalc)(struct p2m_domain *p2m,
> unsigned long gfn);
> +#ifdef CONFIG_LOG_DIRTY
> void (*enable_hardware_log_dirty)(struct p2m_domain *p2m);
> void (*disable_hardware_log_dirty)(struct p2m_domain *p2m);
> void (*flush_hardware_cached_dirty)(struct p2m_domain *p2m);
> +#endif /* CONFIG_LOG_DIRTY */
Will this build when VMX is enabled? I think it's missing an
adjustment to ept_p2m_init() to not initialize those fields if
!CONFIG_LOG_DIRTY.
> void (*change_entry_type_global)(struct p2m_domain *p2m,
> p2m_type_t ot,
> p2m_type_t nt);
> diff --git a/xen/arch/x86/include/asm/paging.h b/xen/arch/x86/include/asm/paging.h
> index 291ab386e8..980cdfa455 100644
> --- 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)
You want an extra set of parentheses around this define I think, just
in case.
Quite a lot of the code is also protected using #if PG_log_dirty. I
think if you introduce a CONFIG_LOG_DIRTY it would best to also guard
the logdirty code using that new define, so it's consistent (iow:
replace `#if PG_log_dirty` and similar usages).
>
> /* All paging modes. */
> #define PG_MASK (PG_refcounts | PG_log_dirty | PG_translate | PG_external)
> @@ -174,6 +171,7 @@ static inline void paging_log_dirty_init(struct domain *d,
> const struct log_dirty_ops *ops) {}
> static inline void paging_mark_dirty(struct domain *d, mfn_t gmfn) {}
> static inline void paging_mark_pfn_dirty(struct domain *d, pfn_t pfn) {}
> +static inline void paging_mark_pfn_clean(struct domain *d, pfn_t pfn) {}
> static inline bool paging_mfn_is_dirty(struct domain *d, mfn_t gmfn) { return false; }
>
> #endif /* PG_log_dirty */
> diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
> index a337752bf4..21672db011 100644
> --- 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
Nit: I would leave the newline as-is.
> int hap_track_dirty_vram(struct domain *d,
> unsigned long begin_pfn,
> unsigned int nr_frames,
> @@ -161,6 +161,7 @@ out:
>
> return rc;
> }
> +#endif /* CONFIG_LOG_DIRTY */
>
> /************************************************/
> /* HAP LOG DIRTY SUPPORT */
> @@ -440,14 +441,17 @@ static bool cf_check flush_tlb(const unsigned long *vcpu_bitmap);
>
> void hap_domain_init(struct domain *d)
> {
> - static const struct log_dirty_ops hap_ops = {
> - .enable = hap_enable_log_dirty,
> - .disable = hap_disable_log_dirty,
> - .clean = hap_clean_dirty_bitmap,
> - };
> -
> - /* Use HAP logdirty mechanism. */
> - paging_log_dirty_init(d, &hap_ops);
> + if ( IS_ENABLED(CONFIG_LOG_DIRTY) )
> + {
> + static const struct log_dirty_ops hap_ops = {
> + .enable = hap_enable_log_dirty,
> + .disable = hap_disable_log_dirty,
> + .clean = hap_clean_dirty_bitmap,
> + };
> +
> + /* Use HAP logdirty mechanism. */
> + paging_log_dirty_init(d, &hap_ops);
> + }
>
> d->arch.paging.update_paging_modes = hap_update_paging_modes;
> d->arch.paging.flush_tlb = flush_tlb;
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index e915da26a8..373382c28c 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -236,6 +236,7 @@ struct ioreq_server *p2m_get_ioreq_server(struct domain *d,
> return s;
> }
>
> +#ifdef CONFIG_LOG_DIRTY
> void p2m_enable_hardware_log_dirty(struct domain *d)
> {
> struct p2m_domain *p2m = p2m_get_hostp2m(d);
> @@ -263,6 +264,7 @@ void p2m_flush_hardware_cached_dirty(struct domain *d)
> p2m_unlock(p2m);
> }
> }
> +#endif /* CONFIG_LOG_DIRTY */
>
> /*
> * Force a synchronous P2M TLB flush if a deferred flush is pending.
> diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
> index 2396f81ad5..13ee137db9 100644
> --- a/xen/arch/x86/mm/paging.c
> +++ b/xen/arch/x86/mm/paging.c
> @@ -623,10 +623,12 @@ int paging_domain_init(struct domain *d)
> INIT_PAGE_LIST_HEAD(&d->arch.paging.freelist);
> mm_lock_init(&d->arch.paging.lock);
>
> +#ifdef CONFIG_LOG_DIRTY
> /* This must be initialized separately from the rest of the
> * log-dirty init code as that can be called more than once and we
> * don't want to leak any active log-dirty bitmaps */
> d->arch.paging.log_dirty.top = INVALID_MFN;
> +#endif /* CONFIG_LOG_DIRTY */
Could you possibly init this field from paging_log_dirty_init()? As
to avoid having more ifdef churn in paging_domain_init().
Thanks, Roger.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Add Kconfig option for log-dirty tracking
2026-02-09 15:24 ` Alejandro Vallejo
@ 2026-02-09 15:55 ` Jan Beulich
2026-02-10 13:13 ` Alejandro Vallejo
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2026-02-09 15:55 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, Michal Orzel,
Julien Grall, Stefano Stabellini, xen-devel
On 09.02.2026 16:24, Alejandro Vallejo wrote:
> On Mon Feb 9, 2026 at 3:48 PM CET, Jan Beulich wrote:
>> 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?
>
> The logic is rather opaque. I admit I'm a bit fuzzy on the uses of logdirty.
>
> I know what it's for and I could navigate the code if a problem arose, but I'm
> less clear about which other elements of the hypervisor rely on it (pod? nsvm?
> vvmx? shadow? hap?).
>
> If it's strictly toolstack/DM-driven maybe it'd be more apt to have a separate
> LIVE_MIGRATION and SAVE_RESTORE configs where LM selects SAVE_RESTORE, which
> selects LOG_DIRTY. That's also improve some defaults auto-downgraded from the
> max policy just in case a VM is migrated.
It's save (not restore) for both PV and HVM, and VRAM dirty tracking for HVM
only. Ordinary HVM guests will want VRAM tracking, so compiling out support
for it will imo want mentioning in the Kconfig help text.
>>> --- 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().
>
> xl doesn't seem to call it at all. hap_set_allocation() is implicitly called
> through paging_enable() -> hap_enable() -> hap_set_allocation()
xl must be calling it, at least in the case where the paging pool size is
explicitly set in the guest config. The important point is - not all of
XEN_DOMCTL_shadow_op's sub-ops are log-dirty related.
It's also odd that you did make changes at the call site here, but then
left the called function (and its sibling paging_domctl_cont()) in place.
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Add Kconfig option for log-dirty tracking
2026-02-09 15:44 ` Roger Pau Monné
@ 2026-02-09 16:00 ` Jan Beulich
0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2026-02-09 16:00 UTC (permalink / raw)
To: Roger Pau Monné
Cc: xen-devel, Andrew Cooper, Anthony PERARD, Michal Orzel,
Julien Grall, Stefano Stabellini, Alejandro Vallejo
On 09.02.2026 16:44, Roger Pau Monné wrote:
> On Mon, Feb 09, 2026 at 11:31:15AM +0100, Alejandro Vallejo wrote:
>> --- a/xen/arch/x86/mm/paging.c
>> +++ b/xen/arch/x86/mm/paging.c
>> @@ -623,10 +623,12 @@ int paging_domain_init(struct domain *d)
>> INIT_PAGE_LIST_HEAD(&d->arch.paging.freelist);
>> mm_lock_init(&d->arch.paging.lock);
>>
>> +#ifdef CONFIG_LOG_DIRTY
>> /* This must be initialized separately from the rest of the
>> * log-dirty init code as that can be called more than once and we
>> * don't want to leak any active log-dirty bitmaps */
>> d->arch.paging.log_dirty.top = INVALID_MFN;
>> +#endif /* CONFIG_LOG_DIRTY */
>
> Could you possibly init this field from paging_log_dirty_init()? As
> to avoid having more ifdef churn in paging_domain_init().
Hmm, I was wondering the same, but then trusted the comment. Looks like it
may be stale, though.
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Add Kconfig option for log-dirty tracking
2026-02-09 15:55 ` Jan Beulich
@ 2026-02-10 13:13 ` Alejandro Vallejo
2026-02-10 13:21 ` Andrew Cooper
0 siblings, 1 reply; 9+ messages in thread
From: Alejandro Vallejo @ 2026-02-10 13:13 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, Michal Orzel,
Julien Grall, Stefano Stabellini, xen-devel
On Mon Feb 9, 2026 at 4:55 PM CET, Jan Beulich wrote:
> On 09.02.2026 16:24, Alejandro Vallejo wrote:
>> On Mon Feb 9, 2026 at 3:48 PM CET, Jan Beulich wrote:
>>> 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?
>>
>> The logic is rather opaque. I admit I'm a bit fuzzy on the uses of logdirty.
>>
>> I know what it's for and I could navigate the code if a problem arose, but I'm
>> less clear about which other elements of the hypervisor rely on it (pod? nsvm?
>> vvmx? shadow? hap?).
>>
>> If it's strictly toolstack/DM-driven maybe it'd be more apt to have a separate
>> LIVE_MIGRATION and SAVE_RESTORE configs where LM selects SAVE_RESTORE, which
>> selects LOG_DIRTY. That's also improve some defaults auto-downgraded from the
>> max policy just in case a VM is migrated.
>
> It's save (not restore) for both PV and HVM, and VRAM dirty tracking for HVM
> only. Ordinary HVM guests will want VRAM tracking, so compiling out support
> for it will imo want mentioning in the Kconfig help text.
ack.
>
>>>> --- 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().
>>
>> xl doesn't seem to call it at all. hap_set_allocation() is implicitly called
>> through paging_enable() -> hap_enable() -> hap_set_allocation()
>
> xl must be calling it, at least in the case where the paging pool size is
> explicitly set in the guest config. The important point is - not all of
> XEN_DOMCTL_shadow_op's sub-ops are log-dirty related.
>
> It's also odd that you did make changes at the call site here, but then
> left the called function (and its sibling paging_domctl_cont()) in place.
>
> Jan
That goes through DOMCTL_set_paging_mempool_size. With identical behaviour
except it uses a different op. Blergh.
There are no uses for shadow ops other than logdirty that I can see.
libs/guest/xg_sr_save.c: rc = xc_shadow_control(xch, ctx->domid,
libs/guest/xg_sr_save.c- XEN_DOMCTL_SHADOW_OP_ENABLE_LOGDIRTY,
--
libs/guest/xg_sr_save.c: rc = xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF,
libs/guest/xg_sr_save.c- NULL, 0);
--
libs/guest/xg_sr_save.c: rc = xc_shadow_control(xch, ctx->domid,
libs/guest/xg_sr_save.c- XEN_DOMCTL_SHADOW_OP_ENABLE_LOGDIRTY,
--
libs/guest/xg_sr_save.c: xc_shadow_control(xch, ctx->domid, XEN_DOMCTL_SHADOW_OP_OFF,
libs/guest/xg_sr_save.c- NULL, 0);
--
libs/light/libxl_colo_restore.c: if (xc_shadow_control(CTX->xch, domid,
libs/light/libxl_colo_restore.c- XEN_DOMCTL_SHADOW_OP_ENABLE_LOGDIRTY,
--
libs/light/libxl_colo_restore.c: if (xc_shadow_control(CTX->xch, domid, XEN_DOMCTL_SHADOW_OP_OFF,
--
ocaml/libs/xc/xenctrl_stubs.c: ret = xc_shadow_control(xch, Int_val(domid),
ocaml/libs/xc/xenctrl_stubs.c- XEN_DOMCTL_SHADOW_OP_GET_ALLOCATION,
--
ocaml/libs/xc/xenctrl_stubs.c: ret = xc_shadow_control(xch, Int_val(domid),
ocaml/libs/xc/xenctrl_stubs.c- XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION,
Not even for shadow, which seems
There's an ocaml stub, but I can't be bothered to check whether XAPI uses it.
I'll leave around the domctl so XEN_DOMCTL_SHADOW_OP_OFF doesn't return
-EOPNOTSUPP, but set/get allocation should go away at some point before
stabilising the control interfaces.
I'll also remove the bad #if PG_log_dirty gating these, as it really just wants
to be !PV_SHIM_EXCLUSIVE (that's why I originally got rid of it all).
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Add Kconfig option for log-dirty tracking
2026-02-10 13:13 ` Alejandro Vallejo
@ 2026-02-10 13:21 ` Andrew Cooper
2026-02-10 17:42 ` Alejandro Vallejo
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2026-02-10 13:21 UTC (permalink / raw)
To: Alejandro Vallejo, Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, Michal Orzel,
Julien Grall, Stefano Stabellini, xen-devel
On 10/02/2026 1:13 pm, Alejandro Vallejo wrote:
> On Mon Feb 9, 2026 at 4:55 PM CET, Jan Beulich wrote:
>> On 09.02.2026 16:24, Alejandro Vallejo wrote:
>>> On Mon Feb 9, 2026 at 3:48 PM CET, Jan Beulich wrote:
>>>> 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?
>>> The logic is rather opaque. I admit I'm a bit fuzzy on the uses of logdirty.
>>>
>>> I know what it's for and I could navigate the code if a problem arose, but I'm
>>> less clear about which other elements of the hypervisor rely on it (pod? nsvm?
>>> vvmx? shadow? hap?).
>>>
>>> If it's strictly toolstack/DM-driven maybe it'd be more apt to have a separate
>>> LIVE_MIGRATION and SAVE_RESTORE configs where LM selects SAVE_RESTORE, which
>>> selects LOG_DIRTY. That's also improve some defaults auto-downgraded from the
>>> max policy just in case a VM is migrated.
>> It's save (not restore) for both PV and HVM, and VRAM dirty tracking for HVM
>> only. Ordinary HVM guests will want VRAM tracking, so compiling out support
>> for it will imo want mentioning in the Kconfig help text.
> ack.
>
>>>>> --- 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().
>>> xl doesn't seem to call it at all. hap_set_allocation() is implicitly called
>>> through paging_enable() -> hap_enable() -> hap_set_allocation()
>> xl must be calling it, at least in the case where the paging pool size is
>> explicitly set in the guest config. The important point is - not all of
>> XEN_DOMCTL_shadow_op's sub-ops are log-dirty related.
>>
>> It's also odd that you did make changes at the call site here, but then
>> left the called function (and its sibling paging_domctl_cont()) in place.
>>
>> Jan
> That goes through DOMCTL_set_paging_mempool_size.
This DOMCTL was added in an XSA because ARM needed the functionality,
hence no cleanup.
I didn't get around to cleaning up
XEN_DOMCTL_SHADOW_OP_{GET,SET}_ALLOCATION, but please do. We should not
have multiple ways of configuring this, and it simplifies the your patch.
~Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Add Kconfig option for log-dirty tracking
2026-02-10 13:21 ` Andrew Cooper
@ 2026-02-10 17:42 ` Alejandro Vallejo
0 siblings, 0 replies; 9+ messages in thread
From: Alejandro Vallejo @ 2026-02-10 17:42 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich
Cc: Roger Pau Monné, Anthony PERARD, Michal Orzel, Julien Grall,
Stefano Stabellini, xen-devel
On Tue Feb 10, 2026 at 2:21 PM CET, Andrew Cooper wrote:
> On 10/02/2026 1:13 pm, Alejandro Vallejo wrote:
>> On Mon Feb 9, 2026 at 4:55 PM CET, Jan Beulich wrote:
>>> On 09.02.2026 16:24, Alejandro Vallejo wrote:
>>>> On Mon Feb 9, 2026 at 3:48 PM CET, Jan Beulich wrote:
>>>>> 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?
>>>> The logic is rather opaque. I admit I'm a bit fuzzy on the uses of logdirty.
>>>>
>>>> I know what it's for and I could navigate the code if a problem arose, but I'm
>>>> less clear about which other elements of the hypervisor rely on it (pod? nsvm?
>>>> vvmx? shadow? hap?).
>>>>
>>>> If it's strictly toolstack/DM-driven maybe it'd be more apt to have a separate
>>>> LIVE_MIGRATION and SAVE_RESTORE configs where LM selects SAVE_RESTORE, which
>>>> selects LOG_DIRTY. That's also improve some defaults auto-downgraded from the
>>>> max policy just in case a VM is migrated.
>>> It's save (not restore) for both PV and HVM, and VRAM dirty tracking for HVM
>>> only. Ordinary HVM guests will want VRAM tracking, so compiling out support
>>> for it will imo want mentioning in the Kconfig help text.
>> ack.
>>
>>>>>> --- 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().
>>>> xl doesn't seem to call it at all. hap_set_allocation() is implicitly called
>>>> through paging_enable() -> hap_enable() -> hap_set_allocation()
>>> xl must be calling it, at least in the case where the paging pool size is
>>> explicitly set in the guest config. The important point is - not all of
>>> XEN_DOMCTL_shadow_op's sub-ops are log-dirty related.
>>>
>>> It's also odd that you did make changes at the call site here, but then
>>> left the called function (and its sibling paging_domctl_cont()) in place.
>>>
>>> Jan
>> That goes through DOMCTL_set_paging_mempool_size.
>
> This DOMCTL was added in an XSA because ARM needed the functionality,
> hence no cleanup.
>
> I didn't get around to cleaning up
> XEN_DOMCTL_SHADOW_OP_{GET,SET}_ALLOCATION, but please do. We should not
> have multiple ways of configuring this, and it simplifies the your patch.
>
> ~Andrew
I have a separate patch for that, but I won't add it here because it does not
simplify the patch at all. hap_domctl() ought to remain to return EINVAL on
unexpected ops anyway, and there's lots of loose ends to tie (python/ocaml
stubs).
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-02-10 17:43 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-09 10:31 [PATCH] x86: Add Kconfig option for log-dirty tracking Alejandro Vallejo
2026-02-09 14:48 ` Jan Beulich
2026-02-09 15:24 ` Alejandro Vallejo
2026-02-09 15:55 ` Jan Beulich
2026-02-10 13:13 ` Alejandro Vallejo
2026-02-10 13:21 ` Andrew Cooper
2026-02-10 17:42 ` Alejandro Vallejo
2026-02-09 15:44 ` Roger Pau Monné
2026-02-09 16:00 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.