Re: -tip tree resume fail, bisect to 5bd5a45(x86: Add NX protection for kernel data)

[matthieu castet <castet.matthieu@free.fr>]

[-- Attachment #1: Type: text/plain, Size: 1659 bytes --]

Hi,

Le Tue, 30 Nov 2010 13:00:30 +0800,
Lin Ming <ming.m.lin@intel.com> a écrit :

> On Sat, 2010-11-27 at 01:31 +0800, mat wrote:
> > Le Tue, 23 Nov 2010 23:55:27 +0100,
> > mat <castet.matthieu@free.fr> a écrit :
> > 
> > > Le Mon, 22 Nov 2010 17:42:47 +0100,
> > > Andi Kleen <andi@firstfloor.org> a écrit :
> > > 
> > > > > That seems to be a S3 specific code path, that won't fix
> > > > > anything. Simply do:
> > > > > 
> > > > > echo 0 > /sys/devices/system/cpu/cpu1/online;
> > > > > echo 1 > /sys/devices/system/cpu/cpu1/online;
> > > > > 
> > > > > and your machine will explode..
> > > > 
> > > > The SMP startup trampoline is copied I believe
> > > > and only executed in real mode without page tables.
> > > > 
> > > > So it's perhaps not the trampoline, but the early startup 
> > > > code that ends up being broken.
> > > yes :
> > > acpi wakeup code and smp trampoline are copied in low memory
> > > (first 1MB).
> > > 
> > > So they can't end up int the kernel data mapping ?
> > > 
> > > So it should something else.
> > > 
> > > I will try to investigate on this.
> > > 
> > Unfortunately on my laptop supporting NX, suspend to ram seems
> > broken (even without this patch) and I got only one core, so I am
> > unable to test it.
> > 
> > Does cpu suspend/resume is broken ? Or it is only S3 ?
> > 
> > If yes, are there any interesting trace if we suspend only one core
> > with sysfs.
> 
> echo 0 > /sys/devices/system/cpu/cpu1/online;
> echo 1 > /sys/devices/system/cpu/cpu1/online;
> 
> then machine just reboots...
> 
Ok,

could you try the attached patch ?

Thanks

Matthieu

[-- Attachment #2: x64_nx_data.diff --]
[-- Type: text/x-patch, Size: 895 bytes --]

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 71a5929..d86552f 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -790,6 +790,7 @@ void mark_rodata_ro(void)
 	unsigned long end = (unsigned long) &__end_rodata_hpage_align;
 	unsigned long text_end = PAGE_ALIGN((unsigned long) &__stop___ex_table);
 	unsigned long rodata_end = PAGE_ALIGN((unsigned long) &__end_rodata);
+	unsigned long data_end = PAGE_ALIGN((unsigned long) &_edata);
 	unsigned long data_start = (unsigned long) &_sdata;
 
 	printk(KERN_INFO "Write protecting the kernel read-only data: %luk\n",
@@ -802,7 +803,7 @@ void mark_rodata_ro(void)
 	 * The rodata section (but not the kernel text!) should also be
 	 * not-executable.
 	 */
-	set_memory_nx(rodata_start, (end - rodata_start) >> PAGE_SHIFT);
+	set_memory_nx(rodata_start, (data_end - rodata_start) >> PAGE_SHIFT);
 
 	rodata_test();