Re: ipmi0: Watchdog set returned 0xc0 (releng_13)

On 15.09.2021 14:49, mike tancsa wrote:
> On 9/15/2021 11:23 AM, mike tancsa wrote:
>>
>> I also tried on a X11SSL-F
>>
>> ipmi0: IPMI device rev. 1, firmware rev. 1.60, version 2.0, device
>> support mask 0xbf
>> ipmi0: Number of channels 2
>> ipmi0: Attached watchdog
>> ipmi0: Establishing power cycle handler
>>
>>  # ipmitool sel list | tail -3
>>    6 | 08/20/2021 | 20:45:38 | Fan #0x45 | Lower Non-recoverable going
>> low  | Asserted
>>    7 | 09/15/2021 | 11:15:28 | Watchdog2 #0xca | Timer interrupt () |
>> Asserted
>>    8 | 09/15/2021 | 11:15:38 | Watchdog2 #0xca | Power cycle () | Asserted
>> #
>>
>> I have a RELENG_12 box in production I will try as well later, but so
>> far so good.  Thanks for fixing!
> 
> On RELENG_12 as of today, I still have the issue on a X11SCL-F board.

What issues do you have there exactly?

> Not sure if thats the board, RELENG_12 or other missing changes from
> HEAD or a combo there of ?
> 
> --- ipmi.c.prev 2021-09-15 14:39:28.943161000 -0400
> +++ ipmi.c      2021-09-15 14:39:50.997836000 -0400
> @@ -662,7 +662,8 @@
>                 req->ir_request[0] = IPMI_SET_WD_TIMER_DONT_STOP
>                     | IPMI_SET_WD_TIMER_SMS_OS;
>                 req->ir_request[1] = (wd_timer_actions & 0xff);
> -               req->ir_request[2] = (wd_pretimeout_countdown & 0xff);
> +               req->ir_request[2] = min(0xff,
> +                       min(wd_pretimeout_countdown, (sec + 2) / 4));
>                 req->ir_request[3] = 0; /* Timer use */
>                 req->ir_request[4] = (sec * 10) & 0xff;
>                 req->ir_request[5] = (sec * 10) >> 8;

I suppose this is what you merged down?  It by itself should fix the
specific issue that affected at least me.

> I did notice, there are other changes in the version in HEAD
> 
> @@ -27,16 +27,19 @@
>   */
>  
>  #include <sys/cdefs.h>
> -__FBSDID("$FreeBSD: stable/12/sys/dev/ipmi/ipmi.c 370303 2021-08-13
> 01:20:59Z git2svn $");
> +__FBSDID("$FreeBSD$");
>  
>  #include <sys/param.h>
>  #include <sys/systm.h>
>  #include <sys/bus.h>
>  #include <sys/condvar.h>
>  #include <sys/conf.h>
> +#include <sys/eventhandler.h>
>  #include <sys/kernel.h>
> +#include <sys/lock.h>
>  #include <sys/malloc.h>
>  #include <sys/module.h>
> +#include <sys/mutex.h>
>  #include <sys/poll.h>
>  #include <sys/reboot.h>
>  #include <sys/rman.h>
> @@ -93,11 +96,14 @@
>  static int wd_startup_countdown = 0; /* sec */
>  static int wd_pretimeout_countdown = 120; /* sec */
>  static int cycle_wait = 10; /* sec */
> +static int wd_init_enable = 1;
>  
> -static SYSCTL_NODE(_hw, OID_AUTO, ipmi, CTLFLAG_RD, 0,
> +static SYSCTL_NODE(_hw, OID_AUTO, ipmi, CTLFLAG_RD | CTLFLAG_MPSAFE, 0,
>      "IPMI driver parameters");
>  SYSCTL_INT(_hw_ipmi, OID_AUTO, on, CTLFLAG_RWTUN,
>         &on, 0, "");
> +SYSCTL_INT(_hw_ipmi, OID_AUTO, wd_init_enable, CTLFLAG_RWTUN,
> +       &wd_init_enable, 1, "Enable watchdog initialization");
>  SYSCTL_INT(_hw_ipmi, OID_AUTO, wd_timer_actions, CTLFLAG_RW,
>         &wd_timer_actions, 0,
>         "IPMI watchdog timer actions (including pre-timeout interrupt)");
> @@ -662,7 +668,8 @@
>                 req->ir_request[0] = IPMI_SET_WD_TIMER_DONT_STOP
>                     | IPMI_SET_WD_TIMER_SMS_OS;
>                 req->ir_request[1] = (wd_timer_actions & 0xff);
> -               req->ir_request[2] = (wd_pretimeout_countdown & 0xff);
> +               req->ir_request[2] = min(0xff,
> +                   min(wd_pretimeout_countdown, (sec + 2) / 4));
>                 req->ir_request[3] = 0; /* Timer use */
>                 req->ir_request[4] = (sec * 10) & 0xff;
>                 req->ir_request[5] = (sec * 10) >> 8;
> @@ -906,7 +913,7 @@
>          * Probe for watchdog, but only for backends which support
>          * polled driver requests.
>          */
> -       if (sc->ipmi_driver_requests_polled) {
> +       if (wd_init_enable && sc->ipmi_driver_requests_polled) {
>                 IPMI_INIT_DRIVER_REQUEST(req,
> IPMI_ADDR(IPMI_APP_REQUEST, 0),
>                     IPMI_GET_WDOG, 0, 0);
>  
> @@ -949,14 +956,14 @@
>         } else if (!on)
>                 (void)ipmi_set_watchdog(sc, 0);
>         /*
> -        * Power cycle the system off using IPMI. We use last - 1 since
> we don't
> +        * Power cycle the system off using IPMI. We use last - 2 since
> we don't
>          * handle all the other kinds of reboots. We'll let others
> handle them.
>          * We only try to do this if the BMC supports the Chassis device.
>          */
>         if (sc->ipmi_dev_support & IPMI_ADS_CHASSIS) {
>                 device_printf(dev, "Establishing power cycle handler\n");
>                 sc->ipmi_power_cycle_tag =
> EVENTHANDLER_REGISTER(shutdown_final,
> -                   ipmi_power_cycle, sc, SHUTDOWN_PRI_LAST - 1);
> +                   ipmi_power_cycle, sc, SHUTDOWN_PRI_LAST - 2);
>         }
>  }

This seems to be irrelevant.

-- 
Alexander Motin