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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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