X86 platform drivers
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: "Shyam Sundar S K" <Shyam-sundar.S-k@amd.com>,
	"Patil Rajesh Reddy" <Patil.Reddy@amd.com>,
	"Yijun Shen" <Yijun.Shen@Dell.com>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Sasha Levin" <sashal@kernel.org>,
	platform-driver-x86@vger.kernel.org
Subject: [PATCH AUTOSEL 6.17] platform/x86/amd/pmf: Fix the custom bios input handling mechanism
Date: Sat, 25 Oct 2025 11:57:19 -0400	[thread overview]
Message-ID: <20251025160905.3857885-208-sashal@kernel.org> (raw)
In-Reply-To: <20251025160905.3857885-1-sashal@kernel.org>

From: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>

[ Upstream commit d82e3d2dd0ba019ac6cdd81e47bf4c8ac895cfa0 ]

Originally, the 'amd_pmf_get_custom_bios_inputs()' function was written
under the assumption that the BIOS would only send a single pending
request for the driver to process. However, following OEM enablement, it
became clear that multiple pending requests for custom BIOS inputs might
be sent at the same time, a scenario that the current code logic does not
support when it comes to handling multiple custom BIOS inputs.

To address this, the code logic needs to be improved to not only manage
multiple simultaneous custom BIOS inputs but also to ensure it is scalable
for future additional inputs.

Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Tested-by: Yijun Shen <Yijun.Shen@Dell.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Link: https://patch.msgid.link/20250901110140.2519072-3-Shyam-sundar.S-k@amd.com
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

- Bug fixed: The original code assumed only one pending bit would ever
  be set for custom BIOS inputs, so multiple simultaneous notifications
  from firmware were mishandled or ignored. This is a real-world OEM-
  triggered bug that affects policy evaluation and thus system behavior
  (performance/thermal) for users.
  - Before: A single-bit switch on `pending_req` handled only exactly
    one notification and treated others as “invalid.”
  - After: Iterates over a bitmask and applies all pending custom BIOS
    inputs, addressing concurrent notifications.

- Scope and risk: Small, localized to AMD PMF Smart PC input plumbing,
  no UAPI changes, and no architectural rework. It mainly:
  - Introduces a small static mapping of notification bits to input
    indices.
  - Switches to a loop over the bitmask to set multiple inputs.
  - Renames the TA input fields to an array to make handling scalable.

- Concrete code changes
  - Introduces a bitmask table and removes rigid enums:
    - Added per-input bit mapping:
      `drivers/platform/x86/amd/pmf/pmf.h:660`
      - `static const struct amd_pmf_pb_bitmap custom_bios_inputs[]
        __used = { {"NOTIFY_CUSTOM_BIOS_INPUT1", BIT(5)},
        {"NOTIFY_CUSTOM_BIOS_INPUT2", BIT(6)}, ... }`
    - Defines the simple bitmap struct:
      `drivers/platform/x86/amd/pmf/pmf.h:655`
      - `struct amd_pmf_pb_bitmap { const char *name; u32 bit_mask; };`
    - This replaces fixed enum dispatch and makes the logic extensible
      and correct for multiple bits.
  - Makes TA inputs scalable but layout-compatible:
    - Replaces two discrete fields with an array of two:
      `drivers/platform/x86/amd/pmf/pmf.h:743`
      - `u32 bios_input_1[2];`
    - This preserves total size/ordering for the two inputs currently
      used and enables indexing (scalable, no user-visible ABI).
  - Correctly handles multiple pending requests:
    - New helper to set the proper field by index (handles non-
      contiguous layout): `drivers/platform/x86/amd/pmf/spc.c:121`
      - `amd_pmf_set_ta_custom_bios_input(in, index, value);`
    - Iterates all pending bits and applies each matching custom BIOS
      input: `drivers/platform/x86/amd/pmf/spc.c:150`
      - Loops over `custom_bios_inputs`, checks `pdev->req.pending_req &
        bit_mask`, and assigns from `pdev->req.custom_policy[i]`.
    - Debug dump now iterates all defined custom inputs instead of only
      two hardcoded fields: `drivers/platform/x86/amd/pmf/spc.c:107`

- Stable backport criteria
  - Fixes a real bug that affects end users (policy decisions based on
    multiple BIOS flags).
  - Small and self-contained to AMD PMF Smart PC path (files: `pmf.h`,
    `spc.c`).
  - Minimal regression risk: logic simply adds proper handling for
    multiple bits; if only one bit is set, behavior remains as before.
    The field change is internal to the driver/TA IPC and not a kernel
    ABI.
  - No architectural overhaul; it’s a straightforward correctness and
    scalability improvement.
  - The commit message clearly explains the OEM-found issue; the patch
    is tested and reviewed.

- Notes
  - Backport where AMD PMF custom BIOS input handling exists. On
    branches without that feature, this patch is not applicable.
  - Later mainline commits add support for more inputs and versions, but
    this change alone fixes the core bug (multiple simultaneous inputs)
    without pulling in larger reworks.

 drivers/platform/x86/amd/pmf/pmf.h | 15 +++++-----
 drivers/platform/x86/amd/pmf/spc.c | 48 +++++++++++++++++++++++-------
 2 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 45b60238d5277..df1b4a4f9586b 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -621,14 +621,14 @@ enum ta_slider {
 	TA_MAX,
 };
 
-enum apmf_smartpc_custom_bios_inputs {
-	APMF_SMARTPC_CUSTOM_BIOS_INPUT1,
-	APMF_SMARTPC_CUSTOM_BIOS_INPUT2,
+struct amd_pmf_pb_bitmap {
+	const char *name;
+	u32 bit_mask;
 };
 
-enum apmf_preq_smartpc {
-	NOTIFY_CUSTOM_BIOS_INPUT1 = 5,
-	NOTIFY_CUSTOM_BIOS_INPUT2,
+static const struct amd_pmf_pb_bitmap custom_bios_inputs[] __used = {
+	{"NOTIFY_CUSTOM_BIOS_INPUT1",     BIT(5)},
+	{"NOTIFY_CUSTOM_BIOS_INPUT2",     BIT(6)},
 };
 
 enum platform_type {
@@ -686,8 +686,7 @@ struct ta_pmf_condition_info {
 	u32 power_slider;
 	u32 lid_state;
 	bool user_present;
-	u32 bios_input1;
-	u32 bios_input2;
+	u32 bios_input_1[2];
 	u32 monitor_count;
 	u32 rsvd2[2];
 	u32 bat_design;
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index 1d90f9382024b..869b4134513f3 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -70,8 +70,20 @@ static const char *ta_slider_as_str(unsigned int state)
 	}
 }
 
+static u32 amd_pmf_get_ta_custom_bios_inputs(struct ta_pmf_enact_table *in, int index)
+{
+	switch (index) {
+	case 0 ... 1:
+		return in->ev_info.bios_input_1[index];
+	default:
+		return 0;
+	}
+}
+
 void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
 {
+	int i;
+
 	dev_dbg(dev->dev, "==== TA inputs START ====\n");
 	dev_dbg(dev->dev, "Slider State: %s\n", ta_slider_as_str(in->ev_info.power_slider));
 	dev_dbg(dev->dev, "Power Source: %s\n", amd_pmf_source_as_str(in->ev_info.power_source));
@@ -90,29 +102,43 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
 	dev_dbg(dev->dev, "Platform type: %s\n", platform_type_as_str(in->ev_info.platform_type));
 	dev_dbg(dev->dev, "Laptop placement: %s\n",
 		laptop_placement_as_str(in->ev_info.device_state));
-	dev_dbg(dev->dev, "Custom BIOS input1: %u\n", in->ev_info.bios_input1);
-	dev_dbg(dev->dev, "Custom BIOS input2: %u\n", in->ev_info.bios_input2);
+	for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
+		dev_dbg(dev->dev, "Custom BIOS input%d: %u\n", i + 1,
+			amd_pmf_get_ta_custom_bios_inputs(in, i));
 	dev_dbg(dev->dev, "==== TA inputs END ====\n");
 }
 #else
 void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in) {}
 #endif
 
+/*
+ * This helper function sets the appropriate BIOS input value in the TA enact
+ * table based on the provided index. We need this approach because the custom
+ * BIOS input array is not continuous, due to the existing TA structure layout.
+ */
+static void amd_pmf_set_ta_custom_bios_input(struct ta_pmf_enact_table *in, int index, u32 value)
+{
+	switch (index) {
+	case 0 ... 1:
+		in->ev_info.bios_input_1[index] = value;
+		break;
+	default:
+		return;
+	}
+}
+
 static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
 					   struct ta_pmf_enact_table *in)
 {
+	unsigned int i;
+
 	if (!pdev->req.pending_req)
 		return;
 
-	switch (pdev->req.pending_req) {
-	case BIT(NOTIFY_CUSTOM_BIOS_INPUT1):
-		in->ev_info.bios_input1 = pdev->req.custom_policy[APMF_SMARTPC_CUSTOM_BIOS_INPUT1];
-		break;
-	case BIT(NOTIFY_CUSTOM_BIOS_INPUT2):
-		in->ev_info.bios_input2 = pdev->req.custom_policy[APMF_SMARTPC_CUSTOM_BIOS_INPUT2];
-		break;
-	default:
-		dev_dbg(pdev->dev, "Invalid preq for BIOS input: 0x%x\n", pdev->req.pending_req);
+	for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++) {
+		if (!(pdev->req.pending_req & custom_bios_inputs[i].bit_mask))
+			continue;
+		amd_pmf_set_ta_custom_bios_input(in, i, pdev->req.custom_policy[i]);
 	}
 
 	/* Clear pending requests after handling */
-- 
2.51.0


  parent reply	other threads:[~2025-10-25 16:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20251025160905.3857885-1-sashal@kernel.org>
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17-6.12] platform/x86/intel-uncore-freq: Fix warning in partitioned system Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17] platform/x86: think-lmi: Add extra TC BIOS error messages Sasha Levin
2025-10-25 15:57 ` Sasha Levin [this message]
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17] platform/x86/intel-uncore-freq: Present unique domain ID per package Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17] platform/x86: x86-android-tablets: Stop using EPROBE_DEFER Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251025160905.3857885-208-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=Patil.Reddy@amd.com \
    --cc=Shyam-sundar.S-k@amd.com \
    --cc=Yijun.Shen@Dell.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=patches@lists.linux.dev \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox