Re: [coreboot] 答复: 答复: 答复: logical error for smm hander in coreboot.

[Zoran Stojsavljevic]

Hello Tank,

I am reading your messages and I am thinking, since the use case you are
presenting here does not make any sense to me.

You are managing DTS (Digital Thermal Sensor), this is something inherently
integrated on silicon die (I think per core), and there is no need to
manage it from SMM prospective. The gradient of Temperature is slow, you
can start checking it from ramstage every minimum 100ms, which is anyhow
great overkill (minimum should be 1000ms), in my opinion. Except if you are
not putting/on purpose missing heatsink on the top of CPU and experimenting
how long the CPU can work.

Yes, MSRs 0x19B/0x19C are General Purpose MSRs (presented in all CORE and
ATOM families), and they are accessible from anywhere, anytime,

I am not sure, but there is also built-in HW T monitor which, while
detecting ~5C below T junction, issues automatically PROCHOT# active low,
which shutdowns all cores and platform to S5 state (PROCHOT# signal should
go to EC or PMIC as well).

PROCHOT# :
https://www.manualslib.com/manual/407246/Intel-Celeron-Processor-E3000-Therm...

Other that that, in Linux you can have a thermal application (user space)
which will every second scan DTS, and show you the graphical value (I
guess, INTEL for this has so-called TAT tool (also works as test load
balancer for cores and integrated GFX), but there are also other apps for
monitoring as well).

Why you need to monitor DTS in SMM, that is my question (seems completely
unnecessary)? What for? You are NOT going to apply turbo mode in Coreboot,
as well as play performance boost with TPD1 to TDP2 (not wise at all to do
these boosts in BSPs)?!

Thank you,
Zoran

