Fix the polling section of the hvc driver to use the global "last_hvc" variable, rather than the ttys. With this change debugging a xen dom0 kernel is possible via the following kernel parameter: kgdboc=hvc0 Signed-off-by: Ben Guthro <Benjamin.Guthro@citrix.com> diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c index 2d691eb..3750e74 100644 --- a/drivers/tty/hvc/hvc_console.c +++ b/drivers/tty/hvc/hvc_console.c @@ -766,12 +766,10 @@ int hvc_poll_init(struct tty_driver *driver, int line, char *options) static int hvc_poll_get_char(struct tty_driver *driver, int line) { - struct tty_struct *tty = driver->ttys[0]; - struct hvc_struct *hp = tty->driver_data; int n; char ch; - n = hp->ops->get_chars(hp->vtermno, &ch, 1); + n = cons_ops[last_hvc]->get_chars(vtermnos[last_hvc], &ch, 1); if (n == 0) return NO_POLL_CHAR; @@ -781,12 +779,10 @@ static int hvc_poll_get_char(struct tty_driver *driver, int line) static void hvc_poll_put_char(struct tty_driver *driver, int line, char ch) { - struct tty_struct *tty = driver->ttys[0]; - struct hvc_struct *hp = tty->driver_data; int n; do { - n = hp->ops->put_chars(hp->vtermno, &ch, 1); + n = cons_ops[last_hvc]->put_chars(vtermnos[last_hvc], &ch, 1); } while (n <= 0); } #endif
Konrad Rzeszutek Wilk
2012-Jun-07 16:40 UTC
Re: [Xen-devel] [PATCH] xen/hvc: Fix polling mode to work with kdb/kgdb
On Thu, Jun 07, 2012 at 09:30:06AM -0400, Ben Guthro wrote:> Fix the polling section of the hvc driver to use the global "last_hvc" > variable, rather than the ttys.Could you just do: struct tty_struct *tty = driver->ttys[last_hvc]; as well? So how come the ''0'' one did not work? Is that b/c of console=tty becoming ''0'' instead of hvc0? Is there a crash involved with this? Or is that it just is listening on the wrong console (and which one is that?)> > With this change debugging a xen dom0 kernel is possible via the > following kernel parameter: > kgdboc=hvc0Hm, if that is the problem then this should also be a problem on IBM Power boxes I would think?> > Signed-off-by: Ben Guthro <Benjamin.Guthro@citrix.com> > > > diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c > index 2d691eb..3750e74 100644 > --- a/drivers/tty/hvc/hvc_console.c > +++ b/drivers/tty/hvc/hvc_console.c > @@ -766,12 +766,10 @@ int hvc_poll_init(struct tty_driver *driver, int > line, char *options) > > static int hvc_poll_get_char(struct tty_driver *driver, int line) > { > - struct tty_struct *tty = driver->ttys[0]; > - struct hvc_struct *hp = tty->driver_data; > int n; > char ch; > > - n = hp->ops->get_chars(hp->vtermno, &ch, 1); > + n = cons_ops[last_hvc]->get_chars(vtermnos[last_hvc], &ch, 1); > > if (n == 0) > return NO_POLL_CHAR; > @@ -781,12 +779,10 @@ static int hvc_poll_get_char(struct tty_driver > *driver, int line) > > static void hvc_poll_put_char(struct tty_driver *driver, int line, char ch) > { > - struct tty_struct *tty = driver->ttys[0]; > - struct hvc_struct *hp = tty->driver_data; > int n; > > do { > - n = hp->ops->put_chars(hp->vtermno, &ch, 1); > + n = cons_ops[last_hvc]->put_chars(vtermnos[last_hvc], &ch, 1); > } while (n <= 0); > } > #endif > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > lists.xen.org/xen-devel
Ben Guthro
2012-Jun-07 17:34 UTC
Re: [Xen-devel] [PATCH] xen/hvc: Fix polling mode to work with kdb/kgdb
On Thu, Jun 7, 2012 at 12:40 PM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:> On Thu, Jun 07, 2012 at 09:30:06AM -0400, Ben Guthro wrote: >> Fix the polling section of the hvc driver to use the global "last_hvc" >> variable, rather than the ttys. > > Could you just do: > > struct tty_struct *tty = driver->ttys[last_hvc]; > > as well?No. I tried this, and never got to the kdb prompt. It seems that the problem is that you need to use the cons_ops variable My efforts to fully understand the inner-workings of the console code were thwarted by time. Its a twisty bunch of code. If I used the cons_ops variable static to the module, it was OK. If I used driver->ttys - {get,put}_chars() never got called.> So how come the ''0'' one did not work? Is that b/c > of console=tty becoming ''0'' instead of hvc0? Is there a crash > involved with this? Or is that it just is listening on the > wrong console (and which one is that?)See above.>> >> With this change debugging a xen dom0 kernel is possible via the >> following kernel parameter: >> kgdboc=hvc0 > > Hm, if that is the problem then this should also be a problem on > IBM Power boxes I would think?Not sure...but I think the original submitter of this code was commit 762e77ae7dd055d0b77e0ad34d87db7416df109e Author: Anton Blanchard <anton@samba.org> Date: Tue Jul 12 19:44:05 2011 +0000 hvc_console: Add kdb support Was that for IBM?> >> >> Signed-off-by: Ben Guthro <Benjamin.Guthro@citrix.com> >> >> >> diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c >> index 2d691eb..3750e74 100644 >> --- a/drivers/tty/hvc/hvc_console.c >> +++ b/drivers/tty/hvc/hvc_console.c >> @@ -766,12 +766,10 @@ int hvc_poll_init(struct tty_driver *driver, int >> line, char *options) >> >> static int hvc_poll_get_char(struct tty_driver *driver, int line) >> { >> - struct tty_struct *tty = driver->ttys[0]; >> - struct hvc_struct *hp = tty->driver_data; >> int n; >> char ch; >> >> - n = hp->ops->get_chars(hp->vtermno, &ch, 1); >> + n = cons_ops[last_hvc]->get_chars(vtermnos[last_hvc], &ch, 1); >> >> if (n == 0) >> return NO_POLL_CHAR; >> @@ -781,12 +779,10 @@ static int hvc_poll_get_char(struct tty_driver >> *driver, int line) >> >> static void hvc_poll_put_char(struct tty_driver *driver, int line, char ch) >> { >> - struct tty_struct *tty = driver->ttys[0]; >> - struct hvc_struct *hp = tty->driver_data; >> int n; >> >> do { >> - n = hp->ops->put_chars(hp->vtermno, &ch, 1); >> + n = cons_ops[last_hvc]->put_chars(vtermnos[last_hvc], &ch, 1); >> } while (n <= 0); >> } >> #endif >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> lists.xen.org/xen-devel
Konrad Rzeszutek Wilk
2012-Jun-07 17:48 UTC
Re: [Xen-devel] [PATCH] xen/hvc: Fix polling mode to work with kdb/kgdb
> > On Thu, Jun 07, 2012 at 09:30:06AM -0400, Ben Guthro wrote: > >> Fix the polling section of the hvc driver to use the global "last_hvc" > >> variable, rather than the ttys. > > > > Could you just do: > > > > struct tty_struct *tty = driver->ttys[last_hvc]; > > > > as well? > > No. I tried this, and never got to the kdb prompt. > It seems that the problem is that you need to use the cons_ops variable > > My efforts to fully understand the inner-workings of the console code > were thwarted by time. Its a twisty bunch of code. > If I used the cons_ops variable static to the module, it was OK. > If I used driver->ttys - {get,put}_chars() never got called.Well, now that you guys are working for a big corporation you can relax and afford to spend some time digging in the inner-workings I think :-) .. snip..> > Hm, if that is the problem then this should also be a problem on > > IBM Power boxes I would think? > > Not sure...but I think the original submitter of this code was > > commit 762e77ae7dd055d0b77e0ad34d87db7416df109e > Author: Anton Blanchard <anton@samba.org> > Date: Tue Jul 12 19:44:05 2011 +0000 > > hvc_console: Add kdb support > > > Was that for IBM?No. But the ''hvc'' system is used on IBM Power machines as well. Hence my thought that if it didn''t work under Xen it probably didn''t work under IBM Power machines either.
Konrad Rzeszutek Wilk
2012-Jun-07 18:01 UTC
Re: [Xen-devel] [PATCH] xen/hvc: Fix polling mode to work with kdb/kgdb
On Thu, Jun 07, 2012 at 02:03:48PM -0400, Ben Guthro wrote:> On Thu, Jun 7, 2012 at 1:48 PM, Konrad Rzeszutek Wilk > <konrad.wilk@oracle.com> wrote: > ...snip... > > > Well, now that you guys are working for a big corporation > > you can relax and afford to spend some time digging in the > > inner-workings I think :-) > > I''ll pass along that suggestion. > So far, that hasn''t come to pass... > > I understood it when I wrote this months ago. I really should have > written this up then. > I''ll try to dig in a bit, and report back.OK. Thanks!
Ben Guthro
2012-Jun-07 18:03 UTC
Re: [Xen-devel] [PATCH] xen/hvc: Fix polling mode to work with kdb/kgdb
On Thu, Jun 7, 2012 at 1:48 PM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: ...snip...> Well, now that you guys are working for a big corporation > you can relax and afford to spend some time digging in the > inner-workings I think :-)I''ll pass along that suggestion. So far, that hasn''t come to pass... I understood it when I wrote this months ago. I really should have written this up then. I''ll try to dig in a bit, and report back.
Ben Guthro
2012-Jun-07 20:07 UTC
Re: [Xen-devel] [PATCH] xen/hvc: Fix polling mode to work with kdb/kgdb
On Thu, Jun 7, 2012 at 2:01 PM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: <snip>>> I''ll try to dig in a bit, and report back. > > OK. Thanks!OK, after throwing a bit of trace in, I remember now, the issue I was working around. The tty layer does not seem keep the console open. It opens, and closes the tty based on whether there is data to print. However, the CONFIG_CONSOLE_POLL code goes around all of this, and uses the polling capabilities of the driver. Most of the drivers that implement this mechanism are serial devices where they are actually polling the UART. When Alt+SysRq+g is pressed, the output is flushed, all references to the hvc are released, and then hvc_close() is called. However, since this code fully bypasses the tty layer, no reference is grabbed - and so, when we come into hvc_poll_put_char, and the necessary structures aren''t available. Below is an updated version of this patch that will use the tty->driver_data, if available - but will fall back to using cons_ops. This will guarantee that any existing users of hvc where the above claim is not true will see no change in behavior. However...The part of this that I don''t really understand why we need is in kernel/debug/debug_core.c I don''t like having to put the ifdef in there. I was hoping to be able to remove it, with the upstreaming of the xen IPI code... but in testing, it seems to still be necessary. diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c index 2d691eb..7f965e1 100644 --- a/drivers/tty/hvc/hvc_console.c +++ b/drivers/tty/hvc/hvc_console.c @@ -767,11 +767,14 @@ int hvc_poll_init(struct tty_driver *driver, int line, char *options) static int hvc_poll_get_char(struct tty_driver *driver, int line) { struct tty_struct *tty = driver->ttys[0]; - struct hvc_struct *hp = tty->driver_data; + struct hvc_struct *hp = tty ? tty->driver_data : NULL; + struct hv_ops *ops = (hp && hp->ops) ? hp->ops : cons_ops[last_hvc]; + uint32_t vtno = hp ? hp->vtermno : vtermnos[last_hvc]; + int n; char ch; - n = hp->ops->get_chars(hp->vtermno, &ch, 1); + n = ops->get_chars(vtno, &ch, 1); if (n == 0) return NO_POLL_CHAR; @@ -782,11 +785,14 @@ static int hvc_poll_get_char(struct tty_driver *driver, int line) static void hvc_poll_put_char(struct tty_driver *driver, int line, char ch) { struct tty_struct *tty = driver->ttys[0]; - struct hvc_struct *hp = tty->driver_data; + struct hvc_struct *hp = tty ? tty->driver_data : NULL; + struct hv_ops *ops = (hp && hp->ops) ? hp->ops : cons_ops[last_hvc]; + uint32_t vtno = hp ? hp->vtermno : vtermnos[last_hvc]; + int n; do { - n = hp->ops->put_chars(hp->vtermno, &ch, 1); + n = ops->put_chars(vtno, &ch, 1); } while (n <= 0); } #endif diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index 0557f24..853b1d7 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -579,12 +579,14 @@ return_normal: kgdb_roundup_cpus(flags); #endif +#ifndef CONFIG_XEN /* * Wait for the other CPUs to be notified and be waiting for us: */ while (kgdb_do_roundup && (atomic_read(&masters_in_kgdb) + atomic_read(&slaves_in_kgdb)) != online_cpus) cpu_relax(); +#endif /* * At this point the primary processor is completely
Ben Guthro
2012-Jun-08 13:13 UTC
Re: [Xen-devel] [PATCH] xen/hvc: Fix polling mode to work with kdb/kgdb
On Thu, Jun 7, 2012 at 4:07 PM, Ben Guthro <ben@guthro.net> wrote: <snip>> +#ifndef CONFIG_XEN > /* > * Wait for the other CPUs to be notified and be waiting for us: > */ > while (kgdb_do_roundup && (atomic_read(&masters_in_kgdb) + > atomic_read(&slaves_in_kgdb)) != online_cpus) > cpu_relax(); > +#endif > > /* > * At this point the primary processor is completelyThe cpu roundup seems to not be working as expected here. While I get into the kgdb exception code on the master cpu - none of the slave cpus ever get into the code that is counting them up. Ignoring this CPU roundup is useful to get into the kdb code, but probably not correct. Does you have any thoughts as to why I might not be getting into this code under Xen?