If the dont-log flag is set in the 'timer use' field for the
'set watchdog' command, a watchdog timeout will not get logged as
a timer use expiration.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/ipmi/ipmi_bmc_sim.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 216bf8ff7f0..3cefc69f8a8 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -514,7 +514,8 @@ static void gen_event(IPMIBmcSim *ibs, unsigned int sens_num, uint8_t deassert,
static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
unsigned int bit, unsigned int val,
- uint8_t evd1, uint8_t evd2, uint8_t evd3)
+ uint8_t evd1, uint8_t evd2, uint8_t evd3,
+ bool do_log)
{
IPMISensor *sens;
uint16_t mask;
@@ -534,7 +535,7 @@ static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
return; /* Already asserted */
}
sens->assert_states |= mask & sens->assert_suppt;
- if (sens->assert_enable & mask & sens->assert_states) {
+ if (do_log && (sens->assert_enable & mask & sens->assert_states)) {
/* Send an event on assert */
gen_event(ibs, sensor, 0, evd1, evd2, evd3);
}
@@ -544,7 +545,7 @@ static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
return; /* Already deasserted */
}
sens->deassert_states |= mask & sens->deassert_suppt;
- if (sens->deassert_enable & mask & sens->deassert_states) {
+ if (do_log && (sens->deassert_enable & mask & sens->deassert_states)) {
/* Send an event on deassert */
gen_event(ibs, sensor, 1, evd1, evd2, evd3);
}
@@ -700,6 +701,7 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
{
IPMIInterface *s = ibs->parent.intf;
IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
+ bool do_log = !IPMI_BMC_WATCHDOG_GET_DONT_LOG(ibs);
if (!ibs->watchdog_running) {
goto out;
@@ -711,14 +713,16 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
ibs->msg_flags |= IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK;
k->do_hw_op(s, IPMI_SEND_NMI, 0);
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 8, 1,
- 0xc8, (2 << 4) | 0xf, 0xff);
+ 0xc8, (2 << 4) | 0xf, 0xff,
+ do_log);
break;
case IPMI_BMC_WATCHDOG_PRE_MSG_INT:
ibs->msg_flags |= IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK;
k->set_atn(s, 1, attn_irq_enabled(ibs));
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 8, 1,
- 0xc8, (3 << 4) | 0xf, 0xff);
+ 0xc8, (3 << 4) | 0xf, 0xff,
+ do_log);
break;
default:
@@ -734,28 +738,36 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
do_full_expiry:
ibs->watchdog_running = 0; /* Stop the watchdog on a timeout */
- ibs->watchdog_expired |= (1 << IPMI_BMC_WATCHDOG_GET_USE(ibs));
+
+ if (do_log) {
+ ibs->watchdog_expired |= (1 << IPMI_BMC_WATCHDOG_GET_USE(ibs));
+ }
+
switch (IPMI_BMC_WATCHDOG_GET_ACTION(ibs)) {
case IPMI_BMC_WATCHDOG_ACTION_NONE:
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 0, 1,
- 0xc0, ibs->watchdog_use & 0xf, 0xff);
+ 0xc0, ibs->watchdog_use & 0xf, 0xff,
+ do_log);
break;
case IPMI_BMC_WATCHDOG_ACTION_RESET:
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 1, 1,
- 0xc1, ibs->watchdog_use & 0xf, 0xff);
+ 0xc1, ibs->watchdog_use & 0xf, 0xff,
+ do_log);
k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
break;
case IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN:
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 2, 1,
- 0xc2, ibs->watchdog_use & 0xf, 0xff);
+ 0xc2, ibs->watchdog_use & 0xf, 0xff,
+ do_log);
k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
break;
case IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE:
sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 2, 1,
- 0xc3, ibs->watchdog_use & 0xf, 0xff);
+ 0xc3, ibs->watchdog_use & 0xf, 0xff,
+ do_log);
k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
break;
}
--
2.47.1On Tue, Apr 01, 2025 at 09:44:11PM +1000, Nicholas Piggin wrote:
> If the dont-log flag is set in the 'timer use' field for the
> 'set watchdog' command, a watchdog timeout will not get logged as
> a timer use expiration.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> hw/ipmi/ipmi_bmc_sim.c | 32 ++++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 216bf8ff7f0..3cefc69f8a8 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -514,7 +514,8 @@ static void gen_event(IPMIBmcSim *ibs, unsigned int sens_num, uint8_t deassert,
>
> static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
> unsigned int bit, unsigned int val,
> - uint8_t evd1, uint8_t evd2, uint8_t evd3)
> + uint8_t evd1, uint8_t evd2, uint8_t evd3,
> + bool do_log)
> {
> IPMISensor *sens;
> uint16_t mask;
> @@ -534,7 +535,7 @@ static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
> return; /* Already asserted */
> }
> sens->assert_states |= mask & sens->assert_suppt;
> - if (sens->assert_enable & mask & sens->assert_states) {
> + if (do_log && (sens->assert_enable & mask & sens->assert_states)) {
> /* Send an event on assert */
> gen_event(ibs, sensor, 0, evd1, evd2, evd3);
> }
> @@ -544,7 +545,7 @@ static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
> return; /* Already deasserted */
> }
> sens->deassert_states |= mask & sens->deassert_suppt;
> - if (sens->deassert_enable & mask & sens->deassert_states) {
> + if (do_log && (sens->deassert_enable & mask & sens->deassert_states)) {
> /* Send an event on deassert */
> gen_event(ibs, sensor, 1, evd1, evd2, evd3);
> }
> @@ -700,6 +701,7 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
> {
> IPMIInterface *s = ibs->parent.intf;
> IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
> + bool do_log = !IPMI_BMC_WATCHDOG_GET_DONT_LOG(ibs);
>
> if (!ibs->watchdog_running) {
> goto out;
> @@ -711,14 +713,16 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
> ibs->msg_flags |= IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK;
> k->do_hw_op(s, IPMI_SEND_NMI, 0);
> sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 8, 1,
> - 0xc8, (2 << 4) | 0xf, 0xff);
> + 0xc8, (2 << 4) | 0xf, 0xff,
> + do_log);
> break;
>
> case IPMI_BMC_WATCHDOG_PRE_MSG_INT:
> ibs->msg_flags |= IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK;
> k->set_atn(s, 1, attn_irq_enabled(ibs));
> sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 8, 1,
> - 0xc8, (3 << 4) | 0xf, 0xff);
> + 0xc8, (3 << 4) | 0xf, 0xff,
> + do_log);
> break;
>
> default:
> @@ -734,28 +738,36 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
>
> do_full_expiry:
> ibs->watchdog_running = 0; /* Stop the watchdog on a timeout */
> - ibs->watchdog_expired |= (1 << IPMI_BMC_WATCHDOG_GET_USE(ibs));
> +
> + if (do_log) {
> + ibs->watchdog_expired |= (1 << IPMI_BMC_WATCHDOG_GET_USE(ibs));
> + }
> +
This change needs to be removed. watchdog_expired has nothing to do
with logs, it's a field reporting in another message.
Other than that, this is good.
-corey
> switch (IPMI_BMC_WATCHDOG_GET_ACTION(ibs)) {
> case IPMI_BMC_WATCHDOG_ACTION_NONE:
> sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 0, 1,
> - 0xc0, ibs->watchdog_use & 0xf, 0xff);
> + 0xc0, ibs->watchdog_use & 0xf, 0xff,
> + do_log);
> break;
>
> case IPMI_BMC_WATCHDOG_ACTION_RESET:
> sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 1, 1,
> - 0xc1, ibs->watchdog_use & 0xf, 0xff);
> + 0xc1, ibs->watchdog_use & 0xf, 0xff,
> + do_log);
> k->do_hw_op(s, IPMI_RESET_CHASSIS, 0);
> break;
>
> case IPMI_BMC_WATCHDOG_ACTION_POWER_DOWN:
> sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 2, 1,
> - 0xc2, ibs->watchdog_use & 0xf, 0xff);
> + 0xc2, ibs->watchdog_use & 0xf, 0xff,
> + do_log);
> k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0);
> break;
>
> case IPMI_BMC_WATCHDOG_ACTION_POWER_CYCLE:
> sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 2, 1,
> - 0xc3, ibs->watchdog_use & 0xf, 0xff);
> + 0xc3, ibs->watchdog_use & 0xf, 0xff,
> + do_log);
> k->do_hw_op(s, IPMI_POWERCYCLE_CHASSIS, 0);
> break;
> }
> --
> 2.47.1
>
On Tue Apr 1, 2025 at 11:05 PM AEST, Corey Minyard wrote:
> On Tue, Apr 01, 2025 at 09:44:11PM +1000, Nicholas Piggin wrote:
>> If the dont-log flag is set in the 'timer use' field for the
>> 'set watchdog' command, a watchdog timeout will not get logged as
>> a timer use expiration.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> hw/ipmi/ipmi_bmc_sim.c | 32 ++++++++++++++++++++++----------
>> 1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
>> index 216bf8ff7f0..3cefc69f8a8 100644
>> --- a/hw/ipmi/ipmi_bmc_sim.c
>> +++ b/hw/ipmi/ipmi_bmc_sim.c
>> @@ -514,7 +514,8 @@ static void gen_event(IPMIBmcSim *ibs, unsigned int sens_num, uint8_t deassert,
>>
>> static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
>> unsigned int bit, unsigned int val,
>> - uint8_t evd1, uint8_t evd2, uint8_t evd3)
>> + uint8_t evd1, uint8_t evd2, uint8_t evd3,
>> + bool do_log)
>> {
>> IPMISensor *sens;
>> uint16_t mask;
>> @@ -534,7 +535,7 @@ static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
>> return; /* Already asserted */
>> }
>> sens->assert_states |= mask & sens->assert_suppt;
>> - if (sens->assert_enable & mask & sens->assert_states) {
>> + if (do_log && (sens->assert_enable & mask & sens->assert_states)) {
>> /* Send an event on assert */
>> gen_event(ibs, sensor, 0, evd1, evd2, evd3);
>> }
>> @@ -544,7 +545,7 @@ static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
>> return; /* Already deasserted */
>> }
>> sens->deassert_states |= mask & sens->deassert_suppt;
>> - if (sens->deassert_enable & mask & sens->deassert_states) {
>> + if (do_log && (sens->deassert_enable & mask & sens->deassert_states)) {
>> /* Send an event on deassert */
>> gen_event(ibs, sensor, 1, evd1, evd2, evd3);
>> }
>> @@ -700,6 +701,7 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
>> {
>> IPMIInterface *s = ibs->parent.intf;
>> IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
>> + bool do_log = !IPMI_BMC_WATCHDOG_GET_DONT_LOG(ibs);
>>
>> if (!ibs->watchdog_running) {
>> goto out;
>> @@ -711,14 +713,16 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
>> ibs->msg_flags |= IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK;
>> k->do_hw_op(s, IPMI_SEND_NMI, 0);
>> sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 8, 1,
>> - 0xc8, (2 << 4) | 0xf, 0xff);
>> + 0xc8, (2 << 4) | 0xf, 0xff,
>> + do_log);
>> break;
>>
>> case IPMI_BMC_WATCHDOG_PRE_MSG_INT:
>> ibs->msg_flags |= IPMI_BMC_MSG_FLAG_WATCHDOG_TIMEOUT_MASK;
>> k->set_atn(s, 1, attn_irq_enabled(ibs));
>> sensor_set_discrete_bit(ibs, IPMI_WATCHDOG_SENSOR, 8, 1,
>> - 0xc8, (3 << 4) | 0xf, 0xff);
>> + 0xc8, (3 << 4) | 0xf, 0xff,
>> + do_log);
>> break;
>>
>> default:
>> @@ -734,28 +738,36 @@ static void ipmi_sim_handle_timeout(IPMIBmcSim *ibs)
>>
>> do_full_expiry:
>> ibs->watchdog_running = 0; /* Stop the watchdog on a timeout */
>> - ibs->watchdog_expired |= (1 << IPMI_BMC_WATCHDOG_GET_USE(ibs));
>> +
>> + if (do_log) {
>> + ibs->watchdog_expired |= (1 << IPMI_BMC_WATCHDOG_GET_USE(ibs));
>> + }
>> +
>
> This change needs to be removed. watchdog_expired has nothing to do
> with logs, it's a field reporting in another message.
Okay you're right, I read through the spec again and yes I was
mistaken on this field.
Thanks,
Nick
© 2016 - 2025 Red Hat, Inc.