From mboxrd@z Thu Jan 1 00:00:00 1970 From: Date: Mon, 05 Dec 2011 12:53:56 -0000 Subject: No subject Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de becoming hung by bad code or corrupted memory or files. By automatically feeding them the code has circumvented this important feature. So some examples of where I would expect the watchdog to recover from but with the current code it will not. If u-boot loads a corrupted OS image, when the code starts but before it gets to the point that it has replaced u-boots exception handler it falls into an infinite loop it will hang and the uboot exception handler will continue to keep the system "running". Another example and how we actually discovered this bug due to a leak in a network driver that after many loops would run u-boot out of memory. Once this occurs the hush parser drops into a "for ;;" loop again this is not recovered by the watchdog. So while I understand that some users may want to have the cpu automatically reset the watchdog, We don't. I believe that setting the default value of CONFIG_SYS_WATCHDOG_FREQ if it was not defined is a serious bug. The feature should not be enabled if I did not define this variable. I have attached what we believe is a patch to address our concerns and still allow this to be set by a target that wishes to do so. Comments are appreciated. --eric --------------------------------- Eric Olsen Staff Software Engineer Google, Inc. 650-253-2767 On Mon, Feb 13, 2012 at 12:36 PM, Wolfgang Denk wrote: > Dear Eric, > > In message LL9NHkyuUtJJbE73DYceOsffphtrGU7ZYg at mail.gmail.com> you wrote: > > > > Today we discovered an issue with the implementation of watchdog on the > > PowerPc. In the file arch/powerpc/lib/interrupt.c, the code currently > > automatically feeds the watchdog via the timer interrupt handler if > either > > CONFIG_WATCHDOG or CONFIG_HW_WATCHDOG are defined. I am not exactly sure > > when this got added or why, but I'd guess that for most systems this is > not > > a good feature. For us it is exactly the opposite of what we want. > Would > > either of you object to a patch that uses a different config flag to > enable > > this. > > You don't mention it, but I assume you are referring to U-Boot code > here. > > Please always poost U-Boot related questions to the U-Boot mailing > list only (unless you are asking for commercial support from DENX). > > > I do not exactly understand why you think the current implementation > is not OK as it - my guess is that your expectations are higher than > what U-Boot offers. U-Boot is a boot loader, not an OS, and we use a > lot of simplistic concepts - trading complexity and features which > you can expect from a full-flavored OS for small code size and > simple, easily maintainable code. U-Boot is capable to triggering a > hardware watchdog (which is nevessary, as there are systems where > this cannot be switched off or suspended), but we make no attempts to > implement watchdog monitoring of the U-Boot code itself. If U-Boot > should hang or crash, this is not expected to be detected or > recovered by the watchdog. > > As such, the existing code does what it is supposed to do. > > > Patches to add additional, new features to U-Boot (like full watchdog > monitoring) are of course welcome. Please post these on the mailing > list, then. > > 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 at denx.de > Is a computer language with goto's totally Wirth-less? > --20cf3071d0cebd882804b8e180df-- --20cf3071d0cebd882b04b8e180e1 Content-Type: application/octet-stream; name=patch Content-Disposition: attachment; filename=patch Content-Transfer-Encoding: base64 X-Attachment-Id: f_gym52aj71 TWFrZSBhdXRvbWF0aWMgd2F0Y2hkb2cgcmVzZXQgb3B0aW9uYWwuCiBCYXNlZCB1cG9uIGRlZmlu aW5nIENPTkZJR19TWVNfV0FUQ0hET0dfRlJFUQoKU2lnbmVkLW9mZi1ieTogRXJpYyBPbHNlbiA8 ZW9sc2VuQGdvb2dsZS5jb20+CgotLS0gdS1ib290L2FyY2gvcG93ZXJwYy9saWIvaW50ZXJydXB0 cy5jLm9yaWcJMjAxMi0wMi0xMiAwMToxMTozMy4wMDAwMDAwMDAgLTA4MDAKKysrIHUtYm9vdC9h cmNoL3Bvd2VycGMvbGliL2ludGVycnVwdHMuYwkyMDEyLTAyLTEzIDE1OjExOjQwLjg5MDc1ODQy NiAtMDgwMApAQCAtNDAsMTAgKzQwLDYgQEAgdm9pZCBfX2JvYXJkX3Nob3dfYWN0aXZpdHkgKHVs b25nIGR1bW15KQogfQogI2VuZGlmIC8qIENPTkZJR19TSE9XX0FDVElWSVRZICovCiAKLSNpZm5k ZWYgQ09ORklHX1NZU19XQVRDSERPR19GUkVRCi0jZGVmaW5lIENPTkZJR19TWVNfV0FUQ0hET0df RlJFUSAoQ09ORklHX1NZU19IWiAvIDIpCi0jZW5kaWYKLQogZXh0ZXJuIGludCBpbnRlcnJ1cHRf aW5pdF9jcHUgKHVuc2lnbmVkICopOwogZXh0ZXJuIHZvaWQgdGltZXJfaW50ZXJydXB0X2NwdSAo c3RydWN0IHB0X3JlZ3MgKik7CiAKQEAgLTEyMywxMCArMTE5LDEyIEBAIHZvaWQgdGltZXJfaW50 ZXJydXB0IChzdHJ1Y3QgcHRfcmVncyAqcmUKIAogCXRpbWVzdGFtcCsrOwogCisjaWZkZWYgQ09O RklHX1NZU19XQVRDSERPR19GUkVRCiAjaWYgZGVmaW5lZChDT05GSUdfV0FUQ0hET0cpIHx8IGRl ZmluZWQgKENPTkZJR19IV19XQVRDSERPRykKIAlpZiAoKHRpbWVzdGFtcCAlIChDT05GSUdfU1lT X1dBVENIRE9HX0ZSRVEpKSA9PSAwKQogCQlXQVRDSERPR19SRVNFVCAoKTsKICNlbmRpZiAgICAv KiBDT05GSUdfV0FUQ0hET0cgfHwgQ09ORklHX0hXX1dBVENIRE9HICovCisjZW5kaWYgICAgLyog Q09ORklHX1NZU19XQVRDSERPR19GUkVRICovCiAKICNpZmRlZiBDT05GSUdfU1RBVFVTX0xFRAog CXN0YXR1c19sZWRfdGljayAodGltZXN0YW1wKTsK --20cf3071d0cebd882b04b8e180e1--