| Message ID | 20240522001817.619072-19-dwmw2@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleaning up the KVM clock mess | expand |
On 22/05/2024 01:17, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > Commit d98d07ca7e034 ("KVM: x86: update pvclock area conditionally, on > cpu migration") turned an unconditional KVM_REQ_CLOCK_UPDATE into a > conditional one, if either the master clock isn't enabled *or* the vCPU > was not previously scheduled (vcpu->cpu == -1). The commit message doesn't > explain the latter condition, which is specifically for the master clock > case. > > Commit 0061d53daf26f ("KVM: x86: limit difference between kvmclock > updates") later turned that into a KVM_REQ_GLOBAL_CLOCK_UPDATE to avoid > skew between vCPUs. > > In master clock mode there is no need for any of that, regardless of > whether/where this vCPU was previously scheduled. > > Do it only if (!kvm->arch.use_master_clock). > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > arch/x86/kvm/x86.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Paul Durrant <paul@xen.org>
On Wed, May 22, 2024, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > Commit d98d07ca7e034 ("KVM: x86: update pvclock area conditionally, on > cpu migration") turned an unconditional KVM_REQ_CLOCK_UPDATE into a > conditional one, if either the master clock isn't enabled *or* the vCPU > was not previously scheduled (vcpu->cpu == -1). The commit message doesn't > explain the latter condition, which is specifically for the master clock > case. It handles the first load of the vCPU, which is distinctly different from CPU migration. > Commit 0061d53daf26f ("KVM: x86: limit difference between kvmclock > updates") later turned that into a KVM_REQ_GLOBAL_CLOCK_UPDATE to avoid > skew between vCPUs. > > In master clock mode there is no need for any of that, regardless of > whether/where this vCPU was previously scheduled. > > Do it only if (!kvm->arch.use_master_clock). > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > arch/x86/kvm/x86.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 32a873d5ed00..dd53860ca284 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5161,7 +5161,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) > * On a host with synchronized TSC, there is no need to update > * kvmclock on vcpu->cpu migration > */ > - if (!vcpu->kvm->arch.use_master_clock || vcpu->cpu == -1) > + if (!vcpu->kvm->arch.use_master_clock) > kvm_make_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu); I believe this needs to be: if (!vcpu->kvm->arch.use_master_clock) kvm_make_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu); else if (vcpu->cpu == -1) kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu); or alternatively do KVM_REQ_CLOCK_UPDATE during vCPU creation (hotplug?). > if (vcpu->cpu != cpu) > kvm_make_request(KVM_REQ_MIGRATE_TIMER, vcpu); > -- > 2.44.0 >
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 32a873d5ed00..dd53860ca284 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5161,7 +5161,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) * On a host with synchronized TSC, there is no need to update * kvmclock on vcpu->cpu migration */ - if (!vcpu->kvm->arch.use_master_clock || vcpu->cpu == -1) + if (!vcpu->kvm->arch.use_master_clock) kvm_make_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu); if (vcpu->cpu != cpu) kvm_make_request(KVM_REQ_MIGRATE_TIMER, vcpu);