[PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[qiaochong]

KGDB_call_nmi_hook is called by other cpu through smp call.
MIPS smp call is processed in ipi irq handler and regs is saved in
 handle_int.
So kgdb_call_nmi_hook get regs by get_irq_regs and regs will be passed
 to kgdb_cpu_enter.

Signed-off-by: qiaochong <qiaochong@loongson.cn>
---
 arch/mips/kernel/kgdb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
index 6e574c02e4c3b..ea781b29f7f17 100644
--- a/arch/mips/kernel/kgdb.c
+++ b/arch/mips/kernel/kgdb.c
@@ -33,6 +33,7 @@
 #include <asm/processor.h>
 #include <asm/sigcontext.h>
 #include <linux/uaccess.h>
+#include <asm/irq_regs.h>
 
 static struct hard_trap_info {
 	unsigned char tt;	/* Trap type code for MIPS R3xxx and R4xxx */
@@ -214,7 +215,7 @@ void kgdb_call_nmi_hook(void *ignored)
 	old_fs = get_fs();
 	set_fs(KERNEL_DS);
 
-	kgdb_nmicallback(raw_smp_processor_id(), NULL);
+	kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
 
 	set_fs(old_fs);
 }
-- 
2.17.0

Re: [PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[Doug Anderson]

Hi,

On Wed, Mar 27, 2019 at 8:06 AM qiaochong <qiaochong@loongson.cn> wrote:
>
> KGDB_call_nmi_hook is called by other cpu through smp call.
> MIPS smp call is processed in ipi irq handler and regs is saved in
>  handle_int.
> So kgdb_call_nmi_hook get regs by get_irq_regs and regs will be passed
>  to kgdb_cpu_enter.
>
> Signed-off-by: qiaochong <qiaochong@loongson.cn>

Note that you might want to adjust your git settings.  Usually in the
kernel they require that a Signed-off-by have your real name, not just
your username.  You probably need to spin your patch to fix this.  You
should make sure that the authorship of the patch also has your real
name.


> ---
>  arch/mips/kernel/kgdb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
> index 6e574c02e4c3b..ea781b29f7f17 100644
> --- a/arch/mips/kernel/kgdb.c
> +++ b/arch/mips/kernel/kgdb.c
> @@ -33,6 +33,7 @@
>  #include <asm/processor.h>
>  #include <asm/sigcontext.h>
>  #include <linux/uaccess.h>
> +#include <asm/irq_regs.h>
>
>  static struct hard_trap_info {
>         unsigned char tt;       /* Trap type code for MIPS R3xxx and R4xxx */
> @@ -214,7 +215,7 @@ void kgdb_call_nmi_hook(void *ignored)
>         old_fs = get_fs();
>         set_fs(KERNEL_DS);
>
> -       kgdb_nmicallback(raw_smp_processor_id(), NULL);
> +       kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
>
>         set_fs(old_fs);
>  }

As per my reply on V1, feel free to add:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re:Re: [PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[qiaochong]

My name  is QiaoChong， which is same to my username.
Qiao is my family name.
Thanks a lot.

在 2019-03-28 00:25:06，"Doug Anderson" <dianders@chromium.org> 写道：

>Hi,
>
>On Wed, Mar 27, 2019 at 8:06 AM qiaochong <qiaochong@loongson.cn> wrote:
>>
>> KGDB_call_nmi_hook is called by other cpu through smp call.
>> MIPS smp call is processed in ipi irq handler and regs is saved in
>>  handle_int.
>> So kgdb_call_nmi_hook get regs by get_irq_regs and regs will be passed
>>  to kgdb_cpu_enter.
>>
>> Signed-off-by: qiaochong <qiaochong@loongson.cn>
>
>Note that you might want to adjust your git settings.  Usually in the
>kernel they require that a Signed-off-by have your real name, not just
>your username.  You probably need to spin your patch to fix this.  You
>should make sure that the authorship of the patch also has your real
>name.
>
>
>> ---
>>  arch/mips/kernel/kgdb.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
>> index 6e574c02e4c3b..ea781b29f7f17 100644
>> --- a/arch/mips/kernel/kgdb.c
>> +++ b/arch/mips/kernel/kgdb.c
>> @@ -33,6 +33,7 @@
>>  #include <asm/processor.h>
>>  #include <asm/sigcontext.h>
>>  #include <linux/uaccess.h>
>> +#include <asm/irq_regs.h>
>>
>>  static struct hard_trap_info {
>>         unsigned char tt;       /* Trap type code for MIPS R3xxx and R4xxx */
>> @@ -214,7 +215,7 @@ void kgdb_call_nmi_hook(void *ignored)
>>         old_fs = get_fs();
>>         set_fs(KERNEL_DS);
>>
>> -       kgdb_nmicallback(raw_smp_processor_id(), NULL);
>> +       kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
>>
>>         set_fs(old_fs);
>>  }
>
>As per my reply on V1, feel free to add:
>
>Reviewed-by: Douglas Anderson <dianders@chromium.org>


北京市海淀区中关村环保科技示范园龙芯产业园2号楼 100095电话: +86 (10) 62546668传真: +86 (10) 62600826www.loongson.cn本邮件及其附件含有龙芯中科技术有限公司的商业秘密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部 分地泄露、复制或散发）本邮件及其附件中的信息。如果您错收本邮件，请您立即电话或邮件通知发件人并删除本邮件。 

This email and its attachments contain confidential information from Loongson
Technology Corporation Limited, which is intended only for the person or entity
whose address is listed above. Any use of the information contained herein in
any way (including, but not limited to, total or partial disclosure,
reproduction or dissemination) by persons other than the intended recipient(s)
is prohibited. If you receive this email in error, please notify the sender by
phone or email immediately and delete it. 

Re: Re: [PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[Doug Anderson]

Hi,

On Wed, Mar 27, 2019 at 3:25 PM qiaochong <qiaochong@loongson.cn> wrote:
>
>
> My name  is QiaoChong， which is same to my username.
> Qiao is my family name.
> Thanks a lot.

I guess it will be up to whichever maintainer lands this (maybe
Daniel?) on whether they want you to spin it.  I think folks expect to
see a real name that is capitalized and usually a space between the
family name and the given name.

-Doug

Re:Re: Re: [PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[qiaochong]

Hi Doug,

I have changed name and signoff info  format and resent patch  just now.
Thanks a lot.

在 2019-03-28 06:27:26，"Doug Anderson" <dianders@chromium.org> 写道：

>Hi,
>
>On Wed, Mar 27, 2019 at 3:25 PM qiaochong <qiaochong@loongson.cn> wrote:
>>
>>
>> My name  is QiaoChong， which is same to my username.
>> Qiao is my family name.
>> Thanks a lot.
>
>I guess it will be up to whichever maintainer lands this (maybe
>Daniel?) on whether they want you to spin it.  I think folks expect to
>see a real name that is capitalized and usually a space between the
>family name and the given name.
>
>-Doug


北京市海淀区中关村环保科技示范园龙芯产业园2号楼 100095电话: +86 (10) 62546668传真: +86 (10) 62600826www.loongson.cn本邮件及其附件含有龙芯中科技术有限公司的商业秘密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部 分地泄露、复制或散发）本邮件及其附件中的信息。如果您错收本邮件，请您立即电话或邮件通知发件人并删除本邮件。 

This email and its attachments contain confidential information from Loongson
Technology Corporation Limited, which is intended only for the person or entity
whose address is listed above. Any use of the information contained herein in
any way (including, but not limited to, total or partial disclosure,
reproduction or dissemination) by persons other than the intended recipient(s)
is prohibited. If you receive this email in error, please notify the sender by
phone or email immediately and delete it. 

Re: [PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[Daniel Thompson]

On Wed, Mar 27, 2019 at 03:27:26PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Wed, Mar 27, 2019 at 3:25 PM qiaochong <qiaochong@loongson.cn> wrote:
> >
> >
> > My name  is QiaoChong， which is same to my username.
> > Qiao is my family name.
> > Thanks a lot.
> 
> I guess it will be up to whichever maintainer lands this (maybe
> Daniel?)

TBH this is mips specific so I'd expect this to get picked up by the
mips arch maintainers. I'll send in an ack in a moment though.


Daniel.


> on whether they want you to spin it.  I think folks expect to
> see a real name that is capitalized and usually a space between the
> family name and the given name.
> 
> -Doug

[PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[Chong Qiao]

KGDB_call_nmi_hook is called by other cpu through smp call.
MIPS smp call is processed in ipi irq handler and regs is saved in
 handle_int.
So kgdb_call_nmi_hook get regs by get_irq_regs and regs will be passed
 to kgdb_cpu_enter.

Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
---
 arch/mips/kernel/kgdb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
index 6e574c02e4c3b..ea781b29f7f17 100644
--- a/arch/mips/kernel/kgdb.c
+++ b/arch/mips/kernel/kgdb.c
@@ -33,6 +33,7 @@
 #include <asm/processor.h>
 #include <asm/sigcontext.h>
 #include <linux/uaccess.h>
+#include <asm/irq_regs.h>
 
 static struct hard_trap_info {
 	unsigned char tt;	/* Trap type code for MIPS R3xxx and R4xxx */
@@ -214,7 +215,7 @@ void kgdb_call_nmi_hook(void *ignored)
 	old_fs = get_fs();
 	set_fs(KERNEL_DS);
 
-	kgdb_nmicallback(raw_smp_processor_id(), NULL);
+	kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
 
 	set_fs(old_fs);
 }
-- 
2.17.0

Re: [PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[Doug Anderson]

Hi,

On Wed, Mar 27, 2019 at 4:08 PM Chong Qiao <qiaochong@loongson.cn> wrote:
>
> KGDB_call_nmi_hook is called by other cpu through smp call.
> MIPS smp call is processed in ipi irq handler and regs is saved in
>  handle_int.
> So kgdb_call_nmi_hook get regs by get_irq_regs and regs will be passed
>  to kgdb_cpu_enter.
>
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>

Since you didn't really change any content, you could have just
carried my previous review.  In any case I'll add my tag again:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

I'd also note that it's nice if you version your patches and include
version history.  If you need help you might consider using patman to
help you send upstream patches.  See:

http://git.denx.de/?p=u-boot.git;a=blob;f=tools/patman/README

...never mind that it lives in the U-Boot tree--it's great for kernel work too.

Thanks for contributing to the kernel!  :-)

-Doug

Re: [PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[Daniel Thompson]

On Thu, Mar 28, 2019 at 07:08:01AM +0800, Chong Qiao wrote:
> KGDB_call_nmi_hook is called by other cpu through smp call.
> MIPS smp call is processed in ipi irq handler and regs is saved in
>  handle_int.
> So kgdb_call_nmi_hook get regs by get_irq_regs and regs will be passed
>  to kgdb_cpu_enter.
> 
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>

Acked-by: Daniel Thompson <daniel.thompson@linaro.org>


> ---
>  arch/mips/kernel/kgdb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
> index 6e574c02e4c3b..ea781b29f7f17 100644
> --- a/arch/mips/kernel/kgdb.c
> +++ b/arch/mips/kernel/kgdb.c
> @@ -33,6 +33,7 @@
>  #include <asm/processor.h>
>  #include <asm/sigcontext.h>
>  #include <linux/uaccess.h>
> +#include <asm/irq_regs.h>
>  
>  static struct hard_trap_info {
>  	unsigned char tt;	/* Trap type code for MIPS R3xxx and R4xxx */
> @@ -214,7 +215,7 @@ void kgdb_call_nmi_hook(void *ignored)
>  	old_fs = get_fs();
>  	set_fs(KERNEL_DS);
>  
> -	kgdb_nmicallback(raw_smp_processor_id(), NULL);
> +	kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
>  
>  	set_fs(old_fs);
>  }
> -- 
> 2.17.0
> 
> 

Re: [PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[Paul Burton]

Hello,

Chong Qiao wrote:
> KGDB_call_nmi_hook is called by other cpu through smp call.
> MIPS smp call is processed in ipi irq handler and regs is saved in
> handle_int.
> So kgdb_call_nmi_hook get regs by get_irq_regs and regs will be passed
> to kgdb_cpu_enter.
> 
> Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>

Applied to mips-fixes.

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.burton@mips.com to report it. ]

[PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[qiaochong]

KGDB_call_nmi_hook is called by other cpu through smp call.
MIPS smp call is processed in ipi irq handler and regs is saved in
 handle_int.
So kgdb_call_nmi_hook get regs by get_irq_regs and regs will be passed
 to kgdb_cpu_enter.

Signed-off-by: qiaochong <qiaochong@loongson.cn>
---
 arch/mips/kernel/kgdb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
index 6e574c02e4c3b..6c438a0fd2075 100644
--- a/arch/mips/kernel/kgdb.c
+++ b/arch/mips/kernel/kgdb.c
@@ -214,7 +214,7 @@ void kgdb_call_nmi_hook(void *ignored)
 	old_fs = get_fs();
 	set_fs(KERNEL_DS);
 
-	kgdb_nmicallback(raw_smp_processor_id(), NULL);
+	kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
 
 	set_fs(old_fs);
 }
-- 
2.17.0

Re: [PATCH] MIPS: KGDB: fix kgdb support for SMP platforms.

[Doug Anderson]

Hi,

On Wed, Mar 27, 2019 at 6:17 AM qiaochong <qiaochong@loongson.cn> wrote:
>
> KGDB_call_nmi_hook is called by other cpu through smp call.
> MIPS smp call is processed in ipi irq handler and regs is saved in
>  handle_int.
> So kgdb_call_nmi_hook get regs by get_irq_regs and regs will be passed
>  to kgdb_cpu_enter.
>
> Signed-off-by: qiaochong <qiaochong@loongson.cn>
> ---
>  arch/mips/kernel/kgdb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
> index 6e574c02e4c3b..6c438a0fd2075 100644
> --- a/arch/mips/kernel/kgdb.c
> +++ b/arch/mips/kernel/kgdb.c
> @@ -214,7 +214,7 @@ void kgdb_call_nmi_hook(void *ignored)
>         old_fs = get_fs();
>         set_fs(KERNEL_DS);
>
> -       kgdb_nmicallback(raw_smp_processor_id(), NULL);
> +       kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
>
>         set_fs(old_fs);
>  }

I'm excited to see others using kgdb!  :-)

As far as I can tell your patch is good, or at least as seems like it
will make MIPS on par with other platforms.  I always wondered why
MIPS (and ARC) didn't have the get_irq_regs() call but when I last
touched this code I left it alone since I didn't have the history for
it and had no way to test.  Since it seems like you have a way to test
this then we should do it.  Now to find someone who would do the same
for ARC.  :-P  Thus:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

NOTE as pointed out in code reviews when I last touched this code,
using get_irq_regs() isn't perfect since it could plausibly return
NULL.  I created a bug in the Chromium tracker with all the details
for this at <https://crbug.com/908723>.

-Doug