Sync commit Upstreamed in linux tree - #21
Open
muralimk-amd wants to merge 24 commits into
Open
Conversation
Revert the out-of-tree variable-sized metrics table support, which this tree carried as commit 88c2e50 ("platform/x86/amd/hsmp: Add support for variable-sized metrics tables") and again, reworked, as commit b0adf52 ("platform/x86/amd/hsmp: Add support for variable-sized metrics tables"). That approach derived the metric table size from the CPU family/model plus the table version reported by HSMP_GET_METRIC_TABLE_VER, stored it once per platform in hsmp_plat_device.hsmp_table_size, and grew hsmp_metric_tbl_read() an offset argument so the sysfs binary attribute could be read in chunks. Upstream solved the same problem differently: the region size is taken straight from HSMP_GET_METRIC_TABLE_DRAM_ADDR args[2], stored per socket, and the table is handed to userspace as one atomic snapshot through the new HSMP_IOCTL_GET_TELEMETRY_DATA ioctl instead of a chunked sysfs read. Those upstream commits are ported in the following patches, so drop this implementation first to keep the driver in sync with upstream and avoid carrying two competing mechanisms. hsmp_metric_tbl_read() is restored to the upstream form, which includes the sock->metric_tbl_addr NULL check from upstream commit 2c78fb287e1f ("platform/x86/amd/hsmp: Ensure sock->metric_tbl_addr is non-NULL"), and the metrics_bin attribute in acpi.c gets its .size back so the two front-ends agree again. The proto_ver gate stays at '>= HSMP_PROTO_VER6' as introduced by commit c8d23ff ("ACPI Driver: Update in Metric Table Logic for ACPI Based Driver"); upstream arrives at the same condition later in this series. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
The mailbox timeout path prints "SMU tmeout". Upstream spells it "SMU timeout"; restore that so the two trees match. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
Although validate_message() checks msg_id, a mispredicted branch can still allow speculative indexing into hsmp_msg_desc_table[]. Clamp msg.msg_id with array_index_nospec() at entry to hsmp_ioctl_msg() so downstream dereferences (including via is_get_msg() and hsmp_send_message()) see a bounded index. Similarly, hsmp_send_message() bounds-checks msg->sock_ind before indexing hsmp_pdev.sock[], but a mispredicted branch can still speculatively use the raw index (Spectre v1, CVE-2017-5753). Apply array_index_nospec() after the check so every caller that reaches hsmp_pdev.sock[] through this helper sees a clamped socket index-including hsmp_ioctl_msg() and any other path that hands a user-derived struct hsmp_message to hsmp_send_message(). Out-of-tree note: linux/nospec.h is included explicitly here rather than relying on it being pulled in transitively, which is not guaranteed across the kernel versions this module builds against. Reviewed-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Link: https://patch.msgid.link/20260612042610.1629037-7-muralidhara.mk@amd.com Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit d20457b46eca76b9bb716dd31af591cad21607b5)
hsmp_get_uid() passed the device UID directly to kstrtou16(uid + 2) without checking it. A NULL UID or one shorter than three characters would dereference a NULL pointer or read past the end of the string. Reject such UIDs with -EINVAL before stripping the "ID" prefix. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Link: https://patch.msgid.link/20260625123337.886435-3-muralidhara.mk@amd.com Link: https://patch.msgid.link/20260629155634.1807598-2-muralidhara.mk@amd.com Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit 4c7d1b4cd75a775326e6802608440f7642284ea6)
hsmp_read_acpi_dsd() dereferenced elements[0] and elements[1] of each mailbox sub-package before confirming the package actually held two elements, allowing an out-of-bounds read on a malformed _DSD. Verify package.count >= 2 first, then fetch the string and integer objects. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Link: https://patch.msgid.link/20260625123337.886435-3-muralidhara.mk@amd.com Link: https://patch.msgid.link/20260629155634.1807598-3-muralidhara.mk@amd.com Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit 0d7a2664494534d6152be9bd9bf1cb1d089c67d3)
…parsers hsmp_read_acpi_crs() and hsmp_read_acpi_dsd() read the ACPI handle and emit error messages via sock->dev. Pass the struct device explicitly to both helpers instead of reading it back from sock->dev. This is a pure refactor with no functional change; it prepares for publishing sock->dev as the data-plane readiness gate only after the socket has been fully initialized, so the parsers must not depend on sock->dev already being set. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Link: https://patch.msgid.link/20260629155634.1807598-4-muralidhara.mk@amd.com Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit b558cbed39af2f7dc6cd86c00916566fc6f170d3)
hsmp_parse_acpi_table() published sock->dev before hsmp_read_acpi_crs() had mapped virt_base_addr. sock->dev is the readiness gate for the lock-free data plane, so on a multi-socket system - where socket 0 exposes /dev/hsmp before later sockets finish probing - an ioctl aimed at a socket still in bring-up could pass the gate and dereference a NULL virt_base_addr. Publish sock->dev last with smp_store_release() once virt_base_addr, the mailbox offsets and the semaphore are initialized, and read it with smp_load_acquire() in hsmp_send_message() so a non-NULL dev guarantees the rest of the socket state is visible. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Link: https://patch.msgid.link/20260629155634.1807598-5-muralidhara.mk@amd.com Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit e5e511f5d9e05e075d114318e707e191ada6e145)
…wsem Add hsmp_sock_rwsem and export it, then hold it for write across ACPI probe, remove and init_acpi() so concurrent per-socket platform probes cannot race the is_probed handshake or the one-time socket-array allocation. Use lockdep_assert_held_write() in init_acpi() to catch incorrect locking under lockdep. An rw_semaphore is used rather than a plain mutex because an upcoming change adds a read side so data-plane messages run concurrently with each other while probe/remove hold it for write to drain in-flight messages. Introducing it as an rwsem now keeps the lock type stable across that change. Out-of-tree note: upstream takes the rwsem with guard(rwsem_write) from linux/cleanup.h, which only exists from v6.5. This module builds against older kernels too, so probe and remove call down_write()/up_write() explicitly instead. The locking scope is unchanged. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Link: https://patch.msgid.link/20260723094656.3806028-2-muralidhara.mk@amd.com Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit 9c40a57258f6cb4109de46467966498e6f90ece6)
…it explicitly The metric-table DRAM region is mapped with devm_ioremap(), which ties the mapping to the socket device's devres scope. An upcoming change lets the ACPI front-end share the socket array across sockets and run its own coordinated teardown, so the mapping can no longer be pinned to a single per-socket devres scope. Map it with plain ioremap() instead and add hsmp_unmap_metric_tbls(), which drops every socket's metric_tbl_addr mapping. The platform driver registers that helper with devm_add_action_or_reset() so the mappings are released on both remove and probe failure, while the socket array itself stays devm-managed. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Link: https://patch.msgid.link/20260723094656.3806028-3-muralidhara.mk@amd.com Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit 5622564fc6924c1607944f0ad22be81ed6fd38fb)
… mutex HSMP_GET_METRIC_TABLE makes the firmware refill a shared per-socket metric DRAM region, which hsmp_metric_tbl_read() then copies out with memcpy_fromio(). Two concurrent readers of the metrics_bin sysfs attribute on the same socket can race: one can trigger a fresh fill while the other is mid-copy and return a torn snapshot. (The hwmon path does not touch this region; it only issues power messages via hsmp_send_message().) Embed a struct mutex metric_read_lock in each hsmp_socket and hold it across the fill-and-copy in hsmp_metric_tbl_read(). Add hsmp_init_metric_read_locks() and hsmp_destroy_metric_read_locks(), which take only struct hsmp_plat_device and iterate pdev->sock[] over pdev->num_sockets so the caller cannot pass a count that disagrees with the array. Wire them into both front-ends' probe and teardown paths so the mutex is always initialized before metrics_bin is exposed: the platform driver and the ACPI driver both drive hsmp_metric_tbl_read() through the same 0444 metrics_bin attribute. Doing this in one patch avoids a bisection point where an ACPI read would lock an uninitialized mutex. Out-of-tree note: upstream holds the mutex with guard(mutex) from linux/cleanup.h, which only exists from v6.5, so this uses explicit mutex_lock()/mutex_unlock() around the same region. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Link: https://patch.msgid.link/20260723094656.3806028-4-muralidhara.mk@amd.com Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit 8da4fedd5d1147d26e6ddbd7be2f1f5519df37dd)
misc_deregister() destroys the device but leaves miscdevice.this_device pointing at the freed struct device. Clear it so any later check of this_device, and a subsequent re-register, does not observe a stale pointer. An upcoming change uses this_device to track whether /dev/hsmp is registered across the shared ACPI sockets and relies on it being NULL after deregister. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Link: https://patch.msgid.link/20260723094656.3806028-5-muralidhara.mk@amd.com Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit 6bd243d5abd91ee7d7fa58a9ed60381da49b4b0a)
…elease The ACPI driver binds one platform device per socket but shares a single socket array and a single /dev/hsmp misc device across them. Replace the is_probed flag with state that tracks this shared ownership: - miscdevice.this_device tells whether /dev/hsmp is registered, so the misc device is registered on the first socket and torn down last. A preceding change clears mdev.this_device on deregister so this gate stays reliable across a re-probe. - a kref tracks the sockets that share the array. The first probe initializes it, each further probe takes a reference and every remove (or probe failure) drops one; the last put runs the release callback. All get/put happen under hsmp_sock_rwsem held for write, so the counting is already serialized and kref's atomic is not strictly needed, but kref gives the clearer get/put interface and a release callback. The shared socket array is allocated with kcalloc() on the first probe and freed by the release callback once the last reference is dropped. hsmp_acpi_sock_release() is the single teardown helper, run from kref_put(): it deregisters /dev/hsmp if registered, unmaps any metric-table DRAM, destroys the per-socket mutexes and frees the array. The remove path and the probe-failure path both reach it through the last put, so the teardown lives in one place. Both paths also clear this socket's dev, so a message issued after a non-final unbind (or to a socket that failed to probe on a multi-socket system, whose array stays alive and whose remove() is never called) cannot reach the mailbox that devres is about to unmap. Two lifetime fixes fall out of the array persisting across a non-final unbind: - hsmp_get_tbl_dram_base() iounmap()s any stale metric_tbl_addr before remapping, so a rebind does not leak one mapping per cycle. It runs during (re)probe before the metric sysfs attribute is exposed, so no reader can be using the old mapping. - The ACPI path registers /dev/hsmp unparented by passing NULL to hsmp_misc_register(). Its per-socket devices can be unbound individually and out of order and the misc device outlives all but the last of them, so parenting it to one socket's device would leave a dangling parent. hsmp_misc_register() now takes the parent from its caller, so the platform driver keeps parenting /dev/hsmp to its single device. hsmp_sock_rwsem is held for write across probe and remove, so the release and probe-failure cleanup run with it already held; an upcoming change adds its read side so the same lock also drains the data plane. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Link: https://patch.msgid.link/20260723094656.3806028-6-muralidhara.mk@amd.com Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit 9b2895edb2b3ec1fb8a5adadb5305f6459151d38)
Before this change the HSMP data plane runs without any coordination with driver teardown: open /dev/hsmp fds and hwmon sysfs reads call hsmp_send_message() while probe and remove bring sockets up and down. misc_deregister() does not drain already-open fds, so an in-flight message can race a concurrent unbind and touch a freed socket array or an unmapped mailbox. Add the read side of hsmp_sock_rwsem to the data plane. Split the message send into hsmp_send_message_locked(), which does the bounds check and MMIO access and asserts the rwsem is held, and hsmp_send_message(), which wraps it in the read side. Probe and remove hold the rwsem for write, so they drain in-flight messages and keep new ones out while they tear a socket down. The probe-time senders run under the probe write lock and so must not take the rwsem again: route hsmp_test(), hsmp_cache_proto_ver() and hsmp_get_tbl_dram_base() through hsmp_send_message_locked() to avoid recursive locking. A single rwsem therefore covers both the data plane and the probe/remove handshake, with no separate probe lock: - acpi.c already holds it for write across probe for the socket-array and misc-registration handshake, so the mailbox handshake now nests under that same lock. - plat.c takes it for write around init_platform_device(). It is not held across devm_add_action_or_reset() so the release action, which also takes it for write, cannot deadlock if that registration fails. Out-of-tree note: upstream uses guard(rwsem_read) and scoped_guard() from linux/cleanup.h, which only exists from v6.5; this uses explicit down_read()/up_read() and down_write()/up_write() over the same regions. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Link: https://patch.msgid.link/20260723094656.3806028-7-muralidhara.mk@amd.com Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit c7acedb2db001160b1ec41cdcc759dd664150666)
Family 1Ah Model 50h-5Fh firmware exposes new HSMP messages (0x29-0x2A,
0x33-0x3A) for PC6/CC6 control, CCD power/thermal monitoring, DIMM
sideband access, floor- and SDPS-limit control, and command-enable
discovery. The same firmware extends three existing SET-only messages
(HSMP_SET_XGMI_LINK_WIDTH 0x0C, HSMP_SET_DF_PSTATE 0x0D,
HSMP_SET_PSTATE_MAX_MIN 0x22) with a read-back path selected by bit[31]
of args[0] (0 = set, 1 = get). Add the new IDs and convert the three
messages to HSMP_SET_GET.
Also add PQoS-related HSMP messages HSMP_PQOS_TRAFFIC_PRIORITY (0x3B)
and HSMP_PQOS_FLOATING_BW (0x3C) with matching hsmp_msg_desc_table[]
descriptors so userspace can reach the new functionality.
Backward compatibility is preserved on prior platforms: new IDs
previously occupied HSMP_RSVD slots, and existing userspace that leaves
bit[31] = 0 continues to take a pure SET path. Converting the three
SET messages to HSMP_SET_GET also keeps them accepted by
validate_message(), which already applies a relaxed upper-bound check
on response_sz for that type.
The message IDs and descriptors already existed out-of-tree; this
replaces them with the upstreamed wording and descriptors so the two
trees no longer drift. That also picks up upstream commit 0d5e2d9b8fcb
("platform/x86/amd/hsmp: mark hsmp_msg_desc_table[] as maybe_unused"),
which this tree had missed.
Two out-of-tree deltas are kept on top:
- HSMP_SET_XGMI_PSTATE_RANGE (0x26) stays {1, 1, HSMP_SET_GET}. Family
1Ah Model 50h-5Fh firmware supports the bit[31] read-back for it, but
that conversion has not been upstreamed yet; downgrading it to
HSMP_SET here would make an existing GET fail with -EINVAL once the
response_sz upper-bound check lands.
- the hsmp_send_message() prototype, which this build needs since it has
no in-kernel asm/amd/hsmp.h to pull it from.
Co-developed-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
Signed-off-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
Link: https://patch.msgid.link/20260727141542.3370108-2-muralidhara.mk@amd.com
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
(cherry picked from commit d30569cb5aadcc623accf0422132d0bc0286a9ff)
… check As HSMP protocol versions evolve, existing message IDs sometimes gain additional response words on newer firmware. validate_message() currently enforces a strict equality (response_sz == table value) for HSMP_SET and HSMP_GET, so userspace compiled against an earlier descriptor table is rejected with -EINVAL when it asks for fewer response words than the in-kernel table now declares - even though that caller has no interest in the additional words. Only HSMP_SET_GET already used a relaxed upper-bound check. Replace the per-type branching with a single upper-bound check for all message types. Userspace can now request fewer response words than hardware provides, while requests that exceed the descriptor table (and therefore the hardware capability) are still rejected. Co-developed-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com> Signed-off-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Link: https://patch.msgid.link/20260727141542.3370108-3-muralidhara.mk@amd.com Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit 9185ad51dfd2bc8bde0c7e7d9f4493d8758d4fab)
The driver hard-codes the metric-table region size to sizeof(struct hsmp_metric_table). That is correct for HSMP protocol version 6 but mis-sizes the ioremap of the SMU DRAM region on newer platforms: Family 1Ah Model 50h-5Fh exposes a ~13 KB table under protocol version 7, and the table is expected to keep growing on future firmware. The same hard-coded value also forces hsmp_metric_tbl_read() to reject any read that follows the actual firmware layout. Pick up the table size from firmware instead. SMU on Family 1Ah Model 50h and later populates HSMP_GET_METRIC_TABLE_DRAM_ADDR's args[2] with the DRAM region size in bytes; older firmware leaves it 0. Bump the descriptor's response_sz to 3 so the field is read, and store the result in the new per-socket hsmp_socket.metric_tbl_size, which is then used both for the ioremap() of the region and as the expected size in hsmp_metric_tbl_read(). The size is stored per socket rather than per platform because hsmp_get_tbl_dram_base() runs once per socket and each socket maps its own region. A single platform-wide field would let the last socket's size be used to copy out of an earlier socket's smaller mapping. Behaviour on existing protocol-version-6 hardware is unchanged. Reading a third response word is safe there: for this command SMU leaves args[2] as 0 rather than a stale value from an earlier mailbox transaction, so the fallback always applies, yielding the same value as the previous hard-coded one, and both the ioremap and the size check produce the same result as before. Out-of-tree note: upstream bumps DRIVER_VERSION 2.5 -> 2.6; this tree carries its own version, bumped 3.0 -> 3.1 here. Co-developed-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com> Signed-off-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com> Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com> Link: https://patch.msgid.link/20260727141542.3370108-4-muralidhara.mk@amd.com Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> (cherry picked from commit 96f1ba765ab55d57ee2a2bf88e05c2ee699c7c5b)
The bundled documentation had drifted a long way behind Documentation/arch/x86/amd_hsmp.rst: it still described struct hsmp_message with a response[] member that no longer exists, listed only Fam19h as supported, pointed at dead PPR/E-SMI links, and was missing everything added upstream since - the metrics_bin binary sysfs file, the per-socket HSMP telemetry sysfs attributes, the expected ACPI device object format and the hwmon power interface. Replace it with the upstream text so the shipped documentation matches the driver in this tree. The following patch adds the telemetry ioctl description on top. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
…reads
The metric table needs to be delivered to userspace as a single
atomic snapshot, but the current sysfs metrics_bin path is a file
read: userspace can read it in chunks and observe a torn snapshot
if an SMU refresh happens between read() calls. The same path is
also bounded by PAGE_SIZE, so the ~13 KB table used by HSMP protocol
version 7 on Family 1Ah Model 50h-5Fh cannot be returned at all,
regardless of how userspace reads it. Rather than extend sysfs to
lift both restrictions, expose the metric table through the
existing HSMP character device using a new ioctl that always copies
the table in one shot.
Add struct hsmp_telemetry_data and HSMP_IOCTL_GET_TELEMETRY_DATA
to the UAPI header. Under the surrounding #pragma pack(4), placing
the __u64 user pointer first gives a tight 16-byte layout that is
identical for 32- and 64-bit callers, and the trailing __u16
reserved field is rejected with -EINVAL if non-zero so future
kernels can repurpose it without breaking already-deployed
userspace. The command is encoded with _IOW because the kernel only
reads the request struct; the snapshot travels through the user
pointer it carries.
The requested size may be anything from one byte up to the size
firmware reported for that socket's table. A short request returns
the leading bytes of the snapshot, so userspace built against an
older table layout keeps working on firmware that grew the table,
mirroring the relaxed response_sz rule applied to HSMP messages
earlier in this series. A request larger than the firmware table is
rejected with -EINVAL rather than short-written, so a caller can
never mistake a partial copy for a full one.
Dispatch hsmp_ioctl() on the ioctl command: the existing message
handler is factored out as hsmp_ioctl_msg() for HSMP_IOCTL_CMD, and
HSMP_IOCTL_GET_TELEMETRY_DATA goes to a new
hsmp_ioctl_get_telemetry() helper.
/dev/hsmp is a singleton character device that outlives an
individual socket unbind, so an ioctl issued on an already-open fd
can run concurrently with socket teardown. hsmp_sock_rwsem is the
driver's contract for that: the data plane takes it for read, and
probe and remove take it for write to drain the data plane before
freeing the socket array, unmapping the metric tables and
destroying the per-socket mutexes. hsmp_ioctl_get_telemetry() takes
it for read across the socket lookup, the checks on that socket's
metric-table state and the table read itself, so none of that state
can be torn down underneath it. Without this the handler would
sleep in its kvmalloc() holding no lock at all, and could resume
with a freed socket, locking a destroyed mutex and reading from an
unmapped iomem region.
The lock is dropped before the copy_to_user(), because faulting in
the destination can block indefinitely on a userfaultfd-backed
buffer and would otherwise leave a socket unbind waiting for the
write lock.
Since hsmp_metric_tbl_read() reached the mailbox through
hsmp_send_message(), which takes hsmp_sock_rwsem itself, calling it
with the lock already held would recursively take the read side and
can deadlock against a queued writer. Split out
hsmp_metric_tbl_read_locked(), which asserts the lock and uses
hsmp_send_message_locked(), and leave hsmp_metric_tbl_read() as a
wrapper that takes the read lock for the sysfs callers. This also
brings the whole fill-and-copy under the rwsem for those callers,
where the memcpy_fromio() previously ran outside it, and makes the
lock order uniformly hsmp_sock_rwsem -> metric_read_lock ->
hsmp_sem.
The user-controlled socket index in HSMP_IOCTL_GET_TELEMETRY_DATA is
clamped with array_index_nospec() before indexing hsmp_pdev.sock[],
mitigating Spectre v1 (CVE-2017-5753). Include linux/nospec.h, which
the file relied on getting transitively.
Out-of-tree delta: cleanup.h class-based cleanup is not available on
the older kernels this module supports, so scoped_guard(rwsem_read)
and the __free(kvfree) bounce buffer are open-coded with explicit
down_read()/up_read() and kvfree() on a goto unwind path.
Upstream commit: 5273183b6362 ("platform/x86/amd/hsmp: Add
IOCTL_GET_TELEMETRY_DATA for metric table reads")
Co-developed-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
Signed-off-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
… ACPI driver
The ACPI driver currently prepares the per-socket metric table only
on HSMP_PROTO_VER6. With protocol version 7 in use on Family 1Ah
Model 50h-5Fh, userspace cannot reach the larger ~13 KB table:
hsmp_get_tbl_dram_base() is skipped, sock->metric_tbl_addr stays
NULL, and the ioctl added earlier in this series has nothing to read.
Widen the proto_ver gate in init_acpi() from '== HSMP_PROTO_VER6'
to '>= HSMP_PROTO_VER6' so the DRAM region is mapped and
sock->metric_tbl_size is populated on protocol version 7 (and any
future compatible version), making the ioctl path functional.
hsmp_metric_tbl_acpi_read() now returns -EOPNOTSUPP whenever the
running protocol version is not VER6, because the sysfs binary
attribute cannot carry a table larger than PAGE_SIZE. Version 7
userspace gets a clear, actionable error and a documented pointer to
HSMP_IOCTL_GET_TELEMETRY_DATA; version 6 userspace sees no change.
The non-ACPI plat.c path is intentionally left untouched: it covers
Family 1Ah Model 0h-Fh hardware fixed at protocol version 6, where
the existing metrics_bin remains the supported interface.
Out-of-tree delta: both proto_ver gates already tested '>=' in this
tree, so only the read handler check and the rationale comments are
added here.
Upstream commit: aca39607c173 ("platform/x86/amd/hsmp: Enable
protocol version 7 metric tables on the ACPI driver")
Co-developed-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
Signed-off-by: Muthusamy Ramalingam <muthusamy.ramalingam@amd.com>
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
Even if hwmon registration fails, HSMP remains accessible through the
device file, so the operation should return success.
This upstream fix predates the series ported into this tree but was
never picked up here, leaving init_acpi() returning the hwmon
registration status and failing the whole ACPI probe over a non-fatal
error.
Upstream commit: de5cec220e4d ("platform/x86/amd/hsmp: Ensure success
even if hwmon registration fails")
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
…l errors
Failure in metric table initialization and hwmon registration are
non-fatal errors. Hence replace them with dev_info().
This upstream change predates the series ported into this tree but was
never picked up here.
Upstream commit: ee1cb9b0e6a8 ("platform/x86/amd/hsmp: Replace
dev_err() with dev_info() for non-fatal errors")
Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
The platform driver builds a static array of 8 + 1 sysfs attribute groups, one per socket, and indexes it by the socket count firmware reports. Upstream guards that assumption by rejecting a count above MAX_AMD_NUM_NODES with a static_assert() tying the check to the array size, but this tree only rejected a count of zero: on a system reporting more than 8 sockets the extra sockets silently got no sysfs group at all. MAX_AMD_NUM_NODES is not available on all kernels supported here, and this tree already carries MAX_AMD_SOCKETS for exactly this purpose, though nothing referenced it. Use it for both the probe-time bound and the static_assert(), which also stops the macro from being dead. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
Housekeeping noticed while diffing this tree against the upstream
driver, no functional change:
- hsmp.c included linux/acpi.h twice and listed linux/mm.h after
linux/mutex.h.
- hsmp.h listed linux/mutex.h after linux/pci.h.
- plat.c had a space before the tab in the .remove initialiser, a
stray blank line inside the attribute-group macro block, and a
misaligned line continuation in it.
- acpi.c indented the pre-5.10 sprintf() fallback in
hsmp_msg_fw_ver_show() with spaces.
Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
The sysfs_emit_at() fallback for kernels older than 5.10 passed the running output length as scnprintf()'s buffer size and always wrote at the start of the buffer, rather than writing at buf + len with the remaining space as the limit. On the first matching source len is still 0, so scnprintf() is told the buffer is zero bytes long, writes nothing and returns 0. len therefore never advances and the attribute reports an empty string on every such kernel, no matter how many limit sources are active. Write at buf + len with PAGE_SIZE - len remaining, matching what sysfs_emit_at() does on newer kernels. Signed-off-by: Muralidhara M K <muralidhara.mk@amd.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.