Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 0/4] HID: amd_sfh: Add DMI quirk and support for new AMD SoCs
@ 2026-09-02 13:44 Basavaraj Natikar
  2026-09-02 13:44 ` [PATCH 1/4] HID: amd_sfh: Add DMI quirk and module param to disable interrupts Basavaraj Natikar
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Basavaraj Natikar @ 2026-09-02 13:44 UTC (permalink / raw)
  To: jikos, bentiss; +Cc: linux-input, Basavaraj Natikar

This series makes two independent improvements to the amd-sfh HID
driver: a DMI quirk for an existing platform, and Sensor Fusion Hub
(SFH) support for newer AMD SoCs (MP2 1.2).

Patch 1 exposes intr_disable as a module parameter and adds a DMI quirk
for the ASUS VivoBook TP420UA/TM420UA, which needs interrupts disabled
for correct sensor operation.

Patch 2 enables the SFH on newer SoCs by adding PCI device ID 0x1152;
MP2 1.2 reuses the MP2 1.1 initialization and operation paths.

Patches 3 and 4 switch human presence detection (HPD) and device-mode
reads to DRAM instead of the C2P registers on these newer SoCs.

Basavaraj Natikar (3):
  HID: amd_sfh: Add support for newer AMD SoCs
  HID: amd_sfh: Add HPD support for new AMD SOC
  HID: amd_sfh: Add device mode support for latest AMD SOC.

Helge Bahmann (1):
  HID: amd_sfh: Add DMI quirk and module param to disable interrupts

 drivers/hid/amd-sfh-hid/amd_sfh_common.h      |  2 +
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c        | 10 +++++
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 13 ++++++-
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 37 +++++++++++++++++--
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    |  7 ++++
 5 files changed, 63 insertions(+), 6 deletions(-)

-- 
2.34.1


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

* [PATCH 1/4] HID: amd_sfh: Add DMI quirk and module param to disable interrupts
  2026-09-02 13:44 [PATCH 0/4] HID: amd_sfh: Add DMI quirk and support for new AMD SoCs Basavaraj Natikar
@ 2026-09-02 13:44 ` Basavaraj Natikar
  2026-09-02 13:54   ` sashiko-bot
  2026-09-02 13:44 ` [PATCH 2/4] HID: amd_sfh: Add support for newer AMD SoCs Basavaraj Natikar
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Basavaraj Natikar @ 2026-09-02 13:44 UTC (permalink / raw)
  To: jikos, bentiss; +Cc: linux-input, Helge Bahmann, Basavaraj Natikar

From: Helge Bahmann <hcb@chaoticmind.net>

Expose intr_disable as a module parameter to allow runtime override.
Add DMI quirk for ASUSTeK VivoBook TP420UA/TM420UA which requires
interrupts disabled for correct sensor operation.

Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index eda26a094d3f..92486f25460b 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -41,6 +41,8 @@ module_param_named(sensor_mask, sensor_mask_override, int, 0444);
 MODULE_PARM_DESC(sensor_mask, "override the detected sensors mask");
 
 static bool intr_disable = true;
+module_param_named(intr_disable, intr_disable, bool, 0444);
+MODULE_PARM_DESC(intr_disable, "override the interrupt disable sensor bit");
 
 static int amd_sfh_wait_response_v2(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts)
 {
@@ -313,6 +315,13 @@ static const struct dmi_system_id dmi_sfh_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "HP ProBook x360 435 G7"),
 		},
 	},
+	{
+		.callback = mp2_disable_intr,
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "VivoBook_ASUSLaptop TP420UA_TM420UA"),
+		},
+	},
 	{}
 };
 
-- 
2.34.1


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

* [PATCH 2/4] HID: amd_sfh: Add support for newer AMD SoCs
  2026-09-02 13:44 [PATCH 0/4] HID: amd_sfh: Add DMI quirk and support for new AMD SoCs Basavaraj Natikar
  2026-09-02 13:44 ` [PATCH 1/4] HID: amd_sfh: Add DMI quirk and module param to disable interrupts Basavaraj Natikar
@ 2026-09-02 13:44 ` Basavaraj Natikar
  2026-09-02 13:44 ` [PATCH 3/4] HID: amd_sfh: Add HPD support for new AMD SOC Basavaraj Natikar
  2026-09-02 13:44 ` [PATCH 4/4] HID: amd_sfh: Add device mode support for latest " Basavaraj Natikar
  3 siblings, 0 replies; 8+ messages in thread
From: Basavaraj Natikar @ 2026-09-02 13:44 UTC (permalink / raw)
  To: jikos, bentiss; +Cc: linux-input, Basavaraj Natikar, Guruvendra Punugupati

Add PCI device ID 0x1152 for AMD Sensor Fusion Hub on newer AMD
SoCs. MP2 1.2 shares the same initialization and operations as
MP2 1.1.

Co-developed-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/hid/amd-sfh-hid/amd_sfh_common.h | 2 ++
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c   | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index f8c6b7fc34fb..a455bb30fc47 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -17,6 +17,7 @@
 
 #define PCI_DEVICE_ID_AMD_MP2		0x15E4
 #define PCI_DEVICE_ID_AMD_MP2_1_1	0x164A
+#define PCI_DEVICE_ID_AMD_MP2_1_2	0x1152
 
 #define AMD_C2P_MSG(regno) (0x10500 + ((regno) * 4))
 #define AMD_P2C_MSG(regno) (0x10680 + ((regno) * 4))
@@ -39,6 +40,7 @@ enum cmd_id {
 enum amd_mp2_version {
 	MP2_VER_V2 = 1,
 	MP2_VER_1_1 = 2,
+	MP2_VER_1_2 = 3,
 };
 
 struct amd_mp2_sensor_info {
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index 92486f25460b..fe4b00227eca 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -597,6 +597,7 @@ static SIMPLE_DEV_PM_OPS(amd_mp2_pm_ops, amd_mp2_pci_suspend,
 static const struct pci_device_id amd_mp2_pci_tbl[] = {
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_MP2) },
 	{ PCI_DEVICE_DATA(AMD, MP2_1_1, MP2_VER_1_1) },
+	{ PCI_DEVICE_DATA(AMD, MP2_1_2, MP2_VER_1_2) },
 	{ }
 };
 MODULE_DEVICE_TABLE(pci, amd_mp2_pci_tbl);
-- 
2.34.1


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

* [PATCH 3/4] HID: amd_sfh: Add HPD support for new AMD SOC
  2026-09-02 13:44 [PATCH 0/4] HID: amd_sfh: Add DMI quirk and support for new AMD SoCs Basavaraj Natikar
  2026-09-02 13:44 ` [PATCH 1/4] HID: amd_sfh: Add DMI quirk and module param to disable interrupts Basavaraj Natikar
  2026-09-02 13:44 ` [PATCH 2/4] HID: amd_sfh: Add support for newer AMD SoCs Basavaraj Natikar
@ 2026-09-02 13:44 ` Basavaraj Natikar
  2026-09-02 14:02   ` sashiko-bot
  2026-09-02 13:44 ` [PATCH 4/4] HID: amd_sfh: Add device mode support for latest " Basavaraj Natikar
  3 siblings, 1 reply; 8+ messages in thread
From: Basavaraj Natikar @ 2026-09-02 13:44 UTC (permalink / raw)
  To: jikos, bentiss; +Cc: linux-input, Basavaraj Natikar, Guruvendra Punugupati

Use HPD data from DRAM instead of reading from the C2P4 register
path in latest AMD SOC.

Co-developed-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c      | 13 +++++++++++--
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 14 ++++++++++++--
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h |  6 ++++++
 3 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
index c8916afefa62..b81a97dbb1b0 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
@@ -187,6 +187,7 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
 	struct sfh_gyro_data gyro_data;
 	struct sfh_mag_data mag_data;
 	struct sfh_als_data als_data;
+	struct sfh_hpd_data hpd_data;
 	struct hpd_status hpdstatus;
 	struct sfh_base_info binfo;
 	void __iomem *sensoraddr;
@@ -251,8 +252,16 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
 		break;
 	case HPD_IDX:
 		get_common_inputs(&hpd_input.common_property, report_id);
-		hpdstatus.val = readl(mp2->mmio + amd_get_c2p_val(mp2, 4));
-		hpd_input.human_presence = hpdstatus.shpd.presence;
+		if (mp2->mp2_ver >= MP2_VER_1_2) {
+			sensoraddr = mp2->vsbase +
+				(HPD_IDX * SENSOR_DATA_MEM_SIZE_DEFAULT) +
+				OFFSET_SENSOR_DATA_DEFAULT;
+			memcpy_fromio(&hpd_data, sensoraddr, sizeof(struct sfh_hpd_data));
+			hpd_input.human_presence = hpd_data.status.shpd.presence;
+		} else {
+			hpdstatus.val = readl(mp2->mmio + amd_get_c2p_val(mp2, 4));
+			hpd_input.human_presence = hpdstatus.shpd.presence;
+		}
 		report_size = sizeof(hpd_input);
 		memcpy(input_report, &hpd_input, sizeof(hpd_input));
 		break;
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 60c185a9cd50..034ec237659a 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -160,7 +160,9 @@ static int amd_sfh_mode_info(u32 *platform_type, u32 *laptop_placement)
 
 static int amd_sfh_hpd_info(u8 *user_present)
 {
+	struct sfh_hpd_data hpd_data;
 	struct hpd_status hpdstatus;
+	void __iomem *sensoraddr;
 
 	if (!user_present)
 		return -EINVAL;
@@ -168,8 +170,16 @@ static int amd_sfh_hpd_info(u8 *user_present)
 	if (!emp2 || !emp2->dev_en.is_hpd_present || !emp2->dev_en.is_hpd_enabled)
 		return -ENODEV;
 
-	hpdstatus.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 4));
-	*user_present = hpdstatus.shpd.presence;
+	if (emp2->mp2_ver >= MP2_VER_1_2) {
+		sensoraddr = emp2->vsbase +
+			(HPD_IDX * SENSOR_DATA_MEM_SIZE_DEFAULT) +
+			OFFSET_SENSOR_DATA_DEFAULT;
+		memcpy_fromio(&hpd_data, sensoraddr, sizeof(struct sfh_hpd_data));
+		*user_present = hpd_data.status.shpd.presence;
+	} else {
+		hpdstatus.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 4));
+		*user_present = hpdstatus.shpd.presence;
+	}
 
 	return 0;
 }
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 56258c4d1b3a..11c7a86ac52e 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -165,6 +165,12 @@ struct hpd_status {
 	};
 };
 
+struct sfh_hpd_data {
+	struct sfh_common_data commondata;
+	u32 reserved;
+	struct hpd_status status;
+};
+
 struct sfh_op_mode {
 	union {
 		u32 val;
-- 
2.34.1


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

* [PATCH 4/4] HID: amd_sfh: Add device mode support for latest AMD SOC.
  2026-09-02 13:44 [PATCH 0/4] HID: amd_sfh: Add DMI quirk and support for new AMD SoCs Basavaraj Natikar
                   ` (2 preceding siblings ...)
  2026-09-02 13:44 ` [PATCH 3/4] HID: amd_sfh: Add HPD support for new AMD SOC Basavaraj Natikar
@ 2026-09-02 13:44 ` Basavaraj Natikar
  2026-09-02 13:59   ` sashiko-bot
  3 siblings, 1 reply; 8+ messages in thread
From: Basavaraj Natikar @ 2026-09-02 13:44 UTC (permalink / raw)
  To: jikos, bentiss; +Cc: linux-input, Basavaraj Natikar, Guruvendra Punugupati

Use device mode data from DRAM instead of reading from the C2P
register in latest AMD SOC.

Co-developed-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Guruvendra Punugupati <Guruvendra.Punugupati@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 23 +++++++++++++++++--
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    |  1 +
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 034ec237659a..bc0adb789896 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -98,6 +98,25 @@ void sfh_interface_init(struct amd_mp2_dev *mp2)
 	emp2 = mp2;
 }
 
+static unsigned int amd_sfh_read_c2p3(struct amd_mp2_dev *mp2)
+{
+	struct sfh_accel_data accel_data;
+	void __iomem *sensoraddr;
+	u32 val;
+
+	if (mp2->mp2_ver >= MP2_VER_1_2) {
+		sensoraddr = mp2->vsbase +
+			(SRA_IDX * SENSOR_DATA_MEM_SIZE_DEFAULT) +
+			OFFSET_SENSOR_DATA_DEFAULT;
+		memcpy_fromio(&accel_data, sensoraddr, sizeof(struct sfh_accel_data));
+		val = accel_data.c2p3_data;
+	} else {
+		val = readl(mp2->mmio + amd_get_c2p_val(mp2, 3));
+	}
+
+	return val;
+}
+
 static int amd_sfh_op_mode_info(u32 *op_mode)
 {
 	struct sfh_op_mode mode;
@@ -114,7 +133,7 @@ static int amd_sfh_op_mode_info(u32 *op_mode)
 	if (!present)
 		return -ENODEV;
 
-	mode.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 3));
+	mode.val = amd_sfh_read_c2p3(emp2);
 	dev_dbg(&emp2->pdev->dev, "op-mode: %s (mode=%u)\n",
 		mode.op_mode.mode == SFH_MODE_TABLET ? "tablet" : "laptop",
 		mode.op_mode.mode);
@@ -133,7 +152,7 @@ static int amd_sfh_mode_info(u32 *platform_type, u32 *laptop_placement)
 	if (!emp2 || !emp2->dev_en.is_sra_present)
 		return -ENODEV;
 
-	mode.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 3));
+	mode.val = amd_sfh_read_c2p3(emp2);
 
 	*platform_type = mode.op_mode.devicemode;
 
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 11c7a86ac52e..eaa90c689049 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -130,6 +130,7 @@ struct sfh_accel_data {
 	struct sfh_common_data commondata;
 	struct sfh_float32 acceldata;
 	u32 accelstatus;
+	u32 c2p3_data;
 };
 
 struct sfh_gyro_data {
-- 
2.34.1


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

* Re: [PATCH 1/4] HID: amd_sfh: Add DMI quirk and module param to disable interrupts
  2026-09-02 13:44 ` [PATCH 1/4] HID: amd_sfh: Add DMI quirk and module param to disable interrupts Basavaraj Natikar
@ 2026-09-02 13:54   ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-02 13:54 UTC (permalink / raw)
  To: Basavaraj Natikar; +Cc: dmitry.torokhov, linux-input

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The DMI quirk for ASUS VivoBook inadvertently ENABLES hardware interrupts, directly contradicting the commit message which states the device requires them DISABLED.
- [Medium] The DMI quirk unconditionally overwrites the module parameter during probe, making it impossible for a user to override the quirk on matching hardware.
- [Low] The module parameter permissions are read-only, preventing the 'runtime override' advertised in the commit message.
--

commit ec52ca66216e002a750c44dedd096bba34993086
Author: Helge Bahmann <hcb@chaoticmind.net>

HID: amd_sfh: Add DMI quirk and module param to disable interrupts

This commit exposes the intr_disable variable as a module parameter
and adds a DMI quirk for the ASUSTeK VivoBook.

> Expose intr_disable as a module parameter to allow runtime override.

[Severity: Low]
The commit message mentions allowing a runtime override, but the module
parameter is defined with 0444 permissions. Doesn't this make it read-only
and prevent runtime modification via sysfs?

> Add DMI quirk for ASUSTeK VivoBook TP420UA/TM420UA which requires
> interrupts disabled for correct sensor operation.
> 
> Signed-off-by: Helge Bahmann <hcb@chaoticmind.net>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>

> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> index eda26a094d3f1..92486f25460bc 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
> @@ -41,6 +41,8 @@ module_param_named(sensor_mask, sensor_mask_override, int, 0444);
>  MODULE_PARM_DESC(sensor_mask, "override the detected sensors mask");
>  
>  static bool intr_disable = true;
> +module_param_named(intr_disable, intr_disable, bool, 0444);

[Severity: Medium]
Does the DMI quirk evaluation in amd_mp2_pci_probe() unconditionally
overwrite this module parameter?

During probe, dmi_check_system(dmi_sfh_table) is called, which executes
mp2_disable_intr() for matching hardware. If a user attempts to override the
interrupt setting via module parameters, won't their setting be silently
ignored and clobbered by the quirk's callback?

> +MODULE_PARM_DESC(intr_disable, "override the interrupt disable sensor bit");
>  
>  static int amd_sfh_wait_response_v2(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts)

[ ... ]

> @@ -313,6 +315,13 @@ static const struct dmi_system_id dmi_sfh_table[] = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "HP ProBook x360 435 G7"),
>  		},
>  	},
> +	{
> +		.callback = mp2_disable_intr,

[Severity: High]
Does this quirk actually enable interrupts rather than disable them?

The commit message states the ASUS VivoBook requires interrupts disabled.
However, the mp2_disable_intr() callback sets the global intr_disable = false.

By setting this to false, the hardware is configured with
cmd_v2.intr_disable = 0, which turns interrupts on. Is this the intended
behavior for a device that requires them to be disabled?

> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "VivoBook_ASUSLaptop TP420UA_TM420UA"),
> +		},
> +	},
>  	{}
>  };
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902134431.3146117-1-Basavaraj.Natikar@amd.com?part=1

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

