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

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

[Marek Marczykowski-Górecki]

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.

Fix it by adding missing assignments.

Fixes: 9f892f84c279 ("x86/domctl: Stop using XLAT_cpu_user_regs()")
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 xen/arch/x86/domain.c | 8 ++++++++
 xen/arch/x86/domctl.c | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 1d458f1372e5..ba7244ccc513 100644
--- 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;
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 07f712a0a40d..3ea4c650eee0 100644
--- 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;
-- 
2.54.0

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

[Jan Beulich]

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@invisiblethingslab.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

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

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

[Oleksii Kurochko]

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@invisiblethingslab.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> 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@gmail.com>

Thanks.

~ Oleksii

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

[Andrew Cooper]

On 20/07/2026 8: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).

Yes, I'm very embarrassed by this mistake.  It also highlights a serious
gap in testing which we need to address.  We've managed a release and a
half with PV migration plain broken before figuring out why.

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

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> 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)?

They are the order that XLAT_cpu_user_regs() expanded in; that is -
x86_32's cpu_user_regs field order.

... which now I think about it is correct for one of the two uses of
XLAT_cpu_user_regs() but suboptimal for the other.

They all want to be field-order of the loading side, because that way
we're working with the prefetcher rather than against it.

I'm making a follow-up, because I think it's more complicated than just
interleaving these lines, and I suggest we take Marek's patch as-is.

~Andrew

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

[Teddy Astie]

[-- Attachment #1.1.1: Type: text/plain, Size: 714 bytes --]

Le 20/07/2026 à 02:16, Marek Marczykowski-Górecki a écrit :
> 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.
> 
> Fix it by adding missing assignments.
> 
> Fixes: 9f892f84c279 ("x86/domctl: Stop using XLAT_cpu_user_regs()")
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Teddy Astie <teddy.astie@vates.tech>

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 2489 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

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

[Jan Beulich]

On 20.07.2026 10:48, Oleksii Kurochko wrote:
> 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@gmail.com>

Btw, I'm transcribing this to Release-acked-by: while preparing to commit.

Jan

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

[Oleksii Kurochko]

On 7/20/26 11:11 AM, Jan Beulich wrote:
> On 20.07.2026 10:48, Oleksii Kurochko wrote:
>> 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@gmail.com>
> 
> Btw, I'm transcribing this to Release-acked-by: while preparing to commit.

Thanks. I missed to add Release-acked-by:

~ Oleksii

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

[Frediano Ziglio]

On Mon, 20 Jul 2026 at 01:13, Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> 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.
>
> Fix it by adding missing assignments.
>
> Fixes: 9f892f84c279 ("x86/domctl: Stop using XLAT_cpu_user_regs()")
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  xen/arch/x86/domain.c | 8 ++++++++
>  xen/arch/x86/domctl.c | 8 ++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 1d458f1372e5..ba7244ccc513 100644
> --- 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;
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 07f712a0a40d..3ea4c650eee0 100644
> --- 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;

Reviewed-by: Frediano Ziglio <frediano.ziglio@citrix.com>

Frediano

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

[Frediano Ziglio]

On Mon, 20 Jul 2026 at 10:09, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 20/07/2026 8: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).
>
> Yes, I'm very embarrassed by this mistake.  It also highlights a serious
> gap in testing which we need to address.  We've managed a release and a
> half with PV migration plain broken before figuring out why.
>
...

Is migration tested by CI or just for some reason this was not captured?

Frediano

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

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]

On Mon, Jul 20, 2026 at 10:59:00AM +0100, Frediano Ziglio wrote:
> On Mon, 20 Jul 2026 at 10:09, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >
> > On 20/07/2026 8: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).
> >
> > Yes, I'm very embarrassed by this mistake.  It also highlights a serious
> > gap in testing which we need to address.  We've managed a release and a
> > half with PV migration plain broken before figuring out why.
> >
> ...
> 
> Is migration tested by CI or just for some reason this was not captured?

It was in osstest, but it isn't in gitlab. I'll cleanup my current test
and submit it, but I'd really appreciate committing the (reviewed/acked)
test-artifacts series[1], as some of the pending tests depend on it
(although the suspend one doesn't).

[1] https://lore.kernel.org/xen-devel/ah24pfWb_orPRaJG@mail-itl/

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]