Messages in this thread | | | Date | Thu, 7 Oct 2004 00:32:03 +0200 | From | Pavel Machek <> | Subject | Re: [PATCH] S3 suspend/resume with noexec |
| |
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! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |