linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86: ioport_{32|64}.c unification
@ 2008-01-05 23:59 Miguel Botón
  2008-01-06  0:47 ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Miguel Botón @ 2008-01-05 23:59 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, tglx

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 <mboton@gmail.com>

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: ioport_{32|64}.c unification
  2008-01-05 23:59 [PATCH] x86: ioport_{32|64}.c unification Miguel Botón
@ 2008-01-06  0:47 ` Arnd Bergmann
  2008-01-06  1:00   ` Miguel Botón
  2008-01-06  1:20   ` Miguel Botón
  0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2008-01-06  0:47 UTC (permalink / raw)
  To: Miguel Botón; +Cc: linux-kernel, Ingo Molnar, tglx

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 <><

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: ioport_{32|64}.c unification
  2008-01-06  0:47 ` Arnd Bergmann
@ 2008-01-06  1:00   ` Miguel Botón
  2008-01-06  1:20   ` Miguel Botón
  1 sibling, 0 replies; 6+ messages in thread
From: Miguel Botón @ 2008-01-06  1:00 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Miguel Botón, linux-kernel, Ingo Molnar, tglx

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: ioport_{32|64}.c unification
  2008-01-06  0:47 ` Arnd Bergmann
  2008-01-06  1:00   ` Miguel Botón
@ 2008-01-06  1:20   ` Miguel Botón
  2008-01-06  5:55     ` Valdis.Kletnieks
  1 sibling, 1 reply; 6+ messages in thread
From: Miguel Botón @ 2008-01-06  1:20 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Miguel Botón, linux-kernel, Ingo Molnar, tglx

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: ioport_{32|64}.c unification
  2008-01-06  1:20   ` Miguel Botón
@ 2008-01-06  5:55     ` Valdis.Kletnieks
  2008-01-06 15:30       ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Valdis.Kletnieks @ 2008-01-06  5:55 UTC (permalink / raw)
  To: Miguel Botón; +Cc: Arnd Bergmann, linux-kernel, Ingo Molnar, tglx

[-- Attachment #1: Type: text/plain, Size: 971 bytes --]

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...

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: ioport_{32|64}.c unification
  2008-01-06  5:55     ` Valdis.Kletnieks
@ 2008-01-06 15:30       ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2008-01-06 15:30 UTC (permalink / raw)
  To: Valdis.Kletnieks; +Cc: Miguel Botón, linux-kernel, Ingo Molnar, tglx

On Sunday 06 January 2008, Valdis.Kletnieks@vt.edu 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 <><

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-01-06 15:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-01-05 23:59 [PATCH] x86: ioport_{32|64}.c unification Miguel Botón
2008-01-06  0:47 ` Arnd Bergmann
2008-01-06  1:00   ` Miguel Botón
2008-01-06  1:20   ` Miguel Botón
2008-01-06  5:55     ` Valdis.Kletnieks
2008-01-06 15:30       ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).