[U-Boot] [RFC PATCH 0/2] Early panic support
This patch set adds support for calling panic() before stdio is initialized. At present this will just hang with little or no indication of what the problem is.
A new board_panic_no_console() function is added to the board API. If provided by the board it will be called in the event of a panic before the console is ready. This function should turn on all UARTS and spray the message out if it possibly can.
This is somewhat related to the Graeme's pre-console buffer patch, which is why I am sending it for comment now.
Simon Glass (2): Add board_panic_no_console to deal with early critical errors Add check that console is ready before output
common/console.c | 4 ++-- include/common.h | 8 ++++++++ lib/vsprintf.c | 26 ++++++++++++++++++++++++-- 3 files changed, 34 insertions(+), 4 deletions(-)
Tested-by: Simon Glass sjg@chromium.org Signed-off-by: Simon Glass sjg@chromium.org --- include/common.h | 8 ++++++++ lib/vsprintf.c | 26 ++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/include/common.h b/include/common.h index 12a1074..856df9a 100644 --- a/include/common.h +++ b/include/common.h @@ -250,6 +250,14 @@ int last_stage_init(void); extern ulong monitor_flash_len; int mac_read_from_eeprom(void);
+/* + * Called during a panic() when no console is available. The board should do + * its best to get a message to the user any way it can. This function should + * return if it can, in which case the system will either hang or reset. + * See panic(). + */ +void board_panic_no_console(const char *str); + /* common/flash.c */ void flash_perror (int);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c index c029fbb..fd742a9 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -24,6 +24,8 @@ # define NUM_TYPE long long #define noinline __attribute__((noinline))
+DECLARE_GLOBAL_DATA_PTR; + const char hex_asc[] = "0123456789abcdef"; #define hex_asc_lo(x) hex_asc[((x) & 0x0f)] #define hex_asc_hi(x) hex_asc[((x) & 0xf0) >> 4] @@ -714,13 +716,33 @@ int sprintf(char * buf, const char *fmt, ...) return i; }
+/* Provide a default function for when no console is available */ +static void __board_panic_no_console(const char *msg) +{ + /* Just return since we can't access the console */ +} + +void board_panic_no_console(const char *msg) __attribute__((weak, + alias("__board_panic_no_console"))); + void panic(const char *fmt, ...) { + char str[CONFIG_SYS_PBSIZE]; va_list args; + va_start(args, fmt); - vprintf(fmt, args); - putc('\n'); + + /* TODO)sjg): Move to vsnprintf() when available */ + vsprintf(str, fmt, args); va_end(args); + + if (gd->have_console) { + puts(str); + putc('\n'); + } else { + board_panic_no_console(str); + } + #if defined (CONFIG_PANIC_HANG) hang(); #else
Hi Simon
On Tue, Aug 30, 2011 at 2:05 AM, Simon Glass sjg@chromium.org wrote:
Tested-by: Simon Glass sjg@chromium.org Signed-off-by: Simon Glass sjg@chromium.org
include/common.h | 8 ++++++++ lib/vsprintf.c | 26 ++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/include/common.h b/include/common.h index 12a1074..856df9a 100644 --- a/include/common.h +++ b/include/common.h @@ -250,6 +250,14 @@ int last_stage_init(void); extern ulong monitor_flash_len; int mac_read_from_eeprom(void);
[snip]
+/* Provide a default function for when no console is available */ +static void __board_panic_no_console(const char *msg) +{
/* Just return since we can't access the console */+}
+void board_panic_no_console(const char *msg) __attribute__((weak,
alias("__board_panic_no_console")));void panic(const char *fmt, ...) {
char str[CONFIG_SYS_PBSIZE]; va_list args;va_start(args, fmt);
vprintf(fmt, args);putc('\n');
/* TODO)sjg): Move to vsnprintf() when available */vsprintf(str, fmt, args); va_end(args);if (gd->have_console) {puts(str);putc('\n');} else {board_panic_no_console(str);}
This patch highlights why console squelching should be done in console.c. We are ending up with more and more if (gd->have_console) all over the place. With the console squelching patch I posted earlier, I think it might be easier to create a weak 'panic_printf()' which aliases to printf() which the board can override if it has the ability to generate emergency output before console is initialised. Console output will be squelched if the board does not define such an override.
Regards,
Graeme
Hi Graeme,
On Mon, Aug 29, 2011 at 4:15 PM, Graeme Russ graeme.russ@gmail.com wrote:
Hi Simon
On Tue, Aug 30, 2011 at 2:05 AM, Simon Glass sjg@chromium.org wrote:
Tested-by: Simon Glass sjg@chromium.org Signed-off-by: Simon Glass sjg@chromium.org
include/common.h | 8 ++++++++ lib/vsprintf.c | 26 ++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/include/common.h b/include/common.h index 12a1074..856df9a 100644 --- a/include/common.h +++ b/include/common.h @@ -250,6 +250,14 @@ int last_stage_init(void); extern ulong monitor_flash_len; int mac_read_from_eeprom(void);
[snip]
+/* Provide a default function for when no console is available */ +static void __board_panic_no_console(const char *msg) +{
- /* Just return since we can't access the console */
+}
+void board_panic_no_console(const char *msg) __attribute__((weak,
- alias("__board_panic_no_console")));
void panic(const char *fmt, ...) {
- char str[CONFIG_SYS_PBSIZE];
va_list args;
va_start(args, fmt);
- vprintf(fmt, args);
- putc('\n');
- /* TODO)sjg): Move to vsnprintf() when available */
- vsprintf(str, fmt, args);
va_end(args);
- if (gd->have_console) {
- puts(str);
- putc('\n');
- } else {
- board_panic_no_console(str);
- }
This patch highlights why console squelching should be done in console.c. We are ending up with more and more if (gd->have_console) all over the place. With the console squelching patch I posted earlier, I think it might be easier to create a weak 'panic_printf()' which aliases to printf() which the board can override if it has the ability to generate emergency output before console is initialised. Console output will be squelched if the board does not define such an override.
My reasoning for putting this in panic is that the board_panic_no_console() turns on all UARTS and might fiddle with baud rates, etc. It should only be called once. Very much an emergency function and not just a different implementation of puts(). That said, with your approach above, I suppose panic_printf() would only be called once.
I would suggest panic_puts() since this would call less external code and be easier for boards to implement.
Regards, Simon
Regards,
Graeme
Dear Simon Glass,
In message 1314633910-8550-2-git-send-email-sjg@chromium.org you wrote:
Tested-by: Simon Glass sjg@chromium.org Signed-off-by: Simon Glass sjg@chromium.org
include/common.h | 8 ++++++++ lib/vsprintf.c | 26 ++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-)
Can we please make this feature conditional, so that boards that are not going to use it don't have to suffer from the increased code size?
Thanks.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Mon, Oct 17, 2011 at 1:06 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message 1314633910-8550-2-git-send-email-sjg@chromium.org you wrote:
Tested-by: Simon Glass sjg@chromium.org Signed-off-by: Simon Glass sjg@chromium.org
include/common.h | 8 ++++++++ lib/vsprintf.c | 26 ++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-)
Can we please make this feature conditional, so that boards that are not going to use it don't have to suffer from the increased code size?
We can easily make some of it conditional (the exported functions), but some of it is changes to make the low-level formatter respect an 'end' pointer. I will take a look as there might be something clever we can do there. We would also need to add, for example, an snprintf() macro which drops the numeric parameter, so that code which uses snprintf() continues to work.
Regards, Simon
Thanks.
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de "The combination of a number of things to make existence worthwhile." "Yes, the philosophy of 'none,' meaning 'all.'" -- Spock and Lincoln, "The Savage Curtain", stardate 5906.4
Hi Wofgang,
On Mon, Oct 17, 2011 at 1:21 PM, Simon Glass sjg@chromium.org wrote:
Hi Wolfgang,
On Mon, Oct 17, 2011 at 1:06 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message 1314633910-8550-2-git-send-email-sjg@chromium.org you wrote:
Tested-by: Simon Glass sjg@chromium.org Signed-off-by: Simon Glass sjg@chromium.org
include/common.h | 8 ++++++++ lib/vsprintf.c | 26 ++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-)
Can we please make this feature conditional, so that boards that are not going to use it don't have to suffer from the increased code size?
We can easily make some of it conditional (the exported functions), but some of it is changes to make the low-level formatter respect an 'end' pointer. I will take a look as there might be something clever we can do there. We would also need to add, for example, an snprintf() macro which drops the numeric parameter, so that code which uses snprintf() continues to work.
Regards, Simon
Thanks.
I was thinking of your code size comments in snprintf() when I wrote this - I sent another patch today which deals with that. I am hoping that that will remove one more barrier to
To address your comment here the code size increase is 42 bytes on ARMv7 with this option. The patch I have just sent makes it optional.
Regards, Simon
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de "The combination of a number of things to make existence worthwhile." "Yes, the philosophy of 'none,' meaning 'all.'" -- Spock and Lincoln, "The Savage Curtain", stardate 5906.4
If puts() or printf() is called before the console is ready, U-Boot will either hang or die. This adds a check for this so that debug() can be used in early code without concern that it will hang.
U-Boot boots properly
Tested-by: Simon Glass sjg@chromium.org Signed-off-by: Simon Glass sjg@chromium.org --- common/console.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/console.c b/common/console.c index 8c650e0..28ddb95 100644 --- a/common/console.c +++ b/common/console.c @@ -338,7 +338,7 @@ void putc(const char c) if (gd->flags & GD_FLG_DEVINIT) { /* Send to the standard output */ fputc(stdout, c); - } else { + } else if (gd->have_console) { /* Send directly to the handler */ serial_putc(c); } @@ -359,7 +359,7 @@ void puts(const char *s) if (gd->flags & GD_FLG_DEVINIT) { /* Send to the standard output */ fputs(stdout, s); - } else { + } else if (gd->have_console) { /* Send directly to the handler */ serial_puts(s); }
Hi Simon,
On Tue, Aug 30, 2011 at 2:05 AM, Simon Glass sjg@chromium.org wrote:
If puts() or printf() is called before the console is ready, U-Boot will either hang or die. This adds a check for this so that debug() can be used in early code without concern that it will hang.
U-Boot boots properly
Tested-by: Simon Glass sjg@chromium.org Signed-off-by: Simon Glass sjg@chromium.org
common/console.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/console.c b/common/console.c index 8c650e0..28ddb95 100644 --- a/common/console.c +++ b/common/console.c @@ -338,7 +338,7 @@ void putc(const char c) if (gd->flags & GD_FLG_DEVINIT) { /* Send to the standard output */ fputc(stdout, c);
- } else {
- } else if (gd->have_console) {
/* Send directly to the handler */ serial_putc(c); } @@ -359,7 +359,7 @@ void puts(const char *s) if (gd->flags & GD_FLG_DEVINIT) { /* Send to the standard output */ fputs(stdout, s);
- } else {
- } else if (gd->have_console) {
/* Send directly to the handler */ serial_puts(s); }
Please have a look at my console squelching patches
Regards,
Graeme
Dear Simon Glass,
In message 1314633910-8550-3-git-send-email-sjg@chromium.org you wrote:
If puts() or printf() is called before the console is ready, U-Boot will either hang or die. This adds a check for this so that debug() can be used in early code without concern that it will hang.
U-Boot boots properly
Tested-by: Simon Glass sjg@chromium.org Signed-off-by: Simon Glass sjg@chromium.org
common/console.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
Isn't this just hushing up implementation errors?
Before, the incorrect call would cause U-Boot to fail - a situation which cannot be overlooked during the port, so the problem will quickly be analyzed and fixed.
With your commit, everything appears to work fine, just some (expected) output will be missing. This is misleading at beats, and will cause that buggy code will go undetected for a long time.
I don't think this is a good strategy.
Please comment.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Mon, Oct 17, 2011 at 1:11 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message 1314633910-8550-3-git-send-email-sjg@chromium.org you wrote:
If puts() or printf() is called before the console is ready, U-Boot will either hang or die. This adds a check for this so that debug() can be used in early code without concern that it will hang.
U-Boot boots properly
Tested-by: Simon Glass sjg@chromium.org Signed-off-by: Simon Glass sjg@chromium.org
common/console.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
Isn't this just hushing up implementation errors?
Before, the incorrect call would cause U-Boot to fail - a situation which cannot be overlooked during the port, so the problem will quickly be analyzed and fixed.
With your commit, everything appears to work fine, just some (expected) output will be missing. This is misleading at beats, and will cause that buggy code will go undetected for a long time.
I don't think this is a good strategy.
Please comment.
Yes indeed - I did actually submit an 'early panic' patch which I should take another look at - it is called 'Add board_panic_no_console to deal with early critical errors'. I think it was in the same patch set but I'm not sure.
It would panic when console output was performed before the console was ready - and therefore give the behaviour you desire.
Regards, Simon
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de Compassion -- that's the one things no machine ever had. Maybe it's the one thing that keeps men ahead of them. -- McCoy, "The Ultimate Computer", stardate 4731.3
Dear Simon Glass,
In message CAPnjgZ3WBmJBs1zCd4_jQsUZcO2BOAOK865CaL6tWeUX9zVVWg@mail.gmail.com you wrote:
Yes indeed - I did actually submit an 'early panic' patch which I should take another look at - it is called 'Add board_panic_no_console to deal with early critical errors'. I think it was in the same patch set but I'm not sure.
It was (I even commented on it).
It would panic when console output was performed before the console was ready - and therefore give the behaviour you desire.
Well, but there will probably be a large number of boards that don't implement this function (or don;t enable it if we make it configurable). And all these will now fail silently.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Mon, Oct 17, 2011 at 1:37 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message CAPnjgZ3WBmJBs1zCd4_jQsUZcO2BOAOK865CaL6tWeUX9zVVWg@mail.gmail.com you wrote:
Yes indeed - I did actually submit an 'early panic' patch which I should take another look at - it is called 'Add board_panic_no_console to deal with early critical errors'. I think it was in the same patch set but I'm not sure.
It was (I even commented on it).
It would panic when console output was performed before the console was ready - and therefore give the behaviour you desire.
Well, but there will probably be a large number of boards that don't implement this function (or don;t enable it if we make it configurable). And all these will now fail silently.
Sorry, just emailed again on this thread - actually the behaviour of that patch has already come in due to Graeme's patch.
And I think it is actually the opposite - they will not fail silently, but in fact ignore the pre-console printf() and continue. Previously (before Graeme's patch and this one had it been applied) any pre-console printf() would cause a silent failure / hang. I think this was the intent of the fix.
The intent of my other patch in the series was to provide a way for a board to panic before the console is ready, and hopefully get a message to the user (flashing lights, LCD, init all the UARTs and blast out a msg, etc.). It is true that most boards will not implement this. We could perhaps look at implementing a weak function to do this at the architecture level (but only for SOCs) but it seems risky.
Regards, Simon
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de panic: kernel trap (ignored)
Hi Wolfgang,
On Mon, Oct 17, 2011 at 1:25 PM, Simon Glass sjg@chromium.org wrote:
Hi Wolfgang,
On Mon, Oct 17, 2011 at 1:11 PM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message 1314633910-8550-3-git-send-email-sjg@chromium.org you wrote:
If puts() or printf() is called before the console is ready, U-Boot will either hang or die. This adds a check for this so that debug() can be used in early code without concern that it will hang.
U-Boot boots properly
Tested-by: Simon Glass sjg@chromium.org Signed-off-by: Simon Glass sjg@chromium.org
common/console.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
Isn't this just hushing up implementation errors?
Before, the incorrect call would cause U-Boot to fail - a situation which cannot be overlooked during the port, so the problem will quickly be analyzed and fixed.
With your commit, everything appears to work fine, just some (expected) output will be missing. This is misleading at beats, and will cause that buggy code will go undetected for a long time.
I don't think this is a good strategy.
Please comment.
Yes indeed - I did actually submit an 'early panic' patch which I should take another look at - it is called 'Add board_panic_no_console to deal with early critical errors'. I think it was in the same patch set but I'm not sure.
It would panic when console output was performed before the console was ready - and therefore give the behaviour you desire.
I took another look at this - in fact this patch has been entirely superseded by Graeme's:
commit e3e454cd72f319908355427b1a3ae54b3dd53356 Author: Graeme Russ graeme.russ@gmail.com Date: Mon Aug 29 02:14:05 2011 +0000
console: Squelch pre-console output in console functions
so there is no need for me to resubmit. The other patch I mentioned, The board_panic_no_console patch is still of interest though.
Regards, Simon
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de Compassion -- that's the one things no machine ever had. Maybe it's the one thing that keeps men ahead of them. -- McCoy, "The Ultimate Computer", stardate 4731.3
participants (3)
-
Graeme Russ -
Simon Glass -
Wolfgang Denk