A different lapic number in devicetree.cb needed for CPU with the same SKU and steping (Intel Atom C3538).

Hi!

We developed our CRB motherboard on Intel Atom C3538 (4 core) Denverton_NS processor. Faced with the following problem. 
For part of processors with the same SKU and steping (Atom C3538), lapic #4 in devicetree.cb needed (95%), and for the other part lapic #0 (5%).
Intel confirmed that it might be so and that's okay ...

Part of devicetree.cb:

device cpu_cluster 0 on
    	device lapic 4 on end
end

If we do not specify lapic id correctly in devicetree.cb, freeBSD OS does not BOOT (Unix like).

FreeBSD BOOT log (set lapic #4 in devicetree.cb but need lapic #0):

Table 'FACP' at 0x7f768070
Table 'SSDT' at 0x7f768170
Table 'MCFG' at 0x7f7693e0
Table 'APIC' at 0x7f769420
APIC: Found table at 0x7f769420
APIC: Using the MADT enumerator.
MADT: Found CPU APIC ID 0 ACPI ID 0: enabled
SMP: Added CPU 0 (AP)
MADT: Found CPU APIC ID 4 ACPI ID 1: enabled
SMP: Added CPU 4 (AP)
MADT: Found CPU APIC ID 12 ACPI ID 2: enabled
SMP: Added CPU 12 (AP)
MADT: Found CPU APIC ID 16 ACPI ID 3: enabled
SMP: Added CPU 16 (AP)
MADT: Found CPU APIC ID 24 ACPI ID 4: enabled
SMP: Added CPU 24 (AP)
Copyright (c) 1992-2019 The FreeBSD Project.
Copyright (c) 1979, 1980, 1983, 1986, 1988, 1989, 1991, 1992, 1993, 1994
The Regents of the University of California. All rights reserved.
FreeBSD is a registered trademark of The FreeBSD Foundation.
FreeBSD 11.3-RELEASE #0 r349754: Fri Jul  5 04:45:24 UTC 2019
    [e-mail]:/usr/obj/usr/src/sys/GENERIC amd64
FreeBSD clang version 8.0.0 (tags/RELEASE_800/final 356365) (based on LLVM 8.0.0)
Table 'FACP' at 0x7f768070
Table 'SSDT' at 0x7f768170
Table 'MCFG' at 0x7f7693e0
Table 'APIC' at 0x7f769420
Table 'HPET' at 0x7f7694a0
ACPI: No SRAT table found
PPIM 0: PA=0xa0000, VA=0xffffffff82410000, size=0x10000, mode=0
VT(vga): resolution 640x480
Preloaded elf kernel "/boot/kernel/kernel" at 0xffffffff8226d000.
Calibrating TSC clock ... TSC clock: 2100071708 Hz
CPU: Intel(R) Atom(TM) CPU C3538 @ 2.10GHz (2100.07-MHz K8-class CPU)
  Origin="GenuineIntel"  Id=0x506f1  Family=0x6  Model=0x5f  Stepping=1
  Features=0xbfebfbff<FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CLFLUSH,DTS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE>
  Features2=0x4ff8ebbf<SSE3,PCLMULQDQ,DTES64,MON,DS_CPL,VMX,EST,TM2,SSSE3,SDBG,CX16,xTPR,PDCM,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,TSCDLT,AESNI,XSAVE,OSXSAVE,RDRAND>
  AMD Features=0x2c100800<SYSCALL,NX,Page1GB,RDTSCP,LM>
  AMD Features2=0x101<LAHF,Prefetch>
  Structured Extended Features=0x2294e283<FSGSBASE,TSCADJ,SMEP,ERMS,NFPUSG,MPX,PQE,RDSEED,SMAP,CLFLUSHOPT,PROCTRACE,SHA>
  Structured Extended Features3=0xac000400<MD_CLEAR,IBPB,STIBP,ARCH_CAP,SSBD>
  XSAVE Features=0xf<XSAVEOPT,XSAVEC,XINUSE,XSAVES>
  IA32_ARCH_CAPS=0x69<RDCL_NO,SKIP_L1DFL_VME>
  VT-x: Basic Features=0xda0400<SMM,INS/OUTS,TRUE>
        Pin-Based Controls=0xff<ExtINT,NMI,VNMI,PreTmr,PostIntr>
        Primary Processor Controls=0xfff9fffe<INTWIN,TSCOff,HLT,INVLPG,MWAIT,RDPMC,RDTSC,CR3-LD,CR3-ST,CR8-LD,CR8-ST,TPR,NMIWIN,MOV-DR,IO,IOmap,MTF,MSRmap,MONITOR,PAUSE>
        Secondary Processor Controls=0x1d6fff<APIC,EPT,DT,RDTSCP,x2APIC,VPID,WBINVD,UG,APIC-reg,VID,PAUSE-loop,RDRAND,VMFUNC,VMCS,XSAVES>
        Exit Controls=0xda0400<PAT-LD,EFER-SV,PTMR-SV>
        Entry Controls=0xda0400
        EPT Features=0x6334141<XO,PW4,UC,WB,2M,1G,INVEPT,AD,single,all>
        VPID Features=0xf01<INVVPID,individual,single,all,single-globals>
  TSC: P-state invariant, performance statistics
DTLB: 4k pages, fully associative, 32 entries
Data TLB: 4 KBytes pages, 4-way set associative, 512 entries
Instruction TLB: 4 KByte pages, fully associative, 48 entries
DTLB: 2M/4M Byte pages, 4-way associative, 32 entries
L2 cache: 2048 kbytes, 16-way associative, 64 bytes/line
real memory  = 8589934592 (8192 MB)
Physical memory chunk(s):
0x0000000000010000 - 0x000000000009bfff, 573440 bytes (140 pages)
0x0000000000100000 - 0x00000000001fffff, 1048576 bytes (256 pages)
0x0000000002400000 - 0x000000007f74ffff, 2100625408 bytes (512848 pages)
0x0000000100000000 - 0x000000027012efff, 6175256576 bytes (1507631 pages)
avail memory = 8220336128 (7839 MB)
Table 'FACP' at 0x7f768070
Table 'SSDT' at 0x7f768170
Table 'MCFG' at 0x7f7693e0
Table 'APIC' at 0x7f769420
Table 'HPET' at 0x7f7694a0
ACPI: No DMAR table found
Event timer "LAPIC" quality 600
ACPI APIC Table: <COREv4 COREBOOT>
WARNING: L1 data cache covers less APIC IDs than a core
0 < 1
Package ID shift: 5
L2 cache ID shift: 2
L1 cache ID shift: 1
Core ID shift: 1
panic: AP #4 (PHY# 0) failed!
cpuid = 0
KDB: stack backtrace:
#0 0xffffffff80b4c4b7 at kdb_backtrace+0x67
#1 0xffffffff80b054ce at vpanic+0x17e
#2 0xffffffff80b05343 at panic+0x43
#3 0xffffffff80f752a4 at native_start_all_aps+0x344
#4 0xffffffff80f74c4f at cpu_mp_start+0x2ef
#5 0xffffffff80b5cb76 at mp_start+0xa6
#6 0xffffffff80aa0b48 at mi_startup+0x118
#7 0xffffffff8031202c at btext+0x2c
Uptime: 1s

Other Linux OS boot but show an incorrect number of cores (5 instead of 4) and offline processor cores appear (see log). 
Ubuntu 18.04 LTS (GNU/Linux 4.15.0-20-generic x86_64)

# lscpu
Architecture:         x86_64
CPU op-mode(s):       32-bit, 64-bit
Byte Order:           Little Endian
CPU(s):               5
On-line CPU(s) list:  0,2-4
Off-line CPU(s) list: 1
Thread(s) per core:   1
Core(s) per socket:   4
Socket(s):            1
NUMA node(s):         1
Vendor ID:            GenuineIntel
CPU family:           6
Model:                95
Model name:           Intel(R) Atom(TM) CPU C3538 @ 2.10GHz
Stepping:             1
CPU MHz:              2097.502
CPU max MHz:          2100.0000
CPU min MHz:          800.0000
BogoMIPS:             4200.00
Virtualization:       VT-x
L1d cache:            24K
L1i cache:            32K
L2 cache:             2048K
NUMA node0 CPU(s):    0,2-4

What can be done in this situation? How to make a universal version of devicetree.cb?

[Javier Galindo]

Does this thread help (reply post by Sumo specifically for this issue)? I don't think this was ever upstreamed.

https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/MCRPR...

Thank you so much Javier Galindo!

Sorry for not finding this case myself ...
I checked it on the motherboard with lapic #4 - everything works as it should. 
Tomorrow I'll check it on the motherboard with lapic #0. 
I wish I could understand how this magic works :)! lapic 0xbeef .....

Thanks Sumo too!

Thank you so much Javier Galindo!

Sorry for not finding this case myself ...
I checked it on the motherboard with lapic #4 - everything works as it should. 
Tomorrow I'll check it on the motherboard with lapic #0. 
I wish I could understand how this magic works :)! lapic 0xbeef .....

Thanks Sumo too!

[Nico Huber]

Hi,

have you tried omitting the `device lapic` line from the devicetree?
It would only matter if there is configuration associated with it, but
I can't see anything like that for `intel/harcuvar`.

What happens is that this `device lapic` line in the devicetree becomes
an entry in a list at runtime. This list is later filled with the actual
cores present. If any of the actual cores has the same APIC id as given
in the devicetree, no additional entry is added for this core. However
if none of the actual cores has that id, the original entry is left
blindly in the list, causing coreboot to report the spurious, fifth
core.

On 07.10.20 21:27, [e-mail] wrote:
> Thank you so much Javier Galindo!
>
> Sorry for not finding this case myself ...
> I checked it on the motherboard with lapic #4 - everything works as it should.
> Tomorrow I'll check it on the motherboard with lapic #0.
> I wish I could understand how this magic works :)! lapic 0xbeef .....

It's kind of a wildcard that gets replaced with the number found in the
hardware. Nothing too special but probably unnecessary.

Nico

[Sumo]

Hi,
> have you tried omitting the `device lapic` line from the devicetree?

I have tested this, in this case Linux shows only one processor core.
Therefore the 'device lapic' line is really needed...

I can submit that Local APIC Fixup patch to gerrit but I'm not sure if this
is really the best solution.

Kind regards,
Sumo

On Wed, Oct 7, 2020 at 6:35 PM Nico Huber  wrote:
> Hi,
>
> have you tried omitting the `device lapic` line from the devicetree?
> It would only matter if there is configuration associated with it, but
> I can't see anything like that for `intel/harcuvar`.
>
> What happens is that this `device lapic` line in the devicetree becomes
> an entry in a list at runtime. This list is later filled with the actual
> cores present. If any of the actual cores has the same APIC id as given
> in the devicetree, no additional entry is added for this core. However
> if none of the actual cores has that id, the original entry is left
> blindly in the list, causing coreboot to report the spurious, fifth
> core.
>
> On 07.10.20 21:27, [e-mail] wrote:
> > Thank you so much Javier Galindo!
> >
> > Sorry for not finding this case myself ...
> > I checked it on the motherboard with lapic #4 - everything works as it
>
> should.
> > Tomorrow I'll check it on the motherboard with lapic #0.
> > I wish I could understand how this magic works :)! lapic 0xbeef .....
>
> It's kind of a wildcard that gets replaced with the number found in the
> hardware. Nothing too special but probably unnecessary.
>
> Nico
> _______________________________________________
> coreboot mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Jay Talbott]

Unfortunately, for the Denverton SoC (C3000 series), the APIC ID of the first core is not always the same. For 16-core SKUs, it’s always 0, but for SKUs with a lower number of cores, it may be a different number. It’s also possible (but not confirmed) that for a particular SKU (other than 16-core SKUs), it might not be consistent between parts. The solution is to basically ignore the value in devicetree and use the actual APIC ID from the first core.

- Jay

From: Sumo [mailto:[e-mail]] 
Sent: Monday, August 16, 2021 9:15 AM
To: Nico Huber
Cc: Дмитрий Понаморев; Coreboot
Subject: [coreboot] Re: A different lapic number in devicetree.cb needed for CPU with the same SKU and steping (Intel Atom C3538).

Hi,
> have you tried omitting the `device lapic` line from the devicetree?

I have tested this, in this case Linux shows only one processor core. Therefore the 'device lapic' line is really needed...

I can submit that Local APIC Fixup patch to gerrit but I'm not sure if this is really the best solution.

Kind regards,

Sumo

On Wed, Oct 7, 2020 at 6:35 PM Nico Huber  wrote:

Hi,

have you tried omitting the `device lapic` line from the devicetree?
It would only matter if there is configuration associated with it, but
I can't see anything like that for `intel/harcuvar`.

What happens is that this `device lapic` line in the devicetree becomes
an entry in a list at runtime. This list is later filled with the actual
cores present. If any of the actual cores has the same APIC id as given
in the devicetree, no additional entry is added for this core. However
if none of the actual cores has that id, the original entry is left
blindly in the list, causing coreboot to report the spurious, fifth
core.

On 07.10.20 21:27, [e-mail] wrote:
> Thank you so much Javier Galindo!
>
> Sorry for not finding this case myself ...
> I checked it on the motherboard with lapic #4 - everything works as it should.
> Tomorrow I'll check it on the motherboard with lapic #0.
> I wish I could understand how this magic works :)! lapic 0xbeef .....

It's kind of a wildcard that gets replaced with the number found in the
hardware. Nothing too special but probably unnecessary.

Nico
_______________________________________________
coreboot mailing list -- [e-mail]
To unsubscribe send an email to [e-mail]

[Дмитрий Понаморев]

The solution to this problem has already been discussed here. You just need
to replace in the file devicetree.cb  "lapic 4" to  "lapic 0xbeef".

*device cpu_cluster 0 on    #device lapic 4 on end    device lapic 0xbeef
on end #NOTE: correct Local APIC ID is set in in dev_enumerate() end*

This works for two cores (C3338), quad cores (C3538), eight cores, twelve
cores, and 16 cores  - I've tested this.

пн, 16 авг. 2021 г. в 19:45, Jay Talbott :
> Unfortunately, for the Denverton SoC (C3000 series), the APIC ID of the
> first core is not always the same. For 16-core SKUs, it’s always 0, but for
> SKUs with a lower number of cores, it may be a different number. It’s also
> possible (but not confirmed) that for a particular SKU (other than 16-core
> SKUs), it might not be consistent between parts. The solution is to
> basically ignore the value in devicetree and use the actual APIC ID from
> the first core.
> Jay
>
> *From:* Sumo [mailto:[e-mail]]
> *Sent:* Monday, August 16, 2021 9:15 AM
> *To:* Nico Huber
> *Cc:* Дмитрий Понаморев; Coreboot
> *Subject:* [coreboot] Re: A different lapic number in devicetree.cb
> needed for CPU with the same SKU and steping (Intel Atom C3538).
>
> Hi,
> > have you tried omitting the `device lapic` line from the devicetree?
>
> I have tested this, in this case Linux shows only one processor core.
> Therefore the 'device lapic' line is really needed...
>
> I can submit that Local APIC Fixup patch to gerrit but I'm not sure if
> this is really the best solution.
>
> Kind regards,
>
> Sumo
>
> On Wed, Oct 7, 2020 at 6:35 PM Nico Huber  wrote:
>
> Hi,
>
> have you tried omitting the `device lapic` line from the devicetree?
> It would only matter if there is configuration associated with it, but
> I can't see anything like that for `intel/harcuvar`.
>
> What happens is that this `device lapic` line in the devicetree becomes
> an entry in a list at runtime. This list is later filled with the actual
> cores present. If any of the actual cores has the same APIC id as given
> in the devicetree, no additional entry is added for this core. However
> if none of the actual cores has that id, the original entry is left
> blindly in the list, causing coreboot to report the spurious, fifth
> core.
>
> On 07.10.20 21:27, [e-mail] wrote:
> > Thank you so much Javier Galindo!
> >
> > Sorry for not finding this case myself ...
> > I checked it on the motherboard with lapic #4 - everything works as it
>
> should.
> > Tomorrow I'll check it on the motherboard with lapic #0.
> > I wish I could understand how this magic works :)! lapic 0xbeef .....
>
> It's kind of a wildcard that gets replaced with the number found in the
> hardware. Nothing too special but probably unnecessary.
>
> Nico
> _______________________________________________
> coreboot mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Sumo]

Hi Jay,
> ... It’s also possible (but not confirmed) that for a particular SKU

(other than 16-core SKUs), it might not be consistent between parts
I can confirm this, I have two C3558 SoC's with first core different APID
ID's...

Do you think I can submit my patch (see previous discussions) or do we have
a better solution?

Kind regards,
Sumo

On Mon, Aug 16, 2021 at 1:45 PM Jay Talbott 
wrote:
> Unfortunately, for the Denverton SoC (C3000 series), the APIC ID of the
> first core is not always the same. For 16-core SKUs, it’s always 0, but for
> SKUs with a lower number of cores, it may be a different number. It’s also
> possible (but not confirmed) that for a particular SKU (other than 16-core
> SKUs), it might not be consistent between parts. The solution is to
> basically ignore the value in devicetree and use the actual APIC ID from
> the first core.
> Jay
>
> *From:* Sumo [mailto:[e-mail]]
> *Sent:* Monday, August 16, 2021 9:15 AM
> *To:* Nico Huber
> *Cc:* Дмитрий Понаморев; Coreboot
> *Subject:* [coreboot] Re: A different lapic number in devicetree.cb
> needed for CPU with the same SKU and steping (Intel Atom C3538).
>
> Hi,
> > have you tried omitting the `device lapic` line from the devicetree?
>
> I have tested this, in this case Linux shows only one processor core.
> Therefore the 'device lapic' line is really needed...
>
> I can submit that Local APIC Fixup patch to gerrit but I'm not sure if
> this is really the best solution.
>
> Kind regards,
>
> Sumo
>
> On Wed, Oct 7, 2020 at 6:35 PM Nico Huber  wrote:
>
> Hi,
>
> have you tried omitting the `device lapic` line from the devicetree?
> It would only matter if there is configuration associated with it, but
> I can't see anything like that for `intel/harcuvar`.
>
> What happens is that this `device lapic` line in the devicetree becomes
> an entry in a list at runtime. This list is later filled with the actual
> cores present. If any of the actual cores has the same APIC id as given
> in the devicetree, no additional entry is added for this core. However
> if none of the actual cores has that id, the original entry is left
> blindly in the list, causing coreboot to report the spurious, fifth
> core.
>
> On 07.10.20 21:27, [e-mail] wrote:
> > Thank you so much Javier Galindo!
> >
> > Sorry for not finding this case myself ...
> > I checked it on the motherboard with lapic #4 - everything works as it
>
> should.
> > Tomorrow I'll check it on the motherboard with lapic #0.
> > I wish I could understand how this magic works :)! lapic 0xbeef .....
>
> It's kind of a wildcard that gets replaced with the number found in the
> hardware. Nothing too special but probably unnecessary.
>
> Nico
> _______________________________________________
> coreboot mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Дмитрий Понаморев]

Hi Sumo,
> ... It’s also possible (but not confirmed) that for a particular SKU
> (other than 16-core SKUs), it might not be consistent between parts
> I can confirm this, I have two C3558 SoC's with first core different APID
> ID's...

I can confirm it too for C3338, C3538.

