2008-01-06 00:07:26

by Miguel Botón

[permalink] [raw]
Subject: [PATCH] x86: ioport_{32|64}.c unification

ioport_{32|64}.c unification.

This patch unifies the code from the ioport_32.c and ioport_64.c files.

Tested and working fine with i386 and x86_64 kernels.

Signed-off-by: Miguel Botón <[email protected]>

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 0903bbf..5ed68b4 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -10,7 +10,7 @@ CFLAGS_vsyscall_64.o := $(PROFILING) -g0

obj-y := process_$(BITS).o signal_$(BITS).o entry_$(BITS).o
obj-y += traps_$(BITS).o irq_$(BITS).o
-obj-y += time_$(BITS).o ioport_$(BITS).o ldt.o
+obj-y += time_$(BITS).o ioport.o ldt.o
obj-y += setup_$(BITS).o i8259_$(BITS).o
obj-$(CONFIG_X86_32) += sys_i386_32.o i386_ksyms_32.o
obj-$(CONFIG_X86_64) += sys_x86_64.o x8664_ksyms_64.o
diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
new file mode 100644
index 0000000..be3d521
--- /dev/null
+++ b/arch/x86/kernel/ioport.c
@@ -0,0 +1,138 @@
+/*
+ * This contains the io-permission bitmap code - written by obz, with changes
+ * by Linus. 32/64 bits code unification by Miguel Botón.
+ */
+
+#include <linux/sched.h>
+#include <linux/kernel.h>
+#include <linux/capability.h>
+#include <linux/errno.h>
+#include <linux/types.h>
+#include <linux/ioport.h>
+#include <linux/smp.h>
+#include <linux/stddef.h>
+#include <linux/slab.h>
+#include <linux/thread_info.h>
+#include <linux/syscalls.h>
+
+/* Set EXTENT bits starting at BASE in BITMAP to value TURN_ON. */
+static void set_bitmap(unsigned long *bitmap, unsigned int base,
+ unsigned int extent, int new_value)
+{
+ unsigned int i;
+
+ for (i = base; i < base + extent; i++) {
+ if (new_value)
+ __set_bit(i, bitmap);
+ else
+ __clear_bit(i, bitmap);
+ }
+}
+
+/*
+ * this changes the io permissions bitmap in the current task.
+ */
+asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
+{
+ struct thread_struct *t = &current->thread;
+ struct tss_struct *tss;
+ unsigned int i, max_long, bytes, bytes_updated;
+
+ if ((from + num <= from) || (from + num > IO_BITMAP_BITS))
+ return -EINVAL;
+ if (turn_on && !capable(CAP_SYS_RAWIO))
+ return -EPERM;
+
+ /*
+ * If it's the first ioperm() call in this thread's lifetime, set the
+ * IO bitmap up. ioperm() is much less timing critical than clone(),
+ * this is why we delay this operation until now:
+ */
+ if (!t->io_bitmap_ptr) {
+ unsigned long *bitmap = kmalloc(IO_BITMAP_BYTES, GFP_KERNEL);
+
+ if (!bitmap)
+ return -ENOMEM;
+
+ memset(bitmap, 0xff, IO_BITMAP_BYTES);
+ t->io_bitmap_ptr = bitmap;
+ set_thread_flag(TIF_IO_BITMAP);
+ }
+
+ /*
+ * do it in the per-thread copy and in the TSS ...
+ *
+ * Disable preemption via get_cpu() - we must not switch away
+ * because the ->io_bitmap_max value must match the bitmap
+ * contents:
+ */
+ tss = &per_cpu(init_tss, get_cpu());
+
+ set_bitmap(t->io_bitmap_ptr, from, num, !turn_on);
+
+ /*
+ * Search for a (possibly new) maximum. This is simple and stupid,
+ * to keep it obviously correct:
+ */
+ max_long = 0;
+ for (i = 0; i < IO_BITMAP_LONGS; i++)
+ if (t->io_bitmap_ptr[i] != ~0UL)
+ max_long = i;
+
+ bytes = (max_long + 1) * sizeof(unsigned long);
+ bytes_updated = max(bytes, t->io_bitmap_max);
+
+ t->io_bitmap_max = bytes;
+
+#ifdef CONFIG_X86_32
+ /*
+ * Sets the lazy trigger so that the next I/O operation will
+ * reload the correct bitmap.
+ * Reset the owner so that a process switch will not set
+ * tss->io_bitmap_base to IO_BITMAP_OFFSET.
+ */
+ tss->x86_tss.io_bitmap_base = INVALID_IO_BITMAP_OFFSET_LAZY;
+ tss->io_bitmap_owner = NULL;
+#else
+ /* Update the TSS: */
+ memcpy(tss->io_bitmap, t->io_bitmap_ptr, bytes_updated);
+#endif
+
+ put_cpu();
+
+ return 0;
+}
+
+/*
+ * sys_iopl has to be used when you want to access the IO ports
+ * beyond the 0x3ff range: to get the full 65536 ports bitmapped
+ * you'd need 8kB of bitmaps/process, which is a bit excessive.
+ *
+ * Here we just change the flags value on the stack: we allow
+ * only the super-user to do it. This depends on the stack-layout
+ * on system-call entry - see also fork() and the signal handling
+ * code.
+ */
+#ifdef CONFIG_X86_32
+asmlinkage long sys_iopl(unsigned long regsp)
+#else
+asmlinkage long sys_iopl(unsigned int level, struct pt_regs *regs)
+#endif
+{
+#ifdef CONFIG_X86_32
+ volatile struct pt_regs *regs = (struct pt_regs *)&regsp;
+ unsigned int level = regs->bx;
+#endif
+ unsigned int old = (regs->flags >> 12) & 3;
+
+ if (level > 3)
+ return -EINVAL;
+ /* Trying to gain more privileges? */
+ if (level > old) {
+ if (!capable(CAP_SYS_RAWIO))
+ return -EPERM;
+ }
+ regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) | (level << 12);;
+
+ return 0;
+}
diff --git a/arch/x86/kernel/ioport_32.c b/arch/x86/kernel/ioport_32.c
deleted file mode 100644
index 9295e01..0000000
--- a/arch/x86/kernel/ioport_32.c
+++ /dev/null
@@ -1,129 +0,0 @@
-/*
- * This contains the io-permission bitmap code - written by obz, with changes
- * by Linus.
- */
-
-#include <linux/sched.h>
-#include <linux/kernel.h>
-#include <linux/capability.h>
-#include <linux/errno.h>
-#include <linux/types.h>
-#include <linux/ioport.h>
-#include <linux/smp.h>
-#include <linux/stddef.h>
-#include <linux/slab.h>
-#include <linux/thread_info.h>
-#include <linux/syscalls.h>
-
-/* Set EXTENT bits starting at BASE in BITMAP to value TURN_ON. */
-static void set_bitmap(unsigned long *bitmap, unsigned int base,
- unsigned int extent, int new_value)
-{
- unsigned int i;
-
- for (i = base; i < base + extent; i++) {
- if (new_value)
- __set_bit(i, bitmap);
- else
- __clear_bit(i, bitmap);
- }
-}
-
-/*
- * this changes the io permissions bitmap in the current task.
- */
-asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
-{
- struct thread_struct * t = &current->thread;
- struct tss_struct * tss;
- unsigned long i, max_long;
-
- if ((from + num <= from) || (from + num > IO_BITMAP_BITS))
- return -EINVAL;
- if (turn_on && !capable(CAP_SYS_RAWIO))
- return -EPERM;
-
- /*
- * If it's the first ioperm() call in this thread's lifetime, set the
- * IO bitmap up. ioperm() is much less timing critical than clone(),
- * this is why we delay this operation until now:
- */
- if (!t->io_bitmap_ptr) {
- unsigned long *bitmap = kmalloc(IO_BITMAP_BYTES, GFP_KERNEL);
-
- if (!bitmap)
- return -ENOMEM;
-
- memset(bitmap, 0xff, IO_BITMAP_BYTES);
- t->io_bitmap_ptr = bitmap;
- set_thread_flag(TIF_IO_BITMAP);
- }
-
- /*
- * do it in the per-thread copy and in the TSS ...
- *
- * Disable preemption via get_cpu() - we must not switch away
- * because the ->io_bitmap_max value must match the bitmap
- * contents:
- */
- tss = &per_cpu(init_tss, get_cpu());
-
- set_bitmap(t->io_bitmap_ptr, from, num, !turn_on);
-
- /*
- * Search for a (possibly new) maximum. This is simple and stupid,
- * to keep it obviously correct:
- */
- max_long = 0;
- for (i = 0; i < IO_BITMAP_LONGS; i++)
- if (t->io_bitmap_ptr[i] != ~0UL)
- max_long = i;
-
- t->io_bitmap_max = (max_long + 1) * sizeof(unsigned long);
-
- /*
- * Sets the lazy trigger so that the next I/O operation will
- * reload the correct bitmap.
- * Reset the owner so that a process switch will not set
- * tss->io_bitmap_base to IO_BITMAP_OFFSET.
- */
- tss->x86_tss.io_bitmap_base = INVALID_IO_BITMAP_OFFSET_LAZY;
- tss->io_bitmap_owner = NULL;
-
- put_cpu();
-
- return 0;
-}
-
-/*
- * sys_iopl has to be used when you want to access the IO ports
- * beyond the 0x3ff range: to get the full 65536 ports bitmapped
- * you'd need 8kB of bitmaps/process, which is a bit excessive.
- *
- * Here we just change the flags value on the stack: we allow
- * only the super-user to do it. This depends on the stack-layout
- * on system-call entry - see also fork() and the signal handling
- * code.
- */
-
-asmlinkage long sys_iopl(unsigned long regsp)
-{
- volatile struct pt_regs *regs = (struct pt_regs *)&regsp;
- unsigned int level = regs->bx;
- unsigned int old = (regs->flags >> 12) & 3;
- struct thread_struct *t = &current->thread;
-
- if (level > 3)
- return -EINVAL;
- /* Trying to gain more privileges? */
- if (level > old) {
- if (!capable(CAP_SYS_RAWIO))
- return -EPERM;
- }
-
- t->iopl = level << 12;
- regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) | t->iopl;
- set_iopl_mask(t->iopl);
-
- return 0;
-}
diff --git a/arch/x86/kernel/ioport_64.c b/arch/x86/kernel/ioport_64.c
deleted file mode 100644
index ff7514b..0000000
--- a/arch/x86/kernel/ioport_64.c
+++ /dev/null
@@ -1,117 +0,0 @@
-/*
- * This contains the io-permission bitmap code - written by obz, with changes
- * by Linus.
- */
-
-#include <linux/sched.h>
-#include <linux/kernel.h>
-#include <linux/capability.h>
-#include <linux/errno.h>
-#include <linux/types.h>
-#include <linux/ioport.h>
-#include <linux/smp.h>
-#include <linux/stddef.h>
-#include <linux/slab.h>
-#include <linux/thread_info.h>
-#include <linux/syscalls.h>
-
-/* Set EXTENT bits starting at BASE in BITMAP to value TURN_ON. */
-static void set_bitmap(unsigned long *bitmap, unsigned int base, unsigned int extent, int new_value)
-{
- int i;
- if (new_value)
- for (i = base; i < base + extent; i++)
- __set_bit(i, bitmap);
- else
- for (i = base; i < base + extent; i++)
- clear_bit(i, bitmap);
-}
-
-/*
- * this changes the io permissions bitmap in the current task.
- */
-asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
-{
- unsigned int i, max_long, bytes, bytes_updated;
- struct thread_struct * t = &current->thread;
- struct tss_struct * tss;
- unsigned long *bitmap;
-
- if ((from + num <= from) || (from + num > IO_BITMAP_BITS))
- return -EINVAL;
- if (turn_on && !capable(CAP_SYS_RAWIO))
- return -EPERM;
-
- /*
- * If it's the first ioperm() call in this thread's lifetime, set the
- * IO bitmap up. ioperm() is much less timing critical than clone(),
- * this is why we delay this operation until now:
- */
- if (!t->io_bitmap_ptr) {
- bitmap = kmalloc(IO_BITMAP_BYTES, GFP_KERNEL);
- if (!bitmap)
- return -ENOMEM;
-
- memset(bitmap, 0xff, IO_BITMAP_BYTES);
- t->io_bitmap_ptr = bitmap;
- set_thread_flag(TIF_IO_BITMAP);
- }
-
- /*
- * do it in the per-thread copy and in the TSS ...
- *
- * Disable preemption via get_cpu() - we must not switch away
- * because the ->io_bitmap_max value must match the bitmap
- * contents:
- */
- tss = &per_cpu(init_tss, get_cpu());
-
- set_bitmap(t->io_bitmap_ptr, from, num, !turn_on);
-
- /*
- * Search for a (possibly new) maximum. This is simple and stupid,
- * to keep it obviously correct:
- */
- max_long = 0;
- for (i = 0; i < IO_BITMAP_LONGS; i++)
- if (t->io_bitmap_ptr[i] != ~0UL)
- max_long = i;
-
- bytes = (max_long + 1) * sizeof(long);
- bytes_updated = max(bytes, t->io_bitmap_max);
-
- t->io_bitmap_max = bytes;
-
- /* Update the TSS: */
- memcpy(tss->io_bitmap, t->io_bitmap_ptr, bytes_updated);
-
- put_cpu();
-
- return 0;
-}
-
-/*
- * sys_iopl has to be used when you want to access the IO ports
- * beyond the 0x3ff range: to get the full 65536 ports bitmapped
- * you'd need 8kB of bitmaps/process, which is a bit excessive.
- *
- * Here we just change the flags value on the stack: we allow
- * only the super-user to do it. This depends on the stack-layout
- * on system-call entry - see also fork() and the signal handling
- * code.
- */
-
-asmlinkage long sys_iopl(unsigned int level, struct pt_regs *regs)
-{
- unsigned int old = (regs->flags >> 12) & 3;
-
- if (level > 3)
- return -EINVAL;
- /* Trying to gain more privileges? */
- if (level > old) {
- if (!capable(CAP_SYS_RAWIO))
- return -EPERM;
- }
- regs->flags = (regs->flags &~ X86_EFLAGS_IOPL) | (level << 12);
- return 0;
-}


