* Re: [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-03 22:51 [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop Andy Lutomirski
@ 2015-04-03 22:52 ` Andy Lutomirski
2015-04-06 14:10 ` Konrad Rzeszutek Wilk
2015-04-06 14:10 ` Konrad Rzeszutek Wilk
2015-04-21 18:56 ` Andy Lutomirski
2015-04-22 14:09 ` [tip:x86/asm] x86, paravirt, xen: Remove the 64-bit -> irq_enable_sysexit() pvop tip-bot for Andy Lutomirski
2 siblings, 2 replies; 17+ messages in thread
From: Andy Lutomirski @ 2015-04-03 22:52 UTC (permalink / raw)
To: Andy Lutomirski, Boris Ostrovsky, Konrad Rzeszutek Wilk
Cc: Linux Kernel Mailing List, X86 ML, Ingo Molnar, Denys Vlasenko,
Linus Torvalds, Denys Vlasenko, H. Peter Anvin, Borislav Petkov
[cc: Boris and Konrad. Whoops]
On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org> wrote:
> We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
> all the paravirt and Xen machinery to support it on 64-bit kernels.
>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>
> I haven't actually tested this on Xen, but it builds for me.
>
> arch/x86/ia32/ia32entry.S | 6 ------
> arch/x86/include/asm/paravirt_types.h | 7 ++++---
> arch/x86/kernel/asm-offsets.c | 2 ++
> arch/x86/kernel/paravirt.c | 4 +++-
> arch/x86/kernel/paravirt_patch_64.c | 1 -
> arch/x86/xen/enlighten.c | 3 ++-
> arch/x86/xen/xen-asm_64.S | 16 ----------------
> arch/x86/xen/xen-ops.h | 2 ++
> 8 files changed, 13 insertions(+), 28 deletions(-)
>
> diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
> index 5d8f987a340d..eb1eb7b70f4b 100644
> --- a/arch/x86/ia32/ia32entry.S
> +++ b/arch/x86/ia32/ia32entry.S
> @@ -77,12 +77,6 @@ ENTRY(native_usergs_sysret32)
> swapgs
> sysretl
> ENDPROC(native_usergs_sysret32)
> -
> -ENTRY(native_irq_enable_sysexit)
> - swapgs
> - sti
> - sysexit
> -ENDPROC(native_irq_enable_sysexit)
> #endif
>
> /*
> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> index 7549b8b369e4..38a0ff9ef06e 100644
> --- a/arch/x86/include/asm/paravirt_types.h
> +++ b/arch/x86/include/asm/paravirt_types.h
> @@ -160,13 +160,14 @@ struct pv_cpu_ops {
> u64 (*read_pmc)(int counter);
> unsigned long long (*read_tscp)(unsigned int *aux);
>
> +#ifdef CONFIG_X86_32
> /*
> * Atomically enable interrupts and return to userspace. This
> - * is only ever used to return to 32-bit processes; in a
> - * 64-bit kernel, it's used for 32-on-64 compat processes, but
> - * never native 64-bit processes. (Jump, not call.)
> + * is only used in 32-bit kernels. 64-bit kernels use
> + * usergs_sysret32 instead.
> */
> void (*irq_enable_sysexit)(void);
> +#endif
>
> /*
> * Switch to usermode gs and return to 64-bit usermode using
> diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
> index 9f6b9341950f..2d27ebf0aed8 100644
> --- a/arch/x86/kernel/asm-offsets.c
> +++ b/arch/x86/kernel/asm-offsets.c
> @@ -49,7 +49,9 @@ void common(void) {
> OFFSET(PV_IRQ_irq_disable, pv_irq_ops, irq_disable);
> OFFSET(PV_IRQ_irq_enable, pv_irq_ops, irq_enable);
> OFFSET(PV_CPU_iret, pv_cpu_ops, iret);
> +#ifdef CONFIG_X86_32
> OFFSET(PV_CPU_irq_enable_sysexit, pv_cpu_ops, irq_enable_sysexit);
> +#endif
> OFFSET(PV_CPU_read_cr0, pv_cpu_ops, read_cr0);
> OFFSET(PV_MMU_read_cr2, pv_mmu_ops, read_cr2);
> #endif
> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> index 548d25f00c90..7563114d9c3a 100644
> --- a/arch/x86/kernel/paravirt.c
> +++ b/arch/x86/kernel/paravirt.c
> @@ -154,7 +154,9 @@ unsigned paravirt_patch_default(u8 type, u16 clobbers, void *insnbuf,
> ret = paravirt_patch_ident_64(insnbuf, len);
>
> else if (type == PARAVIRT_PATCH(pv_cpu_ops.iret) ||
> +#ifdef CONFIG_X86_32
> type == PARAVIRT_PATCH(pv_cpu_ops.irq_enable_sysexit) ||
> +#endif
> type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret32) ||
> type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret64))
> /* If operation requires a jmp, then jmp */
> @@ -371,7 +373,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
>
> .load_sp0 = native_load_sp0,
>
> -#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
> +#if defined(CONFIG_X86_32)
> .irq_enable_sysexit = native_irq_enable_sysexit,
> #endif
> #ifdef CONFIG_X86_64
> diff --git a/arch/x86/kernel/paravirt_patch_64.c b/arch/x86/kernel/paravirt_patch_64.c
> index a1da6737ba5b..0de21c62c348 100644
> --- a/arch/x86/kernel/paravirt_patch_64.c
> +++ b/arch/x86/kernel/paravirt_patch_64.c
> @@ -49,7 +49,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
> PATCH_SITE(pv_irq_ops, save_fl);
> PATCH_SITE(pv_irq_ops, irq_enable);
> PATCH_SITE(pv_irq_ops, irq_disable);
> - PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
> PATCH_SITE(pv_cpu_ops, usergs_sysret32);
> PATCH_SITE(pv_cpu_ops, usergs_sysret64);
> PATCH_SITE(pv_cpu_ops, swapgs);
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 81665c9f2132..3797b6b31f95 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1267,10 +1267,11 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
> .read_tscp = native_read_tscp,
>
> .iret = xen_iret,
> - .irq_enable_sysexit = xen_sysexit,
> #ifdef CONFIG_X86_64
> .usergs_sysret32 = xen_sysret32,
> .usergs_sysret64 = xen_sysret64,
> +#else
> + .irq_enable_sysexit = xen_sysexit,
> #endif
>
> .load_tr_desc = paravirt_nop,
> diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
> index 985fc3ee0973..a2cabb8bd6bf 100644
> --- a/arch/x86/xen/xen-asm_64.S
> +++ b/arch/x86/xen/xen-asm_64.S
> @@ -47,22 +47,6 @@ ENTRY(xen_iret)
> ENDPATCH(xen_iret)
> RELOC(xen_iret, 1b+1)
>
> -/*
> - * sysexit is not used for 64-bit processes, so it's only ever used to
> - * return to 32-bit compat userspace.
> - */
> -ENTRY(xen_sysexit)
> - pushq $__USER32_DS
> - pushq %rcx
> - pushq $X86_EFLAGS_IF
> - pushq $__USER32_CS
> - pushq %rdx
> -
> - pushq $0
> -1: jmp hypercall_iret
> -ENDPATCH(xen_sysexit)
> -RELOC(xen_sysexit, 1b+1)
> -
> ENTRY(xen_sysret64)
> /*
> * We're already on the usermode stack at this point, but
> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
> index 9e195c683549..c20fe29e65f4 100644
> --- a/arch/x86/xen/xen-ops.h
> +++ b/arch/x86/xen/xen-ops.h
> @@ -134,7 +134,9 @@ DECL_ASM(void, xen_restore_fl_direct, unsigned long);
>
> /* These are not functions, and cannot be called normally */
> __visible void xen_iret(void);
> +#ifdef CONFIG_X86_32
> __visible void xen_sysexit(void);
> +#endif
> __visible void xen_sysret32(void);
> __visible void xen_sysret64(void);
> __visible void xen_adjust_exception_frame(void);
> --
> 2.3.0
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-03 22:52 ` Andy Lutomirski
@ 2015-04-06 14:10 ` Konrad Rzeszutek Wilk
2015-04-06 15:29 ` Andy Lutomirski
2015-04-06 15:29 ` Andy Lutomirski
2015-04-06 14:10 ` Konrad Rzeszutek Wilk
1 sibling, 2 replies; 17+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-04-06 14:10 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andy Lutomirski, Boris Ostrovsky, Linux Kernel Mailing List,
X86 ML, Ingo Molnar, Denys Vlasenko, Linus Torvalds,
Denys Vlasenko, H. Peter Anvin, Borislav Petkov, xen-devel
On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
> [cc: Boris and Konrad. Whoops]
>
> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org> wrote:
> > We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
Is there an commit (or name of patch) that explains why 32-bit-user-space-on-64-bit
kernels is unsavory?
Thank you!
> > all the paravirt and Xen machinery to support it on 64-bit kernels.
> >
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> >
> > I haven't actually tested this on Xen, but it builds for me.
> >
> > arch/x86/ia32/ia32entry.S | 6 ------
> > arch/x86/include/asm/paravirt_types.h | 7 ++++---
> > arch/x86/kernel/asm-offsets.c | 2 ++
> > arch/x86/kernel/paravirt.c | 4 +++-
> > arch/x86/kernel/paravirt_patch_64.c | 1 -
> > arch/x86/xen/enlighten.c | 3 ++-
> > arch/x86/xen/xen-asm_64.S | 16 ----------------
> > arch/x86/xen/xen-ops.h | 2 ++
> > 8 files changed, 13 insertions(+), 28 deletions(-)
> >
> > diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
> > index 5d8f987a340d..eb1eb7b70f4b 100644
> > --- a/arch/x86/ia32/ia32entry.S
> > +++ b/arch/x86/ia32/ia32entry.S
> > @@ -77,12 +77,6 @@ ENTRY(native_usergs_sysret32)
> > swapgs
> > sysretl
> > ENDPROC(native_usergs_sysret32)
> > -
> > -ENTRY(native_irq_enable_sysexit)
> > - swapgs
> > - sti
> > - sysexit
> > -ENDPROC(native_irq_enable_sysexit)
> > #endif
> >
> > /*
> > diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> > index 7549b8b369e4..38a0ff9ef06e 100644
> > --- a/arch/x86/include/asm/paravirt_types.h
> > +++ b/arch/x86/include/asm/paravirt_types.h
> > @@ -160,13 +160,14 @@ struct pv_cpu_ops {
> > u64 (*read_pmc)(int counter);
> > unsigned long long (*read_tscp)(unsigned int *aux);
> >
> > +#ifdef CONFIG_X86_32
> > /*
> > * Atomically enable interrupts and return to userspace. This
> > - * is only ever used to return to 32-bit processes; in a
> > - * 64-bit kernel, it's used for 32-on-64 compat processes, but
> > - * never native 64-bit processes. (Jump, not call.)
> > + * is only used in 32-bit kernels. 64-bit kernels use
> > + * usergs_sysret32 instead.
> > */
> > void (*irq_enable_sysexit)(void);
> > +#endif
> >
> > /*
> > * Switch to usermode gs and return to 64-bit usermode using
> > diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
> > index 9f6b9341950f..2d27ebf0aed8 100644
> > --- a/arch/x86/kernel/asm-offsets.c
> > +++ b/arch/x86/kernel/asm-offsets.c
> > @@ -49,7 +49,9 @@ void common(void) {
> > OFFSET(PV_IRQ_irq_disable, pv_irq_ops, irq_disable);
> > OFFSET(PV_IRQ_irq_enable, pv_irq_ops, irq_enable);
> > OFFSET(PV_CPU_iret, pv_cpu_ops, iret);
> > +#ifdef CONFIG_X86_32
> > OFFSET(PV_CPU_irq_enable_sysexit, pv_cpu_ops, irq_enable_sysexit);
> > +#endif
> > OFFSET(PV_CPU_read_cr0, pv_cpu_ops, read_cr0);
> > OFFSET(PV_MMU_read_cr2, pv_mmu_ops, read_cr2);
> > #endif
> > diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> > index 548d25f00c90..7563114d9c3a 100644
> > --- a/arch/x86/kernel/paravirt.c
> > +++ b/arch/x86/kernel/paravirt.c
> > @@ -154,7 +154,9 @@ unsigned paravirt_patch_default(u8 type, u16 clobbers, void *insnbuf,
> > ret = paravirt_patch_ident_64(insnbuf, len);
> >
> > else if (type == PARAVIRT_PATCH(pv_cpu_ops.iret) ||
> > +#ifdef CONFIG_X86_32
> > type == PARAVIRT_PATCH(pv_cpu_ops.irq_enable_sysexit) ||
> > +#endif
> > type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret32) ||
> > type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret64))
> > /* If operation requires a jmp, then jmp */
> > @@ -371,7 +373,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
> >
> > .load_sp0 = native_load_sp0,
> >
> > -#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
> > +#if defined(CONFIG_X86_32)
> > .irq_enable_sysexit = native_irq_enable_sysexit,
> > #endif
> > #ifdef CONFIG_X86_64
> > diff --git a/arch/x86/kernel/paravirt_patch_64.c b/arch/x86/kernel/paravirt_patch_64.c
> > index a1da6737ba5b..0de21c62c348 100644
> > --- a/arch/x86/kernel/paravirt_patch_64.c
> > +++ b/arch/x86/kernel/paravirt_patch_64.c
> > @@ -49,7 +49,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
> > PATCH_SITE(pv_irq_ops, save_fl);
> > PATCH_SITE(pv_irq_ops, irq_enable);
> > PATCH_SITE(pv_irq_ops, irq_disable);
> > - PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
> > PATCH_SITE(pv_cpu_ops, usergs_sysret32);
> > PATCH_SITE(pv_cpu_ops, usergs_sysret64);
> > PATCH_SITE(pv_cpu_ops, swapgs);
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index 81665c9f2132..3797b6b31f95 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1267,10 +1267,11 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
> > .read_tscp = native_read_tscp,
> >
> > .iret = xen_iret,
> > - .irq_enable_sysexit = xen_sysexit,
> > #ifdef CONFIG_X86_64
> > .usergs_sysret32 = xen_sysret32,
> > .usergs_sysret64 = xen_sysret64,
> > +#else
> > + .irq_enable_sysexit = xen_sysexit,
> > #endif
> >
> > .load_tr_desc = paravirt_nop,
> > diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
> > index 985fc3ee0973..a2cabb8bd6bf 100644
> > --- a/arch/x86/xen/xen-asm_64.S
> > +++ b/arch/x86/xen/xen-asm_64.S
> > @@ -47,22 +47,6 @@ ENTRY(xen_iret)
> > ENDPATCH(xen_iret)
> > RELOC(xen_iret, 1b+1)
> >
> > -/*
> > - * sysexit is not used for 64-bit processes, so it's only ever used to
> > - * return to 32-bit compat userspace.
> > - */
> > -ENTRY(xen_sysexit)
> > - pushq $__USER32_DS
> > - pushq %rcx
> > - pushq $X86_EFLAGS_IF
> > - pushq $__USER32_CS
> > - pushq %rdx
> > -
> > - pushq $0
> > -1: jmp hypercall_iret
> > -ENDPATCH(xen_sysexit)
> > -RELOC(xen_sysexit, 1b+1)
> > -
> > ENTRY(xen_sysret64)
> > /*
> > * We're already on the usermode stack at this point, but
> > diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
> > index 9e195c683549..c20fe29e65f4 100644
> > --- a/arch/x86/xen/xen-ops.h
> > +++ b/arch/x86/xen/xen-ops.h
> > @@ -134,7 +134,9 @@ DECL_ASM(void, xen_restore_fl_direct, unsigned long);
> >
> > /* These are not functions, and cannot be called normally */
> > __visible void xen_iret(void);
> > +#ifdef CONFIG_X86_32
> > __visible void xen_sysexit(void);
> > +#endif
> > __visible void xen_sysret32(void);
> > __visible void xen_sysret64(void);
> > __visible void xen_adjust_exception_frame(void);
> > --
> > 2.3.0
> >
>
>
>
> --
> Andy Lutomirski
> AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-06 14:10 ` Konrad Rzeszutek Wilk
@ 2015-04-06 15:29 ` Andy Lutomirski
2015-04-06 17:44 ` Andrew Cooper
2015-04-06 15:29 ` Andy Lutomirski
1 sibling, 1 reply; 17+ messages in thread
From: Andy Lutomirski @ 2015-04-06 15:29 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Andy Lutomirski, Boris Ostrovsky, Linux Kernel Mailing List,
X86 ML, Ingo Molnar, Denys Vlasenko, Linus Torvalds,
Denys Vlasenko, H. Peter Anvin, Borislav Petkov,
xen-devel@lists.xenproject.org
On Mon, Apr 6, 2015 at 7:10 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
>> [cc: Boris and Konrad. Whoops]
>>
>> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> > We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
>
> Is there an commit (or name of patch) that explains why 32-bit-user-space-on-64-bit
> kernels is unsavory?
sysexit never tasted very good :-p
We're (hopefully) not breaking 32-bit-user-space-on-64-bit, but we're
trying an unconventional approach to making the code faster and less
scary. As a result, 64-bit kernels won't use sysexit any more.
Hopefully Xen is okay with the slightly sneaky thing we're doing.
AFAICT Xen thinks of sysretl and sysexit as slightly funny irets, so I
don't expect there to be any problem.
See:
https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=x86/asm&id=4214a16b02971c60960afd675d03544e109e0d75
and
https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=x86/asm&id=47091e3c5b072daca29a15d2a3caf40359b0d140
--Andy
>
> Thank you!
>> > all the paravirt and Xen machinery to support it on 64-bit kernels.
>> >
>> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> > ---
>> >
>> > I haven't actually tested this on Xen, but it builds for me.
>> >
>> > arch/x86/ia32/ia32entry.S | 6 ------
>> > arch/x86/include/asm/paravirt_types.h | 7 ++++---
>> > arch/x86/kernel/asm-offsets.c | 2 ++
>> > arch/x86/kernel/paravirt.c | 4 +++-
>> > arch/x86/kernel/paravirt_patch_64.c | 1 -
>> > arch/x86/xen/enlighten.c | 3 ++-
>> > arch/x86/xen/xen-asm_64.S | 16 ----------------
>> > arch/x86/xen/xen-ops.h | 2 ++
>> > 8 files changed, 13 insertions(+), 28 deletions(-)
>> >
>> > diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
>> > index 5d8f987a340d..eb1eb7b70f4b 100644
>> > --- a/arch/x86/ia32/ia32entry.S
>> > +++ b/arch/x86/ia32/ia32entry.S
>> > @@ -77,12 +77,6 @@ ENTRY(native_usergs_sysret32)
>> > swapgs
>> > sysretl
>> > ENDPROC(native_usergs_sysret32)
>> > -
>> > -ENTRY(native_irq_enable_sysexit)
>> > - swapgs
>> > - sti
>> > - sysexit
>> > -ENDPROC(native_irq_enable_sysexit)
>> > #endif
>> >
>> > /*
>> > diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
>> > index 7549b8b369e4..38a0ff9ef06e 100644
>> > --- a/arch/x86/include/asm/paravirt_types.h
>> > +++ b/arch/x86/include/asm/paravirt_types.h
>> > @@ -160,13 +160,14 @@ struct pv_cpu_ops {
>> > u64 (*read_pmc)(int counter);
>> > unsigned long long (*read_tscp)(unsigned int *aux);
>> >
>> > +#ifdef CONFIG_X86_32
>> > /*
>> > * Atomically enable interrupts and return to userspace. This
>> > - * is only ever used to return to 32-bit processes; in a
>> > - * 64-bit kernel, it's used for 32-on-64 compat processes, but
>> > - * never native 64-bit processes. (Jump, not call.)
>> > + * is only used in 32-bit kernels. 64-bit kernels use
>> > + * usergs_sysret32 instead.
>> > */
>> > void (*irq_enable_sysexit)(void);
>> > +#endif
>> >
>> > /*
>> > * Switch to usermode gs and return to 64-bit usermode using
>> > diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
>> > index 9f6b9341950f..2d27ebf0aed8 100644
>> > --- a/arch/x86/kernel/asm-offsets.c
>> > +++ b/arch/x86/kernel/asm-offsets.c
>> > @@ -49,7 +49,9 @@ void common(void) {
>> > OFFSET(PV_IRQ_irq_disable, pv_irq_ops, irq_disable);
>> > OFFSET(PV_IRQ_irq_enable, pv_irq_ops, irq_enable);
>> > OFFSET(PV_CPU_iret, pv_cpu_ops, iret);
>> > +#ifdef CONFIG_X86_32
>> > OFFSET(PV_CPU_irq_enable_sysexit, pv_cpu_ops, irq_enable_sysexit);
>> > +#endif
>> > OFFSET(PV_CPU_read_cr0, pv_cpu_ops, read_cr0);
>> > OFFSET(PV_MMU_read_cr2, pv_mmu_ops, read_cr2);
>> > #endif
>> > diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
>> > index 548d25f00c90..7563114d9c3a 100644
>> > --- a/arch/x86/kernel/paravirt.c
>> > +++ b/arch/x86/kernel/paravirt.c
>> > @@ -154,7 +154,9 @@ unsigned paravirt_patch_default(u8 type, u16 clobbers, void *insnbuf,
>> > ret = paravirt_patch_ident_64(insnbuf, len);
>> >
>> > else if (type == PARAVIRT_PATCH(pv_cpu_ops.iret) ||
>> > +#ifdef CONFIG_X86_32
>> > type == PARAVIRT_PATCH(pv_cpu_ops.irq_enable_sysexit) ||
>> > +#endif
>> > type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret32) ||
>> > type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret64))
>> > /* If operation requires a jmp, then jmp */
>> > @@ -371,7 +373,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
>> >
>> > .load_sp0 = native_load_sp0,
>> >
>> > -#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
>> > +#if defined(CONFIG_X86_32)
>> > .irq_enable_sysexit = native_irq_enable_sysexit,
>> > #endif
>> > #ifdef CONFIG_X86_64
>> > diff --git a/arch/x86/kernel/paravirt_patch_64.c b/arch/x86/kernel/paravirt_patch_64.c
>> > index a1da6737ba5b..0de21c62c348 100644
>> > --- a/arch/x86/kernel/paravirt_patch_64.c
>> > +++ b/arch/x86/kernel/paravirt_patch_64.c
>> > @@ -49,7 +49,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
>> > PATCH_SITE(pv_irq_ops, save_fl);
>> > PATCH_SITE(pv_irq_ops, irq_enable);
>> > PATCH_SITE(pv_irq_ops, irq_disable);
>> > - PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
>> > PATCH_SITE(pv_cpu_ops, usergs_sysret32);
>> > PATCH_SITE(pv_cpu_ops, usergs_sysret64);
>> > PATCH_SITE(pv_cpu_ops, swapgs);
>> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>> > index 81665c9f2132..3797b6b31f95 100644
>> > --- a/arch/x86/xen/enlighten.c
>> > +++ b/arch/x86/xen/enlighten.c
>> > @@ -1267,10 +1267,11 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
>> > .read_tscp = native_read_tscp,
>> >
>> > .iret = xen_iret,
>> > - .irq_enable_sysexit = xen_sysexit,
>> > #ifdef CONFIG_X86_64
>> > .usergs_sysret32 = xen_sysret32,
>> > .usergs_sysret64 = xen_sysret64,
>> > +#else
>> > + .irq_enable_sysexit = xen_sysexit,
>> > #endif
>> >
>> > .load_tr_desc = paravirt_nop,
>> > diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
>> > index 985fc3ee0973..a2cabb8bd6bf 100644
>> > --- a/arch/x86/xen/xen-asm_64.S
>> > +++ b/arch/x86/xen/xen-asm_64.S
>> > @@ -47,22 +47,6 @@ ENTRY(xen_iret)
>> > ENDPATCH(xen_iret)
>> > RELOC(xen_iret, 1b+1)
>> >
>> > -/*
>> > - * sysexit is not used for 64-bit processes, so it's only ever used to
>> > - * return to 32-bit compat userspace.
>> > - */
>> > -ENTRY(xen_sysexit)
>> > - pushq $__USER32_DS
>> > - pushq %rcx
>> > - pushq $X86_EFLAGS_IF
>> > - pushq $__USER32_CS
>> > - pushq %rdx
>> > -
>> > - pushq $0
>> > -1: jmp hypercall_iret
>> > -ENDPATCH(xen_sysexit)
>> > -RELOC(xen_sysexit, 1b+1)
>> > -
>> > ENTRY(xen_sysret64)
>> > /*
>> > * We're already on the usermode stack at this point, but
>> > diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
>> > index 9e195c683549..c20fe29e65f4 100644
>> > --- a/arch/x86/xen/xen-ops.h
>> > +++ b/arch/x86/xen/xen-ops.h
>> > @@ -134,7 +134,9 @@ DECL_ASM(void, xen_restore_fl_direct, unsigned long);
>> >
>> > /* These are not functions, and cannot be called normally */
>> > __visible void xen_iret(void);
>> > +#ifdef CONFIG_X86_32
>> > __visible void xen_sysexit(void);
>> > +#endif
>> > __visible void xen_sysret32(void);
>> > __visible void xen_sysret64(void);
>> > __visible void xen_adjust_exception_frame(void);
>> > --
>> > 2.3.0
>> >
>>
>>
>>
>> --
>> Andy Lutomirski
>> AMA Capital Management, LLC
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Xen-devel] [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-06 15:29 ` Andy Lutomirski
@ 2015-04-06 17:44 ` Andrew Cooper
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2015-04-06 17:44 UTC (permalink / raw)
To: Andy Lutomirski, Konrad Rzeszutek Wilk
Cc: Denys Vlasenko, Denys Vlasenko, X86 ML, Linux Kernel Mailing List,
Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
xen-devel@lists.xenproject.org, Boris Ostrovsky, Linus Torvalds,
Ingo Molnar
On 06/04/2015 16:29, Andy Lutomirski wrote:
> On Mon, Apr 6, 2015 at 7:10 AM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
>> On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
>>> [cc: Boris and Konrad. Whoops]
>>>
>>> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>>> We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
>> Is there an commit (or name of patch) that explains why 32-bit-user-space-on-64-bit
>> kernels is unsavory?
> sysexit never tasted very good :-p
>
> We're (hopefully) not breaking 32-bit-user-space-on-64-bit, but we're
> trying an unconventional approach to making the code faster and less
> scary. As a result, 64-bit kernels won't use sysexit any more.
> Hopefully Xen is okay with the slightly sneaky thing we're doing.
> AFAICT Xen thinks of sysretl and sysexit as slightly funny irets, so I
> don't expect there to be any problem.
64bit PV kernels must bounce through Xen to switch from the kernel to
the user pagetables (since both kernel and userspace are both actually
running in ring3 with user pages).
As a result, exit to userspace ends up as a hypercall into Xen which has
an effect very similar to an `iret`, but with some extra fixup in the
background.
I can't forsee any Xen issues as a result of this patch.
~Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
@ 2015-04-06 17:44 ` Andrew Cooper
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2015-04-06 17:44 UTC (permalink / raw)
To: Andy Lutomirski, Konrad Rzeszutek Wilk
Cc: Denys Vlasenko, Denys Vlasenko, X86 ML, Linux Kernel Mailing List,
Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
xen-devel@lists.xenproject.org, Boris Ostrovsky, Linus Torvalds,
Ingo Molnar
On 06/04/2015 16:29, Andy Lutomirski wrote:
> On Mon, Apr 6, 2015 at 7:10 AM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
>> On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
>>> [cc: Boris and Konrad. Whoops]
>>>
>>> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>>> We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
>> Is there an commit (or name of patch) that explains why 32-bit-user-space-on-64-bit
>> kernels is unsavory?
> sysexit never tasted very good :-p
>
> We're (hopefully) not breaking 32-bit-user-space-on-64-bit, but we're
> trying an unconventional approach to making the code faster and less
> scary. As a result, 64-bit kernels won't use sysexit any more.
> Hopefully Xen is okay with the slightly sneaky thing we're doing.
> AFAICT Xen thinks of sysretl and sysexit as slightly funny irets, so I
> don't expect there to be any problem.
64bit PV kernels must bounce through Xen to switch from the kernel to
the user pagetables (since both kernel and userspace are both actually
running in ring3 with user pages).
As a result, exit to userspace ends up as a hypercall into Xen which has
an effect very similar to an `iret`, but with some extra fixup in the
background.
I can't forsee any Xen issues as a result of this patch.
~Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-06 17:44 ` Andrew Cooper
(?)
@ 2015-04-06 18:30 ` Boris Ostrovsky
2015-04-06 20:03 ` Andy Lutomirski
2015-04-06 20:03 ` Andy Lutomirski
-1 siblings, 2 replies; 17+ messages in thread
From: Boris Ostrovsky @ 2015-04-06 18:30 UTC (permalink / raw)
To: Andrew Cooper, Andy Lutomirski, Konrad Rzeszutek Wilk
Cc: Denys Vlasenko, Denys Vlasenko, X86 ML, Linux Kernel Mailing List,
Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
xen-devel@lists.xenproject.org, Linus Torvalds, Ingo Molnar
On 04/06/2015 01:44 PM, Andrew Cooper wrote:
> On 06/04/2015 16:29, Andy Lutomirski wrote:
>> On Mon, Apr 6, 2015 at 7:10 AM, Konrad Rzeszutek Wilk
>> <konrad.wilk@oracle.com> wrote:
>>> On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
>>>> [cc: Boris and Konrad. Whoops]
>>>>
>>>> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>>>> We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
>>> Is there an commit (or name of patch) that explains why 32-bit-user-space-on-64-bit
>>> kernels is unsavory?
>> sysexit never tasted very good :-p
>>
>> We're (hopefully) not breaking 32-bit-user-space-on-64-bit, but we're
>> trying an unconventional approach to making the code faster and less
>> scary. As a result, 64-bit kernels won't use sysexit any more.
>> Hopefully Xen is okay with the slightly sneaky thing we're doing.
>> AFAICT Xen thinks of sysretl and sysexit as slightly funny irets, so I
>> don't expect there to be any problem.
> 64bit PV kernels must bounce through Xen to switch from the kernel to
> the user pagetables (since both kernel and userspace are both actually
> running in ring3 with user pages).
>
> As a result, exit to userspace ends up as a hypercall into Xen which has
> an effect very similar to an `iret`, but with some extra fixup in the
> background.
>
> I can't forsee any Xen issues as a result of this patch.
I ran tip plus this patch (plus another patch that fixes an unrelated
Xen regression in tip) through our test suite and it completed without
problems.
I also ran some very simple 32-bit programs in a 64-bit PV guest and
didn't see any problems there neither.
-boris
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-06 18:30 ` [Xen-devel] " Boris Ostrovsky
@ 2015-04-06 20:03 ` Andy Lutomirski
2015-04-06 20:27 ` Boris Ostrovsky
2015-04-06 20:27 ` Boris Ostrovsky
2015-04-06 20:03 ` Andy Lutomirski
1 sibling, 2 replies; 17+ messages in thread
From: Andy Lutomirski @ 2015-04-06 20:03 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: Andrew Cooper, Konrad Rzeszutek Wilk, Denys Vlasenko,
Denys Vlasenko, X86 ML, Linux Kernel Mailing List,
Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
xen-devel@lists.xenproject.org, Linus Torvalds, Ingo Molnar
On Mon, Apr 6, 2015 at 11:30 AM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
>
> On 04/06/2015 01:44 PM, Andrew Cooper wrote:
>>
>> On 06/04/2015 16:29, Andy Lutomirski wrote:
>>>
>>> On Mon, Apr 6, 2015 at 7:10 AM, Konrad Rzeszutek Wilk
>>> <konrad.wilk@oracle.com> wrote:
>>>>
>>>> On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
>>>>>
>>>>> [cc: Boris and Konrad. Whoops]
>>>>>
>>>>> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org>
>>>>> wrote:
>>>>>>
>>>>>> We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
>>>>
>>>> Is there an commit (or name of patch) that explains why
>>>> 32-bit-user-space-on-64-bit
>>>> kernels is unsavory?
>>>
>>> sysexit never tasted very good :-p
>>>
>>> We're (hopefully) not breaking 32-bit-user-space-on-64-bit, but we're
>>> trying an unconventional approach to making the code faster and less
>>> scary. As a result, 64-bit kernels won't use sysexit any more.
>>> Hopefully Xen is okay with the slightly sneaky thing we're doing.
>>> AFAICT Xen thinks of sysretl and sysexit as slightly funny irets, so I
>>> don't expect there to be any problem.
>>
>> 64bit PV kernels must bounce through Xen to switch from the kernel to
>> the user pagetables (since both kernel and userspace are both actually
>> running in ring3 with user pages).
>>
>> As a result, exit to userspace ends up as a hypercall into Xen which has
>> an effect very similar to an `iret`, but with some extra fixup in the
>> background.
>>
>> I can't forsee any Xen issues as a result of this patch.
>
>
>
> I ran tip plus this patch (plus another patch that fixes an unrelated Xen
> regression in tip) through our test suite and it completed without problems.
>
> I also ran some very simple 32-bit programs in a 64-bit PV guest and didn't
> see any problems there neither.
At the risk of redundancy, did you test on Intel hardware? At least
on native systems, the code in question never executes on AMD systems.
--Andy
>
> -boris
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Xen-devel] [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-06 20:03 ` Andy Lutomirski
@ 2015-04-06 20:27 ` Boris Ostrovsky
2015-04-06 20:27 ` Boris Ostrovsky
1 sibling, 0 replies; 17+ messages in thread
From: Boris Ostrovsky @ 2015-04-06 20:27 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andrew Cooper, Konrad Rzeszutek Wilk, Denys Vlasenko,
Denys Vlasenko, X86 ML, Linux Kernel Mailing List,
Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
xen-devel@lists.xenproject.org, Linus Torvalds, Ingo Molnar
On 04/06/2015 04:03 PM, Andy Lutomirski wrote:
> On Mon, Apr 6, 2015 at 11:30 AM, Boris Ostrovsky
> <boris.ostrovsky@oracle.com> wrote:
>> On 04/06/2015 01:44 PM, Andrew Cooper wrote:
>>> On 06/04/2015 16:29, Andy Lutomirski wrote:
>>>> On Mon, Apr 6, 2015 at 7:10 AM, Konrad Rzeszutek Wilk
>>>> <konrad.wilk@oracle.com> wrote:
>>>>> On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
>>>>>> [cc: Boris and Konrad. Whoops]
>>>>>>
>>>>>> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org>
>>>>>> wrote:
>>>>>>> We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
>>>>> Is there an commit (or name of patch) that explains why
>>>>> 32-bit-user-space-on-64-bit
>>>>> kernels is unsavory?
>>>> sysexit never tasted very good :-p
>>>>
>>>> We're (hopefully) not breaking 32-bit-user-space-on-64-bit, but we're
>>>> trying an unconventional approach to making the code faster and less
>>>> scary. As a result, 64-bit kernels won't use sysexit any more.
>>>> Hopefully Xen is okay with the slightly sneaky thing we're doing.
>>>> AFAICT Xen thinks of sysretl and sysexit as slightly funny irets, so I
>>>> don't expect there to be any problem.
>>> 64bit PV kernels must bounce through Xen to switch from the kernel to
>>> the user pagetables (since both kernel and userspace are both actually
>>> running in ring3 with user pages).
>>>
>>> As a result, exit to userspace ends up as a hypercall into Xen which has
>>> an effect very similar to an `iret`, but with some extra fixup in the
>>> background.
>>>
>>> I can't forsee any Xen issues as a result of this patch.
>>
>>
>> I ran tip plus this patch (plus another patch that fixes an unrelated Xen
>> regression in tip) through our test suite and it completed without problems.
>>
>> I also ran some very simple 32-bit programs in a 64-bit PV guest and didn't
>> see any problems there neither.
> At the risk of redundancy, did you test on Intel hardware? At least
> on native systems, the code in question never executes on AMD systems.
Yes, the tests ran on Intel. I left them scheduled for overnight runs
too and that will be executed on both AMD and Intel.
-boris
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-06 20:03 ` Andy Lutomirski
2015-04-06 20:27 ` Boris Ostrovsky
@ 2015-04-06 20:27 ` Boris Ostrovsky
1 sibling, 0 replies; 17+ messages in thread
From: Boris Ostrovsky @ 2015-04-06 20:27 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Denys Vlasenko, Denys Vlasenko, Andrew Cooper, X86 ML,
Linux Kernel Mailing List, Borislav Petkov, Andy Lutomirski,
H. Peter Anvin, xen-devel@lists.xenproject.org, Linus Torvalds,
Ingo Molnar
On 04/06/2015 04:03 PM, Andy Lutomirski wrote:
> On Mon, Apr 6, 2015 at 11:30 AM, Boris Ostrovsky
> <boris.ostrovsky@oracle.com> wrote:
>> On 04/06/2015 01:44 PM, Andrew Cooper wrote:
>>> On 06/04/2015 16:29, Andy Lutomirski wrote:
>>>> On Mon, Apr 6, 2015 at 7:10 AM, Konrad Rzeszutek Wilk
>>>> <konrad.wilk@oracle.com> wrote:
>>>>> On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
>>>>>> [cc: Boris and Konrad. Whoops]
>>>>>>
>>>>>> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org>
>>>>>> wrote:
>>>>>>> We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
>>>>> Is there an commit (or name of patch) that explains why
>>>>> 32-bit-user-space-on-64-bit
>>>>> kernels is unsavory?
>>>> sysexit never tasted very good :-p
>>>>
>>>> We're (hopefully) not breaking 32-bit-user-space-on-64-bit, but we're
>>>> trying an unconventional approach to making the code faster and less
>>>> scary. As a result, 64-bit kernels won't use sysexit any more.
>>>> Hopefully Xen is okay with the slightly sneaky thing we're doing.
>>>> AFAICT Xen thinks of sysretl and sysexit as slightly funny irets, so I
>>>> don't expect there to be any problem.
>>> 64bit PV kernels must bounce through Xen to switch from the kernel to
>>> the user pagetables (since both kernel and userspace are both actually
>>> running in ring3 with user pages).
>>>
>>> As a result, exit to userspace ends up as a hypercall into Xen which has
>>> an effect very similar to an `iret`, but with some extra fixup in the
>>> background.
>>>
>>> I can't forsee any Xen issues as a result of this patch.
>>
>>
>> I ran tip plus this patch (plus another patch that fixes an unrelated Xen
>> regression in tip) through our test suite and it completed without problems.
>>
>> I also ran some very simple 32-bit programs in a 64-bit PV guest and didn't
>> see any problems there neither.
> At the risk of redundancy, did you test on Intel hardware? At least
> on native systems, the code in question never executes on AMD systems.
Yes, the tests ran on Intel. I left them scheduled for overnight runs
too and that will be executed on both AMD and Intel.
-boris
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-06 18:30 ` [Xen-devel] " Boris Ostrovsky
2015-04-06 20:03 ` Andy Lutomirski
@ 2015-04-06 20:03 ` Andy Lutomirski
1 sibling, 0 replies; 17+ messages in thread
From: Andy Lutomirski @ 2015-04-06 20:03 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: Denys Vlasenko, Denys Vlasenko, Andrew Cooper, X86 ML,
Linux Kernel Mailing List, Borislav Petkov, Andy Lutomirski,
H. Peter Anvin, xen-devel@lists.xenproject.org, Linus Torvalds,
Ingo Molnar
On Mon, Apr 6, 2015 at 11:30 AM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
>
> On 04/06/2015 01:44 PM, Andrew Cooper wrote:
>>
>> On 06/04/2015 16:29, Andy Lutomirski wrote:
>>>
>>> On Mon, Apr 6, 2015 at 7:10 AM, Konrad Rzeszutek Wilk
>>> <konrad.wilk@oracle.com> wrote:
>>>>
>>>> On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
>>>>>
>>>>> [cc: Boris and Konrad. Whoops]
>>>>>
>>>>> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org>
>>>>> wrote:
>>>>>>
>>>>>> We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
>>>>
>>>> Is there an commit (or name of patch) that explains why
>>>> 32-bit-user-space-on-64-bit
>>>> kernels is unsavory?
>>>
>>> sysexit never tasted very good :-p
>>>
>>> We're (hopefully) not breaking 32-bit-user-space-on-64-bit, but we're
>>> trying an unconventional approach to making the code faster and less
>>> scary. As a result, 64-bit kernels won't use sysexit any more.
>>> Hopefully Xen is okay with the slightly sneaky thing we're doing.
>>> AFAICT Xen thinks of sysretl and sysexit as slightly funny irets, so I
>>> don't expect there to be any problem.
>>
>> 64bit PV kernels must bounce through Xen to switch from the kernel to
>> the user pagetables (since both kernel and userspace are both actually
>> running in ring3 with user pages).
>>
>> As a result, exit to userspace ends up as a hypercall into Xen which has
>> an effect very similar to an `iret`, but with some extra fixup in the
>> background.
>>
>> I can't forsee any Xen issues as a result of this patch.
>
>
>
> I ran tip plus this patch (plus another patch that fixes an unrelated Xen
> regression in tip) through our test suite and it completed without problems.
>
> I also ran some very simple 32-bit programs in a 64-bit PV guest and didn't
> see any problems there neither.
At the risk of redundancy, did you test on Intel hardware? At least
on native systems, the code in question never executes on AMD systems.
--Andy
>
> -boris
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-06 17:44 ` Andrew Cooper
(?)
(?)
@ 2015-04-06 18:30 ` Boris Ostrovsky
-1 siblings, 0 replies; 17+ messages in thread
From: Boris Ostrovsky @ 2015-04-06 18:30 UTC (permalink / raw)
To: Andrew Cooper, Andy Lutomirski, Konrad Rzeszutek Wilk
Cc: Denys Vlasenko, Denys Vlasenko, X86 ML, Linux Kernel Mailing List,
Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
xen-devel@lists.xenproject.org, Linus Torvalds, Ingo Molnar
On 04/06/2015 01:44 PM, Andrew Cooper wrote:
> On 06/04/2015 16:29, Andy Lutomirski wrote:
>> On Mon, Apr 6, 2015 at 7:10 AM, Konrad Rzeszutek Wilk
>> <konrad.wilk@oracle.com> wrote:
>>> On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
>>>> [cc: Boris and Konrad. Whoops]
>>>>
>>>> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>>>> We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
>>> Is there an commit (or name of patch) that explains why 32-bit-user-space-on-64-bit
>>> kernels is unsavory?
>> sysexit never tasted very good :-p
>>
>> We're (hopefully) not breaking 32-bit-user-space-on-64-bit, but we're
>> trying an unconventional approach to making the code faster and less
>> scary. As a result, 64-bit kernels won't use sysexit any more.
>> Hopefully Xen is okay with the slightly sneaky thing we're doing.
>> AFAICT Xen thinks of sysretl and sysexit as slightly funny irets, so I
>> don't expect there to be any problem.
> 64bit PV kernels must bounce through Xen to switch from the kernel to
> the user pagetables (since both kernel and userspace are both actually
> running in ring3 with user pages).
>
> As a result, exit to userspace ends up as a hypercall into Xen which has
> an effect very similar to an `iret`, but with some extra fixup in the
> background.
>
> I can't forsee any Xen issues as a result of this patch.
I ran tip plus this patch (plus another patch that fixes an unrelated
Xen regression in tip) through our test suite and it completed without
problems.
I also ran some very simple 32-bit programs in a 64-bit PV guest and
didn't see any problems there neither.
-boris
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-06 14:10 ` Konrad Rzeszutek Wilk
2015-04-06 15:29 ` Andy Lutomirski
@ 2015-04-06 15:29 ` Andy Lutomirski
1 sibling, 0 replies; 17+ messages in thread
From: Andy Lutomirski @ 2015-04-06 15:29 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Denys Vlasenko, Denys Vlasenko, X86 ML, Linux Kernel Mailing List,
Borislav Petkov, Andy Lutomirski, H. Peter Anvin,
xen-devel@lists.xenproject.org, Boris Ostrovsky, Linus Torvalds,
Ingo Molnar
On Mon, Apr 6, 2015 at 7:10 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
>> [cc: Boris and Konrad. Whoops]
>>
>> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> > We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
>
> Is there an commit (or name of patch) that explains why 32-bit-user-space-on-64-bit
> kernels is unsavory?
sysexit never tasted very good :-p
We're (hopefully) not breaking 32-bit-user-space-on-64-bit, but we're
trying an unconventional approach to making the code faster and less
scary. As a result, 64-bit kernels won't use sysexit any more.
Hopefully Xen is okay with the slightly sneaky thing we're doing.
AFAICT Xen thinks of sysretl and sysexit as slightly funny irets, so I
don't expect there to be any problem.
See:
https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=x86/asm&id=4214a16b02971c60960afd675d03544e109e0d75
and
https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=x86/asm&id=47091e3c5b072daca29a15d2a3caf40359b0d140
--Andy
>
> Thank you!
>> > all the paravirt and Xen machinery to support it on 64-bit kernels.
>> >
>> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> > ---
>> >
>> > I haven't actually tested this on Xen, but it builds for me.
>> >
>> > arch/x86/ia32/ia32entry.S | 6 ------
>> > arch/x86/include/asm/paravirt_types.h | 7 ++++---
>> > arch/x86/kernel/asm-offsets.c | 2 ++
>> > arch/x86/kernel/paravirt.c | 4 +++-
>> > arch/x86/kernel/paravirt_patch_64.c | 1 -
>> > arch/x86/xen/enlighten.c | 3 ++-
>> > arch/x86/xen/xen-asm_64.S | 16 ----------------
>> > arch/x86/xen/xen-ops.h | 2 ++
>> > 8 files changed, 13 insertions(+), 28 deletions(-)
>> >
>> > diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
>> > index 5d8f987a340d..eb1eb7b70f4b 100644
>> > --- a/arch/x86/ia32/ia32entry.S
>> > +++ b/arch/x86/ia32/ia32entry.S
>> > @@ -77,12 +77,6 @@ ENTRY(native_usergs_sysret32)
>> > swapgs
>> > sysretl
>> > ENDPROC(native_usergs_sysret32)
>> > -
>> > -ENTRY(native_irq_enable_sysexit)
>> > - swapgs
>> > - sti
>> > - sysexit
>> > -ENDPROC(native_irq_enable_sysexit)
>> > #endif
>> >
>> > /*
>> > diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
>> > index 7549b8b369e4..38a0ff9ef06e 100644
>> > --- a/arch/x86/include/asm/paravirt_types.h
>> > +++ b/arch/x86/include/asm/paravirt_types.h
>> > @@ -160,13 +160,14 @@ struct pv_cpu_ops {
>> > u64 (*read_pmc)(int counter);
>> > unsigned long long (*read_tscp)(unsigned int *aux);
>> >
>> > +#ifdef CONFIG_X86_32
>> > /*
>> > * Atomically enable interrupts and return to userspace. This
>> > - * is only ever used to return to 32-bit processes; in a
>> > - * 64-bit kernel, it's used for 32-on-64 compat processes, but
>> > - * never native 64-bit processes. (Jump, not call.)
>> > + * is only used in 32-bit kernels. 64-bit kernels use
>> > + * usergs_sysret32 instead.
>> > */
>> > void (*irq_enable_sysexit)(void);
>> > +#endif
>> >
>> > /*
>> > * Switch to usermode gs and return to 64-bit usermode using
>> > diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
>> > index 9f6b9341950f..2d27ebf0aed8 100644
>> > --- a/arch/x86/kernel/asm-offsets.c
>> > +++ b/arch/x86/kernel/asm-offsets.c
>> > @@ -49,7 +49,9 @@ void common(void) {
>> > OFFSET(PV_IRQ_irq_disable, pv_irq_ops, irq_disable);
>> > OFFSET(PV_IRQ_irq_enable, pv_irq_ops, irq_enable);
>> > OFFSET(PV_CPU_iret, pv_cpu_ops, iret);
>> > +#ifdef CONFIG_X86_32
>> > OFFSET(PV_CPU_irq_enable_sysexit, pv_cpu_ops, irq_enable_sysexit);
>> > +#endif
>> > OFFSET(PV_CPU_read_cr0, pv_cpu_ops, read_cr0);
>> > OFFSET(PV_MMU_read_cr2, pv_mmu_ops, read_cr2);
>> > #endif
>> > diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
>> > index 548d25f00c90..7563114d9c3a 100644
>> > --- a/arch/x86/kernel/paravirt.c
>> > +++ b/arch/x86/kernel/paravirt.c
>> > @@ -154,7 +154,9 @@ unsigned paravirt_patch_default(u8 type, u16 clobbers, void *insnbuf,
>> > ret = paravirt_patch_ident_64(insnbuf, len);
>> >
>> > else if (type == PARAVIRT_PATCH(pv_cpu_ops.iret) ||
>> > +#ifdef CONFIG_X86_32
>> > type == PARAVIRT_PATCH(pv_cpu_ops.irq_enable_sysexit) ||
>> > +#endif
>> > type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret32) ||
>> > type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret64))
>> > /* If operation requires a jmp, then jmp */
>> > @@ -371,7 +373,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
>> >
>> > .load_sp0 = native_load_sp0,
>> >
>> > -#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
>> > +#if defined(CONFIG_X86_32)
>> > .irq_enable_sysexit = native_irq_enable_sysexit,
>> > #endif
>> > #ifdef CONFIG_X86_64
>> > diff --git a/arch/x86/kernel/paravirt_patch_64.c b/arch/x86/kernel/paravirt_patch_64.c
>> > index a1da6737ba5b..0de21c62c348 100644
>> > --- a/arch/x86/kernel/paravirt_patch_64.c
>> > +++ b/arch/x86/kernel/paravirt_patch_64.c
>> > @@ -49,7 +49,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
>> > PATCH_SITE(pv_irq_ops, save_fl);
>> > PATCH_SITE(pv_irq_ops, irq_enable);
>> > PATCH_SITE(pv_irq_ops, irq_disable);
>> > - PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
>> > PATCH_SITE(pv_cpu_ops, usergs_sysret32);
>> > PATCH_SITE(pv_cpu_ops, usergs_sysret64);
>> > PATCH_SITE(pv_cpu_ops, swapgs);
>> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>> > index 81665c9f2132..3797b6b31f95 100644
>> > --- a/arch/x86/xen/enlighten.c
>> > +++ b/arch/x86/xen/enlighten.c
>> > @@ -1267,10 +1267,11 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
>> > .read_tscp = native_read_tscp,
>> >
>> > .iret = xen_iret,
>> > - .irq_enable_sysexit = xen_sysexit,
>> > #ifdef CONFIG_X86_64
>> > .usergs_sysret32 = xen_sysret32,
>> > .usergs_sysret64 = xen_sysret64,
>> > +#else
>> > + .irq_enable_sysexit = xen_sysexit,
>> > #endif
>> >
>> > .load_tr_desc = paravirt_nop,
>> > diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
>> > index 985fc3ee0973..a2cabb8bd6bf 100644
>> > --- a/arch/x86/xen/xen-asm_64.S
>> > +++ b/arch/x86/xen/xen-asm_64.S
>> > @@ -47,22 +47,6 @@ ENTRY(xen_iret)
>> > ENDPATCH(xen_iret)
>> > RELOC(xen_iret, 1b+1)
>> >
>> > -/*
>> > - * sysexit is not used for 64-bit processes, so it's only ever used to
>> > - * return to 32-bit compat userspace.
>> > - */
>> > -ENTRY(xen_sysexit)
>> > - pushq $__USER32_DS
>> > - pushq %rcx
>> > - pushq $X86_EFLAGS_IF
>> > - pushq $__USER32_CS
>> > - pushq %rdx
>> > -
>> > - pushq $0
>> > -1: jmp hypercall_iret
>> > -ENDPATCH(xen_sysexit)
>> > -RELOC(xen_sysexit, 1b+1)
>> > -
>> > ENTRY(xen_sysret64)
>> > /*
>> > * We're already on the usermode stack at this point, but
>> > diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
>> > index 9e195c683549..c20fe29e65f4 100644
>> > --- a/arch/x86/xen/xen-ops.h
>> > +++ b/arch/x86/xen/xen-ops.h
>> > @@ -134,7 +134,9 @@ DECL_ASM(void, xen_restore_fl_direct, unsigned long);
>> >
>> > /* These are not functions, and cannot be called normally */
>> > __visible void xen_iret(void);
>> > +#ifdef CONFIG_X86_32
>> > __visible void xen_sysexit(void);
>> > +#endif
>> > __visible void xen_sysret32(void);
>> > __visible void xen_sysret64(void);
>> > __visible void xen_adjust_exception_frame(void);
>> > --
>> > 2.3.0
>> >
>>
>>
>>
>> --
>> Andy Lutomirski
>> AMA Capital Management, LLC
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-03 22:52 ` Andy Lutomirski
2015-04-06 14:10 ` Konrad Rzeszutek Wilk
@ 2015-04-06 14:10 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 17+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-04-06 14:10 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Denys Vlasenko, Denys Vlasenko, X86 ML, Linux Kernel Mailing List,
Borislav Petkov, Andy Lutomirski, H. Peter Anvin, xen-devel,
Boris Ostrovsky, Linus Torvalds, Ingo Molnar
On Fri, Apr 03, 2015 at 03:52:30PM -0700, Andy Lutomirski wrote:
> [cc: Boris and Konrad. Whoops]
>
> On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org> wrote:
> > We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
Is there an commit (or name of patch) that explains why 32-bit-user-space-on-64-bit
kernels is unsavory?
Thank you!
> > all the paravirt and Xen machinery to support it on 64-bit kernels.
> >
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> >
> > I haven't actually tested this on Xen, but it builds for me.
> >
> > arch/x86/ia32/ia32entry.S | 6 ------
> > arch/x86/include/asm/paravirt_types.h | 7 ++++---
> > arch/x86/kernel/asm-offsets.c | 2 ++
> > arch/x86/kernel/paravirt.c | 4 +++-
> > arch/x86/kernel/paravirt_patch_64.c | 1 -
> > arch/x86/xen/enlighten.c | 3 ++-
> > arch/x86/xen/xen-asm_64.S | 16 ----------------
> > arch/x86/xen/xen-ops.h | 2 ++
> > 8 files changed, 13 insertions(+), 28 deletions(-)
> >
> > diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
> > index 5d8f987a340d..eb1eb7b70f4b 100644
> > --- a/arch/x86/ia32/ia32entry.S
> > +++ b/arch/x86/ia32/ia32entry.S
> > @@ -77,12 +77,6 @@ ENTRY(native_usergs_sysret32)
> > swapgs
> > sysretl
> > ENDPROC(native_usergs_sysret32)
> > -
> > -ENTRY(native_irq_enable_sysexit)
> > - swapgs
> > - sti
> > - sysexit
> > -ENDPROC(native_irq_enable_sysexit)
> > #endif
> >
> > /*
> > diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> > index 7549b8b369e4..38a0ff9ef06e 100644
> > --- a/arch/x86/include/asm/paravirt_types.h
> > +++ b/arch/x86/include/asm/paravirt_types.h
> > @@ -160,13 +160,14 @@ struct pv_cpu_ops {
> > u64 (*read_pmc)(int counter);
> > unsigned long long (*read_tscp)(unsigned int *aux);
> >
> > +#ifdef CONFIG_X86_32
> > /*
> > * Atomically enable interrupts and return to userspace. This
> > - * is only ever used to return to 32-bit processes; in a
> > - * 64-bit kernel, it's used for 32-on-64 compat processes, but
> > - * never native 64-bit processes. (Jump, not call.)
> > + * is only used in 32-bit kernels. 64-bit kernels use
> > + * usergs_sysret32 instead.
> > */
> > void (*irq_enable_sysexit)(void);
> > +#endif
> >
> > /*
> > * Switch to usermode gs and return to 64-bit usermode using
> > diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
> > index 9f6b9341950f..2d27ebf0aed8 100644
> > --- a/arch/x86/kernel/asm-offsets.c
> > +++ b/arch/x86/kernel/asm-offsets.c
> > @@ -49,7 +49,9 @@ void common(void) {
> > OFFSET(PV_IRQ_irq_disable, pv_irq_ops, irq_disable);
> > OFFSET(PV_IRQ_irq_enable, pv_irq_ops, irq_enable);
> > OFFSET(PV_CPU_iret, pv_cpu_ops, iret);
> > +#ifdef CONFIG_X86_32
> > OFFSET(PV_CPU_irq_enable_sysexit, pv_cpu_ops, irq_enable_sysexit);
> > +#endif
> > OFFSET(PV_CPU_read_cr0, pv_cpu_ops, read_cr0);
> > OFFSET(PV_MMU_read_cr2, pv_mmu_ops, read_cr2);
> > #endif
> > diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> > index 548d25f00c90..7563114d9c3a 100644
> > --- a/arch/x86/kernel/paravirt.c
> > +++ b/arch/x86/kernel/paravirt.c
> > @@ -154,7 +154,9 @@ unsigned paravirt_patch_default(u8 type, u16 clobbers, void *insnbuf,
> > ret = paravirt_patch_ident_64(insnbuf, len);
> >
> > else if (type == PARAVIRT_PATCH(pv_cpu_ops.iret) ||
> > +#ifdef CONFIG_X86_32
> > type == PARAVIRT_PATCH(pv_cpu_ops.irq_enable_sysexit) ||
> > +#endif
> > type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret32) ||
> > type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret64))
> > /* If operation requires a jmp, then jmp */
> > @@ -371,7 +373,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
> >
> > .load_sp0 = native_load_sp0,
> >
> > -#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
> > +#if defined(CONFIG_X86_32)
> > .irq_enable_sysexit = native_irq_enable_sysexit,
> > #endif
> > #ifdef CONFIG_X86_64
> > diff --git a/arch/x86/kernel/paravirt_patch_64.c b/arch/x86/kernel/paravirt_patch_64.c
> > index a1da6737ba5b..0de21c62c348 100644
> > --- a/arch/x86/kernel/paravirt_patch_64.c
> > +++ b/arch/x86/kernel/paravirt_patch_64.c
> > @@ -49,7 +49,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
> > PATCH_SITE(pv_irq_ops, save_fl);
> > PATCH_SITE(pv_irq_ops, irq_enable);
> > PATCH_SITE(pv_irq_ops, irq_disable);
> > - PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
> > PATCH_SITE(pv_cpu_ops, usergs_sysret32);
> > PATCH_SITE(pv_cpu_ops, usergs_sysret64);
> > PATCH_SITE(pv_cpu_ops, swapgs);
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index 81665c9f2132..3797b6b31f95 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1267,10 +1267,11 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
> > .read_tscp = native_read_tscp,
> >
> > .iret = xen_iret,
> > - .irq_enable_sysexit = xen_sysexit,
> > #ifdef CONFIG_X86_64
> > .usergs_sysret32 = xen_sysret32,
> > .usergs_sysret64 = xen_sysret64,
> > +#else
> > + .irq_enable_sysexit = xen_sysexit,
> > #endif
> >
> > .load_tr_desc = paravirt_nop,
> > diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
> > index 985fc3ee0973..a2cabb8bd6bf 100644
> > --- a/arch/x86/xen/xen-asm_64.S
> > +++ b/arch/x86/xen/xen-asm_64.S
> > @@ -47,22 +47,6 @@ ENTRY(xen_iret)
> > ENDPATCH(xen_iret)
> > RELOC(xen_iret, 1b+1)
> >
> > -/*
> > - * sysexit is not used for 64-bit processes, so it's only ever used to
> > - * return to 32-bit compat userspace.
> > - */
> > -ENTRY(xen_sysexit)
> > - pushq $__USER32_DS
> > - pushq %rcx
> > - pushq $X86_EFLAGS_IF
> > - pushq $__USER32_CS
> > - pushq %rdx
> > -
> > - pushq $0
> > -1: jmp hypercall_iret
> > -ENDPATCH(xen_sysexit)
> > -RELOC(xen_sysexit, 1b+1)
> > -
> > ENTRY(xen_sysret64)
> > /*
> > * We're already on the usermode stack at this point, but
> > diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
> > index 9e195c683549..c20fe29e65f4 100644
> > --- a/arch/x86/xen/xen-ops.h
> > +++ b/arch/x86/xen/xen-ops.h
> > @@ -134,7 +134,9 @@ DECL_ASM(void, xen_restore_fl_direct, unsigned long);
> >
> > /* These are not functions, and cannot be called normally */
> > __visible void xen_iret(void);
> > +#ifdef CONFIG_X86_32
> > __visible void xen_sysexit(void);
> > +#endif
> > __visible void xen_sysret32(void);
> > __visible void xen_sysret64(void);
> > __visible void xen_adjust_exception_frame(void);
> > --
> > 2.3.0
> >
>
>
>
> --
> Andy Lutomirski
> AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-03 22:51 [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop Andy Lutomirski
2015-04-03 22:52 ` Andy Lutomirski
@ 2015-04-21 18:56 ` Andy Lutomirski
2015-04-22 13:35 ` Borislav Petkov
2015-04-22 14:09 ` [tip:x86/asm] x86, paravirt, xen: Remove the 64-bit -> irq_enable_sysexit() pvop tip-bot for Andy Lutomirski
2 siblings, 1 reply; 17+ messages in thread
From: Andy Lutomirski @ 2015-04-21 18:56 UTC (permalink / raw)
To: Andy Lutomirski, Ingo Molnar
Cc: Linux Kernel Mailing List, X86 ML, Denys Vlasenko, Linus Torvalds,
Denys Vlasenko, H. Peter Anvin, Borislav Petkov
On Fri, Apr 3, 2015 at 3:51 PM, Andy Lutomirski <luto@kernel.org> wrote:
> We don't use irq_enable_sysexit on 64-bit kernels any more. Remove
> all the paravirt and Xen machinery to support it on 64-bit kernels.
>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
Hi, Ingo,
Can you pick this up in -tip? It cleans up code that would otherwise
be unreachable in 4.1, but it doesn't fix any bugs, so waiting for 4.2
would be fine as well.
--Andy
> ---
>
> I haven't actually tested this on Xen, but it builds for me.
>
> arch/x86/ia32/ia32entry.S | 6 ------
> arch/x86/include/asm/paravirt_types.h | 7 ++++---
> arch/x86/kernel/asm-offsets.c | 2 ++
> arch/x86/kernel/paravirt.c | 4 +++-
> arch/x86/kernel/paravirt_patch_64.c | 1 -
> arch/x86/xen/enlighten.c | 3 ++-
> arch/x86/xen/xen-asm_64.S | 16 ----------------
> arch/x86/xen/xen-ops.h | 2 ++
> 8 files changed, 13 insertions(+), 28 deletions(-)
>
> diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
> index 5d8f987a340d..eb1eb7b70f4b 100644
> --- a/arch/x86/ia32/ia32entry.S
> +++ b/arch/x86/ia32/ia32entry.S
> @@ -77,12 +77,6 @@ ENTRY(native_usergs_sysret32)
> swapgs
> sysretl
> ENDPROC(native_usergs_sysret32)
> -
> -ENTRY(native_irq_enable_sysexit)
> - swapgs
> - sti
> - sysexit
> -ENDPROC(native_irq_enable_sysexit)
> #endif
>
> /*
> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> index 7549b8b369e4..38a0ff9ef06e 100644
> --- a/arch/x86/include/asm/paravirt_types.h
> +++ b/arch/x86/include/asm/paravirt_types.h
> @@ -160,13 +160,14 @@ struct pv_cpu_ops {
> u64 (*read_pmc)(int counter);
> unsigned long long (*read_tscp)(unsigned int *aux);
>
> +#ifdef CONFIG_X86_32
> /*
> * Atomically enable interrupts and return to userspace. This
> - * is only ever used to return to 32-bit processes; in a
> - * 64-bit kernel, it's used for 32-on-64 compat processes, but
> - * never native 64-bit processes. (Jump, not call.)
> + * is only used in 32-bit kernels. 64-bit kernels use
> + * usergs_sysret32 instead.
> */
> void (*irq_enable_sysexit)(void);
> +#endif
>
> /*
> * Switch to usermode gs and return to 64-bit usermode using
> diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
> index 9f6b9341950f..2d27ebf0aed8 100644
> --- a/arch/x86/kernel/asm-offsets.c
> +++ b/arch/x86/kernel/asm-offsets.c
> @@ -49,7 +49,9 @@ void common(void) {
> OFFSET(PV_IRQ_irq_disable, pv_irq_ops, irq_disable);
> OFFSET(PV_IRQ_irq_enable, pv_irq_ops, irq_enable);
> OFFSET(PV_CPU_iret, pv_cpu_ops, iret);
> +#ifdef CONFIG_X86_32
> OFFSET(PV_CPU_irq_enable_sysexit, pv_cpu_ops, irq_enable_sysexit);
> +#endif
> OFFSET(PV_CPU_read_cr0, pv_cpu_ops, read_cr0);
> OFFSET(PV_MMU_read_cr2, pv_mmu_ops, read_cr2);
> #endif
> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> index 548d25f00c90..7563114d9c3a 100644
> --- a/arch/x86/kernel/paravirt.c
> +++ b/arch/x86/kernel/paravirt.c
> @@ -154,7 +154,9 @@ unsigned paravirt_patch_default(u8 type, u16 clobbers, void *insnbuf,
> ret = paravirt_patch_ident_64(insnbuf, len);
>
> else if (type == PARAVIRT_PATCH(pv_cpu_ops.iret) ||
> +#ifdef CONFIG_X86_32
> type == PARAVIRT_PATCH(pv_cpu_ops.irq_enable_sysexit) ||
> +#endif
> type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret32) ||
> type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret64))
> /* If operation requires a jmp, then jmp */
> @@ -371,7 +373,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
>
> .load_sp0 = native_load_sp0,
>
> -#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
> +#if defined(CONFIG_X86_32)
> .irq_enable_sysexit = native_irq_enable_sysexit,
> #endif
> #ifdef CONFIG_X86_64
> diff --git a/arch/x86/kernel/paravirt_patch_64.c b/arch/x86/kernel/paravirt_patch_64.c
> index a1da6737ba5b..0de21c62c348 100644
> --- a/arch/x86/kernel/paravirt_patch_64.c
> +++ b/arch/x86/kernel/paravirt_patch_64.c
> @@ -49,7 +49,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
> PATCH_SITE(pv_irq_ops, save_fl);
> PATCH_SITE(pv_irq_ops, irq_enable);
> PATCH_SITE(pv_irq_ops, irq_disable);
> - PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
> PATCH_SITE(pv_cpu_ops, usergs_sysret32);
> PATCH_SITE(pv_cpu_ops, usergs_sysret64);
> PATCH_SITE(pv_cpu_ops, swapgs);
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 81665c9f2132..3797b6b31f95 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1267,10 +1267,11 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
> .read_tscp = native_read_tscp,
>
> .iret = xen_iret,
> - .irq_enable_sysexit = xen_sysexit,
> #ifdef CONFIG_X86_64
> .usergs_sysret32 = xen_sysret32,
> .usergs_sysret64 = xen_sysret64,
> +#else
> + .irq_enable_sysexit = xen_sysexit,
> #endif
>
> .load_tr_desc = paravirt_nop,
> diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
> index 985fc3ee0973..a2cabb8bd6bf 100644
> --- a/arch/x86/xen/xen-asm_64.S
> +++ b/arch/x86/xen/xen-asm_64.S
> @@ -47,22 +47,6 @@ ENTRY(xen_iret)
> ENDPATCH(xen_iret)
> RELOC(xen_iret, 1b+1)
>
> -/*
> - * sysexit is not used for 64-bit processes, so it's only ever used to
> - * return to 32-bit compat userspace.
> - */
> -ENTRY(xen_sysexit)
> - pushq $__USER32_DS
> - pushq %rcx
> - pushq $X86_EFLAGS_IF
> - pushq $__USER32_CS
> - pushq %rdx
> -
> - pushq $0
> -1: jmp hypercall_iret
> -ENDPATCH(xen_sysexit)
> -RELOC(xen_sysexit, 1b+1)
> -
> ENTRY(xen_sysret64)
> /*
> * We're already on the usermode stack at this point, but
> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
> index 9e195c683549..c20fe29e65f4 100644
> --- a/arch/x86/xen/xen-ops.h
> +++ b/arch/x86/xen/xen-ops.h
> @@ -134,7 +134,9 @@ DECL_ASM(void, xen_restore_fl_direct, unsigned long);
>
> /* These are not functions, and cannot be called normally */
> __visible void xen_iret(void);
> +#ifdef CONFIG_X86_32
> __visible void xen_sysexit(void);
> +#endif
> __visible void xen_sysret32(void);
> __visible void xen_sysret64(void);
> __visible void xen_adjust_exception_frame(void);
> --
> 2.3.0
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop
2015-04-21 18:56 ` Andy Lutomirski
@ 2015-04-22 13:35 ` Borislav Petkov
0 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2015-04-22 13:35 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andy Lutomirski, Ingo Molnar, Linux Kernel Mailing List, X86 ML,
Denys Vlasenko, Linus Torvalds, Denys Vlasenko, H. Peter Anvin
On Tue, Apr 21, 2015 at 11:56:39AM -0700, Andy Lutomirski wrote:
> Can you pick this up in -tip? It cleans up code that would otherwise
> be unreachable in 4.1, but it doesn't fix any bugs, so waiting for 4.2
> would be fine as well.
Applied, thanks.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply [flat|nested] 17+ messages in thread
* [tip:x86/asm] x86, paravirt, xen: Remove the 64-bit -> irq_enable_sysexit() pvop
2015-04-03 22:51 [PATCH] x86, paravirt, xen: Remove the 64-bit irq_enable_sysexit pvop Andy Lutomirski
2015-04-03 22:52 ` Andy Lutomirski
2015-04-21 18:56 ` Andy Lutomirski
@ 2015-04-22 14:09 ` tip-bot for Andy Lutomirski
2 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Andy Lutomirski @ 2015-04-22 14:09 UTC (permalink / raw)
To: linux-tip-commits
Cc: vda.linux, hpa, tglx, luto, dvlasenk, linux-kernel, brgerst, bp,
boris.ostrovsky, torvalds, mingo, luto
Commit-ID: aac82d319148c6a84e1bf90b86d3e0ec8bf0ee38
Gitweb: http://git.kernel.org/tip/aac82d319148c6a84e1bf90b86d3e0ec8bf0ee38
Author: Andy Lutomirski <luto@kernel.org>
AuthorDate: Fri, 3 Apr 2015 15:51:54 -0700
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 22 Apr 2015 08:07:45 +0200
x86, paravirt, xen: Remove the 64-bit ->irq_enable_sysexit() pvop
We don't use irq_enable_sysexit on 64-bit kernels any more.
Remove all the paravirt and Xen machinery to support it on
64-bit kernels.
Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Denys Vlasenko <vda.linux@googlemail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/8a03355698fe5b94194e9e7360f19f91c1b2cf1f.1428100853.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/ia32/ia32entry.S | 6 ------
arch/x86/include/asm/paravirt_types.h | 7 ++++---
arch/x86/kernel/asm-offsets.c | 2 ++
arch/x86/kernel/paravirt.c | 4 +++-
arch/x86/kernel/paravirt_patch_64.c | 1 -
arch/x86/xen/enlighten.c | 3 ++-
arch/x86/xen/xen-asm_64.S | 16 ----------------
arch/x86/xen/xen-ops.h | 2 ++
8 files changed, 13 insertions(+), 28 deletions(-)
diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index a821b1c..3cdb9ea 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -77,12 +77,6 @@ ENTRY(native_usergs_sysret32)
swapgs
sysretl
ENDPROC(native_usergs_sysret32)
-
-ENTRY(native_irq_enable_sysexit)
- swapgs
- sti
- sysexit
-ENDPROC(native_irq_enable_sysexit)
#endif
/*
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 7549b8b..38a0ff9 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -160,13 +160,14 @@ struct pv_cpu_ops {
u64 (*read_pmc)(int counter);
unsigned long long (*read_tscp)(unsigned int *aux);
+#ifdef CONFIG_X86_32
/*
* Atomically enable interrupts and return to userspace. This
- * is only ever used to return to 32-bit processes; in a
- * 64-bit kernel, it's used for 32-on-64 compat processes, but
- * never native 64-bit processes. (Jump, not call.)
+ * is only used in 32-bit kernels. 64-bit kernels use
+ * usergs_sysret32 instead.
*/
void (*irq_enable_sysexit)(void);
+#endif
/*
* Switch to usermode gs and return to 64-bit usermode using
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index b27f6ec..8e3d22a1 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -68,7 +68,9 @@ void common(void) {
OFFSET(PV_IRQ_irq_disable, pv_irq_ops, irq_disable);
OFFSET(PV_IRQ_irq_enable, pv_irq_ops, irq_enable);
OFFSET(PV_CPU_iret, pv_cpu_ops, iret);
+#ifdef CONFIG_X86_32
OFFSET(PV_CPU_irq_enable_sysexit, pv_cpu_ops, irq_enable_sysexit);
+#endif
OFFSET(PV_CPU_read_cr0, pv_cpu_ops, read_cr0);
OFFSET(PV_MMU_read_cr2, pv_mmu_ops, read_cr2);
#endif
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 548d25f..7563114 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -154,7 +154,9 @@ unsigned paravirt_patch_default(u8 type, u16 clobbers, void *insnbuf,
ret = paravirt_patch_ident_64(insnbuf, len);
else if (type == PARAVIRT_PATCH(pv_cpu_ops.iret) ||
+#ifdef CONFIG_X86_32
type == PARAVIRT_PATCH(pv_cpu_ops.irq_enable_sysexit) ||
+#endif
type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret32) ||
type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret64))
/* If operation requires a jmp, then jmp */
@@ -371,7 +373,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
.load_sp0 = native_load_sp0,
-#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
+#if defined(CONFIG_X86_32)
.irq_enable_sysexit = native_irq_enable_sysexit,
#endif
#ifdef CONFIG_X86_64
diff --git a/arch/x86/kernel/paravirt_patch_64.c b/arch/x86/kernel/paravirt_patch_64.c
index a1da673..0de21c6 100644
--- a/arch/x86/kernel/paravirt_patch_64.c
+++ b/arch/x86/kernel/paravirt_patch_64.c
@@ -49,7 +49,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
PATCH_SITE(pv_irq_ops, save_fl);
PATCH_SITE(pv_irq_ops, irq_enable);
PATCH_SITE(pv_irq_ops, irq_disable);
- PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
PATCH_SITE(pv_cpu_ops, usergs_sysret32);
PATCH_SITE(pv_cpu_ops, usergs_sysret64);
PATCH_SITE(pv_cpu_ops, swapgs);
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 81665c9..3797b6b 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1267,10 +1267,11 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
.read_tscp = native_read_tscp,
.iret = xen_iret,
- .irq_enable_sysexit = xen_sysexit,
#ifdef CONFIG_X86_64
.usergs_sysret32 = xen_sysret32,
.usergs_sysret64 = xen_sysret64,
+#else
+ .irq_enable_sysexit = xen_sysexit,
#endif
.load_tr_desc = paravirt_nop,
diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index 985fc3e..a2cabb8 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -47,22 +47,6 @@ ENTRY(xen_iret)
ENDPATCH(xen_iret)
RELOC(xen_iret, 1b+1)
-/*
- * sysexit is not used for 64-bit processes, so it's only ever used to
- * return to 32-bit compat userspace.
- */
-ENTRY(xen_sysexit)
- pushq $__USER32_DS
- pushq %rcx
- pushq $X86_EFLAGS_IF
- pushq $__USER32_CS
- pushq %rdx
-
- pushq $0
-1: jmp hypercall_iret
-ENDPATCH(xen_sysexit)
-RELOC(xen_sysexit, 1b+1)
-
ENTRY(xen_sysret64)
/*
* We're already on the usermode stack at this point, but
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 9e195c6..c20fe29 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -134,7 +134,9 @@ DECL_ASM(void, xen_restore_fl_direct, unsigned long);
/* These are not functions, and cannot be called normally */
__visible void xen_iret(void);
+#ifdef CONFIG_X86_32
__visible void xen_sysexit(void);
+#endif
__visible void xen_sysret32(void);
__visible void xen_sysret64(void);
__visible void xen_adjust_exception_frame(void);
^ permalink raw reply related [flat|nested] 17+ messages in thread