* Re: [PATCH 4/4] HID: amd_sfh: Add device mode support for latest AMD SOC.
  2026-09-02 13:44 ` [PATCH 4/4] HID: amd_sfh: Add device mode support for latest " Basavaraj Natikar
@ 2026-09-02 13:59   ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-02 13:59 UTC (permalink / raw)
  To: Basavaraj Natikar; +Cc: dmitry.torokhov, linux-input

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Stack memory disclosure in `get_input_rep` (and `get_feature_rep`) due to uninitialized struct fields.
--

commit ee34af1b468d9118272d016d00f5ca0dd24501d8
Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>

HID: amd_sfh: Add device mode support for latest AMD SOC.

Use device mode data from DRAM instead of reading from the C2P
register in latest AMD SOC.

> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> index d2021f5cc6e59..711174184d2d9 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
[ ... ]
> @@ -132,7 +151,7 @@ static int amd_sfh_mode_info(u32 *platform_type, u32 *laptop_placement)
>  	if (!emp2 || !emp2->dev_en.is_sra_present)
>  		return -ENODEV;
>  
> -	mode.val = readl(emp2->mmio + amd_get_c2p_val(emp2, 3));
> +	mode.val = amd_sfh_read_c2p3(emp2);
>  
>  	*platform_type = mode.op_mode.devicemode;
>  

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing
stack memory disclosure in this subsystem when constructing HID input
reports.

In drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c:get_input_rep():

	struct accel3_input_report acc_input;
	...
	case ACCEL_IDX: /* accelerometer */
		...
		get_common_inputs(&acc_input.common_property, report_id);
		acc_input.in_accel_x_value = amd_sfh_float_to_int(...) / 100;
		acc_input.in_accel_y_value = amd_sfh_float_to_int(...) / 100;
		acc_input.in_accel_z_value = amd_sfh_float_to_int(...) / 100;
		memcpy(input_report, &acc_input, sizeof(acc_input));

Since acc_input is allocated on the stack without being zero-initialized,
could uninitialized fields (like in_accel_shake_detection or compiler
padding bytes) retain old kernel stack data and be leaked to userspace
when the entire struct size is copied into the report buffer?

The same pattern appears in get_input_rep() for als_input, where some
fields are only conditionally initialized:

	struct als_input_report als_input;
	...
	case ALS_IDX:
		...
		als_input.illuminance_value = amd_sfh_float_to_int(als_data.lux);

		memcpy_fromio(&binfo, mp2->vsbase, sizeof(struct sfh_base_info));
		if (binfo.sbase.s_prop[ALS_IDX].sf.feat & 0x2) {
			als_input.light_color_temp = als_data.light_color_temp;
			...
		}

		report_size = sizeof(als_input);
		memcpy(input_report, &als_input, sizeof(als_input));

If the if-condition is false, are the uninitialized light_color_temp and
chromaticity fields leaking kernel memory into the HID report?

Does this same issue also exist in get_feature_rep() where structs like
acc_feature and als_feature are allocated on the stack and copied
directly into feature_report?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902134431.3146117-1-Basavaraj.Natikar@amd.com?part=4

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

* Re: [PATCH 3/4] HID: amd_sfh: Add HPD support for new AMD SOC
  2026-09-02 13:44 ` [PATCH 3/4] HID: amd_sfh: Add HPD support for new AMD SOC Basavaraj Natikar
@ 2026-09-02 14:02   ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-02 14:02 UTC (permalink / raw)
  To: Basavaraj Natikar; +Cc: linux-input, dmitry.torokhov

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Kernel stack memory is leaked to userspace due to uninitialized stack variables being copied to the HID input report buffer.
--

commit fbf2d0c98746dffa45a715c975a05e57c0b3b3fb
Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>

HID: amd_sfh: Add HPD support for new AMD SOC

This commit adds Human Presence Detection (HPD) support for new AMD SOCs.
It updates the driver to use HPD data from DRAM instead of reading from the
C2P4 register path on newer hardware versions.

> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> index c8916afefa626..b81a97dbb1b01 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> @@ -187,6 +187,7 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
>  	struct sfh_gyro_data gyro_data;
>  	struct sfh_mag_data mag_data;
>  	struct sfh_als_data als_data;
> +	struct sfh_hpd_data hpd_data;
>  	struct hpd_status hpdstatus;
>  	struct sfh_base_info binfo;
>  	void __iomem *sensoraddr;

[Severity: High]
This is a pre-existing issue, but do these struct definitions leave
uninitialized stack memory that gets leaked to userspace?

In get_input_rep(), structures like acc_input and als_input are allocated on
the kernel stack without explicit initialization.

For ACCEL_IDX, the in_accel_shake_detection field in acc_input appears to
never be initialized before copying it to the report buffer:

	case ACCEL_IDX:
		...
		acc_input.in_accel_z_value = amd_sfh_float_to_int(accel_data.acceldata.z) / 100;
		memcpy(input_report, &acc_input, sizeof(acc_input));

Similarly for ALS_IDX, light_color_temp, chromaticity_x_value, and
chromaticity_y_value are only initialized conditionally, which can leave
those bytes uninitialized before being copied:

	case ALS_IDX:
		...
		if (binfo.sbase.s_prop[ALS_IDX].sf.feat & 0x2) {
			als_input.light_color_temp = als_data.light_color_temp;
			als_input.chromaticity_x_value =
				amd_sfh_float_to_int(als_data.chromaticity_x);
			als_input.chromaticity_y_value =
				amd_sfh_float_to_int(als_data.chromaticity_y);
		}

		report_size = sizeof(als_input);
		memcpy(input_report, &als_input, sizeof(als_input));

Since input_report is subsequently sent to userspace by the HID core, does
this expose uninitialized kernel stack memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902134431.3146117-1-Basavaraj.Natikar@amd.com?part=3

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

end of thread, other threads:[~2026-09-02 14:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 13:44 [PATCH 0/4] HID: amd_sfh: Add DMI quirk and support for new AMD SoCs Basavaraj Natikar
2026-09-02 13:44 ` [PATCH 1/4] HID: amd_sfh: Add DMI quirk and module param to disable interrupts Basavaraj Natikar
2026-09-02 13:54   ` sashiko-bot
2026-09-02 13:44 ` [PATCH 2/4] HID: amd_sfh: Add support for newer AMD SoCs Basavaraj Natikar
2026-09-02 13:44 ` [PATCH 3/4] HID: amd_sfh: Add HPD support for new AMD SOC Basavaraj Natikar
2026-09-02 14:02   ` sashiko-bot
2026-09-02 13:44 ` [PATCH 4/4] HID: amd_sfh: Add device mode support for latest " Basavaraj Natikar
2026-09-02 13:59   ` sashiko-bot

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