[PATCH 0/2] scsi: mpt3sas: add hwmon support

[PATCH 0/2] scsi: mpt3sas: add hwmon support

[Louis Sautier]

Expose the IOC and board temperature sensors of LSI / Broadcom / Avago
SAS HBAs that bind to mpt3sas through the hwmon interface. The data
lives in MPI IO Unit Page 7.

The same fields are exposed by Broadcom's userspace tooling through
the /dev/mpt[23]ctl ioctl path (typically root-only): IOCTemperature
and BoardTemperature in lsiutil; ROC and Controller in storcli.
With this driver, sensors(1) shows them unprivileged:

  $ sensors mpt3sas-pci-0200
  mpt3sas-pci-0200
  Adapter: PCI adapter
  IOC:          +42.0°C

Testing
-------

Validated across three Broadcom SAS chip generations. None of the
cards had a board sensor present, so the testing only covers the
IOC channel:

  * LSI 9500-8i / SAS3816, SAS-3:
    - hwmon device registers as "mpt3sas" with only temp1 (IOC) exposed
    - IOC reading cross-references with storcli and lsiutil
    - rmmod / modprobe cycle goes through the explicit
      unregister/register paths cleanly

  * LSI 9305-24i / SAS3224, SAS-3: same behaviour.

  * LSI 9211-4i / SAS2004, SAS-2: firmware reports both
    *TemperatureUnits = NOT_PRESENT, no hwmon device registered
    (graceful-skip path).

Sysfs reads share ioc->config_cmds.mutex with the driver's internal
Config consumers; under 8 concurrent readers the queueing wait
imposed on legitimate operations stayed below ~40 µs (SAS3816 /
SAS3224).

Not verified (no available hardware):
  * Path with both IOC and board sensors present.
  * Fahrenheit-units conversion.
  * Sub-zero readings (signed-cast path).

Louis Sautier (2):
  scsi: mpt3sas: add IO Unit Page 7 config accessor
  scsi: mpt3sas: add hwmon support

 Documentation/hwmon/index.rst         |   1 +
 Documentation/hwmon/mpt3sas.rst       |  57 ++++++++
 MAINTAINERS                           |   1 +
 drivers/scsi/mpt3sas/Kconfig          |   9 ++
 drivers/scsi/mpt3sas/Makefile         |   2 +
 drivers/scsi/mpt3sas/mpt3sas_base.h   |  19 +++
 drivers/scsi/mpt3sas/mpt3sas_config.c |  36 +++++
 drivers/scsi/mpt3sas/mpt3sas_hwmon.c  | 200 ++++++++++++++++++++++++++
 drivers/scsi/mpt3sas/mpt3sas_scsih.c  |   6 +
 9 files changed, 331 insertions(+)
 create mode 100644 Documentation/hwmon/mpt3sas.rst
 create mode 100644 drivers/scsi/mpt3sas/mpt3sas_hwmon.c


base-commit: 5d6919055dec134de3c40167a490f33c74c12581
-- 
2.54.0

[PATCH 1/2] scsi: mpt3sas: add IO Unit Page 7 config accessor

[Louis Sautier]

Add mpt3sas_config_get_iounit_pg7(), mirroring the existing iounit
page accessors. Used by the hwmon driver added in the following patch
to read the IOC and board temperatures.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Louis Sautier <sautier.louis@gmail.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.h   |  2 ++
 drivers/scsi/mpt3sas/mpt3sas_config.c | 36 +++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index d4597d058705..c655742d0dde 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1904,6 +1904,8 @@ int mpt3sas_config_get_iounit_pg3(struct MPT3SAS_ADAPTER *ioc,
 	Mpi2ConfigReply_t *mpi_reply, Mpi2IOUnitPage3_t *config_page, u16 sz);
 int mpt3sas_config_set_iounit_pg1(struct MPT3SAS_ADAPTER *ioc, Mpi2ConfigReply_t
 	*mpi_reply, Mpi2IOUnitPage1_t *config_page);
+int mpt3sas_config_get_iounit_pg7(struct MPT3SAS_ADAPTER *ioc,
+	Mpi2ConfigReply_t *mpi_reply, Mpi2IOUnitPage7_t *config_page);
 int mpt3sas_config_get_iounit_pg8(struct MPT3SAS_ADAPTER *ioc, Mpi2ConfigReply_t
 	*mpi_reply, Mpi2IOUnitPage8_t *config_page);
 int mpt3sas_config_get_sas_iounit_pg1(struct MPT3SAS_ADAPTER *ioc,
diff --git a/drivers/scsi/mpt3sas/mpt3sas_config.c b/drivers/scsi/mpt3sas/mpt3sas_config.c
index 45ac853e1289..ef07825046bc 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_config.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_config.c
@@ -991,6 +991,42 @@ mpt3sas_config_get_iounit_pg3(struct MPT3SAS_ADAPTER *ioc,
 	return r;
 }
 
+/**
+ * mpt3sas_config_get_iounit_pg7 - obtain iounit page 7
+ * @ioc: per adapter object
+ * @mpi_reply: reply mf payload returned from firmware
+ * @config_page: contents of the config page
+ * Context: sleep.
+ *
+ * Return: 0 for success, non-zero for failure.
+ */
+int
+mpt3sas_config_get_iounit_pg7(struct MPT3SAS_ADAPTER *ioc,
+	Mpi2ConfigReply_t *mpi_reply, Mpi2IOUnitPage7_t *config_page)
+{
+	Mpi2ConfigRequest_t mpi_request;
+	int r;
+
+	memset(&mpi_request, 0, sizeof(Mpi2ConfigRequest_t));
+	mpi_request.Function = MPI2_FUNCTION_CONFIG;
+	mpi_request.Action = MPI2_CONFIG_ACTION_PAGE_HEADER;
+	mpi_request.Header.PageType = MPI2_CONFIG_PAGETYPE_IO_UNIT;
+	mpi_request.Header.PageNumber = 7;
+	mpi_request.Header.PageVersion = MPI2_IOUNITPAGE7_PAGEVERSION;
+	ioc->build_zero_len_sge_mpi(ioc, &mpi_request.PageBufferSGE);
+	r = _config_request(ioc, &mpi_request, mpi_reply,
+	    MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT, NULL, 0);
+	if (r)
+		goto out;
+
+	mpi_request.Action = MPI2_CONFIG_ACTION_PAGE_READ_CURRENT;
+	r = _config_request(ioc, &mpi_request, mpi_reply,
+	    MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT, config_page,
+	    sizeof(*config_page));
+ out:
+	return r;
+}
+
 /**
  * mpt3sas_config_get_iounit_pg8 - obtain iounit page 8
  * @ioc: per adapter object
-- 
2.54.0

[PATCH 2/2] scsi: mpt3sas: add hwmon support

[Louis Sautier]

Expose the IOC and board temperature sensors of LSI / Broadcom SAS
HBAs through hwmon. Readings come from MPI IO Unit Page 7 via the
accessor added in the preceding patch.

The same fields are exposed by Broadcom's userspace tooling
through the /dev/mpt[23]ctl ioctl path (typically root-only):
IOCTemperature and BoardTemperature in lsiutil; ROC and Controller
in storcli. With this driver, sensors(1) shows them unprivileged:

  $ sensors mpt3sas-pci-0200
  mpt3sas-pci-0200
  Adapter: PCI adapter
  IOC:          +42.0°C

Each channel is gated independently by its *TemperatureUnits field
through is_visible(); cards that populate only one sensor expose
only one input file, and cards that populate neither do not register
an hwmon device.

Built into mpt3sas.ko under a new CONFIG_SCSI_MPT3SAS_HWMON Kconfig
option.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Louis Sautier <sautier.louis@gmail.com>
---
 Documentation/hwmon/index.rst        |   1 +
 Documentation/hwmon/mpt3sas.rst      |  57 ++++++++
 MAINTAINERS                          |   1 +
 drivers/scsi/mpt3sas/Kconfig         |   9 ++
 drivers/scsi/mpt3sas/Makefile        |   2 +
 drivers/scsi/mpt3sas/mpt3sas_base.h  |  17 +++
 drivers/scsi/mpt3sas/mpt3sas_hwmon.c | 200 +++++++++++++++++++++++++++
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |   6 +
 8 files changed, 293 insertions(+)
 create mode 100644 Documentation/hwmon/mpt3sas.rst
 create mode 100644 drivers/scsi/mpt3sas/mpt3sas_hwmon.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 8b655e5d6b68..106f87fa8b18 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -193,6 +193,7 @@ Hardware Monitoring Kernel Drivers
    mp9941
    mp9945
    mpq8785
+   mpt3sas
    nct6683
    nct6775
    nct7363
diff --git a/Documentation/hwmon/mpt3sas.rst b/Documentation/hwmon/mpt3sas.rst
new file mode 100644
index 000000000000..3a260a389d6d
--- /dev/null
+++ b/Documentation/hwmon/mpt3sas.rst
@@ -0,0 +1,57 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver mpt3sas
+=====================
+
+Supported chips:
+
+  * LSI / Broadcom / Avago SAS HBAs handled by the mpt3sas driver,
+    such as the 9300, 9400, and 9500 series.
+
+    Prefix: ``mpt3sas``
+
+
+Description
+-----------
+
+The mpt3sas driver exposes the IOC and board temperature sensors of
+LSI / Broadcom SAS HBAs through the hwmon interface.
+Either or both sensors may be absent depending on the card; the
+corresponding sysfs files only appear when the firmware reports the
+sensor as present, and cards that report neither sensor do not
+register an hwmon device at all.
+
+
+Sysfs entries
+-------------
+
+============  ======================
+Name          Description
+============  ======================
+temp1_input   IOC temperature (mC)
+temp1_label   "IOC"
+temp2_input   Board temperature (mC)
+temp2_label   "Board"
+============  ======================
+
+
+Cross-reference with vendor tooling
+-----------------------------------
+
+The hwmon channels correspond to fields reported by Broadcom's
+proprietary tools as follows:
+
+=================  ==========================  ===============================
+hwmon label        lsiutil                     storcli
+=================  ==========================  ===============================
+``IOC`` (temp1)    ``IOCTemperature``          ``ROC temperature``
+``Board`` (temp2)  ``BoardTemperature``        ``Controller temperature``
+=================  ==========================  ===============================
+
+With lsiutil::
+
+    lsiutil -pN -a 25,2,0,0
+
+With storcli::
+
+    storcli /cN show temperature
diff --git a/MAINTAINERS b/MAINTAINERS
index b2040011a386..e084f710f436 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15154,6 +15154,7 @@ L:	MPT-FusionLinux.pdl@broadcom.com
 L:	linux-scsi@vger.kernel.org
 S:	Supported
 W:	http://www.avagotech.com/support/
+F:	Documentation/hwmon/mpt3sas.rst
 F:	drivers/message/fusion/
 F:	drivers/scsi/mpt3sas/
 
diff --git a/drivers/scsi/mpt3sas/Kconfig b/drivers/scsi/mpt3sas/Kconfig
index c299f7e078fb..638acd2c6623 100644
--- a/drivers/scsi/mpt3sas/Kconfig
+++ b/drivers/scsi/mpt3sas/Kconfig
@@ -73,6 +73,15 @@ config SCSI_MPT3SAS_MAX_SGE
 	can be 256. However, it may decreased down to 16.  Decreasing this
 	parameter will reduce memory requirements on a per controller instance.
 
+config SCSI_MPT3SAS_HWMON
+	bool "LSI MPT Fusion SAS hwmon support"
+	depends on SCSI_MPT3SAS && HWMON
+	depends on !(SCSI_MPT3SAS=y && HWMON=m)
+	help
+	Say Y here to expose the IOC and board temperature sensors of
+	LSI / Broadcom SAS HBAs (such as the 9300, 9400, and 9500 series)
+	through hwmon. See Documentation/hwmon/mpt3sas.rst for details.
+
 config SCSI_MPT2SAS
 	tristate "Legacy MPT2SAS config option"
 	default n
diff --git a/drivers/scsi/mpt3sas/Makefile b/drivers/scsi/mpt3sas/Makefile
index e76d994dbed3..9a2f3ce4158a 100644
--- a/drivers/scsi/mpt3sas/Makefile
+++ b/drivers/scsi/mpt3sas/Makefile
@@ -9,3 +9,5 @@ mpt3sas-y +=  mpt3sas_base.o     \
 		mpt3sas_trigger_diag.o \
 		mpt3sas_warpdrive.o \
 		mpt3sas_debugfs.o \
+
+mpt3sas-$(CONFIG_SCSI_MPT3SAS_HWMON) += mpt3sas_hwmon.o
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index c655742d0dde..63252f30343b 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1629,6 +1629,7 @@ struct MPT3SAS_ADAPTER {
 	u8		is_aero_ioc;
 	struct dentry	*debugfs_root;
 	struct dentry	*ioc_dump;
+	struct mpt3sas_hwmon *hwmon;
 	PUT_SMID_IO_FP_HIP put_smid_scsi_io;
 	PUT_SMID_IO_FP_HIP put_smid_fast_path;
 	PUT_SMID_IO_FP_HIP put_smid_hi_priority;
@@ -2049,6 +2050,22 @@ void mpt3sas_destroy_debugfs(struct MPT3SAS_ADAPTER *ioc);
 void mpt3sas_init_debugfs(void);
 void mpt3sas_exit_debugfs(void);
 
+#if IS_ENABLED(CONFIG_SCSI_MPT3SAS_HWMON)
+int mpt3sas_hwmon_register(struct MPT3SAS_ADAPTER *ioc);
+void mpt3sas_hwmon_unregister(struct MPT3SAS_ADAPTER *ioc);
+#else
+static inline int
+mpt3sas_hwmon_register(struct MPT3SAS_ADAPTER *ioc)
+{
+	return 0;
+}
+
+static inline void
+mpt3sas_hwmon_unregister(struct MPT3SAS_ADAPTER *ioc)
+{
+}
+#endif
+
 /**
  * _scsih_is_pcie_scsi_device - determines if device is an pcie scsi device
  * @device_info: bitfield providing information about the device.
diff --git a/drivers/scsi/mpt3sas/mpt3sas_hwmon.c b/drivers/scsi/mpt3sas/mpt3sas_hwmon.c
new file mode 100644
index 000000000000..26227a992f35
--- /dev/null
+++ b/drivers/scsi/mpt3sas/mpt3sas_hwmon.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hardware monitoring (hwmon) support for the LSI / Broadcom mpt3sas
+ * SAS HBA driver. Exposes the IOC and board temperature sensors by
+ * reading MPI IO Unit Page 7.
+ */
+
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+#include "mpt3sas_base.h"
+
+struct mpt3sas_hwmon {
+	struct MPT3SAS_ADAPTER *ioc;
+	struct device *hwmon_dev;
+	bool ioc_present;
+	bool board_present;
+};
+
+/*
+ * Convert a (raw, units) reading to millidegrees Celsius.
+ * Returns -ENODATA when the sensor reports "not present" or
+ * unknown units. Temperature values are interpreted as signed
+ * two's-complement integers.
+ *
+ * The MPI2_IOUNITPAGE7_IOC_TEMP_* and MPI2_IOUNITPAGE7_BOARD_TEMP_*
+ * defines in mpi2_cnfg.h share the same values; the IOC ones are
+ * used for both channels.
+ */
+static int
+_hwmon_to_mdegc(s16 raw, u8 units, long *out)
+{
+	switch (units) {
+	case MPI2_IOUNITPAGE7_IOC_TEMP_CELSIUS:
+		*out = (long)raw * 1000;
+		return 0;
+	case MPI2_IOUNITPAGE7_IOC_TEMP_FAHRENHEIT:
+		/* (F - 32) * 5 / 9, expressed in milli-units */
+		*out = ((long)raw - 32) * 5000 / 9;
+		return 0;
+	default:
+		return -ENODATA;
+	}
+}
+
+static umode_t
+_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
+		  u32 attr, int channel)
+{
+	const struct mpt3sas_hwmon *h = drvdata;
+
+	if (type != hwmon_temp)
+		return 0;
+	if (attr != hwmon_temp_input && attr != hwmon_temp_label)
+		return 0;
+	if (channel == 0 && h->ioc_present)
+		return 0444;
+	if (channel == 1 && h->board_present)
+		return 0444;
+	return 0;
+}
+
+static int
+_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+	    u32 attr, int channel, long *val)
+{
+	struct mpt3sas_hwmon *h = dev_get_drvdata(dev);
+	Mpi2ConfigReply_t mpi_reply;
+	Mpi2IOUnitPage7_t page;
+	int r;
+
+	if (type != hwmon_temp || attr != hwmon_temp_input)
+		return -EOPNOTSUPP;
+
+	r = mpt3sas_config_get_iounit_pg7(h->ioc, &mpi_reply, &page);
+	if (r)
+		return r;
+
+	if (channel == 0)
+		return _hwmon_to_mdegc((s16)le16_to_cpu(page.IOCTemperature),
+				       page.IOCTemperatureUnits, val);
+	if (channel == 1)
+		return _hwmon_to_mdegc((s16)le16_to_cpu(page.BoardTemperature),
+				       page.BoardTemperatureUnits, val);
+	return -EOPNOTSUPP;
+}
+
+static const char * const mpt3sas_hwmon_temp_labels[] = {
+	"IOC",
+	"Board",
+};
+
+static int
+_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
+		   u32 attr, int channel, const char **str)
+{
+	if (type != hwmon_temp || attr != hwmon_temp_label)
+		return -EOPNOTSUPP;
+	*str = mpt3sas_hwmon_temp_labels[channel];
+	return 0;
+}
+
+static const struct hwmon_channel_info * const mpt3sas_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(temp,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL),
+	NULL,
+};
+
+static const struct hwmon_ops mpt3sas_hwmon_ops = {
+	.is_visible	= _hwmon_is_visible,
+	.read		= _hwmon_read,
+	.read_string	= _hwmon_read_string,
+};
+
+static const struct hwmon_chip_info mpt3sas_hwmon_chip_info = {
+	.ops	= &mpt3sas_hwmon_ops,
+	.info	= mpt3sas_hwmon_info,
+};
+
+/**
+ * mpt3sas_hwmon_register - register an hwmon device for the IOC
+ * @ioc: per adapter object
+ * Context: sleep.
+ *
+ * Succeeds without registering when no temperature sensors are present,
+ * so cards without thermal monitoring do not expose an empty hwmon node.
+ * Paired with mpt3sas_hwmon_unregister() from the driver's remove path.
+ *
+ * Return: 0 for success, non-zero for failure.
+ */
+int
+mpt3sas_hwmon_register(struct MPT3SAS_ADAPTER *ioc)
+{
+	struct device *parent = &ioc->pdev->dev;
+	struct mpt3sas_hwmon *h;
+	struct device *hwdev;
+	Mpi2ConfigReply_t mpi_reply;
+	Mpi2IOUnitPage7_t page;
+	int r;
+
+	h = kzalloc_obj(*h);
+	if (!h)
+		return -ENOMEM;
+
+	h->ioc = ioc;
+
+	r = mpt3sas_config_get_iounit_pg7(ioc, &mpi_reply, &page);
+	if (r) {
+		kfree(h);
+		return r;
+	}
+
+	h->ioc_present = page.IOCTemperatureUnits != MPI2_IOUNITPAGE7_IOC_TEMP_NOT_PRESENT;
+	h->board_present = page.BoardTemperatureUnits != MPI2_IOUNITPAGE7_BOARD_TEMP_NOT_PRESENT;
+
+	/*
+	 * A page where both *TemperatureUnits are NOT_PRESENT covers
+	 * two cases: cards that genuinely lack sensors, and firmware
+	 * errors that left the page zero-filled (the accessor mirrors
+	 * _config_request() behaviour). Either way: skip registration.
+	 */
+	if (!h->ioc_present && !h->board_present) {
+		kfree(h);
+		return 0;
+	}
+
+	hwdev = hwmon_device_register_with_info(parent, "mpt3sas", h,
+						&mpt3sas_hwmon_chip_info,
+						NULL);
+	if (IS_ERR(hwdev)) {
+		kfree(h);
+		return PTR_ERR(hwdev);
+	}
+
+	h->hwmon_dev = hwdev;
+	ioc->hwmon = h;
+	return 0;
+}
+
+/**
+ * mpt3sas_hwmon_unregister - tear down the hwmon device, if any
+ * @ioc: per adapter object
+ *
+ * Safe to call when registration was skipped (no sensors) or
+ * failed; in those cases ioc->hwmon is NULL and this is a no-op.
+ */
+void
+mpt3sas_hwmon_unregister(struct MPT3SAS_ADAPTER *ioc)
+{
+	struct mpt3sas_hwmon *h = ioc->hwmon;
+
+	if (!h)
+		return;
+	hwmon_device_unregister(h->hwmon_dev);
+	kfree(h);
+	ioc->hwmon = NULL;
+}
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 12caffeed3a0..dea78688cc9b 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -12562,6 +12562,7 @@ static void scsih_remove(struct pci_dev *pdev)
 	/* release all the volumes */
 	_scsih_ir_shutdown(ioc);
 	mpt3sas_destroy_debugfs(ioc);
+	mpt3sas_hwmon_unregister(ioc);
 	sas_remove_host(shost);
 	list_for_each_entry_safe(raid_device, next, &ioc->raid_device_list,
 	    list) {
@@ -13651,6 +13652,11 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	}
 
 	scsi_scan_host(shost);
+
+	if (mpt3sas_hwmon_register(ioc))
+		ioc_warn(ioc,
+			 "hwmon registration failed; temperatures not exposed\n");
+
 	mpt3sas_setup_debugfs(ioc);
 	return 0;
 out_add_shost_fail:
-- 
2.54.0

Re: [PATCH 2/2] scsi: mpt3sas: add hwmon support

[Guenter Roeck]

On 5/12/26 14:47, Louis Sautier wrote:
> Expose the IOC and board temperature sensors of LSI / Broadcom SAS
> HBAs through hwmon. Readings come from MPI IO Unit Page 7 via the
> accessor added in the preceding patch.
> 
> The same fields are exposed by Broadcom's userspace tooling
> through the /dev/mpt[23]ctl ioctl path (typically root-only):
> IOCTemperature and BoardTemperature in lsiutil; ROC and Controller
> in storcli. With this driver, sensors(1) shows them unprivileged:
> 
>    $ sensors mpt3sas-pci-0200
>    mpt3sas-pci-0200
>    Adapter: PCI adapter
>    IOC:          +42.0°C
> 
> Each channel is gated independently by its *TemperatureUnits field
> through is_visible(); cards that populate only one sensor expose
> only one input file, and cards that populate neither do not register
> an hwmon device.
> 
> Built into mpt3sas.ko under a new CONFIG_SCSI_MPT3SAS_HWMON Kconfig
> option.
> 
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Louis Sautier <sautier.louis@gmail.com>
> ---
>   Documentation/hwmon/index.rst        |   1 +
>   Documentation/hwmon/mpt3sas.rst      |  57 ++++++++

