* Fix BUG_ON() reporting in real mode on powerpc
@ 2016-02-17 4:43 Balbir Singh
2016-02-17 4:56 ` Paul Mackerras
0 siblings, 1 reply; 8+ messages in thread
From: Balbir Singh @ 2016-02-17 4:43 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: Paul Mackerras
From: Balbir Singh <bsingharora@gmail.com>
I ran into this issue while debugging an early boot problem.
The system hit a BUG_ON() but report bug failed to print the
line number and file name. The reason being that the system
was running in real mode and report_bug() searches for
addresses in the PAGE_OFFSET+ region
Suggested-by: Paul Mackerras <paulus@samba.org>
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/kernel/traps.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index b6becc7..8f28120 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1148,6 +1148,7 @@ void __kprobes program_check_exception(struct pt_regs *regs)
goto bail;
}
if (reason & REASON_TRAP) {
+ unsigned long bugaddr;
/* Debugger is first in line to stop recursive faults in
* rcu_lock, notify_die, or atomic_notifier_call_chain */
if (debugger_bpt(regs))
@@ -1158,8 +1159,13 @@ void __kprobes program_check_exception(struct pt_regs *regs)
== NOTIFY_STOP)
goto bail;
+ if (!(regs->msr & MSR_IR))
+ bugaddr = regs->nip + PAGE_OFFSET;
+ else
+ bugaddr = regs->nip;
+
if (!(regs->msr & MSR_PR) && /* not user-mode */
- report_bug(regs->nip, regs) == BUG_TRAP_TYPE_WARN) {
+ report_bug(bugaddr, regs) == BUG_TRAP_TYPE_WARN) {
regs->nip += 4;
goto bail;
}
--
2.5.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Fix BUG_ON() reporting in real mode on powerpc
2016-02-17 4:43 Fix BUG_ON() reporting in real mode on powerpc Balbir Singh
@ 2016-02-17 4:56 ` Paul Mackerras
2016-02-17 7:16 ` Balbir Singh
0 siblings, 1 reply; 8+ messages in thread
From: Paul Mackerras @ 2016-02-17 4:56 UTC (permalink / raw)
To: Balbir Singh; +Cc: Michael Ellerman, linuxppc-dev
On Wed, Feb 17, 2016 at 03:43:11PM +1100, Balbir Singh wrote:
> From: Balbir Singh <bsingharora@gmail.com>
>
> I ran into this issue while debugging an early boot problem.
> The system hit a BUG_ON() but report bug failed to print the
> line number and file name. The reason being that the system
> was running in real mode and report_bug() searches for
> addresses in the PAGE_OFFSET+ region
>
> Suggested-by: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> ---
> arch/powerpc/kernel/traps.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index b6becc7..8f28120 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1148,6 +1148,7 @@ void __kprobes program_check_exception(struct pt_regs *regs)
> goto bail;
> }
> if (reason & REASON_TRAP) {
> + unsigned long bugaddr;
> /* Debugger is first in line to stop recursive faults in
> * rcu_lock, notify_die, or atomic_notifier_call_chain */
> if (debugger_bpt(regs))
> @@ -1158,8 +1159,13 @@ void __kprobes program_check_exception(struct pt_regs *regs)
> == NOTIFY_STOP)
> goto bail;
>
> + if (!(regs->msr & MSR_IR))
> + bugaddr = regs->nip + PAGE_OFFSET;
> + else
> + bugaddr = regs->nip;
It might be a little better to do this:
bugaddr = regs->nip;
if (REGION_ID(bugaddr) == 0 && !(regs->msr & MSR_IR))
bugaddr += PAGE_OFFSET;
It is possible to execute from addresses with the 0xc000... on top in
real mode, because the CPU ignores the top 4 address bits in real
mode.
Paul.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix BUG_ON() reporting in real mode on powerpc
2016-02-17 4:56 ` Paul Mackerras
@ 2016-02-17 7:16 ` Balbir Singh
2016-02-17 8:03 ` Anshuman Khandual
2016-02-17 15:16 ` Aneesh Kumar K.V
0 siblings, 2 replies; 8+ messages in thread
From: Balbir Singh @ 2016-02-17 7:16 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Michael Ellerman, linuxppc-dev
> It might be a little better to do this:
>
> bugaddr = regs->nip;
> if (REGION_ID(bugaddr) == 0 && !(regs->msr & MSR_IR))
> bugaddr += PAGE_OFFSET;
>
> It is possible to execute from addresses with the 0xc000... on top in
> real mode, because the CPU ignores the top 4 address bits in real
> mode.
Good catch! Thank you
Changelog:
Don't add PAGE_OFFSET blindly, check if REGION_ID is 0
I ran into this issue while debugging an early boot problem.
The system hit a BUG_ON() but report bug failed to print the
line number and file name. The reason being that the system
was running in real mode and report_bug() searches for
addresses in the PAGE_OFFSET+ region
Suggested-by: Paul Mackerras <paulus@samba.org>
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/kernel/traps.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index b6becc7..4de4fe7 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1148,6 +1148,7 @@ void __kprobes program_check_exception(struct pt_regs *regs)
goto bail;
}
if (reason & REASON_TRAP) {
+ unsigned long bugaddr;
/* Debugger is first in line to stop recursive faults in
* rcu_lock, notify_die, or atomic_notifier_call_chain */
if (debugger_bpt(regs))
@@ -1158,8 +1159,12 @@ void __kprobes program_check_exception(struct pt_regs *regs)
== NOTIFY_STOP)
goto bail;
+ bugaddr = regs->nip;
+ if ((REGION_ID(bugaddr) == 0) && !(regs->msr & MSR_IR))
+ bugaddr += PAGE_OFFSET;
+
if (!(regs->msr & MSR_PR) && /* not user-mode */
- report_bug(regs->nip, regs) == BUG_TRAP_TYPE_WARN) {
+ report_bug(bugaddr, regs) == BUG_TRAP_TYPE_WARN) {
regs->nip += 4;
goto bail;
}
--
2.5.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Fix BUG_ON() reporting in real mode on powerpc
2016-02-17 7:16 ` Balbir Singh
@ 2016-02-17 8:03 ` Anshuman Khandual
2016-02-17 8:23 ` Paul Mackerras
2016-02-17 15:16 ` Aneesh Kumar K.V
1 sibling, 1 reply; 8+ messages in thread
From: Anshuman Khandual @ 2016-02-17 8:03 UTC (permalink / raw)
To: Balbir Singh, Paul Mackerras; +Cc: linuxppc-dev
On 02/17/2016 12:46 PM, Balbir Singh wrote:
>> > It might be a little better to do this:
>> >
>> > bugaddr = regs->nip;
>> > if (REGION_ID(bugaddr) == 0 && !(regs->msr & MSR_IR))
>> > bugaddr += PAGE_OFFSET;
>> >
>> > It is possible to execute from addresses with the 0xc000... on top in
>> > real mode, because the CPU ignores the top 4 address bits in real
>> > mode.
> Good catch! Thank you
>
> Changelog:
> Don't add PAGE_OFFSET blindly, check if REGION_ID is 0
Cant we use USER_REGION_ID directly ?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix BUG_ON() reporting in real mode on powerpc
2016-02-17 8:03 ` Anshuman Khandual
@ 2016-02-17 8:23 ` Paul Mackerras
0 siblings, 0 replies; 8+ messages in thread
From: Paul Mackerras @ 2016-02-17 8:23 UTC (permalink / raw)
To: Anshuman Khandual; +Cc: Balbir Singh, linuxppc-dev
On Wed, Feb 17, 2016 at 01:33:32PM +0530, Anshuman Khandual wrote:
> On 02/17/2016 12:46 PM, Balbir Singh wrote:
> >> > It might be a little better to do this:
> >> >
> >> > bugaddr = regs->nip;
> >> > if (REGION_ID(bugaddr) == 0 && !(regs->msr & MSR_IR))
> >> > bugaddr += PAGE_OFFSET;
> >> >
> >> > It is possible to execute from addresses with the 0xc000... on top in
> >> > real mode, because the CPU ignores the top 4 address bits in real
> >> > mode.
> > Good catch! Thank you
> >
> > Changelog:
> > Don't add PAGE_OFFSET blindly, check if REGION_ID is 0
>
> Cant we use USER_REGION_ID directly ?
If we use USER_REGION_ID then the reader needs to know that the user
region is region 0 to understand the code. Thus I think it is clearer
to use REGION_ID(bugaddr) == 0. Whether or not the address is a user
region address is not really relevant to the question of whether it's
a physical address being accessed directly in real mode vs. a kernel
virtual address, which is what we're trying to determine.
Paul.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix BUG_ON() reporting in real mode on powerpc
2016-02-17 7:16 ` Balbir Singh
2016-02-17 8:03 ` Anshuman Khandual
@ 2016-02-17 15:16 ` Aneesh Kumar K.V
2016-02-18 0:25 ` Balbir Singh
1 sibling, 1 reply; 8+ messages in thread
From: Aneesh Kumar K.V @ 2016-02-17 15:16 UTC (permalink / raw)
To: Balbir Singh, Paul Mackerras; +Cc: linuxppc-dev
Balbir Singh <bsingharora@gmail.com> writes:
>> It might be a little better to do this:
>>=20
>> bugaddr =3D regs->nip;
>> if (REGION_ID(bugaddr) =3D=3D 0 && !(regs->msr & MSR_IR))
>> bugaddr +=3D PAGE_OFFSET;
>>=20
>> It is possible to execute from addresses with the 0xc000... on top in
>> real mode, because the CPU ignores the top 4 address bits in real
>> mode.
>
> Good catch! Thank you
>
> Changelog:
> Don't add PAGE_OFFSET blindly, check if REGION_ID is 0
>
> I ran into this issue while debugging an early boot problem.
> The system hit a BUG_ON() but report bug failed to print the
> line number and file name. The reason being that the system
> was running in real mode and report_bug() searches for
> addresses in the PAGE_OFFSET+ region
>
> Suggested-by: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> ---
> =C2=A0arch/powerpc/kernel/traps.c | 7 ++++++-
> =C2=A01 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index b6becc7..4de4fe7 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1148,6 +1148,7 @@ void __kprobes program_check_exception(struct pt_re=
gs *regs)
> =C2=A0 goto bail;
> =C2=A0 }
> =C2=A0 if (reason & REASON_TRAP) {
> + unsigned long bugaddr;
> =C2=A0 /* Debugger is first in line to stop recursive faults in
> =C2=A0 =C2=A0* rcu_lock, notify_die, or atomic_notifier_call_chain */
> =C2=A0 if (debugger_bpt(regs))
> @@ -1158,8 +1159,12 @@ void __kprobes program_check_exception(struct pt_r=
egs *regs)
> =C2=A0 =3D=3D NOTIFY_STOP)
> =C2=A0 goto bail;
> =C2=A0
> + bugaddr =3D regs->nip;
> + if ((REGION_ID(bugaddr) =3D=3D 0) && !(regs->msr & MSR_IR))
> + bugaddr +=3D PAGE_OFFSET;
> +
Can we add some comments around this. When i looked at this first, i was
wondering how nip can be in user region. But then realized that what we
are checking here is kernel address used in real mode. The use of
REGION_ID eventhough simpler is confusing. Hence adding the comment with
details Paul mentioned in email will help.
> =C2=A0 if (!(regs->msr & MSR_PR) &&=C2=A0=C2=A0/* not user-mode */
> - =C2=A0=C2=A0=C2=A0=C2=A0report_bug(regs->nip, regs) =3D=3D BUG_TRAP_TY=
PE_WARN) {
> + =C2=A0=C2=A0=C2=A0=C2=A0report_bug(bugaddr, regs) =3D=3D BUG_TRAP_TYPE=
_WARN) {
> =C2=A0 regs->nip +=3D 4;
> =C2=A0 goto bail;
> =C2=A0 }
> --=C2=A0
-aneesh
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix BUG_ON() reporting in real mode on powerpc
2016-02-17 15:16 ` Aneesh Kumar K.V
@ 2016-02-18 0:25 ` Balbir Singh
2016-02-18 2:41 ` Aneesh Kumar K.V
0 siblings, 1 reply; 8+ messages in thread
From: Balbir Singh @ 2016-02-18 0:25 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Paul Mackerras, linuxppc-dev
>> Changelog:
>> Don't add PAGE_OFFSET blindly, check if REGION_ID is 0
>>
>> I ran into this issue while debugging an early boot problem.
>> The system hit a BUG_ON() but report bug failed to print the
>> line number and file name. The reason being that the system
>> was running in real mode and report_bug() searches for
>> addresses in the PAGE_OFFSET+ region
>>
>> Suggested-by: Paul Mackerras <paulus@samba.org>
>> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
<snip>
> Can we add some comments around this. When i looked at this first, i was
> wondering how nip can be in user region. But then realized that what we
> are checking here is kernel address used in real mode. The use of
> REGION_ID eventhough simpler is confusing. Hence adding the comment with
> details Paul mentioned in email will help.
>
>
I've tried and covered it in the changelog, I thought a code comment
would make sense for the very non obvious cases and not repeat what
the code does as comment
Balbir Singh.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fix BUG_ON() reporting in real mode on powerpc
2016-02-18 0:25 ` Balbir Singh
@ 2016-02-18 2:41 ` Aneesh Kumar K.V
0 siblings, 0 replies; 8+ messages in thread
From: Aneesh Kumar K.V @ 2016-02-18 2:41 UTC (permalink / raw)
To: Balbir Singh; +Cc: Paul Mackerras, linuxppc-dev
Balbir Singh <bsingharora@gmail.com> writes:
>>> Changelog:
>>> Don't add PAGE_OFFSET blindly, check if REGION_ID is 0
>>>
>>> I ran into this issue while debugging an early boot problem.
>>> The system hit a BUG_ON() but report bug failed to print the
>>> line number and file name. The reason being that the system
>>> was running in real mode and report_bug() searches for
>>> addresses in the PAGE_OFFSET+ region
>>>
>>> Suggested-by: Paul Mackerras <paulus@samba.org>
>>> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
>
> <snip>
>
>> Can we add some comments around this. When i looked at this first, i was
>> wondering how nip can be in user region. But then realized that what we
>> are checking here is kernel address used in real mode. The use of
>> REGION_ID eventhough simpler is confusing. Hence adding the comment with
>> details Paul mentioned in email will help.
>>
>>
> I've tried and covered it in the changelog, I thought a code comment
> would make sense for the very non obvious cases and not repeat what
> the code does as comment
>
The use of REGION_ID indicate that you are checking for region. Hence
the suggestion. Looking at this again, I suggest we should add a new
macro or will have to open code it. Because in the radix series we make
REGION_ID a hash config thing and this is generic stuff.
-aneesh
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-02-18 2:41 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-17 4:43 Fix BUG_ON() reporting in real mode on powerpc Balbir Singh
2016-02-17 4:56 ` Paul Mackerras
2016-02-17 7:16 ` Balbir Singh
2016-02-17 8:03 ` Anshuman Khandual
2016-02-17 8:23 ` Paul Mackerras
2016-02-17 15:16 ` Aneesh Kumar K.V
2016-02-18 0:25 ` Balbir Singh
2016-02-18 2:41 ` Aneesh Kumar K.V
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.