| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20061116215154.GD6695@elf.ucw.cz>
Date: Thu, 16 Nov 2006 22:51:54 +0100
From: Pavel Machek <pavel@...e.cz>
To: Vivek Goyal <vgoyal@...ibm.com>
Cc: linux kernel mailing list <linux-kernel@...r.kernel.org>,
akpm@...l.org, rjw@...k.pl, ebiederm@...ssion.com, hpa@...or.com,
magnus.damm@...il.com, Reloc Kernel List <fastboot@...ts.osdl.org>,
ak@...e.de
Subject: Re: [Fastboot] [RFC] [PATCH 10/16] x86_64: 64bit PIC ACPI wakeup
Hi!
> > > 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.
> >
>
> That's fine but it does not harm. Now all the entry path share the
> same sanity check (verify_cpu.S) and I believe it makes code more
> maintanable and more robust. It just makes our checks stronger in
> case somebody really replaces the cpus.
It is probably okay if shared.
> > > - 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)
> >
>
> I think this is because of wakeup_level4_pgt page tables which are now
> part of trampoline. And these page tables got to be at 4K byte boundary.
> Hence now we need two pages for trampoline instead of one.
Aha, ok. Notice that in the changelog.
> > > - 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?
>
> Ok. I will separate the patches.
Thanks!
> > > 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?
>
> Eric did not have but now I have tested it already on two configurations.
> I think that's good enough. Isn't it?
Should be... if it is in -mm for long enough. Unfortunately very
little people are testing x86-64 on notebooks :-(. I guess I should
force myself to install 64-bit distro on old arima here...
> > > -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?
> >
>
> I am not sure where these mappings are required at all in first place?
> While going to sleep state? While resuming we are using wake page tables
> and they already got identity mappings so it should not be an issue.
Ok, I guess that's okay.
> > > @@ -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?
>
> I think mainly it has been modified to be consistent gdt table across
> the kernel (cpu_gdt_table, trampoline.S and wakeup.S). Now __KERNEL_32_CS
> entry has been moved up to keep the size of gdt small on trampoline. This
> change was done in patch number 7 (cleanup segments).
>
> Seondly, I think it is just change of form from using .word to .quad. More
> compact form.
>
> Thirdly I think it does not harm marking that gdt entry has been accessed.
> Eric can elaborate more on it. Patch 7 also has got details.
Ok. Maybe it would be nice to #include the GDT's, too, so they do not
drift apart? Or at least comment "this has to be kept in sync
with..."?
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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists