Re: use x2apic if it is enabled by BIOS
> Date: Sat, 15 Oct 2016 09:55:05 +0200 (CEST) > From: s...@openbsd.org > > On Fri, 14 Oct 2016, Mike Larkin wrote: > > > On Fri, Oct 14, 2016 at 04:49:31PM +0900, YASUOKA Masahiko wrote: > > > Hi, > > > > > > 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? > > > > > > > This should go in snaps, or wait for reports from tech@ with test results > > before it should be committed, IMO. We don't have full support for x2apic, > > and blindly enabling it like this hoping that the bios set everything up > > right is bound to be a bad assumption on at least one machine out there. > > As I understand the code, it should only change behavior on systems where > x2apic is enabled by the bios. And currently on such systems openbsd will > not work anyway, because we then try to use xapic mode without disabling > x2apic mode. Or am I missing something? No, I think you're right and the risk of the diff is low.
Re: use x2apic if it is enabled by BIOS
Hi,
Thanks your comments,
On Fri, 14 Oct 2016 16:23:40 +0200 (CEST)
s...@openbsd.org wrote:
> 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 ...
(snip)
> ... I would leave the comment here why we don't enable it on all hardware
> that supports it.
I reverted the comment and also tweaked white space.
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.c22 Jun 2016 01:12:38 - 1.44
+++ sys/arch/amd64/amd64/lapic.c17 Oct 2016 01:24:50 -
@@ -170,59 +170,57 @@ 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.
-*/
- if ((cpu_ecxfeature_X2APIC) && (cpu_ecxfeature_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))) {
+/*
+ * 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 or it is
+ * enabled by BIOS.
+ */
+ 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, _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)_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)_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();
}
Re: use x2apic if it is enabled by BIOS
On Sat, Oct 15, 2016 at 09:55:05AM +0200, s...@openbsd.org wrote:
>
>
> On Fri, 14 Oct 2016, Mike Larkin wrote:
>
> > On Fri, Oct 14, 2016 at 04:49:31PM +0900, YASUOKA Masahiko wrote:
> > > Hi,
> > >
> > > 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?
> > >
> >
> > This should go in snaps, or wait for reports from tech@ with test results
> > before it should be committed, IMO. We don't have full support for x2apic,
> > and blindly enabling it like this hoping that the bios set everything up
> > right is bound to be a bad assumption on at least one machine out there.
>
> As I understand the code, it should only change behavior on systems where
> x2apic is enabled by the bios. And currently on such systems openbsd will
> not work anyway, because we then try to use xapic mode without disabling
> x2apic mode. Or am I missing something?
>
I'm just saying it needs to be tested on a bunch of machines to make sure
it doesn't break anything, that's all. We've been burned by these sorts of
changes in the past ("make a fundamental change to get one machine working").
-ml
> Stefan
>
> >
> > -ml
> >
> > > 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 - 1.44
> > > +++ sys/arch/amd64/amd64/lapic.c 14 Oct 2016 07:45:50 -
> > > @@ -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.
> > > - */
> > > - if ((cpu_ecxfeature_X2APIC) && (cpu_ecxfeature_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, _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
Re: use x2apic if it is enabled by BIOS
On Fri, 14 Oct 2016, Mike Larkin wrote:
> On Fri, Oct 14, 2016 at 04:49:31PM +0900, YASUOKA Masahiko wrote:
> > Hi,
> >
> > 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?
> >
>
> This should go in snaps, or wait for reports from tech@ with test results
> before it should be committed, IMO. We don't have full support for x2apic,
> and blindly enabling it like this hoping that the bios set everything up
> right is bound to be a bad assumption on at least one machine out there.
As I understand the code, it should only change behavior on systems where
x2apic is enabled by the bios. And currently on such systems openbsd will
not work anyway, because we then try to use xapic mode without disabling
x2apic mode. Or am I missing something?
Stefan
>
> -ml
>
> > 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.c22 Jun 2016 01:12:38 - 1.44
> > +++ sys/arch/amd64/amd64/lapic.c14 Oct 2016 07:45:50 -
> > @@ -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.
> > -*/
> > - if ((cpu_ecxfeature_X2APIC) && (cpu_ecxfeature_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, _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)_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)_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
> > -*
> > -
Re: use x2apic if it is enabled by BIOS
On Fri, Oct 14, 2016 at 04:49:31PM +0900, YASUOKA Masahiko wrote:
> Hi,
>
> 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?
>
This should go in snaps, or wait for reports from tech@ with test results
before it should be committed, IMO. We don't have full support for x2apic,
and blindly enabling it like this hoping that the bios set everything up
right is bound to be a bad assumption on at least one machine out there.
-ml
> 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 - 1.44
> +++ sys/arch/amd64/amd64/lapic.c 14 Oct 2016 07:45:50 -
> @@ -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.
> - */
> - if ((cpu_ecxfeature_X2APIC) && (cpu_ecxfeature_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, _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)_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)_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;
> }
>
> /*
>
> OpenBSD 6.0-current (GENERIC.MP) #44: Fri Oct 14 16:32:03 JST 2016
>
> yasu...@yasuoka-ob1.tokyo.iiji.jp:/source/yasuoka/openbsd/head/git/src/sys/arch/amd64/compile/GENERIC.MP
> real mem = 8207699968 (7827MB)
> avail mem = 7954391
Re: use x2apic if it is enabled by BIOS
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 - 1.44
> +++ sys/arch/amd64/amd64/lapic.c 14 Oct 2016 07:45:50 -
> @@ -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_X2APIC) && (cpu_ecxfeature_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, _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)_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)_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;
> }
Re: use x2apic if it is enabled by BIOS
On Friday, 14 October 2016 15:48:47 CEST Mark Kettenis wrote: > > Date: Fri, 14 Oct 2016 16:49:31 +0900 (JST) > > From: YASUOKA Masahiko <yasu...@openbsd.org> > > > > Hi, > > > > 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? > > is the APICBASE msr available on all amd64 machines? If not, it will > need to be protected with a CPUID check. Yes. I have looked that up before, for its use in the suspend/resume code. Stefan
Re: use x2apic if it is enabled by BIOS
> Date: Fri, 14 Oct 2016 16:49:31 +0900 (JST)
> From: YASUOKA Masahiko <yasu...@openbsd.org>
>
> Hi,
>
> 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?
is the APICBASE msr available on all amd64 machines? If not, it will
need to be protected with a CPUID check.
> 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 - 1.44
> +++ sys/arch/amd64/amd64/lapic.c 14 Oct 2016 07:45:50 -
> @@ -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.
> - */
> - if ((cpu_ecxfeature_X2APIC) && (cpu_ecxfeature_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, _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)_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)_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;
> }
>
> /*
>
> OpenBSD 6.0-current (GENERIC.MP) #44: Fri Oct 14 16:32:03 JST 2016
>
> yasu...@yasuoka-ob1.tokyo.iiji.jp:/source/yasuoka/openbsd/head/git/src/sys/arch/amd64/compile/GENERIC.MP
> real mem = 8207699968 (7827MB)
> avail mem = 7954391040 (7585MB)
> mpath0 at root
> scsibus0 at mpath0: 256 targets
> mainbus0 at root
> bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x7f92b000
use x2apic if it is enabled by BIOS
Hi,
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?
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.c22 Jun 2016 01:12:38 - 1.44
+++ sys/arch/amd64/amd64/lapic.c14 Oct 2016 07:45:50 -
@@ -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.
-*/
- if ((cpu_ecxfeature_X2APIC) && (cpu_ecxfeature_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, _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)_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)_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;
}
/*
OpenBSD 6.0-current (GENERIC.MP) #44: Fri Oct 14 16:32:03 JST 2016
yasu...@yasuoka-ob1.tokyo.iiji.jp:/source/yasuoka/openbsd/head/git/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8207699968 (7827MB)
avail mem = 7954391040 (7585MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x7f92b000 (60 entries)
bios0: vendor American Megatrends Inc. version "5.0.4012" date 06/15/2016
bios0: NEC Express5800/R110h-1 [N8100-2316Y]
acpi0 at bios0: rev 2
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP APIC FIDT MCFG HPET SSDT SSDT SSDT SSDT SPCR DMAR BERT
acpi0: wakeup devices PEGP(S4) PEG0(S4) PEG1(S4) PEG2(S4) PXSX(S4) RP17(S4)
PXSX(S4) RP18(S4) PXSX(S4) RP19(S4) PXSX(S4) RP20(S4) PXSX(S4) RP01(S4)
PXSX(S4) RP02(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Xeon(R) CPU E3-1220 v5 @ 3.00GHz, 3301.20 MHz
cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,
