xen: get rid of paravirt op adjust_exception_frame

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

Signed-off-by Juergen Gross <[email protected]>

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

Signed-off-by Juergen Gross <[email protected]>

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

arch/x86/entry/entry_64.S ENTRY(error_exit) · line 1172

Quoted 13 lines

/* Runs on exception stack */

arch/x86/entry/entry_64.S +0 -11

-ENTRY(nmi)
-       /*
-        * Fix up the exception frame if we're on Xen.
-        * PARAVIRT_ADJUST_EXCEPTION_FRAME is guaranteed to push at most
-        * one value to the stack on native, so it may clobber the rdx
-        * scratch slot, but it won't clobber any of the important
-        * slots past it.
-        *
-        * Xen is a different story, because the Xen frame itself overlaps
-        * the "NMI executing" variable.
-        */

I would keep this comment. The Xen frame really is in the way AFAICT.

arch/x86/entry/entry_64.S ENTRY(error_exit) · line 1172

Quoted 13 lines

/* Runs on exception stack */

arch/x86/entry/entry_64.S +0 -11

-ENTRY(nmi)
-       /*
-        * Fix up the exception frame if we're on Xen.
-        * PARAVIRT_ADJUST_EXCEPTION_FRAME is guaranteed to push at most
-        * one value to the stack on native, so it may clobber the rdx
-        * scratch slot, but it won't clobber any of the important
-        * slots past it.
-        *
-        * Xen is a different story, because the Xen frame itself overlaps
-        * the "NMI executing" variable.
-        */

I would keep this comment. The Xen frame really is in the way AFAICT.

Quoted 13 lines · Andy Lutomirski

Quoted 13 lines

/* Runs on exception stack */

arch/x86/entry/entry_64.S +0 -11

-ENTRY(nmi)
-       /*
-        * Fix up the exception frame if we're on Xen.
-        * PARAVIRT_ADJUST_EXCEPTION_FRAME is guaranteed to push at most
-        * one value to the stack on native, so it may clobber the rdx
-        * scratch slot, but it won't clobber any of the important
-        * slots past it.
-        *
-        * Xen is a different story, because the Xen frame itself overlaps
-        * the "NMI executing" variable.
-        */

Quoted 1 line

I would keep this comment. The Xen frame really is in the way AFAICT.

(For reasons best explained by the original authors) there is only ever a single stack which a PV guest registers with Xen, which functions equivalently to tss.sp0. There is no support for stack switching via task switch or IST.

Therefore, nested NMIs won't clobber the top of this stack.

Quoted 19 lines · Andrew Cooper

Quoted 12 lines · Andy Lutomirski

arch/x86/entry/entry_64.S ENTRY(error_exit) · line 1172

Quoted 12 lines

/* Runs on exception stack */

arch/x86/entry/entry_64.S +0 -11

-ENTRY(nmi)
-       /*
-        * Fix up the exception frame if we're on Xen.
-        * PARAVIRT_ADJUST_EXCEPTION_FRAME is guaranteed to push at most
-        * one value to the stack on native, so it may clobber the rdx
-        * scratch slot, but it won't clobber any of the important
-        * slots past it.
-        *
-        * Xen is a different story, because the Xen frame itself overlaps
-        * the "NMI executing" variable.
-        */

Quoted 1 line

I would keep this comment. The Xen frame really is in the way AFAICT.

(For reasons best explained by the original authors) there is only ever a single stack which a PV guest registers with Xen, which functions equivalently to tss.sp0. There is no support for stack switching via task switch or IST.

Quoted 2 lines

Therefore, nested NMIs won't clobber the top of this stack.

Does that mean that nested NMIs on Xen just nest normally without clobbering each other?

If so, that's neat, although I wonder how we
don't get crashes due to this:

/* Normal 64-bit system call target */
ENTRY(xen_syscall_target)
undo_xen_syscall
<-- NMI right here
jmp entry_SYSCALL_64_after_swapgs
ENDPROC(xen_syscall_target)

I think it would be nice if Xen could instead enter the native syscall path a bit later like this:

ENTRY(entry_SYSCALL_64)
/*
* Interrupts are off on entry.
* We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON,
* it is too small to ever cause noticeable irq latency. */
SWAPGS_UNSAFE_STACK
/*
* A hypervisor implementation might want to use a label
* after the swapgs, so that it can do the swapgs
* for the guest and jump here on syscall.
*/
GLOBAL(entry_SYSCALL_64_after_swapgs)

        movq    %rsp, PER_CPU_VAR(rsp_scratch)
        movq    PER_CPU_VAR(cpu_current_top_of_stack), %rsp

        TRACE_IRQS_OFF

        /* Construct struct pt_regs on stack */
        pushq   $__USER_DS                      /* pt_regs->ss */
        pushq   PER_CPU_VAR(rsp_scratch)        /* pt_regs->sp */
        pushq   %r11                            /* pt_regs->flags */
        pushq   $__USER_CS                      /* pt_regs->cs */
        pushq   %rcx                            /* pt_regs->ip */

<-- Xen enters here

then we wouldn't have all this funny business. And Xen could completely skip the nmi nesting code.

Quoted 19 lines · Andrew Cooper

Quoted 12 lines · Andy Lutomirski

arch/x86/entry/entry_64.S ENTRY(error_exit) · line 1172

Quoted 12 lines

/* Runs on exception stack */

arch/x86/entry/entry_64.S +0 -11

-ENTRY(nmi)
-       /*
-        * Fix up the exception frame if we're on Xen.
-        * PARAVIRT_ADJUST_EXCEPTION_FRAME is guaranteed to push at most
-        * one value to the stack on native, so it may clobber the rdx
-        * scratch slot, but it won't clobber any of the important
-        * slots past it.
-        *
-        * Xen is a different story, because the Xen frame itself overlaps
-        * the "NMI executing" variable.
-        */

Quoted 1 line

I would keep this comment. The Xen frame really is in the way AFAICT.

(For reasons best explained by the original authors) there is only ever a single stack which a PV guest registers with Xen, which functions equivalently to tss.sp0. There is no support for stack switching via task switch or IST.

Quoted 2 lines

Therefore, nested NMIs won't clobber the top of this stack.

Does that mean that nested NMIs on Xen just nest normally without clobbering each other?

If so, that's neat, although I wonder how we
don't get crashes due to this:

/* Normal 64-bit system call target */
ENTRY(xen_syscall_target)
undo_xen_syscall
<-- NMI right here
jmp entry_SYSCALL_64_after_swapgs
ENDPROC(xen_syscall_target)

I think it would be nice if Xen could instead enter the native syscall path a bit later like this:

ENTRY(entry_SYSCALL_64)
/*
* Interrupts are off on entry.
* We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON,
* it is too small to ever cause noticeable irq latency. */
SWAPGS_UNSAFE_STACK
/*
* A hypervisor implementation might want to use a label
* after the swapgs, so that it can do the swapgs
* for the guest and jump here on syscall.
*/
GLOBAL(entry_SYSCALL_64_after_swapgs)

        movq    %rsp, PER_CPU_VAR(rsp_scratch)
        movq    PER_CPU_VAR(cpu_current_top_of_stack), %rsp

        TRACE_IRQS_OFF

        /* Construct struct pt_regs on stack */
        pushq   $__USER_DS                      /* pt_regs->ss */
        pushq   PER_CPU_VAR(rsp_scratch)        /* pt_regs->sp */
        pushq   %r11                            /* pt_regs->flags */
        pushq   $__USER_CS                      /* pt_regs->cs */
        pushq   %rcx                            /* pt_regs->ip */

<-- Xen enters here

then we wouldn't have all this funny business. And Xen could completely skip the nmi nesting code.

Quoted 13 lines · Andy Lutomirski

Quoted 13 lines

/* Runs on exception stack */

arch/x86/entry/entry_64.S +0 -11

-ENTRY(nmi)
-       /*
-        * Fix up the exception frame if we're on Xen.
-        * PARAVIRT_ADJUST_EXCEPTION_FRAME is guaranteed to push at most
-        * one value to the stack on native, so it may clobber the rdx
-        * scratch slot, but it won't clobber any of the important
-        * slots past it.
-        *
-        * Xen is a different story, because the Xen frame itself overlaps
-        * the "NMI executing" variable.
-        */

Quoted 1 line

I would keep this comment. The Xen frame really is in the way AFAICT.

(For reasons best explained by the original authors) there is only ever a single stack which a PV guest registers with Xen, which functions equivalently to tss.sp0. There is no support for stack switching via task switch or IST.

Therefore, nested NMIs won't clobber the top of this stack.

Quoted 7 lines · Juergen Gross

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

I think this is a nice cleanup, but I'm wondering if it would be even nicer if the Xen part was kept out-of-line. That is, could Xen have little stubs like:

xen_alignment_check:
pop %rcx
pop %r11
jmp alignment_check

rather than using the macros in entry_64.S that you have? Then you could adjust set_trap_gate instead of pack_gate and maybe even do something like:

#define set_trap_gate(..., name, ...) set_native_or_xen_trap_gate(..., name, xen_##name, ...)

arch/x86/entry/entry_64.S ENTRY(error_exit) · line 1172

Quoted 12 lines

/* Runs on exception stack */

arch/x86/entry/entry_64.S +0 -11

-ENTRY(nmi)
-       /*
-        * Fix up the exception frame if we're on Xen.
-        * PARAVIRT_ADJUST_EXCEPTION_FRAME is guaranteed to push at most
-        * one value to the stack on native, so it may clobber the rdx
-        * scratch slot, but it won't clobber any of the important
-        * slots past it.
-        *
-        * Xen is a different story, because the Xen frame itself overlaps
-        * the "NMI executing" variable.
-        */

I would keep this comment. The Xen frame really is in the way AFAICT.

Quoted 7 lines · Juergen Gross

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

I think this is a nice cleanup, but I'm wondering if it would be even nicer if the Xen part was kept out-of-line. That is, could Xen have little stubs like:

xen_alignment_check:
pop %rcx
pop %r11
jmp alignment_check

rather than using the macros in entry_64.S that you have? Then you could adjust set_trap_gate instead of pack_gate and maybe even do something like:

#define set_trap_gate(..., name, ...) set_native_or_xen_trap_gate(..., name, xen_##name, ...)

arch/x86/entry/entry_64.S ENTRY(error_exit) · line 1172

Quoted 12 lines

/* Runs on exception stack */

arch/x86/entry/entry_64.S +0 -11

-ENTRY(nmi)
-       /*
-        * Fix up the exception frame if we're on Xen.
-        * PARAVIRT_ADJUST_EXCEPTION_FRAME is guaranteed to push at most
-        * one value to the stack on native, so it may clobber the rdx
-        * scratch slot, but it won't clobber any of the important
-        * slots past it.
-        *
-        * Xen is a different story, because the Xen frame itself overlaps
-        * the "NMI executing" variable.
-        */

I would keep this comment. The Xen frame really is in the way AFAICT.

Quoted 7 lines · Juergen Gross

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

I think this is a nice cleanup, but I'm wondering if it would be even nicer if the Xen part was kept out-of-line. That is, could Xen have little stubs like:

xen_alignment_check:
pop %rcx
pop %r11
jmp alignment_check

rather than using the macros in entry_64.S that you have? Then you could adjust set_trap_gate instead of pack_gate and maybe even do something like:

#define set_trap_gate(..., name, ...) set_native_or_xen_trap_gate(..., name, xen_##name, ...)

Quoted 20 lines · Andy Lutomirski

Quoted 8 lines · Juergen Gross

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

I think this is a nice cleanup, but I'm wondering if it would be even nicer if the Xen part was kept out-of-line. That is, could Xen have little stubs like:

xen_alignment_check:
pop %rcx
pop %r11
jmp alignment_check

Quoted 6 lines

rather than using the macros in entry_64.S that you have? Then you could adjust set_trap_gate instead of pack_gate and maybe even do something like:

#define set_trap_gate(..., name, ...) set_native_or_xen_trap_gate(..., name, xen_##name, ...)

I think I'll have something like:

#define pv_trap_entry(name) (xen_pv_domain() ? xen_ ## name : name)

and use it like:

