Messages in this thread | | | Date | Thu, 16 Nov 2006 21:53:13 +0100 | From | Pavel Machek <> | Subject | Re: [Fastboot] [RFC] [PATCH 10/16] x86_64: 64bit PIC ACPI wakeup |
| |
Hi!
> > Ok. In the new code NX bit protection feature is not being enabled and that > > seems to be causing the problem. I checked and enabled the NX bit feature > > in EFER in wakeup.S and it starts working. > > > > I think my new machine supports NX bit protection feature and if while > > resuming if I don't enable that feature back probably it must have caused > > a GPF while loading the page tables which have got NX bit set. (A guess). > > > > I know that previous machine I was testing on does not support NX bit > > feature and that could be the reason that previous machine did not run into > > the problems. > > Fixed the resume problem happening on my second box which supported NX > protection bit. Please find attached the regenerated patch. > > - Killed lots of dead code
Cleanup. (a)
> - Improve the cpu sanity checks to verify long mode > is enabled when we wake up.
Change. (b). I'm not sure if we really need this one. I do not think replacing cpu while suspended is supported operation.
> - Removed the need for modifying any existing kernel page table.
Unrelated change, probably good one. (c).
> - Moved wakeup_level4_pgt into the wakeup routine so we can > run the kernel above 4G.
The change you really wanted to do in the first place. (d).
> - Increased the size of the wakeup routine to 8K.
You want bigger stack or what? (e)
> - Renamed the variables to use the 64bit register names.
Cleanup. (a)
> - Lots of misc cleanups to match trampoline.S
More cleanups. (a).
Can we at least get (a) (b) (c) (d) and (e) separated?
Oh and please drop the whitespace changes.
> I don't have a configuration I can test this but it compiles cleanly > and it should work, the code is very similar to the SMP trampoline,
I assume you have configuration for test now?
> @@ -60,17 +60,6 @@ extern char wakeup_start, wakeup_end; > > extern unsigned long FASTCALL(acpi_copy_wakeup_routine(unsigned long)); > > -static pgd_t low_ptr; > - > -static void init_low_mapping(void) > -{ > - pgd_t *slot0 = pgd_offset(current->mm, 0UL); > - low_ptr = *slot0; > - set_pgd(slot0, *pgd_offset(current->mm, PAGE_OFFSET)); > - WARN_ON(num_online_cpus() != 1); > - local_flush_tlb(); > -} > -
So you no longer need identity mapping? Is not it specified that when you transition between modes, you should do that while in identity mapping?
> @@ -15,7 +16,6 @@ > # cs = 0x1234, eip = 0x05 > # > > - > ALIGN > .align 16 > ENTRY(wakeup_start)
Whitespace changes.
> @@ -30,22 +30,25 @@ wakeup_code: > cld > # setup data segment > movw %cs, %ax > - movw %ax, %ds # Make ds:0 point to wakeup_start > + movw %ax, %ds # Make ds:0 point to wakeup_start > movw %ax, %ss > - mov $(wakeup_stack - wakeup_code), %sp # Private stack is needed for ASUS board > + # Private stack is needed for ASUS board > + mov $(wakeup_stack - wakeup_code), %sp > > - pushl $0 # Kill any dangerous flags > + pushl $0 # Kill any dangerous flags > popfl
More whitespace changes.
> movl real_magic - wakeup_code, %eax > cmpl $0x12345678, %eax > jne bogus_real_magic > > + call verify_cpu # Verify the cpu supports long mode > +
Check if cpu supports long mode... but we suspended when running long mode, why checking again?
> testl $1, video_flags - wakeup_code > jz 1f > lcall $0xc000,$3 > movw %cs, %ax > - movw %ax, %ds # Bios might have played with that > + movw %ax, %ds # Bios might have played with that > movw %ax, %ss > 1:
More whitespace changes.
> @@ -228,25 +206,10 @@ wakeup_long64: > .align 64 > gdta: > .word 0, 0, 0, 0 # dummy > - > - .word 0, 0, 0, 0 # unused > - > - .word 0xFFFF # 4Gb - (0x100000*0x1000 = 4Gb) > - .word 0 # base address = 0 > - .word 0x9B00 # code read/exec. ??? Why I need 0x9B00 (as opposed to 0x9A00 in order for this to work?) > - .word 0x00CF # granularity = 4096, 386 > - # (+5th nibble of limit) > - > - .word 0xFFFF # 4Gb - (0x100000*0x1000 = 4Gb) > - .word 0 # base address = 0 > - .word 0x9200 # data read/write > - .word 0x00CF # granularity = 4096, 386 > - # (+5th nibble of limit) > -# this is 64bit descriptor for code > - .word 0xFFFF > - .word 0 > - .word 0x9A00 # code read/exec > - .word 0x00AF # as above, but it is long mode and with D=0 > + /* ??? Why I need the accessed bit set in order for this to work? */ > + .quad 0x00cf9b000000ffff # __KERNEL32_CS > + .quad 0x00af9b000000ffff # __KERNEL_CS > + .quad 0x00cf93000000ffff # __KERNEL_DS
Why this change, why did you change the values in here, and why you did not tell me about it in the changelog?
Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - 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/
| |