Re: [PATCH v3 02/10] platform/x86/intel/pmt: Add register access callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Hardware access errors from `read_reg` and `write_reg` callbacks are ignored and not propagated to sysfs callers.
- [Medium] Unratelimited pr_err() in sysfs read/write paths allows kernel log flooding.
--

--- Patch [2]: [PATCH v3 02/10] platform/x86/intel/pmt: Add register access callbacks ---
Note: The format of this review is altered and does not include direct quotes
of the patch code due to recitation restrictions.

commit 6dda4f7e76b3694a6fb7bf21ac1d00ef75cff3ff
Author: Michael J. Ruhl <michael.j.ruhl@intel.com>
Subject: platform/x86/intel/pmt: Add register access callbacks

This commit augments the PMT callback infrastructure to allow drivers to
customize read and write access to control paths for PMT usage. It is intended
for hardware lacking direct MMIO access.

In drivers/platform/x86/intel/pmt/crashlog.c, the modified pmt_crashlog_rmw()
and pmt_crashlog_rc() functions check for errors from the new read_reg and
write_reg callbacks, but they do not propagate these error codes back up the
call stack.

[Severity: Medium]
Is it expected that hardware access failures are silently swallowed here?

When read_reg or write_reg fails in pmt_crashlog_rmw(), the function returns
void, and when read_reg fails in pmt_crashlog_rc(), it returns false. Because
these errors are not propagated, sysfs store operations will falsely report
success to userspace by returning the written byte count. Similarly, sysfs
reads will present hardware errors as a valid false state. Will this break
monitoring and management tools that rely on accurate return codes?

Additionally, the error handling paths in these same functions introduce
logging statements.

[Severity: Medium]
Could the unratelimited pr_err() calls in these functions cause kernel log
flooding?

If the hardware is in a state where read_reg or write_reg consistently fails,
an unprivileged user reading a world-readable sysfs attribute in a tight loop
will repeatedly trigger pmt_crashlog_rc(). Would it be safer to use ratelimited
logging here to prevent potential syslog I/O overhead and the dropping of other
important logs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824162317.2450380-12-michael.j.ruhl@intel.com?part=2