| Message ID | 1478156802-405-1-git-send-email-ppaidipe@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
Hello Pridhiviraj, On Thu, Nov 3, 2016 at 5:36 PM, <ppaidipe@linux.vnet.ibm.com> wrote: > From: Pridhiviraj Paidipeddi <ppaidipe@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 > > 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@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@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
On 2016-11-03 15:59, Joel Stanley wrote: > Hello Pridhiviraj, > > On Thu, Nov 3, 2016 at 5:36 PM, <ppaidipe@linux.vnet.ibm.com> wrote: >> From: Pridhiviraj Paidipeddi <ppaidipe@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@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@lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/skiboot
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