| Age | Commit message (Collapse) | Author |
|---|
|
Pull IPMI updates from Corey Minyard:
"Minor IPMI fixes:
- Some device tree cleanups and a maintainer add
- Fix a race when handling channel updates that could result in
errors being reported to the user in some cases"
* tag 'for-linus-6.19-1' of https://github.com/cminyard/linux-ipmi:
MAINTAINERS: Add entry on Loongson-2K IPMI driver
dt-bindings: ipmi: Convert aspeed,ast2400-ibt-bmc to DT schema
dt-bindings: ipmi: Convert nuvoton,npcm750-kcs-bmc to DT schema
ipmi: Skip channel scan if channels are already marked ready
ipmi: Fix __scan_channels() failing to rescan channels
ipmi: Fix the race between __scan_channels() and deliver_response()
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux
Pull printk updates from Petr Mladek:
- Allow creaing nbcon console drivers with an unsafe write_atomic()
callback that can only be called by the final nbcon_atomic_flush_unsafe().
Otherwise, the driver would rely on the kthread.
It is going to be used as the-best-effort approach for an
experimental nbcon netconsole driver, see
https://lore.kernel.org/r/20251121-nbcon-v1-2-503d17b2b4af@debian.org
Note that a safe .write_atomic() callback is supposed to work in NMI
context. But some networking drivers are not safe even in IRQ
context:
https://lore.kernel.org/r/oc46gdpmmlly5o44obvmoatfqo5bhpgv7pabpvb6sjuqioymcg@gjsma3ghoz35
In an ideal world, all networking drivers would be fixed first and
the atomic flush would be blocked only in NMI context. But it brings
the question how reliable networking drivers are when the system is
in a bad state. They might block flushing more reliable serial
consoles which are more suitable for serious debugging anyway.
- Allow to use the last 4 bytes of the printk ring buffer.
- Prevent queuing IRQ work and block printk kthreads when consoles are
suspended. Otherwise, they create non-necessary churn or even block
the suspend.
- Release console_lock() between each record in the kthread used for
legacy consoles on RT. It might significantly speed up the boot.
- Release nbcon context between each record in the atomic flush. It
prevents stalls of the related printk kthread after it has lost the
ownership in the middle of a record
- Add support for NBCON consoles into KDB
- Add %ptsP modifier for printing struct timespec64 and use it where
possible
- Misc code clean up
* tag 'printk-for-6.19' of git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux: (48 commits)
printk: Use console_is_usable on console_unblank
arch: um: kmsg_dump: Use console_is_usable
drivers: serial: kgdboc: Drop checks for CON_ENABLED and CON_BOOT
lib/vsprintf: Unify FORMAT_STATE_NUM handlers
printk: Avoid irq_work for printk_deferred() on suspend
printk: Avoid scheduling irq_work on suspend
printk: Allow printk_trigger_flush() to flush all types
tracing: Switch to use %ptSp
scsi: snic: Switch to use %ptSp
scsi: fnic: Switch to use %ptSp
s390/dasd: Switch to use %ptSp
ptp: ocp: Switch to use %ptSp
pps: Switch to use %ptSp
PCI: epf-test: Switch to use %ptSp
net: dsa: sja1105: Switch to use %ptSp
mmc: mmc_test: Switch to use %ptSp
media: av7110: Switch to use %ptSp
ipmi: Switch to use %ptSp
igb: Switch to use %ptSp
e1000e: Switch to use %ptSp
...
|
|
Use %ptSp instead of open coded variants to print content of
struct timespec64 in human readable format.
Acked-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patch.msgid.link/20251113150217.3030010-12-andriy.shevchenko@linux.intel.com
Signed-off-by: Petr Mladek <pmladek@suse.com>
|
|
Channels remain static unless the BMC firmware changes.
Therefore, rescanning is unnecessary while they are marked
ready and no BMC update has occurred.
Signed-off-by: Jinhui Guo <guojinhui.liam@bytedance.com>
Message-ID: <20250930074239.2353-4-guojinhui.liam@bytedance.com>
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
channel_handler() sets intf->channels_ready to true but never
clears it, so __scan_channels() skips any rescan. When the BMC
firmware changes a rescan is required. Allow it by clearing
the flag before starting a new scan.
Signed-off-by: Jinhui Guo <guojinhui.liam@bytedance.com>
Message-ID: <20250930074239.2353-3-guojinhui.liam@bytedance.com>
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
The race window between __scan_channels() and deliver_response() causes
the parameters of some channels to be set to 0.
1.[CPUA] __scan_channels() issues an IPMI request and waits with
wait_event() until all channels have been scanned.
wait_event() internally calls might_sleep(), which might
yield the CPU. (Moreover, an interrupt can preempt
wait_event() and force the task to yield the CPU.)
2.[CPUB] deliver_response() is invoked when the CPU receives the
IPMI response. After processing a IPMI response,
deliver_response() directly assigns intf->wchannels to
intf->channel_list and sets intf->channels_ready to true.
However, not all channels are actually ready for use.
3.[CPUA] Since intf->channels_ready is already true, wait_event()
never enters __wait_event(). __scan_channels() immediately
clears intf->null_user_handler and exits.
4.[CPUB] Once intf->null_user_handler is set to NULL, deliver_response()
ignores further IPMI responses, leaving the remaining
channels zero-initialized and unusable.
CPUA CPUB
------------------------------- -----------------------------
__scan_channels()
intf->null_user_handler
= channel_handler;
send_channel_info_cmd(intf,
0);
wait_event(intf->waitq,
intf->channels_ready);
do {
might_sleep();
deliver_response()
channel_handler()
intf->channel_list =
intf->wchannels + set;
intf->channels_ready = true;
send_channel_info_cmd(intf,
intf->curr_channel);
if (condition)
break;
__wait_event(wq_head,
condition);
} while(0)
intf->null_user_handler
= NULL;
deliver_response()
if (!msg->user)
if (intf->null_user_handler)
rv = -EINVAL;
return rv;
------------------------------- -----------------------------
Fix the race between __scan_channels() and deliver_response() by
deferring both the assignment intf->channel_list = intf->wchannels
and the flag intf->channels_ready = true until all channels have
been successfully scanned or until the IPMI request has failed.
Signed-off-by: Jinhui Guo <guojinhui.liam@bytedance.com>
Message-ID: <20250930074239.2353-2-guojinhui.liam@bytedance.com>
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
Prior to commit b52da4054ee0 ("ipmi: Rework user message limit handling"),
i_ipmi_request() used to increase the user reference counter if the receive
message is provided by the caller of IPMI API functions. This is no longer
the case. However, ipmi_free_recv_msg() is still called and decreases the
reference counter. This results in the reference counter reaching zero,
the user data pointer is released, and all kinds of interesting crashes are
seen.
Fix the problem by increasing user reference counter if the receive message
has been provided by the caller.
Fixes: b52da4054ee0 ("ipmi: Rework user message limit handling")
Reported-by: Eric Dumazet <edumazet@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Greg Thelen <gthelen@google.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Message-ID: <20251006201857.3433837-1-linux@roeck-us.net>
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
This patch adds Loongson-2K BMC IPMI support.
According to the existing design, we use software simulation to
implement the KCS interface registers: Stauts/Command/Data_Out/Data_In.
Also since both host side and BMC side read and write kcs status, fifo flag
is used to ensure data consistency.
The single KCS message block is as follows:
+-------------------------------------------------------------------------+
|FIFO flags| KCS register data | CMD data | KCS version | WR REQ | WR ACK |
+-------------------------------------------------------------------------+
Co-developed-by: Chong Qiao <qiaochong@loongson.cn>
Signed-off-by: Chong Qiao <qiaochong@loongson.cn>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
Acked-by: Corey Minyard <corey@minyard.net>
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Message-ID: <8f9ffb6f0405345af8f04193ce1510aacd075e72.1756987761.git.zhoubinbin@loongson.cn>
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
If the BMC is not functional, the driver goes into an error state and
starts a 1 second timer. When the timer times out, it will attempt a
simple message. If the BMC interacts correctly, the driver will start
accepting messages again. If not, it remains in error state.
If the driver goes into error state, all messages current and pending
will return with an error.
This should more gracefully handle when the BMC becomes non-operational,
as opposed to trying each messages individually and failing them.
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
It's only used to hold the corresponding receive message, so fix the
name to make that clear and the type so nothing else can be accidentally
assigned to it.
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
Getting ready for handling when a BMC is non-responsive or broken, allow
the sender operation to fail in an SMI. If it was a user-generated
message it will return the error.
The powernv code was already doing this internally, but the way it was
written could result in deep stack descent if there were a lot of
messages queued. Have its send return an error in this case.
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
It's about to be used from another place, and this looks better,
anyway.
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
Changes resulted in a silly looking piece of logic. Get rid of a goto
and use if statements properly.
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
Now that maintenance mode rejects all messages, there's nothing to
run time timer. Make sure the timer is running in maintenance mode.
Signed-off-by: Corey Minyard <corey@minyard.net>
Tested-by: Frederick Lawler <fred@cloudflare.com>
|
|
So you can see if it's in maintenance mode and see how long is left.
Signed-off-by: Corey Minyard <corey@minyard.net>
Tested-by: Frederick Lawler <fred@cloudflare.com>
|
|
If the driver goes into any maintenance mode, disable sysfs access until
it is done.
If the driver goes into reset maintenance mode, disable all messages
until it is done.
Signed-off-by: Corey Minyard <corey@minyard.net>
Tested-by: Frederick Lawler <fred@cloudflare.com>
|
|
This allows later changes to have different behaviour during a reset
verses a firmware update.
Signed-off-by: Corey Minyard <corey@minyard.net>
Tested-by: Frederick Lawler <fred@cloudflare.com>
|
|
The limit on the number of user messages had a number of issues,
improper counting in some cases and a use after free.
Restructure how this is all done to handle more in the receive message
allocation routine, so all refcouting and user message limit counts
are done in that routine. It's a lot cleaner and safer.
Reported-by: Gilles BULOZ <gilles.buloz@kontron.com>
Closes: https://lore.kernel.org/lkml/aLsw6G0GyqfpKs2S@mail.minyard.net/
Fixes: 8e76741c3d8b ("ipmi: Add a limit on the number of users that may use IPMI")
Cc: <stable@vger.kernel.org> # 4.19
Signed-off-by: Corey Minyard <corey@minyard.net>
Tested-by: Gilles BULOZ <gilles.buloz@kontron.com>
|
|
This reverts commit c608966f3f9c2dca596967501d00753282b395fc.
This patch has a subtle bug that can cause the IPMI driver to go into an
infinite loop if the BMC misbehaves in a certain way. Apparently
certain BMCs do misbehave this way because several reports have come in
recently about this.
Signed-off-by: Corey Minyard <corey@minyard.net>
Tested-by: Eric Hagberg <ehagberg@janestreet.com>
Cc: <stable@vger.kernel.org> # 6.2
|
|
Dan Carpenter got a Smatch warning:
drivers/char/ipmi/ipmi_msghandler.c:5265 ipmi_free_recv_msg()
warn: sleeping in atomic context
due to the recent rework of the IPMI driver's locking. I didn't realize
vfree could block. But there is an easy solution to this, now that
almost everything in the message handler runs in thread context.
I wanted to spend the time earlier to see if seq_lock could be converted
from a spinlock to a mutex, but I wanted the previous changes to go in
and soak before I did that. So I went ahead and did the analysis and
converting should work. And solve this problem.
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202503240244.LR7pOwyr-lkp@intel.com/
Fixes: 3be997d5a64a ("ipmi:msghandler: Remove srcu from the ipmi user structure")
Cc: <stable@vger.kernel.org> # 6.16
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
Pull ipmi updates from Corey Minyard:
"Some small fixes for the IPMI driver
Nothing huge, some rate limiting on logs, a strncpy fix where the
source and destination could be the same, and removal of some unused
cruft"
* tag 'for-linus-6.17-1' of https://github.com/cminyard/linux-ipmi:
ipmi: Use dev_warn_ratelimited() for incorrect message warnings
char: ipmi: remove redundant variable 'type' and check
ipmi: Fix strcpy source and destination the same
|
|
During BMC firmware upgrades on live systems, the ipmi_msghandler
generates excessive "BMC returned incorrect response" warnings
while the BMC is temporarily offline. This can flood system logs
in large deployments.
Replace dev_warn() with dev_warn_ratelimited() to throttle these
warnings and prevent log spam during BMC maintenance operations.
Signed-off-by: Breno Leitao <leitao@debian.org>
Message-ID: <20250710-ipmi_ratelimit-v1-1-6d417015ebe9@debian.org>
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
The variable 'type' is assigned the value SI_INVALID which is zero
and later checks of 'type' is non-zero (which is always false). The
variable is not referenced anywhere else, so it is redundant and
so is the check, so remove these.
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Message-ID: <20250708151805.1893858-1-colin.i.king@gmail.com>
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
The source and destination of some strcpy operations was the same.
Split out the part of the operations that needed to be done for those
particular calls so the unnecessary copy wasn't done.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506140756.EFXXvIP4-lkp@intel.com/
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
Move this API to the canonical timer_*() namespace.
[ tglx: Redone against pre rc1 ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/aB2X0jCKQO56WdMt@gmail.com
|
|
The "intf" list iterator is an invalid pointer if the correct
"intf->intf_num" is not found. Calling atomic_dec(&intf->nr_users) on
and invalid pointer will lead to memory corruption.
We don't really need to call atomic_dec() if we haven't called
atomic_add_return() so update the if (intf->in_shutdown) path as well.
Fixes: 8e76741c3d8b ("ipmi: Add a limit on the number of users that may use IPMI")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Message-ID: <aBjMZ8RYrOt6NOgi@stanley.mountain>
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
It's available, remove all the duplicate code.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
Don't have the other users that do things at panic time (the watchdog)
do all this themselves, provide a function to do it.
Also, with the new design where most stuff happens at thread context,
a few things needed to be fixed to avoid doing locking in a panic
context.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
It was possible for the SSIF thread to stop and quit before the
kthread_stop() call because ssif->stopping was set before the
stop. So only exit the SSIF thread is kthread_should_stop()
returns true.
There is no need to wake the thread, as the wait will be interrupted
by kthread_stop().
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
Check to see if they have been destroyed before trying to deliver a
message.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
It is possible in some situations that IPMI devices won't get started up
properly. This change makes it so all non-duplicate devices will get
started up.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
If we get a command from a LAN channel, return an error instead of just
throwing it away.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
This makes sure any outstanding messages are returned to the user before
the interface is cleaned up.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
It's no longer used.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
The lower level interface shouldn't attempt to unregister if it has a
callback in the pending queue.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
Messages already have a refcount for the user, so there's no need to
account for a new one.
As part of this, grab a refcount to the interface when processing
received messages. The messages can be freed there, cause the user
then the interface to be freed.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
Now that SRCU is gone from IPMI, it can no longer be sloppy about
locking. Use the users mutex now when sending a message, not the big
ipmi_interfaces mutex, because it can result in a recursive lock. The
users mutex will work because the interface destroy code claims it after
setting the interface in shutdown mode.
Also, due to the same changes, rework the refcounting on users and
interfaces. Remove the refcount to an interface when the user is
freed, not when it is destroyed. If the interface is destroyed
while the user still exists, the user will still point to the
interface to test that it is valid if the user tries to do anything
but delete the user.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
When run to completion is set, don't call things that will claim
mutexes or call user callbacks.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
Now that the msghandler does all callbacks in user threads, there is
no need to have a lock any more, a mutex will work fine.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
With reworks srcu is no longer necessary, this simplifies locking a lot.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
With the restructures done, srcu is no longer required, and it's fairly
onerous.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
Everything can be run in thread context now, don't use the bh one.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
It can only be called from thread context now.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
This simplifies the locking and lets us remove some weird event
handling code. deliver_response() and friends can now be called
from an atomic context.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
Get all operations that manipulate the interface list into thread
context.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
It handles both receive and transmit functions, make the name generic.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
It needs to be read only once because it's used in lock/unlock
scenarios.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
|
|
Andy reported:
Debian clang version 19.1.7 is not happy when compiled with
`make W=1` (note, CONFIG_WERROR=y is the default):
ipmi_si_platform.c:268:15: error: cast to smaller integer type 'enum si_type' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
268 | io.si_type = (enum si_type)device_get_match_data(&pdev->dev);
The IPMI SI type is an enum that was cast into a pointer that was
then cast into an enum again. That's not the greatest style, so
instead create an info structure to hold the data and use that.
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Closes: https://lore.kernel.org/lkml/20250415085156.446430-1-andriy.shevchenko@linux.intel.com/
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Corey Minyard <corey@minyard.net>
|
|
timer_delete[_sync]() replaces del_timer[_sync](). Convert the whole tree
over and remove the historical wrapper inlines.
Conversion was done with coccinelle plus manual fixups where necessary.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
|
|
Add the const qualifier to all the ctl_tables in the tree except for
watchdog_hardlockup_sysctl, memory_allocation_profiling_sysctls,
loadpin_sysctl_table and the ones calling register_net_sysctl (./net,
drivers/inifiniband dirs). These are special cases as they use a
registration function with a non-const qualified ctl_table argument or
modify the arrays before passing them on to the registration function.
Constifying ctl_table structs will prevent the modification of
proc_handler function pointers as the arrays would reside in .rodata.
This is made possible after commit 78eb4ea25cd5 ("sysctl: treewide:
constify the ctl_table argument of proc_handlers") constified all the
proc_handlers.
Created this by running an spatch followed by a sed command:
Spatch:
virtual patch
@
depends on !(file in "net")
disable optional_qualifier
@
identifier table_name != {
watchdog_hardlockup_sysctl,
iwcm_ctl_table,
ucma_ctl_table,
memory_allocation_profiling_sysctls,
loadpin_sysctl_table
};
@@
+ const
struct ctl_table table_name [] = { ... };
sed:
sed --in-place \
-e "s/struct ctl_table .table = &uts_kern/const struct ctl_table *table = \&uts_kern/" \
kernel/utsname_sysctl.c
Reviewed-by: Song Liu <song@kernel.org>
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org> # for kernel/trace/
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> # SCSI
Reviewed-by: Darrick J. Wong <djwong@kernel.org> # xfs
Acked-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Corey Minyard <cminyard@mvista.com>
Acked-by: Wei Liu <wei.liu@kernel.org>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Bill O'Donnell <bodonnel@redhat.com>
Acked-by: Baoquan He <bhe@redhat.com>
Acked-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Acked-by: Anna Schumaker <anna.schumaker@oracle.com>
Signed-off-by: Joel Granados <joel.granados@kernel.org>
|
