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:50 PM, Tony Lindgren wrote:
> On Tue, Sep 22, 2026 at 05:37:33PM -0700, Kishen Maloor wrote:
>> On 9/21/26 11:27 PM, Tony Lindgren wrote:
>>> Oh right thanks. I think this is really the maximum transfer buffer size
>>> Peter asked, not just a hint to userspace :)
>>
>> It is a strict upper bound. Maybe it's semantics, but I called 
>> it a "hint" because allocating for that entire size could be optional.
>> If a userspace driver for say TDX wants to send smaller batches (say 128) then 
>> it can refer to the spec, do the math, and allocate 131 pages and the kernel 
>> should permit that; it's not wrong. If userspace ever allocates less room than 
>> a call requires, it would fail. A different userspace driver could simply 
>> allocate that max size and be done; no need to refer to the spec or do the math. 
>> It is for this second case where I thought returning the upper bound would be 
>> useful. Hence the earlier suggestion.
> 
> OK 
>  
>>>>>>>> +struct kvm_transfer_buffer {
>>>>>>>> +	__u64 address;
>>>>>>>> +	__u32 size;
>>>>>>>> +	__u32 reserved;
>>>>>>>> +};
>>>>>>>
>>>>>>> Should this struct include a 'capacity' field (u32) that is set on each command?
>>>>>>> It would be the number of bytes writable at address.
>>>>>>> size would be the input length on entry (0 if the command passes none), and the
>>>>>>> number of bytes produced on return (0 if none).
>>>>>>
>>>>>> Hmm so the transfer command return value can return how many bytes were
>>>>>> written of the input. But yeah we don't know how many bytes were written
>>>>>> back to the transfer buffer as result of the transfer command.
>>>>
>>>> The transfer command return value could return how many bytes were written into the buffer.
>>>> But in an input-output call, the kernel handler wouldn't know how many bytes it could write,
>>>> or for that matter even how many pages to pin up front in case it needs to return an output
>>>> because 'size' couldn't simultaneously convey the input length and buffer capacity. That was
>>>> the gap that I thought a read-only 'capacity' field could bridge. Of course, this
>>>> assumes that the output is written in-place.
>>>
>>> Hmm yeah this inplace capacity vs transferred issue remains still. So I
>>> agree we need to specify the capacity in struct kvm_transfer_buffer like
>>> you suggested.
>>
>> To be clear, I think the in/out split for the buffers along with the convention 
>> I laid out closes that gap I saw without needing a 'capacity' field.
>> Because out/size could now unambiguously convey capacity on entry and output
>> length on return.
> 
> But for an inplace buffer use with some input data smaller than the output
> data, would it work? To me it seems you need both buffer size and data
> size for that.

Yes, with two kvm_transfer_buffers in the transfer struct, it would work, whether
the call uses a single userspace buffer or two.

>  
>>> To me size is already the size of the buffer though. So instead of changing
>>> size to capacity, how about something like datasize or len for the input
>>> and output transfer length?
>>
>> But I understand that (and please correct me if I'm wrong):
>> a) You'd still prefer to not have 'size' serve that double duty.
>> b) 'size' in your mental model already means buffer capacity.
> 
> Heh yes correct for the above.
>  
>> In that case, we could add a 'datasize' field to convey the length
>> of valid data in the buffer, like this:
>>
>> struct kvm_transfer_buffer {
>>     __u64 address;
>>     __u32 size;
>>     __u32 datasize;
>>     __u64 reserved;
>> };
> 
> Maybe bufsize and datasize? Then the difference would be obvious while
> reading the code.

Sure.

> 
>> The convention then becomes:
>> - A non-zero 'datasize' on 'in' at call entry conveys that there is input.
>> - A non-zero 'datasize' on 'out' at call exit conveys that there is output.
>> - out/datasize on call entry is ignored.
>> - in/size and out/size are seeded with the buffer capacity.
>>
>>>
>>>>>> How about if we add the bytes returned to the transfer struct? Then the
>>>>>> kvm_transfer_buffer can stay as just a buffer. 
>>>>>
>>>>> Actually, for the possible cases with input+output, we could reserve space
>>>>> in the transfer struct for another struct kvm_transfer_buffer for the
>>>>> results?
>>>>
>>>> Yes, say an 'in' and 'out' kvm_transfer_buffer inside struct kvm_migrate_cmd should
>>>> close this out and shouldn't require a 'capacity' field. Maybe we then establish this
>>>> convention:
>>>> - A non-zero 'size' on 'in' at call entry would signal that there is input.
>>>> - A non-zero 'size' on 'out' at call entry would convey the buffer capacity.
>>>> - A non-zero 'size' on 'out' at call exit would convey that there is output.
>>>> - A zeroed 'size' on 'out' at call exit would convey that there is no output.
>>>
>>> Looks doable to me but with the inplace issue as above.. Sounds like we just
>>> need to reserve space for a case with a separate output buffer though.
>>
>> To be clear, this is what I thought we were talking about :)
> 
> Heh yeah we're talking two things with the inplace use vs two buffers :)
> 
>> To add a 2nd kvm_transfer_buffer to kvm_migrate_cmd, like this:
>>
>> struct kvm_migrate_cmd {
>>      __u16 command;
>>      __u16 flags;
>>      __u32 reserved;
>>      struct kvm_transfer_buffer in;
>>      struct kvm_transfer_buffer out;
>> };
>>
>> If there is agreement on this model, then yeah, we'd want to define
>> these fields now, since the struct can't grow later without a new ioctl
>> number.
> 
> Based on what we've discussed, my preference is the following:
> 
> Keep the current buf naming. For the EXPORT/IMPORT type functions the use
> should be obvious from the transfer type. 
> 
> Reserve enough space for a separate output buffer or results buffer or
> whatever it might get called if such a use case ever pops up.

Reserved space can be named later without changing sizeof, so either way
works. I'd mildly prefer to declare the second kvm_transfer_buffer just
because declaring both would settle the second buffer's semantics now. Not
something I'd push hard on though if you prefer reserving.

> Add the datasize to struct kvm_transfer_buffer like you suggested and
> rename size to bufsize.