* [PATCH 1/1] EFI: Fix gdt load
@ 2006-03-02 13:18 Edgar Hucek
2006-03-02 16:12 ` Zwane Mwaikambo
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Edgar Hucek @ 2006-03-02 13:18 UTC (permalink / raw)
To: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 116 bytes --]
This patch makes the kernel bootable again on ia32 EFI systems.
Signed-off-by: Edgar Hucek <hostmaster@ed-soft.at>
[-- Attachment #2: 2.6.15-rc5_efi_gdt.patch --]
[-- Type: text/x-patch, Size: 1170 bytes --]
diff -uNr linux-2.6.16-rc5/arch/i386/kernel/efi.c linux-2.6.16-rc5.efi/arch/i386/kernel/efi.c
--- linux-2.6.16-rc5/arch/i386/kernel/efi.c 2006-03-02 14:08:06.000000000 +0100
+++ linux-2.6.16-rc5.efi/arch/i386/kernel/efi.c 2006-03-02 14:04:44.000000000 +0100
@@ -70,7 +70,8 @@
{
unsigned long cr4;
unsigned long temp;
-
+ struct Xgt_desc_struct *cpu_gdt_descr = &per_cpu(cpu_gdt_descr, 0);
+
spin_lock(&efi_rt_lock);
local_irq_save(efi_rt_eflags);
@@ -103,18 +104,17 @@
*/
local_flush_tlb();
- per_cpu(cpu_gdt_descr, 0).address =
- __pa(per_cpu(cpu_gdt_descr, 0).address);
- load_gdt((struct Xgt_desc_struct *)__pa(&per_cpu(cpu_gdt_descr, 0)));
+ cpu_gdt_descr->address = __pa(cpu_gdt_descr->address);
+ load_gdt(cpu_gdt_descr);
}
static void efi_call_phys_epilog(void)
{
unsigned long cr4;
+ struct Xgt_desc_struct *cpu_gdt_descr = &per_cpu(cpu_gdt_descr, 0);
- per_cpu(cpu_gdt_descr, 0).address =
- (unsigned long)__va(per_cpu(cpu_gdt_descr, 0).address);
- load_gdt((struct Xgt_desc_struct *)__va(&per_cpu(cpu_gdt_descr, 0)));
+ cpu_gdt_descr->address = __va(cpu_gdt_descr->address);
+ load_gdt(cpu_gdt_descr);
cr4 = read_cr4();
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] EFI: Fix gdt load
2006-03-02 13:18 [PATCH 1/1] EFI: Fix gdt load Edgar Hucek
@ 2006-03-02 16:12 ` Zwane Mwaikambo
2006-03-05 2:45 ` Andrew Morton
2006-03-03 10:32 ` Edgar Hucek
2006-03-05 2:43 ` Andrew Morton
2 siblings, 1 reply; 5+ messages in thread
From: Zwane Mwaikambo @ 2006-03-02 16:12 UTC (permalink / raw)
To: Edgar Hucek; +Cc: Linux Kernel
On Thu, 2 Mar 2006, Edgar Hucek wrote:
> This patch makes the kernel bootable again on ia32 EFI systems.
>
> Signed-off-by: Edgar Hucek <hostmaster@ed-soft.at>
>
spin_lock(&efi_rt_lock);
local_irq_save(efi_rt_eflags);
That looks like a race, not to mention that efi_rt_eflags is a global
variable. The same strange ordering occurs on unlock, i presume this code
'works' because it's done early during boot?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] EFI: Fix gdt load
2006-03-02 13:18 [PATCH 1/1] EFI: Fix gdt load Edgar Hucek
2006-03-02 16:12 ` Zwane Mwaikambo
@ 2006-03-03 10:32 ` Edgar Hucek
2006-03-05 2:43 ` Andrew Morton
2 siblings, 0 replies; 5+ messages in thread
From: Edgar Hucek @ 2006-03-03 10:32 UTC (permalink / raw)
To: Edgar Hucek; +Cc: linux-kernel
Edgar Hucek wrote:
> This patch makes the kernel bootable again on ia32 EFI systems.
>
> Signed-off-by: Edgar Hucek <hostmaster@ed-soft.at>
>
>
> ------------------------------------------------------------------------
>
> diff -uNr linux-2.6.16-rc5/arch/i386/kernel/efi.c linux-2.6.16-rc5.efi/arch/i386/kernel/efi.c
> --- linux-2.6.16-rc5/arch/i386/kernel/efi.c 2006-03-02 14:08:06.000000000 +0100
> +++ linux-2.6.16-rc5.efi/arch/i386/kernel/efi.c 2006-03-02 14:04:44.000000000 +0100
> @@ -70,7 +70,8 @@
> {
> unsigned long cr4;
> unsigned long temp;
> -
> + struct Xgt_desc_struct *cpu_gdt_descr = &per_cpu(cpu_gdt_descr, 0);
> +
> spin_lock(&efi_rt_lock);
> local_irq_save(efi_rt_eflags);
>
> @@ -103,18 +104,17 @@
> */
> local_flush_tlb();
>
> - per_cpu(cpu_gdt_descr, 0).address =
> - __pa(per_cpu(cpu_gdt_descr, 0).address);
> - load_gdt((struct Xgt_desc_struct *)__pa(&per_cpu(cpu_gdt_descr, 0)));
> + cpu_gdt_descr->address = __pa(cpu_gdt_descr->address);
> + load_gdt(cpu_gdt_descr);
> }
>
> static void efi_call_phys_epilog(void)
> {
> unsigned long cr4;
> + struct Xgt_desc_struct *cpu_gdt_descr = &per_cpu(cpu_gdt_descr, 0);
>
> - per_cpu(cpu_gdt_descr, 0).address =
> - (unsigned long)__va(per_cpu(cpu_gdt_descr, 0).address);
> - load_gdt((struct Xgt_desc_struct *)__va(&per_cpu(cpu_gdt_descr, 0)));
> + cpu_gdt_descr->address = __va(cpu_gdt_descr->address);
> + load_gdt(cpu_gdt_descr);
>
> cr4 = read_cr4();
>
I don't know if it is a race condition. At least it makes the kernel bootable agin on my box.
Any idea what i could try to make a better patch ?
cu
Edgar (gimli) Hucek
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] EFI: Fix gdt load
2006-03-02 13:18 [PATCH 1/1] EFI: Fix gdt load Edgar Hucek
2006-03-02 16:12 ` Zwane Mwaikambo
2006-03-03 10:32 ` Edgar Hucek
@ 2006-03-05 2:43 ` Andrew Morton
2 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2006-03-05 2:43 UTC (permalink / raw)
To: Edgar Hucek; +Cc: linux-kernel, Zach, Yoav, Matt Domsch
Edgar Hucek <hostmaster@ed-soft.at> 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?
diff -puN arch/i386/kernel/efi.c~efi-fix-gdt-load arch/i386/kernel/efi.c
--- devel/arch/i386/kernel/efi.c~efi-fix-gdt-load 2006-03-04 18:40:20.000000000 -0800
+++ devel-akpm/arch/i386/kernel/efi.c 2006-03-04 18:41:50.000000000 -0800
@@ -70,10 +70,13 @@ static void efi_call_phys_prelog(void)
{
unsigned long cr4;
unsigned long temp;
+ struct Xgt_desc_struct *cpu_gdt_descr;
spin_lock(&efi_rt_lock);
local_irq_save(efi_rt_eflags);
+ cpu_gdt_descr = &per_cpu(cpu_gdt_descr, 0);
+
/*
* If I don't have PSE, I should just duplicate two entries in page
* directory. If I have PSE, I just need to duplicate one entry in
@@ -103,18 +106,17 @@ static void efi_call_phys_prelog(void)
*/
local_flush_tlb();
- per_cpu(cpu_gdt_descr, 0).address =
- __pa(per_cpu(cpu_gdt_descr, 0).address);
- load_gdt((struct Xgt_desc_struct *)__pa(&per_cpu(cpu_gdt_descr, 0)));
+ cpu_gdt_descr->address = __pa(cpu_gdt_descr->address);
+ load_gdt(cpu_gdt_descr);
}
static void efi_call_phys_epilog(void)
{
unsigned long cr4;
+ struct Xgt_desc_struct *cpu_gdt_descr = &per_cpu(cpu_gdt_descr, 0);
- per_cpu(cpu_gdt_descr, 0).address =
- (unsigned long)__va(per_cpu(cpu_gdt_descr, 0).address);
- load_gdt((struct Xgt_desc_struct *)__va(&per_cpu(cpu_gdt_descr, 0)));
+ cpu_gdt_descr->address = __va(cpu_gdt_descr->address);
+ load_gdt(cpu_gdt_descr);
cr4 = read_cr4();
_
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] EFI: Fix gdt load
2006-03-02 16:12 ` Zwane Mwaikambo
@ 2006-03-05 2:45 ` Andrew Morton
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2006-03-05 2:45 UTC (permalink / raw)
To: Zwane Mwaikambo; +Cc: hostmaster, linux-kernel
Zwane Mwaikambo <zwane@arm.linux.org.uk> wrote:
>
> On Thu, 2 Mar 2006, Edgar Hucek wrote:
>
> > This patch makes the kernel bootable again on ia32 EFI systems.
> >
> > Signed-off-by: Edgar Hucek <hostmaster@ed-soft.at>
> >
>
> spin_lock(&efi_rt_lock);
> local_irq_save(efi_rt_eflags);
>
> That looks like a race, not to mention that efi_rt_eflags is a global
> variable. The same strange ordering occurs on unlock, i presume this code
> 'works' because it's done early during boot?
No, the code's OK. This is the prologue function. The epilogue function
does the opposite. In between, this CPU will dink around with EFI stuff.
And the order is correct too - the lock protects efi_rt_eflags.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2006-03-05 2:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-02 13:18 [PATCH 1/1] EFI: Fix gdt load Edgar Hucek
2006-03-02 16:12 ` Zwane Mwaikambo
2006-03-05 2:45 ` Andrew Morton
2006-03-03 10:32 ` Edgar Hucek
2006-03-05 2:43 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).