Re: [PATCH 2/2][4.17?] x86: wire up VCPUOP_register_vcpu_time_memory_area for 32-bit guests

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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 <jbeulich@xxxxxxxx>

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

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.