Marcelo Tosatti <mtosa...@redhat.com> writes: > On Thu, Mar 18, 2021 at 05:38:00PM +0100, Vitaly Kuznetsov wrote: >> Paolo Bonzini <pbonz...@redhat.com> writes: >> >> > On 18/03/21 17:02, Vitaly Kuznetsov wrote: >> >> KVM doesn't fully support Hyper-V reenlightenment notifications on >> >> migration. In particular, it doesn't support emulating TSC frequency >> >> of the source host by trapping all TSC accesses so unless TSC scaling >> >> is supported on the destination host and KVM_SET_TSC_KHZ succeeds, it >> >> is unsafe to proceed with migration. >> >> >> >> Normally, we only require KVM_SET_TSC_KHZ to succeed when 'user_tsc_khz' >> >> was set and just 'try' KVM_SET_TSC_KHZ without otherwise. >> >> >> >> Introduce a new vmstate section (which is added when the guest has >> >> reenlightenment feature enabled) and add env.tsc_khz to it. We already >> >> have env.tsc_khz packed in 'cpu/tsc_khz' but we don't want to be dependent >> >> on the section order. >> >> >> >> Signed-off-by: Vitaly Kuznetsov <vkuzn...@redhat.com> >> > >> > Could we instead fail to load the reenlightenment section if >> > user_tsc_khz was not set? This seems to be user (well, management) >> > error really, since reenlightenment has to be enabled manually (or with >> > hv-passthrough which blocks migration too). > > Seems to match the strategy of the patchset... > >> Yes, we certainly could do that but what's the added value of >> user_tsc_khz which upper layer will have to set explicitly (probably to >> the tsc frequency of the source host anyway)? > > Yes. I think what happened was "evolution": > > 1) Added support to set tsc frequency (with hardware multiplier) > in KVM, so add -tsc-khz VAL (kHz) option to KVM. > > 2) Scaling is enabled only if -tsc-khz VAL is supplied. > > 3) libvirt switches to using -tsc-khz HVAL, where HVAL it retrieves > from KVM_GET_TSC_KHZ of newly created KVM_CREATE_VM instance. > > It could have been done inside qemu instead. > >> In case we just want to avoid calling KVM_SET_TSC_KHZ twice, we can probably >> achieve that by >> adding a CPU flag or something. > > Avoid calling KVM_SET_TSC_KHZ twice ? Don't see why you would avoid > that. >
Actually, we already do KVM_SET_TSC_KHZ twice, my patch adds just another call for KVM_SET_TSC_KHZ. We already do one call in kvm_arch_put_registers() but we don't propagate errors from it so in case TSC scaling is unsupported, migration still succeeds and this is intentional unless 'tsc-khz' was explicitly specified. When 'tsc-khz' is specified, the error is propageted from kvm_arch_init_vcpu() (second call site). We can also achieve the goal of this patch if we follow Paolo's suggestion: just make 'tsc-khz' a must with reenlightenment. -- Vitaly
