* Re: [PATCH] S3 suspend/resume with noexec
[not found] <20041005172757.A31514@unix-os.sc.intel.com>
@ 2004-10-06 8:52 ` Pavel Machek
0 siblings, 0 replies; 5+ messages in thread
From: Pavel Machek @ 2004-10-06 8:52 UTC (permalink / raw)
To: Venkatesh Pallipadi; +Cc: akpm, len.brown, linux-kernel
Hi!
> This patch is required for S3 suspend-resume to work on noexec capable systems.
> On these systems, we need to save and restore MSR_EFER during
> suspend-resume.
Hmm, I'm afraid similar patch will be needed for x86-64...
> --- linux-2.6.9-rc2/arch/i386/power/cpu.c.org 2004-09-01 19:05:41.997927104 -0700
> +++ linux-2.6.9-rc2/arch/i386/power/cpu.c 2004-09-02 19:12:54.318407912 -0700
> @@ -62,6 +62,10 @@ void __save_processor_state(struct saved
> asm volatile ("movl %%cr0, %0" : "=r" (ctxt->cr0));
> asm volatile ("movl %%cr2, %0" : "=r" (ctxt->cr2));
> asm volatile ("movl %%cr3, %0" : "=r" (ctxt->cr3));
> +#ifdef CONFIG_X86_PAE
> + if (cpu_has_pae && cpu_has_nx)
> + rdmsr(MSR_EFER, ctxt->efer_lo, ctxt->efer_hi);
> +#endif
> asm volatile ("movl %%cr4, %0" : "=r" (ctxt->cr4));
> }
Are those #ifdefs good idea?
> --- linux-2.6.9-rc2/arch/i386/kernel/acpi/wakeup.S.org 2004-09-01 21:02:14.639883944 -0700
> +++ linux-2.6.9-rc2/arch/i386/kernel/acpi/wakeup.S 2004-09-02 21:35:02.791882872 -0700
> @@ -59,6 +59,20 @@ wakeup_code:
> movl $swapper_pg_dir-__PAGE_OFFSET, %eax
> movl %eax, %cr3
>
> + testl $1, real_efer_save_restore - wakeup_code
> + jz 4f
> + # restore efer setting
> + pushl %eax
> + pushl %ecx
> + pushl %edx
> + movl real_save_efer_edx - wakeup_code, %edx
> + movl real_save_efer_eax - wakeup_code, %eax
> + mov $0xc0000080, %ecx
> + wrmsr
> + popl %edx
> + popl %ecx
> + popl %eax
> +4:
> # make sure %cr4 is set correctly (features, etc)
> movl real_save_cr4 - wakeup_code, %eax
> movl %eax, %cr4
Please analyse surrounding code a bit more. You certainly do not need
to push %eax, because it is scratch register.
Is it neccessary to restore efer this soon, btw? We should be running
from swapper_pg_dir. Does that use nx?
> @@ -223,6 +240,26 @@ ENTRY(acpi_copy_wakeup_routine)
> sldt saved_ldt
> str saved_tss
>
> + movl efer_save_restore, %edx
> + movl %edx, real_efer_save_restore - wakeup_start (%eax)
> + testl $1, real_efer_save_restore - wakeup_start (%eax)
> + jz 2f
> + # save efer setting
> + pushl %eax
> + pushl %ebx
> + pushl %ecx
> + pushl %edx
> + movl %eax, %ebx
> + mov $0xc0000080, %ecx
> + rdmsr
> + movl %edx, real_save_efer_edx - wakeup_start (%ebx)
> + movl %eax, real_save_efer_eax - wakeup_start (%ebx)
> + popl %edx
> + popl %ecx
> + popl %ebx
> + popl %eax
> +2:
> +
> movl %cr3, %edx
> movl %edx, real_save_cr3 - wakeup_start (%eax)
> movl %cr4, %edx
Again, no need to push %edx, it is used as a scratch register.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] S3 suspend/resume with noexec
@ 2004-10-06 18:28 Pallipadi, Venkatesh
2004-10-06 22:32 ` Pavel Machek
0 siblings, 1 reply; 5+ messages in thread
From: Pallipadi, Venkatesh @ 2004-10-06 18:28 UTC (permalink / raw)
To: Pavel Machek; +Cc: akpm, Brown, Len, linux-kernel
>-----Original Message-----
>From: Pavel Machek [mailto:pavel@suse.cz]
>Sent: Wednesday, October 06, 2004 1:52 AM
>To: Pallipadi, Venkatesh
>Cc: akpm@osdl.org; Brown, Len; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH] S3 suspend/resume with noexec
>
>Hi!
>
>> This patch is required for S3 suspend-resume to work on
>noexec capable systems.
>> On these systems, we need to save and restore MSR_EFER during
>> suspend-resume.
>
>Hmm, I'm afraid similar patch will be needed for x86-64...
Yes. I am working on that. But, I don't yet have a system to
test that.
>> --- linux-2.6.9-rc2/arch/i386/power/cpu.c.org
>2004-09-01 19:05:41.997927104 -0700
>> +++ linux-2.6.9-rc2/arch/i386/power/cpu.c 2004-09-02
>19:12:54.318407912 -0700
>> @@ -62,6 +62,10 @@ void __save_processor_state(struct saved
>> asm volatile ("movl %%cr0, %0" : "=r" (ctxt->cr0));
>> asm volatile ("movl %%cr2, %0" : "=r" (ctxt->cr2));
>> asm volatile ("movl %%cr3, %0" : "=r" (ctxt->cr3));
>> +#ifdef CONFIG_X86_PAE
>> + if (cpu_has_pae && cpu_has_nx)
>> + rdmsr(MSR_EFER, ctxt->efer_lo, ctxt->efer_hi);
>> +#endif
>> asm volatile ("movl %%cr4, %0" : "=r" (ctxt->cr4));
>> }
>
>Are those #ifdefs good idea?
That CONFIG is required. Cpu_has_pae and cpu_has_nx doesn't
imply that we are using nx. Only when PAE kernel is configured
and these features are available, we use it.
Looking at the code again, I can remove that CONFIG_X86_PAE,
if I use an additional check for nx_enabled.
>> --- linux-2.6.9-rc2/arch/i386/kernel/acpi/wakeup.S.org
>2004-09-01 21:02:14.639883944 -0700
>> +++ linux-2.6.9-rc2/arch/i386/kernel/acpi/wakeup.S
>2004-09-02 21:35:02.791882872 -0700
>> @@ -59,6 +59,20 @@ wakeup_code:
>> movl $swapper_pg_dir-__PAGE_OFFSET, %eax
>> movl %eax, %cr3
>>
>> + testl $1, real_efer_save_restore - wakeup_code
>> + jz 4f
>> + # restore efer setting
>> + pushl %eax
>> + pushl %ecx
>> + pushl %edx
>> + movl real_save_efer_edx - wakeup_code, %edx
>> + movl real_save_efer_eax - wakeup_code, %eax
>> + mov $0xc0000080, %ecx
>> + wrmsr
>> + popl %edx
>> + popl %ecx
>> + popl %eax
>> +4:
>> # make sure %cr4 is set correctly (features, etc)
>> movl real_save_cr4 - wakeup_code, %eax
>> movl %eax, %cr4
>
>Please analyse surrounding code a bit more. You certainly do not need
>to push %eax, because it is scratch register.
Yes. I saw that eax was not used. But, I thought it is clean to save
and restore every register that is being touched here. Just in case some
other code around these place starts using those registers in future.
If you think that is unnecessary, I will resend the patch with minimal
push/pops.
>Is it neccessary to restore efer this soon, btw? We should be running
>from swapper_pg_dir. Does that use nx?
It seems necessary for swapper_pg_dir too as we use nx even for kernel
pages (stack, module, etc).
Thanks,
Venki
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] S3 suspend/resume with noexec
2004-10-06 18:28 [PATCH] S3 suspend/resume with noexec Pallipadi, Venkatesh
@ 2004-10-06 22:32 ` Pavel Machek
2004-10-07 0:20 ` Venkatesh Pallipadi
0 siblings, 1 reply; 5+ messages in thread
From: Pavel Machek @ 2004-10-06 22:32 UTC (permalink / raw)
To: Pallipadi, Venkatesh; +Cc: akpm, Brown, Len, linux-kernel
Hi!
> >> @@ -62,6 +62,10 @@ void __save_processor_state(struct saved
> >> asm volatile ("movl %%cr0, %0" : "=r" (ctxt->cr0));
> >> asm volatile ("movl %%cr2, %0" : "=r" (ctxt->cr2));
> >> asm volatile ("movl %%cr3, %0" : "=r" (ctxt->cr3));
> >> +#ifdef CONFIG_X86_PAE
> >> + if (cpu_has_pae && cpu_has_nx)
> >> + rdmsr(MSR_EFER, ctxt->efer_lo, ctxt->efer_hi);
> >> +#endif
> >> asm volatile ("movl %%cr4, %0" : "=r" (ctxt->cr4));
> >> }
> >
> >Are those #ifdefs good idea?
>
> That CONFIG is required. Cpu_has_pae and cpu_has_nx doesn't
> imply that we are using nx. Only when PAE kernel is configured
> and these features are available, we use it.
>
> Looking at the code again, I can remove that CONFIG_X86_PAE,
> if I use an additional check for nx_enabled.
Yes, that would be better.
> >> @@ -59,6 +59,20 @@ wakeup_code:
> >> movl $swapper_pg_dir-__PAGE_OFFSET, %eax
> >> movl %eax, %cr3
> >>
> >> + testl $1, real_efer_save_restore - wakeup_code
> >> + jz 4f
> >> + # restore efer setting
> >> + pushl %eax
> >> + pushl %ecx
> >> + pushl %edx
> >> + movl real_save_efer_edx - wakeup_code, %edx
> >> + movl real_save_efer_eax - wakeup_code, %eax
> >> + mov $0xc0000080, %ecx
> >> + wrmsr
> >> + popl %edx
> >> + popl %ecx
> >> + popl %eax
> >> +4:
> >> # make sure %cr4 is set correctly (features, etc)
> >> movl real_save_cr4 - wakeup_code, %eax
> >> movl %eax, %cr4
> >
> >Please analyse surrounding code a bit more. You certainly do not need
> >to push %eax, because it is scratch register.
>
> Yes. I saw that eax was not used. But, I thought it is clean to save
> and restore every register that is being touched here. Just in case some
>
> other code around these place starts using those registers in future.
>
> If you think that is unnecessary, I will resend the patch with minimal
> push/pops.
I'd like minimal push/pops. If we pushed/poped at every possible
place, it would get too long and too unreadable too quickly.
> >Is it neccessary to restore efer this soon, btw? We should be running
> >from swapper_pg_dir. Does that use nx?
>
> It seems necessary for swapper_pg_dir too as we use nx even for kernel
> pages (stack, module, etc).
Ahha, ok.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] S3 suspend/resume with noexec
2004-10-06 22:32 ` Pavel Machek
@ 2004-10-07 0:20 ` Venkatesh Pallipadi
2004-10-07 8:03 ` Pavel Machek
0 siblings, 1 reply; 5+ messages in thread
From: Venkatesh Pallipadi @ 2004-10-07 0:20 UTC (permalink / raw)
To: Pavel Machek; +Cc: Pallipadi, Venkatesh, akpm, Brown, Len, linux-kernel
How about this patch?
I verified that x86-64 already does right things to restore MSR_EFER during
resume. So this issue is there only with i386.
Thanks,
Venki
This patch is required for S3 suspend-resume on noexec capable systems.
On these systems, we need to save and restore MSR_EFER during S3 suspend-resume.
Signed-off-by:: "Venkatesh Pallipadi" <venkatesh.pallipadi@intel.com>
--- linux-2.6.9-rc2/include/asm-i386/page.h.org 2004-09-03 21:13:03.275683336 -0700
+++ linux-2.6.9-rc2/include/asm-i386/page.h 2004-09-03 21:13:35.694754888 -0700
@@ -39,9 +39,9 @@
/*
* These are used to make use of C type-checking..
*/
+extern int nx_enabled;
#ifdef CONFIG_X86_PAE
extern unsigned long long __supported_pte_mask;
-extern int nx_enabled;
typedef struct { unsigned long pte_low, pte_high; } pte_t;
typedef struct { unsigned long long pmd; } pmd_t;
typedef struct { unsigned long long pgd; } pgd_t;
@@ -49,7 +49,6 @@ typedef struct { unsigned long long pgpr
#define pte_val(x) ((x).pte_low | ((unsigned long long)(x).pte_high << 32))
#define HPAGE_SHIFT 21
#else
-#define nx_enabled 0
typedef struct { unsigned long pte_low; } pte_t;
typedef struct { unsigned long pmd; } pmd_t;
typedef struct { unsigned long pgd; } pgd_t;
--- linux-2.6.9-rc2/arch/i386/kernel/acpi/wakeup.S.org 2004-09-01 21:02:14.639883944 -0700
+++ linux-2.6.9-rc2/arch/i386/kernel/acpi/wakeup.S 2004-09-03 21:00:45.113900984 -0700
@@ -59,6 +59,14 @@ wakeup_code:
movl $swapper_pg_dir-__PAGE_OFFSET, %eax
movl %eax, %cr3
+ testl $1, real_efer_save_restore - wakeup_code
+ jz 4f
+ # restore efer setting
+ movl real_save_efer_edx - wakeup_code, %edx
+ movl real_save_efer_eax - wakeup_code, %eax
+ mov $0xc0000080, %ecx
+ wrmsr
+4:
# make sure %cr4 is set correctly (features, etc)
movl real_save_cr4 - wakeup_code, %eax
movl %eax, %cr4
@@ -89,6 +97,9 @@ real_save_cr4: .long 0
real_magic: .long 0
video_mode: .long 0
video_flags: .long 0
+real_efer_save_restore: .long 0
+real_save_efer_edx: .long 0
+real_save_efer_eax: .long 0
bogus_real_magic:
movw $0x0e00 + 'B', %fs:(0x12)
@@ -223,6 +234,20 @@ ENTRY(acpi_copy_wakeup_routine)
sldt saved_ldt
str saved_tss
+ movl nx_enabled, %edx
+ movl %edx, real_efer_save_restore - wakeup_start (%eax)
+ testl $1, real_efer_save_restore - wakeup_start (%eax)
+ jz 2f
+ # save efer setting
+ pushl %eax
+ movl %eax, %ebx
+ mov $0xc0000080, %ecx
+ rdmsr
+ movl %edx, real_save_efer_edx - wakeup_start (%ebx)
+ movl %eax, real_save_efer_eax - wakeup_start (%ebx)
+ popl %eax
+2:
+
movl %cr3, %edx
movl %edx, real_save_cr3 - wakeup_start (%eax)
movl %cr4, %edx
--- linux-2.6.9-rc2/arch/i386/mm/init.c.org 2004-09-03 21:13:50.803458016 -0700
+++ linux-2.6.9-rc2/arch/i386/mm/init.c 2004-09-03 21:14:19.401110512 -0700
@@ -436,8 +436,8 @@ static int __init noexec_setup(char *str
__setup("noexec=", noexec_setup);
-#ifdef CONFIG_X86_PAE
int nx_enabled = 0;
+#ifdef CONFIG_X86_PAE
static void __init set_nx(void)
{
--- linux-2.6.9-rc2//include/asm-i386/suspend.h.org 2004-09-01 19:20:01.842210904 -0700
+++ linux-2.6.9-rc2//include/asm-i386/suspend.h 2004-09-02 19:12:54.317408064 -0700
@@ -18,6 +18,7 @@ arch_prepare_suspend(void)
struct saved_context {
u16 es, fs, gs, ss;
unsigned long cr0, cr2, cr3, cr4;
+ unsigned long efer_lo, efer_hi;
u16 gdt_pad;
u16 gdt_limit;
unsigned long gdt_base;
--- linux-2.6.9-rc2//arch/i386/power/cpu.c.org 2004-09-01 19:05:41.997927104 -0700
+++ linux-2.6.9-rc2//arch/i386/power/cpu.c 2004-09-03 18:58:07.934362280 -0700
@@ -62,6 +62,8 @@ void __save_processor_state(struct saved
asm volatile ("movl %%cr0, %0" : "=r" (ctxt->cr0));
asm volatile ("movl %%cr2, %0" : "=r" (ctxt->cr2));
asm volatile ("movl %%cr3, %0" : "=r" (ctxt->cr3));
+ if (nx_enabled)
+ rdmsr(MSR_EFER, ctxt->efer_lo, ctxt->efer_hi);
asm volatile ("movl %%cr4, %0" : "=r" (ctxt->cr4));
}
@@ -113,6 +115,8 @@ void __restore_processor_state(struct sa
* control registers
*/
asm volatile ("movl %0, %%cr4" :: "r" (ctxt->cr4));
+ if (nx_enabled)
+ wrmsr(MSR_EFER, ctxt->efer_lo, ctxt->efer_hi);
asm volatile ("movl %0, %%cr3" :: "r" (ctxt->cr3));
asm volatile ("movl %0, %%cr2" :: "r" (ctxt->cr2));
asm volatile ("movl %0, %%cr0" :: "r" (ctxt->cr0));
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] S3 suspend/resume with noexec
2004-10-07 0:20 ` Venkatesh Pallipadi
@ 2004-10-07 8:03 ` Pavel Machek
0 siblings, 0 replies; 5+ messages in thread
From: Pavel Machek @ 2004-10-07 8:03 UTC (permalink / raw)
To: Venkatesh Pallipadi; +Cc: akpm, Brown, Len, linux-kernel
Hi!
> How about this patch?
Looks good to me.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2004-10-07 8:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-10-06 18:28 [PATCH] S3 suspend/resume with noexec Pallipadi, Venkatesh
2004-10-06 22:32 ` Pavel Machek
2004-10-07 0:20 ` Venkatesh Pallipadi
2004-10-07 8:03 ` Pavel Machek
[not found] <20041005172757.A31514@unix-os.sc.intel.com>
2004-10-06 8:52 ` Pavel Machek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox