diff mbox

[1/2] target-i386: disable LINT0 after reset

Message ID 1428881529-29459-2-git-send-email-namit@cs.technion.ac.il
State New
Headers show

Commit Message

Nadav Amit April 12, 2015, 11:32 p.m. UTC
Due to old Seabios bug, QEMU reenable LINT0 after reset. This bug is long gone
and therefore this hack is no longer needed.  Since it violates the
specifications, it is removed.

Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
---
 hw/intc/apic_common.c | 9 ---------
 1 file changed, 9 deletions(-)

Comments

Paolo Bonzini April 13, 2015, 2:12 p.m. UTC | #1
On 13/04/2015 01:32, Nadav Amit wrote:
> Due to old Seabios bug, QEMU reenable LINT0 after reset. This bug is long gone
> and therefore this hack is no longer needed.  Since it violates the
> specifications, it is removed.
> 
> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> ---
>  hw/intc/apic_common.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 042e960..d38d24b 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -243,15 +243,6 @@ static void apic_reset_common(DeviceState *dev)
>      info->vapic_base_update(s);
>  
>      apic_init_reset(dev);
> -
> -    if (bsp) {
> -        /*
> -         * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
> -         * time typically by BIOS, so PIC interrupt can be delivered to the
> -         * processor when local APIC is enabled.
> -         */
> -        s->lvt[APIC_LVT_LINT0] = 0x700;
> -    }
>  }
>  
>  /* This function is only used for old state version 1 and 2 */
> 

Thanks, applied this one.  The other will have to wait for a bit, since
it depends on a patch that is destined to Linux 4.2.

Paolo
Alex Williamson Sept. 15, 2015, 9:19 p.m. UTC | #2
On Mon, 2015-04-13 at 02:32 +0300, Nadav Amit wrote:
> Due to old Seabios bug, QEMU reenable LINT0 after reset. This bug is long gone
> and therefore this hack is no longer needed.  Since it violates the
> specifications, it is removed.
> 
> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> ---
>  hw/intc/apic_common.c | 9 ---------
>  1 file changed, 9 deletions(-)

Please see bug: https://bugs.launchpad.net/qemu/+bug/1488363

Is this bug perhaps not as long gone as we thought, or is there
something else going on here?  Thanks,

Alex

> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 042e960..d38d24b 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -243,15 +243,6 @@ static void apic_reset_common(DeviceState *dev)
>      info->vapic_base_update(s);
>  
>      apic_init_reset(dev);
> -
> -    if (bsp) {
> -        /*
> -         * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
> -         * time typically by BIOS, so PIC interrupt can be delivered to the
> -         * processor when local APIC is enabled.
> -         */
> -        s->lvt[APIC_LVT_LINT0] = 0x700;
> -    }
>  }
>  
>  /* This function is only used for old state version 1 and 2 */
Jan Kiszka Sept. 16, 2015, 5:23 a.m. UTC | #3
On 2015-09-15 23:19, Alex Williamson wrote:
> On Mon, 2015-04-13 at 02:32 +0300, Nadav Amit wrote:
>> Due to old Seabios bug, QEMU reenable LINT0 after reset. This bug is long gone
>> and therefore this hack is no longer needed.  Since it violates the
>> specifications, it is removed.
>>
>> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
>> ---
>>  hw/intc/apic_common.c | 9 ---------
>>  1 file changed, 9 deletions(-)
> 
> Please see bug: https://bugs.launchpad.net/qemu/+bug/1488363
> 
> Is this bug perhaps not as long gone as we thought, or is there
> something else going on here?  Thanks,

I would say, someone needs to check if the SeaBIOS line that is supposed
to enable LINT0 is actually executed on one of the broken systems and,
if not, why not.

Jan
Nadav Amit Sept. 16, 2015, 6:22 a.m. UTC | #4
I'll try to reproduce the problem today.

Nadav
On Sep 16, 2015 8:23 AM, "Jan Kiszka" <jan.kiszka@siemens.com> wrote:

> On 2015-09-15 23:19, Alex Williamson wrote:
> > On Mon, 2015-04-13 at 02:32 +0300, Nadav Amit wrote:
> >> Due to old Seabios bug, QEMU reenable LINT0 after reset. This bug is
> long gone
> >> and therefore this hack is no longer needed.  Since it violates the
> >> specifications, it is removed.
> >>
> >> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> >> ---
> >>  hw/intc/apic_common.c | 9 ---------
> >>  1 file changed, 9 deletions(-)
> >
> > Please see bug: https://bugs.launchpad.net/qemu/+bug/1488363
> >
> > Is this bug perhaps not as long gone as we thought, or is there
> > something else going on here?  Thanks,
>
> I would say, someone needs to check if the SeaBIOS line that is supposed
> to enable LINT0 is actually executed on one of the broken systems and,
> if not, why not.
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RTC ITP SES-DE
> Corporate Competence Center Embedded Linux
>
Gerd Hoffmann Sept. 16, 2015, 6:47 a.m. UTC | #5
On Mi, 2015-09-16 at 07:23 +0200, Jan Kiszka wrote:
> On 2015-09-15 23:19, Alex Williamson wrote:
> > On Mon, 2015-04-13 at 02:32 +0300, Nadav Amit wrote:
> >> Due to old Seabios bug, QEMU reenable LINT0 after reset. This bug is long gone
> >> and therefore this hack is no longer needed.  Since it violates the
> >> specifications, it is removed.
> >>
> >> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> >> ---
> >>  hw/intc/apic_common.c | 9 ---------
> >>  1 file changed, 9 deletions(-)
> > 
> > Please see bug: https://bugs.launchpad.net/qemu/+bug/1488363
> > 
> > Is this bug perhaps not as long gone as we thought, or is there
> > something else going on here?  Thanks,
> 
> I would say, someone needs to check if the SeaBIOS line that is supposed
> to enable LINT0 is actually executed on one of the broken systems and,
> if not, why not.

There is only one reason (beside miscompiling seabios with
CONFIG_QEMU=n) why seabios would skip acpi initialization, and that is
apic not being present according to cpuid:

    cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
    if (eax < 1 || !(cpuid_features & CPUID_APIC)) {
        // No apic - only the main cpu is present.

https://www.kraxel.org/cgit/seabios/tree/src/fw/smp.c#n79

cheers,
  Gerd

PS: coreboot tripped over this too, fixed just a few days ago.
Nadav Amit Sept. 16, 2015, 12:52 p.m. UTC | #6
I don’t happen to have a similar platform. On regular qemu/kvm runs with
q35, I see APIC_LVT0 is set once to 0x8700 on the BSP - as expected:

 qemu-system-x86-19345 [011] d... 2583274.503018: kvm_entry: vcpu 0
 qemu-system-x86-19345 [011] d... 2583274.503019: kvm_exit: reason APIC_ACCESS rip 0x7ffb8288 info 1350 0
 qemu-system-x86-19345 [011] .... 2583274.503020: kvm_emulate_insn: 0:7ffb8288:c7 05 50 03 e0 fe 00 87 00 00 (prot32)
 qemu-system-x86-19345 [011] .... 2583274.503021: kvm_mmio: mmio write len 4 gpa 0xfee00350 val 0x8700
 qemu-system-x86-19345 [011] .... 2583274.503021: kvm_apic: apic_write APIC_LVT0 = 0x8700

If someone sends a trace ( http://www.linux-kvm.org/page/Tracing ) of the
failure, I would be happy to assist.

Nadav

Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Mi, 2015-09-16 at 07:23 +0200, Jan Kiszka wrote:
>> On 2015-09-15 23:19, Alex Williamson wrote:
>>> On Mon, 2015-04-13 at 02:32 +0300, Nadav Amit wrote:
>>>> Due to old Seabios bug, QEMU reenable LINT0 after reset. This bug is long gone
>>>> and therefore this hack is no longer needed.  Since it violates the
>>>> specifications, it is removed.
>>>> 
>>>> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
>>>> ---
>>>> hw/intc/apic_common.c | 9 ---------
>>>> 1 file changed, 9 deletions(-)
>>> 
>>> Please see bug: https://bugs.launchpad.net/qemu/+bug/1488363
>>> 
>>> Is this bug perhaps not as long gone as we thought, or is there
>>> something else going on here?  Thanks,
>> 
>> I would say, someone needs to check if the SeaBIOS line that is supposed
>> to enable LINT0 is actually executed on one of the broken systems and,
>> if not, why not.
> 
> There is only one reason (beside miscompiling seabios with
> CONFIG_QEMU=n) why seabios would skip acpi initialization, and that is
> apic not being present according to cpuid:
> 
>    cpuid(1, &eax, &ebx, &ecx, &cpuid_features);
>    if (eax < 1 || !(cpuid_features & CPUID_APIC)) {
>        // No apic - only the main cpu is present.
> 
> https://www.kraxel.org/cgit/seabios/tree/src/fw/smp.c#n79
> 
> cheers,
>  Gerd
> 
> PS: coreboot tripped over this too, fixed just a few days ago.
diff mbox

Patch

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 042e960..d38d24b 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -243,15 +243,6 @@  static void apic_reset_common(DeviceState *dev)
     info->vapic_base_update(s);
 
     apic_init_reset(dev);
-
-    if (bsp) {
-        /*
-         * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
-         * time typically by BIOS, so PIC interrupt can be delivered to the
-         * processor when local APIC is enabled.
-         */
-        s->lvt[APIC_LVT_LINT0] = 0x700;
-    }
 }
 
 /* This function is only used for old state version 1 and 2 */