Re: [PATCH 1/3] ipmi/bmc-sim: implement watchdog dont log flag

[Corey Minyard <corey@minyard.net>]

On Tue, Apr 01, 2025 at 08:37:19AM +1000, Nicholas Piggin wrote:
> On Mon Mar 31, 2025 at 11:13 PM AEST, Corey Minyard wrote:
> > On Mon, Mar 31, 2025 at 10:57:22PM +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 | 7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> >> index 6157ac71201..32161044c0b 100644
> >> --- a/hw/ipmi/ipmi_bmc_sim.c
> >> +++ b/hw/ipmi/ipmi_bmc_sim.c
> >> @@ -733,7 +733,12 @@ 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));
> >> +
> >> +    /* Log the expiry if the don't log bit is clear */
> >> +    if (!IPMI_BMC_WATCHDOG_GET_DONT_LOG(ibs)) {
> >> +        ibs->watchdog_expired |= (1 << IPMI_BMC_WATCHDOG_GET_USE(ibs));
> >> +    }
> >> +
> >
> > Are you sure this is correct?  The spec doesn't say what this means, but
> > I would assume this means "Don't add a system log" not "Don't set the
> > expiry happened bit".
> 
> From IPMI spec, Set Watchdog Timer command timer use field of byte 1
> says "timer use (logged on expiration when “don’t log” bit = 0b)".
> But it also says it should disable the timeout sensor event logging.
> I missed that part, I will see if I can make that work.

It doesn't currently add an event to the log, I don't think.  If you
want to add that, it's fine.

However, as it is, your change will cause the Get Watchdog Timer command
to return the wrong value for Timer Use Expiration flags.  It's not what
you want to do.

What bug are you trying to solve?

-corey