Skip to contentSkip to conversation

check that acpi_wakeup_address is below 1MB v2

PATCH In review matthieu castet 3 participants 4 messages Apr 10, 2008 – Apr 11, 2008

Subsystems (3)
x86 ACPI Suspend to RAM
lore
Share & tools
lore

matthieu castet

author
#0 lore raw

PATCH 1/1 check that acpi_wakeup_address is below 1MB v2

Hi,

this patch add a check that the memory allocated for s3 wakeup is in the first 1MB as required by acpi spec.

Signed-off-by "Matthieu CASTET <[email protected]>"
arch/x86/kernel/acpi/sleep.c +7 -0
index 6bc815c..6636859 100644
@@ -65,6 +65,13 @@ void __init acpi_reserve_bootmem(void)
 	acpi_wakeup_address = (unsigned long)alloc_bootmem_low(PAGE_SIZE*2);
 	if (!acpi_wakeup_address)
 		printk(KERN_ERR "ACPI: Cannot allocate lowmem, S3 disabled.\n");
+	/* check if we are in first 1MB of memory */
+	if (__pa(acpi_wakeup_address) >= 1024*1024-PAGE_SIZE*2) {
+		printk(KERN_ERR "ACPI: Cannot allocate lowmem in first 1MB,"
+				"S3 disabled.\n");
+		free_bootmem(acpi_wakeup_address, PAGE_SIZE*2);
+		acpi_wakeup_address = 0;
+	}
 }
#1 lore raw
Quoted 5 lines · matthieu castet

Hi,

this patch add a check that the memory allocated for s3 wakeup is in the first 1MB as required by acpi spec.

Have you found whether some systems are afflicted by this ?

In fact that OS allacates the acpi_wake_address follows the below two functions:
reserve_bootmem(0, PAGE_SIZE, BOOTMEM_DEFAULT); reserve_bootmem(PAGE_SIZE, PAGE_SIZE, BOOTMEM_DEFAULT);

The above mechanism can insure that the acpi_wake_address is in memroy below 1M. (It is unncessary to apply the patch. IMO)

Of course the addressing check will be more strict after this patch is applied.

Best regards
Yakui

Quoted 3 lines
Signed-off-by "Matthieu CASTET <[email protected]>"
#2 lore raw
Quoted 8 lines · Zhao Yakui
Quoted 5 lines · matthieu castet

Hi,

this patch add a check that the memory allocated for s3 wakeup is in the first 1MB as required by acpi spec.

Have you found whether some systems are afflicted by this ?

Quoted 7 lines

In fact that OS allacates the acpi_wake_address follows the below two functions:
reserve_bootmem(0, PAGE_SIZE, BOOTMEM_DEFAULT); reserve_bootmem(PAGE_SIZE, PAGE_SIZE, BOOTMEM_DEFAULT);

The above mechanism can insure that the acpi_wake_address is in memroy below 1M. (It is unncessary to apply the patch. IMO)

Agreed.

Quoted 2 lines

Of course the addressing check will be more strict after this patch is applied.

Matthieu, can you please explain why you consider the patch as necessary?

Thanks,
Rafael

author
#3 lore raw

Hi,

Quoted 6 lines · Rafael J. Wysocki
Quoted 5 lines · Zhao Yakui
Quoted 4 lines · matthieu castet

Hi,

this patch add a check that the memory allocated for s3 wakeup is in the first 1MB as required by acpi spec.

Quoted 7 lines
Quoted 7 lines

Have you found whether some systems are afflicted by this ?

In fact that OS allacates the acpi_wake_address follows the below two functions:
reserve_bootmem(0, PAGE_SIZE, BOOTMEM_DEFAULT); reserve_bootmem(PAGE_SIZE, PAGE_SIZE, BOOTMEM_DEFAULT);

That's on 32 bits kernel. Aren't the 1MB limitation valid on 64 bits kernel ?

Quoted 9 lines
Quoted 2 lines

The above mechanism can insure that the acpi_wake_address is in memroy below 1M. (It is unncessary to apply the patch. IMO)

Agreed.

Quoted 2 lines

Of course the addressing check will be more strict after this patch is applied.

Matthieu, can you please explain why you consider the patch as necessary?

This can help to debug strange S3 wakeup problem. It make sure the allocation is correct.

Also I believe relying on the bootmem allocator internal to be sure it is in the first 1MB is a bad things (why can't it start allocating memory to the top of 16MB memory). If you are sure of the allocated memory before the call, why don't do a
reserve_bootmem(PAGE_SIZE*2, PAGE_SIZE*2, BOOTMEM_DEFAULT); ?

Matthieu