This is not appropriate. The description is wrong and misleading.
mpt3sas is _not_ a hwmon driver. It is a chip access driver which
happens to support hardware monitoring.

If this is part of the mpt3sas code and not a separate driver,
please keep it there.

Thanks,
Guenter

>   MAINTAINERS                          |   1 +
>   drivers/scsi/mpt3sas/Kconfig         |   9 ++
>   drivers/scsi/mpt3sas/Makefile        |   2 +
>   drivers/scsi/mpt3sas/mpt3sas_base.h  |  17 +++
>   drivers/scsi/mpt3sas/mpt3sas_hwmon.c | 200 +++++++++++++++++++++++++++
>   drivers/scsi/mpt3sas/mpt3sas_scsih.c |   6 +
>   8 files changed, 293 insertions(+)
>   create mode 100644 Documentation/hwmon/mpt3sas.rst
>   create mode 100644 drivers/scsi/mpt3sas/mpt3sas_hwmon.c
> 
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 8b655e5d6b68..106f87fa8b18 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -193,6 +193,7 @@ Hardware Monitoring Kernel Drivers
>      mp9941
>      mp9945
>      mpq8785
> +   mpt3sas
>      nct6683
>      nct6775
>      nct7363
> diff --git a/Documentation/hwmon/mpt3sas.rst b/Documentation/hwmon/mpt3sas.rst
> new file mode 100644
> index 000000000000..3a260a389d6d
> --- /dev/null
> +++ b/Documentation/hwmon/mpt3sas.rst
> @@ -0,0 +1,57 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver mpt3sas
> +=====================
> +
> +Supported chips:
> +
> +  * LSI / Broadcom / Avago SAS HBAs handled by the mpt3sas driver,
> +    such as the 9300, 9400, and 9500 series.
> +
> +    Prefix: ``mpt3sas``
> +
> +
> +Description
> +-----------
> +
> +The mpt3sas driver exposes the IOC and board temperature sensors of
> +LSI / Broadcom SAS HBAs through the hwmon interface.
> +Either or both sensors may be absent depending on the card; the
> +corresponding sysfs files only appear when the firmware reports the
> +sensor as present, and cards that report neither sensor do not
> +register an hwmon device at all.
> +
> +
> +Sysfs entries
> +-------------
> +
> +============  ======================
> +Name          Description
> +============  ======================
> +temp1_input   IOC temperature (mC)
> +temp1_label   "IOC"
> +temp2_input   Board temperature (mC)
> +temp2_label   "Board"
> +============  ======================
> +
> +
> +Cross-reference with vendor tooling
> +-----------------------------------
> +
> +The hwmon channels correspond to fields reported by Broadcom's
> +proprietary tools as follows:
> +
> +=================  ==========================  ===============================
> +hwmon label        lsiutil                     storcli
> +=================  ==========================  ===============================
> +``IOC`` (temp1)    ``IOCTemperature``          ``ROC temperature``
> +``Board`` (temp2)  ``BoardTemperature``        ``Controller temperature``
> +=================  ==========================  ===============================
> +
> +With lsiutil::
> +
> +    lsiutil -pN -a 25,2,0,0
> +
> +With storcli::
> +
> +    storcli /cN show temperature
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b2040011a386..e084f710f436 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15154,6 +15154,7 @@ L:	MPT-FusionLinux.pdl@broadcom.com
>   L:	linux-scsi@vger.kernel.org
>   S:	Supported
>   W:	http://www.avagotech.com/support/
> +F:	Documentation/hwmon/mpt3sas.rst
>   F:	drivers/message/fusion/
>   F:	drivers/scsi/mpt3sas/
>   
> diff --git a/drivers/scsi/mpt3sas/Kconfig b/drivers/scsi/mpt3sas/Kconfig
> index c299f7e078fb..638acd2c6623 100644
> --- a/drivers/scsi/mpt3sas/Kconfig
> +++ b/drivers/scsi/mpt3sas/Kconfig
> @@ -73,6 +73,15 @@ config SCSI_MPT3SAS_MAX_SGE
>   	can be 256. However, it may decreased down to 16.  Decreasing this
>   	parameter will reduce memory requirements on a per controller instance.
>   
> +config SCSI_MPT3SAS_HWMON
> +	bool "LSI MPT Fusion SAS hwmon support"
> +	depends on SCSI_MPT3SAS && HWMON
> +	depends on !(SCSI_MPT3SAS=y && HWMON=m)
> +	help
> +	Say Y here to expose the IOC and board temperature sensors of
> +	LSI / Broadcom SAS HBAs (such as the 9300, 9400, and 9500 series)
> +	through hwmon. See Documentation/hwmon/mpt3sas.rst for details.
> +
>   config SCSI_MPT2SAS
>   	tristate "Legacy MPT2SAS config option"
>   	default n
> diff --git a/drivers/scsi/mpt3sas/Makefile b/drivers/scsi/mpt3sas/Makefile
> index e76d994dbed3..9a2f3ce4158a 100644
> --- a/drivers/scsi/mpt3sas/Makefile
> +++ b/drivers/scsi/mpt3sas/Makefile
> @@ -9,3 +9,5 @@ mpt3sas-y +=  mpt3sas_base.o     \
>   		mpt3sas_trigger_diag.o \
>   		mpt3sas_warpdrive.o \
>   		mpt3sas_debugfs.o \
> +
> +mpt3sas-$(CONFIG_SCSI_MPT3SAS_HWMON) += mpt3sas_hwmon.o
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
> index c655742d0dde..63252f30343b 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.h
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
> @@ -1629,6 +1629,7 @@ struct MPT3SAS_ADAPTER {
>   	u8		is_aero_ioc;
>   	struct dentry	*debugfs_root;
>   	struct dentry	*ioc_dump;
> +	struct mpt3sas_hwmon *hwmon;
>   	PUT_SMID_IO_FP_HIP put_smid_scsi_io;
>   	PUT_SMID_IO_FP_HIP put_smid_fast_path;
>   	PUT_SMID_IO_FP_HIP put_smid_hi_priority;
> @@ -2049,6 +2050,22 @@ void mpt3sas_destroy_debugfs(struct MPT3SAS_ADAPTER *ioc);
>   void mpt3sas_init_debugfs(void);
>   void mpt3sas_exit_debugfs(void);
>   
> +#if IS_ENABLED(CONFIG_SCSI_MPT3SAS_HWMON)
> +int mpt3sas_hwmon_register(struct MPT3SAS_ADAPTER *ioc);
> +void mpt3sas_hwmon_unregister(struct MPT3SAS_ADAPTER *ioc);
> +#else
> +static inline int
> +mpt3sas_hwmon_register(struct MPT3SAS_ADAPTER *ioc)
> +{
> +	return 0;
> +}
> +
> +static inline void
> +mpt3sas_hwmon_unregister(struct MPT3SAS_ADAPTER *ioc)
> +{
> +}
> +#endif
> +
>   /**
>    * _scsih_is_pcie_scsi_device - determines if device is an pcie scsi device
>    * @device_info: bitfield providing information about the device.
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_hwmon.c b/drivers/scsi/mpt3sas/mpt3sas_hwmon.c
> new file mode 100644
> index 000000000000..26227a992f35
> --- /dev/null
> +++ b/drivers/scsi/mpt3sas/mpt3sas_hwmon.c
> @@ -0,0 +1,200 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hardware monitoring (hwmon) support for the LSI / Broadcom mpt3sas
> + * SAS HBA driver. Exposes the IOC and board temperature sensors by
> + * reading MPI IO Unit Page 7.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +
> +#include "mpt3sas_base.h"
> +
> +struct mpt3sas_hwmon {
> +	struct MPT3SAS_ADAPTER *ioc;
> +	struct device *hwmon_dev;
> +	bool ioc_present;
> +	bool board_present;
> +};
> +
> +/*
> + * Convert a (raw, units) reading to millidegrees Celsius.
> + * Returns -ENODATA when the sensor reports "not present" or
> + * unknown units. Temperature values are interpreted as signed
> + * two's-complement integers.
> + *
> + * The MPI2_IOUNITPAGE7_IOC_TEMP_* and MPI2_IOUNITPAGE7_BOARD_TEMP_*
> + * defines in mpi2_cnfg.h share the same values; the IOC ones are
> + * used for both channels.
> + */
> +static int
> +_hwmon_to_mdegc(s16 raw, u8 units, long *out)
> +{
> +	switch (units) {
> +	case MPI2_IOUNITPAGE7_IOC_TEMP_CELSIUS:
> +		*out = (long)raw * 1000;
> +		return 0;
> +	case MPI2_IOUNITPAGE7_IOC_TEMP_FAHRENHEIT:
> +		/* (F - 32) * 5 / 9, expressed in milli-units */
> +		*out = ((long)raw - 32) * 5000 / 9;
> +		return 0;
> +	default:
> +		return -ENODATA;
> +	}
> +}
> +
> +static umode_t
> +_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> +		  u32 attr, int channel)
> +{
> +	const struct mpt3sas_hwmon *h = drvdata;
> +
> +	if (type != hwmon_temp)
> +		return 0;
> +	if (attr != hwmon_temp_input && attr != hwmon_temp_label)
> +		return 0;
> +	if (channel == 0 && h->ioc_present)
> +		return 0444;
> +	if (channel == 1 && h->board_present)
> +		return 0444;
> +	return 0;
> +}
> +
> +static int
> +_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> +	    u32 attr, int channel, long *val)
> +{
> +	struct mpt3sas_hwmon *h = dev_get_drvdata(dev);
> +	Mpi2ConfigReply_t mpi_reply;
> +	Mpi2IOUnitPage7_t page;
> +	int r;
> +
> +	if (type != hwmon_temp || attr != hwmon_temp_input)
> +		return -EOPNOTSUPP;
> +
> +	r = mpt3sas_config_get_iounit_pg7(h->ioc, &mpi_reply, &page);
> +	if (r)
> +		return r;
> +
> +	if (channel == 0)
> +		return _hwmon_to_mdegc((s16)le16_to_cpu(page.IOCTemperature),
> +				       page.IOCTemperatureUnits, val);
> +	if (channel == 1)
> +		return _hwmon_to_mdegc((s16)le16_to_cpu(page.BoardTemperature),
> +				       page.BoardTemperatureUnits, val);
> +	return -EOPNOTSUPP;
> +}
> +
> +static const char * const mpt3sas_hwmon_temp_labels[] = {
> +	"IOC",
> +	"Board",
> +};
> +
> +static int
> +_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
> +		   u32 attr, int channel, const char **str)
> +{
> +	if (type != hwmon_temp || attr != hwmon_temp_label)
> +		return -EOPNOTSUPP;
> +	*str = mpt3sas_hwmon_temp_labels[channel];
> +	return 0;
> +}
> +
> +static const struct hwmon_channel_info * const mpt3sas_hwmon_info[] = {
> +	HWMON_CHANNEL_INFO(temp,
> +			   HWMON_T_INPUT | HWMON_T_LABEL,
> +			   HWMON_T_INPUT | HWMON_T_LABEL),
> +	NULL,
> +};
> +
> +static const struct hwmon_ops mpt3sas_hwmon_ops = {
> +	.is_visible	= _hwmon_is_visible,
> +	.read		= _hwmon_read,
> +	.read_string	= _hwmon_read_string,
> +};
> +
> +static const struct hwmon_chip_info mpt3sas_hwmon_chip_info = {
> +	.ops	= &mpt3sas_hwmon_ops,
> +	.info	= mpt3sas_hwmon_info,
> +};
> +
> +/**
> + * mpt3sas_hwmon_register - register an hwmon device for the IOC
> + * @ioc: per adapter object
> + * Context: sleep.
> + *
> + * Succeeds without registering when no temperature sensors are present,
> + * so cards without thermal monitoring do not expose an empty hwmon node.
> + * Paired with mpt3sas_hwmon_unregister() from the driver's remove path.
> + *
> + * Return: 0 for success, non-zero for failure.
> + */
> +int
> +mpt3sas_hwmon_register(struct MPT3SAS_ADAPTER *ioc)
> +{
> +	struct device *parent = &ioc->pdev->dev;
> +	struct mpt3sas_hwmon *h;
> +	struct device *hwdev;
> +	Mpi2ConfigReply_t mpi_reply;
> +	Mpi2IOUnitPage7_t page;
> +	int r;
> +
> +	h = kzalloc_obj(*h);
> +	if (!h)
> +		return -ENOMEM;
> +
> +	h->ioc = ioc;
> +
> +	r = mpt3sas_config_get_iounit_pg7(ioc, &mpi_reply, &page);
> +	if (r) {
> +		kfree(h);
> +		return r;
> +	}
> +
> +	h->ioc_present = page.IOCTemperatureUnits != MPI2_IOUNITPAGE7_IOC_TEMP_NOT_PRESENT;
> +	h->board_present = page.BoardTemperatureUnits != MPI2_IOUNITPAGE7_BOARD_TEMP_NOT_PRESENT;
> +
> +	/*
> +	 * A page where both *TemperatureUnits are NOT_PRESENT covers
> +	 * two cases: cards that genuinely lack sensors, and firmware
> +	 * errors that left the page zero-filled (the accessor mirrors
> +	 * _config_request() behaviour). Either way: skip registration.
> +	 */
> +	if (!h->ioc_present && !h->board_present) {
> +		kfree(h);
> +		return 0;
> +	}
> +
> +	hwdev = hwmon_device_register_with_info(parent, "mpt3sas", h,
> +						&mpt3sas_hwmon_chip_info,
> +						NULL);
> +	if (IS_ERR(hwdev)) {
> +		kfree(h);
> +		return PTR_ERR(hwdev);
> +	}
> +
> +	h->hwmon_dev = hwdev;
> +	ioc->hwmon = h;
> +	return 0;
> +}
> +
> +/**
> + * mpt3sas_hwmon_unregister - tear down the hwmon device, if any
> + * @ioc: per adapter object
> + *
> + * Safe to call when registration was skipped (no sensors) or
> + * failed; in those cases ioc->hwmon is NULL and this is a no-op.
> + */
> +void
> +mpt3sas_hwmon_unregister(struct MPT3SAS_ADAPTER *ioc)
> +{
> +	struct mpt3sas_hwmon *h = ioc->hwmon;
> +
> +	if (!h)
> +		return;
> +	hwmon_device_unregister(h->hwmon_dev);
> +	kfree(h);
> +	ioc->hwmon = NULL;
> +}
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 12caffeed3a0..dea78688cc9b 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -12562,6 +12562,7 @@ static void scsih_remove(struct pci_dev *pdev)
>   	/* release all the volumes */
>   	_scsih_ir_shutdown(ioc);
>   	mpt3sas_destroy_debugfs(ioc);
> +	mpt3sas_hwmon_unregister(ioc);
>   	sas_remove_host(shost);
>   	list_for_each_entry_safe(raid_device, next, &ioc->raid_device_list,
>   	    list) {
> @@ -13651,6 +13652,11 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	}
>   
>   	scsi_scan_host(shost);
> +
> +	if (mpt3sas_hwmon_register(ioc))
> +		ioc_warn(ioc,
> +			 "hwmon registration failed; temperatures not exposed\n");
> +
>   	mpt3sas_setup_debugfs(ioc);
>   	return 0;
>   out_add_shost_fail:

Re: [PATCH 2/2] scsi: mpt3sas: add hwmon support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unprivileged user-space processes can flood the HBA firmware with synchronous configuration requests, contending a shared mutex and causing a Denial of Service (DoS).
--

commit 12785f6156b64bc1ece9061e087614ba8f75d567
Author: Louis Sautier <sautier.louis@gmail.com>

scsi: mpt3sas: add hwmon support

This commit adds hardware monitoring support for LSI / Broadcom SAS HBAs,
exposing IOC and board temperature sensors. The readings are obtained by
querying MPI IO Unit Page 7 and exposed to userspace through standard
hwmon sysfs files.

> diff --git a/drivers/scsi/mpt3sas/mpt3sas_hwmon.c b/drivers/scsi/mpt3sas/mpt3sas_hwmon.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/scsi/mpt3sas/mpt3sas_hwmon.c
[ ... ]
> +static umode_t
> +_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> +		  u32 attr, int channel)
> +{
[ ... ]
> +	if (channel == 0 && h->ioc_present)
> +		return 0444;
> +	if (channel == 1 && h->board_present)
> +		return 0444;
> +	return 0;
> +}
[ ... ]
> +static int
> +_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> +	    u32 attr, int channel, long *val)
> +{
> +	struct mpt3sas_hwmon *h = dev_get_drvdata(dev);
> +	Mpi2ConfigReply_t mpi_reply;
> +	Mpi2IOUnitPage7_t page;
> +	int r;
> +
> +	if (type != hwmon_temp || attr != hwmon_temp_input)
> +		return -EOPNOTSUPP;
> +
> +	r = mpt3sas_config_get_iounit_pg7(h->ioc, &mpi_reply, &page);

Could exposing this read to unprivileged users lead to excessive firmware
requests and mutex contention?

Since the sysfs files are world-readable, any local user can read them in a
tight loop. Every read directly calls mpt3sas_config_get_iounit_pg7(), which
issues synchronous requests to the firmware over the PCIe bus and acquires
the shared config_cmds.mutex:

_hwmon_read() ->
    mpt3sas_config_get_iounit_pg7() ->
        _config_request() ->
            mutex_lock(&ioc->config_cmds.mutex)

Does this allow an unprivileged user to starve other storage subsystem
components that rely on this mutex for administrative tasks or device
discovery?

Could we implement a caching mechanism (for example, caching the readings
for a short period using jiffies) to rate-limit these firmware requests?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512214703.655633-1-sautier.louis@gmail.com?part=2

Re: [PATCH 2/2] scsi: mpt3sas: add hwmon support

[Louis Sautier]

On 13/05/2026 05:57, Guenter Roeck wrote:
>>   Documentation/hwmon/index.rst        |   1 +
>>   Documentation/hwmon/mpt3sas.rst      |  57 ++++++++
> 
> This is not appropriate. The description is wrong and misleading.
> mpt3sas is _not_ a hwmon driver. It is a chip access driver which
> happens to support hardware monitoring.
> 
> If this is part of the mpt3sas code and not a separate driver,
> please keep it there.
> 
> Thanks,
> Guenter

Hi,

Sorry about that, I had assumed this directory was also meant to contain
documentation for chip drivers that support hardware monitoring.

I will remove the documentation from v2 since there is currently no existing
mpt3sas page under Documentation/scsi/.

Re: [PATCH 2/2] scsi: mpt3sas: add hwmon support

[Guenter Roeck]

On 5/13/26 18:11, Louis Sautier wrote:
> On 13/05/2026 05:57, Guenter Roeck wrote:
>>>    Documentation/hwmon/index.rst        |   1 +
>>>    Documentation/hwmon/mpt3sas.rst      |  57 ++++++++
>>
>> This is not appropriate. The description is wrong and misleading.
>> mpt3sas is _not_ a hwmon driver. It is a chip access driver which
>> happens to support hardware monitoring.
>>
>> If this is part of the mpt3sas code and not a separate driver,
>> please keep it there.
>>
>> Thanks,
>> Guenter
> 
> Hi,
> 
> Sorry about that, I had assumed this directory was also meant to contain
> documentation for chip drivers that support hardware monitoring.
> 

My recommendation is to implement drivers such as this one as auxiliary drivers.
Most people don't want to do that. I accept that, but then I don't want to carry
the documentation either.

Thanks,
Guenter

Re: [PATCH 2/2] scsi: mpt3sas: add hwmon support

[Louis Sautier]

On 14/05/2026 03:02, sashiko-bot@kernel.org wrote:
> Could exposing this read to unprivileged users lead to excessive firmware
> requests and mutex contention?
> 
> Since the sysfs files are world-readable, any local user can read them in a
> tight loop. Every read directly calls mpt3sas_config_get_iounit_pg7(), which
> issues synchronous requests to the firmware over the PCIe bus and acquires
> the shared config_cmds.mutex:
> 
> _hwmon_read() ->
>     mpt3sas_config_get_iounit_pg7() ->
>         _config_request() ->
>             mutex_lock(&ioc->config_cmds.mutex)
> 
> Does this allow an unprivileged user to starve other storage subsystem
> components that rely on this mutex for administrative tasks or device
> discovery?
> 
> Could we implement a caching mechanism (for example, caching the readings
> for a short period using jiffies) to rate-limit these firmware requests?

My initial implementation had a 1-second jiffies-based cache; I
dropped it after seeing that drivers/nvme/host/hwmon.c follows the
same no-cache pattern.

The cover letter's contention figure came from a less direct
measurement and was too optimistic. I re-measured directly on
ioc->config_cmds.mutex, hammering the sysfs temperature file from
N concurrent unprivileged workers on a 9500-8i / SAS3816 while a
separate "administrative" reader runs in the foreground. The same
setup against the system's NVMe is included below for comparison:

hwmon: mpt3sas (/sys/class/hwmon/hwmon3/temp1_input)
nproc: 16

baseline (no concurrent readers): p50=14.6µs  p95=18.9µs

Foreground reader latency with N concurrent unprivileged hammers:

  N   agg r/s   p50 µs   p95 µs   max µs
  1     65244     38.0     60.2     60.8
  2     57722     57.0     60.1    120.5
  4     55026     90.8    115.5    152.9
  8     53688    207.2    247.9    300.1
 16     52188    345.8    390.3    444.8

hwmon: nvme (/sys/class/hwmon/hwmon2/temp1_input)
nproc: 16

baseline (no concurrent readers): p50=268.6µs  p95=289.4µs

Foreground reader latency with N concurrent unprivileged hammers:

  N   agg r/s   p50 µs   p95 µs   max µs
  1      3558    803.8    839.8   1008.3
  2      3549   1342.0   1443.6   1558.7
  4      3543   2154.8   2473.0   2756.6
  8      3518   4130.9   4459.6   4677.4
 16      3522   7394.6   8743.6   8804.3

Aggregate throughput is mutex-bound: every hwmon read takes
config_cmds.mutex, so contention is bounded. The worst
foreground p95 I observed was ~8.5 ms at N=nproc=96 on the
9305-24i (below).

Across the other two test boxes:
- 9305-24i / SAS3224 (nproc=96): mpt3sas baseline 31 µs, p95 at
  N=96 ~8.5 ms; same shape, ~2x slower per read than the more
  recent 9500-8i / SAS3816 above.
- 9211-4i / SAS2004: no mpt3sas sensors reported, no hwmon
  device registered, no contention surface (graceful-skip path
  in the cover letter).

If a maintainer thinks the cache should come back to bound this
independently of the attacker count, I am happy to re-add it.

Re: [PATCH 2/2] scsi: mpt3sas: add hwmon support

[Guenter Roeck]

On 5/13/26 20:39, Louis Sautier wrote:
> On 14/05/2026 03:02, sashiko-bot@kernel.org wrote:
>> Could exposing this read to unprivileged users lead to excessive firmware
>> requests and mutex contention?
>>
>> Since the sysfs files are world-readable, any local user can read them in a
>> tight loop. Every read directly calls mpt3sas_config_get_iounit_pg7(), which
>> issues synchronous requests to the firmware over the PCIe bus and acquires
>> the shared config_cmds.mutex:
>>
>> _hwmon_read() ->
>>      mpt3sas_config_get_iounit_pg7() ->
>>          _config_request() ->
>>              mutex_lock(&ioc->config_cmds.mutex)
>>
>> Does this allow an unprivileged user to starve other storage subsystem
>> components that rely on this mutex for administrative tasks or device
>> discovery?
>>
>> Could we implement a caching mechanism (for example, caching the readings
>> for a short period using jiffies) to rate-limit these firmware requests?
> 
> My initial implementation had a 1-second jiffies-based cache; I
> dropped it after seeing that drivers/nvme/host/hwmon.c follows the
> same no-cache pattern.
> 
> The cover letter's contention figure came from a less direct
> measurement and was too optimistic. I re-measured directly on
> ioc->config_cmds.mutex, hammering the sysfs temperature file from
> N concurrent unprivileged workers on a 9500-8i / SAS3816 while a

If you use a recent kernel, the hwmon subsystem serializes sysfs accesses,
so hammering it from multiple threads should not substantially increase
the back-end load. If you have 16 readers, 15 of them will wait for the
hwmon subsystem lock. All lock contention will happen in the hwmon core.

> separate "administrative" reader runs in the foreground. The same
> setup against the system's NVMe is included below for comparison:
> 
> hwmon: mpt3sas (/sys/class/hwmon/hwmon3/temp1_input)
> nproc: 16
> 
> baseline (no concurrent readers): p50=14.6µs  p95=18.9µs
> 
> Foreground reader latency with N concurrent unprivileged hammers:
> 
>    N   agg r/s   p50 µs   p95 µs   max µs
>    1     65244     38.0     60.2     60.8
>    2     57722     57.0     60.1    120.5
>    4     55026     90.8    115.5    152.9
>    8     53688    207.2    247.9    300.1
>   16     52188    345.8    390.3    444.8
> 
> hwmon: nvme (/sys/class/hwmon/hwmon2/temp1_input)
> nproc: 16
> 
> baseline (no concurrent readers): p50=268.6µs  p95=289.4µs
> 
> Foreground reader latency with N concurrent unprivileged hammers:
> 
>    N   agg r/s   p50 µs   p95 µs   max µs
>    1      3558    803.8    839.8   1008.3
>    2      3549   1342.0   1443.6   1558.7
>    4      3543   2154.8   2473.0   2756.6
>    8      3518   4130.9   4459.6   4677.4
>   16      3522   7394.6   8743.6   8804.3
> 
> Aggregate throughput is mutex-bound: every hwmon read takes
> config_cmds.mutex, so contention is bounded. The worst

It also takes a device mutex in the hwmon core.

Guenter

> foreground p95 I observed was ~8.5 ms at N=nproc=96 on the
> 9305-24i (below).
> 
> Across the other two test boxes:
> - 9305-24i / SAS3224 (nproc=96): mpt3sas baseline 31 µs, p95 at
>    N=96 ~8.5 ms; same shape, ~2x slower per read than the more
>    recent 9500-8i / SAS3816 above.
> - 9211-4i / SAS2004: no mpt3sas sensors reported, no hwmon
>    device registered, no contention surface (graceful-skip path
>    in the cover letter).
> 
> If a maintainer thinks the cache should come back to bound this
> independently of the attacker count, I am happy to re-add it.
> 

Re: [PATCH 2/2] scsi: mpt3sas: add hwmon support

[Louis Sautier]

On Wed, 13 May 2026 22:17:26 -0700, Guenter Roeck wrote:
> If you use a recent kernel, the hwmon subsystem serializes sysfs accesses,
> so hammering it from multiple threads should not substantially increase
> the back-end load. If you have 16 readers, 15 of them will wait for the
> hwmon subsystem lock. All lock contention will happen in the hwmon core.

Right, I wasn't aware of this mechanism. So the amplification
path the bot worried about doesn't actually exist?

Re: [PATCH 2/2] scsi: mpt3sas: add hwmon support

[Guenter Roeck]

On 5/14/26 12:57, Louis Sautier wrote:
> On Wed, 13 May 2026 22:17:26 -0700, Guenter Roeck wrote:
>> If you use a recent kernel, the hwmon subsystem serializes sysfs accesses,
>> so hammering it from multiple threads should not substantially increase
>> the back-end load. If you have 16 readers, 15 of them will wait for the
>> hwmon subsystem lock. All lock contention will happen in the hwmon core.
> 
> Right, I wasn't aware of this mechanism. So the amplification
> path the bot worried about doesn't actually exist?
> 

The bot didn't really claim that multiple hwmon accesses would or could
happen in parallel. Either case, if you want to implement caching in the
driver, please feel free to do so, just don't blame it on lack of
serialization by the hwmon subsystem.

Thanks,
Guenter