Pass the reference pvclock structure that's used to setup each individual
pvclock as a parameter to kvm_setup_guest_pvclock() as a preparatory step
toward removing kvm_vcpu_arch.hv_clock.
No functional change intended.
Reviewed-by: Paul Durrant <paul@xen.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/x86.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5f3ad13a8ac7..06d27b3cc207 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3116,17 +3116,17 @@ u64 get_kvmclock_ns(struct kvm *kvm)
return data.clock;
}
-static void kvm_setup_guest_pvclock(struct kvm_vcpu *v,
+static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
+ struct kvm_vcpu *vcpu,
struct gfn_to_pfn_cache *gpc,
unsigned int offset,
bool force_tsc_unstable)
{
- struct kvm_vcpu_arch *vcpu = &v->arch;
struct pvclock_vcpu_time_info *guest_hv_clock;
struct pvclock_vcpu_time_info hv_clock;
unsigned long flags;
- memcpy(&hv_clock, &vcpu->hv_clock, sizeof(hv_clock));
+ memcpy(&hv_clock, ref_hv_clock, sizeof(hv_clock));
read_lock_irqsave(&gpc->lock, flags);
while (!kvm_gpc_check(gpc, offset + sizeof(*guest_hv_clock))) {
@@ -3165,7 +3165,7 @@ static void kvm_setup_guest_pvclock(struct kvm_vcpu *v,
kvm_gpc_mark_dirty_in_slot(gpc);
read_unlock_irqrestore(&gpc->lock, flags);
- trace_kvm_pvclock_update(v->vcpu_id, &hv_clock);
+ trace_kvm_pvclock_update(vcpu->vcpu_id, &hv_clock);
}
static int kvm_guest_time_update(struct kvm_vcpu *v)
@@ -3272,18 +3272,18 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
vcpu->hv_clock.flags |= PVCLOCK_GUEST_STOPPED;
vcpu->pvclock_set_guest_stopped_request = false;
}
- kvm_setup_guest_pvclock(v, &vcpu->pv_time, 0, false);
+ kvm_setup_guest_pvclock(&vcpu->hv_clock, v, &vcpu->pv_time, 0, false);
vcpu->hv_clock.flags &= ~PVCLOCK_GUEST_STOPPED;
}
#ifdef CONFIG_KVM_XEN
if (vcpu->xen.vcpu_info_cache.active)
- kvm_setup_guest_pvclock(v, &vcpu->xen.vcpu_info_cache,
+ kvm_setup_guest_pvclock(&vcpu->hv_clock, v, &vcpu->xen.vcpu_info_cache,
offsetof(struct compat_vcpu_info, time),
xen_pvclock_tsc_unstable);
if (vcpu->xen.vcpu_time_info_cache.active)
- kvm_setup_guest_pvclock(v, &vcpu->xen.vcpu_time_info_cache, 0,
+ kvm_setup_guest_pvclock(&vcpu->hv_clock, v, &vcpu->xen.vcpu_time_info_cache, 0,
xen_pvclock_tsc_unstable);
#endif
kvm_hv_setup_tsc_page(v->kvm, &vcpu->hv_clock);
--
2.48.1.362.g079036d154-googOn 01/02/2025 01:38, Sean Christopherson wrote:
> Pass the reference pvclock structure that's used to setup each individual
> pvclock as a parameter to kvm_setup_guest_pvclock() as a preparatory step
> toward removing kvm_vcpu_arch.hv_clock.
>
> No functional change intended.
>
> Reviewed-by: Paul Durrant <paul@xen.org>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/x86.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5f3ad13a8ac7..06d27b3cc207 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3116,17 +3116,17 @@ u64 get_kvmclock_ns(struct kvm *kvm)
> return data.clock;
> }
>
> -static void kvm_setup_guest_pvclock(struct kvm_vcpu *v,
> +static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
> + struct kvm_vcpu *vcpu,
So, here 'v' becomes 'vcpu'
> struct gfn_to_pfn_cache *gpc,
> unsigned int offset,
> bool force_tsc_unstable)
> {
> - struct kvm_vcpu_arch *vcpu = &v->arch;
> struct pvclock_vcpu_time_info *guest_hv_clock;
> struct pvclock_vcpu_time_info hv_clock;
> unsigned long flags;
>
> - memcpy(&hv_clock, &vcpu->hv_clock, sizeof(hv_clock));
> + memcpy(&hv_clock, ref_hv_clock, sizeof(hv_clock));
>
> read_lock_irqsave(&gpc->lock, flags);
> while (!kvm_gpc_check(gpc, offset + sizeof(*guest_hv_clock))) {
> @@ -3165,7 +3165,7 @@ static void kvm_setup_guest_pvclock(struct kvm_vcpu *v,
> kvm_gpc_mark_dirty_in_slot(gpc);
> read_unlock_irqrestore(&gpc->lock, flags);
>
> - trace_kvm_pvclock_update(v->vcpu_id, &hv_clock);
> + trace_kvm_pvclock_update(vcpu->vcpu_id, &hv_clock);
> }
>
> static int kvm_guest_time_update(struct kvm_vcpu *v)
> @@ -3272,18 +3272,18 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
> vcpu->hv_clock.flags |= PVCLOCK_GUEST_STOPPED;
> vcpu->pvclock_set_guest_stopped_request = false;
> }
> - kvm_setup_guest_pvclock(v, &vcpu->pv_time, 0, false);
> + kvm_setup_guest_pvclock(&vcpu->hv_clock, v, &vcpu->pv_time, 0, false);
Yet here an below you still use 'v'. Does this actually compile?
>
> vcpu->hv_clock.flags &= ~PVCLOCK_GUEST_STOPPED;
> }
>
> #ifdef CONFIG_KVM_XEN
> if (vcpu->xen.vcpu_info_cache.active)
> - kvm_setup_guest_pvclock(v, &vcpu->xen.vcpu_info_cache,
> + kvm_setup_guest_pvclock(&vcpu->hv_clock, v, &vcpu->xen.vcpu_info_cache,
> offsetof(struct compat_vcpu_info, time),
> xen_pvclock_tsc_unstable);
> if (vcpu->xen.vcpu_time_info_cache.active)
> - kvm_setup_guest_pvclock(v, &vcpu->xen.vcpu_time_info_cache, 0,
> + kvm_setup_guest_pvclock(&vcpu->hv_clock, v, &vcpu->xen.vcpu_time_info_cache, 0,
> xen_pvclock_tsc_unstable);
> #endif
> kvm_hv_setup_tsc_page(v->kvm, &vcpu->hv_clock);
On 04/02/2025 09:33, Paul Durrant wrote:
> On 01/02/2025 01:38, Sean Christopherson wrote:
>> Pass the reference pvclock structure that's used to setup each individual
>> pvclock as a parameter to kvm_setup_guest_pvclock() as a preparatory step
>> toward removing kvm_vcpu_arch.hv_clock.
>>
>> No functional change intended.
>>
>> Reviewed-by: Paul Durrant <paul@xen.org>
>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>> ---
>> arch/x86/kvm/x86.c | 14 +++++++-------
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 5f3ad13a8ac7..06d27b3cc207 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -3116,17 +3116,17 @@ u64 get_kvmclock_ns(struct kvm *kvm)
>> return data.clock;
>> }
>> -static void kvm_setup_guest_pvclock(struct kvm_vcpu *v,
>> +static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info
>> *ref_hv_clock,
>> + struct kvm_vcpu *vcpu,
>
> So, here 'v' becomes 'vcpu'
>
>> struct gfn_to_pfn_cache *gpc,
>> unsigned int offset,
>> bool force_tsc_unstable)
>> {
>> - struct kvm_vcpu_arch *vcpu = &v->arch;
>> struct pvclock_vcpu_time_info *guest_hv_clock;
>> struct pvclock_vcpu_time_info hv_clock;
>> unsigned long flags;
>> - memcpy(&hv_clock, &vcpu->hv_clock, sizeof(hv_clock));
>> + memcpy(&hv_clock, ref_hv_clock, sizeof(hv_clock));
>> read_lock_irqsave(&gpc->lock, flags);
>> while (!kvm_gpc_check(gpc, offset + sizeof(*guest_hv_clock))) {
>> @@ -3165,7 +3165,7 @@ static void kvm_setup_guest_pvclock(struct
>> kvm_vcpu *v,
>> kvm_gpc_mark_dirty_in_slot(gpc);
>> read_unlock_irqrestore(&gpc->lock, flags);
>> - trace_kvm_pvclock_update(v->vcpu_id, &hv_clock);
>> + trace_kvm_pvclock_update(vcpu->vcpu_id, &hv_clock);
>> }
>> static int kvm_guest_time_update(struct kvm_vcpu *v)
>> @@ -3272,18 +3272,18 @@ static int kvm_guest_time_update(struct
>> kvm_vcpu *v)
>> vcpu->hv_clock.flags |= PVCLOCK_GUEST_STOPPED;
>> vcpu->pvclock_set_guest_stopped_request = false;
>> }
>> - kvm_setup_guest_pvclock(v, &vcpu->pv_time, 0, false);
>> + kvm_setup_guest_pvclock(&vcpu->hv_clock, v, &vcpu->pv_time,
>> 0, false);
>
> Yet here an below you still use 'v'. Does this actually compile?
>
Sorry, my misreading of the patch... this is in caller context so no
problem. The inconsistent naming was misleading me.
Reviewed-by: Paul Durrant <paul@xen.org>
On Tue, Feb 04, 2025, Paul Durrant wrote:
> On 04/02/2025 09:33, Paul Durrant wrote:
> > On 01/02/2025 01:38, Sean Christopherson wrote:
> > > -static void kvm_setup_guest_pvclock(struct kvm_vcpu *v,
> > > +static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info
> > > *ref_hv_clock,
> > > + struct kvm_vcpu *vcpu,
> >
> > So, here 'v' becomes 'vcpu'
> >
> > > struct gfn_to_pfn_cache *gpc,
> > > unsigned int offset,
> > > bool force_tsc_unstable)
> > > {
> > > - struct kvm_vcpu_arch *vcpu = &v->arch;
> > > struct pvclock_vcpu_time_info *guest_hv_clock;
> > > struct pvclock_vcpu_time_info hv_clock;
> > > unsigned long flags;
> > > - memcpy(&hv_clock, &vcpu->hv_clock, sizeof(hv_clock));
> > > + memcpy(&hv_clock, ref_hv_clock, sizeof(hv_clock));
> > > read_lock_irqsave(&gpc->lock, flags);
> > > while (!kvm_gpc_check(gpc, offset + sizeof(*guest_hv_clock))) {
...
> > > @@ -3272,18 +3272,18 @@ static int kvm_guest_time_update(struct
> > > kvm_vcpu *v)
> > > vcpu->hv_clock.flags |= PVCLOCK_GUEST_STOPPED;
> > > vcpu->pvclock_set_guest_stopped_request = false;
> > > }
> > > - kvm_setup_guest_pvclock(v, &vcpu->pv_time, 0, false);
> > > + kvm_setup_guest_pvclock(&vcpu->hv_clock, v, &vcpu->pv_time,
> > > 0, false);
> >
> > Yet here an below you still use 'v'. Does this actually compile?
> >
>
> Sorry, my misreading of the patch... this is in caller context so no
> problem. The inconsistent naming was misleading me.
I feel your pain, the use of "vcpu" for kvm_vcpu_arch in kvm_guest_time_update()
kills me. I forget if David's rework of kvm_guest_time_update() fixes that wart.
If it doesn't, I'll suggest that addition. The only reason I haven't posted a
patch was to avoid a bunch of churn for a rename, but if the function is getting
ripped apart anyways...
© 2016 - 2026 Red Hat, Inc.