linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] x86: slightly unify ret_from_fork
@ 2010-02-09 15:56 Ian Campbell
  2010-02-09 15:56 ` [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit Ian Campbell
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Ian Campbell @ 2010-02-09 15:56 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ian Campbell, x86

I came across some minor differences between the 32 and 64 bit
implementations of ret_from_fork. Although I don't expect this is code
which can realistically be completely unified there seemed to be some
value in making the two look a bit more similar.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: x86@kernel.org

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

* [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.
  2010-02-09 15:56 [PATCH 0/3] x86: slightly unify ret_from_fork Ian Campbell
@ 2010-02-09 15:56 ` Ian Campbell
  2010-02-09 15:56 ` [PATCH 2/3] x86: make 64 bit ret_from_fork a little more similar to 32 bit Ian Campbell
  2010-02-09 15:56 ` [PATCH 3/3] x86: ret_from_fork: use symbolic contants for bits in EFLAGS Ian Campbell
  2 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2010-02-09 15:56 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ian Campbell, x86

For no reason that I can determine 64 bit x86 saves the current eflags
in cpu_init purely for use in ret_from_fork. The equivalent 32 bit
code simply hard codes 0x0202 as the new EFLAGS which seems safer than
relying on a potentially arbitrary EFLAGS saved during cpu_init.

Original i386 changeset
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=47a5c6fa0e204a2b63309c648bb2fde36836c826
Original x86_64 changset
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=658fdbef66e5e9be79b457edc2cbbb3add840aa9

The only comment in the later indicates that it is following the
former, but not why it differs in this way.

This change makes 64 bit use the same mechanism to setup the initial
EFLAGS on fork. Note that 64 bit resets EFLAGS before calling
schedule_tail() as opposed to 32 bit which calls schedule_tail()
first. Therefore the correct value for EFLAGS has opposite IF
bit. This will be fixed in a subsequent patch.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: x86@kernel.org
---
 arch/x86/kernel/cpu/common.c |    4 ----
 arch/x86/kernel/entry_64.S   |    2 +-
 2 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 4868e4a..ab766b2 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1040,8 +1040,6 @@ void syscall_init(void)
 	       X86_EFLAGS_TF|X86_EFLAGS_DF|X86_EFLAGS_IF|X86_EFLAGS_IOPL);
 }
 
