Pull IPMI updates from Corey Minyard:
"A couple of very minor fixes for style and rate limiting.
Nothing big, but probably needs to go in"
* tag 'for-linus-5.15-1' of git://github.com/cminyard/linux-ipmi:
char: ipmi: use DEVICE_ATTR helper macro
ipmi: rate limit ipmi smi_event failure message
The caller of this function (parisc_driver_remove() in
arch/parisc/kernel/drivers.c) ignores the return value, so better don't
return any value at all to not wake wrong expectations in driver authors.
The only function that could return a non-zero value before was
ipmi_parisc_remove() which returns the return value of
ipmi_si_remove_by_dev(). Make this function return void, too, as for all
other callers the value is ignored, too.
Also fold in a small checkpatch fix for:
WARNING: Unnecessary space before function pointer arguments
+ void (*remove) (struct parisc_device *dev);
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> (for drivers/input)
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Acked-by: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Helge Deller <deller@gmx.de>
Instead of open coding DEVICE_ATTR, use the helper macro
DEVICE_ATTR_RO to replace DEVICE_ATTR with 0444 octal
permissions.
This was detected as a part of checkpatch evaluation
investigating all reports of DEVICE_ATTR_RO warning
type.
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
Message-Id: <20210730062951.84876-1-dwaipayanray1@gmail.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Sometimes we can't get a valid si_sm_data, and we print an error
message accordingly. But the ipmi module seem to like retrying a lot,
in which case we flood the kernel log with a lot of messages, eg:
[46318019.164726] ipmi_si IPI0001:00: Could not set the global enables: 0xc1.
[46318020.109700] ipmi_si IPI0001:00: Could not set the global enables: 0xc1.
[46318021.158677] ipmi_si IPI0001:00: Could not set the global enables: 0xc1.
[46318022.212598] ipmi_si IPI0001:00: Could not set the global enables: 0xc1.
[46318023.258564] ipmi_si IPI0001:00: Could not set the global enables: 0xc1.
[46318024.210455] ipmi_si IPI0001:00: Could not set the global enables: 0xc1.
[46318025.260473] ipmi_si IPI0001:00: Could not set the global enables: 0xc1.
[46318026.308445] ipmi_si IPI0001:00: Could not set the global enables: 0xc1.
[46318027.356389] ipmi_si IPI0001:00: Could not set the global enables: 0xc1.
[46318028.298288] ipmi_si IPI0001:00: Could not set the global enables: 0xc1.
[46318029.363302] ipmi_si IPI0001:00: Could not set the global enables: 0xc1.
Signed-off-by: Wen Yang <wenyang@linux.alibaba.com>
Cc: Baoyou Xie <baoyou.xie@alibaba-inc.com>
Cc: Corey Minyard <minyard@acm.org>
Cc: openipmi-developer@lists.sourceforge.net
Cc: linux-kernel@vger.kernel.org
Message-Id: <20210729093228.77098-1-wenyang@linux.alibaba.com>
[Added a missing comma]
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Merge more updates from Andrew Morton:
"190 patches.
Subsystems affected by this patch series: mm (hugetlb, userfaultfd,
vmscan, kconfig, proc, z3fold, zbud, ras, mempolicy, memblock,
migration, thp, nommu, kconfig, madvise, memory-hotplug, zswap,
zsmalloc, zram, cleanups, kfence, and hmm), procfs, sysctl, misc,
core-kernel, lib, lz4, checkpatch, init, kprobes, nilfs2, hfs,
signals, exec, kcov, selftests, compress/decompress, and ipc"
* emailed patches from Andrew Morton <akpm@linux-foundation.org>: (190 commits)
ipc/util.c: use binary search for max_idx
ipc/sem.c: use READ_ONCE()/WRITE_ONCE() for use_global_lock
ipc: use kmalloc for msg_queue and shmid_kernel
ipc sem: use kvmalloc for sem_undo allocation
lib/decompressors: remove set but not used variabled 'level'
selftests/vm/pkeys: exercise x86 XSAVE init state
selftests/vm/pkeys: refill shadow register after implicit kernel write
selftests/vm/pkeys: handle negative sys_pkey_alloc() return code
selftests/vm/pkeys: fix alloc_random_pkey() to make it really, really random
kcov: add __no_sanitize_coverage to fix noinstr for all architectures
exec: remove checks in __register_bimfmt()
x86: signal: don't do sas_ss_reset() until we are certain that sigframe won't be abandoned
hfsplus: report create_date to kstat.btime
hfsplus: remove unnecessary oom message
nilfs2: remove redundant continue statement in a while-loop
kprobes: remove duplicated strong free_insn_page in x86 and s390
init: print out unknown kernel parameters
checkpatch: do not complain about positive return values starting with EPOLL
checkpatch: improve the indented label test
checkpatch: scripts/spdxcheck.py now requires python3
...
The comparisons of the unsigned int hw_type to less than zero always
false because it is unsigned. Fix this by using an int for the
assignment and less than zero check.
Addresses-Coverity: ("Unsigned compared against 0")
Fixes: 9d2df9a0ad80 ("ipmi: kcs_bmc_aspeed: Implement KCS SerIRQ configuration")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Message-Id: <20210616162913.15259-1-colin.king@canonical.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Some Aspeed KCS devices can derive the status register address from the
address of the data register. As such, the address of the status
register can be implicit in the configuration if desired. On the other
hand, sometimes address schemes might be requested that are incompatible
with the default addressing scheme. Allow these requests where possible
if the devicetree specifies the status register address.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Chia-Wei Wang <chiawei_wang@aspeedtech.com>
Message-Id: <20210608104757.582199-17-andrew@aj.id.au>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
kcs_bmc_serio acts as a bridge between the KCS drivers in the IPMI
subsystem and the existing userspace interfaces available through the
serio subsystem. This is useful when userspace would like to make use of
the BMC KCS devices for purposes that aren't IPMI.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Message-Id: <20210608104757.582199-12-andrew@aj.id.au>
Reviewed-by: Zev Weiss <zweiss@equinix.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Add a mechanism for controlling whether the client associated with a
KCS device will receive Input Buffer Full (IBF) and Output Buffer Empty
(OBE) events. This enables an abstract implementation of poll() for KCS
devices.
A wart in the implementation is that the ASPEED KCS devices don't
support an OBE interrupt for the BMC. Instead we pretend it has one by
polling the status register waiting for the Output Buffer Full (OBF) bit
to clear, and generating an event when OBE is observed.
Cc: CS20 KWLiu <KWLIU@nuvoton.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Zev Weiss <zweiss@equinix.com>
Message-Id: <20210608104757.582199-10-andrew@aj.id.au>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Now that we have untangled the data-structures, split the userspace
interface out into its own module. Userspace interfaces and drivers are
registered to the KCS BMC core to support arbitrary binding of either.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Message-Id: <20210608104757.582199-9-andrew@aj.id.au>
Reviewed-by: Zev Weiss <zweiss@equinix.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Move all client-private data out of `struct kcs_bmc` into the KCS client
implementation.
With this change the KCS BMC core code now only concerns itself with
abstract `struct kcs_bmc` and `struct kcs_bmc_client` types, achieving
expected separation of concerns. Further, the change clears the path for
implementation of alternative userspace interfaces.
The chardev data-structures are rearranged in the same manner applied to
the KCS device driver data-structures in an earlier patch - `struct
kcs_bmc_client` is embedded in the client's private data and we exploit
container_of() to translate as required.
Finally, now that it is free of client data, `struct kcs_bmc` is renamed
to `struct kcs_bmc_device` to contrast `struct kcs_bmc_client`.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Zev Weiss <zweiss@equinix.com>
Message-Id: <20210608104757.582199-8-andrew@aj.id.au>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Strengthen the distinction between code that abstracts the
implementation of the KCS behaviours (device drivers) and code that
exploits KCS behaviours (clients). Neither needs to know about the APIs
required by the other, so provide separate headers.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Message-Id: <20210608104757.582199-7-andrew@aj.id.au>
Reviewed-by: Zev Weiss <zweiss@equinix.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Make the KCS device drivers responsible for allocating their own memory.
Until now the private data for the device driver was allocated internal
to the private data for the chardev interface. This coupling required
the slightly awkward API of passing through the struct size for the
driver private data to the chardev constructor, and then retrieving a
pointer to the driver private data from the allocated chardev memory.
In addition to being awkward, the arrangement prevents the
implementation of alternative userspace interfaces as the device driver
private data is not independent.
Peel a layer off the onion and turn the data-structures inside out by
exploiting container_of() and embedding `struct kcs_device` in the
driver private data.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Zev Weiss <zweiss@equinix.com>
Message-Id: <20210608104757.582199-6-andrew@aj.id.au>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Take steps towards defining a coherent API to separate the KCS device
drivers from the userspace interface. Decreasing the coupling will
improve the separation of concerns and enable the introduction of
alternative userspace interfaces.
For now, simply split the chardev logic out to a separate file. The code
continues to build into the same module.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Zev Weiss <zweiss@equinix.com>
Message-Id: <20210608104757.582199-5-andrew@aj.id.au>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Enable more efficient implementation of read-modify-write sequences.
Both device drivers for the KCS BMC stack use regmaps. The new callback
allows us to exploit regmap_update_bits().
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Zev Weiss <zweiss@equinix.com>
Message-Id: <20210608104757.582199-3-andrew@aj.id.au>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
When an IPMI watchdog timer is being stopped in ipmi_close() or
ipmi_ioctl(WDIOS_DISABLECARD), the current watchdog action is updated to
WDOG_TIMEOUT_NONE and _ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB) is called
to install this action. The latter function ends up invoking
__ipmi_set_timeout() which makes the actual 'Set Watchdog Timer' IPMI
request.
For IPMI 1.0, this operation results in fully stopping the watchdog timer.
For IPMI >= 1.5, function __ipmi_set_timeout() always specifies the "don't
stop" flag in the prepared 'Set Watchdog Timer' IPMI request. This causes
that the watchdog timer has its action correctly updated to 'none' but the
timer continues to run. A problem is that IPMI firmware can then still log
an expiration event when the configured timeout is reached, which is
unexpected because the watchdog timer was requested to be stopped.
The patch fixes this problem by not setting the "don't stop" flag in
__ipmi_set_timeout() when the current action is WDOG_TIMEOUT_NONE which
results in stopping the watchdog timer. This makes the behaviour for
IPMI >= 1.5 consistent with IPMI 1.0. It also matches the logic in
__ipmi_heartbeat() which does not allow to reset the watchdog if the
current action is WDOG_TIMEOUT_NONE as that would start the timer.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Message-Id: <10a41bdc-9c99-089c-8d89-fa98ce5ea080@suse.com>
Cc: stable@vger.kernel.org
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Pull IPMI updates from Corey Minyard:
"A bunch of little cleanups
Nothing major, no functional changes"
* tag 'for-linus-5.13-1' of git://github.com/cminyard/linux-ipmi:
ipmi_si: Join string literals back
ipmi_si: Drop redundant check before calling put_device()
ipmi_si: Use strstrip() to remove surrounding spaces
ipmi_si: Get rid of ->addr_source_cleanup()
ipmi_si: Reuse si_to_str[] array in ipmi_hardcode_init_one()
ipmi_si: Introduce ipmi_panic_event_str[] array
ipmi_si: Use proper ACPI macros to check error code for failures
ipmi_si: Utilize temporary variable to hold device pointer
ipmi_si: Remove bogus err_free label
ipmi_si: Switch to use platform_get_mem_or_io()
ipmi: Handle device properties with software node API
ipmi:ssif: make ssif_i2c_send() void
ipmi: Refine retry conditions for getting device id
The block-write function of the core was not used because there was no
client-struct to use. However, in this case it seems apropriate to use a
temporary client struct. Because we are answering a request we recieved
when being a client ourselves. So, convert the code to use a temporary
client and use the block-write function of the I2C core.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Asmaa Mnebhi <asmaa@nvidia.com>
Acked-by: Corey Minyard <cminyard@mvista.com>
Message-Id: <20210128085544.7609-1-wsa+renesas@sang-engineering.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
while running ipmi, ipmi_smi_watcher_register() caused
a suspicious RCU usage warning.
-----
=============================
WARNING: suspicious RCU usage
5.10.0-rc3+ #1 Not tainted
-----------------------------
drivers/char/ipmi/ipmi_msghandler.c:750 RCU-list traversed in non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by syz-executor.0/4254:
stack backtrace:
CPU: 0 PID: 4254 Comm: syz-executor.0 Not tainted 5.10.0-rc3+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1 04/ 01/2014
Call Trace:
dump_stack+0x19d/0x200
ipmi_smi_watcher_register+0x2d3/0x340 [ipmi_msghandler]
acpi_ipmi_init+0xb1/0x1000 [acpi_ipmi]
do_one_initcall+0x149/0x7e0
do_init_module+0x1ef/0x700
load_module+0x3467/0x4140
__do_sys_finit_module+0x10d/0x1a0
do_syscall_64+0x34/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x468ded
-----
It is safe because smi_watchers_mutex is locked and srcu_read_lock
has been used, so simply pass lockdep_is_held() to the
list_for_each_entry_rcu() to suppress this warning.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com>
Message-Id: <20201119070839.381-1-miaoqinglang@huawei.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Pull IPMI updates from Corey Minyard:
"Some minor bug fixes, return values, cleanups of prints, conversion of
tasklets to the new API.
The biggest change is retrying the initial information fetch from the
management controller. If that fails, the iterface is not operational,
and one group was having trouble with the management controller not
being ready when the OS started up. So a retry was added"
* tag 'for-linus-5.10-1' of git://github.com/cminyard/linux-ipmi:
ipmi_si: Fix wrong return value in try_smi_init()
ipmi: msghandler: Fix a signedness bug
ipmi: add retry in try_get_dev_id()
ipmi: Clean up some printks
ipmi:msghandler: retry to get device id on an error
ipmi:sm: Print current state when the state is invalid
ipmi: Reset response handler when failing to send the command
ipmi: add a newline when printing parameter 'panic_op' by sysfs
char: ipmi: convert tasklets to use new tasklet_setup() API
The type for the completion codes should be unsigned char instead of
char. If it is declared as a normal char then the conditions in
__get_device_id() are impossible because the IPMI_DEVICE_IN_FW_UPDATE_ERR
error codes are higher than 127.
drivers/char/ipmi/ipmi_msghandler.c:2449 __get_device_id()
warn: impossible condition '(bmc->cc == 209) => ((-128)-127 == 209)'
Fixes: f8910ffa81 ("ipmi:msghandler: retry to get device id on an error")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Message-Id: <20200918142756.GB909725@mwanda>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Use a retry machanism to give the BMC more opportunities to correctly
respond when we receive specific completion codes.
This is similar to what is done in __get_device_id().
Signed-off-by: Xianting Tian <tian.xianting@h3c.com>
Message-Id: <20200916062129.26129-1-tian.xianting@h3c.com>
[Moved GET_DEVICE_ID_MAX_RETRY to include/linux/ipmi.h, reworded some
text.]
Signed-off-by: Corey Minyard <cminyard@mvista.com>
We fail to get the BMCS's device id with low probability when loading
the ipmi driver and it causes BMC device registration failed. When this
issue occurs we got below kernel prints:
[Wed Sep 9 19:52:03 2020] ipmi_si IPI0001:00: IPMI message handler:
device id demangle failed: -22
[Wed Sep 9 19:52:03 2020] IPMI BT: using default values
[Wed Sep 9 19:52:03 2020] IPMI BT: req2rsp=5 secs retries=2
[Wed Sep 9 19:52:03 2020] ipmi_si IPI0001:00: Unable to get the
device id: -5
[Wed Sep 9 19:52:04 2020] ipmi_si IPI0001:00: Unable to register
device: error -5
When this issue happens, we want to manually unload the driver and try to
load it again, but it can't be unloaded by 'rmmod' as it is already 'in
use'.
We add a print in handle_one_recv_msg(), when this issue happens,
the msg we received is "Recv: 1c 01 d5", which means the data_len is 1,
data[0] is 0xd5 (completion code), which means "bmc cannot execute
command. Command, or request parameter(s), not supported in present
state". Debug code:
static int handle_one_recv_msg(struct ipmi_smi *intf,
struct ipmi_smi_msg *msg) {
printk("Recv: %*ph\n", msg->rsp_size, msg->rsp);
... ...
}
Then in ipmi_demangle_device_id(), it returned '-EINVAL' as 'data_len < 7'
and 'data[0] != 0'.
We created this patch to retry the get device id when this error
happens. We reproduced this issue again and the retry succeed on the
first retry, we finally got the correct msg and then all is ok:
Recv: 1c 01 00 01 81 05 84 02 af db 07 00 01 00 b9 00 10 00
So use a retry machanism in this patch to give bmc more opportunity to
correctly response kernel when we received specific completion codes.
Signed-off-by: Xianting Tian <tian.xianting@h3c.com>
Message-Id: <20200915071817.4484-1-tian.xianting@h3c.com>
[Cleaned up the verbage a bit in the header and prints.]
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Print current state before returning IPMI_NOT_IN_MY_STATE_ERR so we can
know where this issue is coming from and possibly fix the state machine.
Signed-off-by: Xianting Tian <tian.xianting@h3c.com>
Message-Id: <20200915074441.4090-1-tian.xianting@h3c.com>
[Converted printk() to pr_xxx().]
Signed-off-by: Corey Minyard <cminyard@mvista.com>
