On Thu, Sep 29, 2022 at 11:51:40AM +0200, Jan Beulich wrote:
> Forever sinced its introduction VCPUOP_register_vcpu_time_memory_area
> was available only to native domains. Linux, for example, would attempt
> to use it irrespective of guest bitness (including in its so called
> PVHVM mode) as long as it finds XEN_PVCLOCK_TSC_STABLE_BIT set (which we
> set only for clocksource=tsc, which in turn needs engaging via command
> line option).
>
> Fixes: a5d39947cb89 ("Allow guests to register secondary vcpu_time_info")
> Signed-off-by: Jan Beulich <[email protected]>
Acked-by: Roger Pau Monné <[email protected]>
Albeit I have concerns with the notes you raise below, not sure we
also want to introduce a (broken') compat version of the same
hypercall wrt v != current.
> ---
> Is it actually correct for us to do cross-vCPU updates of the area here
> (and also in the native counterpart as well as for runstate area
> updates)? The virtual address may be valid for the given vCPU only, but
> may be mapped to something else on the current vCPU (yet we only deal
> with it not being mapped at all). Note how HVM code already calls
> update_vcpu_system_time() only when v == current.
>
> I'm surprised by Linux not using the secondary area in a broader
> fashion. But I'm also surprised that they would only ever register an
> area for vCPU 0.
Would be better to update locally just when v == current, otherwise
issue an IPI to the remote vCPU dirty mask and force an update on
resume to guest path?
>
> --- a/xen/arch/x86/x86_64/domain.c
> +++ b/xen/arch/x86/x86_64/domain.c
> @@ -58,6 +58,26 @@ compat_vcpu_op(int cmd, unsigned int vcp
> break;
> }
>
> + case VCPUOP_register_vcpu_time_memory_area:
> + {
> + struct compat_vcpu_register_time_memory_area area = { .addr.p = 0 };
Why not just use { } to initialize?
Thanks, Roger.