Do you think I can submit my patch (see previous discussions) or do we have
> a better solution?

Your patch works great for me ( for C3338, C3538, C3758, C3958).
Thanks again! (from this thread:
https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/MCRPR...
)

пн, 16 авг. 2021 г. в 20:58, Sumo :
> Hi Jay,
> > ... It’s also possible (but not confirmed) that for a particular SKU
>
> (other than 16-core SKUs), it might not be consistent between parts
> I can confirm this, I have two C3558 SoC's with first core different APID
> ID's...
>
> Do you think I can submit my patch (see previous discussions) or do we
> have a better solution?
>
> Kind regards,
> Sumo
>
> On Mon, Aug 16, 2021 at 1:45 PM Jay Talbott <
> [e-mail]> wrote:
> > Unfortunately, for the Denverton SoC (C3000 series), the APIC ID of the
> > first core is not always the same. For 16-core SKUs, it’s always 0, but for
> > SKUs with a lower number of cores, it may be a different number. It’s also
> > possible (but not confirmed) that for a particular SKU (other than 16-core
> > SKUs), it might not be consistent between parts. The solution is to
> > basically ignore the value in devicetree and use the actual APIC ID from
> > the first core.
> > Jay
> >
> > *From:* Sumo [mailto:[e-mail]]
> > *Sent:* Monday, August 16, 2021 9:15 AM
> > *To:* Nico Huber
> > *Cc:* Дмитрий Понаморев; Coreboot
> > *Subject:* [coreboot] Re: A different lapic number in devicetree.cb
> > needed for CPU with the same SKU and steping (Intel Atom C3538).
> >
> > Hi,
> > > have you tried omitting the `device lapic` line from the devicetree?
> >
> > I have tested this, in this case Linux shows only one processor core.
> > Therefore the 'device lapic' line is really needed...
> >
> > I can submit that Local APIC Fixup patch to gerrit but I'm not sure if
> > this is really the best solution.
> >
> > Kind regards,
> >
> > Sumo
> >
> > On Wed, Oct 7, 2020 at 6:35 PM Nico Huber  wrote:
> >
> > Hi,
> >
> > have you tried omitting the `device lapic` line from the devicetree?
> > It would only matter if there is configuration associated with it, but
> > I can't see anything like that for `intel/harcuvar`.
> >
> > What happens is that this `device lapic` line in the devicetree becomes
> > an entry in a list at runtime. This list is later filled with the actual
> > cores present. If any of the actual cores has the same APIC id as given
> > in the devicetree, no additional entry is added for this core. However
> > if none of the actual cores has that id, the original entry is left
> > blindly in the list, causing coreboot to report the spurious, fifth
> > core.
> >
> > On 07.10.20 21:27, [e-mail] wrote:
> > > Thank you so much Javier Galindo!
> > >
> > > Sorry for not finding this case myself ...
> > > I checked it on the motherboard with lapic #4 - everything works as it
> >
> > should.
> > > Tomorrow I'll check it on the motherboard with lapic #0.
> > > I wish I could understand how this magic works :)! lapic 0xbeef .....
> >
> > It's kind of a wildcard that gets replaced with the number found in the
> > hardware. Nothing too special but probably unnecessary.
> >
> > Nico
> > _______________________________________________
> > coreboot mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]

[Szafranski, MariuszX]

Hi,

Maybe keep 0 for apicid in devicetree.cb and add something like below to denverton soc_init (or define separate function for check and fixup)  in src/soc/intel/denverton_ns/chip.c.
Could anyone with apicid != 0 test and let us know?
BR,
Mariusz