--
Miguel Botón


2008-01-06 00:48:22

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] x86: ioport_{32|64}.c unification

On Sunday 06 January 2008, Miguel Botón wrote:
> +#ifdef CONFIG_X86_32
> +asmlinkage long sys_iopl(unsigned long regsp)
> +#else
> +asmlinkage long sys_iopl(unsigned int level, struct pt_regs *regs)
> +#endif
> +{
> +#ifdef CONFIG_X86_32
> +       volatile struct pt_regs *regs = (struct pt_regs *)&regsp;
> +       unsigned int level = regs->bx;
> +#endif

This #ifdef overload could probably be avoided if you just move
the body of this function into an extra place and do

static int do_iopl(unsigned int level, unsigned long *flags)
{
unsigned int old = (*flags >> 12) & 3;
...
*flags = (*flags & ~X86_EFLAGS_IOPL) | (level << 12);;
}

#ifdef CONFIG_X86_32
asmlinkage long sys_iopl(unsigned long regsp)
{
/* why is this volatle anyway? */
volatile struct pt_regs *regs = (struct pt_regs *)&regsp;
unsigned int level = regs->bx;
return do_iopl(regs->bx, &regs->flags);
}
#else
asmlinkage long sys_iopl(unsigned int level, struct pt_regs *regs)
{
return do_iopl(level, &regs->flags);
}
#endif

Arnd <><

2008-01-06 01:01:09

by Miguel Botón

[permalink] [raw]
Subject: Re: [PATCH] x86: ioport_{32|64}.c unification

On Sunday 06 January 2008 01:47:47 Arnd Bergmann wrote:
> This #ifdef overload could probably be avoided if you just move
> the body of this function into an extra place and do
>
> static int do_iopl(unsigned int level, unsigned long *flags)
> {
> unsigned int old = (*flags >> 12) & 3;
> ...
> *flags = (*flags & ~X86_EFLAGS_IOPL) | (level << 12);;
> }
>
> #ifdef CONFIG_X86_32
> asmlinkage long sys_iopl(unsigned long regsp)
> {
> /* why is this volatle anyway? */
> volatile struct pt_regs *regs = (struct pt_regs *)&regsp;
> unsigned int level = regs->bx;
> return do_iopl(regs->bx, &regs->flags);
> }
> #else
> asmlinkage long sys_iopl(unsigned int level, struct pt_regs *regs)
> {
> return do_iopl(level, &regs->flags);
> }
> #endif
>
> Arnd <><

I agree. I'll send the a proper patch soon.

--
Miguel Botón

2008-01-06 01:21:01

by Miguel Botón

[permalink] [raw]
Subject: Re: [PATCH] x86: ioport_{32|64}.c unification

I just realize that in some cases 'regs->flags' is a long instead of an
unsigned long so... this would be a proper solution?

static long do_iopl(unsigned int level, long *flags)
{
...
}

#ifdef CONFIG_X86_32
asmlinkage long sys_iopl(unsigned long regsp)
{
volatile struct pt_regs *regs = (struct pt_regs *)&regsp;
unsigned int level = regs->bx;
#else
asmlinkage long sys_iopl(unsigned int level, struct pt_regs *regs)
{
#endif
return do_iopl(level, (long *)&regs->flags);
}

Or maybe would be better if 'do_iopl' prototype is:

static long do_iopl(unsigned int level, struct pt_regs *regs);

--
Miguel Botón

2008-01-06 05:56:29

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [PATCH] x86: ioport_{32|64}.c unification

On Sun, 06 Jan 2008 02:20:45 +0100, Miguel =?utf-8?q?Bot=C3=B3n?= said:
> #ifdef CONFIG_X86_32
> asmlinkage long sys_iopl(unsigned long regsp)
> {
> volatile struct pt_regs *regs = (struct pt_regs *)&regsp;
> unsigned int level = regs->bx;
> #else
> asmlinkage long sys_iopl(unsigned int level, struct pt_regs *regs)
> {
> #endif
> return do_iopl(level, (long *)&regs->flags);
> }

That looks like an unholy marriage between the preprocessor, a function
definition, and a Duff Device. :)

Do it like this:
#ifdef CONFIG_X86_32
asmlinkage sys_iopl( prototype_needed)
{
/* the whole thing */
}
#else
asmlinkage long sys_iopl(alternate_prototype)
{
/* the entire body */
}
#endif

You'll still need some ad-croc ifdef-ery at the call site to make it use
the correct prototype (unless you can come up with some clever way of making
the two use the same prototype, possibly by passing a dummy argument on
some architectures...


Attachments:
(No filename) (226.00 B)

2008-01-06 15:31:17

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] x86: ioport_{32|64}.c unification

On Sunday 06 January 2008, [email protected] wrote:

> You'll still need some ad-croc ifdef-ery at the call site to make it use
> the correct prototype (unless you can come up with some clever way of making
> the two use the same prototype, possibly by passing a dummy argument on
> some architectures...

The call site is the sys_call_table, and user space already has different
calling conventions here, which is the whole reason for this mess.

Arnd <><