On Thu, Jan 5, 2017 at 6:20 AM, Tang Tank 
wrote:
> Hi Aaron,
>
> Yes, I do this as you say.
>    Add a ID like CBMEM_ID_MP_SMM(cbmem_add (CBMEM_ID_MP_SMM,
> sync_data_size)), and pass addr to      smm_runtime when smm relocation.
>   It seems ok to install and boot win10/boot ubuntu, but it will hang when
> install ubuntu.
>   I have none debugging methods for this issue. So I hope to define a
> static variable to do this and have a try.
>
> Thanks
>  Tank
>
> 发件人: Aaron Durbin 
> 发送时间: 2017年1月4日 19:47
> 收件人: Tang Tank
> 抄送: Nico Huber; Zoran Stojsavljevic; coreboot
> 主题: Re: 答复: 答复: [coreboot] logical error for smm hander in coreboot.
>
> On Wed, Jan 4, 2017 at 7:12 PM, Tang Tank 
> wrote:
> > Hi Nico,
> >
> > The MSRs can access and modify at any time. But DTS feature need to
>
> done in smm.
> > I need to check this msr 0x19b/0x19c and  set the trigger temperature
>
> dynamically in smm.
> > I will set a software gpe to notify that the _tmp method vaule in the
>
> thermal zone has been changed when a dts smi occured.
> > And then OS will change the speed of fan(active temperature), power
>
> and freq of cpus (passive temperature) or shutdown (critial temperature).
> > So do you know the way to define a static variable which can been used
>
> under smm stage and ram stage?
> > Now I reserved a fixed memory addr in ramstage, hard code the pointer
>
> to the addr in smi hander and complete the dts feature in win10
> successfully.
> > But I found that this will cause a hang when boot linux OSs such as
>
> ubuntu.
>
> You can't just use a fixed memory address w/o reserving the memory
> from the OS. If you need to read and write this memory from SMM and
> ASL I suggest you add the memory to cbmem which will reserve it from
> the OS. From there you can reference the memory in ASL and from SMM
> (once both are in told about the memory address).
> > Thanks
> > Tank
> >
> > 发件人: Nico Huber 
> > 发送时间: 2017年1月4日 7:37
> > 收件人: Tang Tank; Aaron Durbin; Zoran Stojsavljevic
> > 抄送: coreboot
> > 主题: Re: 答复: [coreboot] logical error for smm hander in coreboot.
> >
> > Hi Tank,
> >
> > On 04.01.2017 02:39, Tang Tank wrote:
> > > Hi Nico , Aaron, Zoran,
> > >
> > > Thank you for your help!
> > >    Smi_sts will been cleared after southbridge_smi_handler runs.
> > >    I am working for intel apollake now, need to do a thermal feature
>
> named DTS(Digital Thermal Sensor).
> > > This need to check and modify  MSR in smm for every core .
> >
> > is this MSR only accessible from SMM? or where does the requirement to
> > do it in SMM come from? It sounds like something that's better done in
> > the OS or ACPI.
> >
> > Nico
> > > And when I add a mechanism to sync up bsp and aps like below struct
>
> , I found that I can't define and use the same static variable in ramstage
> and smmstage.
> > > How can I do this?
> > >
> > > typedef struct {
> > >       spin_lock                           busy;
> > >       volatile ap_procedure    procedure;
> > >       volatile void                     *parameter;
> > >       volatile u32                       run;
> > >       volatile bool                     present;
> > >    } smm_cpu_data;
> > >
> > > typedef struct {
> > >      smm_cpu_data               *cpu_data;
> > >      volatile u8                         cpu_count;
> > >      volatile u32                       bsp_index; //we will assusme
>
> index 0 as bsp
> > >  volatile bool                      all_cores_is_sync;
> > >  volatile u64                      time_out_ticker;
> > >
> > > } smm_mp_sync_data;
> > >
> > > Thanks,
> > >  Tank
> > >
> > > 发件人: Aaron Durbin 
> > > 发送时间: 2017年1月3日 7:23
> > > 收件人: Nico Huber
> > > 抄送: Tang Tank; coreboot
> > > 主题: Re: [coreboot] logical error for smm hander in coreboot.
> > >
> > > On Tue, Jan 3, 2017 at 6:37 AM, Nico Huber
>
> wrote:
> > > > Hi Tank,
> > > >
> > > > On 03.01.2017 04:20, Tang Tank wrote:
> > > > > Hi all,
> > > > > For smm handler (func smm_handler_start) in
>
> coreboot/src/cpu/x86/smm/smm_module_handler.c,
> > > > > there may be a logical error.
> > > > > If I have 4 cores in my mainboard in the following special
>
> conditions:
> > > > > 1. core0 run into smm_handler_start and smi_handler_status will
>
> be SMI_LOCKED.
> > > > > 2. core1/2 run into smm_handler_start and pause since
>
> smi_handler_status is locked.
> > > > > 3. core0 release smi_handler_status and then core4 run into
>
> smm_handler_start, it will do
> > > > >     southbridge_smi_handler again.
> > > >
> > > > looking at the code, I too see that possible race. But...
> > > > > So is there have a reliable way of doing smm multi processors
>
> services in coreboot?
> > > > ...IMO, this isn't unreliable. After the first run of a cpu/northbridge
> > > > /southbridge_smi_handler() function, the cause of the SMI should be
> > > > handled and thus the second run should do nothing (but checking for the
> > > > SMI's cause).
> > > >
> > > > Does that help? I wasn't sure if I got your question right.
> > >
> > > Or you have to wait for all the CPUs to check in before dealing with
> > > the SMM handler logic. One would also have to deal with SMIs that were
> > > not broadcast since the other CPUs would never check in.
> > > > Nico
>
> coreboot Info Page
> www.coreboot.org
> coreboot project mailing list. To see the collection of prior postings to
> the list, visit the coreboot Archives. Using coreboot
> > coreboot Info Page
> > www.coreboot.org
> > coreboot project mailing list. To see the collection of prior postings
>
> to the list, visit the coreboot Archives. Using coreboot
> > > coreboot Info Page
> > > www.coreboot.org
> > > coreboot project mailing list. To see the collection of prior postings
>
> to the list, visit the coreboot Archives. Using coreboot
> >