static void soc_init(void *data)
{
  unsigned long bsp_lapicid = lapicid();
  struct device *dev;

if (bsp_lapicid){
    for (dev = &dev_root, cnt = 0; dev; dev = dev->next){
      if ((dev->path.type == DEVICE_PATH_APIC){
        if (bsp_lapicid != dev->path.apic.apic_id){
          printk(BIOS_SPEW,
            "soc_init: BSP lapic ID = 0x%lx - updating in devicetree\n",
            bsp_lapicid);
          dev->path.apic.apic_id = bsp_lapicid;
        } else {
          break;
        };
      };
    };
  };
…..

From: Дмитрий Понаморев 
Sent: Monday, August 16, 2021 8:17 PM
To: Sumo 
Cc: Jay Talbott ; Coreboot 
Subject: [coreboot] Re: A different lapic number in devicetree.cb needed for CPU with the same SKU and steping (Intel Atom C3538).

Hi Sumo,
> ... It’s also possible (but not confirmed) that for a particular SKU (other than 16-core SKUs), it might not be consistent between parts

I can confirm this, I have two C3558 SoC's with first core different APID ID's...

I can confirm it too for C3338, C3538.

Do you think I can submit my patch (see previous discussions) or do we have a better solution?

Your patch works great for me ( for C3338, C3538, C3758, C3958).
Thanks again! (from this thread: https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/MCRPR... )

пн, 16 авг. 2021 г. в 20:58, Sumo <[e-mail]>:
Hi Jay,
> ... It’s also possible (but not confirmed) that for a particular SKU (other than 16-core SKUs), it might not be consistent between parts

I can confirm this, I have two C3558 SoC's with first core different APID ID's...

Do you think I can submit my patch (see previous discussions) or do we have a better solution?

Kind regards,
Sumo

On Mon, Aug 16, 2021 at 1:45 PM Jay Talbott <[e-mail]> wrote:
Unfortunately, for the Denverton SoC (C3000 series), the APIC ID of the first core is not always the same. For 16-core SKUs, it’s always 0, but for SKUs with a lower number of cores, it may be a different number. It’s also possible (but not confirmed) that for a particular SKU (other than 16-core SKUs), it might not be consistent between parts. The solution is to basically ignore the value in devicetree and use the actual APIC ID from the first core.

- Jay

From: Sumo [mailto:[e-mail]]
Sent: Monday, August 16, 2021 9:15 AM
To: Nico Huber
Cc: Дмитрий Понаморев; Coreboot
Subject: [coreboot] Re: A different lapic number in devicetree.cb needed for CPU with the same SKU and steping (Intel Atom C3538).

Hi,
> have you tried omitting the `device lapic` line from the devicetree?

I have tested this, in this case Linux shows only one processor core. Therefore the 'device lapic' line is really needed...

I can submit that Local APIC Fixup patch to gerrit but I'm not sure if this is really the best solution.

Kind regards,
Sumo

On Wed, Oct 7, 2020 at 6:35 PM Nico Huber <[e-mail]> wrote:
Hi,

have you tried omitting the `device lapic` line from the devicetree?
It would only matter if there is configuration associated with it, but
I can't see anything like that for `intel/harcuvar`.

What happens is that this `device lapic` line in the devicetree becomes
an entry in a list at runtime. This list is later filled with the actual
cores present. If any of the actual cores has the same APIC id as given
in the devicetree, no additional entry is added for this core. However
if none of the actual cores has that id, the original entry is left
blindly in the list, causing coreboot to report the spurious, fifth
core.

On 07.10.20 21:27, [e-mail] wrote:
> Thank you so much Javier Galindo!
>
> Sorry for not finding this case myself ...
> I checked it on the motherboard with lapic #4 - everything works as it should.
> Tomorrow I'll check it on the motherboard with lapic #0.
> I wish I could understand how this magic works :)! lapic 0xbeef .....

It's kind of a wildcard that gets replaced with the number found in the
hardware. Nothing too special but probably unnecessary.

Nico
_______________________________________________
coreboot mailing list -- [e-mail]
To unsubscribe send an email to [e-mail]
--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263

This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

[Дмитрий Понаморев]

Hi

I don't understand why to invent something new. Sumo's patch works great. Why
can't you use it?
This Intel Atom Denverton C3000 processor has a maximum of 16 cores (4
clusters of 4 processors each).
The problem with the lapic ID is real for processors with less than 16
cores, especially for 2 and 4 cores - these processors are also 16 only
with disabled cores in clusters.
Intel turns them off randomly and therefore the value lapic ID floats.
Changing this parameter as you suggest will suit you if you have one board
- if there are many of them, it will not work.

Best regards,
Dmitry Ponamorev

пт, 20 авг. 2021 г. в 23:56, Szafranski, MariuszX <
[e-mail]>:
> Hi,
>
> Maybe keep 0 for apicid in devicetree.cb and add something like below to
> denverton soc_init (or define separate function for check and fixup)  in
> src/soc/intel/denverton_ns/chip.c.
>
> Could anyone with apicid != 0 test and let us know?
>
> BR,
>
> Mariusz
>
> static void soc_init(void *data)
>
> {
>
> unsigned long bsp_lapicid = lapicid();
>
> struct device *dev;
>
> if (bsp_lapicid){
> for (dev = &dev_root, cnt = 0; dev; dev = dev->next){
>
>   if ((dev->path.type == DEVICE_PATH_APIC){
>
>     if (bsp_lapicid != dev->path.apic.apic_id){
>
>       printk(BIOS_SPEW,
>
>         "soc_init: BSP lapic ID = 0x%lx - updating in devicetree\n",
>
>         bsp_lapicid);
>
>       dev->path.apic.apic_id = bsp_lapicid;
>
>     } else {
>
>       break;
>
>     };
>
>   };
>
> };
>
> };
>
> …..
>
> *From:* Дмитрий Понаморев 
> *Sent:* Monday, August 16, 2021 8:17 PM
> *To:* Sumo 
> *Cc:* Jay Talbott ; Coreboot <
> [e-mail]>
> *Subject:* [coreboot] Re: A different lapic number in devicetree.cb
> needed for CPU with the same SKU and steping (Intel Atom C3538).
>
> Hi Sumo,
> > ... It’s also possible (but not confirmed) that for a particular SKU
>
> (other than 16-core SKUs), it might not be consistent between parts
>
> I can confirm this, I have two C3558 SoC's with first core different APID
> ID's...
>
> I can confirm it too for C3338, C3538.
>
> Do you think I can submit my patch (see previous discussions) or do we
> have a better solution?
>
> Your patch works great for me ( for C3338, C3538, C3758, C3958).
>
> Thanks again! (from this thread:
> https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/MCRPR...
> )
>
> пн, 16 авг. 2021 г. в 20:58, Sumo :
>
> Hi Jay,
> > ... It’s also possible (but not confirmed) that for a particular SKU
>
> (other than 16-core SKUs), it might not be consistent between parts
>
> I can confirm this, I have two C3558 SoC's with first core different APID
> ID's...
>
> Do you think I can submit my patch (see previous discussions) or do we
> have a better solution?
>
> Kind regards,
>
> Sumo
>
> On Mon, Aug 16, 2021 at 1:45 PM Jay Talbott <
> [e-mail]> wrote:
>
> Unfortunately, for the Denverton SoC (C3000 series), the APIC ID of the
> first core is not always the same. For 16-core SKUs, it’s always 0, but for
> SKUs with a lower number of cores, it may be a different number. It’s also
> possible (but not confirmed) that for a particular SKU (other than 16-core
> SKUs), it might not be consistent between parts. The solution is to
> basically ignore the value in devicetree and use the actual APIC ID from
> the first core.
> Jay
>
> *From:* Sumo [mailto:[e-mail]]
> *Sent:* Monday, August 16, 2021 9:15 AM
> *To:* Nico Huber
> *Cc:* Дмитрий Понаморев; Coreboot
> *Subject:* [coreboot] Re: A different lapic number in devicetree.cb
> needed for CPU with the same SKU and steping (Intel Atom C3538).
>
> Hi,
> > have you tried omitting the `device lapic` line from the devicetree?
>
> I have tested this, in this case Linux shows only one processor core.
> Therefore the 'device lapic' line is really needed...
>
> I can submit that Local APIC Fixup patch to gerrit but I'm not sure if
> this is really the best solution.
>
> Kind regards,
>
> Sumo
>
> On Wed, Oct 7, 2020 at 6:35 PM Nico Huber  wrote:
>
> Hi,
>
> have you tried omitting the `device lapic` line from the devicetree?
> It would only matter if there is configuration associated with it, but
> I can't see anything like that for `intel/harcuvar`.
>
> What happens is that this `device lapic` line in the devicetree becomes
> an entry in a list at runtime. This list is later filled with the actual
> cores present. If any of the actual cores has the same APIC id as given
> in the devicetree, no additional entry is added for this core. However
> if none of the actual cores has that id, the original entry is left
> blindly in the list, causing coreboot to report the spurious, fifth
> core.
>
> On 07.10.20 21:27, [e-mail] wrote:
> > Thank you so much Javier Galindo!
> >
> > Sorry for not finding this case myself ...
> > I checked it on the motherboard with lapic #4 - everything works as it
>
> should.
> > Tomorrow I'll check it on the motherboard with lapic #0.
> > I wish I could understand how this magic works :)! lapic 0xbeef .....
>
> It's kind of a wildcard that gets replaced with the number found in the
> hardware. Nothing too special but probably unnecessary.
>
> Nico
> _______________________________________________
> coreboot mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
>
> Intel Research and Development Ireland Limited
> Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
>
> This e-mail and any attachments may contain confidential material for the
> sole use of the intended recipient(s). Any review or distribution by others
> is strictly prohibited. If you are not the intended recipient, please
> contact the sender and delete all copies.

[Szafranski, MariuszX]

Hi Dimitry,

I can see it and my proposition isn`t “something new” (still updating lapicid with value detected while booting)
Logically little different (keeping 0 in devicetree.cb) without introducing “0xbeef magic” and avoiding future questions like below:
> I wish I could understand how this magic works :)! lapic 0xbeef .....

As it is DNV specific it is just moved to DNV subdirectory without touching coreboot core.

BR,
Mariusz

From: Дмитрий Понаморев 
Sent: Friday, August 20, 2021 11:40 PM
To: Szafranski, MariuszX 
Cc: Sumo ; Jay Talbott ; Coreboot 
Subject: Re: [coreboot] Re: A different lapic number in devicetree.cb needed for CPU with the same SKU and steping (Intel Atom C3538).

Hi

I don't understand why to invent something new. Sumo's patch works great. Why can't you use it?
This Intel Atom Denverton C3000 processor has a maximum of 16 cores (4 clusters of 4 processors each).
The problem with the lapic ID is real for processors with less than 16 cores, especially for 2 and 4 cores - these processors are also 16 only with disabled cores in clusters.
Intel turns them off randomly and therefore the value lapic ID floats.
Changing this parameter as you suggest will suit you if you have one board - if there are many of them, it will not work.

Best regards,
Dmitry Ponamorev

пт, 20 авг. 2021 г. в 23:56, Szafranski, MariuszX <[e-mail]>:
Hi,

Maybe keep 0 for apicid in devicetree.cb and add something like below to denverton soc_init (or define separate function for check and fixup)  in src/soc/intel/denverton_ns/chip.c.
Could anyone with apicid != 0 test and let us know?
BR,
Mariusz

static void soc_init(void *data)
{
  unsigned long bsp_lapicid = lapicid();
  struct device *dev;

if (bsp_lapicid){
    for (dev = &dev_root, cnt = 0; dev; dev = dev->next){
      if ((dev->path.type == DEVICE_PATH_APIC){
        if (bsp_lapicid != dev->path.apic.apic_id){
          printk(BIOS_SPEW,
            "soc_init: BSP lapic ID = 0x%lx - updating in devicetree\n",
            bsp_lapicid);
          dev->path.apic.apic_id = bsp_lapicid;
        } else {
          break;
        };
      };
    };
  };
…..

From: Дмитрий Понаморев <[e-mail]>
Sent: Monday, August 16, 2021 8:17 PM
To: Sumo <[e-mail]>
Cc: Jay Talbott <[e-mail]>; Coreboot <[e-mail]>
Subject: [coreboot] Re: A different lapic number in devicetree.cb needed for CPU with the same SKU and steping (Intel Atom C3538).

Hi Sumo,
> ... It’s also possible (but not confirmed) that for a particular SKU (other than 16-core SKUs), it might not be consistent between parts

I can confirm this, I have two C3558 SoC's with first core different APID ID's...

I can confirm it too for C3338, C3538.

Do you think I can submit my patch (see previous discussions) or do we have a better solution?

Your patch works great for me ( for C3338, C3538, C3758, C3958).
Thanks again! (from this thread: https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/MCRPR... )

пн, 16 авг. 2021 г. в 20:58, Sumo <[e-mail]>:
Hi Jay,
> ... It’s also possible (but not confirmed) that for a particular SKU (other than 16-core SKUs), it might not be consistent between parts

I can confirm this, I have two C3558 SoC's with first core different APID ID's...

Do you think I can submit my patch (see previous discussions) or do we have a better solution?

Kind regards,
Sumo

On Mon, Aug 16, 2021 at 1:45 PM Jay Talbott <[e-mail]> wrote:
Unfortunately, for the Denverton SoC (C3000 series), the APIC ID of the first core is not always the same. For 16-core SKUs, it’s always 0, but for SKUs with a lower number of cores, it may be a different number. It’s also possible (but not confirmed) that for a particular SKU (other than 16-core SKUs), it might not be consistent between parts. The solution is to basically ignore the value in devicetree and use the actual APIC ID from the first core.

- Jay

From: Sumo [mailto:[e-mail]]
Sent: Monday, August 16, 2021 9:15 AM
To: Nico Huber
Cc: Дмитрий Понаморев; Coreboot
Subject: [coreboot] Re: A different lapic number in devicetree.cb needed for CPU with the same SKU and steping (Intel Atom C3538).

Hi,
> have you tried omitting the `device lapic` line from the devicetree?

I have tested this, in this case Linux shows only one processor core. Therefore the 'device lapic' line is really needed...

I can submit that Local APIC Fixup patch to gerrit but I'm not sure if this is really the best solution.

Kind regards,
Sumo

On Wed, Oct 7, 2020 at 6:35 PM Nico Huber <[e-mail]> wrote:
Hi,

have you tried omitting the `device lapic` line from the devicetree?
It would only matter if there is configuration associated with it, but
I can't see anything like that for `intel/harcuvar`.

What happens is that this `device lapic` line in the devicetree becomes
an entry in a list at runtime. This list is later filled with the actual
cores present. If any of the actual cores has the same APIC id as given
in the devicetree, no additional entry is added for this core. However
if none of the actual cores has that id, the original entry is left
blindly in the list, causing coreboot to report the spurious, fifth
core.

On 07.10.20 21:27, [e-mail] wrote:
> Thank you so much Javier Galindo!
>
> Sorry for not finding this case myself ...
> I checked it on the motherboard with lapic #4 - everything works as it should.
> Tomorrow I'll check it on the motherboard with lapic #0.
> I wish I could understand how this magic works :)! lapic 0xbeef .....

It's kind of a wildcard that gets replaced with the number found in the
hardware. Nothing too special but probably unnecessary.

Nico
_______________________________________________
coreboot mailing list -- [e-mail]
To unsubscribe send an email to [e-mail]

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263

This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

[Furquan Shaikh]

On Fri, Aug 20, 2021 at 4:15 PM Szafranski, MariuszX
 wrote:
> Hi Dimitry,
>
> I can see it and my proposition isn`t “something new” (still updating lapicid with value detected while booting)
>
> Logically little different (keeping 0 in devicetree.cb) without introducing “0xbeef magic” and avoiding future questions like below:
> > I wish I could understand how this magic works :)! lapic 0xbeef .....
>
> As it is DNV specific it is just moved to DNV subdirectory without touching coreboot core.
>
> BR,
>
> Mariusz
>
> From: Дмитрий Понаморев 
> Sent: Friday, August 20, 2021 11:40 PM
> To: Szafranski, MariuszX 
> Cc: Sumo ; Jay Talbott ; Coreboot 
> Subject: Re: [coreboot] Re: A different lapic number in devicetree.cb needed for CPU with the same SKU and steping (Intel Atom C3538).
>
> Hi
>
> I don't understand why to invent something new. Sumo's patch works great. Why can't you use it?
>
> This Intel Atom Denverton C3000 processor has a maximum of 16 cores (4 clusters of 4 processors each).
>
> The problem with the lapic ID is real for processors with less than 16 cores, especially for 2 and 4 cores - these processors are also 16 only with disabled cores in clusters.
>
> Intel turns them off randomly and therefore the value lapic ID floats.
>
> Changing this parameter as you suggest will suit you if you have one board - if there are many of them, it will not work.
>
> Best regards,
>
> Dmitry Ponamorev
>
> пт, 20 авг. 2021 г. в 23:56, Szafranski, MariuszX :
>
> Hi,
>
> Maybe keep 0 for apicid in devicetree.cb and add something like below to denverton soc_init (or define separate function for check and fixup)  in src/soc/intel/denverton_ns/chip.c.
>
> Could anyone with apicid != 0 test and let us know?
>
> BR,
>
> Mariusz
>
> static void soc_init(void *data)
>
> {
>
> unsigned long bsp_lapicid = lapicid();
>
> struct device *dev;
>
> if (bsp_lapicid){
> for (dev = &dev_root, cnt = 0; dev; dev = dev->next){
>
>   if ((dev->path.type == DEVICE_PATH_APIC){
>
>     if (bsp_lapicid != dev->path.apic.apic_id){
>
>       printk(BIOS_SPEW,
>
>         "soc_init: BSP lapic ID = 0x%lx - updating in devicetree\n",
>
>         bsp_lapicid);
>
>       dev->path.apic.apic_id = bsp_lapicid;
>
>     } else {
>
>       break;
>
>     };
>
>   };
>
> };
>
> };
>
> …..
>
> From: Дмитрий Понаморев 
> Sent: Monday, August 16, 2021 8:17 PM
> To: Sumo 
> Cc: Jay Talbott ; Coreboot 
> Subject: [coreboot] Re: A different lapic number in devicetree.cb needed for CPU with the same SKU and steping (Intel Atom C3538).
>
> Hi Sumo,
> > ... It’s also possible (but not confirmed) that for a particular SKU (other than 16-core SKUs), it might not be consistent between parts
>
> I can confirm this, I have two C3558 SoC's with first core different APID ID's...
>
> I can confirm it too for C3338, C3538.
>
> Do you think I can submit my patch (see previous discussions) or do we have a better solution?
>
> Your patch works great for me ( for C3338, C3538, C3758, C3958).
>
> Thanks again! (from this thread: https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/MCRPR... )
>
> пн, 16 авг. 2021 г. в 20:58, Sumo :
>
> Hi Jay,
> > ... It’s also possible (but not confirmed) that for a particular SKU (other than 16-core SKUs), it might not be consistent between parts
>
> I can confirm this, I have two C3558 SoC's with first core different APID ID's...
>
> Do you think I can submit my patch (see previous discussions) or do we have a better solution?
>
> Kind regards,
>
> Sumo
>
> On Mon, Aug 16, 2021 at 1:45 PM Jay Talbott  wrote:
>
> Unfortunately, for the Denverton SoC (C3000 series), the APIC ID of the first core is not always the same. For 16-core SKUs, it’s always 0, but for SKUs with a lower number of cores, it may be a different number. It’s also possible (but not confirmed) that for a particular SKU (other than 16-core SKUs), it might not be consistent between parts. The solution is to basically ignore the value in devicetree and use the actual APIC ID from the first core.
> Jay
>
> From: Sumo [mailto:[e-mail]]
> Sent: Monday, August 16, 2021 9:15 AM
> To: Nico Huber
> Cc: Дмитрий Понаморев; Coreboot
> Subject: [coreboot] Re: A different lapic number in devicetree.cb needed for CPU with the same SKU and steping (Intel Atom C3538).
>
> Hi,
> > have you tried omitting the `device lapic` line from the devicetree?
>
> I have tested this, in this case Linux shows only one processor core. Therefore the 'device lapic' line is really needed...

Can you please try dropping `device lapic` from the devicetree along
with this patch:

diff --git a/src/soc/intel/denverton_ns/cpu.c b/src/soc/intel/denverton_ns/cpu.c
index 1dc0830d86..ecefd3a987 100644
--- a/src/soc/intel/denverton_ns/cpu.c
+++ b/src/soc/intel/denverton_ns/cpu.c
@@ -287,6 +287,9 @@ static const struct mp_ops mp_ops = {

void denverton_init_cpus(struct device *dev)
 {
+       if (!dev->link_list)
+               add_more_links(dev, 1);
+
        /* Clear for take-off */
        if (mp_init_with_smm(dev->link_list, &mp_ops) < 0)
                printk(BIOS_ERR, "MP initialization failure.\n");

I think once you drop the device from device tree, dev->link_list
would be NULL and so MP initialization failed for you. This problem is
not really unique to denverton and was fixed just a few days back for
other Intel SoCs using common/block/cpu/mp_init too:

https://review.coreboot.org/c/coreboot/+/56758
https://review.coreboot.org/c/coreboot/+/56852
> I can submit that Local APIC Fixup patch to gerrit but I'm not sure if this is really the best solution.
>
> Kind regards,
>
> Sumo
>
> On Wed, Oct 7, 2020 at 6:35 PM Nico Huber  wrote:
>
> Hi,
>
> have you tried omitting the `device lapic` line from the devicetree?
> It would only matter if there is configuration associated with it, but
> I can't see anything like that for `intel/harcuvar`.
>
> What happens is that this `device lapic` line in the devicetree becomes
> an entry in a list at runtime. This list is later filled with the actual
> cores present. If any of the actual cores has the same APIC id as given
> in the devicetree, no additional entry is added for this core. However
> if none of the actual cores has that id, the original entry is left
> blindly in the list, causing coreboot to report the spurious, fifth
> core.
>
> On 07.10.20 21:27, [e-mail] wrote:
> > Thank you so much Javier Galindo!
> >
> > Sorry for not finding this case myself ...
> > I checked it on the motherboard with lapic #4 - everything works as it should.
> > Tomorrow I'll check it on the motherboard with lapic #0.
> > I wish I could understand how this magic works :)! lapic 0xbeef .....
>
> It's kind of a wildcard that gets replaced with the number found in the
> hardware. Nothing too special but probably unnecessary.
>
> Nico
> _______________________________________________
> coreboot mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
>
> Intel Research and Development Ireland Limited
> Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
>
> This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
>
> Intel Research and Development Ireland Limited
> Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
>
> This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
>
> coreboot mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Szafranski, MariuszX]

Hi Furquan,

Thanks for pointing. I`ve missed this patch series.
Yeah omitting the `device lapic` line from the devicetree and adding this patch looks like correct (common) way to handle this issue.

BR,
Mariusz
> > have you tried omitting the `device lapic` line from the devicetree?
>
> I have tested this, in this case Linux shows only one processor core. Therefore the 'device lapic' line is really needed...

Can you please try dropping `device lapic` from the devicetree along with this patch:

diff --git a/src/soc/intel/denverton_ns/cpu.c b/src/soc/intel/denverton_ns/cpu.c
index 1dc0830d86..ecefd3a987 100644
--- a/src/soc/intel/denverton_ns/cpu.c
+++ b/src/soc/intel/denverton_ns/cpu.c
@@ -287,6 +287,9 @@ static const struct mp_ops mp_ops = {

void denverton_init_cpus(struct device *dev)  {
+       if (!dev->link_list)
+               add_more_links(dev, 1);
+
        /* Clear for take-off */
        if (mp_init_with_smm(dev->link_list, &mp_ops) < 0)
                printk(BIOS_ERR, "MP initialization failure.\n");

I think once you drop the device from device tree, dev->link_list would be NULL and so MP initialization failed for you. This problem is not really unique to denverton and was fixed just a few days back for other Intel SoCs using common/block/cpu/mp_init too:

https://review.coreboot.org/c/coreboot/+/56758
https://review.coreboot.org/c/coreboot/+/56852

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263

This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

[Sumo]

Hi Furquan,

I can confirm your proposal is working, could you please submit the patch
for review?

Thanks,
Sumo

On Fri, Aug 20, 2021 at 9:52 PM Szafranski, MariuszX <
[e-mail]> wrote:
> Hi Furquan,
>
> Thanks for pointing. I`ve missed this patch series.
> Yeah omitting the `device lapic` line from the devicetree and adding this
> patch looks like correct (common) way to handle this issue.
>
> BR,
> Mariusz
> > > have you tried omitting the `device lapic` line from the devicetree?
> >
> > I have tested this, in this case Linux shows only one processor core.
>
> Therefore the 'device lapic' line is really needed...
>
> Can you please try dropping `device lapic` from the devicetree along with
> this patch:
>
> diff --git a/src/soc/intel/denverton_ns/cpu.c
> b/src/soc/intel/denverton_ns/cpu.c
> index 1dc0830d86..ecefd3a987 100644
> --- a/src/soc/intel/denverton_ns/cpu.c
> +++ b/src/soc/intel/denverton_ns/cpu.c
> @@ -287,6 +287,9 @@ static const struct mp_ops mp_ops = {
>
> void denverton_init_cpus(struct device *dev)  {
>   if (!dev->link_list)
>           add_more_links(dev, 1);
>   /* Clear for take-off */
>   if (mp_init_with_smm(dev->link_list, &mp_ops) < 0)
>           printk(BIOS_ERR, "MP initialization failure.\n");
>
> I think once you drop the device from device tree, dev->link_list would be
> NULL and so MP initialization failed for you. This problem is not really
> unique to denverton and was fixed just a few days back for other Intel SoCs
> using common/block/cpu/mp_init too:
>
> https://review.coreboot.org/c/coreboot/+/56758
> https://review.coreboot.org/c/coreboot/+/56852
>
> Intel Research and Development Ireland Limited
> Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
>
> This e-mail and any attachments may contain confidential material for the
> sole
> use of the intended recipient(s). Any review or distribution by others is
> strictly prohibited. If you are not the intended recipient, please contact
> the
> sender and delete all copies.

[Furquan Shaikh]

Uploaded here: https://review.coreboot.org/c/coreboot/+/57152.

- Furquan

On Tue, Aug 24, 2021 at 7:00 AM Sumo  wrote:
> Hi Furquan,
>
> I can confirm your proposal is working, could you please submit the patch
> for review?
>
> Thanks,
> Sumo
>
> On Fri, Aug 20, 2021 at 9:52 PM Szafranski, MariuszX <
> [e-mail]> wrote:
> > Hi Furquan,
> >
> > Thanks for pointing. I`ve missed this patch series.
> > Yeah omitting the `device lapic` line from the devicetree and adding this
> > patch looks like correct (common) way to handle this issue.
> >
> > BR,
> > Mariusz
> > > > have you tried omitting the `device lapic` line from the devicetree?
> > >
> > > I have tested this, in this case Linux shows only one processor core.
> >
> > Therefore the 'device lapic' line is really needed...
> >
> > Can you please try dropping `device lapic` from the devicetree along with
> > this patch:
> >
> > diff --git a/src/soc/intel/denverton_ns/cpu.c
> > b/src/soc/intel/denverton_ns/cpu.c
> > index 1dc0830d86..ecefd3a987 100644
> > --- a/src/soc/intel/denverton_ns/cpu.c
> > +++ b/src/soc/intel/denverton_ns/cpu.c
> > @@ -287,6 +287,9 @@ static const struct mp_ops mp_ops = {
> >
> > void denverton_init_cpus(struct device *dev)  {
> >   if (!dev->link_list)
> >           add_more_links(dev, 1);
> >   /* Clear for take-off */
> >   if (mp_init_with_smm(dev->link_list, &mp_ops) < 0)
> >           printk(BIOS_ERR, "MP initialization failure.\n");
> >
> > I think once you drop the device from device tree, dev->link_list would
> > be NULL and so MP initialization failed for you. This problem is not really
> > unique to denverton and was fixed just a few days back for other Intel SoCs
> > using common/block/cpu/mp_init too:
> >
> > https://review.coreboot.org/c/coreboot/+/56758
> > https://review.coreboot.org/c/coreboot/+/56852
> >
> > Intel Research and Development Ireland Limited
> > Registered in Ireland
> > Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> > Registered Number: 308263
> >
> > This e-mail and any attachments may contain confidential material for the
> > sole
> > use of the intended recipient(s). Any review or distribution by others is
> > strictly prohibited. If you are not the intended recipient, please
> > contact the
> > sender and delete all copies.