Messages in this thread Patch in this message | | | Date | Fri, 29 Aug 2008 14:21:34 +0100 (BST) | From | Hugh Dickins <> | Subject | Re: [PATCH RFC] x86: check for and defend against BIOS memory corruption |
| |
On Fri, 29 Aug 2008, Alan Jenkins wrote: > Rafał Miłecki wrote: > > > > I tried your patch anyway (after applying Jeremy's patch of course) > > and it doesn't seem to work. The only output is: > > scanning 2 areas for BIOS corruption > > after using s2ram. I do not get any > > Corrupted low memory at* > > > It seemed to work for me. Did you remember to plug HDMI to trigger the > corruption before you used s2ram?
I hope that's what got missed.
Here's my version of Jeremy's patch, that I've now tested on my machines, as x86_32 and as x86_64. It addresses none of the points Alan Cox made, and it stays silent for me, even after suspend+resume, unless I actually introduce corruption myself. Omits Jeremy's check in fault.c, but does a check every minute, so should soon detect Rafał's HDMI corruption without any need to suspend+resume.
Hugh
--- 2.6.27-rc5/Documentation/kernel-parameters.txt 2008-08-29 01:02:34.000000000 +0100 +++ linux/Documentation/kernel-parameters.txt 2008-08-29 11:17:16.000000000 +0100 @@ -360,6 +360,11 @@ and is between 256 and 4096 characters. Format: <io>,<irq>,<mode> See header of drivers/net/hamradio/baycom_ser_hdx.c. + bios_corruption_check=0/1 [X86] + Some BIOSes seem to corrupt the first 64k of memory + when doing things like suspend/resume. Setting this + option will scan the memory looking for corruption. + boot_delay= Milliseconds to delay each printk during boot. Values larger than 10 seconds (10000) are changed to no delay (0). --- 2.6.27-rc5/arch/x86/Kconfig 2008-08-29 01:02:35.000000000 +0100 +++ linux/arch/x86/Kconfig 2008-08-29 11:17:16.000000000 +0100 @@ -201,6 +201,9 @@ config X86_TRAMPOLINE depends on X86_SMP || (X86_VOYAGER && SMP) || (64BIT && ACPI_SLEEP) default y +config X86_CHECK_BIOS_CORRUPTION + def_bool y + config KTIME_SCALAR def_bool X86_32 source "init/Kconfig" --- 2.6.27-rc5/arch/x86/kernel/setup.c 2008-08-29 01:02:35.000000000 +0100 +++ linux/arch/x86/kernel/setup.c 2008-08-29 13:50:19.000000000 +0100 @@ -579,6 +579,106 @@ static struct x86_quirks default_x86_qui struct x86_quirks *x86_quirks __initdata = &default_x86_quirks; /* + * Some BIOSes seem to corrupt the low 64k of memory during events + * like suspend/resume and unplugging an HDMI cable. Reserve all + * remaining free memory in that area and fill it with a distinct + * pattern. + */ +#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION +#define MAX_SCAN_AREAS 8 +static struct e820entry scan_areas[MAX_SCAN_AREAS]; +static int num_scan_areas; + +static void __init setup_bios_corruption_check(void) +{ + u64 addr = PAGE_SIZE; /* assume first page is reserved anyway */ + + while (addr < 0x10000 && num_scan_areas < MAX_SCAN_AREAS) { + u64 size; + addr = find_e820_area_size(addr, &size, PAGE_SIZE); + + if (addr == 0) + break; + + if ((addr + size) > 0x10000) + size = 0x10000 - addr; + + if (size == 0) + break; + + e820_update_range(addr, size, E820_RAM, E820_RESERVED); + scan_areas[num_scan_areas].addr = addr; + scan_areas[num_scan_areas].size = size; + num_scan_areas++; + + /* Assume we've already mapped this early memory */ + memset(__va(addr), 0, size); + + addr += size; + } + + printk(KERN_INFO "scanning %d areas for BIOS corruption\n", + num_scan_areas); + update_e820(); +} + +static int __read_mostly bios_corruption_check = 1; +static struct timer_list periodic_check_timer; + +void check_for_bios_corruption(void) +{ + int i; + int corruption = 0; + + if (!bios_corruption_check) + return; + + for (i = 0; i < num_scan_areas; i++) { + unsigned int *addr = __va(scan_areas[i].addr); + unsigned long size = scan_areas[i].size; + + for (; size; addr++, size -= sizeof(unsigned int)) { + if (!*addr) + continue; + printk(KERN_ERR "Corrupted low memory at %p (%lx phys) = %08x\n", + addr, __pa(addr), *addr); + *addr = 0; + corruption = 1; + } + } + + if (corruption) + dump_stack(); +} + +static void periodic_check_for_corruption(unsigned long data) +{ + check_for_bios_corruption(); + mod_timer(&periodic_check_timer, jiffies + 60*HZ); +} + +void start_periodic_check_for_corruption(void) +{ + if (!bios_corruption_check) + return; + + init_timer(&periodic_check_timer); + periodic_check_timer.function = &periodic_check_for_corruption; + periodic_check_for_corruption(0); +} + +static int set_bios_corruption_check(char *arg) +{ + char *end; + + bios_corruption_check = simple_strtol(arg, &end, 10); + + return (*end == 0) ? 0 : -EINVAL; +} +early_param("bios_corruption_check", set_bios_corruption_check); +#endif + +/* * Determine if we were loaded by an EFI loader. If so, then we have also been * passed the efi memmap, systab, etc., so we should use these data structures * for initialization. Note, the efi init code path is determined by the @@ -750,6 +850,10 @@ void __init setup_arch(char **cmdline_p) high_memory = (void *)__va(max_pfn * PAGE_SIZE - 1) + 1; #endif +#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION + setup_bios_corruption_check(); +#endif + /* max_pfn_mapped is updated here */ max_low_pfn_mapped = init_memory_mapping(0, max_low_pfn<<PAGE_SHIFT); max_pfn_mapped = max_low_pfn_mapped; --- 2.6.27-rc5/arch/x86/mm/init_32.c 2008-07-29 04:24:15.000000000 +0100 +++ linux/arch/x86/mm/init_32.c 2008-08-29 11:52:21.000000000 +0100 @@ -907,6 +907,8 @@ void __init mem_init(void) int codesize, reservedpages, datasize, initsize; int tmp; + start_periodic_check_for_corruption(); + #ifdef CONFIG_FLATMEM BUG_ON(!mem_map); #endif --- 2.6.27-rc5/arch/x86/mm/init_64.c 2008-08-29 01:02:35.000000000 +0100 +++ linux/arch/x86/mm/init_64.c 2008-08-29 11:53:09.000000000 +0100 @@ -769,6 +769,8 @@ void __init mem_init(void) { long codesize, reservedpages, datasize, initsize; + start_periodic_check_for_corruption(); + pci_iommu_alloc(); /* clear_bss() already clear the empty_zero_page */ --- 2.6.27-rc5/drivers/base/power/main.c 2008-08-29 01:02:35.000000000 +0100 +++ linux/drivers/base/power/main.c 2008-08-29 11:17:16.000000000 +0100 @@ -254,6 +254,7 @@ static char *pm_verb(int event) static void pm_dev_dbg(struct device *dev, pm_message_t state, char *info) { + check_for_bios_corruption(); dev_dbg(dev, "%s%s%s\n", info, pm_verb(state.event), ((state.event & PM_EVENT_SLEEP) && device_may_wakeup(dev)) ? ", may wakeup" : ""); --- 2.6.27-rc5/include/linux/kernel.h 2008-08-13 04:14:50.000000000 +0100 +++ linux/include/linux/kernel.h 2008-08-29 11:49:23.000000000 +0100 @@ -240,6 +240,22 @@ extern const char *print_tainted(void); extern void add_taint(unsigned); extern int root_mountflags; +#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION +/* + * This is obviously not a great place for this, but we want to be + * able to scatter it around anywhere in the kernel. + */ +void check_for_bios_corruption(void); +void start_periodic_check_for_corruption(void); +#else +static inline void check_for_bios_corruption(void) +{ +} +static inline void start_periodic_check_for_corruption(void) +{ +} +#endif + /* Values used for system_state */ extern enum system_states { SYSTEM_BOOTING, | |