Re: [RFC PATCH v2 2/4] KVM: x86: Add optional KVM_CAP_LIVE_MIGRATION and KVM_MIGRATE_CMD

[Kishen Maloor <kishen.maloor@intel.com>]

On 9/22/26 11:04 PM, Tony Lindgren wrote:
> On Tue, Sep 22, 2026 at 05:38:43PM -0700, Kishen Maloor wrote:
>> On 9/21/26 10:25 PM, Tony Lindgren wrote:
>>> On Mon, Sep 21, 2026 at 08:57:45PM -0700, Kishen Maloor wrote:
>>>> On 9/20/26 11:52 PM, Tony Lindgren wrote:
>>>>> Then for KVM tracking the role, I don't think we need it with the two
>>>>> above. The role tracking can always be added if really needed. Any other
>>>>> opinions on this one?
>>>>
>>>> I do think it's useful for generic KVM to track this role (1 byte).
>>>> - It lets _TRANSFER_ style calls dispatch directly to import or export 
>>>>   callbacks based on the role.
>>>> - Even if we don't adopt the _TRANSFER_ style, it enables generic KVM to
>>>>   reject mismatched calls, e.g., KVM_EXPORT_MEMORY on a destination.
>>>
>>> Having KVM do generic checks on the calls is a good idea. There might be
>>> a simpler way of handling it though. Rather than having KVM track the
>>> migration state, how about we add a function to check for the migration
>>> session state from the vendor code?
>>>
>>> So something like this for the states you suggested earlier:
>>>
>>> enum kvm_lmstate {
>>> 	KVM_LM_NONE,
>>> 	KVM_LM_SOURCE,
>>> 	KVM_LM_DESTINATION,
>>> };
>>>
>>> With something like this to get the state from the vendor code:
>>>
>>> enum kmv_lmstate kvm_arch_get_lmstate(struct kvm *);
>>>
>>> For x86 it would end up calling kvm_x86_call(get_lmstate)(kvm) and for
>>> the TDX specific case tdx_get_lmstate().
>>
>> How is this simpler? It trades one byte in a KVM struct for a new generic
>> enum, a new kvm_arch_get_lmstate(), a new kvm_x86_ops entry, and a vendor
>> implementation per vendor, plus a cross-layer call on every command just
>> to learn the role.
>> Directly checking a stored byte (0=unset/1=src/2=dst) seems simplest, no?
> 
> It would avoid dragging KVM into the "track the migration state" business
> at least for now.
> 
> I guess the question in general is: What does KVM need to do with the
> migration role beyond generic checks on the migration related calls?

I haven't thought of any other uses for it. But if we do want those generic
checks, I'd still lean toward the stored byte.

>  
>>> It would allow KVM to do the generic checks for the migration related
>>> calls you're describing. And having KVM start tracking the state can be
>>> still added later on too if it is needed.
>>
>> I think we'd need to pick one way or the other before the UAPI settles
>> if we want generic KVM to reject mismatched calls. OTOH if we want to
>> defer this generic KVM validation, then yeah, it could be settled later.
>>
>>>
>>>>> The transfer direction is there with the EXPORT/IMPORT naming. Maybe
>>>>> just let's keep that naming for easier readability rather than try to 
>>>>> switch to TRANSFER style naming. No transfer direction flag needed.
>>>>
>>>> I can't say I have a clear preference between EXPORT/IMPORT vs _TRANSFER_.
>>>> If we keep the EXPORT/IMPORT naming, then consistency would arguably call for
>>>> splitting MIGRATE_CMD too, which makes it 6 vs 3 (or 5 vs 3 against the RFC
>>>> as posted):
>>>>
>>>> EXPORT/IMPORT style              _TRANSFER_ style
>>>> KVM_EXPORT_CMD                   KVM_MIGRATE_CMD
>>>> KVM_IMPORT_CMD
>>>> KVM_EXPORT_MEMORY                KVM_TRANSFER_MEMORY
>>>> KVM_IMPORT_MEMORY
>>>> KVM_EXPORT_VCPU                  KVM_TRANSFER_VCPU
>>>> KVM_IMPORT_VCPU            
>>>
>>> Agreed we should split the MIGRATE_CMD too. Probably the number of ioctls
>>> is not and issue compared to following the KVM style and better
>>> readability. So my vote is now on EXPORT/IMPORT style naming.
>>>  
>>>> I guess the main benefit of the _TRANSFER_ style is reduced duplication. 
>>>> Each EXPORT/IMPORT pair takes the same struct and differs only by the role
>>>> that the session already knows. Merging them gives one entry point per call
>>>> type and lets userspace drive both ends from the same call site which could
>>>> be considered a win. It doesn't reduce kernel code though as the top-level
>>>> handler still branches internally on the role.
>>>
>>> Yup not much of a win for the TRANSFER style naming.
>>
>> Yeah, my goal was just to enumerate alternatives for consideration.
>>
>> One other benefit of KVM_EXPORT_CMD/KVM_IMPORT_CMD is that the per-session
>> role is implicitly conveyed - in other words, a successful KVM_EXPORT_CMD/SETUP
>> (for e.g.) would indicate that this is a 'source'. So, we wouldn't require a 'role'
>> field in struct kvm_migrate_cmd to explicitly assert one during SETUP.
> 
> Yes good point with the KVM_EXPORT/IMPORT_CMD, that sounds good to me. 
>  
>>> Trying to summarize again after we sorted out the direction flag issue in
>>> the transfer:
>>>
>>> role            per migration session (cannot change during the migration)
>>> direction       per command, EXPORT/IMPORT
>>> hardware state  set and tracked by vendor specific code
>>
>> The 'role' and 'direction' as you define it above are essentially saying the
>> same thing - a source only invokes the vendor's EXPORT call, a destination only
>> invokes the vendor's IMPORT call, and the role doesn't change over the session.
>> If a destination needs to send data to be consumed by the source, then that
>> still invokes the vendor's EXPORT call.
> 
> Yup.
>  
>>> And checking again against the dmaengine analogy:
>>>
>>> Compared to dmaengine, the migration role is modeled similar to the dma
>>> channel configuration.
>>>
>>> The migration direction with EXPORT/IMPORT is modeled similar to
>>> dmaengine_prep_slave_sg().
>> I'll leave the dmaengine comparison to you, I don't know that API well enough
>> to map it properly :)
> 
> Heh just a sanity check for trying to relate this to something existing.