set_intr_gate(X86_TRAP_AC, pv_trap_entry(alignment_check));

This will avoid having to define macros for all variants of set_intr_gate(), e.g. set_intr_gate_ist(), set_system_intr_gate().

Do you have any objections?

Juergen

Quoted 34 lines · Juergen Gross

Quoted 20 lines · Andy Lutomirski

Quoted 8 lines · Juergen Gross

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

I think this is a nice cleanup, but I'm wondering if it would be even nicer if the Xen part was kept out-of-line. That is, could Xen have little stubs like:

xen_alignment_check:
pop %rcx
pop %r11
jmp alignment_check

Quoted 6 lines

rather than using the macros in entry_64.S that you have? Then you could adjust set_trap_gate instead of pack_gate and maybe even do something like:

#define set_trap_gate(..., name, ...) set_native_or_xen_trap_gate(..., name, xen_##name, ...)

I think I'll have something like:

#define pv_trap_entry(name) (xen_pv_domain() ? xen_ ## name : name)

Quoted 9 lines

and use it like:

set_intr_gate(X86_TRAP_AC, pv_trap_entry(alignment_check));

This will avoid having to define macros for all variants of set_intr_gate(), e.g. set_intr_gate_ist(), set_system_intr_gate().

Do you have any objections?

Sounds good to me.

FWIW, I have no real objection to putting the Xen entry right before the native entry and falling through. I don't love the ip -= 3 bit, though, and I think that the PV_ENTRY macro is too magical.

This might be okay, though:

XEN_PV_ENTRY_FALLTHROUGH(foo)
ENTRY(foo)

Quoted 34 lines · Juergen Gross

Quoted 20 lines · Andy Lutomirski

Quoted 8 lines · Juergen Gross

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

I think this is a nice cleanup, but I'm wondering if it would be even nicer if the Xen part was kept out-of-line. That is, could Xen have little stubs like:

xen_alignment_check:
pop %rcx
pop %r11
jmp alignment_check

Quoted 6 lines

rather than using the macros in entry_64.S that you have? Then you could adjust set_trap_gate instead of pack_gate and maybe even do something like:

#define set_trap_gate(..., name, ...) set_native_or_xen_trap_gate(..., name, xen_##name, ...)

I think I'll have something like:

#define pv_trap_entry(name) (xen_pv_domain() ? xen_ ## name : name)

Quoted 9 lines

and use it like:

set_intr_gate(X86_TRAP_AC, pv_trap_entry(alignment_check));

This will avoid having to define macros for all variants of set_intr_gate(), e.g. set_intr_gate_ist(), set_system_intr_gate().

Do you have any objections?

Sounds good to me.

FWIW, I have no real objection to putting the Xen entry right before the native entry and falling through. I don't love the ip -= 3 bit, though, and I think that the PV_ENTRY macro is too magical.

This might be okay, though:

XEN_PV_ENTRY_FALLTHROUGH(foo)
ENTRY(foo)

Quoted 46 lines · Andy Lutomirski

Quoted 34 lines

Quoted 23 lines · Juergen Gross

Quoted 10 lines · Andy Lutomirski

On Mon, Jul 24, 2017 at 7:28 AM, Juergen Gross <[email protected]> wrote:
When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

I think this is a nice cleanup, but I'm wondering if it would be even nicer if the Xen part was kept out-of-line. That is, could Xen have little stubs like:

xen_alignment_check:
pop %rcx
pop %r11
jmp alignment_check

Quoted 6 lines

rather than using the macros in entry_64.S that you have? Then you could adjust set_trap_gate instead of pack_gate and maybe even do something like:

#define set_trap_gate(..., name, ...) set_native_or_xen_trap_gate(..., name, xen_##name, ...)

I think I'll have something like:

#define pv_trap_entry(name) (xen_pv_domain() ? xen_ ## name : name)

Quoted 9 lines

and use it like:

set_intr_gate(X86_TRAP_AC, pv_trap_entry(alignment_check));

This will avoid having to define macros for all variants of set_intr_gate(), e.g. set_intr_gate_ist(), set_system_intr_gate().

Do you have any objections?

Quoted 8 lines

Sounds good to me.