-unsigned long kernel_eflags;
-
 /*
  * Copies of the original ist values from the tss are only accessed during
  * debugging, no special alignment required.
@@ -1189,8 +1187,6 @@ void __cpuinit cpu_init(void)
 
 	fpu_init();
 
-	raw_local_save_flags(kernel_eflags);
-
 	if (is_uv_system())
 		uv_cpu_init();
 }
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 0697ff1..18e4c59 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -398,7 +398,7 @@ ENTRY(ret_from_fork)
 
 	LOCK ; btr $TIF_FORK,TI_flags(%r8)
 
-	push kernel_eflags(%rip)
+	push $0x0002
 	CFI_ADJUST_CFA_OFFSET 8
 	popf					# reset kernel eflags
 	CFI_ADJUST_CFA_OFFSET -8
-- 
1.5.6.5


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

* [PATCH 2/3] x86: make 64 bit ret_from_fork a little more similar to 32 bit
  2010-02-09 15:56 [PATCH 0/3] x86: slightly unify ret_from_fork Ian Campbell
  2010-02-09 15:56 ` [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit Ian Campbell
@ 2010-02-09 15:56 ` Ian Campbell
  2010-02-09 15:56 ` [PATCH 3/3] x86: ret_from_fork: use symbolic contants for bits in EFLAGS Ian Campbell
  2 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2010-02-09 15:56 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ian Campbell, x86

The 64 bit version resets EFLAGS before calling schedule_tail() and
therefore leaves EFLAGS.IF clear. 32 bit resets EFLAGS after calling
schedule_tail() and therefore leaves EFLAGS.IF set. I don't think
there is any practical difference between the two approaches since
interrupts are actually reenabled within schedule_tail
(schedule_tail->finish_task_switch->finish_lock_switch->raw_spin_unlock_irq->...->local_irq_enable)
so arbitrarily pick the 32 bit version and make 64 bit look like that.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: x86@kernel.org
---
 arch/x86/kernel/entry_64.S |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 18e4c59..b771c2f 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -398,13 +398,13 @@ ENTRY(ret_from_fork)
 
 	LOCK ; btr $TIF_FORK,TI_flags(%r8)
 
-	push $0x0002
+	call schedule_tail			# rdi: 'prev' task parameter
+
+	push $0x0202
 	CFI_ADJUST_CFA_OFFSET 8
 	popf					# reset kernel eflags
 	CFI_ADJUST_CFA_OFFSET -8
 
-	call schedule_tail			# rdi: 'prev' task parameter
-
 	GET_THREAD_INFO(%rcx)
 
 	RESTORE_REST
-- 
1.5.6.5


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

* [PATCH 3/3] x86: ret_from_fork: use symbolic contants for bits in EFLAGS
  2010-02-09 15:56 [PATCH 0/3] x86: slightly unify ret_from_fork Ian Campbell
  2010-02-09 15:56 ` [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit Ian Campbell
  2010-02-09 15:56 ` [PATCH 2/3] x86: make 64 bit ret_from_fork a little more similar to 32 bit Ian Campbell
@ 2010-02-09 15:56 ` Ian Campbell
  2 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2010-02-09 15:56 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ian Campbell, x86

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: x86@kernel.org
---
 arch/x86/kernel/entry_32.S |    2 +-
 arch/x86/kernel/entry_64.S |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index 44a8e0d..02a5a18 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -325,7 +325,7 @@ ENTRY(ret_from_fork)
 	GET_THREAD_INFO(%ebp)
 	popl %eax
 	CFI_ADJUST_CFA_OFFSET -4
-	pushl $0x0202			# Reset kernel eflags
+	pushl $(X86_EFLAGS_IF|0x2)	# Reset kernel eflags
 	CFI_ADJUST_CFA_OFFSET 4
 	popfl
 	CFI_ADJUST_CFA_OFFSET -4
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index b771c2f..f5ceffa 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -400,7 +400,7 @@ ENTRY(ret_from_fork)
 
 	call schedule_tail			# rdi: 'prev' task parameter
 
-	push $0x0202
+	push $(X86_EFLAGS_IF|0x2)
 	CFI_ADJUST_CFA_OFFSET 8
 	popf					# reset kernel eflags
 	CFI_ADJUST_CFA_OFFSET -8
-- 
1.5.6.5


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

* [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.
  2011-07-25  9:58 [PATCH 0/3] minor cleanups to EFLAGS initialisation in ret_from_fork Ian Campbell
@ 2011-07-25 10:03 ` Ian Campbell
  0 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2011-07-25 10:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: Pekka Enberg, Cyrill Gorcunov, H. Peter Anvin, Andi Kleen,
	Ian Campbell, x86

On 64 bit x86 we save the current eflags in cpu_init for use in ret_from_fork.
Strictly speaking reserved bits in EFLAGS should be read as written but in
practise it is unlikely that EFLAGS could ever be extended in this way and the
kernel alread clears any undefined flags early on

The equivalent 32 bit code simply hard codes 0x0202 as the new EFLAGS (fixed to
use symbolic constants in a subsequent patch).

This change makes 64 bit use the same mechanism to setup the initial EFLAGS on
fork. Note that 64 bit resets EFLAGS before calling schedule_tail() as opposed
to 32 bit which calls schedule_tail() first. Therefore the correct value for
EFLAGS has opposite IF bit. This will be fixed in a subsequent patch.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: x86@kernel.org
---
 arch/x86/include/asm/processor.h |    1 -
 arch/x86/kernel/cpu/common.c     |    4 ----
 arch/x86/kernel/entry_64.S       |    2 +-
 3 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 2193715..698af88 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -398,7 +398,6 @@ DECLARE_INIT_PER_CPU(irq_stack_union);
 
 DECLARE_PER_CPU(char *, irq_stack_ptr);
 DECLARE_PER_CPU(unsigned int, irq_count);
-extern unsigned long kernel_eflags;
 extern asmlinkage void ignore_sysret(void);
 #else	/* X86_64 */
 #ifdef CONFIG_CC_STACKPROTECTOR
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 22a073d..178c3b4 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1077,8 +1077,6 @@ void syscall_init(void)
 	       X86_EFLAGS_TF|X86_EFLAGS_DF|X86_EFLAGS_IF|X86_EFLAGS_IOPL);
 }
 
