On 28.07.2025 20:40, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kv...@epam.com>
>
> Add domain_{lock,unlock} in the XEN_DOMCTL_setvcpucontext operation
> for protecting arch_set_info_guest.
>
> This aligns with the locking pattern used by other operations that
> modify vCPU state.
>
> Signed-off-by: Mykola Kvach <mykola_kv...@epam.com>
I think this requires more of a description / justification. May I in
particular turn your attention to this comment that we have in x86'es
handling of HVM_PARAM_IDENT_PT (disregard the 1st sentence for the
purpose here):
/*
* Update GUEST_CR3 in each VMCS to point at identity map.
* All foreign updates to guest state must synchronise on
* the domctl_lock.
*/
rc = -ERESTART;
if ( !domctl_lock_acquire(d) )
break;
IOW in particular I'd expect you to explain why holding the domctl
lock isn't sufficient here, and hence what (theoretical?) race it is
you're concerned about. That may in turn clarify whether a Fixes: tag
would actually be appropriate here.
Jan
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -392,7 +392,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
> u_domctl)
> if ( ret == 0 )
> {
> domain_pause(d);
> + domain_lock(d);
> ret = arch_set_info_guest(v, c);
> + domain_unlock(d);
> domain_unpause(d);
>
> if ( ret == -ERESTART )