FWIW, I have no real objection to putting the Xen entry right before the native entry and falling through. I don't love the ip -= 3 bit, though, and I think that the PV_ENTRY macro is too magical.

This might be okay, though:

XEN_PV_ENTRY_FALLTHROUGH(foo)
ENTRY(foo)

ENTRY() aligns on 16 byte boundary. So I have to avoid ENTRY(foo) above in the Xen case when I want to fall through.

So either I have to do something like PV_ENTRY (I could avoid the magic "3" by using the xen_foo entry via pv_trap_entry()), or I need the stub with "jmp" for Xen.

Juergen

Quoted 46 lines · Andy Lutomirski

Quoted 34 lines

Quoted 23 lines · Juergen Gross

Quoted 10 lines · Andy Lutomirski

On Mon, Jul 24, 2017 at 7:28 AM, Juergen Gross <[email protected]> wrote:
When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

I think this is a nice cleanup, but I'm wondering if it would be even nicer if the Xen part was kept out-of-line. That is, could Xen have little stubs like:

xen_alignment_check:
pop %rcx
pop %r11
jmp alignment_check

Quoted 6 lines

rather than using the macros in entry_64.S that you have? Then you could adjust set_trap_gate instead of pack_gate and maybe even do something like:

#define set_trap_gate(..., name, ...) set_native_or_xen_trap_gate(..., name, xen_##name, ...)

I think I'll have something like:

#define pv_trap_entry(name) (xen_pv_domain() ? xen_ ## name : name)

Quoted 9 lines

and use it like:

set_intr_gate(X86_TRAP_AC, pv_trap_entry(alignment_check));

This will avoid having to define macros for all variants of set_intr_gate(), e.g. set_intr_gate_ist(), set_system_intr_gate().

Do you have any objections?

Quoted 8 lines

Sounds good to me.

FWIW, I have no real objection to putting the Xen entry right before the native entry and falling through. I don't love the ip -= 3 bit, though, and I think that the PV_ENTRY macro is too magical.

This might be okay, though:

XEN_PV_ENTRY_FALLTHROUGH(foo)
ENTRY(foo)

ENTRY() aligns on 16 byte boundary. So I have to avoid ENTRY(foo) above in the Xen case when I want to fall through.

So either I have to do something like PV_ENTRY (I could avoid the magic "3" by using the xen_foo entry via pv_trap_entry()), or I need the stub with "jmp" for Xen.

Juergen

Quoted 20 lines · Andy Lutomirski

Quoted 8 lines · Juergen Gross

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

I think this is a nice cleanup, but I'm wondering if it would be even nicer if the Xen part was kept out-of-line. That is, could Xen have little stubs like:

xen_alignment_check:
pop %rcx
pop %r11
jmp alignment_check

Quoted 6 lines

rather than using the macros in entry_64.S that you have? Then you could adjust set_trap_gate instead of pack_gate and maybe even do something like:

#define set_trap_gate(..., name, ...) set_native_or_xen_trap_gate(..., name, xen_##name, ...)

I think I'll have something like:

#define pv_trap_entry(name) (xen_pv_domain() ? xen_ ## name : name)

and use it like:

set_intr_gate(X86_TRAP_AC, pv_trap_entry(alignment_check));

This will avoid having to define macros for all variants of set_intr_gate(), e.g. set_intr_gate_ist(), set_system_intr_gate().

Do you have any objections?

Juergen

Quoted 7 lines · Juergen Gross

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.

I think this is a nice cleanup, but I'm wondering if it would be even nicer if the Xen part was kept out-of-line. That is, could Xen have little stubs like:

xen_alignment_check:
pop %rcx
pop %r11
jmp alignment_check

rather than using the macros in entry_64.S that you have? Then you could adjust set_trap_gate instead of pack_gate and maybe even do something like:

#define set_trap_gate(..., name, ...) set_native_or_xen_trap_gate(..., name, xen_##name, ...)

When running as Xen pv-guest the exception frame on the stack contains %r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type prepend the Xen specific code to each exception entry. When running as Xen pv-guest just use the exception entry with prepended instructions, otherwise use the entry without the Xen specific code.