On 12/14/20 8:39 AM, David Woodhouse wrote:
> From: Joao Martins <joao.m.martins@xxxxxxxxxx>
>
> Parameterise kvm_setup_pvclock_page() a little bit so that it can be
> invoked for different gfn_to_hva_cache structures, and with different
> offsets. Then we can invoke it for the normal KVM pvclock and also for
> the Xen one in the vcpu_info.
>
> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
> Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
> ---
> arch/x86/kvm/x86.c | 31 ++++++++++++++++++-------------
> arch/x86/kvm/xen.c | 4 ++++
> 2 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 64016443159c..cbdc05bb53bd 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2582,13 +2582,15 @@ u64 get_kvmclock_ns(struct kvm *kvm)
> return ret;
> }
>
> -static void kvm_setup_pvclock_page(struct kvm_vcpu *v)
> +static void kvm_setup_pvclock_page(struct kvm_vcpu *v,
> + struct gfn_to_hva_cache *cache,
> + unsigned int offset)
> {
> struct kvm_vcpu_arch *vcpu = &v->arch;
> struct pvclock_vcpu_time_info guest_hv_clock;
>
> - if (unlikely(kvm_read_guest_cached(v->kvm, &vcpu->pv_time,
> - &guest_hv_clock, sizeof(guest_hv_clock))))
> + if (unlikely(kvm_read_guest_offset_cached(v->kvm, cache,
> + &guest_hv_clock, offset, sizeof(guest_hv_clock))))
> return;
>
> /* This VCPU is paused, but it's legal for a guest to read another
> @@ -2611,9 +2613,9 @@ static void kvm_setup_pvclock_page(struct kvm_vcpu *v)
> ++guest_hv_clock.version; /* first time write, random junk */
>
> vcpu->hv_clock.version = guest_hv_clock.version + 1;
> - kvm_write_guest_cached(v->kvm, &vcpu->pv_time,
> - &vcpu->hv_clock,
> - sizeof(vcpu->hv_clock.version));
> + kvm_write_guest_offset_cached(v->kvm, cache,
> + &vcpu->hv_clock, offset,
> + sizeof(vcpu->hv_clock.version));
>
> smp_wmb();
>
> @@ -2627,16 +2629,16 @@ static void kvm_setup_pvclock_page(struct kvm_vcpu *v)
>
> trace_kvm_pvclock_update(v->vcpu_id, &vcpu->hv_clock);
>
> - kvm_write_guest_cached(v->kvm, &vcpu->pv_time,
> - &vcpu->hv_clock,
> - sizeof(vcpu->hv_clock));
> + kvm_write_guest_offset_cached(v->kvm, cache,
> + &vcpu->hv_clock, offset,
> + sizeof(vcpu->hv_clock));
>
> smp_wmb();
>
> vcpu->hv_clock.version++;
> - kvm_write_guest_cached(v->kvm, &vcpu->pv_time,
> - &vcpu->hv_clock,
> - sizeof(vcpu->hv_clock.version));
> + kvm_write_guest_offset_cached(v->kvm, cache,
> + &vcpu->hv_clock, offset,
> + sizeof(vcpu->hv_clock.version));
> }
>
> static int kvm_guest_time_update(struct kvm_vcpu *v)
> @@ -2723,7 +2725,10 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
> vcpu->hv_clock.flags = pvclock_flags;
>
> if (vcpu->pv_time_enabled)
> - kvm_setup_pvclock_page(v);
> + kvm_setup_pvclock_page(v, &vcpu->pv_time, 0);
> + if (vcpu->xen.vcpu_info_set)
> + kvm_setup_pvclock_page(v, &vcpu->xen.vcpu_info_cache,
> + offsetof(struct compat_vcpu_info, time));
We might be missing the case where only shared_info is registered. Something like:
if (vcpu->xen.shinfo_set && !vcpu->xen.vcpu_info_set) {
offset = offsetof(struct compat_vcpu_info, time);
offset += offsetof(struct shared_info, vcpu_info);
offset += (v - kvm_get_vcpu_by_id(0)) * sizeof(struct compat_vcpu_info);
kvm_setup_pvclock_page(v, &vcpu->xen.shinfo_cache, offset);
}
Part of the reason I had a kvm_xen_setup_pvclock_page() was to handle this all these
combinations i.e. 1) shared_info && !vcpu_info 2) vcpu_info and unilaterially updating
secondary time info.
But maybe introducing this xen_vcpu_info() helper to accommodate all this.
> if (v == kvm_get_vcpu(v->kvm, 0))
> kvm_hv_setup_tsc_page(v->kvm, &vcpu->hv_clock);
> return 0;
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 4bc72e0b9928..d2055b60fdc1 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -82,6 +82,9 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
> /* No compat necessary here. */
> BUILD_BUG_ON(sizeof(struct vcpu_info) !=
> sizeof(struct compat_vcpu_info));
> + BUILD_BUG_ON(offsetof(struct vcpu_info, time) !=
> + offsetof(struct compat_vcpu_info, time));
> +
> r = kvm_gfn_to_hva_cache_init(kvm, &v->arch.xen.vcpu_info_cache,
> data->u.vcpu_attr.gpa,
> sizeof(struct vcpu_info));
> @@ -89,6 +92,7 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
> return r;
>
> v->arch.xen.vcpu_info_set = true;
> + kvm_make_request(KVM_REQ_CLOCK_UPDATE, v);
> break;
>
> default:
>
