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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 7/20/26 9:19 AM, Jan Beulich wrote:
> 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.
Considering that it leads to crash of Linux I agree that we want to have 
this in 4.22:
 Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

Thanks.

~ Oleksii