Re: [PATCH 00/10] KVM: Add idempotent controls for migrating system counter state

[Paolo Bonzini <pbonzini@redhat.com>]

On 10/06/21 00:04, Oliver Upton wrote:
>> Your approach still needs to use the "quirky" approach to host-initiated
>> MSR_IA32_TSC_ADJUST writes, which write the MSR without affecting the
>> VMCS offset.  This is just a documentation issue.
> 
> My suggested ioctl for the vCPU will still exist, and it will still
> affect the VMCS tsc offset, right? However, we need to do one of the
> following:
> 
> - Stash the guest's MSR_IA32_TSC_ADJUST value in the
> kvm_system_counter_state structure. During
> KVM_SET_SYSTEM_COUNTER_STATE, check to see if the field is valid. If
> so, treat it as a dumb value (i.e. show it to the guest but don't fold
> it into the offset).

Yes, it's already folded into the guestTSC-hostTSC offset that the 
caller provides.

> - Inform userspace that it must still migrate MSR_IA32_TSC_ADJUST, and
> continue to our quirky behavior around host-initiated writes of the
> MSR.
> 
> This is why Maxim's spin migrated a value for IA32_TSC_ADJUST, right?

Yes, so that he could then remove (optionally) the quirk for 
host-initiated writes to the TSC and TSC_ADJUST MSRs.

> Doing so ensures we don't have any guest-observable consequences due
> to our migration of TSC state. To me, adding the guest IA32_TSC_ADJUST
> MSR into the new counter state structure is probably best. No strong
> opinions in either direction on this point, though:)

Either is good for me, since documentation will be very important either 
way.  This is a complex API to use due to the possibility of skewed TSCs.

Just one thing, please don't introduce a new ioctl and use 
KVM_GET_DEVICE_ATTR/KVM_SET_DEVICE_ATTR/KVM_HAS_DEVICE_ATTR.

Christian, based on what Oliver mentions here, it's probably useful for 
s390 to have functionality to get/set kvm->arch.epoch and kvm->arch.epdx 
in addition to the absolute TOD values that you are migrating now.

Paolo

>> 1) In the kernel:
>>
>> * KVM_GET_CLOCK should also return kvmclock_ns - realtime_ns and
>> host_TSC.  It should set two flags in struct kvm_clock_data saying that
>> the respective fields are valid.
>>
>> * KVM_SET_CLOCK checks the flag for kvmclock_ns - realtime_ns.  If set,
>> it looks at the kvmclock_ns - realtime_ns field and disregards the
>> kvmclock_ns field.
>>
>> 2) On the source, userspace will:
>>
>> * per-VM: invoke KVM_GET_CLOCK.  Migrate kvmclock_ns - realtime_ns and
>> kvmclock_ns.  Stash host_TSC for subsequent use.
>>
>> * per-vCPU: retrieve guest_TSC - host_TSC with your new ioctl.  Sum it
>> to the stashed host_TSC value; migrate the resulting value (a guest TSC).
>>
>> 3) On the destination:
>>
>> * per-VM: Pass the migrated kvmclock_ns - realtime_ns to KVM_SET_CLOCK.
>>   Use KVM_GET_CLOCK to get a consistent pair of kvmclock_ns ("newNS"
>> below) and host TSC ("newHostTSC").  Stash them for subsequent use,
>> together with the migrated kvmclock_ns value ("sourceNS") that you
>> haven't used yet.
>>
>> * per-vCPU: using the data of the previous step, and the sourceGuestTSC
>> in the migration stream, compute sourceGuestTSC + (newNS - sourceNS) *
>> freq - newHostTSC.  This is the TSC offset to be passed to your new ioctl.

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm