X86 platform drivers
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.17-6.12] platform/x86/intel-uncore-freq: Fix warning in partitioned system
       [not found] <20251025160905.3857885-1-sashal@kernel.org>
@ 2025-10-25 15:55 ` Sasha Levin
  2025-10-25 15:56 ` [PATCH AUTOSEL 6.17] platform/x86: think-lmi: Add extra TC BIOS error messages Sasha Levin
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2025-10-25 15:55 UTC (permalink / raw)
  To: patches, stable
  Cc: Srinivas Pandruvada, Ilpo Järvinen, Sasha Levin,
	platform-driver-x86

From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

[ Upstream commit 6d47b4f08436cb682fb2644e6265a3897fd42a77 ]

A partitioned system configured with only one package and one compute
die, warning will be generated for duplicate sysfs entry. This typically
occurs during the platform bring-up phase.

Partitioned systems expose dies, equivalent to TPMI compute domains,
through the CPUID. Each partitioned system must contains at least one
compute die per partition, resulting in a minimum of two dies per
package. Hence the function topology_max_dies_per_package() returns at
least two, and the condition "topology_max_dies_per_package() > 1"
prevents the creation of a root domain.

In this case topology_max_dies_per_package() will return 1 and root
domain will be created for partition 0 and a duplicate sysfs warning
for partition 1 as both partitions have same package ID.

To address this also check for non zero partition in addition to
topology_max_dies_per_package() > 1.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Link: https://lore.kernel.org/r/20250819211034.3776284-1-srinivas.pandruvada@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

- What it fixes
  - Prevents duplicate sysfs root-domain creation on partitioned systems
    that expose only one die per package via CPU topology, which leads
    to a duplicate-name error and probe failure for the second
    partition.
  - The duplicate arises because both partitions share the same
    `package_id`, so the root-domain sysfs name “package_%02d_die_%02d”
    collides.

- Precise change
  - Adds a guard to skip creating the per-package root domain if the
    device is for a non-zero partition:
    - drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
      tpmi.c:713
      - Changed from `if (topology_max_dies_per_package() > 1)` to `if
        (topology_max_dies_per_package() > 1 || plat_info->partition)`.
  - This ensures only partition 0 attempts the root-domain sysfs,
    avoiding a collision on partition 1.

- Why the issue occurs
  - Platform partition information is provided via TPMI
    (`tpmi_get_platform_data`), including `partition` and `package_id`:
    - drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
      tpmi.c:590
    - drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
      tpmi.c:597
  - The `partition` field comes from `struct oobmsm_plat_info`, where it
    denotes the per-package partition id:
    - include/linux/intel_vsec.h:164
  - Root-domain sysfs naming uses `package_id` and `die_id`:
    - drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
      common.c:274
      - `sprintf(data->name, "package_%02d_die_%02d", data->package_id,
        data->die_id);`
  - On partitioned systems where `topology_max_dies_per_package()`
    (CPUID-based) returns 1, both partition 0 and 1 attempt to create
    the same “package_%02d_die_%02d” entry, causing a duplicate.

- User-visible impact of the bug
  - The duplicate sysfs group creation fails; in the TPMI probe path
    this failure tears down all already-created cluster entries for that
    device:
    - drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
      tpmi.c:721 calls `uncore_freq_add_entry(...)`
    - drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
      tpmi.c:722–723 jumps to `remove_clusters` on error, removing
      entries
  - So this is not just a warning; it can cause probe failure for the
    second partition, removing uncore controls for that partition.

- Why the fix is safe and minimal
  - One-line condition change in a single driver; no API/ABI changes.
  - Only alters behavior when `plat_info->partition != 0`, a case where
    creating the root domain would conflict. Non-partitioned systems
    (`partition == 0`) and multi-die systems
    (`topology_max_dies_per_package() > 1`) are unaffected.
  - The logic remains consistent with existing behavior that already
    skips root-domain creation on multi-die systems.

- Stable backport criteria
  - Fixes a real bug that affects users of partitioned platforms
    (duplicate sysfs + probe failure).
  - Small, contained change with minimal regression risk.
  - No architectural changes or new features; confined to `platform/x86`
    Intel uncore-frequency TPMI path.

Given the above, this is a clear, low-risk bug fix that prevents a
probe-time failure on partitioned systems and should be backported to
stable.

 .../platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
index bfcf92aa4d69d..3e531fd1c6297 100644
--- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
+++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
@@ -638,7 +638,7 @@ static int uncore_probe(struct auxiliary_device *auxdev, const struct auxiliary_
 
 	auxiliary_set_drvdata(auxdev, tpmi_uncore);
 
-	if (topology_max_dies_per_package() > 1)
+	if (topology_max_dies_per_package() > 1 || plat_info->partition)
 		return 0;
 
 	tpmi_uncore->root_cluster.root_domain = true;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH AUTOSEL 6.17] platform/x86: think-lmi: Add extra TC BIOS error messages
       [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 ` Sasha Levin
  2025-10-25 15:57 ` [PATCH AUTOSEL 6.17] platform/x86/amd/pmf: Fix the custom bios input handling mechanism Sasha Levin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2025-10-25 15:56 UTC (permalink / raw)
  To: patches, stable
  Cc: Mark Pearson, Kean Ren, Ilpo Järvinen, Sasha Levin,
	derekjohn.clark, platform-driver-x86

From: Mark Pearson <mpearson-lenovo@squebb.ca>

[ Upstream commit a0d6959c345d89d811288a718e3f6b145dcadc8c ]

Add extra error messages that are used by ThinkCenter platforms.

Signed-off-by: Kean Ren <kean0048@gmail.com>
Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Link: https://lore.kernel.org/r/20250903173824.1472244-4-mpearson-lenovo@squebb.ca
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

- What it fixes
  - On ThinkCentre systems, BIOS WMI calls for certificate operations
    can return TC‑specific strings that this driver did not recognize.
    Unknown strings fall back to -EPERM, so a successful operation could
    be reported as failure, and real failures would be collapsed into a
    generic error. This creates real user-facing breakage for sysfs
    writes that manage BIOS certificates.

- Where the bug is
  - Error mapping logic: `tlmi_errstr_to_err()` maps BIOS strings to
    errno by scanning `tlmi_errs[]`, and returns -EPERM on no match:
    drivers/platform/x86/lenovo/think-lmi.c:247–257.
  - All BIOS WMI method wrappers use this path via `tlmi_simple_call()`:
    drivers/platform/x86/lenovo/think-lmi.c:273–300. Any non-zero
    mapping is propagated as the sysfs write result.

- What changed
  - Added ThinkCentre-specific strings to the mapping table
    `tlmi_errs[]`:
    - Success string: `"Set Certificate operation was successful."` →
      `0`
    - Specific failure strings: invalid parameter/type/password, retry
      exceeded, password invalid, operation aborted, no free slots,
      certificate not found, internal error, certificate too large →
      appropriate `-EINVAL`, `-EACCES`, `-EBUSY`, `-ENOSPC`, `-EEXIST`,
      `-EFAULT`, `-EFBIG`
    - Location: drivers/platform/x86/lenovo/think-lmi.c:207–224
  - This ensures ThinkCentre BIOS responses are properly interpreted
    instead of defaulting to -EPERM.

- Why it matters in practice
  - Certificate operations in this driver (e.g., install/update/clear
    certificate, cert→password) call `tlmi_simple_call()` with
    ThinkCentre certificate GUIDs (see call sites in
    `certificate_store()` and `cert_to_password_store()`):
    drivers/platform/x86/lenovo/think-lmi.c:841, 895–906, 795. With the
    old table, a genuine success response like `"Set Certificate
    operation was successful."` would be treated as an error (-EPERM),
    causing sysfs writes such as `.../authentication/*/certificate` to
    fail even though the BIOS accepted the operation.
  - The new entries also surface more precise errno for failures,
    improving diagnostics for userspace tools and admins.

- Risk and scope
  - Minimal: a localized addition to a string→errno table; no control
    flow or architectural changes.
  - Affects only Lenovo think-lmi driver behavior on ThinkCentre
    platforms when handling certificate-related WMI responses.
  - No user-visible API changes beyond correcting erroneous return
    codes; improves correctness and debuggability.

- Stable backport fit
  - Fixes a real user-impacting bug (false -EPERM on success, ambiguous
    errors).
  - Small, self-contained, and low-risk.
  - Confined to platform/x86/lenovo/think-lmi.

Given the above, this is a good candidate for stable backporting.

 drivers/platform/x86/lenovo/think-lmi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/platform/x86/lenovo/think-lmi.c b/drivers/platform/x86/lenovo/think-lmi.c
index 0992b41b6221d..e6a2c8e94cfdc 100644
--- a/drivers/platform/x86/lenovo/think-lmi.c
+++ b/drivers/platform/x86/lenovo/think-lmi.c
@@ -179,10 +179,21 @@ MODULE_PARM_DESC(debug_support, "Enable debug command support");
 
 static const struct tlmi_err_codes tlmi_errs[] = {
 	{"Success", 0},
+	{"Set Certificate operation was successful.", 0},
 	{"Not Supported", -EOPNOTSUPP},
 	{"Invalid Parameter", -EINVAL},
 	{"Access Denied", -EACCES},
 	{"System Busy", -EBUSY},
+	{"Set Certificate operation failed with status:Invalid Parameter.", -EINVAL},
+	{"Set Certificate operation failed with status:Invalid certificate type.", -EINVAL},
+	{"Set Certificate operation failed with status:Invalid password format.", -EINVAL},
+	{"Set Certificate operation failed with status:Password retry count exceeded.", -EACCES},
+	{"Set Certificate operation failed with status:Password Invalid.", -EACCES},
+	{"Set Certificate operation failed with status:Operation aborted.", -EBUSY},
+	{"Set Certificate operation failed with status:No free slots to write.", -ENOSPC},
+	{"Set Certificate operation failed with status:Certificate not found.", -EEXIST},
+	{"Set Certificate operation failed with status:Internal error.", -EFAULT},
+	{"Set Certificate operation failed with status:Certificate too large.", -EFBIG},
 };
 
 static const char * const encoding_options[] = {
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH AUTOSEL 6.17] platform/x86/amd/pmf: Fix the custom bios input handling mechanism
       [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
  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
  4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2025-10-25 15:57 UTC (permalink / raw)
  To: patches, stable
  Cc: Shyam Sundar S K, Patil Rajesh Reddy, Yijun Shen,
	Ilpo Järvinen, Sasha Levin, platform-driver-x86

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH AUTOSEL 6.17] platform/x86/intel-uncore-freq: Present unique domain ID per package
       [not found] <20251025160905.3857885-1-sashal@kernel.org>
                   ` (2 preceding siblings ...)
  2025-10-25 15:57 ` [PATCH AUTOSEL 6.17] platform/x86/amd/pmf: Fix the custom bios input handling mechanism Sasha Levin
@ 2025-10-25 16:00 ` Sasha Levin
  2025-10-25 16:00 ` [PATCH AUTOSEL 6.17] platform/x86: x86-android-tablets: Stop using EPROBE_DEFER Sasha Levin
  4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2025-10-25 16:00 UTC (permalink / raw)
  To: patches, stable
  Cc: Srinivas Pandruvada, Ilpo Järvinen, Sasha Levin,
	platform-driver-x86

From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

[ Upstream commit a191224186ec16a4cb1775b2a647ea91f5c139e1 ]

In partitioned systems, the domain ID is unique in the partition and a
package can have multiple partitions.

Some user-space tools, such as turbostat, assume the domain ID is unique
per package. These tools map CPU power domains, which are unique to a
package. However, this approach does not work in partitioned systems.

There is no architectural definition of "partition" to present to user
space.

To support these tools, set the domain_id to be unique per package. For
compute die IDs, uniqueness can be achieved using the platform info
cdie_mask, mirroring the behavior observed in non-partitioned systems.

For IO dies, which lack a direct CPU relationship, any unique logical
ID can be assigned. Here domain IDs for IO dies are configured after all
compute domain IDs. During the probe, keep the index of the next IO
domain ID after the last IO domain ID of the current partition. Since
CPU packages are symmetric, partition information is same for all
packages.

The Intel Speed Select driver has already implemented a similar change
to make the domain ID unique, with compute dies listed first, followed
by I/O dies.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Link: https://lore.kernel.org/r/20250903191154.1081159-1-srinivas.pandruvada@linux.intel.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

- What it fixes
  - Fixes a user-visible inconsistency in partitioned systems where
    `domain_id` values repeat per partition instead of being unique per
    package, which breaks userspace tools that assume package-unique
    domain IDs (e.g., turbostat). Turbostat reads `domain_id` from sysfs
    and uses it to name counters per package
    (tools/power/x86/turbostat/turbostat.c:7065,
    tools/power/x86/turbostat/turbostat.c:7072), so duplicate IDs cause
    mislabeling/misaggregation.

- Key changes
  - Introduces package-unique domain ID assignment logic via
    `set_domain_id()`, replacing the previous direct assignment
    `domain_id = i` done during probe.
    - Adds the new helper and supporting state:
      - `MAX_PARTITIONS`, `io_die_start[]`, `io_die_index_next`,
        `domain_lock` to coordinate ID space allocation across
        partitions (drivers/platform/x86/intel/uncore-frequency/uncore-
        frequency-tpmi.c:377–386).
      - New `set_domain_id(int id, int num_resources, struct
        oobmsm_plat_info *plat_info, struct tpmi_uncore_cluster_info
        *cluster_info)` that:
        - Returns old behavior if `plat_info->partition >=
          MAX_PARTITIONS` (drivers/platform/x86/intel/uncore-
          frequency/uncore-frequency-tpmi.c:394–397).
        - For compute dies (AGENT_TYPE_CORE), sets `domain_id = cdie_id`
          to mirror non-partitioned behavior
          (drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
          tpmi.c:399–402).
        - For IO dies, allocates IDs after the compute-die range using
          `io_die_start[partition]` and advances `io_die_index_next`,
          protected by `domain_lock` to ensure uniqueness across
          partitions (drivers/platform/x86/intel/uncore-
          frequency/uncore-frequency-tpmi.c:404–445).
    - Hooks the new logic into probe by removing
      `cluster_info->uncore_data.domain_id = i` and calling
      `set_domain_id(...)` instead (drivers/platform/x86/intel/uncore-
      frequency/uncore-frequency-tpmi.c:684–689).
  - Leaves non-partitioned systems behavior unchanged (compute dies use
    `cdie_id`, IO dies follow compute dies, matching pre-existing
    expectations).

- Why it matters (userspace impact)
  - The driver exposes `domain_id` via sysfs
    (drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
    common.c:25–33, 200–208) and creates per-domain entries used by
    turbostat (e.g., `uncoreXX/domain_id`, `.../package_id`,
    `.../fabric_cluster_id`). Turbostat assumes `domain_id` is unique
    per package to generate per-domain counter names
    (tools/power/x86/turbostat/turbostat.c:7054–7099), which is violated
    in partitioned systems without this patch. This is a clear user-
    visible bug fix, not a feature.

- Scope and risk
  - Small, contained change in one driver file; pure ID assignment
    during probe. No changes to the uncore frequency control logic, MMIO
    programming, or sysfs schema. Only the values of `domain_id` change
    for partitioned platforms.
  - Concurrency is correctly handled by `domain_lock`
    (drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
    tpmi.c:385–386, 415).
  - Safe fallback: if an unexpected partition count (`>=
    MAX_PARTITIONS`) appears, it falls back to the old `domain_id = id`
    behavior (drivers/platform/x86/intel/uncore-frequency/uncore-
    frequency-tpmi.c:394–397), avoiding regressions.
  - Non-partitioned systems and compute dies keep previous semantics
    (`domain_id = cdie_id`), preserving existing userspace behavior
    (drivers/platform/x86/intel/uncore-frequency/uncore-frequency-
    tpmi.c:399–402).

- Alignment with stable criteria
  - Fixes an important userspace-visible bug (domain ID non-uniqueness
    per package in partitioned systems).
  - Change is minimal and isolated to a single driver’s probe-time
    bookkeeping.
  - No architectural changes; no cross-subsystem impact; low regression
    risk.
  - Mirrors existing precedent in a related Intel driver (commit message
    notes Intel Speed Select already made domain IDs unique in a similar
    way).

- Backport notes
  - The patch uses `guard(mutex)` (include/linux/cleanup.h); if
    backporting to older stable kernels lacking this helper, a trivial
    conversion to `mutex_lock()`/`mutex_unlock()` is sufficient.
  - Depends on `struct oobmsm_plat_info` providing `partition` and
    `cdie_mask` (include/linux/intel_vsec.h:161–172), and on the TPMI
    plumbing already present in the target stable series. Ensure these
    prior platform bits exist in that series.

Given the userspace breakage it resolves and the low-risk, self-
contained nature of the change, this is a good candidate for stable
backport.

 .../uncore-frequency/uncore-frequency-tpmi.c  | 74 ++++++++++++++++++-
 1 file changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
index 3e531fd1c6297..1237d95708865 100644
--- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
+++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
@@ -374,6 +374,77 @@ static void uncore_set_agent_type(struct tpmi_uncore_cluster_info *cluster_info)
 	cluster_info->uncore_data.agent_type_mask = FIELD_GET(UNCORE_AGENT_TYPES, status);
 }
 
+#define MAX_PARTITIONS	2
+
+/* IO domain ID start index for a partition */
+static u8 io_die_start[MAX_PARTITIONS];
+
+/* Next IO domain ID index after the current partition IO die IDs */
+static u8 io_die_index_next;
+
+/* Lock to protect io_die_start, io_die_index_next */
+static DEFINE_MUTEX(domain_lock);
+
+static void set_domain_id(int id,  int num_resources,
+			  struct oobmsm_plat_info *plat_info,
+			  struct tpmi_uncore_cluster_info *cluster_info)
+{
+	u8 part_io_index, cdie_range, pkg_io_index, max_dies;
+
+	if (plat_info->partition >= MAX_PARTITIONS) {
+		cluster_info->uncore_data.domain_id = id;
+		return;
+	}
+
+	if (cluster_info->uncore_data.agent_type_mask & AGENT_TYPE_CORE) {
+		cluster_info->uncore_data.domain_id = cluster_info->cdie_id;
+		return;
+	}
+
+	/* Unlikely but cdie_mask may have holes, so take range */
+	cdie_range = fls(plat_info->cdie_mask) - ffs(plat_info->cdie_mask) + 1;
+	max_dies = topology_max_dies_per_package();
+
+	/*
+	 * If the CPU doesn't enumerate dies, then use current cdie range
+	 * as the max.
+	 */
+	if (cdie_range > max_dies)
+		max_dies = cdie_range;
+
+	guard(mutex)(&domain_lock);
+
+	if (!io_die_index_next)
+		io_die_index_next = max_dies;
+
+	if (!io_die_start[plat_info->partition]) {
+		io_die_start[plat_info->partition] = io_die_index_next;
+		/*
+		 * number of IO dies = num_resources - cdie_range. Hence
+		 * next partition io_die_index_next is set after IO dies
+		 * in the current partition.
+		 */
+		io_die_index_next += (num_resources - cdie_range);
+	}
+
+	/*
+	 * Index from IO die start within the partition:
+	 * This is the first valid domain after the cdies.
+	 * For example the current resource index 5 and cdies end at
+	 * index 3 (cdie_cnt = 4). Then the IO only index 5 - 4 = 1.
+	 */
+	part_io_index = id - cdie_range;
+
+	/*
+	 * Add to the IO die start index for this partition in this package
+	 * to make unique in the package.
+	 */
+	pkg_io_index = io_die_start[plat_info->partition] + part_io_index;
+
+	/* Assign this to domain ID */
+	cluster_info->uncore_data.domain_id = pkg_io_index;
+}
+
 /* Callback for sysfs read for TPMI uncore values. Called under mutex locks. */
 static int uncore_read(struct uncore_data *data, unsigned int *value, enum uncore_index index)
 {
@@ -610,11 +681,12 @@ static int uncore_probe(struct auxiliary_device *auxdev, const struct auxiliary_
 			cluster_info->uncore_data.package_id = pkg;
 			/* There are no dies like Cascade Lake */
 			cluster_info->uncore_data.die_id = 0;
-			cluster_info->uncore_data.domain_id = i;
 			cluster_info->uncore_data.cluster_id = j;
 
 			set_cdie_id(i, cluster_info, plat_info);
 
+			set_domain_id(i, num_resources, plat_info, cluster_info);
+
 			cluster_info->uncore_root = tpmi_uncore;
 
 			if (TPMI_MINOR_VERSION(pd_info->ufs_header_ver) >= UNCORE_ELC_SUPPORTED_VERSION)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH AUTOSEL 6.17] platform/x86: x86-android-tablets: Stop using EPROBE_DEFER
       [not found] <20251025160905.3857885-1-sashal@kernel.org>
                   ` (3 preceding siblings ...)
  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 ` Sasha Levin
  4 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2025-10-25 16:00 UTC (permalink / raw)
  To: patches, stable
  Cc: Hans de Goede, Dmitry Torokhov, Andy Shevchenko,
	Ilpo Järvinen, Sasha Levin, platform-driver-x86

From: Hans de Goede <hansg@kernel.org>

[ Upstream commit 01fd7cf3534aa107797d130f461ba7bcad30414d ]

Since the x86-android-tablets code uses platform_create_bundle() it cannot
use EPROBE_DEFER and the driver-core will translate EPROBE_DEFER to ENXIO.

Stop using EPROBE_DEFER instead log an error and return ENODEV, or for
non-fatal cases log a warning and return 0.

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Hans de Goede <hansg@kernel.org>
Link: https://patch.msgid.link/20250920200713.20193-21-hansg@kernel.org
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
- `__platform_driver_probe()` sets `drv->prevent_deferred_probe = true`
  and `platform_probe()` converts any `-EPROBE_DEFER` into `-ENXIO` with
  only a warning (drivers/base/platform.c:935,1408-1410). The
  x86-android-tablets driver is created through
  `platform_create_bundle()` (core.c:523-530), so any deferral request
  from this code path is doomed to a permanent failure of the bundle.
- Before this commit `get_serdev_controller_by_pci_parent()` returned
  `ERR_PTR(-EPROBE_DEFER)` when the PCI parent was missing, which
  immediately tripped the `prevent_deferred_probe` guard and killed the
  whole probe with an opaque `-ENXIO`. The patch replaces that with an
  explicit error message and `-ENODEV` (core.c:276-282), aligning the
  driver with the documented restriction in `x86_android_tablet_probe()`
  that “it cannot use -EPROBE_DEFER” (core.c:411-416). This removes the
  bogus deferral while keeping the failure visible to users and
  diagnostic logs intact.
- The more severe issue was in `vexia_edu_atla10_9v_init()`: if the
  expected SDIO PCI function was absent, the code returned
  `-EPROBE_DEFER`, which, once translated to `-ENXIO`, caused
  `x86_android_tablet_probe()` to unwind and prevented every board quirk
  (touchscreen, sensors, etc.) from being instantiated. The fix
  downgrades this path to a warning and success return
  (other.c:701-716), allowing the tablet support driver to finish
  probing even when that optional Wi-Fi controller is missing or late to
  appear.
- No behaviour changes occur on the success paths; only error-handling
  logic is touched, so the regression risk is very low. The change is
  self-contained, affects just two helper functions, and has no
  dependency on the rest of the series. Given that the preexisting code
  can leave entire tablet models without platform devices because of an
  impossible deferral, this is an important bugfix that fits stable
  backport criteria.

 drivers/platform/x86/x86-android-tablets/core.c  | 6 ++++--
 drivers/platform/x86/x86-android-tablets/other.c | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index 2a9c471785050..8c8f10983f289 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -277,8 +277,10 @@ get_serdev_controller_by_pci_parent(const struct x86_serdev_info *info)
 	struct pci_dev *pdev;
 
 	pdev = pci_get_domain_bus_and_slot(0, 0, info->ctrl.pci.devfn);
-	if (!pdev)
-		return ERR_PTR(-EPROBE_DEFER);
+	if (!pdev) {
+		pr_err("error could not get PCI serdev at devfn 0x%02x\n", info->ctrl.pci.devfn);
+		return ERR_PTR(-ENODEV);
+	}
 
 	/* This puts our reference on pdev and returns a ref on the ctrl */
 	return get_serdev_controller_from_parent(&pdev->dev, 0, info->ctrl_devname);
diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index f7bd9f863c85e..aa4f8810974d5 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -809,8 +809,10 @@ static int __init vexia_edu_atla10_9v_init(struct device *dev)
 
 	/* Reprobe the SDIO controller to enumerate the now enabled Wifi module */
 	pdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0x11, 0));
-	if (!pdev)
-		return -EPROBE_DEFER;
+	if (!pdev) {
+		pr_warn("Could not get PCI SDIO at devfn 0x%02x\n", PCI_DEVFN(0x11, 0));
+		return 0;
+	}
 
 	ret = device_reprobe(&pdev->dev);
 	if (ret)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-10-25 16:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [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 ` [PATCH AUTOSEL 6.17] platform/x86/amd/pmf: Fix the custom bios input handling mechanism Sasha Levin
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox