Re: [PATCH] ipmi:kcs: Rate limit the invalid state warning

[Corey Minyard]

On Fri, Oct 30, 2026 at 03:29:45AM -0700, Breno Leitao wrote:
> start_kcs_transaction() warns each time it is asked to start a
> transaction while the KCS state machine is busy. The warning is not
> rate limited, so if the SI layer believes the interface is idle while
> the state machine is still running, every message sent to the BMC logs
> another line.
> 
> Kernels carrying commit c608966f3f9c ("ipmi: fix msg stack when IPMI is
> disconnected") hit this. When the interface is declared hosed, the
> state machine is reset to KCS_ERROR0 rather than KCS_IDLE, and keeps
> walking the error states while the SI layer, which treats it as idle,
> tries to start new messages.
> 
> One host logged 29K of these lines in four days, several per second:
> 
>   [Thu Oct 29 14:55:54 2026] ipmi_si dmi-ipmi-si.0: KCS in invalid state 9
>   [Thu Oct 29 14:55:55 2026] ipmi_si dmi-ipmi-si.0: KCS in invalid state 7
>   [Thu Oct 29 14:55:55 2026] ipmi_si dmi-ipmi-si.0: KCS in invalid state 8
>   [Thu Oct 29 14:55:55 2026] ipmi_si dmi-ipmi-si.0: KCS in invalid state 9
>   [Thu Oct 29 14:55:55 2026] ipmi_si dmi-ipmi-si.0: KCS in invalid state 7

The rate limiting is a good idea.  I've included the patch.

However, it also sounds like you don't have working IPMI hardware on
these systems.  The change you mention just exacerbates the problem
here, but I think it would be a good idea to track down why this is
happening.

If you really have working IPMI hardware, then it would be a good
idea to trace down why this is happening.

If you don't have working IPMI hardware, then it sounds like your
system is advertising that it has good hardware through SMBIOS or
ACPI.  It would be good to either fix your firmware or add a deny
for this particular system.

In either case, this points to a bug in the driver; it shouldn't try to
start a transaction while in error state.  I can't see it, but I believe
I can reproduce in simulation.

What specific kernel version are you running?

BTW, I need a link to the discussion about this so I can add a Closes:
tag to the git change.

Thanks,

-corey

> 
> Across hosts running such kernels at Meta fleet, about 887 million of
> these lines were logged in one week, all for states 6 to 9 (KCS_ERROR0
> to KCS_ERROR3).
> 
> That commit was reverted by commit 5d09ee1bec87 ("Revert "ipmi: fix msg
> stack when IPMI is disconnected""), but nothing bounds the warning if
> the two layers get out of sync again.
> 
> Use dev_warn_ratelimited(), as the SI layer already does for its
> transaction warnings, given it is cheap and will protect us from this
> crazy flood.
> 
> Reported-by: Dan Melnic <dmm@xxxxxxxx>
> Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>
> ---
>  drivers/char/ipmi/ipmi_kcs_sm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_kcs_sm.c b/drivers/char/ipmi/ipmi_kcs_sm.c
> index c89055db39e52e..ae0399ccd82196 100644
> --- a/drivers/char/ipmi/ipmi_kcs_sm.c
> +++ b/drivers/char/ipmi/ipmi_kcs_sm.c
> @@ -271,7 +271,8 @@ static int start_kcs_transaction(struct si_sm_data *kcs, unsigned char *data,
>  		return IPMI_REQ_LEN_EXCEEDED_ERR;
>  
>  	if ((kcs->state != KCS_IDLE) && (kcs->state != KCS_HOSED)) {
> -		dev_warn(kcs->io->dev, "KCS in invalid state %d\n", kcs->state);
> +		dev_warn_ratelimited(kcs->io->dev, "KCS in invalid state %d\n",
> +				     kcs->state);
>  		return IPMI_NOT_IN_MY_STATE_ERR;
>  	}
>  
> 
> ---
> base-commit: a39c950806213f6861d00f90729acaaa21f2d95a
> base-tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> change-id: 20261030-ipmi-a2223415ce84
> 
> Best regards,
> --  
> Breno Leitao <leitao@xxxxxxxxxx>
>