From: Scott Wood <hidden> Date: 2006-12-06 21:50:46
This patch causes interrupts to be enabled in program_check_exception()
if they were enabled in the faulting context. This is required because
do_mathemu() calls get_user(), which can sleep, and thus triggers an
assertion if called with interrupts disabled.
Signed-off-by: Scott Wood <redacted>
---
Is there any reason why ProgramCheck needs to be EXC_XFER_STD? Has
nobody ever used math emulation with CONFIG_DEBUG_SPINLOCK_SLEEP before?
arch/powerpc/kernel/head_32.S | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
From: Kumar Gala <hidden> Date: 2006-12-06 21:59:49
On Dec 6, 2006, at 3:50 PM, Scott Wood wrote:
This patch causes interrupts to be enabled in
program_check_exception()
if they were enabled in the faulting context. This is required
because
do_mathemu() calls get_user(), which can sleep, and thus triggers an
assertion if called with interrupts disabled.
Signed-off-by: Scott Wood <redacted>
---
Is there any reason why ProgramCheck needs to be EXC_XFER_STD? Has
nobody ever used math emulation with CONFIG_DEBUG_SPINLOCK_SLEEP
before?
From: Paul Mackerras <hidden> Date: 2006-12-06 22:09:38
Scott Wood writes:
Is there any reason why ProgramCheck needs to be EXC_XFER_STD?
Yes. It used to be EXC_XFER_EE, but I changed it to EXC_XFER_STD
because I didn't want the possibility of preemption on the way from a
kernel breakpoint into a kernel debugger (xmon/kgdb/etc.).
I think the solution to your problem is to rearrange
program_check_exception a little, to make sure we do the
local_irq_enable before calling do_mathemu (but still after the
debugger_bpt call).
Paul.
From: Kumar Gala <hidden> Date: 2006-12-06 22:15:45
On Dec 6, 2006, at 4:09 PM, Paul Mackerras wrote:
Scott Wood writes:
quoted
Is there any reason why ProgramCheck needs to be EXC_XFER_STD?
Yes. It used to be EXC_XFER_EE, but I changed it to EXC_XFER_STD
because I didn't want the possibility of preemption on the way from a
kernel breakpoint into a kernel debugger (xmon/kgdb/etc.).
I think the solution to your problem is to rearrange
program_check_exception a little, to make sure we do the
local_irq_enable before calling do_mathemu (but still after the
debugger_bpt call).
We don't need to rearrange anything, just put the local_irq_enable()
call in do_mathemu(). If we are a debugger than REASON_TRAP will
have to be set and we dont bother with do_mathemu() than.
Do you think we should also move the local_irq_enable() that exists
in the handler into emulate_instruction().
- k
From: Paul Mackerras <hidden> Date: 2006-12-08 04:26:38
Kumar Gala writes:
We don't need to rearrange anything, just put the local_irq_enable()
call in do_mathemu(). If we are a debugger than REASON_TRAP will
have to be set and we dont bother with do_mathemu() than.
Do you think we should also move the local_irq_enable() that exists
in the handler into emulate_instruction().
I'd rather have the local_irq_enable() calls in
program_check_exception() so they're all in one place, preferably with
a comment explaining why they're there. Having an unpaired
local_irq_enable in do_mathemu() would seem a bit weird to me.
Paul.
From: Kumar Gala <hidden> Date: 2006-12-08 08:07:23
On Fri, 8 Dec 2006, Paul Mackerras wrote:
Kumar Gala writes:
quoted
We don't need to rearrange anything, just put the local_irq_enable()
call in do_mathemu(). If we are a debugger than REASON_TRAP will
have to be set and we dont bother with do_mathemu() than.
Do you think we should also move the local_irq_enable() that exists
in the handler into emulate_instruction().
I'd rather have the local_irq_enable() calls in
program_check_exception() so they're all in one place, preferably with
a comment explaining why they're there. Having an unpaired
local_irq_enable in do_mathemu() would seem a bit weird to me.
Ok, how about something like this, I'm a bit concerned about what effect
moving the do_mathemu call will have on things like the 405.
I'll put this in my tree with some other cleanup patches if it looks ok.
- k
From: Kumar Gala <hidden> Date: 2007-01-26 06:49:07
On Dec 8, 2006, at 2:07 AM, Kumar Gala wrote:
On Fri, 8 Dec 2006, Paul Mackerras wrote:
quoted
Kumar Gala writes:
quoted
We don't need to rearrange anything, just put the local_irq_enable()
call in do_mathemu(). If we are a debugger than REASON_TRAP will
have to be set and we dont bother with do_mathemu() than.
Do you think we should also move the local_irq_enable() that exists
in the handler into emulate_instruction().
I'd rather have the local_irq_enable() calls in
program_check_exception() so they're all in one place, preferably
with
a comment explaining why they're there. Having an unpaired
local_irq_enable in do_mathemu() would seem a bit weird to me.
Ok, how about something like this, I'm a bit concerned about what
effect
moving the do_mathemu call will have on things like the 405.
I'll put this in my tree with some other cleanup patches if it
looks ok.
- k
From: Kumar Gala <hidden> Date: 2007-01-26 19:50:30
On Dec 8, 2006, at 2:07 AM, Kumar Gala wrote:
On Fri, 8 Dec 2006, Paul Mackerras wrote:
quoted
Kumar Gala writes:
quoted
We don't need to rearrange anything, just put the local_irq_enable()
call in do_mathemu(). If we are a debugger than REASON_TRAP will
have to be set and we dont bother with do_mathemu() than.
Do you think we should also move the local_irq_enable() that exists
in the handler into emulate_instruction().
I'd rather have the local_irq_enable() calls in
program_check_exception() so they're all in one place, preferably
with
a comment explaining why they're there. Having an unpaired
local_irq_enable in do_mathemu() would seem a bit weird to me.
Ok, how about something like this, I'm a bit concerned about what
effect
moving the do_mathemu call will have on things like the 405.
I'll put this in my tree with some other cleanup patches if it
looks ok.
- k