-unsigned long kernel_eflags;
-
 /*
  * Copies of the original ist values from the tss are only accessed during
  * debugging, no special alignment required.
@@ -1231,8 +1229,6 @@ void __cpuinit cpu_init(void)
 	fpu_init();
 	xsave_init();
 
-	raw_local_save_flags(kernel_eflags);
-
 	if (is_uv_system())
 		uv_cpu_init();
 }
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index e13329d..b50d142 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -396,7 +396,7 @@ ENTRY(ret_from_fork)
 
 	LOCK ; btr $TIF_FORK,TI_flags(%r8)
 
-	pushq_cfi kernel_eflags(%rip)
+	pushq_cfi $0x0002
 	popfq_cfi				# reset kernel eflags
 
 	call schedule_tail			# rdi: 'prev' task parameter
-- 
1.7.2.5


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

* Re: [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.
  2011-07-06 23:17     ` Andi Kleen
@ 2011-07-07  0:00       ` H. Peter Anvin
  0 siblings, 0 replies; 14+ messages in thread
From: H. Peter Anvin @ 2011-07-07  0:00 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Pekka Enberg, Ian Campbell, Cyrill Gorcunov, linux-kernel,
	Ingo Molnar, Thomas Gleixner, Brian Gerst, Jan Beulich,
	Peter Zijlstra, Frederic Weisbecker, x86, Linus Torvalds

On 07/06/2011 04:17 PM, Andi Kleen wrote:
>> Well lets ask Linus and Andi?
> 
> The original idea was that if someone extends EFLAGS with new bits
> the BIOS could set them.  Strictly said the Intel Manual 
> specifies that ("reserved bits -- always set to values previously read")
>  
> But that never really worked out and I don't think anyone
> would dare extending it now.  So hardcoding is fine I guess.
> 

We already clear any undefined flags almost as soon as we enter the
kernel... and quite rightly so, because there are BIOSes which leave
random crap in the flags.

It is very different from what an *application* should do in this case,
however.

	-hpa

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

* Re: [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.
  2011-07-06  9:39   ` Pekka Enberg
@ 2011-07-06 23:17     ` Andi Kleen
  2011-07-07  0:00       ` H. Peter Anvin
  0 siblings, 1 reply; 14+ messages in thread
From: Andi Kleen @ 2011-07-06 23:17 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Ian Campbell, Cyrill Gorcunov, linux-kernel, Ingo Molnar,
	Thomas Gleixner, H. Peter Anvin, Brian Gerst, Jan Beulich,
	Peter Zijlstra, Frederic Weisbecker, x86, Linus Torvalds

> Well lets ask Linus and Andi?

The original idea was that if someone extends EFLAGS with new bits
the BIOS could set them.  Strictly said the Intel Manual 
specifies that ("reserved bits -- always set to values previously read")
 
But that never really worked out and I don't think anyone
would dare extending it now.  So hardcoding is fine I guess.

-Andi

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

* Re: [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.
  2011-07-06  9:57         ` Ian Campbell
@ 2011-07-06 10:12           ` Cyrill Gorcunov
  0 siblings, 0 replies; 14+ messages in thread
From: Cyrill Gorcunov @ 2011-07-06 10:12 UTC (permalink / raw)
  To: Ian Campbell
  Cc: linux-kernel, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	Brian Gerst, Jan Beulich, Peter Zijlstra, Frederic Weisbecker,
	x86

On Wed, Jul 06, 2011 at 10:57:05AM +0100, Ian Campbell wrote:
> On Wed, 2011-07-06 at 10:46 +0100, Cyrill Gorcunov wrote:
> > On Wed, Jul 06, 2011 at 10:25:29AM +0100, Ian Campbell wrote:
> > ...
> > > > 
> > > > The whole series looks good to me, thanks Ian! I hope I don't
> > > > miss anything, so lets wait for more feedback ;)
> > > 
> > > Thanks. Actually I missed the extern declaration in asm/processor.h so
> > > this updated patch should be used instead:
> > > 
> > 
> > Great, thanks Ian. Have you a chance to run this series? ;)
> 
> Yeah I booted it before I posted it. I figured that the initscripts etc
> would be a sufficient test of fork() and that if it were wrong it would
> fall over pretty quickly and obviously.
> 
> Ian.
> 

OK, thanks!

	Cyrill

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

* Re: [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.
  2011-07-06  9:46       ` Cyrill Gorcunov
@ 2011-07-06  9:57         ` Ian Campbell
  2011-07-06 10:12           ` Cyrill Gorcunov
  0 siblings, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2011-07-06  9:57 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: linux-kernel, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	Brian Gerst, Jan Beulich, Peter Zijlstra, Frederic Weisbecker,
	x86

On Wed, 2011-07-06 at 10:46 +0100, Cyrill Gorcunov wrote:
> On Wed, Jul 06, 2011 at 10:25:29AM +0100, Ian Campbell wrote:
> ...
> > > 
> > > The whole series looks good to me, thanks Ian! I hope I don't
> > > miss anything, so lets wait for more feedback ;)
> > 
> > Thanks. Actually I missed the extern declaration in asm/processor.h so
> > this updated patch should be used instead:
> > 
> 
> Great, thanks Ian. Have you a chance to run this series? ;)

Yeah I booted it before I posted it. I figured that the initscripts etc
would be a sufficient test of fork() and that if it were wrong it would
fall over pretty quickly and obviously.

Ian.


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

* Re: [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.
  2011-07-06  9:25     ` Ian Campbell
@ 2011-07-06  9:46       ` Cyrill Gorcunov
  2011-07-06  9:57         ` Ian Campbell
  0 siblings, 1 reply; 14+ messages in thread
From: Cyrill Gorcunov @ 2011-07-06  9:46 UTC (permalink / raw)
  To: Ian Campbell
  Cc: linux-kernel, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	Brian Gerst, Jan Beulich, Peter Zijlstra, Frederic Weisbecker,
	x86

On Wed, Jul 06, 2011 at 10:25:29AM +0100, Ian Campbell wrote:
...
> > 
> > The whole series looks good to me, thanks Ian! I hope I don't
> > miss anything, so lets wait for more feedback ;)
> 
> Thanks. Actually I missed the extern declaration in asm/processor.h so
> this updated patch should be used instead:
> 

Great, thanks Ian. Have you a chance to run this series? ;)

	Cyrill

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

* Re: [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.
  2011-07-05 13:00 ` [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit Ian Campbell
  2011-07-05 13:28   ` Cyrill Gorcunov
@ 2011-07-06  9:39   ` Pekka Enberg
  2011-07-06 23:17     ` Andi Kleen
  1 sibling, 1 reply; 14+ messages in thread
From: Pekka Enberg @ 2011-07-06  9:39 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Cyrill Gorcunov, linux-kernel, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin, Brian Gerst, Jan Beulich, Peter Zijlstra,
	Frederic Weisbecker, x86, Andi Kleen, Linus Torvalds

On Tue, Jul 5, 2011 at 4:00 PM, Ian Campbell <ian.campbell@citrix.com> wrote:
> For no reason that I can determine 64 bit x86 saves the current eflags
> in cpu_init purely for use in ret_from_fork. The equivalent 32 bit
> code simply hard codes 0x0202 as the new EFLAGS which seems safer than
> relying on a potentially arbitrary EFLAGS saved during cpu_init.
>
> Original i386 changeset
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=47a5c6fa0e204a2b63309c648bb2fde36836c826
> Original x86_64 changset
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=658fdbef66e5e9be79b457edc2cbbb3add840aa9
>
> The only comment in the later indicates that it is following the
> former, but not why it differs in this way.

Well lets ask Linus and Andi?

> This change makes 64 bit use the same mechanism to setup the initial
> EFLAGS on fork. Note that 64 bit resets EFLAGS before calling
> schedule_tail() as opposed to 32 bit which calls schedule_tail()
> first. Therefore the correct value for EFLAGS has opposite IF
> bit. This will be fixed in a subsequent patch.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: x86@kernel.org
> ---
>  arch/x86/kernel/cpu/common.c |    4 ----
>  arch/x86/kernel/entry_64.S   |    2 +-
>  2 files changed, 1 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 22a073d..178c3b4 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1077,8 +1077,6 @@ void syscall_init(void)
>               X86_EFLAGS_TF|X86_EFLAGS_DF|X86_EFLAGS_IF|X86_EFLAGS_IOPL);
>  }
>
> -unsigned long kernel_eflags;
> -
>  /*
>  * Copies of the original ist values from the tss are only accessed during
>  * debugging, no special alignment required.
> @@ -1231,8 +1229,6 @@ void __cpuinit cpu_init(void)
>        fpu_init();
>        xsave_init();
>
> -       raw_local_save_flags(kernel_eflags);
> -
>        if (is_uv_system())
>                uv_cpu_init();
>  }
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index e13329d..b50d142 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -396,7 +396,7 @@ ENTRY(ret_from_fork)
>
>        LOCK ; btr $TIF_FORK,TI_flags(%r8)
>
> -       pushq_cfi kernel_eflags(%rip)
> +       pushq_cfi $0x0002
>        popfq_cfi                               # reset kernel eflags
>
>        call schedule_tail                      # rdi: 'prev' task parameter

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

* Re: [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.
  2011-07-05 13:28   ` Cyrill Gorcunov
@ 2011-07-06  9:25     ` Ian Campbell
  2011-07-06  9:46       ` Cyrill Gorcunov
  0 siblings, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2011-07-06  9:25 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: linux-kernel, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	Brian Gerst, Jan Beulich, Peter Zijlstra, Frederic Weisbecker,
	x86

On Tue, 2011-07-05 at 17:28 +0400, Cyrill Gorcunov wrote:
> On Tue, Jul 05, 2011 at 02:00:12PM +0100, Ian Campbell wrote:
> > For no reason that I can determine 64 bit x86 saves the current eflags
> > in cpu_init purely for use in ret_from_fork. The equivalent 32 bit
> > code simply hard codes 0x0202 as the new EFLAGS which seems safer than
> > relying on a potentially arbitrary EFLAGS saved during cpu_init.
> > 
> > Original i386 changeset
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=47a5c6fa0e204a2b63309c648bb2fde36836c826
> > Original x86_64 changset
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=658fdbef66e5e9be79b457edc2cbbb3add840aa9
> > 
> > The only comment in the later indicates that it is following the
> > former, but not why it differs in this way.
> > 
> > This change makes 64 bit use the same mechanism to setup the initial
> > EFLAGS on fork. Note that 64 bit resets EFLAGS before calling
> > schedule_tail() as opposed to 32 bit which calls schedule_tail()
> > first. Therefore the correct value for EFLAGS has opposite IF
> > bit. This will be fixed in a subsequent patch.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: x86@kernel.org
> 
> The whole series looks good to me, thanks Ian! I hope I don't
> miss anything, so lets wait for more feedback ;)

Thanks. Actually I missed the extern declaration in asm/processor.h so
this updated patch should be used instead:

8<-----------------------------------------------

>From a450bf9e1c6e3f436c652b3e4d63b50c1f9848d4 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Wed, 6 Jul 2011 10:24:46 +0100
Subject: [PATCH] x86: drop unnecessary kernel_eflags variable from 64 bit.

For no reason that I can determine 64 bit x86 saves the current eflags
in cpu_init purely for use in ret_from_fork. The equivalent 32 bit
code simply hard codes 0x0202 as the new EFLAGS which seems safer than
relying on a potentially arbitrary EFLAGS saved during cpu_init.

Original i386 changeset
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=47a5c6fa0e204a2b63309c648bb2fde36836c826
Original x86_64 changset
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=658fdbef66e5e9be79b457edc2cbbb3add840aa9

The only comment in the later indicates that it is following the
former, but not why it differs in this way.

This change makes 64 bit use the same mechanism to setup the initial
EFLAGS on fork. Note that 64 bit resets EFLAGS before calling
schedule_tail() as opposed to 32 bit which calls schedule_tail()
first. Therefore the correct value for EFLAGS has opposite IF
bit. This will be fixed in a subsequent patch.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: x86@kernel.org
---
 arch/x86/include/asm/processor.h |    1 -
 arch/x86/kernel/cpu/common.c     |    4 ----
 arch/x86/kernel/entry_64.S       |    2 +-
 3 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 2193715..698af88 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -398,7 +398,6 @@ DECLARE_INIT_PER_CPU(irq_stack_union);
 
 DECLARE_PER_CPU(char *, irq_stack_ptr);
 DECLARE_PER_CPU(unsigned int, irq_count);
-extern unsigned long kernel_eflags;
 extern asmlinkage void ignore_sysret(void);
 #else	/* X86_64 */
 #ifdef CONFIG_CC_STACKPROTECTOR
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 22a073d..178c3b4 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1077,8 +1077,6 @@ void syscall_init(void)
 	       X86_EFLAGS_TF|X86_EFLAGS_DF|X86_EFLAGS_IF|X86_EFLAGS_IOPL);
 }
 
