Skip to contentSkip to conversation

x86: ioport_{32|64}.c unification

PATCH In review Miguel Botón 3 participants 6 messages Jan 6, 2008 x86

lore
Share & tools
lore

Miguel Botón

author
#0 lore raw

PATCH 1/1 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]>
arch/x86/kernel/Makefile +1 -1
index 0903bbf..5ed68b4 100644
@@ -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
arch/x86/kernel/ioport.c new file +138 -0

138 lines not on this page · raw message

arch/x86/kernel/ioport_32.c deleted +0 -129

129 lines not on this page · raw message

arch/x86/kernel/ioport_64.c deleted +0 -117

117 lines not on this page · raw message

Signature · 1 line
	Miguel Botón
#1 lore raw

arch/x86/kernel/ioport.c line 125

Quoted 10 lines · Miguel Botón
arch/x86/kernel/ioport.c new file +10 -0
+#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 <><
author
#2 lore raw
Quoted 23 lines · 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);
}
Quoted 3 lines

#endif

	Arnd <><

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

Signature · 1 line
	Miguel Botón
author
#3 lore raw
In reply to Arnd Bergmann

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

Signature · 1 line
	Miguel Botón
#4 lore raw
Quoted 11 lines · 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...

#5 lore raw
Quoted 4 lines · 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 <><