On Mon, Mar 31, 2025 at 06:03:11PM -0500, Corey Minyard wrote:
> 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 <npig...@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.
Actually, I'm wrong here. It does generate an event through the sensor
handling, the sensor_set_discrete_bit() function will cause an event to
be generated. However, you can't just not call that function, you need
the event set. I think the best way would be to add a "do_log"
parameter to that function to suppress the log in this case.
-corey
>
> 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
