[kGDB]Cannot compile KGDB on ARM SMP

[kGDB]Cannot compile KGDB on ARM SMP

[Jean-Michel Hautbois]

Hi there !

I am  curently trying to compile kgdb on an ARM SMP core.
I have the following error :
kernel/kgdb.c:1481: undefined reference to `kgdb_roundup_cpus'

When I am looking at the function 'kgdb_roundup_cpus', it is defined
for x86, mips, blackfin, powerpc, but not for ARM.
Is there any specific reason for that ?
I can't see why...

Thanks in advance for your help !

Best Regards,
JM

[kGDB]Cannot compile KGDB on ARM SMP

[Jason Wessel]

Jean-Michel Hautbois wrote:
> Hi there !
>
> I am  curently trying to compile kgdb on an ARM SMP core.
> I have the following error :
> kernel/kgdb.c:1481: undefined reference to `kgdb_roundup_cpus'
>
> When I am looking at the function 'kgdb_roundup_cpus', it is defined
> for x86, mips, blackfin, powerpc, but not for ARM.
> Is there any specific reason for that ?
> I can't see why...
>
>   

No one has yet to submit a patch which implements the proper way to
interrupt the slave CPUs and get them to call the kgdb_nmicallback(), as
well as to run the kgdb test suite on the SMP device.

I do not have any SMP ARM hw to test this, or I probably would have
implemented myself.

If you come up with any patches you can submit them here and please cc
kgdb-bugreport at lists.sourceforge.net.  If you have questions about the
kgdb test suite I am sure we can find some answers.

Thanks,
Jason.

[kGDB]Cannot compile KGDB on ARM SMP

[Will Deacon]

Hello,

> No one has yet to submit a patch which implements the proper way to
> interrupt the slave CPUs and get them to call the kgdb_nmicallback(), as
> well as to run the kgdb test suite on the SMP device.

I've done the obvious thing with this patch:

diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c
index ba8ccfe..a5b846b 100644
--- a/arch/arm/kernel/kgdb.c
+++ b/arch/arm/kernel/kgdb.c
@@ -9,6 +9,7 @@
  * Authors:  George Davis <davis_g@mvista.com>
  *           Deepak Saxena <dsaxena@plexity.net>
  */
+#include <linux/irq.h>
 #include <linux/kgdb.h>
 #include <asm/traps.h>
 
@@ -158,6 +159,18 @@ static struct undef_hook kgdb_compiled_brkpt_hook = {
        .fn                     = kgdb_compiled_brk_fn
 };
 
+static void kgdb_call_nmi_hook(void *ignored)
+{
+       kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
+}
+
+void kgdb_roundup_cpus(unsigned long flags)
+{
+       local_irq_enable();
+       smp_call_function(kgdb_call_nmi_hook, NULL, 0);
+       local_irq_disable();
+}
+
 /**
  *     kgdb_arch_init - Perform any architecture specific initalization.
  *

This is all well and good and solves the compilation issue. However, in an
SMP environment [quad-core ARM 11MPCore] the testsuite very quickly deadlocks:

rv-pb11mpcore:~# echo kgdbts=V1 > /sys/module/kgdbts/parameters/kgdbts
[   62.787343] kgdb: Registered I/O driver kgdbts.
[   62.801176] kgdbts:RUN plant and detach test
[   62.814748] kgdbts:RUN sw breakpoint test
[   62.828126] kgdbts:RUN bad memory access test
[   62.841687] kgdbts:RUN singlestep test 1000 iterations
[   62.859816] kgdbts:RUN singlestep [0/1000]
[   63.132699] kgdbts:RUN singlestep [100/1000]
[   63.406217] kgdbts:RUN singlestep [200/1000]
[   63.679694] kgdbts:RUN singlestep [300/1000]

<machine completely unresponsive>

I took a quick look at the code in kernel/kgdb.c and adding the following
memory barrier appears to resolve the issue:

diff --git a/kernel/kgdb.c b/kernel/kgdb.c
index 761fdd2..1308381 100644
--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -1537,6 +1537,7 @@ acquirelock:
                 * Wait till all the CPUs have quit
                 * from the debugger.
                 */
+               smp_wmb();
                for_each_online_cpu(i) {
                        while (atomic_read(&cpu_in_kgdb[i]))
                                cpu_relax();

There may be more that are missing, but I'm not familiar enough with KGDB
internals to know what's going on.

I can submit this as a patch if you'd like, but I'd value some feedback
first. Adding random memory barriers isn't a great solution!

Cheers,

Will

[kGDB]Cannot compile KGDB on ARM SMP

[Catalin Marinas]

On Thu, 2010-03-04 at 17:28 +0000, Will Deacon wrote:
> I took a quick look at the code in kernel/kgdb.c and adding the following
> memory barrier appears to resolve the issue:
> 
> diff --git a/kernel/kgdb.c b/kernel/kgdb.c
> index 761fdd2..1308381 100644
> --- a/kernel/kgdb.c
> +++ b/kernel/kgdb.c
> @@ -1537,6 +1537,7 @@ acquirelock:
>                  * Wait till all the CPUs have quit
>                  * from the debugger.
>                  */
> +               smp_wmb();
>                 for_each_online_cpu(i) {
>                         while (atomic_read(&cpu_in_kgdb[i]))
>                                 cpu_relax();

Shouldn't this be smp_rmb()?

-- 
Catalin

[kGDB]Cannot compile KGDB on ARM SMP

[Will Deacon]

Hi Catalin,

> On Thu, 2010-03-04 at 17:28 +0000, Will Deacon wrote:
> > I took a quick look at the code in kernel/kgdb.c and adding the following
> > memory barrier appears to resolve the issue:
> >
> > diff --git a/kernel/kgdb.c b/kernel/kgdb.c
> > index 761fdd2..1308381 100644
> > --- a/kernel/kgdb.c
> > +++ b/kernel/kgdb.c
> > @@ -1537,6 +1537,7 @@ acquirelock:
> >                  * Wait till all the CPUs have quit
> >                  * from the debugger.
> >                  */
> > +               smp_wmb();
> >                 for_each_online_cpu(i) {
> >                         while (atomic_read(&cpu_in_kgdb[i]))
> >                                 cpu_relax();
> 
> Shouldn't this be smp_rmb()?

Actually, I think that we need an smp_mb() because there is
another variable involved (passive_cpu_wait[i]) as well as
cpu_in_kgdb[i], both of which are being read from and written
to.

I think deadlock occurs because the store buffers don't drain
on the 11MPCore, so the CPUs happily cpu_relax() while polling
on variables that they will never see change. Perhaps cpu_relax()
should be defined to be something stronger than a compiler memory
barrier for ARM?

Will