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

From: matthieu castet
Date: Fri Dec 24 2010 - 12:27:00 EST


Hi,

Le Tue, 30 Nov 2010 13:00:30 +0800,
Lin Ming <ming.m.lin@xxxxxxxxx> 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@xxxxxxx> a écrit :
> >
> > > Le Mon, 22 Nov 2010 17:42:47 +0100,
> > > Andi Kleen <andi@xxxxxxxxxxxxxx> 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
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();