[4.22] Re: [PATCH] x86/domctl: restore all registers in arch_{get,set}_info_guest()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.07.2026 02:12, Marek Marczykowski-Górecki wrote:
> Commit 9f892f84c279 ("x86/domctl: Stop using XLAT_cpu_user_regs()")
> converted memcpy() of the cpu_user_regs structure to explicit copy of
> its fields. In the compat case, it intentionally missed few of them,
> named in the commit message. But the 64bit case missed also r8-r15
> registers, which was not intentional. This, at least, caused Linux
> 6.18.x crash when resuming PVH domU.

Oh, wow, what a bad mistake (including by me as the reviewer).

> Fix it by adding missing assignments.
> 
> Fixes: 9f892f84c279 ("x86/domctl: Stop using XLAT_cpu_user_regs()")
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

One remark though (equally applying to the original change):

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1255,6 +1255,14 @@ int arch_set_info_guest(
>  
>      if ( !compat )
>      {
> +        v->arch.user_regs.r15               = c.nat->user_regs.r15;
> +        v->arch.user_regs.r14               = c.nat->user_regs.r14;
> +        v->arch.user_regs.r13               = c.nat->user_regs.r13;
> +        v->arch.user_regs.r12               = c.nat->user_regs.r12;
> +        v->arch.user_regs.r11               = c.nat->user_regs.r11;
> +        v->arch.user_regs.r10               = c.nat->user_regs.r10;
> +        v->arch.user_regs.r9                = c.nat->user_regs.r9;
> +        v->arch.user_regs.r8                = c.nat->user_regs.r8;
>          v->arch.user_regs.rbx               = c.nat->user_regs.rbx;
>          v->arch.user_regs.rcx               = c.nat->user_regs.rcx;
>          v->arch.user_regs.rdx               = c.nat->user_regs.rdx;

Neither here nor ...

> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1481,6 +1481,14 @@ void arch_get_info_guest(struct vcpu *v, 
> vcpu_guest_context_u c)
>      if ( !compat )
>      {
>          /* Backing memory is pre-zeroed. */
> +        c.nat->user_regs.r15               = v->arch.user_regs.r15;
> +        c.nat->user_regs.r14               = v->arch.user_regs.r14;
> +        c.nat->user_regs.r13               = v->arch.user_regs.r13;
> +        c.nat->user_regs.r12               = v->arch.user_regs.r12;
> +        c.nat->user_regs.r11               = v->arch.user_regs.r11;
> +        c.nat->user_regs.r10               = v->arch.user_regs.r10;
> +        c.nat->user_regs.r9                = v->arch.user_regs.r9;
> +        c.nat->user_regs.r8                = v->arch.user_regs.r8;
>          c.nat->user_regs.rbx               = v->arch.user_regs.rbx;
>          c.nat->user_regs.rcx               = v->arch.user_regs.rcx;
>          c.nat->user_regs.rdx               = v->arch.user_regs.rdx;

... here it becomes clear what has determined the order in which fields
are copied. It's neither by register number nor by field order nor
alphabetically. We might do ourselves a (however minor) favor if we used
a clear criteria; likely field order would be best. The more that the
new internal struct cpu_user_regs mirrors field order from the original
external one. Andrew (in particular)?

Also, despite this being an issue in 4.21 already, I think this definitely
wants considering for 4.22.

Jan