On Wed, Feb 16, 2022, Vitaly Kuznetsov wrote:
> Anton Romanov <romanton@xxxxxxxxxx> writes:
>
> > If vcpu has tsc_always_catchup set each request updates pvclock data.
> > KVM_HC_CLOCK_PAIRING consumers such as ptp_kvm_x86 rely on tsc read on
> > host's side and do hypercall inside pvclock_read_retry loop leading to
> > infinite loop in such situation.
> >
> > v2:
> > Added warn
Versioning info goes in the "ignored" section, not the changelog.
> > Signed-off-by: Anton Romanov <romanton@xxxxxxxxxx>
> > ---
This part is ignored by git. Versioning info, and/or any commentary that doesn't
belong in the changelog, for a patch goes here.
> > arch/x86/kvm/x86.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 7131d735b1ef..aaafb46a6048 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -8945,6 +8945,15 @@ static int kvm_pv_clock_pairing(struct kvm_vcpu *vcpu, gpa_t paddr,
> > if (!kvm_get_walltime_and_clockread(&ts, &cycle))
> > return -KVM_EOPNOTSUPP;
> >
> > + /*
> > + * When tsc is in permanent catchup mode guests won't be able to use
> > + * pvclock_read_retry loop to get consistent view of pvclock
> > + */
> > + if (vcpu->arch.tsc_always_catchup) {
> > + pr_warn_ratelimited("KVM_HC_CLOCK_PAIRING not supported if vcpu is in tsc catchup mode\n");
> > + return -KVM_EOPNOTSUPP;
>
> I'm not sure this warn is a good idea. It is guest triggerable and
> 'tsc_always_catchup' is not a bug, it is a perfectly valid situation in
> the configuration when TSC scaling is unavailable. Even ratelimited,
> it's not nice when guests can pollute host's logs.
Agreed. And if we want to alert the user/admin, it'd probably be better do so on
tsc_always_catchup first being set. Doubt it's worth it though, assuming my other
patch to prevent KVM from setting tsc_always_catchup=true on non-ancient hardware
without userspace interaction gets merged.
> Also, EOPNOTSUPP makes it sound like the hypercall is unsupported, I'd
> suggest changing this to KVM_EFAULT.
Eh, it's consistent with the above check though, where KVM returns KVM_EOPNOTSUPP
due to the vclock mode being incompatible. This is more or less the same, it's
just a different "mode". KVM_EFAULT suggests that the guest did something wrong
and/or that the guest can remedy the problem in someway, e.g. by providing a
different address. This issue is purely in the host's domain.
