On Wed, May 03, 2023 at 05:58:01PM +0200, Jan Beulich wrote:
> The registration by virtual/linear address has downsides: The access is
> expensive for HVM/PVH domains. Furthermore for 64-bit PV domains the area
> is inaccessible (and hence cannot be updated by Xen) when in guest-user
> mode.
>
> Introduce a new vCPU operation allowing to register the secondary time
> area by guest-physical address.
>
> An at least theoretical downside to using physically registered areas is
> that PV then won't see dirty (and perhaps also accessed) bits set in its
> respective page table entries.
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Roger Pau Monné <roger....@citrix.com>
> ---
> v3: Re-base.
> v2: Forge version in force_update_secondary_system_time().
>
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1504,6 +1504,15 @@ int arch_vcpu_reset(struct vcpu *v)
> return 0;
> }
>
> +static void cf_check
> +time_area_populate(void *map, struct vcpu *v)
> +{
> + if ( is_pv_vcpu(v) )
> + v->arch.pv.pending_system_time.version = 0;
> +
> + force_update_secondary_system_time(v, map);
> +}
> +
> long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void)
> arg)
> {
> long rc = 0;
> @@ -1541,6 +1550,25 @@ long do_vcpu_op(int cmd, unsigned int vc
>
> break;
> }
> +
> + case VCPUOP_register_vcpu_time_phys_area:
> + {
> + struct vcpu_register_time_memory_area area;
> +
> + rc = -EFAULT;
> + if ( copy_from_guest(&area.addr.p, arg, 1) )
> + break;
> +
> + rc = map_guest_area(v, area.addr.p,
> + sizeof(vcpu_time_info_t),
> + &v->arch.time_guest_area,
> + time_area_populate);
> + if ( rc == -ERESTART )
> + rc = hypercall_create_continuation(__HYPERVISOR_vcpu_op, "iih",
> + cmd, vcpuid, arg);
> +
> + break;
> + }
>
> case VCPUOP_get_physid:
> {
> --- a/xen/arch/x86/include/asm/domain.h
> +++ b/xen/arch/x86/include/asm/domain.h
> @@ -692,6 +692,8 @@ void domain_cpu_policy_changed(struct do
>
> bool update_secondary_system_time(struct vcpu *,
> struct vcpu_time_info *);
> +void force_update_secondary_system_time(struct vcpu *,
> + struct vcpu_time_info *);
>
> void vcpu_show_registers(const struct vcpu *);
>
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1633,6 +1633,16 @@ void force_update_vcpu_system_time(struc
> __update_vcpu_system_time(v, 1);
> }
>
> +void force_update_secondary_system_time(struct vcpu *v,
> + struct vcpu_time_info *map)
> +{
> + struct vcpu_time_info u;
> +
> + collect_time_info(v, &u);
> + u.version = -1; /* Compensate for version_update_end(). */
Hm, we do not seem to compensate in
VCPUOP_register_vcpu_time_memory_area, what's more, in that case the
initial version is picked from the contents of the guest address.
Hopefully the guest will have zeroed the memory.
FWIW, I would be fine with leaving this at 0, so the first version
guest sees is 1.
Thanks, Roger.
