[Adeos-main] [pull request] x86: Fix up regs unconditionally on exceptions

[Adeos-main] [pull request] x86: Fix up regs unconditionally on exceptions

[Jan Kiszka]

The following changes since commit 14023bef1806dd640030d3eaf73c26736345bc30:
  Philippe Gerum (1):
        ipipe-2.6.32.7-x86-2.6-01

are available in the git repository at:

  git://git.kiszka.org/ipipe-2.6 queues/2.6.32-x86

Found while making KVM work over I-pipe/Xenomai. Its hardware
activation code triggered a planned #GP in IRQ context.


Jan Kiszka (1):
      x86: Fix up regs unconditionally on exceptions

 arch/x86/kernel/ipipe.c |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

------

x86: Fix up regs unconditionally on exceptions

Some Linux exception handlers - at least do_general_protection -
evaluate regs->flags, and that on both x86-32 and -64. So we should fix
up the flags according to the pipeline state unconditionally.

Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
---
 arch/x86/kernel/ipipe.c |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c
index 36cd591..dfb76ad 100644
--- a/arch/x86/kernel/ipipe.c
+++ b/arch/x86/kernel/ipipe.c
@@ -495,8 +495,6 @@ out:
 	local_irq_restore_hw(flags);
 }
 
-#ifdef CONFIG_X86_32
-
 static inline void __fixup_if(int s, struct pt_regs *regs)
 {
 	/*
@@ -510,6 +508,8 @@ static inline void __fixup_if(int s, struct pt_regs *regs)
 		regs->flags |= X86_EFLAGS_IF;
 }
 
+#ifdef CONFIG_X86_32
+
 /*
  * Check the stall bit of the root domain to make sure the existing
  * preemption opportunity upon in-kernel resumption could be
@@ -571,10 +571,6 @@ asmlinkage void __ipipe_unstall_iret_root(struct pt_regs regs)
 
 #else /* !CONFIG_X86_32 */
 
-static inline void __fixup_if(int s, struct pt_regs *regs)
-{
-}
-
 #ifdef CONFIG_PREEMPT
 
 asmlinkage void preempt_schedule_irq(void);
@@ -746,11 +742,11 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
 
 	if (likely(ipipe_root_domain_p)) {
 		/*
-		 * 32-bit: In case we faulted in the iret path, regs.flags do
-		 * not match the root domain state as the low-level return
-		 * code will evaluate it. Fix this up, either by the root
-		 * state sampled on entry or, if we migrated to root, with the
-		 * current state.
+		 * In case we faulted in the iret path, regs.flags do not
+		 * match the root domain state. The fault handler or the
+		 * low-level return code may evaluate it. Fix this up, either
+		 * by the root state sampled on entry or, if we migrated to
+		 * root, with the current state.
 		 */
 		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
 					raw_irqs_disabled(), regs);
@@ -876,7 +872,13 @@ int __ipipe_syscall_root(struct pt_regs *regs)
 
 	local_irq_save_hw(flags);
 	p = ipipe_root_cpudom_ptr();
+#ifdef CONFIG_X86_32
+	/*
+	 * Fix-up only required on 32-bit as only here the IRET return code
+	 * will evaluate the flags.
+	 */
 	__fixup_if(test_bit(IPIPE_STALL_FLAG, &p->status), regs);
+#endif
 	/*
 	 * If allowed, sync pending VIRQs before _TIF_NEED_RESCHED is
 	 * tested.
-- 
1.6.0.2

Re: [Adeos-main] [pull request] x86: Fix up regs unconditionally on exceptions

[Jan Kiszka]

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

Jan Kiszka wrote:
> The following changes since commit 14023bef1806dd640030d3eaf73c26736345bc30:
>   Philippe Gerum (1):
>         ipipe-2.6.32.7-x86-2.6-01
> 
> are available in the git repository at:
> 
>   git://git.kiszka.org/ipipe-2.6 queues/2.6.32-x86
> 
> Found while making KVM work over I-pipe/Xenomai. Its hardware
> activation code triggered a planned #GP in IRQ context.
> 
> 
> Jan Kiszka (1):
>       x86: Fix up regs unconditionally on exceptions
> 
>  arch/x86/kernel/ipipe.c |   24 +++++++++++++-----------
>  1 files changed, 13 insertions(+), 11 deletions(-)
> 
> ------
> 
> x86: Fix up regs unconditionally on exceptions
> 
> Some Linux exception handlers - at least do_general_protection -
> evaluate regs->flags, and that on both x86-32 and -64. So we should fix
> up the flags according to the pipeline state unconditionally.

Forgotten or intentionally skipped for 2.6-02?

Hmm, I see that even the fixed-up comment in __ipipe_handle_exception is
not correct anymore. I can push an updated version if you're fine with
picking this up.

Jan

> 
> Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
> ---
>  arch/x86/kernel/ipipe.c |   24 +++++++++++++-----------
>  1 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c
> index 36cd591..dfb76ad 100644
> --- a/arch/x86/kernel/ipipe.c
> +++ b/arch/x86/kernel/ipipe.c
> @@ -495,8 +495,6 @@ out:
>  	local_irq_restore_hw(flags);
>  }
>  
> -#ifdef CONFIG_X86_32
> -
>  static inline void __fixup_if(int s, struct pt_regs *regs)
>  {
>  	/*
> @@ -510,6 +508,8 @@ static inline void __fixup_if(int s, struct pt_regs *regs)
>  		regs->flags |= X86_EFLAGS_IF;
>  }
>  
> +#ifdef CONFIG_X86_32
> +
>  /*
>   * Check the stall bit of the root domain to make sure the existing
>   * preemption opportunity upon in-kernel resumption could be
> @@ -571,10 +571,6 @@ asmlinkage void __ipipe_unstall_iret_root(struct pt_regs regs)
>  
>  #else /* !CONFIG_X86_32 */
>  
> -static inline void __fixup_if(int s, struct pt_regs *regs)
> -{
> -}
> -
>  #ifdef CONFIG_PREEMPT
>  
>  asmlinkage void preempt_schedule_irq(void);
> @@ -746,11 +742,11 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
>  
>  	if (likely(ipipe_root_domain_p)) {
>  		/*
> -		 * 32-bit: In case we faulted in the iret path, regs.flags do
> -		 * not match the root domain state as the low-level return
> -		 * code will evaluate it. Fix this up, either by the root
> -		 * state sampled on entry or, if we migrated to root, with the
> -		 * current state.
> +		 * In case we faulted in the iret path, regs.flags do not
> +		 * match the root domain state. The fault handler or the
> +		 * low-level return code may evaluate it. Fix this up, either
> +		 * by the root state sampled on entry or, if we migrated to
> +		 * root, with the current state.
>  		 */
>  		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
>  					raw_irqs_disabled(), regs);
> @@ -876,7 +872,13 @@ int __ipipe_syscall_root(struct pt_regs *regs)
>  
>  	local_irq_save_hw(flags);
>  	p = ipipe_root_cpudom_ptr();
> +#ifdef CONFIG_X86_32
> +	/*
> +	 * Fix-up only required on 32-bit as only here the IRET return code
> +	 * will evaluate the flags.
> +	 */
>  	__fixup_if(test_bit(IPIPE_STALL_FLAG, &p->status), regs);
> +#endif
>  	/*
>  	 * If allowed, sync pending VIRQs before _TIF_NEED_RESCHED is
>  	 * tested.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Adeos-main] [pull request] x86: Fix up regs unconditionally on exceptions

[Philippe Gerum]

On Sun, 2010-04-11 at 18:37 +0200, Jan Kiszka wrote:
> Jan Kiszka wrote:
> > The following changes since commit 14023bef1806dd640030d3eaf73c26736345bc30:
> >   Philippe Gerum (1):
> >         ipipe-2.6.32.7-x86-2.6-01
> > 
> > are available in the git repository at:
> > 
> >   git://git.kiszka.org/ipipe-2.6 queues/2.6.32-x86
> > 
> > Found while making KVM work over I-pipe/Xenomai. Its hardware
> > activation code triggered a planned #GP in IRQ context.
> > 
> > 
> > Jan Kiszka (1):
> >       x86: Fix up regs unconditionally on exceptions
> > 
> >  arch/x86/kernel/ipipe.c |   24 +++++++++++++-----------
> >  1 files changed, 13 insertions(+), 11 deletions(-)
> > 
> > ------
> > 
> > x86: Fix up regs unconditionally on exceptions
> > 
> > Some Linux exception handlers - at least do_general_protection -
> > evaluate regs->flags, and that on both x86-32 and -64. So we should fix
> > up the flags according to the pipeline state unconditionally.
> 
> Forgotten or intentionally skipped for 2.6-02?

Intentionally left aside for now. This is a touchy code, and I want to
make sure to properly assess the implications of such change before
merging.

> 
> Hmm, I see that even the fixed-up comment in __ipipe_handle_exception is
> not correct anymore. I can push an updated version if you're fine with
> picking this up.
> 
> Jan
> 
> > 
> > Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
> > ---
> >  arch/x86/kernel/ipipe.c |   24 +++++++++++++-----------
> >  1 files changed, 13 insertions(+), 11 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/ipipe.c b/arch/x86/kernel/ipipe.c
> > index 36cd591..dfb76ad 100644
> > --- a/arch/x86/kernel/ipipe.c
> > +++ b/arch/x86/kernel/ipipe.c
> > @@ -495,8 +495,6 @@ out:
> >  	local_irq_restore_hw(flags);
> >  }
> >  
> > -#ifdef CONFIG_X86_32
> > -
> >  static inline void __fixup_if(int s, struct pt_regs *regs)
> >  {
> >  	/*
> > @@ -510,6 +508,8 @@ static inline void __fixup_if(int s, struct pt_regs *regs)
> >  		regs->flags |= X86_EFLAGS_IF;
> >  }
> >  
> > +#ifdef CONFIG_X86_32
> > +
> >  /*
> >   * Check the stall bit of the root domain to make sure the existing
> >   * preemption opportunity upon in-kernel resumption could be
> > @@ -571,10 +571,6 @@ asmlinkage void __ipipe_unstall_iret_root(struct pt_regs regs)
> >  
> >  #else /* !CONFIG_X86_32 */
> >  
> > -static inline void __fixup_if(int s, struct pt_regs *regs)
> > -{
> > -}
> > -
> >  #ifdef CONFIG_PREEMPT
> >  
> >  asmlinkage void preempt_schedule_irq(void);
> > @@ -746,11 +742,11 @@ int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
> >  
> >  	if (likely(ipipe_root_domain_p)) {
> >  		/*
> > -		 * 32-bit: In case we faulted in the iret path, regs.flags do
> > -		 * not match the root domain state as the low-level return
> > -		 * code will evaluate it. Fix this up, either by the root
> > -		 * state sampled on entry or, if we migrated to root, with the
> > -		 * current state.
> > +		 * In case we faulted in the iret path, regs.flags do not
> > +		 * match the root domain state. The fault handler or the
> > +		 * low-level return code may evaluate it. Fix this up, either
> > +		 * by the root state sampled on entry or, if we migrated to
> > +		 * root, with the current state.
> >  		 */
> >  		__fixup_if(root_entry ? raw_irqs_disabled_flags(flags) :
> >  					raw_irqs_disabled(), regs);
> > @@ -876,7 +872,13 @@ int __ipipe_syscall_root(struct pt_regs *regs)
> >  
> >  	local_irq_save_hw(flags);
> >  	p = ipipe_root_cpudom_ptr();
> > +#ifdef CONFIG_X86_32
> > +	/*
> > +	 * Fix-up only required on 32-bit as only here the IRET return code
> > +	 * will evaluate the flags.
> > +	 */
> >  	__fixup_if(test_bit(IPIPE_STALL_FLAG, &p->status), regs);
> > +#endif
> >  	/*
> >  	 * If allowed, sync pending VIRQs before _TIF_NEED_RESCHED is
> >  	 * tested.
> 
> 


-- 
Philippe.