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/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 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.

> 
> 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.

> 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 :)