x86: Support compiling out userspace IO (iopl and ioperm)
Subsystems (7)
v4 cover x86: Support compiling out userspace IO (iopl and ioperm)
This patch series makes it possible to compile out the userspace IO system calls, iopl and ioperm.
The first patch does some 32/64 unification in copy_thread to make subsequent changes easier. The second patch simplifies the complex calculation of the TSS segment limit, which also makes it easier to change in the last patch. Patches 3-9 introduce helpers to make it easier to compile out IO. The last patch adds and uses the new CONFIG_X86_IOPORT to support compiling out userspace IO.
v3 had patches 3-10 as a single patch; v4 splits out the various helpers and macros into separate patches, as requested by Thomas Gleixner.
I've verified that this compiles after each patch.
Josh Triplett (10):
x86: process: Unify 32-bit and 64-bit copy_thread I/O bitmap handling
x86: tss: Eliminate fragile calculation of TSS segment limit
x86: processor.h: Introduce macros to initialize IO fields of thread
and TSS
x86: paravirt: Wrap initialization of set_iopl_mask in a macro
x86: cpu: Add helper function unifying 32-bit and 64-bit IO init in
cpu_init
x86: process: Introduce helper to clear a thread's IO bitmap
x86: process: Introduce helper to switch iopl mask
x86: process: Introduce helper for IO-related bits of exit_thread
x86: process: Introduce helper to switch IO bitmap
x86: Support compiling out userspace IO (iopl and ioperm)
arch/x86/Kconfig | 10 ++++ arch/x86/include/asm/desc.h | 11 +---- arch/x86/include/asm/paravirt.h | 2 + arch/x86/include/asm/paravirt_types.h | 5 ++ arch/x86/include/asm/processor.h | 55 +++++++++++++++++---- arch/x86/include/asm/syscalls.h | 3 ++ arch/x86/kernel/Makefile | 3 +- arch/x86/kernel/cpu/common.c | 12 +---- arch/x86/kernel/entry_64.S | 9 ++-- arch/x86/kernel/paravirt.c | 2 +- arch/x86/kernel/process-io.h | 93 +++++++++++++++++++++++++++++++++++ arch/x86/kernel/process.c | 34 ++----------- arch/x86/kernel/process_32.c | 41 +++++---------- arch/x86/kernel/process_64.c | 27 +++------- arch/x86/kernel/ptrace.c | 8 +++ arch/x86/xen/enlighten.c | 4 +- drivers/tty/vt/vt_ioctl.c | 2 +- kernel/sys_ni.c | 5 ++ 18 files changed, 208 insertions(+), 118 deletions(-) create mode 100644 arch/x86/kernel/process-io.h
Signature · 1 line
2.1.1
PATCH v4 1/10 x86: process: Unify 32-bit and 64-bit copy_thread I/O bitmap handling
The 32-bit and 64-bit versions of copy_thread have functionally identical handling for copying the I/O bitmap, modulo differences in error handling. Clean up the error paths in both by moving the copy of the I/O bitmap to the end, to eliminate the need to free it if subsequent copy steps fail; move the resulting identical code to a static inline in a common header.
--- arch/x86/kernel/process-io.h | 22 ++++++++++++++++++++++ arch/x86/kernel/process_32.c | 28 ++++++++-------------------- arch/x86/kernel/process_64.c | 25 +++++-------------------- 3 files changed, 35 insertions(+), 40 deletions(-) create mode 100644 arch/x86/kernel/process-io.h
arch/x86/kernel/process-io.h new file +22 -0
@@ -0,0 +1,22 @@ +#ifndef _X86_KERNEL_PROCESS_IO_H +#define _X86_KERNEL_PROCESS_IO_H + +static inline int copy_io_bitmap (struct task_struct *me, +struct task_struct *p) +{ +if (unlikely (test_tsk_thread_flag (me,TIF_IO_BITMAP ))) { + p->thread.io_bitmap_ptr =kmemdup (me->thread.io_bitmap_ptr, +IO_BITMAP_BYTES ,GFP_KERNEL ); +if (!p->thread.io_bitmap_ptr) { + p->thread.io_bitmap_max =0 ; +return -ENOMEM ; + } +set_tsk_thread_flag (p,TIF_IO_BITMAP ); + }else { + p->thread.io_bitmap_ptr =NULL ; + } + +return 0 ; +} + +#endif /* _X86_KERNEL_PROCESS_IO_H */
arch/x86/kernel/process_32.c +8 -20
@@ -55,6 +55,8 @@#include <asm/debugreg.h> #include <asm/switch_to.h> +#include "process-io.h" + asmlinkage void ret_from_fork(void) __asm__("ret_from_fork"); asmlinkage void ret_from_kernel_thread(void) __asm__("ret_from_kernel_thread");
@@ -134,7 +136,6 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, {struct pt_regs *childregs =task_pt_regs (p);struct task_struct *tsk; -int err; p->thread.sp = (unsigned long ) childregs; p->thread.sp0 = (unsigned long ) (childregs+1 );
@@ -166,32 +167,19 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, p->thread.io_bitmap_ptr =NULL ; tsk = current; - err = -ENOMEM ; - -if (unlikely (test_tsk_thread_flag (tsk,TIF_IO_BITMAP ))) { - p->thread.io_bitmap_ptr =kmemdup (tsk->thread.io_bitmap_ptr, -IO_BITMAP_BYTES ,GFP_KERNEL ); -if (!p->thread.io_bitmap_ptr) { - p->thread.io_bitmap_max =0 ; -return -ENOMEM ; - } -set_tsk_thread_flag (p,TIF_IO_BITMAP ); - } - - err =0 ;/* * Set a new TLS for the child thread? */ -if (clone_flags &CLONE_SETTLS ) +if (clone_flags &CLONE_SETTLS ) { +int err; err =do_set_thread_area (p,-1 , (struct user_desc __user *)childregs->si,0 ); - -if (err && p->thread.io_bitmap_ptr) { -kfree (p->thread.io_bitmap_ptr); - p->thread.io_bitmap_max =0 ; +if (err) +return err; } -return err; + +return copy_io_bitmap (tsk, p); }void
arch/x86/kernel/process_64.c +5 -20
@@ -50,6 +50,8 @@#include <asm/debugreg.h> #include <asm/switch_to.h> +#include "process-io.h" + asmlinkage extern void ret_from_fork(void);__visible DEFINE_PER_CPU (unsigned long ,old_rsp );
@@ -154,7 +156,6 @@ static inline u32 read_32bit_tls(struct task_struct *t, int tls)int copy_thread (unsigned long clone_flags,unsigned long sp,unsigned long arg,struct task_struct *p) { -int err;struct pt_regs *childregs;struct task_struct *me = current;
@@ -191,21 +192,11 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,if (sp) childregs->sp = sp; - err = -ENOMEM ; -if (unlikely (test_tsk_thread_flag (me,TIF_IO_BITMAP ))) { - p->thread.io_bitmap_ptr =kmemdup (me->thread.io_bitmap_ptr, -IO_BITMAP_BYTES ,GFP_KERNEL ); -if (!p->thread.io_bitmap_ptr) { - p->thread.io_bitmap_max =0 ; -return -ENOMEM ; - } -set_tsk_thread_flag (p,TIF_IO_BITMAP ); - } -/* * Set a new TLS for the child thread? */ if (clone_flags &CLONE_SETTLS ) { +int err;#ifdef CONFIG_IA32_EMULATION if (test_thread_flag (TIF_IA32 )) err = do_set_thread_area(p, -1,
@@ -214,16 +205,10 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,#endif err =do_arch_prctl (p,ARCH_SET_FS , childregs->r8);if (err) - goto out; - } - err =0 ; -out: -if (err && p->thread.io_bitmap_ptr) { -kfree (p->thread.io_bitmap_ptr); - p->thread.io_bitmap_max =0 ; +return err; } -return err; +return copy_io_bitmap (me, p); } static void
Signature · 1 line
2.1.1
PATCH v4 1/10 x86: process: Unify 32-bit and 64-bit copy_thread I/O bitmap handling
In reply to Josh TriplettThe 32-bit and 64-bit versions of copy_thread have functionally identical handling for copying the I/O bitmap, modulo differences in error handling. Clean up the error paths in both by moving the copy of the I/O bitmap to the end, to eliminate the need to free it if subsequent copy steps fail; move the resulting identical code to a static inline in a common header.
--- arch/x86/kernel/process-io.h | 22 ++++++++++++++++++++++ arch/x86/kernel/process_32.c | 28 ++++++++-------------------- arch/x86/kernel/process_64.c | 25 +++++-------------------- 3 files changed, 35 insertions(+), 40 deletions(-) create mode 100644 arch/x86/kernel/process-io.h
arch/x86/kernel/process-io.h new file +22 -0
@@ -0,0 +1,22 @@ +#ifndef _X86_KERNEL_PROCESS_IO_H +#define _X86_KERNEL_PROCESS_IO_H + +static inline int copy_io_bitmap (struct task_struct *me, +struct task_struct *p) +{ +if (unlikely (test_tsk_thread_flag (me,TIF_IO_BITMAP ))) { + p->thread.io_bitmap_ptr =kmemdup (me->thread.io_bitmap_ptr, +IO_BITMAP_BYTES ,GFP_KERNEL ); +if (!p->thread.io_bitmap_ptr) { + p->thread.io_bitmap_max =0 ; +return -ENOMEM ; + } +set_tsk_thread_flag (p,TIF_IO_BITMAP ); + }else { + p->thread.io_bitmap_ptr =NULL ; + } + +return 0 ; +} + +#endif /* _X86_KERNEL_PROCESS_IO_H */
arch/x86/kernel/process_32.c +8 -20
@@ -55,6 +55,8 @@#include <asm/debugreg.h> #include <asm/switch_to.h> +#include "process-io.h" + asmlinkage void ret_from_fork(void) __asm__("ret_from_fork"); asmlinkage void ret_from_kernel_thread(void) __asm__("ret_from_kernel_thread");
@@ -134,7 +136,6 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, {struct pt_regs *childregs =task_pt_regs (p);struct task_struct *tsk; -int err; p->thread.sp = (unsigned long ) childregs; p->thread.sp0 = (unsigned long ) (childregs+1 );
@@ -166,32 +167,19 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, p->thread.io_bitmap_ptr =NULL ; tsk = current; - err = -ENOMEM ; - -if (unlikely (test_tsk_thread_flag (tsk,TIF_IO_BITMAP ))) { - p->thread.io_bitmap_ptr =kmemdup (tsk->thread.io_bitmap_ptr, -IO_BITMAP_BYTES ,GFP_KERNEL ); -if (!p->thread.io_bitmap_ptr) { - p->thread.io_bitmap_max =0 ; -return -ENOMEM ; - } -set_tsk_thread_flag (p,TIF_IO_BITMAP ); - } - - err =0 ;/* * Set a new TLS for the child thread? */ -if (clone_flags &CLONE_SETTLS ) +if (clone_flags &CLONE_SETTLS ) { +int err; err =do_set_thread_area (p,-1 , (struct user_desc __user *)childregs->si,0 ); - -if (err && p->thread.io_bitmap_ptr) { -kfree (p->thread.io_bitmap_ptr); - p->thread.io_bitmap_max =0 ; +if (err) +return err; } -return err; + +return copy_io_bitmap (tsk, p); }void
arch/x86/kernel/process_64.c +5 -20
@@ -50,6 +50,8 @@#include <asm/debugreg.h> #include <asm/switch_to.h> +#include "process-io.h" + asmlinkage extern void ret_from_fork(void);__visible DEFINE_PER_CPU (unsigned long ,old_rsp );
@@ -154,7 +156,6 @@ static inline u32 read_32bit_tls(struct task_struct *t, int tls)int copy_thread (unsigned long clone_flags,unsigned long sp,unsigned long arg,struct task_struct *p) { -int err;struct pt_regs *childregs;struct task_struct *me = current;
@@ -191,21 +192,11 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,if (sp) childregs->sp = sp; - err = -ENOMEM ; -if (unlikely (test_tsk_thread_flag (me,TIF_IO_BITMAP ))) { - p->thread.io_bitmap_ptr =kmemdup (me->thread.io_bitmap_ptr, -IO_BITMAP_BYTES ,GFP_KERNEL ); -if (!p->thread.io_bitmap_ptr) { - p->thread.io_bitmap_max =0 ; -return -ENOMEM ; - } -set_tsk_thread_flag (p,TIF_IO_BITMAP ); - } -/* * Set a new TLS for the child thread? */ if (clone_flags &CLONE_SETTLS ) { +int err;#ifdef CONFIG_IA32_EMULATION if (test_thread_flag (TIF_IA32 )) err = do_set_thread_area(p, -1,
@@ -214,16 +205,10 @@ int copy_thread(unsigned long clone_flags, unsigned long sp,#endif err =do_arch_prctl (p,ARCH_SET_FS , childregs->r8);if (err) - goto out; - } - err =0 ; -out: -if (err && p->thread.io_bitmap_ptr) { -kfree (p->thread.io_bitmap_ptr); - p->thread.io_bitmap_max =0 ; +return err; } -return err; +return copy_io_bitmap (me, p); } static void
Signature · 1 line
2.1.1
PATCH v4 2/10 x86: tss: Eliminate fragile calculation of TSS segment limit
In reply to Josh Triplett__set_tss_desc has a complex calculation of the TSS segment limit, duplicating the quirky details of the I/O bitmap array length, and requiring a complex comment to explain. Replace that calculation with a simpler one based on the offsetof the "stack" field that follows the array.
That then removes the last use of IO_BITMAP_OFFSET, so delete it.
--- arch/x86/include/asm/desc.h | 11 +---------- arch/x86/include/asm/processor.h | 4 +++- 2 files changed, 4 insertions(+), 11 deletions(-)
arch/x86/include/asm/desc.h +1 -10
@@ -177,16 +177,7 @@ static inline void __set_tss_desc(unsigned cpu, unsigned int entry, void *addr)struct desc_struct *d =get_cpu_gdt_table (cpu);tss_desc tss; -/* -* sizeof(unsigned long) coming from an extra "long" at the end -* of the iobitmap. See tss_struct definition in processor.h -* -* -1? seg base+limit should be pointing to the address of the -* last valid byte -*/ -set_tssldt_descriptor (&tss, (unsigned long )addr,DESC_TSS , -IO_BITMAP_OFFSET +IO_BITMAP_BYTES + -sizeof (unsigned long ) -1 ); +set_tssldt_descriptor (&tss, (unsigned long )addr,DESC_TSS ,TSS_LIMIT );write_gdt_entry (d, entry, &tss,DESC_TSS ); }
arch/x86/include/asm/processor.h +3 -1
@@ -258,9 +258,11 @@ struct x86_hw_tss {#define IO_BITMAP_BITS 65536#define IO_BITMAP_BYTES (IO_BITMAP_BITS/8)#define IO_BITMAP_LONGS (IO_BITMAP_BYTES/sizeof(long)) -#define IO_BITMAP_OFFSET offsetof(struct tss_struct, io_bitmap)#define INVALID_IO_BITMAP_OFFSET 0x8000 +/* Segment limits point to the last valid byte, hence the -1 */ +#define TSS_LIMIT (offsetof(struct tss_struct, stack) - 1) +struct tss_struct {/* * The hardware state:
Signature · 1 line
2.1.1
PATCH v4 2/10 x86: tss: Eliminate fragile calculation of TSS segment limit
In reply to Josh Triplett__set_tss_desc has a complex calculation of the TSS segment limit, duplicating the quirky details of the I/O bitmap array length, and requiring a complex comment to explain. Replace that calculation with a simpler one based on the offsetof the "stack" field that follows the array.
That then removes the last use of IO_BITMAP_OFFSET, so delete it.
--- arch/x86/include/asm/desc.h | 11 +---------- arch/x86/include/asm/processor.h | 4 +++- 2 files changed, 4 insertions(+), 11 deletions(-)
arch/x86/include/asm/desc.h +1 -10
@@ -177,16 +177,7 @@ static inline void __set_tss_desc(unsigned cpu, unsigned int entry, void *addr)struct desc_struct *d =get_cpu_gdt_table (cpu);tss_desc tss; -/* -* sizeof(unsigned long) coming from an extra "long" at the end -* of the iobitmap. See tss_struct definition in processor.h -* -* -1? seg base+limit should be pointing to the address of the -* last valid byte -*/ -set_tssldt_descriptor (&tss, (unsigned long )addr,DESC_TSS , -IO_BITMAP_OFFSET +IO_BITMAP_BYTES + -sizeof (unsigned long ) -1 ); +set_tssldt_descriptor (&tss, (unsigned long )addr,DESC_TSS ,TSS_LIMIT );write_gdt_entry (d, entry, &tss,DESC_TSS ); }
arch/x86/include/asm/processor.h +3 -1
@@ -258,9 +258,11 @@ struct x86_hw_tss {#define IO_BITMAP_BITS 65536#define IO_BITMAP_BYTES (IO_BITMAP_BITS/8)#define IO_BITMAP_LONGS (IO_BITMAP_BYTES/sizeof(long)) -#define IO_BITMAP_OFFSET offsetof(struct tss_struct, io_bitmap)#define INVALID_IO_BITMAP_OFFSET 0x8000 +/* Segment limits point to the last valid byte, hence the -1 */ +#define TSS_LIMIT (offsetof(struct tss_struct, stack) - 1) +struct tss_struct {/* * The hardware state:
Signature · 1 line
2.1.1
PATCH v4 3/10 x86: processor.h: Introduce macros to initialize IO fields of thread and TSS
In reply to Josh TriplettNo functional change, just splitting out parts of the existing INIT_THREAD and INIT_TSS macros. This will simplify making those fields optional later.
--- arch/x86/include/asm/processor.h | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
arch/x86/include/asm/processor.h +11 -7
@@ -842,12 +842,7 @@ static inline void spin_lock_prefetch(const void *x)#define STACK_TOP TASK_SIZE#define STACK_TOP_MAX STACK_TOP -#define INIT_THREAD { \ - .sp0 = sizeof(init_stack) + (long)&init_stack, \ - .vm86_info = NULL, \ - .sysenter_cs = __KERNEL_CS, \ - .io_bitmap_ptr = NULL, \ -} +#define INIT_THREAD_IO .io_bitmap_ptr = NULL,/* * Note that the .io_bitmap member must be extra-big. This is because
@@ -855,6 +850,15 @@ static inline void spin_lock_prefetch(const void *x)* permission bitmap. The extra byte must be all 1 bits, and must * be within the limit. */ +#define INIT_TSS_IO .io_bitmap = { [0 ... IO_BITMAP_LONGS] = ~0 }, + +#define INIT_THREAD { \ + .sp0 = sizeof(init_stack) + (long)&init_stack, \ + .vm86_info = NULL, \ + .sysenter_cs = __KERNEL_CS, \ + INIT_THREAD_IO \ +} +#define INIT_TSS { \ .x86_tss = { \ .sp0 = sizeof(init_stack) + (long)&init_stack, \
@@ -862,7 +866,7 @@ static inline void spin_lock_prefetch(const void *x) .ss1 = __KERNEL_CS, \ .io_bitmap_base = INVALID_IO_BITMAP_OFFSET, \ }, \ - .io_bitmap = { [0 ... IO_BITMAP_LONGS] = ~0 }, \ + INIT_TSS_IO \ }extern unsigned long thread_saved_pc (struct task_struct *tsk);
Signature · 1 line
2.1.1
PATCH v4 3/10 x86: processor.h: Introduce macros to initialize IO fields of thread and TSS
In reply to Josh TriplettNo functional change, just splitting out parts of the existing INIT_THREAD and INIT_TSS macros. This will simplify making those fields optional later.
--- arch/x86/include/asm/processor.h | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
arch/x86/include/asm/processor.h +11 -7
@@ -842,12 +842,7 @@ static inline void spin_lock_prefetch(const void *x)#define STACK_TOP TASK_SIZE#define STACK_TOP_MAX STACK_TOP -#define INIT_THREAD { \ - .sp0 = sizeof(init_stack) + (long)&init_stack, \ - .vm86_info = NULL, \ - .sysenter_cs = __KERNEL_CS, \ - .io_bitmap_ptr = NULL, \ -} +#define INIT_THREAD_IO .io_bitmap_ptr = NULL,/* * Note that the .io_bitmap member must be extra-big. This is because
@@ -855,6 +850,15 @@ static inline void spin_lock_prefetch(const void *x)* permission bitmap. The extra byte must be all 1 bits, and must * be within the limit. */ +#define INIT_TSS_IO .io_bitmap = { [0 ... IO_BITMAP_LONGS] = ~0 }, + +#define INIT_THREAD { \ + .sp0 = sizeof(init_stack) + (long)&init_stack, \ + .vm86_info = NULL, \ + .sysenter_cs = __KERNEL_CS, \ + INIT_THREAD_IO \ +} +#define INIT_TSS { \ .x86_tss = { \ .sp0 = sizeof(init_stack) + (long)&init_stack, \
@@ -862,7 +866,7 @@ static inline void spin_lock_prefetch(const void *x) .ss1 = __KERNEL_CS, \ .io_bitmap_base = INVALID_IO_BITMAP_OFFSET, \ }, \ - .io_bitmap = { [0 ... IO_BITMAP_LONGS] = ~0 }, \ + INIT_TSS_IO \ }extern unsigned long thread_saved_pc (struct task_struct *tsk);
Signature · 1 line
2.1.1
PATCH v4 4/10 x86: paravirt: Wrap initialization of set_iopl_mask in a macro
In reply to Josh TriplettThis will allow making set_iopl_mask optional later.
--- arch/x86/include/asm/paravirt_types.h | 1 + arch/x86/kernel/paravirt.c | 2 +- arch/x86/xen/enlighten.c | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-)
arch/x86/include/asm/paravirt_types.h +1 -0
@@ -143,6 +143,7 @@ struct pv_cpu_ops {void (*load_sp0)(struct tss_struct *tss,struct thread_struct *t);void (*set_iopl_mask)(unsigned mask); +#define INIT_SET_IOPL_MASK (f) .set_iopl_mask = f,void (*wbinvd)(void );void (*io_delay)(void );
arch/x86/kernel/paravirt.c +1 -1
@@ -383,7 +383,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = { .iret = native_iret, .swapgs = native_swapgs, - .set_iopl_mask = native_set_iopl_mask, +INIT_SET_IOPL_MASK (native_set_iopl_mask) .io_delay = native_io_delay, .start_context_switch = paravirt_nop,
arch/x86/xen/enlighten.c +2 -2
@@ -912,7 +912,7 @@ static void xen_load_sp0(struct tss_struct *tss,xen_mc_issue (PARAVIRT_LAZY_CPU ); } -static void xen_set_iopl_mask (unsigned mask) +static void __maybe_unused xen_set_iopl_mask (unsigned mask) {struct physdev_set_iopl set_iopl;
@@ -1279,7 +1279,7 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = { .write_idt_entry = xen_write_idt_entry, .load_sp0 = xen_load_sp0, - .set_iopl_mask = xen_set_iopl_mask, +INIT_SET_IOPL_MASK (xen_set_iopl_mask) .io_delay = xen_io_delay,/* Xen takes care of %gs when switching to usermode for us */
Signature · 1 line
2.1.1
Re: [Xen-devel] [PATCH v4 04/10] x86: paravirt: Wrap initialization of set_iopl_mask in a macro
Quoted 3 lines · Josh Triplett
This will allow making
set_iopl_maskoptional later.Signed-off-by Josh Triplett <[email protected]>
Quoted 61 lines
--- arch/x86/include/asm/paravirt_types.h | 1 + arch/x86/kernel/paravirt.c | 2 +- arch/x86/xen/enlighten.c | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-)arch/x86/include/asm/paravirt_types.h +1
-0@@ -143,6 +143,7 @@ struct pv_cpu_ops {void (*load_sp0)(struct tss_struct *tss,struct thread_struct *t);void (*set_iopl_mask)(unsigned mask); +#define INIT_SET_IOPL_MASK (f) .set_iopl_mask = f,void (*wbinvd)(void );void (*io_delay)(void );arch/x86/kernel/paravirt.c +1
-1@@ -383,7 +383,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = { .iret = native_iret, .swapgs = native_swapgs, - .set_iopl_mask = native_set_iopl_mask, +INIT_SET_IOPL_MASK (native_set_iopl_mask) .io_delay = native_io_delay, .start_context_switch = paravirt_nop,arch/x86/xen/enlighten.c +2
-2@@ -912,7 +912,7 @@ static void xen_load_sp0(struct tss_struct *tss,xen_mc_issue (PARAVIRT_LAZY_CPU ); } -static void xen_set_iopl_mask (unsigned mask) +static void __maybe_unused xen_set_iopl_mask (unsigned mask) {struct physdev_set_iopl set_iopl;@@ -1279,7 +1279,7 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = { .write_idt_entry = xen_write_idt_entry, .load_sp0 = xen_load_sp0, - .set_iopl_mask = xen_set_iopl_mask, +INIT_SET_IOPL_MASK (xen_set_iopl_mask) .io_delay = xen_io_delay,/* Xen takes care of %gs when switching to usermode for us */ Signature · 7 lines
2.1.1 _______________________________________________ Xen-devel mailing list [email protected] http://lists.xen.org/xen-devel
Re: [PATCH v4 04/10] x86: paravirt: Wrap initialization of set_iopl_mask in a macro
In reply to Josh TriplettQuoted 3 lines · Josh Triplett
This will allow making
set_iopl_maskoptional later.Signed-off-by Josh Triplett <[email protected]>
Quoted 61 lines
--- arch/x86/include/asm/paravirt_types.h | 1 + arch/x86/kernel/paravirt.c | 2 +- arch/x86/xen/enlighten.c | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-)arch/x86/include/asm/paravirt_types.h +1
-0@@ -143,6 +143,7 @@ struct pv_cpu_ops {void (*load_sp0)(struct tss_struct *tss,struct thread_struct *t);void (*set_iopl_mask)(unsigned mask); +#define INIT_SET_IOPL_MASK (f) .set_iopl_mask = f,void (*wbinvd)(void );void (*io_delay)(void );arch/x86/kernel/paravirt.c +1
-1@@ -383,7 +383,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = { .iret = native_iret, .swapgs = native_swapgs, - .set_iopl_mask = native_set_iopl_mask, +INIT_SET_IOPL_MASK (native_set_iopl_mask) .io_delay = native_io_delay, .start_context_switch = paravirt_nop,arch/x86/xen/enlighten.c +2
-2@@ -912,7 +912,7 @@ static void xen_load_sp0(struct tss_struct *tss,xen_mc_issue (PARAVIRT_LAZY_CPU ); } -static void xen_set_iopl_mask (unsigned mask) +static void __maybe_unused xen_set_iopl_mask (unsigned mask) {struct physdev_set_iopl set_iopl;@@ -1279,7 +1279,7 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = { .write_idt_entry = xen_write_idt_entry, .load_sp0 = xen_load_sp0, - .set_iopl_mask = xen_set_iopl_mask, +INIT_SET_IOPL_MASK (xen_set_iopl_mask) .io_delay = xen_io_delay,/* Xen takes care of %gs when switching to usermode for us */ Signature · 7 lines
2.1.1 _______________________________________________ Xen-devel mailing list [email protected] http://lists.xen.org/xen-devel
PATCH v4 4/10 x86: paravirt: Wrap initialization of set_iopl_mask in a macro
In reply to Josh TriplettThis will allow making set_iopl_mask optional later.
--- arch/x86/include/asm/paravirt_types.h | 1 + arch/x86/kernel/paravirt.c | 2 +- arch/x86/xen/enlighten.c | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-)
arch/x86/include/asm/paravirt_types.h +1 -0
@@ -143,6 +143,7 @@ struct pv_cpu_ops {void (*load_sp0)(struct tss_struct *tss,struct thread_struct *t);void (*set_iopl_mask)(unsigned mask); +#define INIT_SET_IOPL_MASK (f) .set_iopl_mask = f,void (*wbinvd)(void );void (*io_delay)(void );
arch/x86/kernel/paravirt.c +1 -1
@@ -383,7 +383,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = { .iret = native_iret, .swapgs = native_swapgs, - .set_iopl_mask = native_set_iopl_mask, +INIT_SET_IOPL_MASK (native_set_iopl_mask) .io_delay = native_io_delay, .start_context_switch = paravirt_nop,
arch/x86/xen/enlighten.c +2 -2
@@ -912,7 +912,7 @@ static void xen_load_sp0(struct tss_struct *tss,xen_mc_issue (PARAVIRT_LAZY_CPU ); } -static void xen_set_iopl_mask (unsigned mask) +static void __maybe_unused xen_set_iopl_mask (unsigned mask) {struct physdev_set_iopl set_iopl;
@@ -1279,7 +1279,7 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = { .write_idt_entry = xen_write_idt_entry, .load_sp0 = xen_load_sp0, - .set_iopl_mask = xen_set_iopl_mask, +INIT_SET_IOPL_MASK (xen_set_iopl_mask) .io_delay = xen_io_delay,/* Xen takes care of %gs when switching to usermode for us */
Signature · 1 line
2.1.1
PATCH v4 5/10 x86: cpu: Add helper function unifying 32-bit and 64-bit IO init in cpu_init
In reply to Josh TriplettPreviously, the 64-bit code initialized both the io_bitmap_base and io_bitmap, while the 32-bit code only initialized io_bitmap_base.
Factor the initialization out into a helper function init_tss_io, and call it from both versions of cpu_init.
This will also make it easier to make these IO-related fields optional later.
--- arch/x86/include/asm/processor.h | 14 ++++++++++++++ arch/x86/kernel/cpu/common.c | 12 ++---------- 2 files changed, 16 insertions(+), 10 deletions(-)
arch/x86/include/asm/processor.h +14 -0
@@ -286,6 +286,20 @@ struct tss_struct { DECLARE_PER_CPU_SHARED_ALIGNED(struct tss_struct, init_tss); +static inline void init_tss_io (struct tss_struct *t) +{ +int i; + + t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap); + +/* +* <= is required because the CPU will access up to +* 8 bits beyond the end of the IO permission bitmap. +*/ +for (i =0 ; i <=IO_BITMAP_LONGS ; i++) + t->io_bitmap[i] = ~0UL ; +} +/* * Save the original ist values for checking stack pointers during debugging */
arch/x86/kernel/cpu/common.c +2 -10
@@ -1297,7 +1297,6 @@ void cpu_init(void)struct tss_struct *t;unsigned long v;int cpu =stack_smp_processor_id (); -int i;wait_for_master_cpu (cpu);
@@ -1357,14 +1356,7 @@ void cpu_init(void) } } - t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap); - -/* -* <= is required because the CPU will access up to -* 8 bits beyond the end of the IO permission bitmap. -*/ -for (i =0 ; i <=IO_BITMAP_LONGS ; i++) - t->io_bitmap[i] = ~0UL ; +init_tss_io (t);atomic_inc (&init_mm.mm_count); me->active_mm = &init_mm;
@@ -1419,7 +1411,7 @@ void cpu_init(void)load_TR_desc ();load_LDT (&init_mm.context); - t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap); +init_tss_io (t);#ifdef CONFIG_DOUBLEFAULT /* Set up doublefault TSS pointer in the GDT */
Signature · 1 line
2.1.1
PATCH v4 5/10 x86: cpu: Add helper function unifying 32-bit and 64-bit IO init in cpu_init
In reply to Josh TriplettPreviously, the 64-bit code initialized both the io_bitmap_base and io_bitmap, while the 32-bit code only initialized io_bitmap_base.
Factor the initialization out into a helper function init_tss_io, and call it from both versions of cpu_init.
This will also make it easier to make these IO-related fields optional later.
--- arch/x86/include/asm/processor.h | 14 ++++++++++++++ arch/x86/kernel/cpu/common.c | 12 ++---------- 2 files changed, 16 insertions(+), 10 deletions(-)
arch/x86/include/asm/processor.h +14 -0
@@ -286,6 +286,20 @@ struct tss_struct { DECLARE_PER_CPU_SHARED_ALIGNED(struct tss_struct, init_tss); +static inline void init_tss_io (struct tss_struct *t) +{ +int i; + + t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap); + +/* +* <= is required because the CPU will access up to +* 8 bits beyond the end of the IO permission bitmap. +*/ +for (i =0 ; i <=IO_BITMAP_LONGS ; i++) + t->io_bitmap[i] = ~0UL ; +} +/* * Save the original ist values for checking stack pointers during debugging */
arch/x86/kernel/cpu/common.c +2 -10
@@ -1297,7 +1297,6 @@ void cpu_init(void)struct tss_struct *t;unsigned long v;int cpu =stack_smp_processor_id (); -int i;wait_for_master_cpu (cpu);
@@ -1357,14 +1356,7 @@ void cpu_init(void) } } - t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap); - -/* -* <= is required because the CPU will access up to -* 8 bits beyond the end of the IO permission bitmap. -*/ -for (i =0 ; i <=IO_BITMAP_LONGS ; i++) - t->io_bitmap[i] = ~0UL ; +init_tss_io (t);atomic_inc (&init_mm.mm_count); me->active_mm = &init_mm;
@@ -1419,7 +1411,7 @@ void cpu_init(void)load_TR_desc ();load_LDT (&init_mm.context); - t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap); +init_tss_io (t);#ifdef CONFIG_DOUBLEFAULT /* Set up doublefault TSS pointer in the GDT */
Signature · 1 line
2.1.1
PATCH v4 6/10 x86: process: Introduce helper to clear a thread's IO bitmap
In reply to Josh TriplettThis will make it easier to make the io_bitmap_ptr field optional later.
--- arch/x86/kernel/process-io.h | 5 +++++ arch/x86/kernel/process_32.c | 4 ++-- arch/x86/kernel/process_64.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-)
arch/x86/kernel/process-io.h +5 -0
@@ -1,6 +1,11 @@#ifndef _X86_KERNEL_PROCESS_IO_H#define _X86_KERNEL_PROCESS_IO_H +static inline void clear_thread_io_bitmap (struct task_struct *p) +{ + p->thread.io_bitmap_ptr =NULL ; +} +static inline int copy_io_bitmap (struct task_struct *me,struct task_struct *p) {
arch/x86/kernel/process_32.c +2 -2
@@ -154,7 +154,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, childregs->orig_ax =-1 ; childregs->cs = __KERNEL_CS |get_kernel_rpl (); childregs->flags =X86_EFLAGS_IF |X86_EFLAGS_FIXED ; - p->thread.io_bitmap_ptr =NULL ; +clear_thread_io_bitmap (p);return 0 ; } *childregs = *current_pt_regs ();
@@ -165,7 +165,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, p->thread.ip = (unsigned long ) ret_from_fork;task_user_gs (p) =get_user_gs (current_pt_regs ()); - p->thread.io_bitmap_ptr =NULL ; +clear_thread_io_bitmap (p); tsk = current;/*
arch/x86/kernel/process_64.c +1 -1
@@ -164,7 +164,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, p->thread.sp = (unsigned long ) childregs; p->thread.usersp = me->thread.usersp;set_tsk_thread_flag (p,TIF_FORK ); - p->thread.io_bitmap_ptr =NULL ; +clear_thread_io_bitmap (p);savesegment (gs, p->thread.gsindex); p->thread.gs = p->thread.gsindex ?0 : me->thread.gs;
Signature · 1 line
2.1.1
PATCH v4 6/10 x86: process: Introduce helper to clear a thread's IO bitmap
In reply to Josh TriplettThis will make it easier to make the io_bitmap_ptr field optional later.
--- arch/x86/kernel/process-io.h | 5 +++++ arch/x86/kernel/process_32.c | 4 ++-- arch/x86/kernel/process_64.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-)
arch/x86/kernel/process-io.h +5 -0
@@ -1,6 +1,11 @@#ifndef _X86_KERNEL_PROCESS_IO_H#define _X86_KERNEL_PROCESS_IO_H +static inline void clear_thread_io_bitmap (struct task_struct *p) +{ + p->thread.io_bitmap_ptr =NULL ; +} +static inline int copy_io_bitmap (struct task_struct *me,struct task_struct *p) {
arch/x86/kernel/process_32.c +2 -2
@@ -154,7 +154,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, childregs->orig_ax =-1 ; childregs->cs = __KERNEL_CS |get_kernel_rpl (); childregs->flags =X86_EFLAGS_IF |X86_EFLAGS_FIXED ; - p->thread.io_bitmap_ptr =NULL ; +clear_thread_io_bitmap (p);return 0 ; } *childregs = *current_pt_regs ();
@@ -165,7 +165,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, p->thread.ip = (unsigned long ) ret_from_fork;task_user_gs (p) =get_user_gs (current_pt_regs ()); - p->thread.io_bitmap_ptr =NULL ; +clear_thread_io_bitmap (p); tsk = current;/*
arch/x86/kernel/process_64.c +1 -1
@@ -164,7 +164,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, p->thread.sp = (unsigned long ) childregs; p->thread.usersp = me->thread.usersp;set_tsk_thread_flag (p,TIF_FORK ); - p->thread.io_bitmap_ptr =NULL ; +clear_thread_io_bitmap (p);savesegment (gs, p->thread.gsindex); p->thread.gs = p->thread.gsindex ?0 : me->thread.gs;
Signature · 1 line
2.1.1
PATCH v4 6/10 x86: process: Introduce helper to clear a thread's IO bitmap
In reply to Josh TriplettThis will make it easier to make the io_bitmap_ptr field optional later.
--- arch/x86/kernel/process-io.h | 5 +++++ arch/x86/kernel/process_32.c | 4 ++-- arch/x86/kernel/process_64.c | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-)
arch/x86/kernel/process-io.h +5 -0
@@ -1,6 +1,11 @@#ifndef _X86_KERNEL_PROCESS_IO_H#define _X86_KERNEL_PROCESS_IO_H +static inline void clear_thread_io_bitmap (struct task_struct *p) +{ + p->thread.io_bitmap_ptr =NULL ; +} +static inline int copy_io_bitmap (struct task_struct *me,struct task_struct *p) {
arch/x86/kernel/process_32.c +2 -2
@@ -154,7 +154,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, childregs->orig_ax =-1 ; childregs->cs = __KERNEL_CS |get_kernel_rpl (); childregs->flags =X86_EFLAGS_IF |X86_EFLAGS_FIXED ; - p->thread.io_bitmap_ptr =NULL ; +clear_thread_io_bitmap (p);return 0 ; } *childregs = *current_pt_regs ();
@@ -165,7 +165,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, p->thread.ip = (unsigned long ) ret_from_fork;task_user_gs (p) =get_user_gs (current_pt_regs ()); - p->thread.io_bitmap_ptr =NULL ; +clear_thread_io_bitmap (p); tsk = current;/*
arch/x86/kernel/process_64.c +1 -1
@@ -164,7 +164,7 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, p->thread.sp = (unsigned long ) childregs; p->thread.usersp = me->thread.usersp;set_tsk_thread_flag (p,TIF_FORK ); - p->thread.io_bitmap_ptr =NULL ; +clear_thread_io_bitmap (p);savesegment (gs, p->thread.gsindex); p->thread.gs = p->thread.gsindex ?0 : me->thread.gs;
Signature · 1 line
2.1.1
PATCH v4 7/10 x86: process: Introduce helper to switch iopl mask
In reply to Josh TriplettThis simplifies __switch_to a bit, and will make it easier to make iopl optional later.
--- arch/x86/kernel/process-io.h | 13 +++++++++++++ arch/x86/kernel/process_32.c | 9 +-------- 2 files changed, 14 insertions(+), 8 deletions(-)
arch/x86/kernel/process-io.h +13 -0
@@ -24,4 +24,17 @@ static inline int copy_io_bitmap(struct task_struct *me, return 0; } +static inline void switch_iopl_mask (struct thread_struct *prev, +struct thread_struct *next) +{ +/* +* Restore IOPL if needed. In normal use, the flags restore +* in the switch assembly will handle this. But if the kernel +* is running virtualized at a non-zero CPL, the popf will +* not restore flags, so it must be done in a separate step. +*/ +if (get_kernel_rpl () &&unlikely (prev->iopl != next->iopl)) +set_iopl_mask (next->iopl); +} +#endif /* _X86_KERNEL_PROCESS_IO_H */
arch/x86/kernel/process_32.c +1 -8
@@ -265,14 +265,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)*/ load_TLS (next, cpu); -/* -* Restore IOPL if needed. In normal use, the flags restore -* in the switch assembly will handle this. But if the kernel -* is running virtualized at a non-zero CPL, the popf will -* not restore flags, so it must be done in a separate step. -*/ -if (get_kernel_rpl () &&unlikely (prev->iopl != next->iopl)) -set_iopl_mask (next->iopl); +switch_iopl_mask (prev, next);/* * If it were not for PREEMPT_ACTIVE we could guarantee that the
Signature · 1 line
2.1.1
PATCH v4 7/10 x86: process: Introduce helper to switch iopl mask
In reply to Josh TriplettThis simplifies __switch_to a bit, and will make it easier to make iopl optional later.
--- arch/x86/kernel/process-io.h | 13 +++++++++++++ arch/x86/kernel/process_32.c | 9 +-------- 2 files changed, 14 insertions(+), 8 deletions(-)
arch/x86/kernel/process-io.h +13 -0
@@ -24,4 +24,17 @@ static inline int copy_io_bitmap(struct task_struct *me, return 0; } +static inline void switch_iopl_mask (struct thread_struct *prev, +struct thread_struct *next) +{ +/* +* Restore IOPL if needed. In normal use, the flags restore +* in the switch assembly will handle this. But if the kernel +* is running virtualized at a non-zero CPL, the popf will +* not restore flags, so it must be done in a separate step. +*/ +if (get_kernel_rpl () &&unlikely (prev->iopl != next->iopl)) +set_iopl_mask (next->iopl); +} +#endif /* _X86_KERNEL_PROCESS_IO_H */
arch/x86/kernel/process_32.c +1 -8
@@ -265,14 +265,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)*/ load_TLS (next, cpu); -/* -* Restore IOPL if needed. In normal use, the flags restore -* in the switch assembly will handle this. But if the kernel -* is running virtualized at a non-zero CPL, the popf will -* not restore flags, so it must be done in a separate step. -*/ -if (get_kernel_rpl () &&unlikely (prev->iopl != next->iopl)) -set_iopl_mask (next->iopl); +switch_iopl_mask (prev, next);/* * If it were not for PREEMPT_ACTIVE we could guarantee that the
Signature · 1 line
2.1.1
PATCH v4 8/10 x86: process: Introduce helper for IO-related bits of exit_thread
In reply to Josh TriplettThis makes the two major functions of exit_thread (IO and FPU) more obvious, and will make it easier to make IO optional later.
--- arch/x86/kernel/process-io.h | 20 ++++++++++++++++++++ arch/x86/kernel/process.c | 20 +++----------------- 2 files changed, 23 insertions(+), 17 deletions(-)
arch/x86/kernel/process-io.h +20 -0
@@ -24,6 +24,26 @@ static inline int copy_io_bitmap(struct task_struct *me,return 0 ; } +static inline void exit_thread_io (struct task_struct *me) +{ +struct thread_struct *t = &me->thread; +unsigned long *bp = t->io_bitmap_ptr; + +if (bp) { +struct tss_struct *tss = &per_cpu (init_tss,get_cpu ()); + + t->io_bitmap_ptr =NULL ; +clear_thread_flag (TIF_IO_BITMAP ); +/* +* Careful, clear this in the TSS too: +*/ +memset (tss->io_bitmap,0xff , t->io_bitmap_max); + t->io_bitmap_max =0 ; +put_cpu (); +kfree (bp); + } +} +static inline void switch_iopl_mask (struct thread_struct *prev,struct thread_struct *next) {
arch/x86/kernel/process.c +3 -17
@@ -29,6 +29,8 @@#include <asm/debugreg.h> #include <asm/nmi.h> +#include "process-io.h" +/* * per-CPU TSS segments. Threads are completely 'soft' on Linux, * no more per-task TSS's. The TSS size is kept cacheline-aligned
@@ -104,23 +106,7 @@ void arch_task_cache_init(void)void exit_thread (void ) {struct task_struct *me = current; -struct thread_struct *t = &me->thread; -unsigned long *bp = t->io_bitmap_ptr; - -if (bp) { -struct tss_struct *tss = &per_cpu (init_tss,get_cpu ()); - - t->io_bitmap_ptr =NULL ; -clear_thread_flag (TIF_IO_BITMAP ); -/* -* Careful, clear this in the TSS too: -*/ -memset (tss->io_bitmap,0xff , t->io_bitmap_max); - t->io_bitmap_max =0 ; -put_cpu (); -kfree (bp); - } - +exit_thread_io (me);drop_fpu (me); }
Signature · 1 line
2.1.1
PATCH v4 8/10 x86: process: Introduce helper for IO-related bits of exit_thread
In reply to Josh TriplettThis makes the two major functions of exit_thread (IO and FPU) more obvious, and will make it easier to make IO optional later.
--- arch/x86/kernel/process-io.h | 20 ++++++++++++++++++++ arch/x86/kernel/process.c | 20 +++----------------- 2 files changed, 23 insertions(+), 17 deletions(-)
arch/x86/kernel/process-io.h +20 -0
@@ -24,6 +24,26 @@ static inline int copy_io_bitmap(struct task_struct *me,return 0 ; } +static inline void exit_thread_io (struct task_struct *me) +{ +struct thread_struct *t = &me->thread; +unsigned long *bp = t->io_bitmap_ptr; + +if (bp) { +struct tss_struct *tss = &per_cpu (init_tss,get_cpu ()); + + t->io_bitmap_ptr =NULL ; +clear_thread_flag (TIF_IO_BITMAP ); +/* +* Careful, clear this in the TSS too: +*/ +memset (tss->io_bitmap,0xff , t->io_bitmap_max); + t->io_bitmap_max =0 ; +put_cpu (); +kfree (bp); + } +} +static inline void switch_iopl_mask (struct thread_struct *prev,struct thread_struct *next) {
arch/x86/kernel/process.c +3 -17
@@ -29,6 +29,8 @@#include <asm/debugreg.h> #include <asm/nmi.h> +#include "process-io.h" +/* * per-CPU TSS segments. Threads are completely 'soft' on Linux, * no more per-task TSS's. The TSS size is kept cacheline-aligned
@@ -104,23 +106,7 @@ void arch_task_cache_init(void)void exit_thread (void ) {struct task_struct *me = current; -struct thread_struct *t = &me->thread; -unsigned long *bp = t->io_bitmap_ptr; - -if (bp) { -struct tss_struct *tss = &per_cpu (init_tss,get_cpu ()); - - t->io_bitmap_ptr =NULL ; -clear_thread_flag (TIF_IO_BITMAP ); -/* -* Careful, clear this in the TSS too: -*/ -memset (tss->io_bitmap,0xff , t->io_bitmap_max); - t->io_bitmap_max =0 ; -put_cpu (); -kfree (bp); - } - +exit_thread_io (me);drop_fpu (me); }
Signature · 1 line
2.1.1
PATCH v4 9/10 x86: process: Introduce helper to switch IO bitmap
In reply to Josh TriplettThis simplifies __switch_to_xtra, and will make it easier to make IO optional later.
--- arch/x86/kernel/process-io.h | 23 +++++++++++++++++++++++ arch/x86/kernel/process.c | 14 +------------- 2 files changed, 24 insertions(+), 13 deletions(-)
arch/x86/kernel/process-io.h +23 -0
@@ -57,4 +57,27 @@ static inline void switch_iopl_mask(struct thread_struct *prev, set_iopl_mask(next->iopl); } +static inline void switch_io_bitmap (struct tss_struct *tss, +struct task_struct *prev_p, +struct task_struct *next_p) +{ +struct thread_struct *prev, *next; + prev = &prev_p->thread; + next = &next_p->thread; + +if (test_tsk_thread_flag (next_p,TIF_IO_BITMAP )) { +/* +* Copy the relevant range of the IO bitmap. +* Normally this is 128 bytes or less: +*/ +memcpy (tss->io_bitmap, next->io_bitmap_ptr, +max (prev->io_bitmap_max, next->io_bitmap_max)); + }else if (test_tsk_thread_flag (prev_p,TIF_IO_BITMAP )) { +/* +* Clear any possible leftover bits: +*/ +memset (tss->io_bitmap,0xff , prev->io_bitmap_max); + } +} +#endif /* _X86_KERNEL_PROCESS_IO_H */
arch/x86/kernel/process.c +1 -13
@@ -211,19 +211,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,hard_enable_TSC (); } -if (test_tsk_thread_flag (next_p,TIF_IO_BITMAP )) { -/* -* Copy the relevant range of the IO bitmap. -* Normally this is 128 bytes or less: -*/ -memcpy (tss->io_bitmap, next->io_bitmap_ptr, -max (prev->io_bitmap_max, next->io_bitmap_max)); - }else if (test_tsk_thread_flag (prev_p,TIF_IO_BITMAP )) { -/* -* Clear any possible leftover bits: -*/ -memset (tss->io_bitmap,0xff , prev->io_bitmap_max); - } +switch_io_bitmap (tss, prev_p, next_p);propagate_user_return_notify (prev_p, next_p); }
Signature · 1 line
2.1.1
PATCH v4 9/10 x86: process: Introduce helper to switch IO bitmap
In reply to Josh TriplettThis simplifies __switch_to_xtra, and will make it easier to make IO optional later.
--- arch/x86/kernel/process-io.h | 23 +++++++++++++++++++++++ arch/x86/kernel/process.c | 14 +------------- 2 files changed, 24 insertions(+), 13 deletions(-)
arch/x86/kernel/process-io.h +23 -0
@@ -57,4 +57,27 @@ static inline void switch_iopl_mask(struct thread_struct *prev, set_iopl_mask(next->iopl); } +static inline void switch_io_bitmap (struct tss_struct *tss, +struct task_struct *prev_p, +struct task_struct *next_p) +{ +struct thread_struct *prev, *next; + prev = &prev_p->thread; + next = &next_p->thread; + +if (test_tsk_thread_flag (next_p,TIF_IO_BITMAP )) { +/* +* Copy the relevant range of the IO bitmap. +* Normally this is 128 bytes or less: +*/ +memcpy (tss->io_bitmap, next->io_bitmap_ptr, +max (prev->io_bitmap_max, next->io_bitmap_max)); + }else if (test_tsk_thread_flag (prev_p,TIF_IO_BITMAP )) { +/* +* Clear any possible leftover bits: +*/ +memset (tss->io_bitmap,0xff , prev->io_bitmap_max); + } +} +#endif /* _X86_KERNEL_PROCESS_IO_H */
arch/x86/kernel/process.c +1 -13
@@ -211,19 +211,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,hard_enable_TSC (); } -if (test_tsk_thread_flag (next_p,TIF_IO_BITMAP )) { -/* -* Copy the relevant range of the IO bitmap. -* Normally this is 128 bytes or less: -*/ -memcpy (tss->io_bitmap, next->io_bitmap_ptr, -max (prev->io_bitmap_max, next->io_bitmap_max)); - }else if (test_tsk_thread_flag (prev_p,TIF_IO_BITMAP )) { -/* -* Clear any possible leftover bits: -*/ -memset (tss->io_bitmap,0xff , prev->io_bitmap_max); - } +switch_io_bitmap (tss, prev_p, next_p);propagate_user_return_notify (prev_p, next_p); }
Signature · 1 line
2.1.1
PATCH v4 9/10 x86: process: Introduce helper to switch IO bitmap
In reply to Josh TriplettThis simplifies __switch_to_xtra, and will make it easier to make IO optional later.
--- arch/x86/kernel/process-io.h | 23 +++++++++++++++++++++++ arch/x86/kernel/process.c | 14 +------------- 2 files changed, 24 insertions(+), 13 deletions(-)
arch/x86/kernel/process-io.h +23 -0
@@ -57,4 +57,27 @@ static inline void switch_iopl_mask(struct thread_struct *prev, set_iopl_mask(next->iopl); } +static inline void switch_io_bitmap (struct tss_struct *tss, +struct task_struct *prev_p, +struct task_struct *next_p) +{ +struct thread_struct *prev, *next; + prev = &prev_p->thread; + next = &next_p->thread; + +if (test_tsk_thread_flag (next_p,TIF_IO_BITMAP )) { +/* +* Copy the relevant range of the IO bitmap. +* Normally this is 128 bytes or less: +*/ +memcpy (tss->io_bitmap, next->io_bitmap_ptr, +max (prev->io_bitmap_max, next->io_bitmap_max)); + }else if (test_tsk_thread_flag (prev_p,TIF_IO_BITMAP )) { +/* +* Clear any possible leftover bits: +*/ +memset (tss->io_bitmap,0xff , prev->io_bitmap_max); + } +} +#endif /* _X86_KERNEL_PROCESS_IO_H */
arch/x86/kernel/process.c +1 -13
@@ -211,19 +211,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,hard_enable_TSC (); } -if (test_tsk_thread_flag (next_p,TIF_IO_BITMAP )) { -/* -* Copy the relevant range of the IO bitmap. -* Normally this is 128 bytes or less: -*/ -memcpy (tss->io_bitmap, next->io_bitmap_ptr, -max (prev->io_bitmap_max, next->io_bitmap_max)); - }else if (test_tsk_thread_flag (prev_p,TIF_IO_BITMAP )) { -/* -* Clear any possible leftover bits: -*/ -memset (tss->io_bitmap,0xff , prev->io_bitmap_max); - } +switch_io_bitmap (tss, prev_p, next_p);propagate_user_return_notify (prev_p, next_p); }
Signature · 1 line
2.1.1
PATCH v4 10/10 x86: Support compiling out userspace IO (iopl and ioperm)
In reply to Josh TriplettOn the vast majority of modern systems, no processes will use the userspsace IO syscalls, iopl and ioperm. Add a new config option, CONFIG_X86_IOPORT, to support configuring them out of the kernel entirely. Most current systems do not run programs using these syscalls, so X86_IOPORT does not depend on EXPERT, though it does still default to y.
In addition to saving a significant amount of space, this also reduces the size of several major kernel data structures, drops a bit of code from several task-related hot paths, and reduces the attack surface of the kernel.
bloat-o-meter results:
add/remove: 0/4 grow/shrink: 4/9 up/down: 83/-9074 (-8991)
function old new delta drop_fpu 80 160 +80 perf_event_init_task 638 639 +1 perf_event_exec 192 193 +1 perf_event_aux 132 133 +1 test_tsk_thread_flag 10 - -10 ioperm_active 14 3 -11 init_task 916 904 -12 flush_thread 168 149 -19 cpu_init 364 339 -25 __drop_fpu 60 - -60 ioperm_get 92 6 -86 exit_thread 105 10 -95 sys_iopl 106 - -106 copy_thread 364 257 -107 __switch_to_xtra 234 123 -111 sys_ioperm 240 - -240
init_tss 8576 384 -8192
--- arch/x86/Kconfig | 10 ++++++++++ arch/x86/include/asm/paravirt.h | 2 ++ arch/x86/include/asm/paravirt_types.h | 4 ++++ arch/x86/include/asm/processor.h | 19 ++++++++++++++++++- arch/x86/include/asm/syscalls.h | 3 +++ arch/x86/kernel/Makefile | 3 ++- arch/x86/kernel/entry_64.S | 9 ++++++--- arch/x86/kernel/process-io.h | 10 ++++++++++ arch/x86/kernel/ptrace.c | 8 ++++++++ drivers/tty/vt/vt_ioctl.c | 2 +- kernel/sys_ni.c | 5 +++++ 11 files changed, 69 insertions(+), 6 deletions(-)
arch/x86/Kconfig +10 -0
@@ -984,6 +984,16 @@ config X86_ESPFIX64 def_bool y depends on X86_16BIT && X86_64 +config X86_IOPORT + bool "iopl and ioperm system calls" + default y + ---help--- + This option enables the iopl and ioperm system calls, which allow + privileged userspace processes to directly access I/O ports. This + is used by software that drives hardware directly from userspace + without using a kernel driver. Unless you intend to run such + software, you can safely say N here. + config TOSHIBA tristate "Toshiba Laptop support" depends on X86_32
arch/x86/include/asm/paravirt.h +2 -0
@@ -299,10 +299,12 @@ static inline void write_idt_entry(gate_desc *dt, int entry, const gate_desc *g) {PVOP_VCALL3 (pv_cpu_ops.write_idt_entry, dt, entry, g); } +#ifdef CONFIG_X86_IOPORT static inline void set_iopl_mask (unsigned mask) {PVOP_VCALL1 (pv_cpu_ops.set_iopl_mask, mask); } +#endif /* CONFIG_X86_IOPORT */ /* The paravirtualized I/O functions */ static inline void slow_down_io (void )
arch/x86/include/asm/paravirt_types.h +4 -0
@@ -142,8 +142,12 @@ struct pv_cpu_ops {void (*load_sp0)(struct tss_struct *tss,struct thread_struct *t); +#ifdef CONFIG_X86_IOPORT void (*set_iopl_mask)(unsigned mask);#define INIT_SET_IOPL_MASK (f) .set_iopl_mask = f, +#else +#define INIT_SET_IOPL_MASK (f) +#endif void (*wbinvd)(void );void (*io_delay)(void );
arch/x86/include/asm/processor.h +18 -1
@@ -255,7 +255,11 @@ struct x86_hw_tss {/* * IO-bitmap sizes: */ +#ifdef CONFIG_X86_IOPORT #define IO_BITMAP_BITS 65536 +#else +#define IO_BITMAP_BITS 0 +#endif #define IO_BITMAP_BYTES (IO_BITMAP_BITS/8)#define IO_BITMAP_LONGS (IO_BITMAP_BYTES/sizeof(long))#define INVALID_IO_BITMAP_OFFSET 0x8000
@@ -269,6 +273,7 @@ struct tss_struct {*/ struct x86_hw_tss x86_tss; +#ifdef CONFIG_X86_IOPORT /* * The extra 1 is there because the CPU will access an * additional byte beyond the end of the IO permission
@@ -276,6 +281,7 @@ struct tss_struct {* be within the limit. */ unsigned long io_bitmap[IO_BITMAP_LONGS +1 ]; +#endif /* CONFIG_X86_IOPORT */ /* * .. and then another 0x100 bytes for the emergency kernel stack:
@@ -288,6 +294,7 @@ DECLARE_PER_CPU_SHARED_ALIGNED(struct tss_struct, init_tss);static inline void init_tss_io (struct tss_struct *t) { +#ifdef CONFIG_X86_IOPORT int i; t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap);
@@ -298,6 +305,9 @@ static inline void init_tss_io(struct tss_struct *t)*/ for (i =0 ; i <=IO_BITMAP_LONGS ; i++) t->io_bitmap[i] = ~0UL ; +#else + t->x86_tss.io_bitmap_base =INVALID_IO_BITMAP_OFFSET ; +#endif }/*
@@ -524,11 +534,13 @@ struct thread_struct {unsigned int saved_fs;unsigned int saved_gs;#endif +#ifdef CONFIG_X86_IOPORT /* IO permissions: */ unsigned long *io_bitmap_ptr;unsigned long iopl;/* Max allowed port in the bitmap, in bytes: */ unsigned io_bitmap_max; +#endif /* CONFIG_X86_IOPORT */ /* * fpu_counter contains the number of consecutive context switches * that the FPU is used. If this is over a threshold, the lazy fpu
@@ -545,7 +557,7 @@ struct thread_struct {*/ static inline void native_set_iopl_mask (unsigned mask) { -#ifdef CONFIG_X86_32 +#if defined(CONFIG_X86_IOPORT ) && defined(CONFIG_X86_32 )unsigned int reg; asm volatile ("pushfl;"
@@ -856,6 +868,7 @@ static inline void spin_lock_prefetch(const void *x)#define STACK_TOP TASK_SIZE#define STACK_TOP_MAX STACK_TOP +#ifdef CONFIG_X86_IOPORT #define INIT_THREAD_IO .io_bitmap_ptr = NULL,/*
@@ -865,6 +878,10 @@ static inline void spin_lock_prefetch(const void *x)* be within the limit. */ #define INIT_TSS_IO .io_bitmap = { [0 ... IO_BITMAP_LONGS] = ~0 }, +#else +#define INIT_THREAD_IO +#define INIT_TSS_IO +#endif #define INIT_THREAD { \ .sp0 = sizeof(init_stack) + (long)&init_stack, \
arch/x86/include/asm/syscalls.h +3 -0
@@ -16,9 +16,12 @@#include <linux/types.h> /* Common in X86_32 and X86_64 */ + +#ifdef CONFIG_X86_IOPORT /* kernel/ioport.c */ asmlinkage long sys_ioperm (unsigned long ,unsigned long ,int );asmlinkage long sys_iopl (unsigned int ); +#endif /* CONFIG_X86_IOPORT */ /* kernel/ldt.c */ asmlinkage int sys_modify_ldt(int, void __user *, unsigned long);
arch/x86/kernel/Makefile +2 -1
@@ -20,7 +20,8 @@ CFLAGS_irq.o := -I$(src)/../include/asm/trace obj-y := process_$(BITS).o signal.o entry_$(BITS).o obj-y += traps.o irq.o irq_$(BITS).o dumpstack_$(BITS).o -obj-y += time.o ioport.o ldt.o dumpstack.o nmi.o +obj-y += time.o ldt.o dumpstack.o nmi.o +obj-$(CONFIG_X86_IOPORT) += ioport.o obj-y += setup.o x86_init.o i8259.o irqinit.o jump_label.o obj-$(CONFIG_IRQ_WORK) += irq_work.o obj-y += probe_roms.o
arch/x86/kernel/entry_64.S +6 -3
@@ -609,6 +609,11 @@ ENTRY(stub_\func) END(stub_\func) .endm + FORK_LIKE clone + FORK_LIKE fork + FORK_LIKE vfork + +#ifdef CONFIG_X86_IOPORT .macro FIXED_FRAME label,func ENTRY(\label) CFI_STARTPROC
@@ -621,10 +626,8 @@ ENTRY(\label) END(\label) .endm - FORK_LIKE clone - FORK_LIKE fork - FORK_LIKE vfork FIXED_FRAME stub_iopl, sys_iopl +#endif /* CONFIG_X86_IOPORT */ ENTRY(ptregscall_common) DEFAULT_FRAME 1 8 /* offset 8: return address */
arch/x86/kernel/process-io.h +10 -0
@@ -3,12 +3,15 @@static inline void clear_thread_io_bitmap (struct task_struct *p) { +#ifdef CONFIG_X86_IOPORT p->thread.io_bitmap_ptr =NULL ; +#endif /* CONFIG_X86_IOPORT */ }static inline int copy_io_bitmap (struct task_struct *me,struct task_struct *p) { +#ifdef CONFIG_X86_IOPORT if (unlikely (test_tsk_thread_flag (me,TIF_IO_BITMAP ))) { p->thread.io_bitmap_ptr =kmemdup (me->thread.io_bitmap_ptr,IO_BITMAP_BYTES ,GFP_KERNEL );
@@ -20,12 +23,14 @@ static inline int copy_io_bitmap(struct task_struct *me, } else { p->thread.io_bitmap_ptr =NULL ; } +#endif /* CONFIG_X86_IOPORT */ return 0 ; }static inline void exit_thread_io (struct task_struct *me) { +#ifdef CONFIG_X86_IOPORT struct thread_struct *t = &me->thread;unsigned long *bp = t->io_bitmap_ptr;
@@ -42,11 +47,13 @@ static inline void exit_thread_io(struct task_struct *me)put_cpu (); kfree(bp); } +#endif /* CONFIG_X86_IOPORT */ }static inline void switch_iopl_mask (struct thread_struct *prev,struct thread_struct *next) { +#ifdef CONFIG_X86_IOPORT /* * Restore IOPL if needed. In normal use, the flags restore * in the switch assembly will handle this. But if the kernel
@@ -55,12 +62,14 @@ static inline void switch_iopl_mask(struct thread_struct *prev,*/ if (get_kernel_rpl () &&unlikely (prev->iopl != next->iopl))set_iopl_mask (next->iopl); +#endif /* CONFIG_X86_IOPORT */ }static inline void switch_io_bitmap (struct tss_struct *tss,struct task_struct *prev_p,struct task_struct *next_p) { +#ifdef CONFIG_X86_IOPORT struct thread_struct *prev, *next; prev = &prev_p->thread; next = &next_p->thread;
@@ -78,6 +87,7 @@ static inline void switch_io_bitmap(struct tss_struct *tss,*/ memset (tss->io_bitmap,0xff , prev->io_bitmap_max); } +#endif /* CONFIG_X86_IOPORT */ }#endif /* _X86_KERNEL_PROCESS_IO_H */
arch/x86/kernel/ptrace.c +8 -0
@@ -785,7 +785,11 @@ static int ptrace_set_debugreg(struct task_struct *tsk, int n,static int ioperm_active (struct task_struct *target,const struct user_regset *regset) { +#ifdef CONFIG_X86_IOPORT return target->thread.io_bitmap_max / regset->size; +#else +return 0 ; +#endif } static int ioperm_get(struct task_struct *target,
@@ -793,12 +797,16 @@ static int ioperm_get(struct task_struct *target, unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf) { +#ifdef CONFIG_X86_IOPORT if (!target->thread.io_bitmap_ptr) return -ENXIO; return user_regset_copyout(&pos, &count, &kbuf, &ubuf, target->thread.io_bitmap_ptr, 0, IO_BITMAP_BYTES); +#else +return -ENXIO ; +#endif }/*
drivers/tty/vt/vt_ioctl.c +1 -1
@@ -410,7 +410,7 @@ int vt_ioctl(struct tty_struct *tty,* * XXX: you should never use these, just call ioperm directly.. */ -#ifdef CONFIG_X86 +#ifdef CONFIG_X86_IOPORT case KDADDIO :case KDDELIO :/*
kernel/sys_ni.c +5 -0
@@ -224,3 +224,8 @@ cond_syscall(sys_seccomp);/* access BPF programs and maps */ cond_syscall (sys_bpf); + +/* userspace I/O port access */ +cond_syscall (stub_iopl); +cond_syscall (sys_iopl); +cond_syscall (sys_ioperm);
Signature · 1 line
2.1.1
PATCH v4 10/10 x86: Support compiling out userspace IO (iopl and ioperm)
In reply to Josh TriplettOn the vast majority of modern systems, no processes will use the userspsace IO syscalls, iopl and ioperm. Add a new config option, CONFIG_X86_IOPORT, to support configuring them out of the kernel entirely. Most current systems do not run programs using these syscalls, so X86_IOPORT does not depend on EXPERT, though it does still default to y.
In addition to saving a significant amount of space, this also reduces the size of several major kernel data structures, drops a bit of code from several task-related hot paths, and reduces the attack surface of the kernel.
bloat-o-meter results:
add/remove: 0/4 grow/shrink: 4/9 up/down: 83/-9074 (-8991)
function old new delta drop_fpu 80 160 +80 perf_event_init_task 638 639 +1 perf_event_exec 192 193 +1 perf_event_aux 132 133 +1 test_tsk_thread_flag 10 - -10 ioperm_active 14 3 -11 init_task 916 904 -12 flush_thread 168 149 -19 cpu_init 364 339 -25 __drop_fpu 60 - -60 ioperm_get 92 6 -86 exit_thread 105 10 -95 sys_iopl 106 - -106 copy_thread 364 257 -107 __switch_to_xtra 234 123 -111 sys_ioperm 240 - -240
init_tss 8576 384 -8192
--- arch/x86/Kconfig | 10 ++++++++++ arch/x86/include/asm/paravirt.h | 2 ++ arch/x86/include/asm/paravirt_types.h | 4 ++++ arch/x86/include/asm/processor.h | 19 ++++++++++++++++++- arch/x86/include/asm/syscalls.h | 3 +++ arch/x86/kernel/Makefile | 3 ++- arch/x86/kernel/entry_64.S | 9 ++++++--- arch/x86/kernel/process-io.h | 10 ++++++++++ arch/x86/kernel/ptrace.c | 8 ++++++++ drivers/tty/vt/vt_ioctl.c | 2 +- kernel/sys_ni.c | 5 +++++ 11 files changed, 69 insertions(+), 6 deletions(-)
arch/x86/Kconfig +10 -0
@@ -984,6 +984,16 @@ config X86_ESPFIX64 def_bool y depends on X86_16BIT && X86_64 +config X86_IOPORT + bool "iopl and ioperm system calls" + default y + ---help--- + This option enables the iopl and ioperm system calls, which allow + privileged userspace processes to directly access I/O ports. This + is used by software that drives hardware directly from userspace + without using a kernel driver. Unless you intend to run such + software, you can safely say N here. + config TOSHIBA tristate "Toshiba Laptop support" depends on X86_32
arch/x86/include/asm/paravirt.h +2 -0
@@ -299,10 +299,12 @@ static inline void write_idt_entry(gate_desc *dt, int entry, const gate_desc *g) {PVOP_VCALL3 (pv_cpu_ops.write_idt_entry, dt, entry, g); } +#ifdef CONFIG_X86_IOPORT static inline void set_iopl_mask (unsigned mask) {PVOP_VCALL1 (pv_cpu_ops.set_iopl_mask, mask); } +#endif /* CONFIG_X86_IOPORT */ /* The paravirtualized I/O functions */ static inline void slow_down_io (void )
arch/x86/include/asm/paravirt_types.h +4 -0
@@ -142,8 +142,12 @@ struct pv_cpu_ops {void (*load_sp0)(struct tss_struct *tss,struct thread_struct *t); +#ifdef CONFIG_X86_IOPORT void (*set_iopl_mask)(unsigned mask);#define INIT_SET_IOPL_MASK (f) .set_iopl_mask = f, +#else +#define INIT_SET_IOPL_MASK (f) +#endif void (*wbinvd)(void );void (*io_delay)(void );
arch/x86/include/asm/processor.h +18 -1
@@ -255,7 +255,11 @@ struct x86_hw_tss {/* * IO-bitmap sizes: */ +#ifdef CONFIG_X86_IOPORT #define IO_BITMAP_BITS 65536 +#else +#define IO_BITMAP_BITS 0 +#endif #define IO_BITMAP_BYTES (IO_BITMAP_BITS/8)#define IO_BITMAP_LONGS (IO_BITMAP_BYTES/sizeof(long))#define INVALID_IO_BITMAP_OFFSET 0x8000
@@ -269,6 +273,7 @@ struct tss_struct {*/ struct x86_hw_tss x86_tss; +#ifdef CONFIG_X86_IOPORT /* * The extra 1 is there because the CPU will access an * additional byte beyond the end of the IO permission
@@ -276,6 +281,7 @@ struct tss_struct {* be within the limit. */ unsigned long io_bitmap[IO_BITMAP_LONGS +1 ]; +#endif /* CONFIG_X86_IOPORT */ /* * .. and then another 0x100 bytes for the emergency kernel stack:
@@ -288,6 +294,7 @@ DECLARE_PER_CPU_SHARED_ALIGNED(struct tss_struct, init_tss);static inline void init_tss_io (struct tss_struct *t) { +#ifdef CONFIG_X86_IOPORT int i; t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap);
@@ -298,6 +305,9 @@ static inline void init_tss_io(struct tss_struct *t)*/ for (i =0 ; i <=IO_BITMAP_LONGS ; i++) t->io_bitmap[i] = ~0UL ; +#else + t->x86_tss.io_bitmap_base =INVALID_IO_BITMAP_OFFSET ; +#endif }/*
@@ -524,11 +534,13 @@ struct thread_struct {unsigned int saved_fs;unsigned int saved_gs;#endif +#ifdef CONFIG_X86_IOPORT /* IO permissions: */ unsigned long *io_bitmap_ptr;unsigned long iopl;/* Max allowed port in the bitmap, in bytes: */ unsigned io_bitmap_max; +#endif /* CONFIG_X86_IOPORT */ /* * fpu_counter contains the number of consecutive context switches * that the FPU is used. If this is over a threshold, the lazy fpu
@@ -545,7 +557,7 @@ struct thread_struct {*/ static inline void native_set_iopl_mask (unsigned mask) { -#ifdef CONFIG_X86_32 +#if defined(CONFIG_X86_IOPORT ) && defined(CONFIG_X86_32 )unsigned int reg; asm volatile ("pushfl;"
@@ -856,6 +868,7 @@ static inline void spin_lock_prefetch(const void *x)#define STACK_TOP TASK_SIZE#define STACK_TOP_MAX STACK_TOP +#ifdef CONFIG_X86_IOPORT #define INIT_THREAD_IO .io_bitmap_ptr = NULL,/*
@@ -865,6 +878,10 @@ static inline void spin_lock_prefetch(const void *x)* be within the limit. */ #define INIT_TSS_IO .io_bitmap = { [0 ... IO_BITMAP_LONGS] = ~0 }, +#else +#define INIT_THREAD_IO +#define INIT_TSS_IO +#endif #define INIT_THREAD { \ .sp0 = sizeof(init_stack) + (long)&init_stack, \
arch/x86/include/asm/syscalls.h +3 -0
@@ -16,9 +16,12 @@#include <linux/types.h> /* Common in X86_32 and X86_64 */ + +#ifdef CONFIG_X86_IOPORT /* kernel/ioport.c */ asmlinkage long sys_ioperm (unsigned long ,unsigned long ,int );asmlinkage long sys_iopl (unsigned int ); +#endif /* CONFIG_X86_IOPORT */ /* kernel/ldt.c */ asmlinkage int sys_modify_ldt(int, void __user *, unsigned long);
arch/x86/kernel/Makefile +2 -1
@@ -20,7 +20,8 @@ CFLAGS_irq.o := -I$(src)/../include/asm/trace obj-y := process_$(BITS).o signal.o entry_$(BITS).o obj-y += traps.o irq.o irq_$(BITS).o dumpstack_$(BITS).o -obj-y += time.o ioport.o ldt.o dumpstack.o nmi.o +obj-y += time.o ldt.o dumpstack.o nmi.o +obj-$(CONFIG_X86_IOPORT) += ioport.o obj-y += setup.o x86_init.o i8259.o irqinit.o jump_label.o obj-$(CONFIG_IRQ_WORK) += irq_work.o obj-y += probe_roms.o
arch/x86/kernel/entry_64.S +6 -3
@@ -609,6 +609,11 @@ ENTRY(stub_\func) END(stub_\func) .endm + FORK_LIKE clone + FORK_LIKE fork + FORK_LIKE vfork + +#ifdef CONFIG_X86_IOPORT .macro FIXED_FRAME label,func ENTRY(\label) CFI_STARTPROC
@@ -621,10 +626,8 @@ ENTRY(\label) END(\label) .endm - FORK_LIKE clone - FORK_LIKE fork - FORK_LIKE vfork FIXED_FRAME stub_iopl, sys_iopl +#endif /* CONFIG_X86_IOPORT */ ENTRY(ptregscall_common) DEFAULT_FRAME 1 8 /* offset 8: return address */
arch/x86/kernel/process-io.h +10 -0
@@ -3,12 +3,15 @@static inline void clear_thread_io_bitmap (struct task_struct *p) { +#ifdef CONFIG_X86_IOPORT p->thread.io_bitmap_ptr =NULL ; +#endif /* CONFIG_X86_IOPORT */ }static inline int copy_io_bitmap (struct task_struct *me,struct task_struct *p) { +#ifdef CONFIG_X86_IOPORT if (unlikely (test_tsk_thread_flag (me,TIF_IO_BITMAP ))) { p->thread.io_bitmap_ptr =kmemdup (me->thread.io_bitmap_ptr,IO_BITMAP_BYTES ,GFP_KERNEL );
@@ -20,12 +23,14 @@ static inline int copy_io_bitmap(struct task_struct *me, } else { p->thread.io_bitmap_ptr =NULL ; } +#endif /* CONFIG_X86_IOPORT */ return 0 ; }static inline void exit_thread_io (struct task_struct *me) { +#ifdef CONFIG_X86_IOPORT struct thread_struct *t = &me->thread;unsigned long *bp = t->io_bitmap_ptr;
@@ -42,11 +47,13 @@ static inline void exit_thread_io(struct task_struct *me)put_cpu (); kfree(bp); } +#endif /* CONFIG_X86_IOPORT */ }static inline void switch_iopl_mask (struct thread_struct *prev,struct thread_struct *next) { +#ifdef CONFIG_X86_IOPORT /* * Restore IOPL if needed. In normal use, the flags restore * in the switch assembly will handle this. But if the kernel
@@ -55,12 +62,14 @@ static inline void switch_iopl_mask(struct thread_struct *prev,*/ if (get_kernel_rpl () &&unlikely (prev->iopl != next->iopl))set_iopl_mask (next->iopl); +#endif /* CONFIG_X86_IOPORT */ }static inline void switch_io_bitmap (struct tss_struct *tss,struct task_struct *prev_p,struct task_struct *next_p) { +#ifdef CONFIG_X86_IOPORT struct thread_struct *prev, *next; prev = &prev_p->thread; next = &next_p->thread;
@@ -78,6 +87,7 @@ static inline void switch_io_bitmap(struct tss_struct *tss,*/ memset (tss->io_bitmap,0xff , prev->io_bitmap_max); } +#endif /* CONFIG_X86_IOPORT */ }#endif /* _X86_KERNEL_PROCESS_IO_H */
arch/x86/kernel/ptrace.c +8 -0
@@ -785,7 +785,11 @@ static int ptrace_set_debugreg(struct task_struct *tsk, int n,static int ioperm_active (struct task_struct *target,const struct user_regset *regset) { +#ifdef CONFIG_X86_IOPORT return target->thread.io_bitmap_max / regset->size; +#else +return 0 ; +#endif } static int ioperm_get(struct task_struct *target,
@@ -793,12 +797,16 @@ static int ioperm_get(struct task_struct *target, unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf) { +#ifdef CONFIG_X86_IOPORT if (!target->thread.io_bitmap_ptr) return -ENXIO; return user_regset_copyout(&pos, &count, &kbuf, &ubuf, target->thread.io_bitmap_ptr, 0, IO_BITMAP_BYTES); +#else +return -ENXIO ; +#endif }/*
drivers/tty/vt/vt_ioctl.c +1 -1
@@ -410,7 +410,7 @@ int vt_ioctl(struct tty_struct *tty,* * XXX: you should never use these, just call ioperm directly.. */ -#ifdef CONFIG_X86 +#ifdef CONFIG_X86_IOPORT case KDADDIO :case KDDELIO :/*
kernel/sys_ni.c +5 -0
@@ -224,3 +224,8 @@ cond_syscall(sys_seccomp);/* access BPF programs and maps */ cond_syscall (sys_bpf); + +/* userspace I/O port access */ +cond_syscall (stub_iopl); +cond_syscall (sys_iopl); +cond_syscall (sys_ioperm);
Signature · 1 line
2.1.1
PATCH v4 10/10 x86: Support compiling out userspace IO (iopl and ioperm)
In reply to Josh TriplettOn the vast majority of modern systems, no processes will use the userspsace IO syscalls, iopl and ioperm. Add a new config option, CONFIG_X86_IOPORT, to support configuring them out of the kernel entirely. Most current systems do not run programs using these syscalls, so X86_IOPORT does not depend on EXPERT, though it does still default to y.
In addition to saving a significant amount of space, this also reduces the size of several major kernel data structures, drops a bit of code from several task-related hot paths, and reduces the attack surface of the kernel.
bloat-o-meter results:
add/remove: 0/4 grow/shrink: 4/9 up/down: 83/-9074 (-8991)
function old new delta drop_fpu 80 160 +80 perf_event_init_task 638 639 +1 perf_event_exec 192 193 +1 perf_event_aux 132 133 +1 test_tsk_thread_flag 10 - -10 ioperm_active 14 3 -11 init_task 916 904 -12 flush_thread 168 149 -19 cpu_init 364 339 -25 __drop_fpu 60 - -60 ioperm_get 92 6 -86 exit_thread 105 10 -95 sys_iopl 106 - -106 copy_thread 364 257 -107 __switch_to_xtra 234 123 -111 sys_ioperm 240 - -240
init_tss 8576 384 -8192
--- arch/x86/Kconfig | 10 ++++++++++ arch/x86/include/asm/paravirt.h | 2 ++ arch/x86/include/asm/paravirt_types.h | 4 ++++ arch/x86/include/asm/processor.h | 19 ++++++++++++++++++- arch/x86/include/asm/syscalls.h | 3 +++ arch/x86/kernel/Makefile | 3 ++- arch/x86/kernel/entry_64.S | 9 ++++++--- arch/x86/kernel/process-io.h | 10 ++++++++++ arch/x86/kernel/ptrace.c | 8 ++++++++ drivers/tty/vt/vt_ioctl.c | 2 +- kernel/sys_ni.c | 5 +++++ 11 files changed, 69 insertions(+), 6 deletions(-)
arch/x86/Kconfig +10 -0
@@ -984,6 +984,16 @@ config X86_ESPFIX64 def_bool y depends on X86_16BIT && X86_64 +config X86_IOPORT + bool "iopl and ioperm system calls" + default y + ---help--- + This option enables the iopl and ioperm system calls, which allow + privileged userspace processes to directly access I/O ports. This + is used by software that drives hardware directly from userspace + without using a kernel driver. Unless you intend to run such + software, you can safely say N here. + config TOSHIBA tristate "Toshiba Laptop support" depends on X86_32
arch/x86/include/asm/paravirt.h +2 -0
@@ -299,10 +299,12 @@ static inline void write_idt_entry(gate_desc *dt, int entry, const gate_desc *g) {PVOP_VCALL3 (pv_cpu_ops.write_idt_entry, dt, entry, g); } +#ifdef CONFIG_X86_IOPORT static inline void set_iopl_mask (unsigned mask) {PVOP_VCALL1 (pv_cpu_ops.set_iopl_mask, mask); } +#endif /* CONFIG_X86_IOPORT */ /* The paravirtualized I/O functions */ static inline void slow_down_io (void )
arch/x86/include/asm/paravirt_types.h +4 -0
@@ -142,8 +142,12 @@ struct pv_cpu_ops {void (*load_sp0)(struct tss_struct *tss,struct thread_struct *t); +#ifdef CONFIG_X86_IOPORT void (*set_iopl_mask)(unsigned mask);#define INIT_SET_IOPL_MASK (f) .set_iopl_mask = f, +#else +#define INIT_SET_IOPL_MASK (f) +#endif void (*wbinvd)(void );void (*io_delay)(void );
arch/x86/include/asm/processor.h +18 -1
@@ -255,7 +255,11 @@ struct x86_hw_tss {/* * IO-bitmap sizes: */ +#ifdef CONFIG_X86_IOPORT #define IO_BITMAP_BITS 65536 +#else +#define IO_BITMAP_BITS 0 +#endif #define IO_BITMAP_BYTES (IO_BITMAP_BITS/8)#define IO_BITMAP_LONGS (IO_BITMAP_BYTES/sizeof(long))#define INVALID_IO_BITMAP_OFFSET 0x8000
@@ -269,6 +273,7 @@ struct tss_struct {*/ struct x86_hw_tss x86_tss; +#ifdef CONFIG_X86_IOPORT /* * The extra 1 is there because the CPU will access an * additional byte beyond the end of the IO permission
@@ -276,6 +281,7 @@ struct tss_struct {* be within the limit. */ unsigned long io_bitmap[IO_BITMAP_LONGS +1 ]; +#endif /* CONFIG_X86_IOPORT */ /* * .. and then another 0x100 bytes for the emergency kernel stack:
@@ -288,6 +294,7 @@ DECLARE_PER_CPU_SHARED_ALIGNED(struct tss_struct, init_tss);static inline void init_tss_io (struct tss_struct *t) { +#ifdef CONFIG_X86_IOPORT int i; t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap);
@@ -298,6 +305,9 @@ static inline void init_tss_io(struct tss_struct *t)*/ for (i =0 ; i <=IO_BITMAP_LONGS ; i++) t->io_bitmap[i] = ~0UL ; +#else + t->x86_tss.io_bitmap_base =INVALID_IO_BITMAP_OFFSET ; +#endif }/*
@@ -524,11 +534,13 @@ struct thread_struct {unsigned int saved_fs;unsigned int saved_gs;#endif +#ifdef CONFIG_X86_IOPORT /* IO permissions: */ unsigned long *io_bitmap_ptr;unsigned long iopl;/* Max allowed port in the bitmap, in bytes: */ unsigned io_bitmap_max; +#endif /* CONFIG_X86_IOPORT */ /* * fpu_counter contains the number of consecutive context switches * that the FPU is used. If this is over a threshold, the lazy fpu
@@ -545,7 +557,7 @@ struct thread_struct {*/ static inline void native_set_iopl_mask (unsigned mask) { -#ifdef CONFIG_X86_32 +#if defined(CONFIG_X86_IOPORT ) && defined(CONFIG_X86_32 )unsigned int reg; asm volatile ("pushfl;"
@@ -856,6 +868,7 @@ static inline void spin_lock_prefetch(const void *x)#define STACK_TOP TASK_SIZE#define STACK_TOP_MAX STACK_TOP +#ifdef CONFIG_X86_IOPORT #define INIT_THREAD_IO .io_bitmap_ptr = NULL,/*
@@ -865,6 +878,10 @@ static inline void spin_lock_prefetch(const void *x)* be within the limit. */ #define INIT_TSS_IO .io_bitmap = { [0 ... IO_BITMAP_LONGS] = ~0 }, +#else +#define INIT_THREAD_IO +#define INIT_TSS_IO +#endif #define INIT_THREAD { \ .sp0 = sizeof(init_stack) + (long)&init_stack, \
arch/x86/include/asm/syscalls.h +3 -0
@@ -16,9 +16,12 @@#include <linux/types.h> /* Common in X86_32 and X86_64 */ + +#ifdef CONFIG_X86_IOPORT /* kernel/ioport.c */ asmlinkage long sys_ioperm (unsigned long ,unsigned long ,int );asmlinkage long sys_iopl (unsigned int ); +#endif /* CONFIG_X86_IOPORT */ /* kernel/ldt.c */ asmlinkage int sys_modify_ldt(int, void __user *, unsigned long);
arch/x86/kernel/Makefile +2 -1
@@ -20,7 +20,8 @@ CFLAGS_irq.o := -I$(src)/../include/asm/trace obj-y := process_$(BITS).o signal.o entry_$(BITS).o obj-y += traps.o irq.o irq_$(BITS).o dumpstack_$(BITS).o -obj-y += time.o ioport.o ldt.o dumpstack.o nmi.o +obj-y += time.o ldt.o dumpstack.o nmi.o +obj-$(CONFIG_X86_IOPORT) += ioport.o obj-y += setup.o x86_init.o i8259.o irqinit.o jump_label.o obj-$(CONFIG_IRQ_WORK) += irq_work.o obj-y += probe_roms.o
arch/x86/kernel/entry_64.S +6 -3
@@ -609,6 +609,11 @@ ENTRY(stub_\func) END(stub_\func) .endm + FORK_LIKE clone + FORK_LIKE fork + FORK_LIKE vfork + +#ifdef CONFIG_X86_IOPORT .macro FIXED_FRAME label,func ENTRY(\label) CFI_STARTPROC
@@ -621,10 +626,8 @@ ENTRY(\label) END(\label) .endm - FORK_LIKE clone - FORK_LIKE fork - FORK_LIKE vfork FIXED_FRAME stub_iopl, sys_iopl +#endif /* CONFIG_X86_IOPORT */ ENTRY(ptregscall_common) DEFAULT_FRAME 1 8 /* offset 8: return address */
arch/x86/kernel/process-io.h +10 -0
@@ -3,12 +3,15 @@static inline void clear_thread_io_bitmap (struct task_struct *p) { +#ifdef CONFIG_X86_IOPORT p->thread.io_bitmap_ptr =NULL ; +#endif /* CONFIG_X86_IOPORT */ }static inline int copy_io_bitmap (struct task_struct *me,struct task_struct *p) { +#ifdef CONFIG_X86_IOPORT if (unlikely (test_tsk_thread_flag (me,TIF_IO_BITMAP ))) { p->thread.io_bitmap_ptr =kmemdup (me->thread.io_bitmap_ptr,IO_BITMAP_BYTES ,GFP_KERNEL );
@@ -20,12 +23,14 @@ static inline int copy_io_bitmap(struct task_struct *me, } else { p->thread.io_bitmap_ptr =NULL ; } +#endif /* CONFIG_X86_IOPORT */ return 0 ; }static inline void exit_thread_io (struct task_struct *me) { +#ifdef CONFIG_X86_IOPORT struct thread_struct *t = &me->thread;unsigned long *bp = t->io_bitmap_ptr;
@@ -42,11 +47,13 @@ static inline void exit_thread_io(struct task_struct *me)put_cpu (); kfree(bp); } +#endif /* CONFIG_X86_IOPORT */ }static inline void switch_iopl_mask (struct thread_struct *prev,struct thread_struct *next) { +#ifdef CONFIG_X86_IOPORT /* * Restore IOPL if needed. In normal use, the flags restore * in the switch assembly will handle this. But if the kernel
@@ -55,12 +62,14 @@ static inline void switch_iopl_mask(struct thread_struct *prev,*/ if (get_kernel_rpl () &&unlikely (prev->iopl != next->iopl))set_iopl_mask (next->iopl); +#endif /* CONFIG_X86_IOPORT */ }static inline void switch_io_bitmap (struct tss_struct *tss,struct task_struct *prev_p,struct task_struct *next_p) { +#ifdef CONFIG_X86_IOPORT struct thread_struct *prev, *next; prev = &prev_p->thread; next = &next_p->thread;
@@ -78,6 +87,7 @@ static inline void switch_io_bitmap(struct tss_struct *tss,*/ memset (tss->io_bitmap,0xff , prev->io_bitmap_max); } +#endif /* CONFIG_X86_IOPORT */ }#endif /* _X86_KERNEL_PROCESS_IO_H */
arch/x86/kernel/ptrace.c +8 -0
@@ -785,7 +785,11 @@ static int ptrace_set_debugreg(struct task_struct *tsk, int n,static int ioperm_active (struct task_struct *target,const struct user_regset *regset) { +#ifdef CONFIG_X86_IOPORT return target->thread.io_bitmap_max / regset->size; +#else +return 0 ; +#endif } static int ioperm_get(struct task_struct *target,
@@ -793,12 +797,16 @@ static int ioperm_get(struct task_struct *target, unsigned int pos, unsigned int count, void *kbuf, void __user *ubuf) { +#ifdef CONFIG_X86_IOPORT if (!target->thread.io_bitmap_ptr) return -ENXIO; return user_regset_copyout(&pos, &count, &kbuf, &ubuf, target->thread.io_bitmap_ptr, 0, IO_BITMAP_BYTES); +#else +return -ENXIO ; +#endif }/*
drivers/tty/vt/vt_ioctl.c +1 -1
@@ -410,7 +410,7 @@ int vt_ioctl(struct tty_struct *tty,* * XXX: you should never use these, just call ioperm directly.. */ -#ifdef CONFIG_X86 +#ifdef CONFIG_X86_IOPORT case KDADDIO :case KDDELIO :/*
kernel/sys_ni.c +5 -0
@@ -224,3 +224,8 @@ cond_syscall(sys_seccomp);/* access BPF programs and maps */ cond_syscall (sys_bpf); + +/* userspace I/O port access */ +cond_syscall (stub_iopl); +cond_syscall (sys_iopl); +cond_syscall (sys_ioperm);
Signature · 1 line
2.1.1
Quoted 6 lines · On Sun, 2 Nov 2014 09:33:01 -0800 Josh Triplett <[email protected]> wrote:
On the vast majority of modern systems, no processes will use the userspsace IO syscalls, iopl and ioperm. Add a new config option,
CONFIG_X86_IOPORT, to support configuring them out of the kernel entirely. Most current systems do not run programs using these syscalls, soX86_IOPORTdoes not depend on EXPERT, though it does still default to y.
This isn't unreasonable but there are drivers with userspace helpers that use iopl/ioperm type functionality where you should be doing a SELECT of X86_IOPORT. The one that comes to mind is the uvesa driver. From a quick scan it may these days be the only mainstream one that needs the select adding.
Some X servers for legacy cards still use io port access. There are also a couple of other highly non-obvious userspace users that hang on for some systems - eg some older servers DMI and error records can only by read via a real mode BIOS call so management tools have no choice but to go the lrmi/io path.
Still makes sense IMHO.
From a code perspective however you could define IO_BITMAP_LONGS to 0, add an IO_BITMAP_SIZE (defined as LONGS + 1 or 0) and as far as I can see gcc would then optimise out a lot of the code you are ifdeffing
Alan
Quoted 6 lines · On Sun, 2 Nov 2014 09:33:01 -0800 Josh Triplett <[email protected]> wrote:
On the vast majority of modern systems, no processes will use the userspsace IO syscalls, iopl and ioperm. Add a new config option,
CONFIG_X86_IOPORT, to support configuring them out of the kernel entirely. Most current systems do not run programs using these syscalls, soX86_IOPORTdoes not depend on EXPERT, though it does still default to y.
This isn't unreasonable but there are drivers with userspace helpers that use iopl/ioperm type functionality where you should be doing a SELECT of X86_IOPORT. The one that comes to mind is the uvesa driver. From a quick scan it may these days be the only mainstream one that needs the select adding.
Some X servers for legacy cards still use io port access. There are also a couple of other highly non-obvious userspace users that hang on for some systems - eg some older servers DMI and error records can only by read via a real mode BIOS call so management tools have no choice but to go the lrmi/io path.
Still makes sense IMHO.
Quoted 1 line
From a code perspective however you could define
IO_BITMAP_LONGSto 0,
add an IO_BITMAP_SIZE (defined as LONGS + 1 or 0) and as far as I can see gcc would then optimise out a lot of the code you are ifdeffing
Alan
Quoted 10 lines · One Thousand Gnomes
Quoted 6 lines · On Sun, 2 Nov 2014 09:33:01 -0800 Josh Triplett <[email protected]> wrote:
On the vast majority of modern systems, no processes will use the userspsace IO syscalls, iopl and ioperm. Add a new config option,
CONFIG_X86_IOPORT, to support configuring them out of the kernel entirely. Most current systems do not run programs using these syscalls, soX86_IOPORTdoes not depend on EXPERT, though it does still default to y.
Quoted 5 lines
This isn't unreasonable but there are drivers with userspace helpers that use iopl/ioperm type functionality where you should be doing a SELECT of
X86_IOPORT. The one that comes to mind is the uvesa driver. From a quick scan it may these days be the only mainstream one that needs the select adding.
Should kernel drivers really express dependencies that only their (current instances of) corresponding userspace components need? Something seems wrong about that.
Quoted 1 line
Some X servers for legacy cards still use io port access.
Sure, X servers using UMS rather than KMS seem like a common reason to need this.
Quoted 5 lines
There are also
a couple of other highly non-obvious userspace users that hang on for some systems - eg some older servers DMI and error records can only by read via a real mode BIOS call so management tools have no choice but to go the lrmi/io path.
As with any userspace interface, some callers may potentially still exist. And this still has "default y", too, to avoid user surprises.
Quoted 5 lines
Still makes sense IMHO.
From a code perspective however you could define
IO_BITMAP_LONGSto 0, add anIO_BITMAP_SIZE(defined as LONGS + 1 or 0) and as far as I can see gcc would then optimise out a lot of the code you are ifdeffing
IO_BITMAP_LONGS already gets defined to (0/sizeof(long)). And as far as I can tell, that would only work for init_tss_io, not anything else. Even then, that would only work with a zero-size array left around in tss_struct, which doesn't seem appropriate. The remaining ifdefs wrap code that GCC could not constant-fold away, and making that code constant-foldable seems significantly more invasive than the ifdefs.
- Josh Triplett
Quoted 9 lines
Quoted 5 lines
This isn't unreasonable but there are drivers with userspace helpers that use iopl/ioperm type functionality where you should be doing a SELECT of
X86_IOPORT. The one that comes to mind is the uvesa driver. From a quick scan it may these days be the only mainstream one that needs the select adding.Should kernel drivers really express dependencies that only their (current instances of) corresponding userspace components need? Something seems wrong about that.
uvesafb will always need X86_IOPORT. It's kind of implicit in the design. I'm not suggesting that fbdev should select X86_IOPORT but in the uvesafb case at least it's completely useless to have one and not the other.
Quoted 6 lines
IO_BITMAP_LONGSalready gets defined to (0/sizeof(long)). And as far as I can tell, that would only work forinit_tss_io, not anything else. Even then, that would only work with a zero-size array left around intss_struct, which doesn't seem appropriate. The remaining ifdefs wrap code that GCC could not constant-fold away, and making that code constant-foldable seems significantly more invasive than the ifdefs.
OK
Quoted 9 lines
Quoted 5 lines
This isn't unreasonable but there are drivers with userspace helpers that use iopl/ioperm type functionality where you should be doing a SELECT of
X86_IOPORT. The one that comes to mind is the uvesa driver. From a quick scan it may these days be the only mainstream one that needs the select adding.Should kernel drivers really express dependencies that only their (current instances of) corresponding userspace components need? Something seems wrong about that.
uvesafb will always need X86_IOPORT. It's kind of implicit in the design. I'm not suggesting that fbdev should select X86_IOPORT but in the uvesafb case at least it's completely useless to have one and not the other.
Quoted 6 lines
IO_BITMAP_LONGSalready gets defined to (0/sizeof(long)). And as far as I can tell, that would only work forinit_tss_io, not anything else. Even then, that would only work with a zero-size array left around intss_struct, which doesn't seem appropriate. The remaining ifdefs wrap code that GCC could not constant-fold away, and making that code constant-foldable seems significantly more invasive than the ifdefs.
OK
Quoted 6 lines · One Thousand Gnomes
Quoted 6 lines
Quoted 5 lines
This isn't unreasonable but there are drivers with userspace helpers that use iopl/ioperm type functionality where you should be doing a SELECT of
X86_IOPORT. The one that comes to mind is the uvesa driver. From a quick scan it may these days be the only mainstream one that needs the select adding.
Quoted 7 lines
Quoted 3 lines
Should kernel drivers really express dependencies that only their (current instances of) corresponding userspace components need? Something seems wrong about that.
uvesafb will always need
X86_IOPORT. It's kind of implicit in the design. I'm not suggesting that fbdev should selectX86_IOPORTbut in the uvesafb case at least it's completely useless to have one and not the other.
OK, fair enough. Do you want the patch series respun to add that select in patch 10/10, or would you consider it sufficient to add that in a followup patch, since the kernel will build and boot either way (so it won't break bisection)?
Related to that: Is it intentional that FB_UVESA doesn't depend on X86, even though FB_VESA does? Does v86d run on non-x86 hardware via emulation? If so, should FB_UVESA have "select X86_IOPORT if X86"?
- Josh Triplett
Quoted 6 lines · One Thousand Gnomes
Quoted 6 lines
Quoted 5 lines
This isn't unreasonable but there are drivers with userspace helpers that use iopl/ioperm type functionality where you should be doing a SELECT of
X86_IOPORT. The one that comes to mind is the uvesa driver. From a quick scan it may these days be the only mainstream one that needs the select adding.
Quoted 7 lines
Quoted 3 lines
Should kernel drivers really express dependencies that only their (current instances of) corresponding userspace components need? Something seems wrong about that.
uvesafb will always need
X86_IOPORT. It's kind of implicit in the design. I'm not suggesting that fbdev should selectX86_IOPORTbut in the uvesafb case at least it's completely useless to have one and not the other.
Are there any users of uvesafb at all? Last time I changed that driver, I tried to test it, and I was unable to find a copy of the userspace helper.
--Andy
Quoted 10 lines
Quoted 7 lines
IO_BITMAP_LONGSalready gets defined to (0/sizeof(long)). And as far as I can tell, that would only work forinit_tss_io, not anything else. Even then, that would only work with a zero-size array left around intss_struct, which doesn't seem appropriate. The remaining ifdefs wrap code that GCC could not constant-fold away, and making that code constant-foldable seems significantly more invasive than the ifdefs.OK