[coreboot] Re: Regarding Intel CPU frequency (Intel Denverton based)

[Sumo]

Hi Jay,

Regarding the APIC ID problem, besides updating devicetree.cb with the
correct value do you think something else is required?
I have created a patch to make the APIC ID of devicetree.cb "dynamic" for
Denverton SoC, so it can be updated during runtime, see below:

diff --git a/src/device/device.c b/src/device/device.c
index 02209d9e..8852270b 100644
--- a/src/device/device.c
+++ b/src/device/device.c
@@ -50,6 +50,9 @@
 #include <arch/ebda.h>
 #endif
 #include <timer.h>
+#if IS_ENABLED(CONFIG_SOC_INTEL_DENVERTON_NS)
+#include <cpu/x86/lapic.h>
+#endif

/** Linked list of ALL devices */
 struct device *all_devices = &dev_root;
@@ -983,6 +986,16 @@ void dev_enumerate(void)
 {
        struct device *root;

+#if IS_ENABLED(CONFIG_SOC_INTEL_DENVERTON_NS)
+       /* Bootstrap processor Local APIC fixup */
+       struct device *t = dev_find_lapic(0xbeef);
+       if (t) {
+               unsigned int apic_id = lapicid();
+               printk(BIOS_INFO, "Denverton-NS BSP Local APIC fixup:
lapic=%u\n", apic_id);
+               t->path.apic.apic_id = apic_id;
+       }
+#endif
+
        printk(BIOS_INFO, "Enumerating buses...\n");

root = &dev_root;
diff --git a/src/mainboard/intel/harcuvar/devicetree.cb
b/src/mainboard/intel/harcuvar/devicetree.cb
index 2e463c09..18654d42 100644
--- a/src/mainboard/intel/harcuvar/devicetree.cb
+++ b/src/mainboard/intel/harcuvar/devicetree.cb
@@ -45,7 +45,7 @@ chip soc/intel/denverton_ns
        register "ipc3" = "0x00000000" # IPC3

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

device domain 0 on

Thanks,
Sumo

On Fri, Apr 24, 2020 at 12:50 PM Jay Talbott <
[e-mail]> wrote:
> Nitin,
>
> We have encountered both of these issues and have corrected them in our own
> code base for a particular client. We are not in a position to upstream our
> changes, but I can tell you the source of each problem.
> CPU frequency: For Denverton SKUs that do NOT support turbo mode (like
>
> the one you are using), the code does not properly enable speedstep. The
> code needs to be modified to correctly enable speedstep in the case of a
> SKU
> that does not support turbo mode.
> Linux log: For Denverton SKUs with less than 16 cores, processor 0 is
>
> not
> guaranteed  to have an APIC ID of 0 (as specified in devicetree). If you
> know the APIC ID of processor 0 and change devicetree to match, the problem
> you are seeing will go away. However, that's not a generalized solution, as
> the APIC ID can be different from SKU to SKU and, I believe, even between
> different parts of the same SKU (other than 16-core SKUs). So the code
> needs
> to be modified to use the actual APIC ID of processor 0 instead of the
> fixed
> value specified in devicetree.
>
> The original code developed by Intel was tested using a Harcuvar CRB with a
> 16-core SKU that supports turbo mode, and that's why neither of these
> issues
> were discovered in the original implementation.
>
> Without actually looking at the code, I believe both of these can be fixed
> in src/soc/intel/denverton_ns/cpu.c.
> Jay
>
> Jay Talbott
> Principal Consulting Engineer
> SysPro Consulting, LLC
> 3057 E. Muirfield St.
> Gilbert, AZ 85298
> [phone]
> [phone] (FAX)
> [e-mail]
> http://www.sysproconsulting.com
> > -----Original Message-----
> > From: [e-mail] [mailto:[e-mail]]
> > Sent: Friday, April 24, 2020 5:35 AM
> > To: [e-mail]
> > Subject: [coreboot] Re: Regarding Intel CPU frequency (Intel Denverton
> > based)
> >
> > Hi Paul,
> >
> > As far as cpu init is concerned, I haven't modified the cpu
>
> initialization
> > sequence for the given board. The code is  located under following sub-
> > folder "src/soc/intel/denverton_ns/cpu.c".
> >
> > The given CPU (CPU C3558)  has 4 cores, and I am noticing the following
>
> logs
> > while booting up,
> > which I am trying to debug in parallel by inserting some delays.
> >
> > The wakeup fails for cpu (core) 1, but it continues and passes for 2,3,4.
> > So at the end, cores are getting recognized as CPU 0,2,3,4 respectively.
> >
> > There are few records available for the same sort of cases:
> > https://lore.kernel.org/lkml/0bc26e9524533c38fdbdc95eed2b1448@teksavv
> > y.com/T/
> >
> > "   1.620879] x86: Booting SMP configuration:
> > [    1.624592] .... node  #0, CPUs:      #1
> > [   11.624587] smpboot: do_boot_cpu failed(-1) to wakeup CPU#1
> > [   11.632919]  #2 #3 #4
> > [   11.636707] smp: Brought up 1 node, 4 CPUs
> > [   11.640585] smpboot: Max logical packages: 2
> > [   11.644585] ----------------
> > [   11.644587] | NMI testsuite:
> > [   11.644588] --------------------
> > [   11.644590]   remote IPI:  ok  |
> > [   11.644623]    local IPI:  ok  |
> > [   11.644642] --------------------
> > [   11.644644] Good, all   2 testcases passed! |
> > [   11.644646] ---------------------------------
> > [   11.644650] smpboot: Total of 4 processors activated (17600.00
>
> BogoMIPS)
> > "
> >
> > Thanks for you help.
> >
> > Thanks,
> > Nitin.
> > _______________________________________________
> > coreboot mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
>
> coreboot mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]