Andrew Morton wrote:
> Doh. Too many Zachs.
>
>
> Begin forwarded message:
>
> Date: Sat, 4 Mar 2006 18:43:19 -0800
> From: Andrew Morton <[email protected]>
> To: Edgar Hucek <[email protected]>
> Cc: [email protected], "Zach, Yoav" <[email protected]>, Matt Domsch <[email protected]>
> Subject: Re: [PATCH 1/1] EFI: Fix gdt load
>
>
> Edgar Hucek <[email protected]> wrote:
>
>> This patch makes the kernel bootable again on ia32 EFI systems.
>>
>>
>
> Argh, thanks. I'll move the per_cpu() call inside the lock, just in case
> we happen to be running preemptibly there.
>
> Zach, Matt: please review, test and ack asap?
>
Ok, that was subtle. It took me 10 minutes staring at this code to
notice the extra __pa and __va in the load_gdt call. Actually, by sheer
coincidence, the first one was actually still correct. Normally, this
code would just totally blow up, but you've just identity mapped virtual
and physical addresses. The second one will blow up after the EFI call
without the fix.
Unfortunately, I can't test EFI; I have no machines here that are EFI
capable.
This code has always confused me, though. Why do we do this crazy hack
to begin with? The crazy hack is not remapping the GDT in physical
space, or simulating non-paging memory with paging enabled - that is
completely normal. But why do we muck with the GDT for CPU zero instead
of the current CPU? If the EFI code decides to reload FS or GS, we have
now leaked the user FS or GS from CPU zero onto the current CPU, and I
see no code here which restricts EFI to run on the BSP. This will break
userspace TLS programs. Of course, I have no evidence that EFI will
reload FS or GS, but it must be doing something with segmentation, or
you would not have needed to reload the GDT.
Second, there is another bug in this code as well. Why do we care if
PSE is enabled when identity mapping virtual to physical space? PSE has
_nothing_ to do with this. You are copying top level page ranges, which
are the same size, with or without PSE. We should be checking if PAE is
enabled, and we shouldn't even need to check, since it will either be
compiled in or not. This code is scarily just quite lucky that the
kernel is small enough to fit.
For PAE mode, PSE is always going to be enabled (I believe), so you end
up remapping 1GB of virtual space into physical space. For non-PAE, PSE
may or may not be enabled, in which case, you end up remapping either
4MB or 8MB of the kernel virtual address space back at zero.
I don't believe 4MB is enough to make sure all of the per-cpu variables
can be safely referenced, although I could be wrong. So if there are
EFI machines out there with processors installed that have no PSE
support, and the kernel gets large enough, this code blows up again. I
actually think that is quite likely as EFI becomes more prevalent and
older core processors continue to be made for the embedded market.
Zach
Hi.
I fight against a new EFI bug. With 512MB ram the iMac boots fine, but when i add
extra 512MB to have 1GB ram the kernel stops booting. Still didn't found the problem.
I think it have something to do with the efi_memmap_walk funtction. The major problem
in debuging the problem is that i have no video out at this boot stage. I only can
test it with a machine reboot code.
cu
Edgar (gimli) Hucek
Zachary Amsden wrote:
> Andrew Morton wrote:
>> Doh. Too many Zachs.
>>
>>
>> Begin forwarded message:
>>
>> Date: Sat, 4 Mar 2006 18:43:19 -0800
>> From: Andrew Morton <[email protected]>
>> To: Edgar Hucek <[email protected]>
>> Cc: [email protected], "Zach, Yoav" <[email protected]>,
>> Matt Domsch <[email protected]>
>> Subject: Re: [PATCH 1/1] EFI: Fix gdt load
>>
>>
>> Edgar Hucek <[email protected]> wrote:
>>
>>> This patch makes the kernel bootable again on ia32 EFI systems.
>>>
>>>
>>
>> Argh, thanks. I'll move the per_cpu() call inside the lock, just in case
>> we happen to be running preemptibly there.
>>
>> Zach, Matt: please review, test and ack asap?
>>
>
> Ok, that was subtle. It took me 10 minutes staring at this code to
> notice the extra __pa and __va in the load_gdt call. Actually, by sheer
> coincidence, the first one was actually still correct. Normally, this
> code would just totally blow up, but you've just identity mapped virtual
> and physical addresses. The second one will blow up after the EFI call
> without the fix.
>
> Unfortunately, I can't test EFI; I have no machines here that are EFI
> capable.
>
> This code has always confused me, though. Why do we do this crazy hack
> to begin with? The crazy hack is not remapping the GDT in physical
> space, or simulating non-paging memory with paging enabled - that is
> completely normal. But why do we muck with the GDT for CPU zero instead
> of the current CPU? If the EFI code decides to reload FS or GS, we have
> now leaked the user FS or GS from CPU zero onto the current CPU, and I
> see no code here which restricts EFI to run on the BSP. This will break
> userspace TLS programs. Of course, I have no evidence that EFI will
> reload FS or GS, but it must be doing something with segmentation, or
> you would not have needed to reload the GDT.
>
> Second, there is another bug in this code as well. Why do we care if
> PSE is enabled when identity mapping virtual to physical space? PSE has
> _nothing_ to do with this. You are copying top level page ranges, which
> are the same size, with or without PSE. We should be checking if PAE is
> enabled, and we shouldn't even need to check, since it will either be
> compiled in or not. This code is scarily just quite lucky that the
> kernel is small enough to fit.
>
> For PAE mode, PSE is always going to be enabled (I believe), so you end
> up remapping 1GB of virtual space into physical space. For non-PAE, PSE
> may or may not be enabled, in which case, you end up remapping either
> 4MB or 8MB of the kernel virtual address space back at zero.
>
> I don't believe 4MB is enough to make sure all of the per-cpu variables
> can be safely referenced, although I could be wrong. So if there are
> EFI machines out there with processors installed that have no PSE
> support, and the kernel gets large enough, this code blows up again. I
> actually think that is quite likely as EFI becomes more prevalent and
> older core processors continue to be made for the embedded market.
>
> Zach
>