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

[Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>]

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