[Skiboot] [PATCH] core/init: set firmware progress sensor only on bmc systems.

[ppaidipe]

On 2016-11-03 15:59, Joel Stanley wrote:
> Hello Pridhiviraj,
> 
> On Thu, Nov 3, 2016 at 5:36 PM,  <ppaidipe at linux.vnet.ibm.com> wrote:
>> From: Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com>
>> 
>> Currently Hostboot populates /bmc/sensors dt node and corresponding 
>> sensors
>> only for BMC platforms, And for FSP platforms hostboot is not 
>> populating any
>> fsp sensors(Management sensors) and also there is no firmware progress
>> sensor exist in fsp platforms. due to which OPAL incorrectly setting 
>> firmware
>> status on a sensor id "00" which is not at all exist.
> 
> Nice catch. When we wrote this code the FSP lacked IPMI, so the check
> in ipmi_set_fw_progress_sensor for ipmi_present() was enough to guard
> against this.
> 
> Perhaps we should fix the checks inside the
> ipmi_set_fw_progress_sensor for the presence of the sensor in the
> device tree, instead of adding fsp_present() checks.
> 
> Cheers,
> 
> Joel
> 

Thanks Joel.
Sent a V2 with our findings, added the necessary checks in 
ipmi_set_fw_progress_sensor
function itself.


>> 
>> On a FSP system:
>>  cat /sys/firmware/opal/msglog | grep -i setting
>> [   21.189204883,6] IPMI: setting fw progress sensor 00 to 07
>> [   21.189559121,6] IPMI: setting fw progress sensor 00 to 13
>> cat /sys/firmware/opal/msglog | grep -i skiboot
>> [   84.127416495,5] SkiBoot skiboot-5.4.0-rc3 starting...
>> 
>> On a BMC system:
>> cat /sys/firmware/opal/msglog | grep -i setting
>> [    3.166286901,6] IPMI: setting fw progress sensor 05 to 14
>> [   14.259153338,6] IPMI: setting fw progress sensor 05 to 07
>> [   14.469070593,5] IPMI: Resetting boot count on successful boot
>> [   15.001210324,6] IPMI: setting fw progress sensor 05 to 13
>> 
>> So this patch fixes this incorrect setting on a fsp system, and sets 
>> only for BMC
>> platforms.
>> 
>> After patch:
>> On a FSP system:
>> cat /sys/firmware/opal/msglog | grep -i setting
>> 
>> On a BMC system:
>> cat /sys/firmware/opal/msglog | grep -i setting
>> [    3.167498705,6] IPMI: setting fw progress sensor 05 to 14
>> [   15.395841913,6] IPMI: setting fw progress sensor 05 to 07
>> [   16.117858189,5] IPMI: Resetting boot count on successful boot
>> [   16.163131535,6] IPMI: setting fw progress sensor 05 to 13
>> 
>> Need to enable this for FSP platforms once OPAL recieves sensors dt 
>> node
>> and fsp implements corresponding firmware progress sensor type(0x0f).
>> And also ipmi_sensor_init is only doing for ast bmc 
>> platforms(astbmc_init), and hence
>> there are sensors array created for FSP platforms.
>> 
>> Signed-off-by: Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com>
>> ---
>>  core/init.c | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>> 
>> diff --git a/core/init.c b/core/init.c
>> index 9d4ab60..4e9dcb8 100644
>> --- a/core/init.c
>> +++ b/core/init.c
>> @@ -481,7 +481,14 @@ void __noreturn load_and_boot_kernel(bool 
>> is_reboot)
>> 
>>         load_initramfs();
>> 
>> -       ipmi_set_fw_progress_sensor(IPMI_FW_OS_BOOT);
>> +       /* Currently hostboot populates only BMC IPMI sensors dt node
>> +        * and corresponding sensors, it is not populating fsp 
>> sensors.
>> +        * And also there is no firmware progress sensor available for 
>> fsp
>> +        * platforms. Enable this for fsp platforms once OPAL has 
>> corresponding
>> +        * dt node and corresponding sensor exist.
>> +        */
>> +        if (!fsp_present())
>> +                ipmi_set_fw_progress_sensor(IPMI_FW_OS_BOOT);
>> 
>>         if (!is_reboot) {
>>                 /* We wait for the nvram read to complete here so we 
>> can
>> @@ -930,7 +937,11 @@ void __noreturn __nomcount main_cpu_entry(const 
>> void *fdt)
>>         /* Add OPAL timer related properties */
>>         late_init_timers();
>> 
>> -       ipmi_set_fw_progress_sensor(IPMI_FW_PCI_INIT);
>> +       /* Enable this for fsp platforms once OPAL has corresponding
>> +         * sensors dt node and corresponding firmware progress sensor 
>> exist.
>> +         */
>> +        if (!fsp_present())
>> +                ipmi_set_fw_progress_sensor(IPMI_FW_PCI_INIT);
>> 
>>         /*
>>          * These last few things must be done as late as possible
>> --
>> 2.7.4
>> 
>> _______________________________________________
>> Skiboot mailing list
>> Skiboot at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/skiboot