<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
On 2/1/11 2:50 AM, Patrick Georgi wrote:
<blockquote cite="mid:1296566674.6489.78.camel@linux-0a8x.site"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-unicode">
<pre wrap="">The cmos.default code wasn't actually used so far, due to an oversight
when forward-porting this feature from an old branch.
- Extend walkcbfs' use by factoring out the stage handling into C code.
- New sanitize_cmos() function that looks if CMOS data is invalid and
cmos.default exists and if so overwrites CMOS with cmos.default data.
- Use sanitize_cmos() in both bootblock implementations.
- Drop the need to reboot after writing CMOS: CMOS wasn't used so far,
so we can go on without a reboot.
- Remove the restriction that cmos.default only works on CAR boards.
- Always build in cmos.default support on boards that USE_OPTION_TABLE.
Signed-off-by: Patrick Georgi <a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:patrick.georgi@secunet.com"><patrick.georgi@secunet.com></a>
</pre>
</div>
</blockquote>
<br>
Acked-by: Stefan Reinauer <a class="moz-txt-link-rfc2396E" href="mailto:stefan.reinauer@coreboot.org"><stefan.reinauer@coreboot.org></a><br>
<br>
with some optional comments below<br>
<br>
<blockquote cite="mid:1296566674.6489.78.camel@linux-0a8x.site"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">index 895a185..a808cec 100644
--- a/src/arch/x86/include/bootblock_common.h
+++ b/src/arch/x86/include/bootblock_common.h
@@ -17,17 +17,45 @@ static void bootblock_northbridge_init(void) { }
static void bootblock_southbridge_init(void) { }
#endif
-static unsigned long findstage(char* target)
+static void *walkcbfs(char *target)
{
- unsigned long entry;
+ void *entry;
asm volatile (
"mov $1f, %%esp\n\t"
- "jmp walkcbfs\n\t"
+ "jmp walkcbfs_asm\n\t"
</pre>
</div>
</blockquote>
maybe just call it _walkcbfs ?<br>
<blockquote cite="mid:1296566674.6489.78.camel@linux-0a8x.site"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">+/* just enough to support findstage. copied because the original version doesn't easily pass through romcc */
+struct cbfs_stage {
+ unsigned long compression;
+ unsigned long entry; // this is really 64bit, but properly endianized
</pre>
</div>
</blockquote>
Would it make sense to add an unsigned long entry_high after this,
in this case? Or use a union or uint64_t for entry?<br>
<br>
<blockquote cite="mid:1296566674.6489.78.camel@linux-0a8x.site"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">+#if CONFIG_USE_OPTION_TABLE
+#include <pc80/mc146818rtc.h>
</pre>
</div>
</blockquote>
Since you start using cmos in the bootblock, you might have to
consider enabling RCBA and the upper 128 bytes of CMOS in some Intel
southbridges' bootblock.c<br>
<br>
<blockquote cite="mid:1296566674.6489.78.camel@linux-0a8x.site"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">+static void sanitize_cmos(void)
+{
+ if (cmos_error() || !cmos_chksum_valid()) {
</pre>
</div>
</blockquote>
Is this reliably working on hardware? I remember cmos_error being
flaky on ICH7 early on at some point.<br>
<br>
<blockquote cite="mid:1296566674.6489.78.camel@linux-0a8x.site"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">diff --git a/src/pc80/mc146818rtc_early.c b/src/pc80/mc146818rtc_early.c
index 920deda..d09d6b9 100644
--- a/src/pc80/mc146818rtc_early.c
+++ b/src/pc80/mc146818rtc_early.c
@@ -11,15 +11,6 @@
#error "CONFIG_MAX_REBOOT_CNT too high"
#endif
-#if CONFIG_USE_CMOS_RECOVERY
-#include <cbfs.h>
-#include <console/loglevel.h>
-
-int do_printk(int msg_level, const char *fmt, ...) __attribute__((format(printf, 2, 3)));
-#define printk_warning(fmt, arg...) do_printk(BIOS_WARNING ,fmt, ##arg)
-#define printk_debug(fmt, arg...) do_printk(BIOS_DEBUG ,fmt, ##arg)
-#endif
-
static int cmos_error(void)
{
unsigned char reg_d;
@@ -63,25 +54,6 @@ static inline int do_normal_boot(void)
unsigned char byte;
if (cmos_error() || !cmos_chksum_valid()) {
-#if CONFIG_USE_CMOS_RECOVERY
- char *cmos_default = cbfs_find_file("cmos.default", CBFS_COMPONENT_CMOS_DEFAULT);
- if (cmos_default) {
- int i;
- printk_warning("WARNING - CMOS CORRUPTED. RESTORING DEFAULTS.\n");
- /* First 14 bytes are reserved for
- RTC and ignored by nvramtool, too.
- Only 128 bytes: 128+ requires cmos configuration and
- contains only suspend-to-ram data, which isn't part
- of the recovery procedure. */
- for (i = 14; i < 128; i++) {
- cmos_write(cmos_default[i], i);
- }
- /* Now reboot to run with default cmos. */
- outb(0x06, 0xcf9);
- for (;;) asm("hlt"); /* Wait for reset! */
- }
-#endif
-
</pre>
</div>
</blockquote>
How does cmos recovery behave on non-CAR systems if this is removed?
It would be nice to if we could make sure it works everywhere<br>
<br>
<br>
<br>
</body>
</html>