On Fri, 14 Oct 2016, YASUOKA Masahiko wrote:
> I'm working on NEC Express5800/R110h-1 (dmesg is attached).  On this
> machine, our kernel panics with following message.
> 
>   cpu0 at mainbus0panic: cpu at apic id 0 already attached?
> 
> This seems to happen since x2APIC on the machine is enabled by BIOS
> and the kernel doesn't assume that.  The diff makes the kernel use
> x2APIC if it is enabled by BIOS.
> 
> ok?

the code looks ok, but ...

> 
> Index: sys/arch/amd64/amd64/lapic.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 lapic.c
> --- sys/arch/amd64/amd64/lapic.c      22 Jun 2016 01:12:38 -0000      1.44
> +++ sys/arch/amd64/amd64/lapic.c      14 Oct 2016 07:45:50 -0000
> @@ -170,60 +170,55 @@ lapic_map(paddr_t lapic_base)
>       int s;
>       pt_entry_t *pte;
>       vaddr_t va;
> +     u_int64_t msr;
>  
> -     /*
> -      * On real hardware, x2apic must only be enabled if interrupt remapping
> -      * is also enabled. See 10.12.7 of the SDM vol 3.
> -      * On hypervisors, this is not necessary. Hypervisors can implement
> -      * x2apic support even if the host CPU does not support it.
> -      * Until we support interrupt remapping, use x2apic only if the
> -      * hypervisor flag is also set.
> -      */

... I would leave the comment here why we don't enable it on all hardware 
that supports it.

> -     if ((cpu_ecxfeature&CPUIDECX_X2APIC) && (cpu_ecxfeature&CPUIDECX_HV)) {
> -             u_int64_t msr;
> -
> -             disable_intr();
> -             s = lapic_tpr;
> -
> -             msr = rdmsr(MSR_APICBASE);
> -             msr |= APICBASE_ENABLE_X2APIC;
> -             wrmsr(MSR_APICBASE, msr);
> +     disable_intr();
> +     s = lapic_tpr;
> +
> +     msr = rdmsr(MSR_APICBASE);
>  
> +     if (ISSET(msr, APICBASE_ENABLE_X2APIC) ||
> +         (ISSET(cpu_ecxfeature, CPUIDECX_HV) &&
> +         ISSET(cpu_ecxfeature, CPUIDECX_X2APIC))) {
> +             /*
> +              * If x2APIC is enabled already, use it since it is enabled
> +              * by BIOS.  On hypervisor, use it if it exists.
> +              */
> +             if (!ISSET(msr, APICBASE_ENABLE_X2APIC)) {
> +                     msr |= APICBASE_ENABLE_X2APIC;
> +                     wrmsr(MSR_APICBASE, msr);
> +             }
>               lapic_readreg = x2apic_readreg;
>               lapic_writereg = x2apic_writereg;
>  #ifdef MULTIPROCESSOR
>               x86_ipi = x2apic_ipi;
>  #endif
>               x2apic_enabled = 1;
> -
>               codepatch_call(CPTAG_EOI, &x2apic_eoi);
>  
>               lapic_writereg(LAPIC_TPRI, s);
> -             enable_intr();
> +     } else {
> +             /*
> +              * Map local apic.  If we have a local apic, it's safe to
> +              * assume we're on a 486 or better and can use invlpg and
> +              * non-cacheable PTE's
> +              *
> +              * Whap the PTE "by hand" rather than calling pmap_kenter_pa
> +              * because the latter will attempt to invoke TLB shootdown
> +              * code just as we might have changed the value of
> +              * cpu_number()..
> +              */
> +             va = (vaddr_t)&local_apic;
> +             pte = kvtopte(va);
> +             *pte = lapic_base | PG_RW | PG_V | PG_N | PG_G | pg_nx;
> +             invlpg(va);
>  
> -             return;
> +             lapic_tpr = s;
>       }
>  
> -     va = (vaddr_t)&local_apic;
> -
> -     disable_intr();
> -     s = lapic_tpr;
> -
> -     /*
> -      * Map local apic.  If we have a local apic, it's safe to assume
> -      * we're on a 486 or better and can use invlpg and non-cacheable PTE's
> -      *
> -      * Whap the PTE "by hand" rather than calling pmap_kenter_pa because
> -      * the latter will attempt to invoke TLB shootdown code just as we
> -      * might have changed the value of cpu_number()..
> -      */
> -
> -     pte = kvtopte(va);
> -     *pte = lapic_base | PG_RW | PG_V | PG_N | PG_G | pg_nx;
> -     invlpg(va);
> -
> -     lapic_tpr = s;
>       enable_intr();
> +
> +     return;
>  }

Reply via email to