[PATCH iwl-next v4] igc: Support ACPI-based MAC passthrough

[PATCH iwl-next v4] igc: Support ACPI-based MAC passthrough

[Dima Ruinskiy]

Some systems implement a system MAC address object in the ACPI table,
using either \\_SB.AMAC or \\MACA object names. This system MAC address,
when enabled, is intended to override the permanent MAC address of the
any network controller attached via a docking station. The purpose is to
allow the host PC retain its network identity in a corporate environment,
regardless of the specific docking station it is connected to. The
expectation is that only one docking station is attached at a time, or
that only a single network controller is plugged in at a time.

Implement seamless passthrough of the system MAC address to the
I225/I226 network controller, by looking up the relevant ACPI object names
and using them to initialize the MAC address. Limit the lookup only to
controllers that are Thunderbolt-attached, as these are the only docking
stations that can pass through an I225/I226 controller to the host.

On systems where the feature is disabled or unsupported, the ACPI objects
do not exist or do not contain a valid Ethernet MAC, the default MAC
address of the device is used.

Assisted-by: GitHub-Copilot:claude-opus-4.7
Signed-off-by: Dima Ruinskiy <dima.ruinskiy@intel.com>
---
Change log
v3->v4:
	use pci_is_thunderbolt_attached instead of device id check
	explicitly save permanent address when using passthrough mac
v2->v3:
	fix typo that introduced check reversal
v1->v2:
	limit scope to dock device ids
---
 drivers/net/ethernet/intel/igc/igc_main.c | 70 +++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index f6d1ee75627d..d51e9bcdfc3e 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c)  2018 Intel Corporation */
 
+#include <linux/acpi.h>
+#include <linux/hex.h>
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/if_vlan.h>
@@ -7134,6 +7136,58 @@ static enum hrtimer_restart igc_qbv_scheduling_timer(struct hrtimer *timer)
 	return HRTIMER_NORESTART;
 }
 
+static bool igc_get_acpi_mac_passthru(u8 *mac)
+{
+	static const struct {
+		const char *name;
+		acpi_object_type type;
+		u32 length;
+	} sources[] = {
+		{ "\\_SB.AMAC", ACPI_TYPE_BUFFER, 23 },
+		{ "\\MACA",     ACPI_TYPE_STRING, 22 },
+	};
+	struct acpi_buffer buffer;
+	union acpi_object *obj;
+	bool mac_found = false;
+	acpi_status status;
+	u8 buf[ETH_ALEN];
+	int i;
+
+	if (!IS_ENABLED(CONFIG_ACPI))
+		return false;
+
+	for (i = 0; i < ARRAY_SIZE(sources) && !mac_found; i++) {
+		buffer.length = ACPI_ALLOCATE_BUFFER;
+		buffer.pointer = NULL;
+
+		status = acpi_evaluate_object(NULL, (char *)sources[i].name,
+					      NULL, &buffer);
+		if (ACPI_FAILURE(status))
+			continue;
+
+		obj = buffer.pointer;
+		if (!obj || obj->type != sources[i].type ||
+		    obj->string.length != sources[i].length ||
+		    !obj->string.pointer)
+			goto free_obj;
+
+		if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) ||
+		    obj->string.pointer[21] != '#')
+			goto free_obj;
+
+		if (hex2bin(buf, obj->string.pointer + 9, ETH_ALEN) ||
+		    !is_valid_ether_addr(buf))
+			goto free_obj;
+
+		ether_addr_copy(mac, buf);
+		mac_found = true;
+free_obj:
+		ACPI_FREE(obj);
+	}
+
+	return mac_found;
+}
+
 /**
  * igc_probe - Device Initialization Routine
  * @pdev: PCI device information struct
@@ -7300,6 +7354,22 @@ static int igc_probe(struct pci_dev *pdev,
 		/* copy the MAC address out of the NVM */
 		if (hw->mac.ops.read_mac_addr(hw))
 			dev_err(&pdev->dev, "NVM Read Error\n");
+
+		/* For devices behind a Thunderbolt/USB4 dock, look for a
+		 * system-provided MAC in the ACPI table
+		 */
+		if (pci_is_thunderbolt_attached(pdev) &&
+		    igc_get_acpi_mac_passthru(hw->mac.addr)) {
+			/* Set the permanent MAC address. Leave blank if invalid to
+			 * reflect there is something wrong with the NVM MAC.
+			 */
+			if (is_valid_ether_addr(hw->mac.perm_addr))
+				ether_addr_copy(netdev->perm_addr, hw->mac.perm_addr);
+			netdev->addr_assign_type = NET_ADDR_STOLEN;
+			dev_info(&pdev->dev,
+				 "Using ACPI pass-thru MAC addr %pM\n",
+				 hw->mac.addr);
+		}
 	}
 
 	eth_hw_addr_set(netdev, hw->mac.addr);
-- 
2.55.0

Re: [PATCH iwl-next v4] igc: Support ACPI-based MAC passthrough

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/#/patchset/20260910193824.3257378-1-dima.ruinskiy@intel.com?part=1
---
igc: Support ACPI-based MAC passthrough

This commit implements support for seamlessly passing through a system MAC
address provided in the ACPI table to an I225/I226 network controller
attached via a Thunderbolt dock.

> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 1fb5f3cbe93c5..abffd385f3e8c 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
[ ... ]
> @@ -7106,6 +7108,58 @@ static enum hrtimer_restart igc_qbv_scheduling_timer(struct hrtimer *timer)
>  	return HRTIMER_NORESTART;
>  }
>  
> +static bool igc_get_acpi_mac_passthru(u8 *mac)
> +{
[ ... ]
> +	acpi_status status;
> +	u8 buf[ETH_ALEN];

[Severity: High]
Does this array lack a 16-bit alignment guarantee? 

Because it is declared as a simple byte array on the stack, the compiler is
free to place it at an odd memory address.

> +	int i;
> +
> +	if (!IS_ENABLED(CONFIG_ACPI))
> +		return false;
> +
> +	for (i = 0; i < ARRAY_SIZE(sources) && !mac_found; i++) {
[ ... ]
> +		if (hex2bin(buf, obj->string.pointer + 9, ETH_ALEN) ||
> +		    !is_valid_ether_addr(buf))
> +			goto free_obj;
> +
> +		ether_addr_copy(mac, buf);

[Severity: High]
Can passing this buffer to is_valid_ether_addr() and ether_addr_copy() cause
an alignment trap on architectures that strictly enforce memory alignment?

Both of these helpers cast the pointer to a u16 pointer to perform 16-bit
memory operations when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is not set. 
If buf ends up placed at an odd address, this could cause a kernel panic.

Could this be avoided by defining buf as a u16 array or by adding an explicit
__aligned(2) attribute?

> +		mac_found = true;
> +free_obj:
> +		ACPI_FREE(obj);
> +	}
> +
> +	return mac_found;
> +}
> +