* [PATCH] x86-64/Xen: fix stack switching
@ 2018-05-07 11:55 Jan Beulich
2018-05-08 2:38 ` Andy Lutomirski
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: Jan Beulich @ 2018-05-07 11:55 UTC (permalink / raw)
To: mingo, tglx, hpa
Cc: Andy Lutomirski, xen-devel, Boris Ostrovsky, Juergen Gross, linux-kernel
While on native entry into the kernel happens on the trampoline stack,
PV Xen kernels are being entered with the current thread stack right
away. Hence source and destination stacks are identical in that case,
and special care is needed.
Other than in sync_regs() the copying done on the INT80 path as well as
on the NMI path itself isn't NMI / #MC safe, as either of these events
occurring in the middle of the stack copying would clobber data on the
(source) stack. (Of course, in the NMI case only #MC could break
things.)
I'm not altering the similar code in interrupt_entry(), as that code
path is unreachable when running an PV Xen guest afaict.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: stable@kernel.org
---
There would certainly have been the option of using alternatives
patching, but afaict the patching code isn't NMI / #MC safe, so I'd
rather stay away from patching the NMI path. And I thought it would be
better to use similar code in both cases.
Another option would be to make the Xen case match the native one, by
going through the trampoline stack, but to me this would look like extra
overhead for no gain.
---
arch/x86/entry/entry_64.S | 8 ++++++++
arch/x86/entry/entry_64_compat.S | 8 +++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
--- 4.17-rc4/arch/x86/entry/entry_64.S
+++ 4.17-rc4-x86_64-stack-switch-Xen/arch/x86/entry/entry_64.S
@@ -1399,6 +1399,12 @@ ENTRY(nmi)
swapgs
cld
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
+
+ movq PER_CPU_VAR(cpu_current_top_of_stack), %rdx
+ subq $8, %rdx
+ xorq %rsp, %rdx
+ shrq $PAGE_SHIFT, %rdx
+ jz .Lnmi_keep_stack
movq %rsp, %rdx
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
UNWIND_HINT_IRET_REGS base=%rdx offset=8
@@ -1408,6 +1414,8 @@ ENTRY(nmi)
pushq 2*8(%rdx) /* pt_regs->cs */
pushq 1*8(%rdx) /* pt_regs->rip */
UNWIND_HINT_IRET_REGS
+.Lnmi_keep_stack:
+
pushq $-1 /* pt_regs->orig_ax */
PUSH_AND_CLEAR_REGS rdx=(%rdx)
ENCODE_FRAME_POINTER
--- 4.17-rc4/arch/x86/entry/entry_64_compat.S
+++ 4.17-rc4-x86_64-stack-switch-Xen/arch/x86/entry/entry_64_compat.S
@@ -356,15 +356,21 @@ ENTRY(entry_INT80_compat)
/* Need to switch before accessing the thread stack. */
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
+
+ movq PER_CPU_VAR(cpu_current_top_of_stack), %rdi
+ subq $8, %rdi
+ xorq %rsp, %rdi
+ shrq $PAGE_SHIFT, %rdi
+ jz .Lint80_keep_stack
movq %rsp, %rdi
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
-
pushq 6*8(%rdi) /* regs->ss */
pushq 5*8(%rdi) /* regs->rsp */
pushq 4*8(%rdi) /* regs->eflags */
pushq 3*8(%rdi) /* regs->cs */
pushq 2*8(%rdi) /* regs->ip */
pushq 1*8(%rdi) /* regs->orig_ax */
+.Lint80_keep_stack:
pushq (%rdi) /* pt_regs->di */
pushq %rsi /* pt_regs->si */
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64/Xen: fix stack switching
2018-05-07 11:55 [PATCH] x86-64/Xen: fix stack switching Jan Beulich
2018-05-08 2:38 ` Andy Lutomirski
@ 2018-05-08 2:38 ` Andy Lutomirski
2018-05-14 10:28 ` Jan Beulich
` (2 more replies)
[not found] ` <5AF03EBD02000000000F91D6@prv1-mh.provo.novell.com>
2 siblings, 3 replies; 22+ messages in thread
From: Andy Lutomirski @ 2018-05-08 2:38 UTC (permalink / raw)
To: Jan Beulich
Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Lutomirski,
xen-devel, Boris Ostrovsky, Juergen Gross, LKML
On Mon, May 7, 2018 at 5:16 AM Jan Beulich <JBeulich@suse.com> wrote:
> While on native entry into the kernel happens on the trampoline stack,
> PV Xen kernels are being entered with the current thread stack right
> away. Hence source and destination stacks are identical in that case,
> and special care is needed.
> Other than in sync_regs() the copying done on the INT80 path as well as
> on the NMI path itself isn't NMI / #MC safe, as either of these events
> occurring in the middle of the stack copying would clobber data on the
> (source) stack. (Of course, in the NMI case only #MC could break
> things.)
I think I'd rather fix this by changing the stack switch code or
alternativing around it on non-stack-switching kernels. Or make Xen use a
trampoline stack just like native.
> I'm not altering the similar code in interrupt_entry(), as that code
> path is unreachable when running an PV Xen guest afaict.
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: stable@kernel.org
> ---
> There would certainly have been the option of using alternatives
> patching, but afaict the patching code isn't NMI / #MC safe, so I'd
> rather stay away from patching the NMI path. And I thought it would be
> better to use similar code in both cases.
I would hope we do the patching before we enable any NMIs.
> Another option would be to make the Xen case match the native one, by
> going through the trampoline stack, but to me this would look like extra
> overhead for no gain.
Avoiding even more complexity in the nmi code seems like a big gain to me.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64/Xen: fix stack switching
2018-05-07 11:55 [PATCH] x86-64/Xen: fix stack switching Jan Beulich
@ 2018-05-08 2:38 ` Andy Lutomirski
2018-05-08 2:38 ` Andy Lutomirski
[not found] ` <5AF03EBD02000000000F91D6@prv1-mh.provo.novell.com>
2 siblings, 0 replies; 22+ messages in thread
From: Andy Lutomirski @ 2018-05-08 2:38 UTC (permalink / raw)
To: Jan Beulich
Cc: Juergen Gross, LKML, Thomas Gleixner, Andrew Lutomirski,
H. Peter Anvin, xen-devel, Boris Ostrovsky, Ingo Molnar
On Mon, May 7, 2018 at 5:16 AM Jan Beulich <JBeulich@suse.com> wrote:
> While on native entry into the kernel happens on the trampoline stack,
> PV Xen kernels are being entered with the current thread stack right
> away. Hence source and destination stacks are identical in that case,
> and special care is needed.
> Other than in sync_regs() the copying done on the INT80 path as well as
> on the NMI path itself isn't NMI / #MC safe, as either of these events
> occurring in the middle of the stack copying would clobber data on the
> (source) stack. (Of course, in the NMI case only #MC could break
> things.)
I think I'd rather fix this by changing the stack switch code or
alternativing around it on non-stack-switching kernels. Or make Xen use a
trampoline stack just like native.
> I'm not altering the similar code in interrupt_entry(), as that code
> path is unreachable when running an PV Xen guest afaict.
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: stable@kernel.org
> ---
> There would certainly have been the option of using alternatives
> patching, but afaict the patching code isn't NMI / #MC safe, so I'd
> rather stay away from patching the NMI path. And I thought it would be
> better to use similar code in both cases.
I would hope we do the patching before we enable any NMIs.
> Another option would be to make the Xen case match the native one, by
> going through the trampoline stack, but to me this would look like extra
> overhead for no gain.
Avoiding even more complexity in the nmi code seems like a big gain to me.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64/Xen: fix stack switching
2018-05-08 2:38 ` Andy Lutomirski
2018-05-14 10:28 ` Jan Beulich
@ 2018-05-14 10:28 ` Jan Beulich
[not found] ` <5AF964B302000078001C26BC@suse.com>
2 siblings, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2018-05-14 10:28 UTC (permalink / raw)
To: Andrew Lutomirski
Cc: mingo, tglx, xen-devel, Boris Ostrovsky, Juergen Gross,
linux-kernel, hpa
>>> On 08.05.18 at 04:38, <luto@kernel.org> wrote:
> On Mon, May 7, 2018 at 5:16 AM Jan Beulich <JBeulich@suse.com> wrote:
>
>> While on native entry into the kernel happens on the trampoline stack,
>> PV Xen kernels are being entered with the current thread stack right
>> away. Hence source and destination stacks are identical in that case,
>> and special care is needed.
>
>> Other than in sync_regs() the copying done on the INT80 path as well as
>> on the NMI path itself isn't NMI / #MC safe, as either of these events
>> occurring in the middle of the stack copying would clobber data on the
>> (source) stack. (Of course, in the NMI case only #MC could break
>> things.)
>
> I think I'd rather fix this by changing the stack switch code or
Well, isn't that what I'm doing in the patch?
> alternativing around it on non-stack-switching kernels.
Fine with me if that's considered better than adding the conditionals.
> Or make Xen use a trampoline stack just like native.
Well, as said I'd rather not, unless x86 and Xen maintainers agree
that's the way to go. But see below for NMI.
>> I'm not altering the similar code in interrupt_entry(), as that code
>> path is unreachable when running an PV Xen guest afaict.
>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Cc: stable@kernel.org
>> ---
>> There would certainly have been the option of using alternatives
>> patching, but afaict the patching code isn't NMI / #MC safe, so I'd
>> rather stay away from patching the NMI path. And I thought it would be
>> better to use similar code in both cases.
>
> I would hope we do the patching before we enable any NMIs.
"Enable NMIs"? I don't think they're getting disabled anywhere in the
kernel. Perhaps you merely mean ones the kernel sends itself (which
I agree would hopefully only be enabled after alternatives patching?
>> Another option would be to make the Xen case match the native one, by
>> going through the trampoline stack, but to me this would look like extra
>> overhead for no gain.
>
> Avoiding even more complexity in the nmi code seems like a big gain to me.
I'm not sure the added conditional is more complexity than making Xen
switch to the trampoline stack just to switch back almost immediately.
But yes, I could see complexity of the NMI code to be a reason to use
different solutions on the NMI and INT80 paths. It's just that I'd like
you, the x86 maintainters, and the Xen ones to agree on which solution
to use where before I'd send a v2.
Jan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64/Xen: fix stack switching
2018-05-08 2:38 ` Andy Lutomirski
@ 2018-05-14 10:28 ` Jan Beulich
2018-05-14 10:28 ` Jan Beulich
[not found] ` <5AF964B302000078001C26BC@suse.com>
2 siblings, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2018-05-14 10:28 UTC (permalink / raw)
To: Andrew Lutomirski
Cc: Juergen Gross, linux-kernel, tglx, hpa, xen-devel,
Boris Ostrovsky, mingo
>>> On 08.05.18 at 04:38, <luto@kernel.org> wrote:
> On Mon, May 7, 2018 at 5:16 AM Jan Beulich <JBeulich@suse.com> wrote:
>
>> While on native entry into the kernel happens on the trampoline stack,
>> PV Xen kernels are being entered with the current thread stack right
>> away. Hence source and destination stacks are identical in that case,
>> and special care is needed.
>
>> Other than in sync_regs() the copying done on the INT80 path as well as
>> on the NMI path itself isn't NMI / #MC safe, as either of these events
>> occurring in the middle of the stack copying would clobber data on the
>> (source) stack. (Of course, in the NMI case only #MC could break
>> things.)
>
> I think I'd rather fix this by changing the stack switch code or
Well, isn't that what I'm doing in the patch?
> alternativing around it on non-stack-switching kernels.
Fine with me if that's considered better than adding the conditionals.
> Or make Xen use a trampoline stack just like native.
Well, as said I'd rather not, unless x86 and Xen maintainers agree
that's the way to go. But see below for NMI.
>> I'm not altering the similar code in interrupt_entry(), as that code
>> path is unreachable when running an PV Xen guest afaict.
>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Cc: stable@kernel.org
>> ---
>> There would certainly have been the option of using alternatives
>> patching, but afaict the patching code isn't NMI / #MC safe, so I'd
>> rather stay away from patching the NMI path. And I thought it would be
>> better to use similar code in both cases.
>
> I would hope we do the patching before we enable any NMIs.
"Enable NMIs"? I don't think they're getting disabled anywhere in the
kernel. Perhaps you merely mean ones the kernel sends itself (which
I agree would hopefully only be enabled after alternatives patching?
>> Another option would be to make the Xen case match the native one, by
>> going through the trampoline stack, but to me this would look like extra
>> overhead for no gain.
>
> Avoiding even more complexity in the nmi code seems like a big gain to me.
I'm not sure the added conditional is more complexity than making Xen
switch to the trampoline stack just to switch back almost immediately.
But yes, I could see complexity of the NMI code to be a reason to use
different solutions on the NMI and INT80 paths. It's just that I'd like
you, the x86 maintainters, and the Xen ones to agree on which solution
to use where before I'd send a v2.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64/Xen: fix stack switching
[not found] ` <5AF964B302000078001C26BC@suse.com>
@ 2018-05-14 12:08 ` Juergen Gross
2018-05-14 12:08 ` Juergen Gross
1 sibling, 0 replies; 22+ messages in thread
From: Juergen Gross @ 2018-05-14 12:08 UTC (permalink / raw)
To: Jan Beulich, Andrew Lutomirski
Cc: Ingo Molnar, Thomas Gleixner, xen-devel, Boris Ostrovsky, lkml,
H. Peter Anvin
On 14/05/18 12:28, Jan Beulich wrote:
>>>> On 08.05.18 at 04:38, <luto@kernel.org> wrote:
>> On Mon, May 7, 2018 at 5:16 AM Jan Beulich <JBeulich@suse.com> wrote:
>>
>>> While on native entry into the kernel happens on the trampoline stack,
>>> PV Xen kernels are being entered with the current thread stack right
>>> away. Hence source and destination stacks are identical in that case,
>>> and special care is needed.
>>
>>> Other than in sync_regs() the copying done on the INT80 path as well as
>>> on the NMI path itself isn't NMI / #MC safe, as either of these events
>>> occurring in the middle of the stack copying would clobber data on the
>>> (source) stack. (Of course, in the NMI case only #MC could break
>>> things.)
>>
>> I think I'd rather fix this by changing the stack switch code or
>
> Well, isn't that what I'm doing in the patch?
>
>> alternativing around it on non-stack-switching kernels.
>
> Fine with me if that's considered better than adding the conditionals.
>
>> Or make Xen use a trampoline stack just like native.
>
> Well, as said I'd rather not, unless x86 and Xen maintainers agree
> that's the way to go. But see below for NMI.
I'd prefer not using a trampoline stack, too.
>
>>> I'm not altering the similar code in interrupt_entry(), as that code
>>> path is unreachable when running an PV Xen guest afaict.
>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> Cc: stable@kernel.org
>>> ---
>>> There would certainly have been the option of using alternatives
>>> patching, but afaict the patching code isn't NMI / #MC safe, so I'd
>>> rather stay away from patching the NMI path. And I thought it would be
>>> better to use similar code in both cases.
>>
>> I would hope we do the patching before we enable any NMIs.
>
> "Enable NMIs"? I don't think they're getting disabled anywhere in the
> kernel. Perhaps you merely mean ones the kernel sends itself (which
> I agree would hopefully only be enabled after alternatives patching?
>
>>> Another option would be to make the Xen case match the native one, by
>>> going through the trampoline stack, but to me this would look like extra
>>> overhead for no gain.
>>
>> Avoiding even more complexity in the nmi code seems like a big gain to me.
>
> I'm not sure the added conditional is more complexity than making Xen
> switch to the trampoline stack just to switch back almost immediately.
I agree.
> But yes, I could see complexity of the NMI code to be a reason to use
> different solutions on the NMI and INT80 paths. It's just that I'd like
> you, the x86 maintainters, and the Xen ones to agree on which solution
> to use where before I'd send a v2.
With my Xen maintainer hat on I'd prefer Jan's current solution.
Juergen
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] x86-64/Xen: fix stack switching
[not found] ` <5AF964B302000078001C26BC@suse.com>
2018-05-14 12:08 ` Juergen Gross
@ 2018-05-14 12:08 ` Juergen Gross
1 sibling, 0 replies; 22+ messages in thread
From: Juergen Gross @ 2018-05-14 12:08 UTC (permalink / raw)
To: Jan Beulich, Andrew Lutomirski
Cc: lkml, Thomas Gleixner, H. Peter Anvin, xen-devel,
Boris Ostrovsky, Ingo Molnar
On 14/05/18 12:28, Jan Beulich wrote:
>>>> On 08.05.18 at 04:38, <luto@kernel.org> wrote:
>> On Mon, May 7, 2018 at 5:16 AM Jan Beulich <JBeulich@suse.com> wrote:
>>
>>> While on native entry into the kernel happens on the trampoline stack,
>>> PV Xen kernels are being entered with the current thread stack right
>>> away. Hence source and destination stacks are identical in that case,
>>> and special care is needed.
>>
>>> Other than in sync_regs() the copying done on the INT80 path as well as
>>> on the NMI path itself isn't NMI / #MC safe, as either of these events
>>> occurring in the middle of the stack copying would clobber data on the
>>> (source) stack. (Of course, in the NMI case only #MC could break
>>> things.)
>>
>> I think I'd rather fix this by changing the stack switch code or
>
> Well, isn't that what I'm doing in the patch?
>
>> alternativing around it on non-stack-switching kernels.
>
> Fine with me if that's considered better than adding the conditionals.
>
>> Or make Xen use a trampoline stack just like native.
>
> Well, as said I'd rather not, unless x86 and Xen maintainers agree
> that's the way to go. But see below for NMI.
I'd prefer not using a trampoline stack, too.
>
>>> I'm not altering the similar code in interrupt_entry(), as that code
>>> path is unreachable when running an PV Xen guest afaict.
>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> Cc: stable@kernel.org
>>> ---
>>> There would certainly have been the option of using alternatives
>>> patching, but afaict the patching code isn't NMI / #MC safe, so I'd
>>> rather stay away from patching the NMI path. And I thought it would be
>>> better to use similar code in both cases.
>>
>> I would hope we do the patching before we enable any NMIs.
>
> "Enable NMIs"? I don't think they're getting disabled anywhere in the
> kernel. Perhaps you merely mean ones the kernel sends itself (which
> I agree would hopefully only be enabled after alternatives patching?
>
>>> Another option would be to make the Xen case match the native one, by
>>> going through the trampoline stack, but to me this would look like extra
>>> overhead for no gain.
>>
>> Avoiding even more complexity in the nmi code seems like a big gain to me.
>
> I'm not sure the added conditional is more complexity than making Xen
> switch to the trampoline stack just to switch back almost immediately.
I agree.
> But yes, I could see complexity of the NMI code to be a reason to use
> different solutions on the NMI and INT80 paths. It's just that I'd like
> you, the x86 maintainters, and the Xen ones to agree on which solution
> to use where before I'd send a v2.
With my Xen maintainer hat on I'd prefer Jan's current solution.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2] x86-64/Xen: fix stack switching
[not found] ` <5AF03EBD02000078001FE590@prv1-mh.provo.novell.com>
@ 2018-11-21 10:10 ` Jan Beulich
2018-11-21 15:24 ` Andy Lutomirski
2018-11-21 15:24 ` Andy Lutomirski
2018-11-21 10:10 ` Jan Beulich
` (3 subsequent siblings)
4 siblings, 2 replies; 22+ messages in thread
From: Jan Beulich @ 2018-11-21 10:10 UTC (permalink / raw)
To: mingo, tglx, hpa
Cc: Andrew Lutomirski, xen-devel, Boris Ostrovsky, Juergen Gross,
linux-kernel
While in the native case entry into the kernel happens on the trampoline
stack, PV Xen kernels get entered with the current thread stack right
away. Hence source and destination stacks are identical in that case,
and special care is needed.
Other than in sync_regs() the copying done on the INT80 path as well as
on the NMI path itself isn't NMI / #MC safe, as either of these events
occurring in the middle of the stack copying would clobber data on the
(source) stack. (Of course, in the NMI case only #MC could break
things.)
I'm not altering the similar code in interrupt_entry(), as that code
path is unreachable afaict when running PV Xen guests.
Fixes: 7f2590a110b837af5679d08fc25c6227c5a8c497
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: stable@kernel.org
---
v2: Correct placement of .Lint80_keep_stack label.
---
arch/x86/entry/entry_64.S | 8 ++++++++
arch/x86/entry/entry_64_compat.S | 10 ++++++++--
2 files changed, 16 insertions(+), 2 deletions(-)
--- 4.20-rc3/arch/x86/entry/entry_64.S
+++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64.S
@@ -1380,6 +1380,12 @@ ENTRY(nmi)
swapgs
cld
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
+
+ movq PER_CPU_VAR(cpu_current_top_of_stack), %rdx
+ subq $8, %rdx
+ xorq %rsp, %rdx
+ shrq $PAGE_SHIFT, %rdx
+ jz .Lnmi_keep_stack
movq %rsp, %rdx
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
UNWIND_HINT_IRET_REGS base=%rdx offset=8
@@ -1389,6 +1395,8 @@ ENTRY(nmi)
pushq 2*8(%rdx) /* pt_regs->cs */
pushq 1*8(%rdx) /* pt_regs->rip */
UNWIND_HINT_IRET_REGS
+.Lnmi_keep_stack:
+
pushq $-1 /* pt_regs->orig_ax */
PUSH_AND_CLEAR_REGS rdx=(%rdx)
ENCODE_FRAME_POINTER
--- 4.20-rc3/arch/x86/entry/entry_64_compat.S
+++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64_compat.S
@@ -361,17 +361,23 @@ ENTRY(entry_INT80_compat)
/* Need to switch before accessing the thread stack. */
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
+
+ movq PER_CPU_VAR(cpu_current_top_of_stack), %rdi
+ subq $8, %rdi
+ xorq %rsp, %rdi
+ shrq $PAGE_SHIFT, %rdi
+ jz .Lint80_keep_stack
movq %rsp, %rdi
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
-
pushq 6*8(%rdi) /* regs->ss */
pushq 5*8(%rdi) /* regs->rsp */
pushq 4*8(%rdi) /* regs->eflags */
pushq 3*8(%rdi) /* regs->cs */
pushq 2*8(%rdi) /* regs->ip */
pushq 1*8(%rdi) /* regs->orig_ax */
-
pushq (%rdi) /* pt_regs->di */
+.Lint80_keep_stack:
+
pushq %rsi /* pt_regs->si */
xorl %esi, %esi /* nospec si */
pushq %rdx /* pt_regs->dx */
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2] x86-64/Xen: fix stack switching
[not found] ` <5AF03EBD02000078001FE590@prv1-mh.provo.novell.com>
2018-11-21 10:10 ` [PATCH v2] " Jan Beulich
@ 2018-11-21 10:10 ` Jan Beulich
[not found] ` <909090E0020000E2851CF06B@prv1-mh.provo.novell.com>
` (2 subsequent siblings)
4 siblings, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2018-11-21 10:10 UTC (permalink / raw)
To: mingo, tglx, hpa
Cc: Juergen Gross, xen-devel, Boris Ostrovsky, linux-kernel,
Andrew Lutomirski
While in the native case entry into the kernel happens on the trampoline
stack, PV Xen kernels get entered with the current thread stack right
away. Hence source and destination stacks are identical in that case,
and special care is needed.
Other than in sync_regs() the copying done on the INT80 path as well as
on the NMI path itself isn't NMI / #MC safe, as either of these events
occurring in the middle of the stack copying would clobber data on the
(source) stack. (Of course, in the NMI case only #MC could break
things.)
I'm not altering the similar code in interrupt_entry(), as that code
path is unreachable afaict when running PV Xen guests.
Fixes: 7f2590a110b837af5679d08fc25c6227c5a8c497
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: stable@kernel.org
---
v2: Correct placement of .Lint80_keep_stack label.
---
arch/x86/entry/entry_64.S | 8 ++++++++
arch/x86/entry/entry_64_compat.S | 10 ++++++++--
2 files changed, 16 insertions(+), 2 deletions(-)
--- 4.20-rc3/arch/x86/entry/entry_64.S
+++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64.S
@@ -1380,6 +1380,12 @@ ENTRY(nmi)
swapgs
cld
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
+
+ movq PER_CPU_VAR(cpu_current_top_of_stack), %rdx
+ subq $8, %rdx
+ xorq %rsp, %rdx
+ shrq $PAGE_SHIFT, %rdx
+ jz .Lnmi_keep_stack
movq %rsp, %rdx
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
UNWIND_HINT_IRET_REGS base=%rdx offset=8
@@ -1389,6 +1395,8 @@ ENTRY(nmi)
pushq 2*8(%rdx) /* pt_regs->cs */
pushq 1*8(%rdx) /* pt_regs->rip */
UNWIND_HINT_IRET_REGS
+.Lnmi_keep_stack:
+
pushq $-1 /* pt_regs->orig_ax */
PUSH_AND_CLEAR_REGS rdx=(%rdx)
ENCODE_FRAME_POINTER
--- 4.20-rc3/arch/x86/entry/entry_64_compat.S
+++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64_compat.S
@@ -361,17 +361,23 @@ ENTRY(entry_INT80_compat)
/* Need to switch before accessing the thread stack. */
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
+
+ movq PER_CPU_VAR(cpu_current_top_of_stack), %rdi
+ subq $8, %rdi
+ xorq %rsp, %rdi
+ shrq $PAGE_SHIFT, %rdi
+ jz .Lint80_keep_stack
movq %rsp, %rdi
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
-
pushq 6*8(%rdi) /* regs->ss */
pushq 5*8(%rdi) /* regs->rsp */
pushq 4*8(%rdi) /* regs->eflags */
pushq 3*8(%rdi) /* regs->cs */
pushq 2*8(%rdi) /* regs->ip */
pushq 1*8(%rdi) /* regs->orig_ax */
-
pushq (%rdi) /* pt_regs->di */
+.Lint80_keep_stack:
+
pushq %rsi /* pt_regs->si */
xorl %esi, %esi /* nospec si */
pushq %rdx /* pt_regs->dx */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] x86-64/Xen: fix stack switching
2018-11-21 10:10 ` [PATCH v2] " Jan Beulich
2018-11-21 15:24 ` Andy Lutomirski
@ 2018-11-21 15:24 ` Andy Lutomirski
2018-11-22 8:07 ` Jan Beulich
2018-11-22 8:07 ` Jan Beulich
1 sibling, 2 replies; 22+ messages in thread
From: Andy Lutomirski @ 2018-11-21 15:24 UTC (permalink / raw)
To: Jan Beulich
Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Lutomirski,
xen-devel, Boris Ostrovsky, Juergen Gross, LKML
On Wed, Nov 21, 2018 at 2:10 AM Jan Beulich <JBeulich@suse.com> wrote:
>
> While in the native case entry into the kernel happens on the trampoline
> stack, PV Xen kernels get entered with the current thread stack right
> away. Hence source and destination stacks are identical in that case,
> and special care is needed.
>
> Other than in sync_regs() the copying done on the INT80 path as well as
> on the NMI path itself isn't NMI / #MC safe, as either of these events
> occurring in the middle of the stack copying would clobber data on the
> (source) stack. (Of course, in the NMI case only #MC could break
> things.)
>
> I'm not altering the similar code in interrupt_entry(), as that code
> path is unreachable afaict when running PV Xen guests.
>
> Fixes: 7f2590a110b837af5679d08fc25c6227c5a8c497
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: stable@kernel.org
> ---
> v2: Correct placement of .Lint80_keep_stack label.
> ---
> arch/x86/entry/entry_64.S | 8 ++++++++
> arch/x86/entry/entry_64_compat.S | 10 ++++++++--
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> --- 4.20-rc3/arch/x86/entry/entry_64.S
> +++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64.S
> @@ -1380,6 +1380,12 @@ ENTRY(nmi)
> swapgs
> cld
> SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
> +
> + movq PER_CPU_VAR(cpu_current_top_of_stack), %rdx
> + subq $8, %rdx
> + xorq %rsp, %rdx
> + shrq $PAGE_SHIFT, %rdx
> + jz .Lnmi_keep_stack
This code shouldn't even be reachable on Xen:
commit 43e4111086a70c78bedb6ad990bee97f17b27a6e
Author: Juergen Gross <jgross@suse.com>
Date: Thu Nov 2 00:59:07 2017 -0700
xen, x86/entry/64: Add xen NMI trap entry
Instead of trying to execute any NMI via the bare metal's NMI trap
handler use a Xen specific one for PV domains, like we do for e.g.
debug traps. As in a PV domain the NMI is handled via the normal
kernel stack this is the correct thing to do.
This will enable us to get rid of the very fragile and questionable
dependencies between the bare metal NMI handler and Xen assumptions
believed to be broken anyway.
> movq %rsp, %rdx
> movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
> UNWIND_HINT_IRET_REGS base=%rdx offset=8
> @@ -1389,6 +1395,8 @@ ENTRY(nmi)
> pushq 2*8(%rdx) /* pt_regs->cs */
> pushq 1*8(%rdx) /* pt_regs->rip */
> UNWIND_HINT_IRET_REGS
> +.Lnmi_keep_stack:
> +
> pushq $-1 /* pt_regs->orig_ax */
> PUSH_AND_CLEAR_REGS rdx=(%rdx)
> ENCODE_FRAME_POINTER
> --- 4.20-rc3/arch/x86/entry/entry_64_compat.S
> +++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64_compat.S
> @@ -361,17 +361,23 @@ ENTRY(entry_INT80_compat)
>
> /* Need to switch before accessing the thread stack. */
> SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
> +
> + movq PER_CPU_VAR(cpu_current_top_of_stack), %rdi
> + subq $8, %rdi
> + xorq %rsp, %rdi
> + shrq $PAGE_SHIFT, %rdi
> + jz .Lint80_keep_stack
This comparison is IMO the wrong test entirely. How about something like:
/* On Xen PV, entry_INT80_compat is called on the thread stack, so
rewinding to the top of the thread stack would allow an NMI to
overwrite the hardware frame before we copy it. */
ALTERNATIVE "", "jmp .Lint80_keep_stack", X86_FEATURE_XENPV
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] x86-64/Xen: fix stack switching
2018-11-21 10:10 ` [PATCH v2] " Jan Beulich
@ 2018-11-21 15:24 ` Andy Lutomirski
2018-11-21 15:24 ` Andy Lutomirski
1 sibling, 0 replies; 22+ messages in thread
From: Andy Lutomirski @ 2018-11-21 15:24 UTC (permalink / raw)
To: Jan Beulich
Cc: Juergen Gross, LKML, Thomas Gleixner, Andrew Lutomirski,
H. Peter Anvin, xen-devel, Boris Ostrovsky, Ingo Molnar
On Wed, Nov 21, 2018 at 2:10 AM Jan Beulich <JBeulich@suse.com> wrote:
>
> While in the native case entry into the kernel happens on the trampoline
> stack, PV Xen kernels get entered with the current thread stack right
> away. Hence source and destination stacks are identical in that case,
> and special care is needed.
>
> Other than in sync_regs() the copying done on the INT80 path as well as
> on the NMI path itself isn't NMI / #MC safe, as either of these events
> occurring in the middle of the stack copying would clobber data on the
> (source) stack. (Of course, in the NMI case only #MC could break
> things.)
>
> I'm not altering the similar code in interrupt_entry(), as that code
> path is unreachable afaict when running PV Xen guests.
>
> Fixes: 7f2590a110b837af5679d08fc25c6227c5a8c497
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: stable@kernel.org
> ---
> v2: Correct placement of .Lint80_keep_stack label.
> ---
> arch/x86/entry/entry_64.S | 8 ++++++++
> arch/x86/entry/entry_64_compat.S | 10 ++++++++--
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> --- 4.20-rc3/arch/x86/entry/entry_64.S
> +++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64.S
> @@ -1380,6 +1380,12 @@ ENTRY(nmi)
> swapgs
> cld
> SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
> +
> + movq PER_CPU_VAR(cpu_current_top_of_stack), %rdx
> + subq $8, %rdx
> + xorq %rsp, %rdx
> + shrq $PAGE_SHIFT, %rdx
> + jz .Lnmi_keep_stack
This code shouldn't even be reachable on Xen:
commit 43e4111086a70c78bedb6ad990bee97f17b27a6e
Author: Juergen Gross <jgross@suse.com>
Date: Thu Nov 2 00:59:07 2017 -0700
xen, x86/entry/64: Add xen NMI trap entry
Instead of trying to execute any NMI via the bare metal's NMI trap
handler use a Xen specific one for PV domains, like we do for e.g.
debug traps. As in a PV domain the NMI is handled via the normal
kernel stack this is the correct thing to do.
This will enable us to get rid of the very fragile and questionable
dependencies between the bare metal NMI handler and Xen assumptions
believed to be broken anyway.
> movq %rsp, %rdx
> movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
> UNWIND_HINT_IRET_REGS base=%rdx offset=8
> @@ -1389,6 +1395,8 @@ ENTRY(nmi)
> pushq 2*8(%rdx) /* pt_regs->cs */
> pushq 1*8(%rdx) /* pt_regs->rip */
> UNWIND_HINT_IRET_REGS
> +.Lnmi_keep_stack:
> +
> pushq $-1 /* pt_regs->orig_ax */
> PUSH_AND_CLEAR_REGS rdx=(%rdx)
> ENCODE_FRAME_POINTER
> --- 4.20-rc3/arch/x86/entry/entry_64_compat.S
> +++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64_compat.S
> @@ -361,17 +361,23 @@ ENTRY(entry_INT80_compat)
>
> /* Need to switch before accessing the thread stack. */
> SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
> +
> + movq PER_CPU_VAR(cpu_current_top_of_stack), %rdi
> + subq $8, %rdi
> + xorq %rsp, %rdi
> + shrq $PAGE_SHIFT, %rdi
> + jz .Lint80_keep_stack
This comparison is IMO the wrong test entirely. How about something like:
/* On Xen PV, entry_INT80_compat is called on the thread stack, so
rewinding to the top of the thread stack would allow an NMI to
overwrite the hardware frame before we copy it. */
ALTERNATIVE "", "jmp .Lint80_keep_stack", X86_FEATURE_XENPV
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] x86-64/Xen: fix stack switching
2018-11-21 15:24 ` Andy Lutomirski
2018-11-22 8:07 ` Jan Beulich
@ 2018-11-22 8:07 ` Jan Beulich
1 sibling, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2018-11-22 8:07 UTC (permalink / raw)
To: Andrew Lutomirski
Cc: mingo, tglx, xen-devel, Boris Ostrovsky, Juergen Gross,
linux-kernel, hpa
>>> On 21.11.18 at 16:24, <luto@kernel.org> wrote:
> On Wed, Nov 21, 2018 at 2:10 AM Jan Beulich <JBeulich@suse.com> wrote:
>> --- 4.20-rc3/arch/x86/entry/entry_64.S
>> +++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64.S
>> @@ -1380,6 +1380,12 @@ ENTRY(nmi)
>> swapgs
>> cld
>> SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
>> +
>> + movq PER_CPU_VAR(cpu_current_top_of_stack), %rdx
>> + subq $8, %rdx
>> + xorq %rsp, %rdx
>> + shrq $PAGE_SHIFT, %rdx
>> + jz .Lnmi_keep_stack
>
> This code shouldn't even be reachable on Xen:
>
> commit 43e4111086a70c78bedb6ad990bee97f17b27a6e
> Author: Juergen Gross <jgross@suse.com>
> Date: Thu Nov 2 00:59:07 2017 -0700
>
> xen, x86/entry/64: Add xen NMI trap entry
>
> Instead of trying to execute any NMI via the bare metal's NMI trap
> handler use a Xen specific one for PV domains, like we do for e.g.
> debug traps. As in a PV domain the NMI is handled via the normal
> kernel stack this is the correct thing to do.
>
> This will enable us to get rid of the very fragile and questionable
> dependencies between the bare metal NMI handler and Xen assumptions
> believed to be broken anyway.
Oh, I didn't notice this. The beginnings of the patch here pre-date
this, though, and then I didn't notice the addition. Thanks for
pointing this out.
>> --- 4.20-rc3/arch/x86/entry/entry_64_compat.S
>> +++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64_compat.S
>> @@ -361,17 +361,23 @@ ENTRY(entry_INT80_compat)
>>
>> /* Need to switch before accessing the thread stack. */
>> SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
>> +
>> + movq PER_CPU_VAR(cpu_current_top_of_stack), %rdi
>> + subq $8, %rdi
>> + xorq %rsp, %rdi
>> + shrq $PAGE_SHIFT, %rdi
>> + jz .Lint80_keep_stack
>
> This comparison is IMO the wrong test entirely. How about something like:
>
> /* On Xen PV, entry_INT80_compat is called on the thread stack, so
> rewinding to the top of the thread stack would allow an NMI to
> overwrite the hardware frame before we copy it. */
> ALTERNATIVE "", "jmp .Lint80_keep_stack", X86_FEATURE_XENPV
Indeed I had noted this as an alternative option in v1, but
didn't get respective feedback. If that's the preferred route, I'll
certainly switch.
Jan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] x86-64/Xen: fix stack switching
2018-11-21 15:24 ` Andy Lutomirski
@ 2018-11-22 8:07 ` Jan Beulich
2018-11-22 8:07 ` Jan Beulich
1 sibling, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2018-11-22 8:07 UTC (permalink / raw)
To: Andrew Lutomirski
Cc: Juergen Gross, linux-kernel, tglx, hpa, xen-devel,
Boris Ostrovsky, mingo
>>> On 21.11.18 at 16:24, <luto@kernel.org> wrote:
> On Wed, Nov 21, 2018 at 2:10 AM Jan Beulich <JBeulich@suse.com> wrote:
>> --- 4.20-rc3/arch/x86/entry/entry_64.S
>> +++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64.S
>> @@ -1380,6 +1380,12 @@ ENTRY(nmi)
>> swapgs
>> cld
>> SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
>> +
>> + movq PER_CPU_VAR(cpu_current_top_of_stack), %rdx
>> + subq $8, %rdx
>> + xorq %rsp, %rdx
>> + shrq $PAGE_SHIFT, %rdx
>> + jz .Lnmi_keep_stack
>
> This code shouldn't even be reachable on Xen:
>
> commit 43e4111086a70c78bedb6ad990bee97f17b27a6e
> Author: Juergen Gross <jgross@suse.com>
> Date: Thu Nov 2 00:59:07 2017 -0700
>
> xen, x86/entry/64: Add xen NMI trap entry
>
> Instead of trying to execute any NMI via the bare metal's NMI trap
> handler use a Xen specific one for PV domains, like we do for e.g.
> debug traps. As in a PV domain the NMI is handled via the normal
> kernel stack this is the correct thing to do.
>
> This will enable us to get rid of the very fragile and questionable
> dependencies between the bare metal NMI handler and Xen assumptions
> believed to be broken anyway.
Oh, I didn't notice this. The beginnings of the patch here pre-date
this, though, and then I didn't notice the addition. Thanks for
pointing this out.
>> --- 4.20-rc3/arch/x86/entry/entry_64_compat.S
>> +++ 4.20-rc3-x86_64-stack-switch-Xen/arch/x86/entry/entry_64_compat.S
>> @@ -361,17 +361,23 @@ ENTRY(entry_INT80_compat)
>>
>> /* Need to switch before accessing the thread stack. */
>> SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
>> +
>> + movq PER_CPU_VAR(cpu_current_top_of_stack), %rdi
>> + subq $8, %rdi
>> + xorq %rsp, %rdi
>> + shrq $PAGE_SHIFT, %rdi
>> + jz .Lint80_keep_stack
>
> This comparison is IMO the wrong test entirely. How about something like:
>
> /* On Xen PV, entry_INT80_compat is called on the thread stack, so
> rewinding to the top of the thread stack would allow an NMI to
> overwrite the hardware frame before we copy it. */
> ALTERNATIVE "", "jmp .Lint80_keep_stack", X86_FEATURE_XENPV
Indeed I had noted this as an alternative option in v1, but
didn't get respective feedback. If that's the preferred route, I'll
certainly switch.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] x86-64/Xen: fix stack switching
[not found] ` <909090E0020000E2851CF06B@prv1-mh.provo.novell.com>
@ 2018-11-22 8:07 ` Jan Beulich
2018-11-22 15:58 ` Sasha Levin
0 siblings, 1 reply; 22+ messages in thread
From: Jan Beulich @ 2018-11-22 8:07 UTC (permalink / raw)
To: Sasha Levin; +Cc: mingo, Andrew Lutomirski, stable, tglx, stable, hpa
>>> On 22.11.18 at 07:58, <sashal@kernel.org> wrote:
> [This is an automated email]
>
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: 7f2590a110b8 x86/entry/64: Use a per-CPU trampoline stack for
> IDT entries.
>
> The bot has tested the following trees: v4.19.2, v4.18.19, v4.14.81.
>
> v4.19.2: Build OK!
> v4.18.19: Build OK!
> v4.14.81: Failed to apply! Possible dependencies:
> Unable to calculate
>
>
> How should we proceed with this patch?
Before evaluating backporting options, how about we first settle
on what's to go into the master branch?
Jan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] x86-64/Xen: fix stack switching
2018-11-22 8:07 ` Jan Beulich
@ 2018-11-22 15:58 ` Sasha Levin
0 siblings, 0 replies; 22+ messages in thread
From: Sasha Levin @ 2018-11-22 15:58 UTC (permalink / raw)
To: Jan Beulich; +Cc: mingo, Andrew Lutomirski, stable, tglx, stable, hpa
On Thu, Nov 22, 2018 at 01:07:56AM -0700, Jan Beulich wrote:
>>>> On 22.11.18 at 07:58, <sashal@kernel.org> wrote:
>> [This is an automated email]
>>
>> This commit has been processed because it contains a "Fixes:" tag,
>> fixing commit: 7f2590a110b8 x86/entry/64: Use a per-CPU trampoline stack for
>> IDT entries.
>>
>> The bot has tested the following trees: v4.19.2, v4.18.19, v4.14.81.
>>
>> v4.19.2: Build OK!
>> v4.18.19: Build OK!
>> v4.14.81: Failed to apply! Possible dependencies:
>> Unable to calculate
>>
>>
>> How should we proceed with this patch?
>
>Before evaluating backporting options, how about we first settle
>on what's to go into the master branch?
That's fine, it was just an automatic mail.
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3] x86-64/Xen: fix stack switching
[not found] ` <5AF03EBD02000078001FE590@prv1-mh.provo.novell.com>
` (3 preceding siblings ...)
2019-01-15 16:58 ` [PATCH v3] " Jan Beulich
@ 2019-01-15 16:58 ` Jan Beulich
2019-01-17 0:09 ` Andy Lutomirski
` (2 more replies)
4 siblings, 3 replies; 22+ messages in thread
From: Jan Beulich @ 2019-01-15 16:58 UTC (permalink / raw)
To: mingo, tglx, hpa
Cc: Andrew Lutomirski, xen-devel, Boris Ostrovsky, Juergen Gross,
linux-kernel
While in the native case entry into the kernel happens on the trampoline
stack, PV Xen kernels get entered with the current thread stack right
away. Hence source and destination stacks are identical in that case,
and special care is needed.
Other than in sync_regs() the copying done on the INT80 path isn't
NMI / #MC safe, as either of these events occurring in the middle of the
stack copying would clobber data on the (source) stack.
I'm not altering the similar code in interrupt_entry() and nmi(), as
those code paths are unreachable afaict when running PV Xen guests.
Fixes: 7f2590a110b837af5679d08fc25c6227c5a8c497
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: stable@kernel.org
---
v3: Drop NMI path change. Use ALTERNATIVE.
v2: Correct placement of .Lint80_keep_stack label.
---
arch/x86/entry/entry_64_compat.S | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
--- 5.0-rc2/arch/x86/entry/entry_64_compat.S
+++ 5.0-rc2-x86_64-stack-switch-Xen/arch/x86/entry/entry_64_compat.S
@@ -361,7 +361,8 @@ ENTRY(entry_INT80_compat)
/* Need to switch before accessing the thread stack. */
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
- movq %rsp, %rdi
+ /* In the Xen PV case we already run on the thread stack. */
+ ALTERNATIVE "movq %rsp, %rdi", "jmp .Lint80_keep_stack", X86_FEATURE_XENPV
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
pushq 6*8(%rdi) /* regs->ss */
@@ -370,8 +371,9 @@ ENTRY(entry_INT80_compat)
pushq 3*8(%rdi) /* regs->cs */
pushq 2*8(%rdi) /* regs->ip */
pushq 1*8(%rdi) /* regs->orig_ax */
-
pushq (%rdi) /* pt_regs->di */
+.Lint80_keep_stack:
+
pushq %rsi /* pt_regs->si */
xorl %esi, %esi /* nospec si */
pushq %rdx /* pt_regs->dx */
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3] x86-64/Xen: fix stack switching
[not found] ` <5AF03EBD02000078001FE590@prv1-mh.provo.novell.com>
` (2 preceding siblings ...)
[not found] ` <909090E0020000E2851CF06B@prv1-mh.provo.novell.com>
@ 2019-01-15 16:58 ` Jan Beulich
2019-01-15 16:58 ` Jan Beulich
4 siblings, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2019-01-15 16:58 UTC (permalink / raw)
To: mingo, tglx, hpa
Cc: Juergen Gross, xen-devel, Boris Ostrovsky, linux-kernel,
Andrew Lutomirski
While in the native case entry into the kernel happens on the trampoline
stack, PV Xen kernels get entered with the current thread stack right
away. Hence source and destination stacks are identical in that case,
and special care is needed.
Other than in sync_regs() the copying done on the INT80 path isn't
NMI / #MC safe, as either of these events occurring in the middle of the
stack copying would clobber data on the (source) stack.
I'm not altering the similar code in interrupt_entry() and nmi(), as
those code paths are unreachable afaict when running PV Xen guests.
Fixes: 7f2590a110b837af5679d08fc25c6227c5a8c497
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: stable@kernel.org
---
v3: Drop NMI path change. Use ALTERNATIVE.
v2: Correct placement of .Lint80_keep_stack label.
---
arch/x86/entry/entry_64_compat.S | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
--- 5.0-rc2/arch/x86/entry/entry_64_compat.S
+++ 5.0-rc2-x86_64-stack-switch-Xen/arch/x86/entry/entry_64_compat.S
@@ -361,7 +361,8 @@ ENTRY(entry_INT80_compat)
/* Need to switch before accessing the thread stack. */
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
- movq %rsp, %rdi
+ /* In the Xen PV case we already run on the thread stack. */
+ ALTERNATIVE "movq %rsp, %rdi", "jmp .Lint80_keep_stack", X86_FEATURE_XENPV
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
pushq 6*8(%rdi) /* regs->ss */
@@ -370,8 +371,9 @@ ENTRY(entry_INT80_compat)
pushq 3*8(%rdi) /* regs->cs */
pushq 2*8(%rdi) /* regs->ip */
pushq 1*8(%rdi) /* regs->orig_ax */
-
pushq (%rdi) /* pt_regs->di */
+.Lint80_keep_stack:
+
pushq %rsi /* pt_regs->si */
xorl %esi, %esi /* nospec si */
pushq %rdx /* pt_regs->dx */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] x86-64/Xen: fix stack switching
2019-01-15 16:58 ` Jan Beulich
2019-01-17 0:09 ` Andy Lutomirski
@ 2019-01-17 0:09 ` Andy Lutomirski
2019-01-17 23:42 ` [tip:x86/urgent] x86/entry/64/compat: Fix stack switching for XEN PV tip-bot for Jan Beulich
2 siblings, 0 replies; 22+ messages in thread
From: Andy Lutomirski @ 2019-01-17 0:09 UTC (permalink / raw)
To: Jan Beulich
Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, Andrew Lutomirski,
xen-devel, Boris Ostrovsky, Juergen Gross, LKML
On Tue, Jan 15, 2019 at 8:58 AM Jan Beulich <JBeulich@suse.com> wrote:
>
> While in the native case entry into the kernel happens on the trampoline
> stack, PV Xen kernels get entered with the current thread stack right
> away. Hence source and destination stacks are identical in that case,
> and special care is needed.
>
> Other than in sync_regs() the copying done on the INT80 path isn't
> NMI / #MC safe, as either of these events occurring in the middle of the
> stack copying would clobber data on the (source) stack.
>
> I'm not altering the similar code in interrupt_entry() and nmi(), as
> those code paths are unreachable afaict when running PV Xen guests.
Acked-by: Andy Lutomirski <luto@kernel.org>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] x86-64/Xen: fix stack switching
2019-01-15 16:58 ` Jan Beulich
@ 2019-01-17 0:09 ` Andy Lutomirski
2019-01-17 0:09 ` Andy Lutomirski
2019-01-17 23:42 ` [tip:x86/urgent] x86/entry/64/compat: Fix stack switching for XEN PV tip-bot for Jan Beulich
2 siblings, 0 replies; 22+ messages in thread
From: Andy Lutomirski @ 2019-01-17 0:09 UTC (permalink / raw)
To: Jan Beulich
Cc: Juergen Gross, LKML, Thomas Gleixner, Andrew Lutomirski,
H. Peter Anvin, xen-devel, Boris Ostrovsky, Ingo Molnar
On Tue, Jan 15, 2019 at 8:58 AM Jan Beulich <JBeulich@suse.com> wrote:
>
> While in the native case entry into the kernel happens on the trampoline
> stack, PV Xen kernels get entered with the current thread stack right
> away. Hence source and destination stacks are identical in that case,
> and special care is needed.
>
> Other than in sync_regs() the copying done on the INT80 path isn't
> NMI / #MC safe, as either of these events occurring in the middle of the
> stack copying would clobber data on the (source) stack.
>
> I'm not altering the similar code in interrupt_entry() and nmi(), as
> those code paths are unreachable afaict when running PV Xen guests.
Acked-by: Andy Lutomirski <luto@kernel.org>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [tip:x86/urgent] x86/entry/64/compat: Fix stack switching for XEN PV
2019-01-15 16:58 ` Jan Beulich
2019-01-17 0:09 ` Andy Lutomirski
2019-01-17 0:09 ` Andy Lutomirski
@ 2019-01-17 23:42 ` tip-bot for Jan Beulich
2019-01-22 15:56 ` Sasha Levin
2 siblings, 1 reply; 22+ messages in thread
From: tip-bot for Jan Beulich @ 2019-01-17 23:42 UTC (permalink / raw)
To: linux-tip-commits
Cc: luto, linux-kernel, jbeulich, mingo, boris.ostrovsky, hpa,
JBeulich, tglx, jgross
Commit-ID: fc24d75a7f91837d7918e40719575951820b2b8f
Gitweb: https://git.kernel.org/tip/fc24d75a7f91837d7918e40719575951820b2b8f
Author: Jan Beulich <JBeulich@suse.com>
AuthorDate: Tue, 15 Jan 2019 09:58:16 -0700
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 18 Jan 2019 00:39:33 +0100
x86/entry/64/compat: Fix stack switching for XEN PV
While in the native case entry into the kernel happens on the trampoline
stack, PV Xen kernels get entered with the current thread stack right
away. Hence source and destination stacks are identical in that case,
and special care is needed.
Other than in sync_regs() the copying done on the INT80 path isn't
NMI / #MC safe, as either of these events occurring in the middle of the
stack copying would clobber data on the (source) stack.
There is similar code in interrupt_entry() and nmi(), but there is no fixup
required because those code paths are unreachable in XEN PV guests.
[ tglx: Sanitized subject, changelog, Fixes tag and stable mail address. Sigh ]
Fixes: 7f2590a110b8 ("x86/entry/64: Use a per-CPU trampoline stack for IDT entries")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Andy Lutomirski <luto@kernel.org>
Cc: Peter Anvin <hpa@zytor.com>
Cc: xen-devel@lists.xenproject.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/5C3E1128020000780020DFAD@prv1-mh.provo.novell.com
---
arch/x86/entry/entry_64_compat.S | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 8eaf8952c408..39913770a44d 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -361,7 +361,8 @@ ENTRY(entry_INT80_compat)
/* Need to switch before accessing the thread stack. */
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
- movq %rsp, %rdi
+ /* In the Xen PV case we already run on the thread stack. */
+ ALTERNATIVE "movq %rsp, %rdi", "jmp .Lint80_keep_stack", X86_FEATURE_XENPV
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
pushq 6*8(%rdi) /* regs->ss */
@@ -370,8 +371,9 @@ ENTRY(entry_INT80_compat)
pushq 3*8(%rdi) /* regs->cs */
pushq 2*8(%rdi) /* regs->ip */
pushq 1*8(%rdi) /* regs->orig_ax */
-
pushq (%rdi) /* pt_regs->di */
+.Lint80_keep_stack:
+
pushq %rsi /* pt_regs->si */
xorl %esi, %esi /* nospec si */
pushq %rdx /* pt_regs->dx */
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [tip:x86/urgent] x86/entry/64/compat: Fix stack switching for XEN PV
2019-01-17 23:42 ` [tip:x86/urgent] x86/entry/64/compat: Fix stack switching for XEN PV tip-bot for Jan Beulich
@ 2019-01-22 15:56 ` Sasha Levin
0 siblings, 0 replies; 22+ messages in thread
From: Sasha Levin @ 2019-01-22 15:56 UTC (permalink / raw)
To: Sasha Levin, tip-bot for Jan Beulich, linux-tip-commits
Cc: luto, linux-kernel, stable, jbeulich, Peter Anvin, xen-devel,
Boris Ostrovsky
Hi,
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 7f2590a110b8 x86/entry/64: Use a per-CPU trampoline stack for IDT entries.
The bot has tested the following trees: v4.20.3, v4.19.16, v4.14.94.
v4.20.3: Build OK!
v4.19.16: Build OK!
v4.14.94: Failed to apply! Possible dependencies:
0e34d226342c ("x86/entry/64: Move PUSH_AND_CLEAR_REGS from interrupt macro to helper function")
2ba6474104a1 ("x86/entry/64: Move ENTER_IRQ_STACK from interrupt macro to interrupt_entry")
90a6acc4e7eb ("x86/entry/64: Move the switch_to_thread_stack() call to interrupt_entry()")
91c5f0de64a2 ("x86/entry/64/compat: Save one instruction in entry_INT80_compat()")
b2855d8d2de0 ("x86/entry/64: Move ASM_CLAC to interrupt_entry()")
f3d415ea4696 ("x86/entry/64: Open-code switch_to_thread_stack()")
How should we proceed with this patch?
--
Thanks,
Sasha
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] x86-64/Xen: fix stack switching
@ 2018-05-07 11:55 Jan Beulich
0 siblings, 0 replies; 22+ messages in thread
From: Jan Beulich @ 2018-05-07 11:55 UTC (permalink / raw)
To: mingo, tglx, hpa
Cc: Juergen Gross, xen-devel, Boris Ostrovsky, linux-kernel, Andy Lutomirski
While on native entry into the kernel happens on the trampoline stack,
PV Xen kernels are being entered with the current thread stack right
away. Hence source and destination stacks are identical in that case,
and special care is needed.
Other than in sync_regs() the copying done on the INT80 path as well as
on the NMI path itself isn't NMI / #MC safe, as either of these events
occurring in the middle of the stack copying would clobber data on the
(source) stack. (Of course, in the NMI case only #MC could break
things.)
I'm not altering the similar code in interrupt_entry(), as that code
path is unreachable when running an PV Xen guest afaict.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: stable@kernel.org
---
There would certainly have been the option of using alternatives
patching, but afaict the patching code isn't NMI / #MC safe, so I'd
rather stay away from patching the NMI path. And I thought it would be
better to use similar code in both cases.
Another option would be to make the Xen case match the native one, by
going through the trampoline stack, but to me this would look like extra
overhead for no gain.
---
arch/x86/entry/entry_64.S | 8 ++++++++
arch/x86/entry/entry_64_compat.S | 8 +++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
--- 4.17-rc4/arch/x86/entry/entry_64.S
+++ 4.17-rc4-x86_64-stack-switch-Xen/arch/x86/entry/entry_64.S
@@ -1399,6 +1399,12 @@ ENTRY(nmi)
swapgs
cld
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
+
+ movq PER_CPU_VAR(cpu_current_top_of_stack), %rdx
+ subq $8, %rdx
+ xorq %rsp, %rdx
+ shrq $PAGE_SHIFT, %rdx
+ jz .Lnmi_keep_stack
movq %rsp, %rdx
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
UNWIND_HINT_IRET_REGS base=%rdx offset=8
@@ -1408,6 +1414,8 @@ ENTRY(nmi)
pushq 2*8(%rdx) /* pt_regs->cs */
pushq 1*8(%rdx) /* pt_regs->rip */
UNWIND_HINT_IRET_REGS
+.Lnmi_keep_stack:
+
pushq $-1 /* pt_regs->orig_ax */
PUSH_AND_CLEAR_REGS rdx=(%rdx)
ENCODE_FRAME_POINTER
--- 4.17-rc4/arch/x86/entry/entry_64_compat.S
+++ 4.17-rc4-x86_64-stack-switch-Xen/arch/x86/entry/entry_64_compat.S
@@ -356,15 +356,21 @@ ENTRY(entry_INT80_compat)
/* Need to switch before accessing the thread stack. */
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
+
+ movq PER_CPU_VAR(cpu_current_top_of_stack), %rdi
+ subq $8, %rdi
+ xorq %rsp, %rdi
+ shrq $PAGE_SHIFT, %rdi
+ jz .Lint80_keep_stack
movq %rsp, %rdi
movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
-
pushq 6*8(%rdi) /* regs->ss */
pushq 5*8(%rdi) /* regs->rsp */
pushq 4*8(%rdi) /* regs->eflags */
pushq 3*8(%rdi) /* regs->cs */
pushq 2*8(%rdi) /* regs->ip */
pushq 1*8(%rdi) /* regs->orig_ax */
+.Lint80_keep_stack:
pushq (%rdi) /* pt_regs->di */
pushq %rsi /* pt_regs->si */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2019-01-22 15:56 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-07 11:55 [PATCH] x86-64/Xen: fix stack switching Jan Beulich
2018-05-08 2:38 ` Andy Lutomirski
2018-05-08 2:38 ` Andy Lutomirski
2018-05-14 10:28 ` Jan Beulich
2018-05-14 10:28 ` Jan Beulich
[not found] ` <5AF964B302000078001C26BC@suse.com>
2018-05-14 12:08 ` Juergen Gross
2018-05-14 12:08 ` Juergen Gross
[not found] ` <5AF03EBD02000000000F91D6@prv1-mh.provo.novell.com>
[not found] ` <5AF03EBD02000078001FE590@prv1-mh.provo.novell.com>
2018-11-21 10:10 ` [PATCH v2] " Jan Beulich
2018-11-21 15:24 ` Andy Lutomirski
2018-11-21 15:24 ` Andy Lutomirski
2018-11-22 8:07 ` Jan Beulich
2018-11-22 8:07 ` Jan Beulich
2018-11-21 10:10 ` Jan Beulich
[not found] ` <909090E0020000E2851CF06B@prv1-mh.provo.novell.com>
2018-11-22 8:07 ` Jan Beulich
2018-11-22 15:58 ` Sasha Levin
2019-01-15 16:58 ` [PATCH v3] " Jan Beulich
2019-01-15 16:58 ` Jan Beulich
2019-01-17 0:09 ` Andy Lutomirski
2019-01-17 0:09 ` Andy Lutomirski
2019-01-17 23:42 ` [tip:x86/urgent] x86/entry/64/compat: Fix stack switching for XEN PV tip-bot for Jan Beulich
2019-01-22 15:56 ` Sasha Levin
-- strict thread matches above, loose matches on Subject: below --
2018-05-07 11:55 [PATCH] x86-64/Xen: fix stack switching Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.