Skip to contentSkip to conversation

NX protection for kernel data : fix 32 bits S3 suspend

PATCH In review matthieu castet 7 participants 24 messages Jan 31, 2011 – Feb 28, 2011

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 NX protection for kernel data : fix 32 bits S3 suspend

I think it should be applied before 2.6.38 release, because without this patch S3 suspend doesn't work on x86_32 with CONFIG_DEBUG_RODATA.

Quoted 1 line

From a8d56e665c9b26c953f355b6e8eeeecafa07efdb Mon Sep 17 00:00:00 2001

From: Matthieu CASTET <[email protected]>
Date: Thu, 27 Jan 2011 21:36:07 +0100
Subject: [PATCH] NX protection for kernel data : fix 32 bits S3 suspend

32 bits wakeup realmode trampoline enable paging, while still in low memory.

We should make this memory !NX in order it works.

---
 arch/x86/mm/init_32.c  |    8 ++++++++
 arch/x86/mm/pageattr.c |    7 +++++++
 2 files changed, 15 insertions(+), 0 deletions(-)
arch/x86/mm/init_32.c +8 -0
index c821074..0048738 100644
@@ -227,6 +227,14 @@ page_table_range_init(unsigned long start, unsigned long end, pgd_t *pgd_base)
 
 static inline int is_kernel_text(unsigned long addr)
 {
+#if defined(CONFIG_X86_32) && defined(CONFIG_ACPI_SLEEP)
+	/*
+	 * We need to make the wakeup trampoline in first 1MB !NX
+	 */
+	if (addr >= PAGE_OFFSET && addr <= (PAGE_OFFSET + (1<<20)))
+		return 1;
+#endif
+
 	if (addr >= (unsigned long)_text && addr <= (unsigned long)__init_end)
 		return 1;
 	return 0;
arch/x86/mm/pageattr.c +7 -0
index d343b3c..f1d6cf5 100644
@@ -265,6 +265,13 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
 	if (pcibios_enabled && within(pfn, BIOS_BEGIN >> PAGE_SHIFT, BIOS_END >> PAGE_SHIFT))
 		pgprot_val(forbidden) |= _PAGE_NX;
 #endif
+	/*
+	 * We need to make the wakeup trampoline in first 1MB !NX
+	 */
+#if defined(CONFIG_X86_32) && defined(CONFIG_ACPI_SLEEP)
+	if (within(address, PAGE_OFFSET, PAGE_OFFSET + (1<<20)))
+		pgprot_val(forbidden) |= _PAGE_NX;
+#endif
 
 	/*
 	 * The kernel text needs to be executable for obvious reasons
Signature · 1 line
1.7.2.3
#1 lore raw

arch/x86/mm/init_32.c page_table_range_init(unsigned long start, unsigned long end, pgd_t *pgd_base) · line 236

Quoted 9 lines · matthieu castet

static inline int is_kernel_text(unsigned long addr) {

arch/x86/mm/init_32.c +7 -0
+#if defined(CONFIG_X86_32) && defined(CONFIG_ACPI_SLEEP)
+	/*
+	 * We need to make the wakeup trampoline in first 1MB !NX
+	 */
+	if (addr >= PAGE_OFFSET && addr <= (PAGE_OFFSET + (1<<20)))
+		return 1;
+#endif

That's pretty ugly. Why not use set_memory_x()/set_memory_nx(), and only for the trampoline itself? Does the whole 1MB need to be marked X?

Same goes for the Xen fix.

Thanks,

Ingo

#2 lore raw

Quoting Ingo Molnar <[email protected]>:

Quoted 16 lines · matthieu castet

arch/x86/mm/init_32.c page_table_range_init(unsigned long start, unsigned long end, pgd_t *pgd_base) · line 236

Quoted 10 lines

static inline int is_kernel_text(unsigned long addr) {

arch/x86/mm/init_32.c +7 -0
+#if defined(CONFIG_X86_32) && defined(CONFIG_ACPI_SLEEP)
+	/*
+	 * We need to make the wakeup trampoline in first 1MB !NX
+	 */
+	if (addr >= PAGE_OFFSET && addr <= (PAGE_OFFSET + (1<<20)))
+		return 1;
+#endif

That's pretty ugly. Why not use set_memory_x()/set_memory_nx(), and only for the
trampoline itself? Does the whole 1MB need to be marked X?

The previous code was doing that.

If you prefer I can revert to the old code :

static inline int is_kernel_text(unsigned long addr)

{
    if (addr >= PAGE_OFFSET && addr <= (unsigned long)__init_end)
        return 1;
    return 0;
}

The proper fix will be done in 2.6.29, when S3 resume 32 bit trampoline will be merged with 64 bit (ie call smp trampoline) [1]

Quoted 3 lines

Same goes for the Xen fix.

The Xen fix reverts code to old behavior.
We can't assume that all data/bss will be RW. And I see no way to detect witch page in data/bss we should not force to RW in static protection. So we assume the code that change protection of data/bss knows what it is doing (that what was doing old code).

Matthieu

[1]http://marc.info/?l=linux-kernel&m=129616541603610&w=2

#3 lore raw
Quoted 4 lines · On 02/01/2011 05:25 AM, [email protected] wrote:

The proper fix will be done in 2.6.29, when S3 resume 32 bit trampoline will be merged with 64 bit (ie call smp trampoline) [1]

That's insufficient; the actual unified lowmem trampoline allocator should take care of that bit per se.

However, the bottom line seems to be that this patch was done with some extreme lack of care. To really be proper, every allocation from the get go -- including the brk and memblock based ones -- should be tagged with their execution status, and that information should be used, not some magic ranges.

-hpa

#4 lore raw
In reply to [email protected]
Quoted 19 lines · * [email protected] <[email protected]> wrote:

Quoting Ingo Molnar <[email protected]>:

Quoted 16 lines · matthieu castet

arch/x86/mm/init_32.c page_table_range_init(unsigned long start, unsigned long end, pgd_t *pgd_base) · line 236

Quoted 10 lines

static inline int is_kernel_text(unsigned long addr) {

arch/x86/mm/init_32.c +7 -0
+#if defined(CONFIG_X86_32) && defined(CONFIG_ACPI_SLEEP)
+	/*
+	 * We need to make the wakeup trampoline in first 1MB !NX
+	 */
+	if (addr >= PAGE_OFFSET && addr <= (PAGE_OFFSET + (1<<20)))
+		return 1;
+#endif

That's pretty ugly. Why not use set_memory_x()/set_memory_nx(), and only for the
trampoline itself? Does the whole 1MB need to be marked X?

Quoted 1 line

The previous code was doing that.

So why not call set_memory_x() in your patch? Mind trying that?

Thanks,

Ingo

#5 lore raw
Quoted 22 lines · Ingo Molnar
Quoted 19 lines · * [email protected] <[email protected]> wrote:

Quoting Ingo Molnar <[email protected]>:

Quoted 16 lines · matthieu castet

arch/x86/mm/init_32.c page_table_range_init(unsigned long start, unsigned long end, pgd_t *pgd_base) · line 236

Quoted 10 lines

static inline int is_kernel_text(unsigned long addr) {

arch/x86/mm/init_32.c +7 -0
+#if defined(CONFIG_X86_32) && defined(CONFIG_ACPI_SLEEP)
+	/*
+	 * We need to make the wakeup trampoline in first 1MB !NX
+	 */
+	if (addr >= PAGE_OFFSET && addr <= (PAGE_OFFSET + (1<<20)))
+		return 1;
+#endif

That's pretty ugly. Why not use set_memory_x()/set_memory_nx(), and only for the
trampoline itself? Does the whole 1MB need to be marked X?

Quoted 4 lines
Quoted 1 line

The previous code was doing that.

So why not call set_memory_x() in your patch? Mind trying that?

OK, there seems to be considerable duplication between static_protections() and local invocation. Consider PCI BIOS, which is another X-needed region.

In static_protections() we have:

Quoted 8 lines

/*
* The BIOS area between 640k and 1Mb needs to be executable for
* PCI BIOS based config access (CONFIG_PCI_GOBIOS) support. */

#ifdef CONFIG_PCI_BIOS
        if (pcibios_enabled && within(pfn, BIOS_BEGIN >> PAGE_SHIFT, BIOS_END >> PAGE_SHIFT))
                pgprot_val(forbidden) |= _PAGE_NX;

#endif

... however, in arch/x86/pci/pcbios.c:

Quoted 7 lines

static inline void set_bios_x(void)

{
        pcibios_enabled = 1;
        set_memory_x(PAGE_OFFSET + BIOS_BEGIN, (BIOS_END - BIOS_BEGIN) >> PAGE_SHIFT);
        if (__supported_pte_mask & _PAGE_NX)
                printk(KERN_INFO "PCI : PCI BIOS aera is rw and x. Use pci=nobios if you want it NX.\n");
}

This is blatant and insanely ugly code duplication! In particular, static_protections() is "action at a distance" which has no business existing at all.

What I want to know is if static_protections() can somehow override set_bios_x() in this context (in which case it's a serious design error), or if it is plain redundant -- in the latter case we should simply use the same technique elsewhere.

-hpa

#6 lore raw
In reply to Ingo Molnar
Quoted 6 lines · Ingo Molnar

So why not call set_memory_x() in your patch? Mind trying that?

Thanks,

Ingo

So I just tried that... it doesn't work. The resulting pages still end up NX:

---[ Kernel Mapping ]---
0xc0000000-0xc00a0000 640K RW GLB NX pte

This implies that the NX protection is applied after these allocations happen, which is probably why the ugly hack in static_protections() to set the PCI BIOS +x is there as well.

I have to admit to being rather at a loss for where in the boot sequence the NX mappings get set up, despite staring at the code for some time. mark_nxdata_nx() seems to only mark the kernel data/rodata and init region... even though the latter is already done in free_init_pages(). It is also run way, way, way too late in the process -- why on earth should we not have these protections during the driver initialization phase of booting?

I talked to Suresh about the whole static_protections() bit, and as far as he recalls it is because the entire set_memory_*() interface is misdesigned to work on all aliases of a page, despite the fact that protections are per mapping, not per physical page.

However, this isn't something we can fix for .38... I suspect unprotecting the entire 0-640K region might just make most sense, and then we need to do some serious restructuring of the entire handling of this stuff, because it's broken seven ways to Sunday.

-hpa

#7 lore raw

Selon "H. Peter Anvin" <[email protected]>:

Quoted 11 lines · Ingo Molnar
Quoted 6 lines

So why not call set_memory_x() in your patch? Mind trying that?

Thanks,

Ingo

So I just tried that... it doesn't work. The resulting pages still end up NX:

Quoted 6 lines

---[ Kernel Mapping ]---
0xc0000000-0xc00a0000 640K RW GLB NX pte

This implies that the NX protection is applied after these allocations happen, which is probably why the ugly hack in static_protections() to set the PCI BIOS +x is there as well.

You could remove PCI BIOS +x hack in static protection, and the x mapping will be set by set_memory_x().

The problem is that acpi_reserve_wakeup_memory is called too early, before we build the page table with kernel_physical_mapping_init.

Doing the setting in a arch_initcall make it work.

Quoted 5 lines

I talked to Suresh about the whole static_protections() bit, and as far as he recalls it is because the entire set_memory_*() interface is misdesigned to work on all aliases of a page, despite the fact that protections are per mapping, not per physical page.

The only stuff I understood of static_protections is the comment on top of it :
mapping of bios/kernel region should be on it otherwise, some callers can mess the protection flags.

Matthieu

#8 lore raw
Quoted 7 lines · On 02/05/2011 08:46 AM, [email protected] wrote:

You could remove PCI BIOS +x hack in static protection, and the x mapping will be set by set_memory_x().

The problem is that acpi_reserve_wakeup_memory is called too early, before we build the page table with kernel_physical_mapping_init.

Doing the setting in a arch_initcall make it work.

No, the problem is that the code is braindamaged and don't take into account reserved areas or have a mechanism for marking the reserved areas so that kernel_physical_mapping_init can do the right thing... and then it's hacked around instead of done properly.

We obviously need to reserve this memory very early in order to make sure it exists, and init_memory_mapping() -> kernel_physical_mapping_init() really should be able to deal with that (for example by walking the list of reserved memory regions and look which ones of them should have specific protection bits -- not just NX -- set appropriately.)

The trampoline unification patch could have made this less broken, but that code is certainly not ready for .38.

I'm officially proposing that we mark the low 1 MiB +x for .38, and do something saner for .39, including my finishing the trampoline unification code.

Ingo: ok with you?

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#9 lore raw
Quoted 8 lines · H. Peter Anvin

The trampoline unification patch could have made this less broken, but that code is certainly not ready for .38.

I'm officially proposing that we mark the low 1 MiB +x for .38, and do something saner for .39, including my finishing the trampoline unification code.

Ingo: ok with you?

Sure thing, sounds sane!

Thanks,

Ingo

#10 lore raw
In reply to H. Peter Anvin

Thanks for cleaning the patch.

Selon "H. Peter Anvin" <[email protected]>:

Quoted 14 lines · On 02/05/2011 08:46 AM, [email protected] wrote:

No, the problem is that the code is braindamaged and don't take into account reserved areas or have a mechanism for marking the reserved areas so that kernel_physical_mapping_init can do the right thing... and then it's hacked around instead of done properly.

We obviously need to reserve this memory very early in order to make sure it exists, and init_memory_mapping() -> kernel_physical_mapping_init() really should be able to deal with that (for example by walking the list of reserved memory regions and look which ones of them should have specific protection bits -- not just NX -- set appropriately.)

Quoted 3 lines

The trampoline unification patch could have made this less broken, but that code is certainly not ready for .38.

For .39 I hope we could remove most of the RWX rights after init (This means make low memory trampoline NX or !RW).
This should be possible on :
- 32 bit if wakeup use trampoline_32 [1] that doesn't enable paging in low memory (can be NX)
- trampoline_64 need fix to support NX on data section. It tries to read data section before enabling NX. A possible fix is to use its own page table [2]. And the kernel one can be NX.

Matthieu

[1]
http://marc.info/?l=linux-acpi&m=129616540303575&w=2

[2]
http://marc.info/?l=linux-kernel&m=129590778414274&w=2

#11 lore raw
Quoted 8 lines · On 02/07/2011 11:59 AM, [email protected] wrote:

For .39 I hope we could remove most of the RWX rights after init (This means make low memory trampoline NX or !RW).
This should be possible on :
- 32 bit if wakeup use trampoline_32 [1] that doesn't enable paging in low memory (can be NX)
- trampoline_64 need fix to support NX on data section. It tries to read data section before enabling NX. A possible fix is to use its own page table [2]. And the kernel one can be NX.

No, you're really barking down the wrong path on this. The trampoline code is tiny; I don't think it is really worth trying to NX-ify it. The additional complexity caused by not being able to execute in this space will really damage some other incoming code, so it isn't an option as far as I'm concerned.

-hpa

#12 lore raw
Quoted 10 lines · H. Peter Anvin
Quoted 8 lines · On 02/07/2011 11:59 AM, [email protected] wrote:

For .39 I hope we could remove most of the RWX rights after init (This means make low memory trampoline NX or !RW).
This should be possible on :
- 32 bit if wakeup use trampoline_32 [1] that doesn't enable paging in low memory (can be NX)
- trampoline_64 need fix to support NX on data section. It tries to read data section before enabling NX. A possible fix is to use its own page table [2]. And the kernel one can be NX.

Quoted 2 lines

No, you're really barking down the wrong path on this. The trampoline code is tiny; I don't think it is really worth trying to NX-ify it. The

Even if the trampoline is tiny, a hole is a hole.

The trampoline code job is to jump from low memory (realmode) to somewhere in kernel text. Why should we enable paging or use kernel page table for doing that ?

Quoted 4 lines

additional complexity caused by not being able to execute in this space will really damage some other incoming code, so it isn't an option as far as I'm concerned.

What do you plan to add that won't be compatible with that ?

Matthieu

#13 lore raw
Quoted 9 lines · matthieu castet
Quoted 3 lines

No, you're really barking down the wrong path on this. The trampoline code is tiny; I don't think it is really worth trying to NX-ify it. The

Even if the trampoline is tiny, a hole is a hole.

The trampoline code job is to jump from low memory (realmode) to somewhere in kernel text.
Why should we enable paging or use kernel page table for doing that ?

That's not the problem. The problem is that most of the "trampoline codes" need parameters which need to be written by the kernel before invocation. Separating the address spaces out into text and writable-for-the-kernel data is possible, but messy since the individual chunks each have different addressing constraints. I tried to get them into the same link, but that has turned out to be very difficult and probably will require a fair bit of restructuring, especially the code shared with the boot code.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#14 lore raw
In reply to matthieu castet
Quoted 11 lines · matthieu castet
Quoted 10 lines · H. Peter Anvin
Quoted 8 lines · On 02/07/2011 11:59 AM, [email protected] wrote:

For .39 I hope we could remove most of the RWX rights after init (This means make low memory trampoline NX or !RW).
This should be possible on :
- 32 bit if wakeup use trampoline_32 [1] that doesn't enable paging in low memory (can be NX)
- trampoline_64 need fix to support NX on data section. It tries to read data section before enabling NX. A possible fix is to use its own page table [2]. And the kernel one can be NX.

Quoted 3 lines
Quoted 2 lines

No, you're really barking down the wrong path on this. The trampoline code is tiny; I don't think it is really worth trying to NX-ify it. The

Even if the trampoline is tiny, a hole is a hole.

What kind of hole are you talking about?

If evil code is running in ring0, you already lost.

I doubt you can remove all the trampolines, and I do not think you should uglify the kernel trying to do that.
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#15 lore raw
In reply to [email protected]
Quoted 8 lines · On 02/07/2011 11:59 AM, [email protected] wrote:

For .39 I hope we could remove most of the RWX rights after init (This means make low memory trampoline NX or !RW).
This should be possible on :
- 32 bit if wakeup use trampoline_32 [1] that doesn't enable paging in low memory (can be NX)
- trampoline_64 need fix to support NX on data section. It tries to read data section before enabling NX. A possible fix is to use its own page table [2]. And the kernel one can be NX.

What *should* happen -- ideally for .39 -- is that NX (and RO!) protection should be done per linear mapping, not per physical page. A page that is mapped more than once is mapped for a different purpose, and as such probably should have different permissions. A lot of the static_protections() garbage is about enforcing those as exceptions, but let's face it, that should be the *norm*.

-hpa

#16 lore raw
In reply to [email protected]
Quoted 3 lines · On 02/07/2011 11:59 AM, [email protected] wrote:

For .39 I hope we could remove most of the RWX rights after init (This means make low memory trampoline NX or !RW).

By the way, I think this is the wrong goal. I think we should have things enabled at their lowest permission level *as early as possible*.
The current model of tightening down permissions late in the boot is
really the wrong model.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#17 lore raw
Quoted 9 lines · H. Peter Anvin
Quoted 3 lines · On 02/07/2011 11:59 AM, [email protected] wrote:

For .39 I hope we could remove most of the RWX rights after init (This means make low memory trampoline NX or !RW).

By the way, I think this is the wrong goal. I think we should have things enabled at their lowest permission level *as early as possible*.
The current model of tightening down permissions late in the boot is
really the wrong model.

FWIW, I agree.

Rafael

#18 lore raw
In reply to [email protected]
Quoted 3 lines · On 02/05/2011 08:46 AM, [email protected] wrote:

Doing the setting in a arch_initcall make it work.

Thinking about it, it's probably the sanest thing to do in the very short term. I don't get the #ifdef CONFIG_X86_32 bit in your patch, though. We reserve this area even on 64 bits, and so we should logically do the same thing, if nothing else to avoid gratuitous differences.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#19 lore raw
In reply to [email protected]

Here is my cleaned-up version of Matthieu's patch... please test so we can commit it as soon as possible.

If this one doesn't work either it is time to totally revert.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
Quoted 1 line

From 0b327f122ccfbbb28f2c22834916f12d03fc2a06 Mon Sep 17 00:00:00 2001

From: H. Peter Anvin <[email protected]>
Date: Sun, 6 Feb 2011 19:58:38 -0800
Subject: [PATCH] x86, nx: Mark the ACPI resume trampoline code as +x

We reserve lowmem for the things that need it, like the ACPI wakeup code, way early to guarantee availability. This happens before we set up the proper pagetables, so set_memory_x() has no effect.

Until we have a better solution, use an initcall to mark the wakeup code executable.

LKML-Reference: <[email protected]>

---
 arch/x86/kernel/acpi/sleep.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)
arch/x86/kernel/acpi/sleep.c +10 -3
index 4d9ebba..68d1537 100644
@@ -12,10 +12,8 @@
 #include <linux/cpumask.h>
 #include <asm/segment.h>
 #include <asm/desc.h>
-
-#ifdef CONFIG_X86_32
 #include <asm/pgtable.h>
-#endif
+#include <asm/cacheflush.h>
 
 #include "realmode/wakeup.h"
 #include "sleep.h"
@@ -149,6 +147,15 @@ void __init acpi_reserve_wakeup_memory(void)
 	memblock_x86_reserve_range(mem, mem + WAKEUP_SIZE, "ACPI WAKEUP");
 }
 
+int __init acpi_configure_wakeup_memory(void)
+{
+	if (acpi_realmode)
+		set_memory_x(acpi_realmode, WAKEUP_SIZE >> PAGE_SHIFT);
+
+	return 0;
+}
+arch_initcall(acpi_configure_wakeup_memory);
+
 
 static int __init acpi_sleep_setup(char *str)
 {
Signature · 1 line
1.7.4
#20 lore raw

[tip:x86/urgent] x86, nx: Mark the ACPI resume trampoline code as +x

Commit-ID:  d344e38b2c151ca5e5e39f562017127e93912528
Gitweb:     http://git.kernel.org/tip/d344e38b2c151ca5e5e39f562017127e93912528
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Sun, 6 Feb 2011 21:16:09 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Mon, 7 Feb 2011 09:07:13 +0100

x86, nx: Mark the ACPI resume trampoline code as +x

We reserve lowmem for the things that need it, like the ACPI wakeup code, way early to guarantee availability. This happens before we set up the proper pagetables, so set_memory_x() has no effect.

Until we have a better solution, use an initcall to mark the wakeup code executable.

LKML-Reference: <[email protected]>

---
 arch/x86/kernel/acpi/sleep.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)
arch/x86/kernel/acpi/sleep.c +10 -3
index 4d9ebba..68d1537 100644
@@ -12,10 +12,8 @@
 #include <linux/cpumask.h>
 #include <asm/segment.h>
 #include <asm/desc.h>
-
-#ifdef CONFIG_X86_32
 #include <asm/pgtable.h>
-#endif
+#include <asm/cacheflush.h>
 
 #include "realmode/wakeup.h"
 #include "sleep.h"
@@ -149,6 +147,15 @@ void __init acpi_reserve_wakeup_memory(void)
 	memblock_x86_reserve_range(mem, mem + WAKEUP_SIZE, "ACPI WAKEUP");
 }
 
+int __init acpi_configure_wakeup_memory(void)
+{
+	if (acpi_realmode)
+		set_memory_x(acpi_realmode, WAKEUP_SIZE >> PAGE_SHIFT);
+
+	return 0;
+}
+arch_initcall(acpi_configure_wakeup_memory);
+
 
 static int __init acpi_sleep_setup(char *str)
 {
#21 lore raw

Re: [tip:x86/urgent] x86, nx: Mark the ACPI resume trampoline code as +x - fixed

* tip-bot for H. Peter Anvin <[email protected]> [2011-02-07 09:24:55 +0000]:

Great! That one fixes ACPI S3 resume on i7.

Regards,
Marc

Quoted 67 lines
Commit-ID:  d344e38b2c151ca5e5e39f562017127e93912528
Gitweb:     http://git.kernel.org/tip/d344e38b2c151ca5e5e39f562017127e93912528
Author:     H. Peter Anvin <[email protected]>
AuthorDate: Sun, 6 Feb 2011 21:16:09 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Mon, 7 Feb 2011 09:07:13 +0100

x86, nx: Mark the ACPI resume trampoline code as +x

We reserve lowmem for the things that need it, like the ACPI wakeup code, way early to guarantee availability. This happens before we set up the proper pagetables, so set_memory_x() has no effect.

Until we have a better solution, use an initcall to mark the wakeup code executable.

LKML-Reference: <[email protected]>

---
 arch/x86/kernel/acpi/sleep.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)
arch/x86/kernel/acpi/sleep.c +10 -4
index 4d9ebba..68d1537 100644
@@ -12,10 +12,8 @@
 #include <linux/cpumask.h>
 #include <asm/segment.h>
 #include <asm/desc.h>
-
-#ifdef CONFIG_X86_32
 #include <asm/pgtable.h>
-#endif
+#include <asm/cacheflush.h>
 
 #include "realmode/wakeup.h"
 #include "sleep.h"
@@ -149,6 +147,15 @@ void __init acpi_reserve_wakeup_memory(void)
 	memblock_x86_reserve_range(mem, mem + WAKEUP_SIZE, "ACPI WAKEUP");
 }
 
+int __init acpi_configure_wakeup_memory(void)
+{
+	if (acpi_realmode)
+		set_memory_x(acpi_realmode, WAKEUP_SIZE >> PAGE_SHIFT);
+
+	return 0;
+}
+arch_initcall(acpi_configure_wakeup_memory);
+
 
 static int __init acpi_sleep_setup(char *str)
 {
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 1 line
Marc Koschewski
#22 lore raw

Re: [tip:x86/urgent] x86, nx: Mark the ACPI resume trampoline code as +x - fixed

Quoted 3 lines · Marc Koschewski

* tip-bot for H. Peter Anvin <[email protected]> [2011-02-07 09:24:55 +0000]:

Great! That one fixes ACPI S3 resume on i7.

Ok, great - i'm sending it to Linus then.

Thanks,

Ingo

Tested-by
#23 lore raw
In reply to H. Peter Anvin
Quoted 2 lines · H. Peter Anvin

Here is my cleaned-up version of Matthieu's patch... please test so we can commit it as soon as possible.

This works as well, tested against Linus' master from today (which doesn't work).

Quoted 4 lines

From 0b327f122ccfbbb28f2c22834916f12d03fc2a06 Mon Sep 17 00:00:00 2001
From: H. Peter Anvin <[email protected]>
Date: Sun, 6 Feb 2011 19:58:38 -0800
Subject: [PATCH] x86, nx: Mark the ACPI resume trampoline code as +x

Matthias

Signature · 3 lines
Matthias Hopf <[email protected]>      __        __   __
Maxfeldstr. 5 / 90409 Nuernberg   (_   | |  (_   |__          [email protected]
Phone +49-911-74053-715           __)  |_|  __)  |__  R & D   www.mshopf.de