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
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
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>
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
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
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
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
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
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
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
© 2016 - 2026 Red Hat, Inc.