-unsigned long kernel_eflags;
-
 /*
  * Copies of the original ist values from the tss are only accessed during
  * debugging, no special alignment required.
@@ -1231,8 +1229,6 @@ void __cpuinit cpu_init(void)
 	fpu_init();
 	xsave_init();
 
-	raw_local_save_flags(kernel_eflags);
-
 	if (is_uv_system())
 		uv_cpu_init();
 }
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index e13329d..b50d142 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -396,7 +396,7 @@ ENTRY(ret_from_fork)
 
 	LOCK ; btr $TIF_FORK,TI_flags(%r8)
 
-	pushq_cfi kernel_eflags(%rip)
+	pushq_cfi $0x0002
 	popfq_cfi				# reset kernel eflags
 
 	call schedule_tail			# rdi: 'prev' task parameter
-- 
1.7.2.5




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

* Re: [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.
  2011-07-05 13:00 ` [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit Ian Campbell
@ 2011-07-05 13:28   ` Cyrill Gorcunov
  2011-07-06  9:25     ` Ian Campbell
  2011-07-06  9:39   ` Pekka Enberg
  1 sibling, 1 reply; 14+ messages in thread
From: Cyrill Gorcunov @ 2011-07-05 13:28 UTC (permalink / raw)
  To: Ian Campbell
  Cc: linux-kernel, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	Brian Gerst, Jan Beulich, Peter Zijlstra, Frederic Weisbecker,
	x86

On Tue, Jul 05, 2011 at 02:00:12PM +0100, Ian Campbell wrote:
> For no reason that I can determine 64 bit x86 saves the current eflags
> in cpu_init purely for use in ret_from_fork. The equivalent 32 bit
> code simply hard codes 0x0202 as the new EFLAGS which seems safer than
> relying on a potentially arbitrary EFLAGS saved during cpu_init.
> 
> Original i386 changeset
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=47a5c6fa0e204a2b63309c648bb2fde36836c826
> Original x86_64 changset
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=658fdbef66e5e9be79b457edc2cbbb3add840aa9
> 
> The only comment in the later indicates that it is following the
> former, but not why it differs in this way.
> 
> This change makes 64 bit use the same mechanism to setup the initial
> EFLAGS on fork. Note that 64 bit resets EFLAGS before calling
> schedule_tail() as opposed to 32 bit which calls schedule_tail()
> first. Therefore the correct value for EFLAGS has opposite IF
> bit. This will be fixed in a subsequent patch.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: x86@kernel.org

The whole series looks good to me, thanks Ian! I hope I don't
miss anything, so lets wait for more feedback ;)

	Cyrill

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

* [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.
  2011-07-05 11:15 [Q x86-64] on kernel_eflags Cyrill Gorcunov
@ 2011-07-05 13:00 ` Ian Campbell
  2011-07-05 13:28   ` Cyrill Gorcunov
  2011-07-06  9:39   ` Pekka Enberg
  0 siblings, 2 replies; 14+ messages in thread
From: Ian Campbell @ 2011-07-05 13:00 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: linux-kernel, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	Brian Gerst, Jan Beulich, Peter Zijlstra, Frederic Weisbecker,
	Ian Campbell, x86

For no reason that I can determine 64 bit x86 saves the current eflags
in cpu_init purely for use in ret_from_fork. The equivalent 32 bit
code simply hard codes 0x0202 as the new EFLAGS which seems safer than
relying on a potentially arbitrary EFLAGS saved during cpu_init.

Original i386 changeset
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=47a5c6fa0e204a2b63309c648bb2fde36836c826
Original x86_64 changset
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=658fdbef66e5e9be79b457edc2cbbb3add840aa9

The only comment in the later indicates that it is following the
former, but not why it differs in this way.

This change makes 64 bit use the same mechanism to setup the initial
EFLAGS on fork. Note that 64 bit resets EFLAGS before calling
schedule_tail() as opposed to 32 bit which calls schedule_tail()
first. Therefore the correct value for EFLAGS has opposite IF
bit. This will be fixed in a subsequent patch.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: x86@kernel.org
---
 arch/x86/kernel/cpu/common.c |    4 ----
 arch/x86/kernel/entry_64.S   |    2 +-
 2 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 22a073d..178c3b4 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1077,8 +1077,6 @@ void syscall_init(void)
 	       X86_EFLAGS_TF|X86_EFLAGS_DF|X86_EFLAGS_IF|X86_EFLAGS_IOPL);
 }
 
-unsigned long kernel_eflags;
-
 /*
  * Copies of the original ist values from the tss are only accessed during
  * debugging, no special alignment required.
@@ -1231,8 +1229,6 @@ void __cpuinit cpu_init(void)
 	fpu_init();
 	xsave_init();
 
-	raw_local_save_flags(kernel_eflags);
-
 	if (is_uv_system())
 		uv_cpu_init();
 }
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index e13329d..b50d142 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -396,7 +396,7 @@ ENTRY(ret_from_fork)
 
 	LOCK ; btr $TIF_FORK,TI_flags(%r8)
 
-	pushq_cfi kernel_eflags(%rip)
+	pushq_cfi $0x0002
 	popfq_cfi				# reset kernel eflags
 
 	call schedule_tail			# rdi: 'prev' task parameter
-- 
1.7.2.5


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

end of thread, other threads:[~2011-07-25 10:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-09 15:56 [PATCH 0/3] x86: slightly unify ret_from_fork Ian Campbell
2010-02-09 15:56 ` [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit Ian Campbell
2010-02-09 15:56 ` [PATCH 2/3] x86: make 64 bit ret_from_fork a little more similar to 32 bit Ian Campbell
2010-02-09 15:56 ` [PATCH 3/3] x86: ret_from_fork: use symbolic contants for bits in EFLAGS Ian Campbell
2011-07-05 11:15 [Q x86-64] on kernel_eflags Cyrill Gorcunov
2011-07-05 13:00 ` [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit Ian Campbell
2011-07-05 13:28   ` Cyrill Gorcunov
2011-07-06  9:25     ` Ian Campbell
2011-07-06  9:46       ` Cyrill Gorcunov
2011-07-06  9:57         ` Ian Campbell
2011-07-06 10:12           ` Cyrill Gorcunov
2011-07-06  9:39   ` Pekka Enberg
2011-07-06 23:17     ` Andi Kleen
2011-07-07  0:00       ` H. Peter Anvin
2011-07-25  9:58 [PATCH 0/3] minor cleanups to EFLAGS initialisation in ret_from_fork Ian Campbell
2011-07-25 10:03 ` [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit Ian Campbell

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