* [PATCH 0/9] Introducing firmware late binding
@ 2025-07-10 15:08 Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 1/9] mei: bus: add mei_cldev_mtu interface Rodrigo Vivi
` (8 more replies)
0 siblings, 9 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2025-07-10 15:08 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: gregkh, daniele.ceraolospurio, anshuman.gupta, alexander.usyskin,
Rodrigo Vivi
Introducing firmware late binding feature to enable firmware loading
for the devices, such as the fan controller and voltage regulator,
during the driver probe.
Typically, firmware for these devices are part of IFWI flash image but
can be replaced at probe after OEM tuning.
This version (v8?): I'm covering for Badal's vacation since I'd like
to take this as soon as possible to drm-xe-next.
The changes in this version are mostly to address the valid Greg's
concerns on the mei patches. In summary:
- Proper commit messages
- 'Late Binding' documentation and explanation
- Proper code documentation, fixing word and style
- Bring consistency to the prefixes and naming
Previous revision history from Badal:
https://lore.kernel.org/intel-xe/20250707191237.1782824-1-badal.nilawar@intel.com
Thanks,
Rodrigo.
Alexander Usyskin (2):
mei: bus: add mei_cldev_mtu interface
mei: late_bind: add late binding component driver
Badal Nilawar (7):
drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw
drm/xe/xe_late_bind_fw: Initialize late binding firmware
drm/xe/xe_late_bind_fw: Load late binding firmware
drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume
drm/xe/xe_late_bind_fw: Reload late binding fw during system resume
drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late
binding
drm/xe/xe_late_bind_fw: Extract and print version info
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_debugfs.c | 41 ++
drivers/gpu/drm/xe/xe_device.c | 5 +
drivers/gpu/drm/xe/xe_device_types.h | 6 +
drivers/gpu/drm/xe/xe_late_bind_fw.c | 464 +++++++++++++++++++++
drivers/gpu/drm/xe/xe_late_bind_fw.h | 17 +
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 75 ++++
drivers/gpu/drm/xe/xe_pci.c | 2 +
drivers/gpu/drm/xe/xe_pci_types.h | 1 +
drivers/gpu/drm/xe/xe_pm.c | 8 +
drivers/gpu/drm/xe/xe_uc_fw_abi.h | 66 +++
drivers/misc/mei/Kconfig | 13 +
drivers/misc/mei/Makefile | 1 +
drivers/misc/mei/bus.c | 13 +
drivers/misc/mei/mei_lb.c | 315 ++++++++++++++
include/drm/intel/i915_component.h | 1 +
include/drm/intel/intel_lb_mei_interface.h | 70 ++++
include/linux/mei_cl_bus.h | 1 +
18 files changed, 1100 insertions(+)
create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.c
create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.h
create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw_types.h
create mode 100644 drivers/misc/mei/mei_lb.c
create mode 100644 include/drm/intel/intel_lb_mei_interface.h
--
2.49.0
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/9] mei: bus: add mei_cldev_mtu interface
2025-07-10 15:08 [PATCH 0/9] Introducing firmware late binding Rodrigo Vivi
@ 2025-07-10 15:08 ` Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 2/9] mei: late_bind: add late binding component driver Rodrigo Vivi
` (7 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2025-07-10 15:08 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: gregkh, daniele.ceraolospurio, anshuman.gupta, alexander.usyskin,
Badal Nilawar, Umesh Nerlige Ramappa, Rodrigo Vivi
From: Alexander Usyskin <alexander.usyskin@intel.com>
Add a new helper function that allows MEI client drivers
to query the maximum transmission unit (MTU) for a connected
MEI client.
This is useful for clients that need to transmit large payloads,
such as firmware blobs, allowing them to determine the maximum
message size that can be safely sent before starting transmission and
size of the buffer to allocate when receiving data.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
Change in this revision: Proper commit message.
drivers/misc/mei/bus.c | 13 +++++++++++++
include/linux/mei_cl_bus.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 67176caf5416..f860b1b6eda0 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -614,6 +614,19 @@ u8 mei_cldev_ver(const struct mei_cl_device *cldev)
}
EXPORT_SYMBOL_GPL(mei_cldev_ver);
+/**
+ * mei_cldev_mtu - max message that client can send and receive
+ *
+ * @cldev: mei client device
+ *
+ * Return: mtu or 0 if client is not connected
+ */
+size_t mei_cldev_mtu(const struct mei_cl_device *cldev)
+{
+ return mei_cl_mtu(cldev->cl);
+}
+EXPORT_SYMBOL_GPL(mei_cldev_mtu);
+
/**
* mei_cldev_enabled - check whether the device is enabled
*
diff --git a/include/linux/mei_cl_bus.h b/include/linux/mei_cl_bus.h
index 725fd7727422..a82755e1fc40 100644
--- a/include/linux/mei_cl_bus.h
+++ b/include/linux/mei_cl_bus.h
@@ -113,6 +113,7 @@ int mei_cldev_register_notif_cb(struct mei_cl_device *cldev,
mei_cldev_cb_t notif_cb);
u8 mei_cldev_ver(const struct mei_cl_device *cldev);
+size_t mei_cldev_mtu(const struct mei_cl_device *cldev);
void *mei_cldev_get_drvdata(const struct mei_cl_device *cldev);
void mei_cldev_set_drvdata(struct mei_cl_device *cldev, void *data);
--
2.49.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/9] mei: late_bind: add late binding component driver
2025-07-10 15:08 [PATCH 0/9] Introducing firmware late binding Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 1/9] mei: bus: add mei_cldev_mtu interface Rodrigo Vivi
@ 2025-07-10 15:08 ` Rodrigo Vivi
2025-07-16 11:48 ` Greg KH
2025-07-10 15:08 ` [PATCH 3/9] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw Rodrigo Vivi
` (6 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Rodrigo Vivi @ 2025-07-10 15:08 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: gregkh, daniele.ceraolospurio, anshuman.gupta, alexander.usyskin,
Badal Nilawar, Rodrigo Vivi
From: Alexander Usyskin <alexander.usyskin@intel.com>
Introduce a new MEI client driver to support Late Binding firmware
upload/update for Intel discrete graphics platforms.
Late Binding is a runtime firmware upload/update mechanism that allows
payloads, such as fan control and voltage regulator, to be securely
delivered and applied without requiring SPI flash updates or
system reboots. This driver enables the Xe graphics driver and other
user-space tools to push such firmware blobs to the authentication
firmware via the MEI interface.
The driver handles authentication, versioning, and communication
with the authentication firmware, which in turn coordinates with
the PUnit/PCODE to apply the payload.
This is a foundational component for enabling dynamic, secure,
and re-entrant configuration updates on platforms like Battlemage.
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
Changes in this revision:
- Proper commit message
- Proper explanation of 'Late Binding' on Kconfig help and doc
- Consistency in naming:
+ mei_ prefix where it makes sense
+ use 'lb' for short of 'Late Binding' instead of 'late_bind'
Including s/CONFIG_INTEL_MEI_LATE_BIND/CONFIG_INTEL_MEI_LB
+ remove stray 'struct module'
+ Fix structs and enum documentation style and fields
+ Remove 'CSC' to avoid yet another acronym. 'Authentication firmware' it is.
+ specify size unit
+ s/push_config/push_payload
drivers/misc/mei/Kconfig | 13 +
drivers/misc/mei/Makefile | 1 +
drivers/misc/mei/mei_lb.c | 315 +++++++++++++++++++++
include/drm/intel/i915_component.h | 1 +
include/drm/intel/intel_lb_mei_interface.h | 70 +++++
5 files changed, 400 insertions(+)
create mode 100644 drivers/misc/mei/mei_lb.c
create mode 100644 include/drm/intel/intel_lb_mei_interface.h
diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
index 7575fee96cc6..f8b04e49e4ba 100644
--- a/drivers/misc/mei/Kconfig
+++ b/drivers/misc/mei/Kconfig
@@ -81,6 +81,19 @@ config INTEL_MEI_VSC
This driver can also be built as a module. If so, the module
will be called mei-vsc.
+config INTEL_MEI_LB
+ tristate "Intel Late Binding (LB) support on ME Interface"
+ depends on INTEL_MEI_ME
+ depends on DRM_XE
+ help
+ Enable support for Intel Late Binding (LB) via the MEI interface.
+
+ Late Binding is a method for applying firmware updates at runtime,
+ allowing the Intel Xe driver to load firmware payloads such as
+ fan controller or voltage regulator. These firmware updates are
+ authenticated and versioned, and do not require firmware flashing
+ or system reboot.
+
source "drivers/misc/mei/hdcp/Kconfig"
source "drivers/misc/mei/pxp/Kconfig"
source "drivers/misc/mei/gsc_proxy/Kconfig"
diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
index 6f9fdbf1a495..a203ed766b33 100644
--- a/drivers/misc/mei/Makefile
+++ b/drivers/misc/mei/Makefile
@@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src)
obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/
obj-$(CONFIG_INTEL_MEI_PXP) += pxp/
obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/
+obj-$(CONFIG_INTEL_MEI_LB) += mei_lb.o
obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o
mei-vsc-hw-y := vsc-tp.o
diff --git a/drivers/misc/mei/mei_lb.c b/drivers/misc/mei/mei_lb.c
new file mode 100644
index 000000000000..fddef862712d
--- /dev/null
+++ b/drivers/misc/mei/mei_lb.c
@@ -0,0 +1,315 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2025 Intel Corporation
+ */
+#include <drm/intel/i915_component.h>
+#include <drm/intel/intel_lb_mei_interface.h>
+#include <linux/component.h>
+#include <linux/pci.h>
+#include <linux/mei_cl_bus.h>
+#include <linux/module.h>
+#include <linux/overflow.h>
+#include <linux/slab.h>
+#include <linux/uuid.h>
+
+#include "mkhi.h"
+
+/**
+ * DOC: Late Binding Firmware Update/Upload
+ *
+ * Late Binding is a firmware update/upload mechanism that allows configuration
+ * payloads to be securely delivered and applied at runtime, rather than
+ * being embedded in the system firmware image (e.g., IFWI or SPI flash).
+ *
+ * This mechanism is used to update device-level configuration such as:
+ * - Fan controller
+ * - Voltage regulator (VR)
+ *
+ * Key Characteristics:
+ * ---------------------
+ * - Runtime Delivery:
+ * Firmware blobs are loaded by the host driver (e.g., Xe KMD)
+ * after the GPU or SoC has booted.
+ *
+ * - Secure and Authenticated:
+ * All payloads are signed and verified by the authentication firmware.
+ *
+ * - No Firmware Flashing Required:
+ * Updates are applied in volatile memory and do not require SPI flash
+ * modification or system reboot.
+ *
+ * - Re-entrant:
+ * Multiple updates of the same or different types can be applied
+ * sequentially within a single boot session.
+ *
+ * - Version Controlled:
+ * Each payload includes version and security version number (SVN)
+ * metadata to support anti-rollback enforcement.
+ *
+ * Upload Flow:
+ * ------------
+ * 1. Host driver (KMD or user-space tool) loads the late binding firmware.
+ * 2. Firmware is passed to the MEI interface and forwarded to
+ * authentication firmware.
+ * 3. Authentication firmware authenticates the payload and extracts
+ * command and data arrays.
+ * 4. Authentication firmware delivers the configuration to PUnit/PCODE.
+ * 5. Status is returned back to the host via MEI.
+ */
+
+#define INTEL_LB_CMD 0x12
+#define INTEL_LB_RSP (INTEL_LB_CMD | 0x80)
+
+#define INTEL_LB_SEND_TIMEOUT_MSEC 3000
+#define INTEL_LB_RECV_TIMEOUT_MSEC 3000
+
+/**
+ * struct mei_lb_req - Late Binding request structure
+ * @header: MKHI message header (see struct mkhi_msg_hdr)
+ * @type: Type of the Late Binding payload
+ * @flags: Flags to be passed to the authentication firmware (e.g. %INTEL_LB_FLAGS_IS_PERSISTENT)
+ * @reserved: Reserved for future use by authentication firmware, must be set to 0
+ * @payload_size: Size of the payload data in bytes
+ * @payload: Payload data to be sent to the authentication firmware
+ */
+struct mei_lb_req {
+ struct mkhi_msg_hdr header;
+ __le32 type;
+ __le32 flags;
+ __le32 reserved[2];
+ __le32 payload_size;
+ u8 payload[] __counted_by(payload_size);
+} __packed;
+
+/**
+ * struct mei_lb_rsp - Late Binding response structure
+ * @header: MKHI message header (see struct mkhi_msg_hdr)
+ * @type: Type of the Late Binding payload
+ * @reserved: Reserved for future use by authentication firmware, must be set to 0
+ * @status: Status returned by authentication firmware (see enum intel_lb_status)
+ */
+struct mei_lb_rsp {
+ struct mkhi_msg_hdr header;
+ __le32 type;
+ __le32 reserved[2];
+ __le32 status;
+} __packed;
+
+static int mei_lb_check_response(const struct device *dev, const struct mkhi_msg_hdr *hdr)
+{
+ if (hdr->group_id != MKHI_GROUP_ID_GFX) {
+ dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n",
+ hdr->group_id, MKHI_GROUP_ID_GFX);
+ return -EINVAL;
+ }
+
+ if (hdr->command != INTEL_LB_RSP) {
+ dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n",
+ hdr->command, INTEL_LB_RSP);
+ return -EINVAL;
+ }
+
+ if (hdr->result) {
+ dev_err(dev, "Error in result: 0x%x\n", hdr->result);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int mei_lb_push_payload(struct device *dev,
+ enum intel_lb_type type, u32 flags,
+ const void *payload, size_t payload_size)
+{
+ struct mei_cl_device *cldev;
+ struct mei_lb_req *req = NULL;
+ struct mei_lb_rsp rsp;
+ size_t req_size;
+ ssize_t bytes;
+ int ret;
+
+ cldev = to_mei_cl_device(dev);
+
+ ret = mei_cldev_enable(cldev);
+ if (ret) {
+ dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret);
+ return ret;
+ }
+
+ req_size = struct_size(req, payload, payload_size);
+ if (req_size > mei_cldev_mtu(cldev)) {
+ dev_err(dev, "Payload is too big %zu\n", payload_size);
+ ret = -EMSGSIZE;
+ goto end;
+ }
+
+ req = kmalloc(req_size, GFP_KERNEL);
+ if (!req) {
+ ret = -ENOMEM;
+ goto end;
+ }
+
+ req->header.group_id = MKHI_GROUP_ID_GFX;
+ req->header.command = INTEL_LB_CMD;
+ req->type = cpu_to_le32(type);
+ req->flags = cpu_to_le32(flags);
+ req->reserved[0] = 0;
+ req->reserved[1] = 0;
+ req->payload_size = cpu_to_le32(payload_size);
+ memcpy(req->payload, payload, payload_size);
+
+ bytes = mei_cldev_send_timeout(cldev,
+ (void *)req, req_size, INTEL_LB_SEND_TIMEOUT_MSEC);
+ if (bytes < 0) {
+ dev_err(dev, "mei_cldev_send failed. %zd\n", bytes);
+ ret = bytes;
+ goto end;
+ }
+
+ bytes = mei_cldev_recv_timeout(cldev,
+ (void *)&rsp, sizeof(rsp), INTEL_LB_RECV_TIMEOUT_MSEC);
+ if (bytes < 0) {
+ dev_err(dev, "mei_cldev_recv failed. %zd\n", bytes);
+ ret = bytes;
+ goto end;
+ }
+ if (bytes < sizeof(rsp.header)) {
+ dev_err(dev, "bad response header from the firmware: size %zd < %zu\n",
+ bytes, sizeof(rsp.header));
+ ret = -EPROTO;
+ goto end;
+ }
+ if (mei_lb_check_response(dev, &rsp.header)) {
+ dev_err(dev, "bad result response from the firmware: 0x%x\n",
+ *(uint32_t *)&rsp.header);
+ ret = -EPROTO;
+ goto end;
+ }
+ if (bytes < sizeof(rsp)) {
+ dev_err(dev, "bad response from the firmware: size %zd < %zu\n",
+ bytes, sizeof(rsp));
+ ret = -EPROTO;
+ goto end;
+ }
+
+ dev_dbg(dev, "status = %u\n", le32_to_cpu(rsp.status));
+ ret = (int)le32_to_cpu(rsp.status);
+end:
+ mei_cldev_disable(cldev);
+ kfree(req);
+ return ret;
+}
+
+static const struct intel_lb_component_ops mei_lb_ops = {
+ .push_payload = mei_lb_push_payload,
+};
+
+static int mei_lb_component_master_bind(struct device *dev)
+{
+ return component_bind_all(dev, (void *)&mei_lb_ops);
+}
+
+static void mei_lb_component_master_unbind(struct device *dev)
+{
+ component_unbind_all(dev, (void *)&mei_lb_ops);
+}
+
+static const struct component_master_ops mei_component_master_ops = {
+ .bind = mei_lb_component_master_bind,
+ .unbind = mei_lb_component_master_unbind,
+};
+
+/**
+ * mei_lb_component_match - compare function for matching mei late bind.
+ *
+ * This function checks if requester is Intel PCI_CLASS_DISPLAY_VGA or
+ * PCI_CLASS_DISPLAY_OTHER device, and checks if the requester is the
+ * grand parent of mei_if i.e. lb mei device
+ *
+ * @dev: master device
+ * @subcomponent: subcomponent to match (INTEL_COMPONENT_LB)
+ * @data: compare data (lb mei device on mei bus)
+ *
+ * Return:
+ * * 1 - if components match
+ * * 0 - otherwise
+ */
+static int mei_lb_component_match(struct device *dev, int subcomponent,
+ void *data)
+{
+ struct device *base = data;
+ struct pci_dev *pdev;
+
+ if (!dev)
+ return 0;
+
+ if (!dev_is_pci(dev))
+ return 0;
+
+ pdev = to_pci_dev(dev);
+
+ if (pdev->vendor != PCI_VENDOR_ID_INTEL)
+ return 0;
+
+ if (pdev->class != (PCI_CLASS_DISPLAY_VGA << 8) &&
+ pdev->class != (PCI_CLASS_DISPLAY_OTHER << 8))
+ return 0;
+
+ if (subcomponent != INTEL_COMPONENT_LB)
+ return 0;
+
+ base = base->parent;
+ if (!base) /* mei device */
+ return 0;
+
+ base = base->parent; /* pci device */
+
+ return !!base && dev == base;
+}
+
+static int mei_lb_probe(struct mei_cl_device *cldev,
+ const struct mei_cl_device_id *id)
+{
+ struct component_match *master_match = NULL;
+ int ret;
+
+ component_match_add_typed(&cldev->dev, &master_match,
+ mei_lb_component_match, &cldev->dev);
+ if (IS_ERR_OR_NULL(master_match))
+ return -ENOMEM;
+
+ ret = component_master_add_with_match(&cldev->dev,
+ &mei_component_master_ops,
+ master_match);
+ if (ret < 0)
+ dev_err(&cldev->dev, "Master comp add failed %d\n", ret);
+
+ return ret;
+}
+
+static void mei_lb_remove(struct mei_cl_device *cldev)
+{
+ component_master_del(&cldev->dev, &mei_component_master_ops);
+}
+
+#define MEI_GUID_MKHI UUID_LE(0xe2c2afa2, 0x3817, 0x4d19, \
+ 0x9d, 0x95, 0x6, 0xb1, 0x6b, 0x58, 0x8a, 0x5d)
+
+static struct mei_cl_device_id mei_lb_tbl[] = {
+ { .uuid = MEI_GUID_MKHI, .version = MEI_CL_VERSION_ANY },
+ { }
+};
+MODULE_DEVICE_TABLE(mei, mei_lb_tbl);
+
+static struct mei_cl_driver mei_lb_driver = {
+ .id_table = mei_lb_tbl,
+ .name = KBUILD_MODNAME,
+ .probe = mei_lb_probe,
+ .remove = mei_lb_remove,
+};
+
+module_mei_cl_driver(mei_lb_driver);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("MEI LB");
diff --git a/include/drm/intel/i915_component.h b/include/drm/intel/i915_component.h
index 4ea3b17aa143..8082db222e00 100644
--- a/include/drm/intel/i915_component.h
+++ b/include/drm/intel/i915_component.h
@@ -31,6 +31,7 @@ enum i915_component_type {
I915_COMPONENT_HDCP,
I915_COMPONENT_PXP,
I915_COMPONENT_GSC_PROXY,
+ INTEL_COMPONENT_LB,
};
/* MAX_PORT is the number of port
diff --git a/include/drm/intel/intel_lb_mei_interface.h b/include/drm/intel/intel_lb_mei_interface.h
new file mode 100644
index 000000000000..602822706eb0
--- /dev/null
+++ b/include/drm/intel/intel_lb_mei_interface.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright (c) 2025 Intel Corporation
+ */
+
+#ifndef _INTEL_LB_MEI_INTERFACE_H_
+#define _INTEL_LB_MEI_INTERFACE_H_
+
+#include <linux/types.h>
+
+struct device;
+
+/**
+ * INTEL_LB_FLAGS_IS_PERSISTENT - Mark the payload as persistent
+ *
+ * This flag indicates that the Late Binding payload should be stored
+ * persistently in flash across warm resets.
+ */
+#define INTEL_LB_FLAGS_IS_PERSISTENT BIT(0)
+
+/**
+ * intel_lb_type - enum to determine late binding payload type
+ */
+enum intel_lb_type {
+ INTEL_LB_TYPE_FAN_CONTROL = 1,
+};
+
+/**
+ * enum intel_lb_status - Status codes returned on Late Binding transmissions
+ * @INTEL_LB_STATUS_SUCCESS: Operation completed successfully
+ * @INTEL_LB_STATUS_4ID_MISMATCH: Mismatch in the expected 4ID (firmware identity/token)
+ * @INTEL_LB_STATUS_ARB_FAILURE: Arbitration failure (e.g. conflicting access or state)
+ * @INTEL_LB_STATUS_GENERAL_ERROR: General firmware error not covered by other codes
+ * @INTEL_LB_STATUS_INVALID_PARAMS: One or more input parameters are invalid
+ * @INTEL_LB_STATUS_INVALID_SIGNATURE: Payload has an invalid or untrusted signature
+ * @INTEL_LB_STATUS_INVALID_PAYLOAD: Payload contents are not accepted by firmware
+ * @INTEL_LB_STATUS_TIMEOUT: Operation timed out before completion
+ */
+enum intel_lb_status {
+ INTEL_LB_STATUS_SUCCESS = 0,
+ INTEL_LB_STATUS_4ID_MISMATCH = 1,
+ INTEL_LB_STATUS_ARB_FAILURE = 2,
+ INTEL_LB_STATUS_GENERAL_ERROR = 3,
+ INTEL_LB_STATUS_INVALID_PARAMS = 4,
+ INTEL_LB_STATUS_INVALID_SIGNATURE = 5,
+ INTEL_LB_STATUS_INVALID_PAYLOAD = 6,
+ INTEL_LB_STATUS_TIMEOUT = 7,
+};
+
+/**
+ * struct intel_lb_component_ops - Ops for Late Binding services.
+ * @push_payload: Callback to send a Late Binding payload
+ */
+struct intel_lb_component_ops {
+ /**
+ * push_payload - Sends a payload to the authentication firmware
+ * @dev: Device struct corresponding to the mei device
+ * @type: Payload type (see enum intel_lb_type)
+ * @flags: Payload flags bitmap (e.g. %INTEL_LB_FLAGS_IS_PERSISTENT)
+ * @payload: Pointer to payload buffer
+ * @payload_size: Payload buffer size in bytes
+ *
+ * Return: 0 success, negative errno value on transport failure,
+ * positive status returned by firmware
+ */
+ int (*push_payload)(struct device *dev, u32 type, u32 flags,
+ const void *payload, size_t payload_size);
+};
+
+#endif /* _INTEL_LB_MEI_INTERFACE_H_ */
--
2.49.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/9] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw
2025-07-10 15:08 [PATCH 0/9] Introducing firmware late binding Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 1/9] mei: bus: add mei_cldev_mtu interface Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 2/9] mei: late_bind: add late binding component driver Rodrigo Vivi
@ 2025-07-10 15:08 ` Rodrigo Vivi
2025-07-25 21:38 ` Lucas De Marchi
2025-07-10 15:08 ` [PATCH 4/9] drm/xe/xe_late_bind_fw: Initialize late binding firmware Rodrigo Vivi
` (5 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Rodrigo Vivi @ 2025-07-10 15:08 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: gregkh, daniele.ceraolospurio, anshuman.gupta, alexander.usyskin,
Badal Nilawar, Rodrigo Vivi
From: Badal Nilawar <badal.nilawar@intel.com>
Introducing xe_late_bind_fw to enable firmware loading for the devices,
such as the fan controller, during the driver probe. Typically,
firmware for such devices are part of IFWI flash image but can be
replaced at probe after OEM tuning.
This patch binds mei late binding component to enable firmware loading.
v2:
- Add devm_add_action_or_reset to remove the component (Daniele)
- Add INTEL_MEI_GSC check in xe_late_bind_init() (Daniele)
v3:
- Fail driver probe if late bind initialization fails,
add has_late_bind flag (Daniele)
v4:
- %S/I915_COMPONENT_LATE_BIND/INTEL_COMPONENT_LATE_BIND/
v6:
- rebased
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_device.c | 5 ++
drivers/gpu/drm/xe/xe_device_types.h | 6 ++
drivers/gpu/drm/xe/xe_late_bind_fw.c | 84 ++++++++++++++++++++++
drivers/gpu/drm/xe/xe_late_bind_fw.h | 15 ++++
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 33 +++++++++
drivers/gpu/drm/xe/xe_pci.c | 2 +
drivers/gpu/drm/xe/xe_pci_types.h | 1 +
8 files changed, 147 insertions(+)
create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.c
create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.h
create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw_types.h
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 1d97e5b63f4e..97e2b1368a6e 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -76,6 +76,7 @@ xe-y += xe_bb.o \
xe_hw_fence.o \
xe_irq.o \
xe_lrc.o \
+ xe_late_bind_fw.o \
xe_migrate.o \
xe_mmio.o \
xe_mocs.o \
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 0b73cb72bad1..cb595bae5f55 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -44,6 +44,7 @@
#include "xe_hw_engine_group.h"
#include "xe_hwmon.h"
#include "xe_irq.h"
+#include "xe_late_bind_fw.h"
#include "xe_mmio.h"
#include "xe_module.h"
#include "xe_nvm.h"
@@ -866,6 +867,10 @@ int xe_device_probe(struct xe_device *xe)
if (err)
return err;
+ err = xe_late_bind_init(&xe->late_bind);
+ if (err && err != -ENODEV)
+ return err;
+
err = xe_oa_init(xe);
if (err)
return err;
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 78c4acafd268..a8891833f980 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -16,6 +16,7 @@
#include "xe_devcoredump_types.h"
#include "xe_heci_gsc.h"
#include "xe_lmtt_types.h"
+#include "xe_late_bind_fw_types.h"
#include "xe_memirq_types.h"
#include "xe_oa_types.h"
#include "xe_platform_types.h"
@@ -325,6 +326,8 @@ struct xe_device {
u8 has_heci_cscfi:1;
/** @info.has_heci_gscfi: device has heci gscfi */
u8 has_heci_gscfi:1;
+ /** @info.has_late_bind: Device has firmware late binding support */
+ u8 has_late_bind:1;
/** @info.has_llc: Device has a shared CPU+GPU last level cache */
u8 has_llc:1;
/** @info.has_mbx_power_limits: Device has support to manage power limits using
@@ -557,6 +560,9 @@ struct xe_device {
/** @nvm: discrete graphics non-volatile memory */
struct intel_dg_nvm_dev *nvm;
+ /** @late_bind: xe mei late bind interface */
+ struct xe_late_bind late_bind;
+
/** @oa: oa observation subsystem */
struct xe_oa oa;
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
new file mode 100644
index 000000000000..152f3e58de94
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#include <linux/component.h>
+#include <linux/delay.h>
+
+#include <drm/drm_managed.h>
+#include <drm/intel/i915_component.h>
+#include <drm/intel/intel_lb_mei_interface.h>
+#include <drm/drm_print.h>
+
+#include "xe_device.h"
+#include "xe_late_bind_fw.h"
+
+static struct xe_device *
+late_bind_to_xe(struct xe_late_bind *late_bind)
+{
+ return container_of(late_bind, struct xe_device, late_bind);
+}
+
+static int xe_late_bind_component_bind(struct device *xe_kdev,
+ struct device *mei_kdev, void *data)
+{
+ struct xe_device *xe = kdev_to_xe_device(xe_kdev);
+ struct xe_late_bind *late_bind = &xe->late_bind;
+
+ late_bind->component.ops = data;
+ late_bind->component.mei_dev = mei_kdev;
+
+ return 0;
+}
+
+static void xe_late_bind_component_unbind(struct device *xe_kdev,
+ struct device *mei_kdev, void *data)
+{
+ struct xe_device *xe = kdev_to_xe_device(xe_kdev);
+ struct xe_late_bind *late_bind = &xe->late_bind;
+
+ late_bind->component.ops = NULL;
+}
+
+static const struct component_ops xe_late_bind_component_ops = {
+ .bind = xe_late_bind_component_bind,
+ .unbind = xe_late_bind_component_unbind,
+};
+
+static void xe_late_bind_remove(void *arg)
+{
+ struct xe_late_bind *late_bind = arg;
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+
+ component_del(xe->drm.dev, &xe_late_bind_component_ops);
+}
+
+/**
+ * xe_late_bind_init() - add xe mei late binding component
+ * @late_bind: pointer to late bind structure.
+ *
+ * Return: 0 if the initialization was successful, a negative errno otherwise.
+ */
+int xe_late_bind_init(struct xe_late_bind *late_bind)
+{
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ int err;
+
+ if (!xe->info.has_late_bind)
+ return 0;
+
+ if (!IS_ENABLED(CONFIG_INTEL_MEI_LATE_BIND) || !IS_ENABLED(CONFIG_INTEL_MEI_GSC)) {
+ drm_info(&xe->drm, "Can't init xe mei late bind missing mei component\n");
+ return -ENODEV;
+ }
+
+ err = component_add_typed(xe->drm.dev, &xe_late_bind_component_ops,
+ INTEL_COMPONENT_LB);
+ if (err < 0) {
+ drm_info(&xe->drm, "Failed to add mei late bind component (%pe)\n", ERR_PTR(err));
+ return err;
+ }
+
+ return devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove, late_bind);
+}
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
new file mode 100644
index 000000000000..4c73571c3e62
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#ifndef _XE_LATE_BIND_FW_H_
+#define _XE_LATE_BIND_FW_H_
+
+#include <linux/types.h>
+
+struct xe_late_bind;
+
+int xe_late_bind_init(struct xe_late_bind *late_bind);
+
+#endif
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
new file mode 100644
index 000000000000..f79e5aefed94
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2025 Intel Corporation
+ */
+
+#ifndef _XE_LATE_BIND_TYPES_H_
+#define _XE_LATE_BIND_TYPES_H_
+
+#include <linux/iosys-map.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+
+/**
+ * struct xe_late_bind_component - Late Binding services component
+ * @mei_dev: device that provide Late Binding service.
+ * @ops: Ops implemented by Late Binding driver, used by Xe driver.
+ *
+ * Communication between Xe and MEI drivers for Late Binding services
+ */
+struct xe_late_bind_component {
+ struct device *mei_dev;
+ const struct late_bind_component_ops *ops;
+};
+
+/**
+ * struct xe_late_bind
+ */
+struct xe_late_bind {
+ /** @component: struct for communication with mei component */
+ struct xe_late_bind_component component;
+};
+
+#endif
diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index ffd6ad569b7c..69b8ead9ca59 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -332,6 +332,7 @@ static const struct xe_device_desc bmg_desc = {
.has_gsc_nvm = 1,
.has_heci_cscfi = 1,
.max_gt_per_tile = 2,
+ .has_late_bind = true,
.needs_scratch = true,
};
@@ -578,6 +579,7 @@ static int xe_info_init_early(struct xe_device *xe,
xe->info.has_gsc_nvm = desc->has_gsc_nvm;
xe->info.has_heci_gscfi = desc->has_heci_gscfi;
xe->info.has_heci_cscfi = desc->has_heci_cscfi;
+ xe->info.has_late_bind = desc->has_late_bind;
xe->info.has_llc = desc->has_llc;
xe->info.has_pxp = desc->has_pxp;
xe->info.has_sriov = desc->has_sriov;
diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
index 4de6f69ed975..51a607d323fb 100644
--- a/drivers/gpu/drm/xe/xe_pci_types.h
+++ b/drivers/gpu/drm/xe/xe_pci_types.h
@@ -39,6 +39,7 @@ struct xe_device_desc {
u8 has_gsc_nvm:1;
u8 has_heci_gscfi:1;
u8 has_heci_cscfi:1;
+ u8 has_late_bind:1;
u8 has_llc:1;
u8 has_mbx_power_limits:1;
u8 has_pxp:1;
--
2.49.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/9] drm/xe/xe_late_bind_fw: Initialize late binding firmware
2025-07-10 15:08 [PATCH 0/9] Introducing firmware late binding Rodrigo Vivi
` (2 preceding siblings ...)
2025-07-10 15:08 ` [PATCH 3/9] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw Rodrigo Vivi
@ 2025-07-10 15:08 ` Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 5/9] drm/xe/xe_late_bind_fw: Load " Rodrigo Vivi
` (4 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2025-07-10 15:08 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: gregkh, daniele.ceraolospurio, anshuman.gupta, alexander.usyskin,
Badal Nilawar, Rodrigo Vivi
From: Badal Nilawar <badal.nilawar@intel.com>
Search for late binding firmware binaries and populate the meta data of
firmware structures.
v2 (Daniele):
- drm_err if firmware size is more than max pay load size
- s/request_firmware/firmware_request_nowarn/ as firmware will
not be available for all possible cards
v3 (Daniele):
- init firmware from within xe_late_bind_init, propagate error
- switch late_bind_fw to array to handle multiple firmware types
v4 (Daniele):
- Alloc payload dynamically, fix nits
v6 (Daniele)
- %s/MAX_PAYLOAD_SIZE/XE_LB_MAX_PAYLOAD_SIZE/
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/xe/xe_late_bind_fw.c | 100 ++++++++++++++++++++-
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 30 +++++++
2 files changed, 129 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
index 152f3e58de94..008be9b12fd9 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -5,6 +5,7 @@
#include <linux/component.h>
#include <linux/delay.h>
+#include <linux/firmware.h>
#include <drm/drm_managed.h>
#include <drm/intel/i915_component.h>
@@ -13,6 +14,16 @@
#include "xe_device.h"
#include "xe_late_bind_fw.h"
+#include "xe_pcode.h"
+#include "xe_pcode_api.h"
+
+static const u32 fw_id_to_type[] = {
+ [XE_LB_FW_FAN_CONTROL] = INTEL_LB_TYPE_FAN_CONTROL,
+ };
+
+static const char * const fw_id_to_name[] = {
+ [XE_LB_FW_FAN_CONTROL] = "fan_control",
+ };
static struct xe_device *
late_bind_to_xe(struct xe_late_bind *late_bind)
@@ -20,6 +31,89 @@ late_bind_to_xe(struct xe_late_bind *late_bind)
return container_of(late_bind, struct xe_device, late_bind);
}
+static int xe_late_bind_fw_num_fans(struct xe_late_bind *late_bind)
+{
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ struct xe_tile *root_tile = xe_device_get_root_tile(xe);
+ u32 uval;
+
+ if (!xe_pcode_read(root_tile,
+ PCODE_MBOX(FAN_SPEED_CONTROL, FSC_READ_NUM_FANS, 0), &uval, NULL))
+ return uval;
+ else
+ return 0;
+}
+
+static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
+{
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
+ struct xe_late_bind_fw *lb_fw;
+ const struct firmware *fw;
+ u32 num_fans;
+ int ret;
+
+ if (fw_id >= XE_LB_FW_MAX_ID)
+ return -EINVAL;
+
+ lb_fw = &late_bind->late_bind_fw[fw_id];
+
+ lb_fw->id = fw_id;
+ lb_fw->type = fw_id_to_type[lb_fw->id];
+ lb_fw->flags &= ~INTEL_LB_FLAGS_IS_PERSISTENT;
+
+ if (lb_fw->type == INTEL_LB_TYPE_FAN_CONTROL) {
+ num_fans = xe_late_bind_fw_num_fans(late_bind);
+ drm_dbg(&xe->drm, "Number of Fans: %d\n", num_fans);
+ if (!num_fans)
+ return 0;
+ }
+
+ snprintf(lb_fw->blob_path, sizeof(lb_fw->blob_path), "xe/%s_8086_%04x_%04x_%04x.bin",
+ fw_id_to_name[lb_fw->id], pdev->device,
+ pdev->subsystem_vendor, pdev->subsystem_device);
+
+ drm_dbg(&xe->drm, "Request late binding firmware %s\n", lb_fw->blob_path);
+ ret = firmware_request_nowarn(&fw, lb_fw->blob_path, xe->drm.dev);
+ if (ret) {
+ drm_dbg(&xe->drm, "%s late binding fw not available for current device",
+ fw_id_to_name[lb_fw->id]);
+ return 0;
+ }
+
+ if (fw->size > XE_LB_MAX_PAYLOAD_SIZE) {
+ drm_err(&xe->drm, "Firmware %s size %zu is larger than max pay load size %u\n",
+ lb_fw->blob_path, fw->size, XE_LB_MAX_PAYLOAD_SIZE);
+ release_firmware(fw);
+ return -ENODATA;
+ }
+
+ lb_fw->payload_size = fw->size;
+ lb_fw->payload = drmm_kzalloc(&xe->drm, lb_fw->payload_size, GFP_KERNEL);
+ if (!lb_fw->payload) {
+ release_firmware(fw);
+ return -ENOMEM;
+ }
+
+ memcpy((void *)lb_fw->payload, fw->data, lb_fw->payload_size);
+ release_firmware(fw);
+
+ return 0;
+}
+
+static int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
+{
+ int ret;
+ int fw_id;
+
+ for (fw_id = 0; fw_id < XE_LB_FW_MAX_ID; fw_id++) {
+ ret = __xe_late_bind_fw_init(late_bind, fw_id);
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
static int xe_late_bind_component_bind(struct device *xe_kdev,
struct device *mei_kdev, void *data)
{
@@ -80,5 +174,9 @@ int xe_late_bind_init(struct xe_late_bind *late_bind)
return err;
}
- return devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove, late_bind);
+ err = devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove, late_bind);
+ if (err)
+ return err;
+
+ return xe_late_bind_fw_init(late_bind);
}
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
index f79e5aefed94..c4a8042f2600 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -10,6 +10,34 @@
#include <linux/mutex.h>
#include <linux/types.h>
+#define XE_LB_MAX_PAYLOAD_SIZE SZ_4K
+
+/**
+ * xe_late_bind_fw_id - enum to determine late binding fw index
+ */
+enum xe_late_bind_fw_id {
+ XE_LB_FW_FAN_CONTROL = 0,
+ XE_LB_FW_MAX_ID
+};
+
+/**
+ * struct xe_late_bind_fw
+ */
+struct xe_late_bind_fw {
+ /** @id: firmware index */
+ u32 id;
+ /** @blob_path: firmware binary path */
+ char blob_path[PATH_MAX];
+ /** @type: firmware type */
+ u32 type;
+ /** @flags: firmware flags */
+ u32 flags;
+ /** @payload: to store the late binding blob */
+ const u8 *payload;
+ /** @payload_size: late binding blob payload_size */
+ size_t payload_size;
+};
+
/**
* struct xe_late_bind_component - Late Binding services component
* @mei_dev: device that provide Late Binding service.
@@ -28,6 +56,8 @@ struct xe_late_bind_component {
struct xe_late_bind {
/** @component: struct for communication with mei component */
struct xe_late_bind_component component;
+ /** @late_bind_fw: late binding firmware array */
+ struct xe_late_bind_fw late_bind_fw[XE_LB_FW_MAX_ID];
};
#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 5/9] drm/xe/xe_late_bind_fw: Load late binding firmware
2025-07-10 15:08 [PATCH 0/9] Introducing firmware late binding Rodrigo Vivi
` (3 preceding siblings ...)
2025-07-10 15:08 ` [PATCH 4/9] drm/xe/xe_late_bind_fw: Initialize late binding firmware Rodrigo Vivi
@ 2025-07-10 15:08 ` Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 6/9] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume Rodrigo Vivi
` (3 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2025-07-10 15:08 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: gregkh, daniele.ceraolospurio, anshuman.gupta, alexander.usyskin,
Badal Nilawar, Rodrigo Vivi
From: Badal Nilawar <badal.nilawar@intel.com>
Load late binding firmware
v2:
- s/EAGAIN/EBUSY/
- Flush worker in suspend and driver unload (Daniele)
v3:
- Use retry interval of 6s, in steps of 200ms, to allow
other OS components release MEI CL handle (Sasha)
v4:
- return -ENODEV if component not added (Daniele)
- parse and print status returned by csc
v5:
- Use payload to check firmware valid (Daniele)
- Obtain the RPM reference before scheduling the worker to
ensure the device remains awake until the worker completes
firmware loading (Rodrigo)
v6:
- In case of error donot re-attempt fw download (Daniele)
v7 (Rodrigo):
- Rename of mei structs and callback.
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/xe/xe_late_bind_fw.c | 157 ++++++++++++++++++++-
drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 9 +-
3 files changed, 165 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
index 008be9b12fd9..85cd42a1ec05 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -16,6 +16,20 @@
#include "xe_late_bind_fw.h"
#include "xe_pcode.h"
#include "xe_pcode_api.h"
+#include "xe_pm.h"
+
+/*
+ * The component should load quite quickly in most cases, but it could take
+ * a bit. Using a very big timeout just to cover the worst case scenario
+ */
+#define LB_INIT_TIMEOUT_MS 20000
+
+/*
+ * Retry interval set to 6 seconds, in steps of 200 ms, to allow time for
+ * other OS components to release the MEI CL handle
+ */
+#define LB_FW_LOAD_RETRY_MAXCOUNT 30
+#define LB_FW_LOAD_RETRY_PAUSE_MS 200
static const u32 fw_id_to_type[] = {
[XE_LB_FW_FAN_CONTROL] = INTEL_LB_TYPE_FAN_CONTROL,
@@ -31,6 +45,30 @@ late_bind_to_xe(struct xe_late_bind *late_bind)
return container_of(late_bind, struct xe_device, late_bind);
}
+static const char *xe_late_bind_parse_status(uint32_t status)
+{
+ switch (status) {
+ case INTEL_LB_STATUS_SUCCESS:
+ return "success";
+ case INTEL_LB_STATUS_4ID_MISMATCH:
+ return "4Id Mismatch";
+ case INTEL_LB_STATUS_ARB_FAILURE:
+ return "ARB Failure";
+ case INTEL_LB_STATUS_GENERAL_ERROR:
+ return "General Error";
+ case INTEL_LB_STATUS_INVALID_PARAMS:
+ return "Invalid Params";
+ case INTEL_LB_STATUS_INVALID_SIGNATURE:
+ return "Invalid Signature";
+ case INTEL_LB_STATUS_INVALID_PAYLOAD:
+ return "Invalid Payload";
+ case INTEL_LB_STATUS_TIMEOUT:
+ return "Timeout";
+ default:
+ return "Unknown error";
+ }
+}
+
static int xe_late_bind_fw_num_fans(struct xe_late_bind *late_bind)
{
struct xe_device *xe = late_bind_to_xe(late_bind);
@@ -44,6 +82,101 @@ static int xe_late_bind_fw_num_fans(struct xe_late_bind *late_bind)
return 0;
}
+static void xe_late_bind_wait_for_worker_completion(struct xe_late_bind *late_bind)
+{
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ struct xe_late_bind_fw *lbfw;
+ int fw_id;
+
+ for (fw_id = 0; fw_id < XE_LB_FW_MAX_ID; fw_id++) {
+ lbfw = &late_bind->late_bind_fw[fw_id];
+ if (lbfw->payload && late_bind->wq) {
+ drm_dbg(&xe->drm, "Flush work: load %s firmware\n",
+ fw_id_to_name[lbfw->id]);
+ flush_work(&lbfw->work);
+ }
+ }
+}
+
+static void xe_late_bind_work(struct work_struct *work)
+{
+ struct xe_late_bind_fw *lbfw = container_of(work, struct xe_late_bind_fw, work);
+ struct xe_late_bind *late_bind = container_of(lbfw, struct xe_late_bind,
+ late_bind_fw[lbfw->id]);
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ int retry = LB_FW_LOAD_RETRY_MAXCOUNT;
+ int ret;
+ int slept;
+
+ xe_device_assert_mem_access(xe);
+
+ /* we can queue this before the component is bound */
+ for (slept = 0; slept < LB_INIT_TIMEOUT_MS; slept += 100) {
+ if (late_bind->component.ops)
+ break;
+ msleep(100);
+ }
+
+ if (!late_bind->component.ops) {
+ drm_err(&xe->drm, "Late bind component not bound\n");
+ /* Do not re-attempt fw load */
+ drmm_kfree(&xe->drm, (void *)lbfw->payload);
+ lbfw->payload = NULL;
+ goto out;
+ }
+
+ drm_dbg(&xe->drm, "Load %s firmware\n", fw_id_to_name[lbfw->id]);
+
+ do {
+ ret = late_bind->component.ops->push_payload(late_bind->component.mei_dev,
+ lbfw->type,
+ lbfw->flags,
+ lbfw->payload,
+ lbfw->payload_size);
+ if (!ret)
+ break;
+ msleep(LB_FW_LOAD_RETRY_PAUSE_MS);
+ } while (--retry && ret == -EBUSY);
+
+ if (!ret) {
+ drm_dbg(&xe->drm, "Load %s firmware successful\n",
+ fw_id_to_name[lbfw->id]);
+ goto out;
+ }
+
+ if (ret > 0)
+ drm_err(&xe->drm, "Load %s firmware failed with err %d, %s\n",
+ fw_id_to_name[lbfw->id], ret, xe_late_bind_parse_status(ret));
+ else
+ drm_err(&xe->drm, "Load %s firmware failed with err %d",
+ fw_id_to_name[lbfw->id], ret);
+ /* Do not re-attempt fw load */
+ drmm_kfree(&xe->drm, (void *)lbfw->payload);
+ lbfw->payload = NULL;
+
+out:
+ xe_pm_runtime_put(xe);
+}
+
+int xe_late_bind_fw_load(struct xe_late_bind *late_bind)
+{
+ struct xe_device *xe = late_bind_to_xe(late_bind);
+ struct xe_late_bind_fw *lbfw;
+ int fw_id;
+
+ if (!late_bind->component_added)
+ return -ENODEV;
+
+ for (fw_id = 0; fw_id < XE_LB_FW_MAX_ID; fw_id++) {
+ lbfw = &late_bind->late_bind_fw[fw_id];
+ if (lbfw->payload) {
+ xe_pm_runtime_get_noresume(xe);
+ queue_work(late_bind->wq, &lbfw->work);
+ }
+ }
+ return 0;
+}
+
static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
{
struct xe_device *xe = late_bind_to_xe(late_bind);
@@ -97,6 +230,7 @@ static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
memcpy((void *)lb_fw->payload, fw->data, lb_fw->payload_size);
release_firmware(fw);
+ INIT_WORK(&lb_fw->work, xe_late_bind_work);
return 0;
}
@@ -106,11 +240,16 @@ static int xe_late_bind_fw_init(struct xe_late_bind *late_bind)
int ret;
int fw_id;
+ late_bind->wq = alloc_ordered_workqueue("late-bind-ordered-wq", 0);
+ if (!late_bind->wq)
+ return -ENOMEM;
+
for (fw_id = 0; fw_id < XE_LB_FW_MAX_ID; fw_id++) {
ret = __xe_late_bind_fw_init(late_bind, fw_id);
if (ret)
return ret;
}
+
return 0;
}
@@ -132,6 +271,8 @@ static void xe_late_bind_component_unbind(struct device *xe_kdev,
struct xe_device *xe = kdev_to_xe_device(xe_kdev);
struct xe_late_bind *late_bind = &xe->late_bind;
+ xe_late_bind_wait_for_worker_completion(late_bind);
+
late_bind->component.ops = NULL;
}
@@ -145,7 +286,15 @@ static void xe_late_bind_remove(void *arg)
struct xe_late_bind *late_bind = arg;
struct xe_device *xe = late_bind_to_xe(late_bind);
+ xe_late_bind_wait_for_worker_completion(late_bind);
+
+ late_bind->component_added = false;
+
component_del(xe->drm.dev, &xe_late_bind_component_ops);
+ if (late_bind->wq) {
+ destroy_workqueue(late_bind->wq);
+ late_bind->wq = NULL;
+ }
}
/**
@@ -174,9 +323,15 @@ int xe_late_bind_init(struct xe_late_bind *late_bind)
return err;
}
+ late_bind->component_added = true;
+
err = devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove, late_bind);
if (err)
return err;
- return xe_late_bind_fw_init(late_bind);
+ err = xe_late_bind_fw_init(late_bind);
+ if (err)
+ return err;
+
+ return xe_late_bind_fw_load(late_bind);
}
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
index 4c73571c3e62..28d56ed2bfdc 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
@@ -11,5 +11,6 @@
struct xe_late_bind;
int xe_late_bind_init(struct xe_late_bind *late_bind);
+int xe_late_bind_fw_load(struct xe_late_bind *late_bind);
#endif
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
index c4a8042f2600..5c0574aff7b9 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -9,6 +9,7 @@
#include <linux/iosys-map.h>
#include <linux/mutex.h>
#include <linux/types.h>
+#include <linux/workqueue.h>
#define XE_LB_MAX_PAYLOAD_SIZE SZ_4K
@@ -36,6 +37,8 @@ struct xe_late_bind_fw {
const u8 *payload;
/** @payload_size: late binding blob payload_size */
size_t payload_size;
+ /** @work: worker to upload latebind blob */
+ struct work_struct work;
};
/**
@@ -47,7 +50,7 @@ struct xe_late_bind_fw {
*/
struct xe_late_bind_component {
struct device *mei_dev;
- const struct late_bind_component_ops *ops;
+ const struct intel_lb_component_ops *ops;
};
/**
@@ -58,6 +61,10 @@ struct xe_late_bind {
struct xe_late_bind_component component;
/** @late_bind_fw: late binding firmware array */
struct xe_late_bind_fw late_bind_fw[XE_LB_FW_MAX_ID];
+ /** @wq: workqueue to submit request to download late bind blob */
+ struct workqueue_struct *wq;
+ /** @component_added: whether the component has been added */
+ bool component_added;
};
#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 6/9] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume
2025-07-10 15:08 [PATCH 0/9] Introducing firmware late binding Rodrigo Vivi
` (4 preceding siblings ...)
2025-07-10 15:08 ` [PATCH 5/9] drm/xe/xe_late_bind_fw: Load " Rodrigo Vivi
@ 2025-07-10 15:08 ` Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 7/9] drm/xe/xe_late_bind_fw: Reload late binding fw during system resume Rodrigo Vivi
` (2 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2025-07-10 15:08 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: gregkh, daniele.ceraolospurio, anshuman.gupta, alexander.usyskin,
Badal Nilawar, Rodrigo Vivi
From: Badal Nilawar <badal.nilawar@intel.com>
Reload late binding fw during runtime resume.
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/xe/xe_late_bind_fw.c | 2 +-
drivers/gpu/drm/xe/xe_late_bind_fw.h | 1 +
drivers/gpu/drm/xe/xe_pm.c | 4 ++++
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
index 85cd42a1ec05..df43523e9043 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -82,7 +82,7 @@ static int xe_late_bind_fw_num_fans(struct xe_late_bind *late_bind)
return 0;
}
-static void xe_late_bind_wait_for_worker_completion(struct xe_late_bind *late_bind)
+void xe_late_bind_wait_for_worker_completion(struct xe_late_bind *late_bind)
{
struct xe_device *xe = late_bind_to_xe(late_bind);
struct xe_late_bind_fw *lbfw;
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
index 28d56ed2bfdc..07e437390539 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
@@ -12,5 +12,6 @@ struct xe_late_bind;
int xe_late_bind_init(struct xe_late_bind *late_bind);
int xe_late_bind_fw_load(struct xe_late_bind *late_bind);
+void xe_late_bind_wait_for_worker_completion(struct xe_late_bind *late_bind);
#endif
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index 7131c403cbe8..b2cd94dd817e 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -20,6 +20,7 @@
#include "xe_gt.h"
#include "xe_guc.h"
#include "xe_irq.h"
+#include "xe_late_bind_fw.h"
#include "xe_pcode.h"
#include "xe_pxp.h"
#include "xe_trace.h"
@@ -545,6 +546,9 @@ int xe_pm_runtime_resume(struct xe_device *xe)
xe_pxp_pm_resume(xe->pxp);
+ if (xe->d3cold.allowed)
+ xe_late_bind_fw_load(&xe->late_bind);
+
out:
xe_rpm_lockmap_release(xe);
xe_pm_write_callback_task(xe, NULL);
--
2.49.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 7/9] drm/xe/xe_late_bind_fw: Reload late binding fw during system resume
2025-07-10 15:08 [PATCH 0/9] Introducing firmware late binding Rodrigo Vivi
` (5 preceding siblings ...)
2025-07-10 15:08 ` [PATCH 6/9] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume Rodrigo Vivi
@ 2025-07-10 15:08 ` Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 8/9] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 9/9] drm/xe/xe_late_bind_fw: Extract and print version info Rodrigo Vivi
8 siblings, 0 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2025-07-10 15:08 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: gregkh, daniele.ceraolospurio, anshuman.gupta, alexander.usyskin,
Badal Nilawar, Rodrigo Vivi
From: Badal Nilawar <badal.nilawar@intel.com>
Reload late binding fw during resume from system suspend
v2:
- Unconditionally reload late binding fw (Rodrigo)
- Flush worker during system suspend
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/xe/xe_pm.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index b2cd94dd817e..75b65eb61c76 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -127,6 +127,8 @@ int xe_pm_suspend(struct xe_device *xe)
if (err)
goto err;
+ xe_late_bind_wait_for_worker_completion(&xe->late_bind);
+
for_each_gt(gt, xe, id)
xe_gt_suspend_prepare(gt);
@@ -204,6 +206,8 @@ int xe_pm_resume(struct xe_device *xe)
xe_pxp_pm_resume(xe->pxp);
+ xe_late_bind_fw_load(&xe->late_bind);
+
drm_dbg(&xe->drm, "Device resumed\n");
return 0;
err:
--
2.49.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 8/9] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding
2025-07-10 15:08 [PATCH 0/9] Introducing firmware late binding Rodrigo Vivi
` (6 preceding siblings ...)
2025-07-10 15:08 ` [PATCH 7/9] drm/xe/xe_late_bind_fw: Reload late binding fw during system resume Rodrigo Vivi
@ 2025-07-10 15:08 ` Rodrigo Vivi
2025-07-31 20:03 ` Summers, Stuart
2025-07-10 15:08 ` [PATCH 9/9] drm/xe/xe_late_bind_fw: Extract and print version info Rodrigo Vivi
8 siblings, 1 reply; 22+ messages in thread
From: Rodrigo Vivi @ 2025-07-10 15:08 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: gregkh, daniele.ceraolospurio, anshuman.gupta, alexander.usyskin,
Badal Nilawar, Rodrigo Vivi
From: Badal Nilawar <badal.nilawar@intel.com>
Introduce a debug filesystem node to disable late binding fw reload
during the system or runtime resume. This is intended for situations
where the late binding fw needs to be loaded from user mode,
perticularly for validation purpose.
Note that xe kmd doesn't participate in late binding flow from user
space. Binary loaded from the userspace will be lost upon entering to
D3 cold hence user space app need to handle this situation.
v2:
- s/(uval == 1) ? true : false/!!uval/ (Daniele)
v3:
- Refine the commit message (Daniele)
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/xe/xe_debugfs.c | 41 ++++++++++++++++++++++
drivers/gpu/drm/xe/xe_late_bind_fw.c | 3 ++
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 2 ++
3 files changed, 46 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
index d83cd6ed3fa8..d1f6f556efa2 100644
--- a/drivers/gpu/drm/xe/xe_debugfs.c
+++ b/drivers/gpu/drm/xe/xe_debugfs.c
@@ -226,6 +226,44 @@ static const struct file_operations atomic_svm_timeslice_ms_fops = {
.write = atomic_svm_timeslice_ms_set,
};
+static ssize_t disable_late_binding_show(struct file *f, char __user *ubuf,
+ size_t size, loff_t *pos)
+{
+ struct xe_device *xe = file_inode(f)->i_private;
+ struct xe_late_bind *late_bind = &xe->late_bind;
+ char buf[32];
+ int len;
+
+ len = scnprintf(buf, sizeof(buf), "%d\n", late_bind->disable);
+
+ return simple_read_from_buffer(ubuf, size, pos, buf, len);
+}
+
+static ssize_t disable_late_binding_set(struct file *f, const char __user *ubuf,
+ size_t size, loff_t *pos)
+{
+ struct xe_device *xe = file_inode(f)->i_private;
+ struct xe_late_bind *late_bind = &xe->late_bind;
+ u32 uval;
+ ssize_t ret;
+
+ ret = kstrtouint_from_user(ubuf, size, sizeof(uval), &uval);
+ if (ret)
+ return ret;
+
+ if (uval > 1)
+ return -EINVAL;
+
+ late_bind->disable = !!uval;
+ return size;
+}
+
+static const struct file_operations disable_late_binding_fops = {
+ .owner = THIS_MODULE,
+ .read = disable_late_binding_show,
+ .write = disable_late_binding_set,
+};
+
void xe_debugfs_register(struct xe_device *xe)
{
struct ttm_device *bdev = &xe->ttm;
@@ -249,6 +287,9 @@ void xe_debugfs_register(struct xe_device *xe)
debugfs_create_file("atomic_svm_timeslice_ms", 0600, root, xe,
&atomic_svm_timeslice_ms_fops);
+ debugfs_create_file("disable_late_binding", 0600, root, xe,
+ &disable_late_binding_fops);
+
for (mem_type = XE_PL_VRAM0; mem_type <= XE_PL_VRAM1; ++mem_type) {
man = ttm_manager_type(bdev, mem_type);
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
index df43523e9043..88355adce1d0 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -167,6 +167,9 @@ int xe_late_bind_fw_load(struct xe_late_bind *late_bind)
if (!late_bind->component_added)
return -ENODEV;
+ if (late_bind->disable)
+ return 0;
+
for (fw_id = 0; fw_id < XE_LB_FW_MAX_ID; fw_id++) {
lbfw = &late_bind->late_bind_fw[fw_id];
if (lbfw->payload) {
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
index 5c0574aff7b9..158dc1abe072 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -65,6 +65,8 @@ struct xe_late_bind {
struct workqueue_struct *wq;
/** @component_added: whether the component has been added */
bool component_added;
+ /** @disable: to block late binding reload during pm resume flow*/
+ bool disable;
};
#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 9/9] drm/xe/xe_late_bind_fw: Extract and print version info
2025-07-10 15:08 [PATCH 0/9] Introducing firmware late binding Rodrigo Vivi
` (7 preceding siblings ...)
2025-07-10 15:08 ` [PATCH 8/9] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding Rodrigo Vivi
@ 2025-07-10 15:08 ` Rodrigo Vivi
8 siblings, 0 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2025-07-10 15:08 UTC (permalink / raw)
To: intel-xe, dri-devel, linux-kernel
Cc: gregkh, daniele.ceraolospurio, anshuman.gupta, alexander.usyskin,
Badal Nilawar, Rodrigo Vivi
From: Badal Nilawar <badal.nilawar@intel.com>
Extract and print version info of the late binding binary.
v2: Some refinements (Daniele)
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/xe/xe_late_bind_fw.c | 124 +++++++++++++++++++++
drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 3 +
drivers/gpu/drm/xe/xe_uc_fw_abi.h | 66 +++++++++++
3 files changed, 193 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
index 88355adce1d0..171771639761 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
@@ -45,6 +45,121 @@ late_bind_to_xe(struct xe_late_bind *late_bind)
return container_of(late_bind, struct xe_device, late_bind);
}
+static struct xe_device *
+late_bind_fw_to_xe(struct xe_late_bind_fw *lb_fw)
+{
+ return container_of(lb_fw, struct xe_device, late_bind.late_bind_fw[lb_fw->id]);
+}
+
+/* Refer to the "Late Bind based Firmware Layout" documentation entry for details */
+static int parse_cpd_header(struct xe_late_bind_fw *lb_fw,
+ const void *data, size_t size, const char *manifest_entry)
+{
+ struct xe_device *xe = late_bind_fw_to_xe(lb_fw);
+ const struct gsc_cpd_header_v2 *header = data;
+ const struct gsc_manifest_header *manifest;
+ const struct gsc_cpd_entry *entry;
+ size_t min_size = sizeof(*header);
+ u32 offset;
+ int i;
+
+ /* manifest_entry is mandatory */
+ xe_assert(xe, manifest_entry);
+
+ if (size < min_size || header->header_marker != GSC_CPD_HEADER_MARKER)
+ return -ENOENT;
+
+ if (header->header_length < sizeof(struct gsc_cpd_header_v2)) {
+ drm_err(&xe->drm, "%s late binding fw: Invalid CPD header length %u!\n",
+ fw_id_to_name[lb_fw->id], header->header_length);
+ return -EINVAL;
+ }
+
+ min_size = header->header_length + sizeof(struct gsc_cpd_entry) * header->num_of_entries;
+ if (size < min_size) {
+ drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
+ fw_id_to_name[lb_fw->id], size, min_size);
+ return -ENODATA;
+ }
+
+ /* Look for the manifest first */
+ entry = (void *)header + header->header_length;
+ for (i = 0; i < header->num_of_entries; i++, entry++)
+ if (strcmp(entry->name, manifest_entry) == 0)
+ offset = entry->offset & GSC_CPD_ENTRY_OFFSET_MASK;
+
+ if (!offset) {
+ drm_err(&xe->drm, "%s late binding fw: Failed to find manifest_entry\n",
+ fw_id_to_name[lb_fw->id]);
+ return -ENODATA;
+ }
+
+ min_size = offset + sizeof(struct gsc_manifest_header);
+ if (size < min_size) {
+ drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
+ fw_id_to_name[lb_fw->id], size, min_size);
+ return -ENODATA;
+ }
+
+ manifest = data + offset;
+
+ lb_fw->version = manifest->fw_version;
+
+ return 0;
+}
+
+/* Refer to the "Late Bind based Firmware Layout" documentation entry for details */
+static int parse_lb_layout(struct xe_late_bind_fw *lb_fw,
+ const void *data, size_t size, const char *fpt_entry)
+{
+ struct xe_device *xe = late_bind_fw_to_xe(lb_fw);
+ const struct csc_fpt_header *header = data;
+ const struct csc_fpt_entry *entry;
+ size_t min_size = sizeof(*header);
+ u32 offset;
+ int i;
+
+ /* fpt_entry is mandatory */
+ xe_assert(xe, fpt_entry);
+
+ if (size < min_size || header->header_marker != CSC_FPT_HEADER_MARKER)
+ return -ENOENT;
+
+ if (header->header_length < sizeof(struct csc_fpt_header)) {
+ drm_err(&xe->drm, "%s late binding fw: Invalid FPT header length %u!\n",
+ fw_id_to_name[lb_fw->id], header->header_length);
+ return -EINVAL;
+ }
+
+ min_size = header->header_length + sizeof(struct csc_fpt_entry) * header->num_of_entries;
+ if (size < min_size) {
+ drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
+ fw_id_to_name[lb_fw->id], size, min_size);
+ return -ENODATA;
+ }
+
+ /* Look for the cpd header first */
+ entry = (void *)header + header->header_length;
+ for (i = 0; i < header->num_of_entries; i++, entry++)
+ if (strcmp(entry->name, fpt_entry) == 0)
+ offset = entry->offset;
+
+ if (!offset) {
+ drm_err(&xe->drm, "%s late binding fw: Failed to find fpt_entry\n",
+ fw_id_to_name[lb_fw->id]);
+ return -ENODATA;
+ }
+
+ min_size = offset + sizeof(struct gsc_cpd_header_v2);
+ if (size < min_size) {
+ drm_err(&xe->drm, "%s late binding fw: too small! %zu < %zu\n",
+ fw_id_to_name[lb_fw->id], size, min_size);
+ return -ENODATA;
+ }
+
+ return parse_cpd_header(lb_fw, data + offset, size - offset, "LTES.man");
+}
+
static const char *xe_late_bind_parse_status(uint32_t status)
{
switch (status) {
@@ -224,6 +339,10 @@ static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
return -ENODATA;
}
+ ret = parse_lb_layout(lb_fw, fw->data, fw->size, "LTES");
+ if (ret)
+ return ret;
+
lb_fw->payload_size = fw->size;
lb_fw->payload = drmm_kzalloc(&xe->drm, lb_fw->payload_size, GFP_KERNEL);
if (!lb_fw->payload) {
@@ -231,6 +350,11 @@ static int __xe_late_bind_fw_init(struct xe_late_bind *late_bind, u32 fw_id)
return -ENOMEM;
}
+ drm_info(&xe->drm, "Using %s firmware from %s version %u.%u.%u.%u\n",
+ fw_id_to_name[lb_fw->id], lb_fw->blob_path,
+ lb_fw->version.major, lb_fw->version.minor,
+ lb_fw->version.hotfix, lb_fw->version.build);
+
memcpy((void *)lb_fw->payload, fw->data, lb_fw->payload_size);
release_firmware(fw);
INIT_WORK(&lb_fw->work, xe_late_bind_work);
diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
index 158dc1abe072..0f5da89ce98b 100644
--- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
@@ -10,6 +10,7 @@
#include <linux/mutex.h>
#include <linux/types.h>
#include <linux/workqueue.h>
+#include "xe_uc_fw_abi.h"
#define XE_LB_MAX_PAYLOAD_SIZE SZ_4K
@@ -39,6 +40,8 @@ struct xe_late_bind_fw {
size_t payload_size;
/** @work: worker to upload latebind blob */
struct work_struct work;
+ /** @version: late binding blob manifest version */
+ struct gsc_version version;
};
/**
diff --git a/drivers/gpu/drm/xe/xe_uc_fw_abi.h b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
index 87ade41209d0..78782d105fa9 100644
--- a/drivers/gpu/drm/xe/xe_uc_fw_abi.h
+++ b/drivers/gpu/drm/xe/xe_uc_fw_abi.h
@@ -318,4 +318,70 @@ struct gsc_manifest_header {
u32 exponent_size; /* in dwords */
} __packed;
+/**
+ * DOC: Late binding Firmware Layout
+ *
+ * The Late binding binary starts with FPT header, which contains locations
+ * of various partitions of the binary. Here we're interested in finding out
+ * manifest version. To the manifest version, we need to locate CPD header
+ * one of the entry in CPD header points to manifest header. Manifest header
+ * contains the version.
+ *
+ * +================================================+
+ * | FPT Header |
+ * +================================================+
+ * | FPT entries[] |
+ * | entry1 |
+ * | ... |
+ * | entryX |
+ * | "LTES" |
+ * | ... |
+ * | offset >-----------------------------|------o
+ * +================================================+ |
+ * |
+ * +================================================+ |
+ * | CPD Header |<-----o
+ * +================================================+
+ * | CPD entries[] |
+ * | entry1 |
+ * | ... |
+ * | entryX |
+ * | "LTES.man" |
+ * | ... |
+ * | offset >----------------------------|------o
+ * +================================================+ |
+ * |
+ * +================================================+ |
+ * | Manifest Header |<-----o
+ * | ... |
+ * | FW version |
+ * | ... |
+ * +================================================+
+ */
+
+/* FPT Headers */
+struct csc_fpt_header {
+ u32 header_marker;
+#define CSC_FPT_HEADER_MARKER 0x54504624
+ u32 num_of_entries;
+ u8 header_version;
+ u8 entry_version;
+ u8 header_length; /* in bytes */
+ u8 flags;
+ u16 ticks_to_add;
+ u16 tokens_to_add;
+ u32 uma_size;
+ u32 crc32;
+ struct gsc_version fitc_version;
+} __packed;
+
+struct csc_fpt_entry {
+ u8 name[4]; /* partition name */
+ u32 reserved1;
+ u32 offset; /* offset from beginning of CSE region */
+ u32 length; /* partition length in bytes */
+ u32 reserved2[3];
+ u32 partition_flags;
+} __packed;
+
#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/9] mei: late_bind: add late binding component driver
2025-07-10 15:08 ` [PATCH 2/9] mei: late_bind: add late binding component driver Rodrigo Vivi
@ 2025-07-16 11:48 ` Greg KH
2025-07-16 11:58 ` Usyskin, Alexander
0 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2025-07-16 11:48 UTC (permalink / raw)
To: Rodrigo Vivi
Cc: intel-xe, dri-devel, linux-kernel, daniele.ceraolospurio,
anshuman.gupta, alexander.usyskin, Badal Nilawar
On Thu, Jul 10, 2025 at 11:08:33AM -0400, Rodrigo Vivi wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
>
> Introduce a new MEI client driver to support Late Binding firmware
> upload/update for Intel discrete graphics platforms.
>
> Late Binding is a runtime firmware upload/update mechanism that allows
> payloads, such as fan control and voltage regulator, to be securely
> delivered and applied without requiring SPI flash updates or
> system reboots. This driver enables the Xe graphics driver and other
> user-space tools to push such firmware blobs to the authentication
> firmware via the MEI interface.
>
> The driver handles authentication, versioning, and communication
> with the authentication firmware, which in turn coordinates with
> the PUnit/PCODE to apply the payload.
>
> This is a foundational component for enabling dynamic, secure,
> and re-entrant configuration updates on platforms like Battlemage.
>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>
> Changes in this revision:
> - Proper commit message
> - Proper explanation of 'Late Binding' on Kconfig help and doc
> - Consistency in naming:
> + mei_ prefix where it makes sense
> + use 'lb' for short of 'Late Binding' instead of 'late_bind'
> Including s/CONFIG_INTEL_MEI_LATE_BIND/CONFIG_INTEL_MEI_LB
> + remove stray 'struct module'
> + Fix structs and enum documentation style and fields
> + Remove 'CSC' to avoid yet another acronym. 'Authentication firmware' it is.
> + specify size unit
> + s/push_config/push_payload
>
> drivers/misc/mei/Kconfig | 13 +
> drivers/misc/mei/Makefile | 1 +
> drivers/misc/mei/mei_lb.c | 315 +++++++++++++++++++++
> include/drm/intel/i915_component.h | 1 +
> include/drm/intel/intel_lb_mei_interface.h | 70 +++++
> 5 files changed, 400 insertions(+)
> create mode 100644 drivers/misc/mei/mei_lb.c
> create mode 100644 include/drm/intel/intel_lb_mei_interface.h
>
> diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
> index 7575fee96cc6..f8b04e49e4ba 100644
> --- a/drivers/misc/mei/Kconfig
> +++ b/drivers/misc/mei/Kconfig
> @@ -81,6 +81,19 @@ config INTEL_MEI_VSC
> This driver can also be built as a module. If so, the module
> will be called mei-vsc.
>
> +config INTEL_MEI_LB
> + tristate "Intel Late Binding (LB) support on ME Interface"
> + depends on INTEL_MEI_ME
> + depends on DRM_XE
> + help
> + Enable support for Intel Late Binding (LB) via the MEI interface.
> +
> + Late Binding is a method for applying firmware updates at runtime,
> + allowing the Intel Xe driver to load firmware payloads such as
> + fan controller or voltage regulator. These firmware updates are
> + authenticated and versioned, and do not require firmware flashing
> + or system reboot.
> +
> source "drivers/misc/mei/hdcp/Kconfig"
> source "drivers/misc/mei/pxp/Kconfig"
> source "drivers/misc/mei/gsc_proxy/Kconfig"
> diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
> index 6f9fdbf1a495..a203ed766b33 100644
> --- a/drivers/misc/mei/Makefile
> +++ b/drivers/misc/mei/Makefile
> @@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src)
> obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/
> obj-$(CONFIG_INTEL_MEI_PXP) += pxp/
> obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/
> +obj-$(CONFIG_INTEL_MEI_LB) += mei_lb.o
>
> obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o
> mei-vsc-hw-y := vsc-tp.o
> diff --git a/drivers/misc/mei/mei_lb.c b/drivers/misc/mei/mei_lb.c
> new file mode 100644
> index 000000000000..fddef862712d
> --- /dev/null
> +++ b/drivers/misc/mei/mei_lb.c
> @@ -0,0 +1,315 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2025 Intel Corporation
> + */
> +#include <drm/intel/i915_component.h>
> +#include <drm/intel/intel_lb_mei_interface.h>
> +#include <linux/component.h>
> +#include <linux/pci.h>
> +#include <linux/mei_cl_bus.h>
> +#include <linux/module.h>
> +#include <linux/overflow.h>
> +#include <linux/slab.h>
> +#include <linux/uuid.h>
> +
> +#include "mkhi.h"
> +
> +/**
> + * DOC: Late Binding Firmware Update/Upload
> + *
> + * Late Binding is a firmware update/upload mechanism that allows configuration
> + * payloads to be securely delivered and applied at runtime, rather than
> + * being embedded in the system firmware image (e.g., IFWI or SPI flash).
> + *
> + * This mechanism is used to update device-level configuration such as:
> + * - Fan controller
> + * - Voltage regulator (VR)
> + *
> + * Key Characteristics:
> + * ---------------------
> + * - Runtime Delivery:
> + * Firmware blobs are loaded by the host driver (e.g., Xe KMD)
> + * after the GPU or SoC has booted.
> + *
> + * - Secure and Authenticated:
> + * All payloads are signed and verified by the authentication firmware.
> + *
> + * - No Firmware Flashing Required:
> + * Updates are applied in volatile memory and do not require SPI flash
> + * modification or system reboot.
> + *
> + * - Re-entrant:
> + * Multiple updates of the same or different types can be applied
> + * sequentially within a single boot session.
> + *
> + * - Version Controlled:
> + * Each payload includes version and security version number (SVN)
> + * metadata to support anti-rollback enforcement.
> + *
> + * Upload Flow:
> + * ------------
> + * 1. Host driver (KMD or user-space tool) loads the late binding firmware.
> + * 2. Firmware is passed to the MEI interface and forwarded to
> + * authentication firmware.
> + * 3. Authentication firmware authenticates the payload and extracts
> + * command and data arrays.
> + * 4. Authentication firmware delivers the configuration to PUnit/PCODE.
> + * 5. Status is returned back to the host via MEI.
> + */
> +
> +#define INTEL_LB_CMD 0x12
> +#define INTEL_LB_RSP (INTEL_LB_CMD | 0x80)
> +
> +#define INTEL_LB_SEND_TIMEOUT_MSEC 3000
> +#define INTEL_LB_RECV_TIMEOUT_MSEC 3000
> +
> +/**
> + * struct mei_lb_req - Late Binding request structure
> + * @header: MKHI message header (see struct mkhi_msg_hdr)
> + * @type: Type of the Late Binding payload
> + * @flags: Flags to be passed to the authentication firmware (e.g. %INTEL_LB_FLAGS_IS_PERSISTENT)
> + * @reserved: Reserved for future use by authentication firmware, must be set to 0
> + * @payload_size: Size of the payload data in bytes
> + * @payload: Payload data to be sent to the authentication firmware
> + */
> +struct mei_lb_req {
> + struct mkhi_msg_hdr header;
> + __le32 type;
> + __le32 flags;
> + __le32 reserved[2];
> + __le32 payload_size;
> + u8 payload[] __counted_by(payload_size);
> +} __packed;
> +
> +/**
> + * struct mei_lb_rsp - Late Binding response structure
> + * @header: MKHI message header (see struct mkhi_msg_hdr)
> + * @type: Type of the Late Binding payload
> + * @reserved: Reserved for future use by authentication firmware, must be set to 0
> + * @status: Status returned by authentication firmware (see enum intel_lb_status)
> + */
> +struct mei_lb_rsp {
> + struct mkhi_msg_hdr header;
> + __le32 type;
> + __le32 reserved[2];
> + __le32 status;
> +} __packed;
> +
> +static int mei_lb_check_response(const struct device *dev, const struct mkhi_msg_hdr *hdr)
> +{
> + if (hdr->group_id != MKHI_GROUP_ID_GFX) {
> + dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n",
> + hdr->group_id, MKHI_GROUP_ID_GFX);
> + return -EINVAL;
> + }
> +
> + if (hdr->command != INTEL_LB_RSP) {
> + dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n",
> + hdr->command, INTEL_LB_RSP);
> + return -EINVAL;
> + }
> +
> + if (hdr->result) {
> + dev_err(dev, "Error in result: 0x%x\n", hdr->result);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int mei_lb_push_payload(struct device *dev,
> + enum intel_lb_type type, u32 flags,
> + const void *payload, size_t payload_size)
> +{
> + struct mei_cl_device *cldev;
> + struct mei_lb_req *req = NULL;
> + struct mei_lb_rsp rsp;
> + size_t req_size;
> + ssize_t bytes;
> + int ret;
> +
> + cldev = to_mei_cl_device(dev);
> +
> + ret = mei_cldev_enable(cldev);
> + if (ret) {
> + dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret);
> + return ret;
> + }
> +
> + req_size = struct_size(req, payload, payload_size);
> + if (req_size > mei_cldev_mtu(cldev)) {
> + dev_err(dev, "Payload is too big %zu\n", payload_size);
> + ret = -EMSGSIZE;
> + goto end;
> + }
> +
> + req = kmalloc(req_size, GFP_KERNEL);
> + if (!req) {
> + ret = -ENOMEM;
> + goto end;
> + }
> +
> + req->header.group_id = MKHI_GROUP_ID_GFX;
> + req->header.command = INTEL_LB_CMD;
> + req->type = cpu_to_le32(type);
> + req->flags = cpu_to_le32(flags);
> + req->reserved[0] = 0;
> + req->reserved[1] = 0;
> + req->payload_size = cpu_to_le32(payload_size);
> + memcpy(req->payload, payload, payload_size);
> +
> + bytes = mei_cldev_send_timeout(cldev,
> + (void *)req, req_size, INTEL_LB_SEND_TIMEOUT_MSEC);
> + if (bytes < 0) {
> + dev_err(dev, "mei_cldev_send failed. %zd\n", bytes);
> + ret = bytes;
> + goto end;
> + }
> +
> + bytes = mei_cldev_recv_timeout(cldev,
> + (void *)&rsp, sizeof(rsp), INTEL_LB_RECV_TIMEOUT_MSEC);
> + if (bytes < 0) {
> + dev_err(dev, "mei_cldev_recv failed. %zd\n", bytes);
> + ret = bytes;
> + goto end;
> + }
> + if (bytes < sizeof(rsp.header)) {
> + dev_err(dev, "bad response header from the firmware: size %zd < %zu\n",
> + bytes, sizeof(rsp.header));
> + ret = -EPROTO;
> + goto end;
> + }
> + if (mei_lb_check_response(dev, &rsp.header)) {
> + dev_err(dev, "bad result response from the firmware: 0x%x\n",
> + *(uint32_t *)&rsp.header);
What exactly are you printing out to userspace here? A pointer? Or a
random value from the firmware? Why?
> + ret = -EPROTO;
> + goto end;
> + }
You forgot to check the type and reserved fields of the rsp structure :(
> + if (bytes < sizeof(rsp)) {
> + dev_err(dev, "bad response from the firmware: size %zd < %zu\n",
> + bytes, sizeof(rsp));
> + ret = -EPROTO;
> + goto end;
> + }
Why not check this above when you check against the size of the header?
You only need one size check, not 2.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH 2/9] mei: late_bind: add late binding component driver
2025-07-16 11:48 ` Greg KH
@ 2025-07-16 11:58 ` Usyskin, Alexander
2025-07-16 12:07 ` Greg KH
0 siblings, 1 reply; 22+ messages in thread
From: Usyskin, Alexander @ 2025-07-16 11:58 UTC (permalink / raw)
To: Greg KH, Vivi, Rodrigo
Cc: intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, Ceraolo Spurio, Daniele,
Gupta, Anshuman, Nilawar, Badal
> Subject: Re: [PATCH 2/9] mei: late_bind: add late binding component driver
>
> On Thu, Jul 10, 2025 at 11:08:33AM -0400, Rodrigo Vivi wrote:
> > From: Alexander Usyskin <alexander.usyskin@intel.com>
> >
> > Introduce a new MEI client driver to support Late Binding firmware
> > upload/update for Intel discrete graphics platforms.
> >
> > Late Binding is a runtime firmware upload/update mechanism that allows
> > payloads, such as fan control and voltage regulator, to be securely
> > delivered and applied without requiring SPI flash updates or
> > system reboots. This driver enables the Xe graphics driver and other
> > user-space tools to push such firmware blobs to the authentication
> > firmware via the MEI interface.
> >
> > The driver handles authentication, versioning, and communication
> > with the authentication firmware, which in turn coordinates with
> > the PUnit/PCODE to apply the payload.
> >
> > This is a foundational component for enabling dynamic, secure,
> > and re-entrant configuration updates on platforms like Battlemage.
> >
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >
> > Changes in this revision:
> > - Proper commit message
> > - Proper explanation of 'Late Binding' on Kconfig help and doc
> > - Consistency in naming:
> > + mei_ prefix where it makes sense
> > + use 'lb' for short of 'Late Binding' instead of 'late_bind'
> > Including s/CONFIG_INTEL_MEI_LATE_BIND/CONFIG_INTEL_MEI_LB
> > + remove stray 'struct module'
> > + Fix structs and enum documentation style and fields
> > + Remove 'CSC' to avoid yet another acronym. 'Authentication firmware' it
> is.
> > + specify size unit
> > + s/push_config/push_payload
> >
> > drivers/misc/mei/Kconfig | 13 +
> > drivers/misc/mei/Makefile | 1 +
> > drivers/misc/mei/mei_lb.c | 315 +++++++++++++++++++++
> > include/drm/intel/i915_component.h | 1 +
> > include/drm/intel/intel_lb_mei_interface.h | 70 +++++
> > 5 files changed, 400 insertions(+)
> > create mode 100644 drivers/misc/mei/mei_lb.c
> > create mode 100644 include/drm/intel/intel_lb_mei_interface.h
> >
> > diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
> > index 7575fee96cc6..f8b04e49e4ba 100644
> > --- a/drivers/misc/mei/Kconfig
> > +++ b/drivers/misc/mei/Kconfig
> > @@ -81,6 +81,19 @@ config INTEL_MEI_VSC
> > This driver can also be built as a module. If so, the module
> > will be called mei-vsc.
> >
> > +config INTEL_MEI_LB
> > + tristate "Intel Late Binding (LB) support on ME Interface"
> > + depends on INTEL_MEI_ME
> > + depends on DRM_XE
> > + help
> > + Enable support for Intel Late Binding (LB) via the MEI interface.
> > +
> > + Late Binding is a method for applying firmware updates at runtime,
> > + allowing the Intel Xe driver to load firmware payloads such as
> > + fan controller or voltage regulator. These firmware updates are
> > + authenticated and versioned, and do not require firmware flashing
> > + or system reboot.
> > +
> > source "drivers/misc/mei/hdcp/Kconfig"
> > source "drivers/misc/mei/pxp/Kconfig"
> > source "drivers/misc/mei/gsc_proxy/Kconfig"
> > diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
> > index 6f9fdbf1a495..a203ed766b33 100644
> > --- a/drivers/misc/mei/Makefile
> > +++ b/drivers/misc/mei/Makefile
> > @@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src)
> > obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/
> > obj-$(CONFIG_INTEL_MEI_PXP) += pxp/
> > obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/
> > +obj-$(CONFIG_INTEL_MEI_LB) += mei_lb.o
> >
> > obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o
> > mei-vsc-hw-y := vsc-tp.o
> > diff --git a/drivers/misc/mei/mei_lb.c b/drivers/misc/mei/mei_lb.c
> > new file mode 100644
> > index 000000000000..fddef862712d
> > --- /dev/null
> > +++ b/drivers/misc/mei/mei_lb.c
> > @@ -0,0 +1,315 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2025 Intel Corporation
> > + */
> > +#include <drm/intel/i915_component.h>
> > +#include <drm/intel/intel_lb_mei_interface.h>
> > +#include <linux/component.h>
> > +#include <linux/pci.h>
> > +#include <linux/mei_cl_bus.h>
> > +#include <linux/module.h>
> > +#include <linux/overflow.h>
> > +#include <linux/slab.h>
> > +#include <linux/uuid.h>
> > +
> > +#include "mkhi.h"
> > +
> > +/**
> > + * DOC: Late Binding Firmware Update/Upload
> > + *
> > + * Late Binding is a firmware update/upload mechanism that allows
> configuration
> > + * payloads to be securely delivered and applied at runtime, rather than
> > + * being embedded in the system firmware image (e.g., IFWI or SPI flash).
> > + *
> > + * This mechanism is used to update device-level configuration such as:
> > + * - Fan controller
> > + * - Voltage regulator (VR)
> > + *
> > + * Key Characteristics:
> > + * ---------------------
> > + * - Runtime Delivery:
> > + * Firmware blobs are loaded by the host driver (e.g., Xe KMD)
> > + * after the GPU or SoC has booted.
> > + *
> > + * - Secure and Authenticated:
> > + * All payloads are signed and verified by the authentication firmware.
> > + *
> > + * - No Firmware Flashing Required:
> > + * Updates are applied in volatile memory and do not require SPI flash
> > + * modification or system reboot.
> > + *
> > + * - Re-entrant:
> > + * Multiple updates of the same or different types can be applied
> > + * sequentially within a single boot session.
> > + *
> > + * - Version Controlled:
> > + * Each payload includes version and security version number (SVN)
> > + * metadata to support anti-rollback enforcement.
> > + *
> > + * Upload Flow:
> > + * ------------
> > + * 1. Host driver (KMD or user-space tool) loads the late binding firmware.
> > + * 2. Firmware is passed to the MEI interface and forwarded to
> > + * authentication firmware.
> > + * 3. Authentication firmware authenticates the payload and extracts
> > + * command and data arrays.
> > + * 4. Authentication firmware delivers the configuration to PUnit/PCODE.
> > + * 5. Status is returned back to the host via MEI.
> > + */
> > +
> > +#define INTEL_LB_CMD 0x12
> > +#define INTEL_LB_RSP (INTEL_LB_CMD | 0x80)
> > +
> > +#define INTEL_LB_SEND_TIMEOUT_MSEC 3000
> > +#define INTEL_LB_RECV_TIMEOUT_MSEC 3000
> > +
> > +/**
> > + * struct mei_lb_req - Late Binding request structure
> > + * @header: MKHI message header (see struct mkhi_msg_hdr)
> > + * @type: Type of the Late Binding payload
> > + * @flags: Flags to be passed to the authentication firmware (e.g.
> %INTEL_LB_FLAGS_IS_PERSISTENT)
> > + * @reserved: Reserved for future use by authentication firmware, must be
> set to 0
> > + * @payload_size: Size of the payload data in bytes
> > + * @payload: Payload data to be sent to the authentication firmware
> > + */
> > +struct mei_lb_req {
> > + struct mkhi_msg_hdr header;
> > + __le32 type;
> > + __le32 flags;
> > + __le32 reserved[2];
> > + __le32 payload_size;
> > + u8 payload[] __counted_by(payload_size);
> > +} __packed;
> > +
> > +/**
> > + * struct mei_lb_rsp - Late Binding response structure
> > + * @header: MKHI message header (see struct mkhi_msg_hdr)
> > + * @type: Type of the Late Binding payload
> > + * @reserved: Reserved for future use by authentication firmware, must be
> set to 0
> > + * @status: Status returned by authentication firmware (see enum
> intel_lb_status)
> > + */
> > +struct mei_lb_rsp {
> > + struct mkhi_msg_hdr header;
> > + __le32 type;
> > + __le32 reserved[2];
> > + __le32 status;
> > +} __packed;
> > +
> > +static int mei_lb_check_response(const struct device *dev, const struct
> mkhi_msg_hdr *hdr)
> > +{
> > + if (hdr->group_id != MKHI_GROUP_ID_GFX) {
> > + dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n",
> > + hdr->group_id, MKHI_GROUP_ID_GFX);
> > + return -EINVAL;
> > + }
> > +
> > + if (hdr->command != INTEL_LB_RSP) {
> > + dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n",
> > + hdr->command, INTEL_LB_RSP);
> > + return -EINVAL;
> > + }
> > +
> > + if (hdr->result) {
> > + dev_err(dev, "Error in result: 0x%x\n", hdr->result);
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int mei_lb_push_payload(struct device *dev,
> > + enum intel_lb_type type, u32 flags,
> > + const void *payload, size_t payload_size)
> > +{
> > + struct mei_cl_device *cldev;
> > + struct mei_lb_req *req = NULL;
> > + struct mei_lb_rsp rsp;
> > + size_t req_size;
> > + ssize_t bytes;
> > + int ret;
> > +
> > + cldev = to_mei_cl_device(dev);
> > +
> > + ret = mei_cldev_enable(cldev);
> > + if (ret) {
> > + dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret);
> > + return ret;
> > + }
> > +
> > + req_size = struct_size(req, payload, payload_size);
> > + if (req_size > mei_cldev_mtu(cldev)) {
> > + dev_err(dev, "Payload is too big %zu\n", payload_size);
> > + ret = -EMSGSIZE;
> > + goto end;
> > + }
> > +
> > + req = kmalloc(req_size, GFP_KERNEL);
> > + if (!req) {
> > + ret = -ENOMEM;
> > + goto end;
> > + }
> > +
> > + req->header.group_id = MKHI_GROUP_ID_GFX;
> > + req->header.command = INTEL_LB_CMD;
> > + req->type = cpu_to_le32(type);
> > + req->flags = cpu_to_le32(flags);
> > + req->reserved[0] = 0;
> > + req->reserved[1] = 0;
> > + req->payload_size = cpu_to_le32(payload_size);
> > + memcpy(req->payload, payload, payload_size);
> > +
> > + bytes = mei_cldev_send_timeout(cldev,
> > + (void *)req, req_size,
> INTEL_LB_SEND_TIMEOUT_MSEC);
> > + if (bytes < 0) {
> > + dev_err(dev, "mei_cldev_send failed. %zd\n", bytes);
> > + ret = bytes;
> > + goto end;
> > + }
> > +
> > + bytes = mei_cldev_recv_timeout(cldev,
> > + (void *)&rsp, sizeof(rsp),
> INTEL_LB_RECV_TIMEOUT_MSEC);
> > + if (bytes < 0) {
> > + dev_err(dev, "mei_cldev_recv failed. %zd\n", bytes);
> > + ret = bytes;
> > + goto end;
> > + }
> > + if (bytes < sizeof(rsp.header)) {
> > + dev_err(dev, "bad response header from the firmware: size
> %zd < %zu\n",
> > + bytes, sizeof(rsp.header));
> > + ret = -EPROTO;
> > + goto end;
> > + }
> > + if (mei_lb_check_response(dev, &rsp.header)) {
> > + dev_err(dev, "bad result response from the firmware:
> 0x%x\n",
> > + *(uint32_t *)&rsp.header);
>
> What exactly are you printing out to userspace here? A pointer? Or a
> random value from the firmware? Why?
We've checked this data for validity and check is failed.
Sometimes whole header needed to debug the issue, so we are printing it here.
>
> > + ret = -EPROTO;
> > + goto end;
> > + }
>
> You forgot to check the type and reserved fields of the rsp structure :(
You are right, better to check type too.
Should reserved bits be checked for 0? Or it will be overkill?
>
> > + if (bytes < sizeof(rsp)) {
> > + dev_err(dev, "bad response from the firmware: size %zd <
> %zu\n",
> > + bytes, sizeof(rsp));
> > + ret = -EPROTO;
> > + goto end;
> > + }
>
> Why not check this above when you check against the size of the header?
> You only need one size check, not 2.
Firmware may return only header with result field set without the data.
We are parsing the header first and then starting to parse data.
If we check for whole message size at the beginning we'll miss the result data.
- -
Thanks,
Sasha
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/9] mei: late_bind: add late binding component driver
2025-07-16 11:58 ` Usyskin, Alexander
@ 2025-07-16 12:07 ` Greg KH
2025-07-16 14:26 ` Usyskin, Alexander
0 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2025-07-16 12:07 UTC (permalink / raw)
To: Usyskin, Alexander
Cc: Vivi, Rodrigo, intel-xe@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
Ceraolo Spurio, Daniele, Gupta, Anshuman, Nilawar, Badal
On Wed, Jul 16, 2025 at 11:58:19AM +0000, Usyskin, Alexander wrote:
> > Subject: Re: [PATCH 2/9] mei: late_bind: add late binding component driver
> >
> > On Thu, Jul 10, 2025 at 11:08:33AM -0400, Rodrigo Vivi wrote:
> > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > >
> > > Introduce a new MEI client driver to support Late Binding firmware
> > > upload/update for Intel discrete graphics platforms.
> > >
> > > Late Binding is a runtime firmware upload/update mechanism that allows
> > > payloads, such as fan control and voltage regulator, to be securely
> > > delivered and applied without requiring SPI flash updates or
> > > system reboots. This driver enables the Xe graphics driver and other
> > > user-space tools to push such firmware blobs to the authentication
> > > firmware via the MEI interface.
> > >
> > > The driver handles authentication, versioning, and communication
> > > with the authentication firmware, which in turn coordinates with
> > > the PUnit/PCODE to apply the payload.
> > >
> > > This is a foundational component for enabling dynamic, secure,
> > > and re-entrant configuration updates on platforms like Battlemage.
> > >
> > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > > Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >
> > > Changes in this revision:
> > > - Proper commit message
> > > - Proper explanation of 'Late Binding' on Kconfig help and doc
> > > - Consistency in naming:
> > > + mei_ prefix where it makes sense
> > > + use 'lb' for short of 'Late Binding' instead of 'late_bind'
> > > Including s/CONFIG_INTEL_MEI_LATE_BIND/CONFIG_INTEL_MEI_LB
> > > + remove stray 'struct module'
> > > + Fix structs and enum documentation style and fields
> > > + Remove 'CSC' to avoid yet another acronym. 'Authentication firmware' it
> > is.
> > > + specify size unit
> > > + s/push_config/push_payload
> > >
> > > drivers/misc/mei/Kconfig | 13 +
> > > drivers/misc/mei/Makefile | 1 +
> > > drivers/misc/mei/mei_lb.c | 315 +++++++++++++++++++++
> > > include/drm/intel/i915_component.h | 1 +
> > > include/drm/intel/intel_lb_mei_interface.h | 70 +++++
> > > 5 files changed, 400 insertions(+)
> > > create mode 100644 drivers/misc/mei/mei_lb.c
> > > create mode 100644 include/drm/intel/intel_lb_mei_interface.h
> > >
> > > diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
> > > index 7575fee96cc6..f8b04e49e4ba 100644
> > > --- a/drivers/misc/mei/Kconfig
> > > +++ b/drivers/misc/mei/Kconfig
> > > @@ -81,6 +81,19 @@ config INTEL_MEI_VSC
> > > This driver can also be built as a module. If so, the module
> > > will be called mei-vsc.
> > >
> > > +config INTEL_MEI_LB
> > > + tristate "Intel Late Binding (LB) support on ME Interface"
> > > + depends on INTEL_MEI_ME
> > > + depends on DRM_XE
> > > + help
> > > + Enable support for Intel Late Binding (LB) via the MEI interface.
> > > +
> > > + Late Binding is a method for applying firmware updates at runtime,
> > > + allowing the Intel Xe driver to load firmware payloads such as
> > > + fan controller or voltage regulator. These firmware updates are
> > > + authenticated and versioned, and do not require firmware flashing
> > > + or system reboot.
> > > +
> > > source "drivers/misc/mei/hdcp/Kconfig"
> > > source "drivers/misc/mei/pxp/Kconfig"
> > > source "drivers/misc/mei/gsc_proxy/Kconfig"
> > > diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
> > > index 6f9fdbf1a495..a203ed766b33 100644
> > > --- a/drivers/misc/mei/Makefile
> > > +++ b/drivers/misc/mei/Makefile
> > > @@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src)
> > > obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/
> > > obj-$(CONFIG_INTEL_MEI_PXP) += pxp/
> > > obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/
> > > +obj-$(CONFIG_INTEL_MEI_LB) += mei_lb.o
> > >
> > > obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o
> > > mei-vsc-hw-y := vsc-tp.o
> > > diff --git a/drivers/misc/mei/mei_lb.c b/drivers/misc/mei/mei_lb.c
> > > new file mode 100644
> > > index 000000000000..fddef862712d
> > > --- /dev/null
> > > +++ b/drivers/misc/mei/mei_lb.c
> > > @@ -0,0 +1,315 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2025 Intel Corporation
> > > + */
> > > +#include <drm/intel/i915_component.h>
> > > +#include <drm/intel/intel_lb_mei_interface.h>
> > > +#include <linux/component.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/mei_cl_bus.h>
> > > +#include <linux/module.h>
> > > +#include <linux/overflow.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/uuid.h>
> > > +
> > > +#include "mkhi.h"
> > > +
> > > +/**
> > > + * DOC: Late Binding Firmware Update/Upload
> > > + *
> > > + * Late Binding is a firmware update/upload mechanism that allows
> > configuration
> > > + * payloads to be securely delivered and applied at runtime, rather than
> > > + * being embedded in the system firmware image (e.g., IFWI or SPI flash).
> > > + *
> > > + * This mechanism is used to update device-level configuration such as:
> > > + * - Fan controller
> > > + * - Voltage regulator (VR)
> > > + *
> > > + * Key Characteristics:
> > > + * ---------------------
> > > + * - Runtime Delivery:
> > > + * Firmware blobs are loaded by the host driver (e.g., Xe KMD)
> > > + * after the GPU or SoC has booted.
> > > + *
> > > + * - Secure and Authenticated:
> > > + * All payloads are signed and verified by the authentication firmware.
> > > + *
> > > + * - No Firmware Flashing Required:
> > > + * Updates are applied in volatile memory and do not require SPI flash
> > > + * modification or system reboot.
> > > + *
> > > + * - Re-entrant:
> > > + * Multiple updates of the same or different types can be applied
> > > + * sequentially within a single boot session.
> > > + *
> > > + * - Version Controlled:
> > > + * Each payload includes version and security version number (SVN)
> > > + * metadata to support anti-rollback enforcement.
> > > + *
> > > + * Upload Flow:
> > > + * ------------
> > > + * 1. Host driver (KMD or user-space tool) loads the late binding firmware.
> > > + * 2. Firmware is passed to the MEI interface and forwarded to
> > > + * authentication firmware.
> > > + * 3. Authentication firmware authenticates the payload and extracts
> > > + * command and data arrays.
> > > + * 4. Authentication firmware delivers the configuration to PUnit/PCODE.
> > > + * 5. Status is returned back to the host via MEI.
> > > + */
> > > +
> > > +#define INTEL_LB_CMD 0x12
> > > +#define INTEL_LB_RSP (INTEL_LB_CMD | 0x80)
> > > +
> > > +#define INTEL_LB_SEND_TIMEOUT_MSEC 3000
> > > +#define INTEL_LB_RECV_TIMEOUT_MSEC 3000
> > > +
> > > +/**
> > > + * struct mei_lb_req - Late Binding request structure
> > > + * @header: MKHI message header (see struct mkhi_msg_hdr)
> > > + * @type: Type of the Late Binding payload
> > > + * @flags: Flags to be passed to the authentication firmware (e.g.
> > %INTEL_LB_FLAGS_IS_PERSISTENT)
> > > + * @reserved: Reserved for future use by authentication firmware, must be
> > set to 0
> > > + * @payload_size: Size of the payload data in bytes
> > > + * @payload: Payload data to be sent to the authentication firmware
> > > + */
> > > +struct mei_lb_req {
> > > + struct mkhi_msg_hdr header;
> > > + __le32 type;
> > > + __le32 flags;
> > > + __le32 reserved[2];
> > > + __le32 payload_size;
> > > + u8 payload[] __counted_by(payload_size);
> > > +} __packed;
> > > +
> > > +/**
> > > + * struct mei_lb_rsp - Late Binding response structure
> > > + * @header: MKHI message header (see struct mkhi_msg_hdr)
> > > + * @type: Type of the Late Binding payload
> > > + * @reserved: Reserved for future use by authentication firmware, must be
> > set to 0
> > > + * @status: Status returned by authentication firmware (see enum
> > intel_lb_status)
> > > + */
> > > +struct mei_lb_rsp {
> > > + struct mkhi_msg_hdr header;
> > > + __le32 type;
> > > + __le32 reserved[2];
> > > + __le32 status;
> > > +} __packed;
> > > +
> > > +static int mei_lb_check_response(const struct device *dev, const struct
> > mkhi_msg_hdr *hdr)
> > > +{
> > > + if (hdr->group_id != MKHI_GROUP_ID_GFX) {
> > > + dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n",
> > > + hdr->group_id, MKHI_GROUP_ID_GFX);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + if (hdr->command != INTEL_LB_RSP) {
> > > + dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n",
> > > + hdr->command, INTEL_LB_RSP);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + if (hdr->result) {
> > > + dev_err(dev, "Error in result: 0x%x\n", hdr->result);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int mei_lb_push_payload(struct device *dev,
> > > + enum intel_lb_type type, u32 flags,
> > > + const void *payload, size_t payload_size)
> > > +{
> > > + struct mei_cl_device *cldev;
> > > + struct mei_lb_req *req = NULL;
> > > + struct mei_lb_rsp rsp;
> > > + size_t req_size;
> > > + ssize_t bytes;
> > > + int ret;
> > > +
> > > + cldev = to_mei_cl_device(dev);
> > > +
> > > + ret = mei_cldev_enable(cldev);
> > > + if (ret) {
> > > + dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + req_size = struct_size(req, payload, payload_size);
> > > + if (req_size > mei_cldev_mtu(cldev)) {
> > > + dev_err(dev, "Payload is too big %zu\n", payload_size);
> > > + ret = -EMSGSIZE;
> > > + goto end;
> > > + }
> > > +
> > > + req = kmalloc(req_size, GFP_KERNEL);
> > > + if (!req) {
> > > + ret = -ENOMEM;
> > > + goto end;
> > > + }
> > > +
> > > + req->header.group_id = MKHI_GROUP_ID_GFX;
> > > + req->header.command = INTEL_LB_CMD;
> > > + req->type = cpu_to_le32(type);
> > > + req->flags = cpu_to_le32(flags);
> > > + req->reserved[0] = 0;
> > > + req->reserved[1] = 0;
> > > + req->payload_size = cpu_to_le32(payload_size);
> > > + memcpy(req->payload, payload, payload_size);
> > > +
> > > + bytes = mei_cldev_send_timeout(cldev,
> > > + (void *)req, req_size,
> > INTEL_LB_SEND_TIMEOUT_MSEC);
> > > + if (bytes < 0) {
> > > + dev_err(dev, "mei_cldev_send failed. %zd\n", bytes);
> > > + ret = bytes;
> > > + goto end;
> > > + }
> > > +
> > > + bytes = mei_cldev_recv_timeout(cldev,
> > > + (void *)&rsp, sizeof(rsp),
> > INTEL_LB_RECV_TIMEOUT_MSEC);
> > > + if (bytes < 0) {
> > > + dev_err(dev, "mei_cldev_recv failed. %zd\n", bytes);
> > > + ret = bytes;
> > > + goto end;
> > > + }
> > > + if (bytes < sizeof(rsp.header)) {
> > > + dev_err(dev, "bad response header from the firmware: size
> > %zd < %zu\n",
> > > + bytes, sizeof(rsp.header));
> > > + ret = -EPROTO;
> > > + goto end;
> > > + }
> > > + if (mei_lb_check_response(dev, &rsp.header)) {
> > > + dev_err(dev, "bad result response from the firmware:
> > 0x%x\n",
> > > + *(uint32_t *)&rsp.header);
> >
> > What exactly are you printing out to userspace here? A pointer? Or a
> > random value from the firmware? Why?
> We've checked this data for validity and check is failed.
> Sometimes whole header needed to debug the issue, so we are printing it here.
>
> >
> > > + ret = -EPROTO;
> > > + goto end;
> > > + }
> >
> > You forgot to check the type and reserved fields of the rsp structure :(
> You are right, better to check type too.
> Should reserved bits be checked for 0? Or it will be overkill?
The reserved bytes HAVE TO BE CHECKED, otherwise you can NEVER use them
in the future. You say so in your documentation for the structure :)
> > > + if (bytes < sizeof(rsp)) {
> > > + dev_err(dev, "bad response from the firmware: size %zd <
> > %zu\n",
> > > + bytes, sizeof(rsp));
> > > + ret = -EPROTO;
> > > + goto end;
> > > + }
> >
> > Why not check this above when you check against the size of the header?
> > You only need one size check, not 2.
> Firmware may return only header with result field set without the data.
Then the firmware is broken :)
> We are parsing the header first and then starting to parse data.
> If we check for whole message size at the beginning we'll miss the result data.
You mean you will make it harder to debug the firmware, as you will not
be printing out the header information? Or something else? The
bytes variable HAS to match the full structure size, not just the header
size, according to this code. So just test for that and be done with
it!
thanks,
greg k-h
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH 2/9] mei: late_bind: add late binding component driver
2025-07-16 12:07 ` Greg KH
@ 2025-07-16 14:26 ` Usyskin, Alexander
2025-07-16 14:45 ` Greg KH
0 siblings, 1 reply; 22+ messages in thread
From: Usyskin, Alexander @ 2025-07-16 14:26 UTC (permalink / raw)
To: Greg KH
Cc: Vivi, Rodrigo, intel-xe@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
Ceraolo Spurio, Daniele, Gupta, Anshuman, Nilawar, Badal
> Subject: Re: [PATCH 2/9] mei: late_bind: add late binding component driver
>
> On Wed, Jul 16, 2025 at 11:58:19AM +0000, Usyskin, Alexander wrote:
> > > Subject: Re: [PATCH 2/9] mei: late_bind: add late binding component driver
> > >
> > > On Thu, Jul 10, 2025 at 11:08:33AM -0400, Rodrigo Vivi wrote:
> > > > From: Alexander Usyskin <alexander.usyskin@intel.com>
> > > >
> > > > Introduce a new MEI client driver to support Late Binding firmware
> > > > upload/update for Intel discrete graphics platforms.
> > > >
> > > > Late Binding is a runtime firmware upload/update mechanism that allows
> > > > payloads, such as fan control and voltage regulator, to be securely
> > > > delivered and applied without requiring SPI flash updates or
> > > > system reboots. This driver enables the Xe graphics driver and other
> > > > user-space tools to push such firmware blobs to the authentication
> > > > firmware via the MEI interface.
> > > >
> > > > The driver handles authentication, versioning, and communication
> > > > with the authentication firmware, which in turn coordinates with
> > > > the PUnit/PCODE to apply the payload.
> > > >
> > > > This is a foundational component for enabling dynamic, secure,
> > > > and re-entrant configuration updates on platforms like Battlemage.
> > > >
> > > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > > > Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > ---
> > > >
> > > > Changes in this revision:
> > > > - Proper commit message
> > > > - Proper explanation of 'Late Binding' on Kconfig help and doc
> > > > - Consistency in naming:
> > > > + mei_ prefix where it makes sense
> > > > + use 'lb' for short of 'Late Binding' instead of 'late_bind'
> > > > Including s/CONFIG_INTEL_MEI_LATE_BIND/CONFIG_INTEL_MEI_LB
> > > > + remove stray 'struct module'
> > > > + Fix structs and enum documentation style and fields
> > > > + Remove 'CSC' to avoid yet another acronym. 'Authentication firmware'
> it
> > > is.
> > > > + specify size unit
> > > > + s/push_config/push_payload
> > > >
> > > > drivers/misc/mei/Kconfig | 13 +
> > > > drivers/misc/mei/Makefile | 1 +
> > > > drivers/misc/mei/mei_lb.c | 315 +++++++++++++++++++++
> > > > include/drm/intel/i915_component.h | 1 +
> > > > include/drm/intel/intel_lb_mei_interface.h | 70 +++++
> > > > 5 files changed, 400 insertions(+)
> > > > create mode 100644 drivers/misc/mei/mei_lb.c
> > > > create mode 100644 include/drm/intel/intel_lb_mei_interface.h
> > > >
> > > > diff --git a/drivers/misc/mei/Kconfig b/drivers/misc/mei/Kconfig
> > > > index 7575fee96cc6..f8b04e49e4ba 100644
> > > > --- a/drivers/misc/mei/Kconfig
> > > > +++ b/drivers/misc/mei/Kconfig
> > > > @@ -81,6 +81,19 @@ config INTEL_MEI_VSC
> > > > This driver can also be built as a module. If so, the module
> > > > will be called mei-vsc.
> > > >
> > > > +config INTEL_MEI_LB
> > > > + tristate "Intel Late Binding (LB) support on ME Interface"
> > > > + depends on INTEL_MEI_ME
> > > > + depends on DRM_XE
> > > > + help
> > > > + Enable support for Intel Late Binding (LB) via the MEI interface.
> > > > +
> > > > + Late Binding is a method for applying firmware updates at runtime,
> > > > + allowing the Intel Xe driver to load firmware payloads such as
> > > > + fan controller or voltage regulator. These firmware updates are
> > > > + authenticated and versioned, and do not require firmware flashing
> > > > + or system reboot.
> > > > +
> > > > source "drivers/misc/mei/hdcp/Kconfig"
> > > > source "drivers/misc/mei/pxp/Kconfig"
> > > > source "drivers/misc/mei/gsc_proxy/Kconfig"
> > > > diff --git a/drivers/misc/mei/Makefile b/drivers/misc/mei/Makefile
> > > > index 6f9fdbf1a495..a203ed766b33 100644
> > > > --- a/drivers/misc/mei/Makefile
> > > > +++ b/drivers/misc/mei/Makefile
> > > > @@ -31,6 +31,7 @@ CFLAGS_mei-trace.o = -I$(src)
> > > > obj-$(CONFIG_INTEL_MEI_HDCP) += hdcp/
> > > > obj-$(CONFIG_INTEL_MEI_PXP) += pxp/
> > > > obj-$(CONFIG_INTEL_MEI_GSC_PROXY) += gsc_proxy/
> > > > +obj-$(CONFIG_INTEL_MEI_LB) += mei_lb.o
> > > >
> > > > obj-$(CONFIG_INTEL_MEI_VSC_HW) += mei-vsc-hw.o
> > > > mei-vsc-hw-y := vsc-tp.o
> > > > diff --git a/drivers/misc/mei/mei_lb.c b/drivers/misc/mei/mei_lb.c
> > > > new file mode 100644
> > > > index 000000000000..fddef862712d
> > > > --- /dev/null
> > > > +++ b/drivers/misc/mei/mei_lb.c
> > > > @@ -0,0 +1,315 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2025 Intel Corporation
> > > > + */
> > > > +#include <drm/intel/i915_component.h>
> > > > +#include <drm/intel/intel_lb_mei_interface.h>
> > > > +#include <linux/component.h>
> > > > +#include <linux/pci.h>
> > > > +#include <linux/mei_cl_bus.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/overflow.h>
> > > > +#include <linux/slab.h>
> > > > +#include <linux/uuid.h>
> > > > +
> > > > +#include "mkhi.h"
> > > > +
> > > > +/**
> > > > + * DOC: Late Binding Firmware Update/Upload
> > > > + *
> > > > + * Late Binding is a firmware update/upload mechanism that allows
> > > configuration
> > > > + * payloads to be securely delivered and applied at runtime, rather than
> > > > + * being embedded in the system firmware image (e.g., IFWI or SPI
> flash).
> > > > + *
> > > > + * This mechanism is used to update device-level configuration such as:
> > > > + * - Fan controller
> > > > + * - Voltage regulator (VR)
> > > > + *
> > > > + * Key Characteristics:
> > > > + * ---------------------
> > > > + * - Runtime Delivery:
> > > > + * Firmware blobs are loaded by the host driver (e.g., Xe KMD)
> > > > + * after the GPU or SoC has booted.
> > > > + *
> > > > + * - Secure and Authenticated:
> > > > + * All payloads are signed and verified by the authentication firmware.
> > > > + *
> > > > + * - No Firmware Flashing Required:
> > > > + * Updates are applied in volatile memory and do not require SPI flash
> > > > + * modification or system reboot.
> > > > + *
> > > > + * - Re-entrant:
> > > > + * Multiple updates of the same or different types can be applied
> > > > + * sequentially within a single boot session.
> > > > + *
> > > > + * - Version Controlled:
> > > > + * Each payload includes version and security version number (SVN)
> > > > + * metadata to support anti-rollback enforcement.
> > > > + *
> > > > + * Upload Flow:
> > > > + * ------------
> > > > + * 1. Host driver (KMD or user-space tool) loads the late binding
> firmware.
> > > > + * 2. Firmware is passed to the MEI interface and forwarded to
> > > > + * authentication firmware.
> > > > + * 3. Authentication firmware authenticates the payload and extracts
> > > > + * command and data arrays.
> > > > + * 4. Authentication firmware delivers the configuration to
> PUnit/PCODE.
> > > > + * 5. Status is returned back to the host via MEI.
> > > > + */
> > > > +
> > > > +#define INTEL_LB_CMD 0x12
> > > > +#define INTEL_LB_RSP (INTEL_LB_CMD | 0x80)
> > > > +
> > > > +#define INTEL_LB_SEND_TIMEOUT_MSEC 3000
> > > > +#define INTEL_LB_RECV_TIMEOUT_MSEC 3000
> > > > +
> > > > +/**
> > > > + * struct mei_lb_req - Late Binding request structure
> > > > + * @header: MKHI message header (see struct mkhi_msg_hdr)
> > > > + * @type: Type of the Late Binding payload
> > > > + * @flags: Flags to be passed to the authentication firmware (e.g.
> > > %INTEL_LB_FLAGS_IS_PERSISTENT)
> > > > + * @reserved: Reserved for future use by authentication firmware, must
> be
> > > set to 0
> > > > + * @payload_size: Size of the payload data in bytes
> > > > + * @payload: Payload data to be sent to the authentication firmware
> > > > + */
> > > > +struct mei_lb_req {
> > > > + struct mkhi_msg_hdr header;
> > > > + __le32 type;
> > > > + __le32 flags;
> > > > + __le32 reserved[2];
> > > > + __le32 payload_size;
> > > > + u8 payload[] __counted_by(payload_size);
> > > > +} __packed;
> > > > +
> > > > +/**
> > > > + * struct mei_lb_rsp - Late Binding response structure
> > > > + * @header: MKHI message header (see struct mkhi_msg_hdr)
> > > > + * @type: Type of the Late Binding payload
> > > > + * @reserved: Reserved for future use by authentication firmware, must
> be
> > > set to 0
> > > > + * @status: Status returned by authentication firmware (see enum
> > > intel_lb_status)
> > > > + */
> > > > +struct mei_lb_rsp {
> > > > + struct mkhi_msg_hdr header;
> > > > + __le32 type;
> > > > + __le32 reserved[2];
> > > > + __le32 status;
> > > > +} __packed;
> > > > +
> > > > +static int mei_lb_check_response(const struct device *dev, const struct
> > > mkhi_msg_hdr *hdr)
> > > > +{
> > > > + if (hdr->group_id != MKHI_GROUP_ID_GFX) {
> > > > + dev_err(dev, "Mismatch group id: 0x%x instead of 0x%x\n",
> > > > + hdr->group_id, MKHI_GROUP_ID_GFX);
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + if (hdr->command != INTEL_LB_RSP) {
> > > > + dev_err(dev, "Mismatch command: 0x%x instead of 0x%x\n",
> > > > + hdr->command, INTEL_LB_RSP);
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + if (hdr->result) {
> > > > + dev_err(dev, "Error in result: 0x%x\n", hdr->result);
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int mei_lb_push_payload(struct device *dev,
> > > > + enum intel_lb_type type, u32 flags,
> > > > + const void *payload, size_t payload_size)
> > > > +{
> > > > + struct mei_cl_device *cldev;
> > > > + struct mei_lb_req *req = NULL;
> > > > + struct mei_lb_rsp rsp;
> > > > + size_t req_size;
> > > > + ssize_t bytes;
> > > > + int ret;
> > > > +
> > > > + cldev = to_mei_cl_device(dev);
> > > > +
> > > > + ret = mei_cldev_enable(cldev);
> > > > + if (ret) {
> > > > + dev_dbg(dev, "mei_cldev_enable failed. %d\n", ret);
> > > > + return ret;
> > > > + }
> > > > +
> > > > + req_size = struct_size(req, payload, payload_size);
> > > > + if (req_size > mei_cldev_mtu(cldev)) {
> > > > + dev_err(dev, "Payload is too big %zu\n", payload_size);
> > > > + ret = -EMSGSIZE;
> > > > + goto end;
> > > > + }
> > > > +
> > > > + req = kmalloc(req_size, GFP_KERNEL);
> > > > + if (!req) {
> > > > + ret = -ENOMEM;
> > > > + goto end;
> > > > + }
> > > > +
> > > > + req->header.group_id = MKHI_GROUP_ID_GFX;
> > > > + req->header.command = INTEL_LB_CMD;
> > > > + req->type = cpu_to_le32(type);
> > > > + req->flags = cpu_to_le32(flags);
> > > > + req->reserved[0] = 0;
> > > > + req->reserved[1] = 0;
> > > > + req->payload_size = cpu_to_le32(payload_size);
> > > > + memcpy(req->payload, payload, payload_size);
> > > > +
> > > > + bytes = mei_cldev_send_timeout(cldev,
> > > > + (void *)req, req_size,
> > > INTEL_LB_SEND_TIMEOUT_MSEC);
> > > > + if (bytes < 0) {
> > > > + dev_err(dev, "mei_cldev_send failed. %zd\n", bytes);
> > > > + ret = bytes;
> > > > + goto end;
> > > > + }
> > > > +
> > > > + bytes = mei_cldev_recv_timeout(cldev,
> > > > + (void *)&rsp, sizeof(rsp),
> > > INTEL_LB_RECV_TIMEOUT_MSEC);
> > > > + if (bytes < 0) {
> > > > + dev_err(dev, "mei_cldev_recv failed. %zd\n", bytes);
> > > > + ret = bytes;
> > > > + goto end;
> > > > + }
> > > > + if (bytes < sizeof(rsp.header)) {
> > > > + dev_err(dev, "bad response header from the firmware: size
> > > %zd < %zu\n",
> > > > + bytes, sizeof(rsp.header));
> > > > + ret = -EPROTO;
> > > > + goto end;
> > > > + }
> > > > + if (mei_lb_check_response(dev, &rsp.header)) {
> > > > + dev_err(dev, "bad result response from the firmware:
> > > 0x%x\n",
> > > > + *(uint32_t *)&rsp.header);
> > >
> > > What exactly are you printing out to userspace here? A pointer? Or a
> > > random value from the firmware? Why?
> > We've checked this data for validity and check is failed.
> > Sometimes whole header needed to debug the issue, so we are printing it
> here.
> >
> > >
> > > > + ret = -EPROTO;
> > > > + goto end;
> > > > + }
> > >
> > > You forgot to check the type and reserved fields of the rsp structure :(
> > You are right, better to check type too.
> > Should reserved bits be checked for 0? Or it will be overkill?
>
> The reserved bytes HAVE TO BE CHECKED, otherwise you can NEVER use them
> in the future. You say so in your documentation for the structure :)
>
> > > > + if (bytes < sizeof(rsp)) {
> > > > + dev_err(dev, "bad response from the firmware: size %zd <
> > > %zu\n",
> > > > + bytes, sizeof(rsp));
> > > > + ret = -EPROTO;
> > > > + goto end;
> > > > + }
> > >
> > > Why not check this above when you check against the size of the header?
> > > You only need one size check, not 2.
> > Firmware may return only header with result field set without the data.
>
> Then the firmware is broken :)
>
> > We are parsing the header first and then starting to parse data.
> > If we check for whole message size at the beginning we'll miss the result
> data.
>
> You mean you will make it harder to debug the firmware, as you will not
> be printing out the header information? Or something else? The
> bytes variable HAS to match the full structure size, not just the header
> size, according to this code. So just test for that and be done with
> it!
The CSME firmware returns only command header if, like, command is not recognised.
This may happen because of firmware bug or for firmware is configured/compiled
that way.
This seems reasonable for the complex protocols where firmware may not be
aware of this particular command at all and have no idea what the data size it
should send in reply.
Printing result from the header will allow us to understand either this is the firmware
problem or driver sent something wrong.
- -
Thanks,
Sasha
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/9] mei: late_bind: add late binding component driver
2025-07-16 14:26 ` Usyskin, Alexander
@ 2025-07-16 14:45 ` Greg KH
2025-07-27 14:46 ` Usyskin, Alexander
0 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2025-07-16 14:45 UTC (permalink / raw)
To: Usyskin, Alexander
Cc: Vivi, Rodrigo, intel-xe@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
Ceraolo Spurio, Daniele, Gupta, Anshuman, Nilawar, Badal
On Wed, Jul 16, 2025 at 02:26:26PM +0000, Usyskin, Alexander wrote:
> > > > > + if (bytes < sizeof(rsp)) {
> > > > > + dev_err(dev, "bad response from the firmware: size %zd <
> > > > %zu\n",
> > > > > + bytes, sizeof(rsp));
> > > > > + ret = -EPROTO;
> > > > > + goto end;
> > > > > + }
> > > >
> > > > Why not check this above when you check against the size of the header?
> > > > You only need one size check, not 2.
> > > Firmware may return only header with result field set without the data.
> >
> > Then the firmware is broken :)
> >
> > > We are parsing the header first and then starting to parse data.
> > > If we check for whole message size at the beginning we'll miss the result
> > data.
> >
> > You mean you will make it harder to debug the firmware, as you will not
> > be printing out the header information? Or something else? The
> > bytes variable HAS to match the full structure size, not just the header
> > size, according to this code. So just test for that and be done with
> > it!
>
> The CSME firmware returns only command header if, like, command is not recognised.
> This may happen because of firmware bug or for firmware is configured/compiled
> that way.
> This seems reasonable for the complex protocols where firmware may not be
> aware of this particular command at all and have no idea what the data size it
> should send in reply.
> Printing result from the header will allow us to understand either this is the firmware
> problem or driver sent something wrong.
Then make it obvious in your checking and in your error messages as to
what you are doing here. Checking the size of the buffer in two
different places, with different values is very odd, and deserves a lot
of explaination.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/9] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw
2025-07-10 15:08 ` [PATCH 3/9] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw Rodrigo Vivi
@ 2025-07-25 21:38 ` Lucas De Marchi
2025-07-30 11:47 ` Nilawar, Badal
0 siblings, 1 reply; 22+ messages in thread
From: Lucas De Marchi @ 2025-07-25 21:38 UTC (permalink / raw)
To: Rodrigo Vivi
Cc: intel-xe, dri-devel, linux-kernel, gregkh, daniele.ceraolospurio,
anshuman.gupta, alexander.usyskin, Badal Nilawar
On Thu, Jul 10, 2025 at 11:08:34AM -0400, Rodrigo Vivi wrote:
>From: Badal Nilawar <badal.nilawar@intel.com>
>
>Introducing xe_late_bind_fw to enable firmware loading for the devices,
here and in the subject: no gerund, please.
>such as the fan controller, during the driver probe. Typically,
>firmware for such devices are part of IFWI flash image but can be
>replaced at probe after OEM tuning.
>This patch binds mei late binding component to enable firmware loading.
>
>v2:
> - Add devm_add_action_or_reset to remove the component (Daniele)
> - Add INTEL_MEI_GSC check in xe_late_bind_init() (Daniele)
>v3:
> - Fail driver probe if late bind initialization fails,
> add has_late_bind flag (Daniele)
>v4:
> - %S/I915_COMPONENT_LATE_BIND/INTEL_COMPONENT_LATE_BIND/
>v6:
> - rebased
>
>Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/xe/Makefile | 1 +
> drivers/gpu/drm/xe/xe_device.c | 5 ++
> drivers/gpu/drm/xe/xe_device_types.h | 6 ++
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 84 ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_late_bind_fw.h | 15 ++++
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 33 +++++++++
> drivers/gpu/drm/xe/xe_pci.c | 2 +
> drivers/gpu/drm/xe/xe_pci_types.h | 1 +
> 8 files changed, 147 insertions(+)
> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.c
> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.h
> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>
>diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>index 1d97e5b63f4e..97e2b1368a6e 100644
>--- a/drivers/gpu/drm/xe/Makefile
>+++ b/drivers/gpu/drm/xe/Makefile
>@@ -76,6 +76,7 @@ xe-y += xe_bb.o \
> xe_hw_fence.o \
> xe_irq.o \
> xe_lrc.o \
>+ xe_late_bind_fw.o \
almost there, but still not sorted as it should be
> xe_migrate.o \
> xe_mmio.o \
> xe_mocs.o \
>diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>index 0b73cb72bad1..cb595bae5f55 100644
>--- a/drivers/gpu/drm/xe/xe_device.c
>+++ b/drivers/gpu/drm/xe/xe_device.c
>@@ -44,6 +44,7 @@
> #include "xe_hw_engine_group.h"
> #include "xe_hwmon.h"
> #include "xe_irq.h"
>+#include "xe_late_bind_fw.h"
> #include "xe_mmio.h"
> #include "xe_module.h"
> #include "xe_nvm.h"
>@@ -866,6 +867,10 @@ int xe_device_probe(struct xe_device *xe)
> if (err)
> return err;
>
>+ err = xe_late_bind_init(&xe->late_bind);
>+ if (err && err != -ENODEV)
let's not be creative in xe_device_probe(). Boring is good.
All the other parts in this function handle ENODEV internally. It's up
to them to decide if ENODEV -> return 0, not here.
>+ return err;
>+
> err = xe_oa_init(xe);
> if (err)
> return err;
>diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>index 78c4acafd268..a8891833f980 100644
>--- a/drivers/gpu/drm/xe/xe_device_types.h
>+++ b/drivers/gpu/drm/xe/xe_device_types.h
>@@ -16,6 +16,7 @@
> #include "xe_devcoredump_types.h"
> #include "xe_heci_gsc.h"
> #include "xe_lmtt_types.h"
>+#include "xe_late_bind_fw_types.h"
should be sorted
> #include "xe_memirq_types.h"
> #include "xe_oa_types.h"
> #include "xe_platform_types.h"
>@@ -325,6 +326,8 @@ struct xe_device {
> u8 has_heci_cscfi:1;
> /** @info.has_heci_gscfi: device has heci gscfi */
> u8 has_heci_gscfi:1;
>+ /** @info.has_late_bind: Device has firmware late binding support */
>+ u8 has_late_bind:1;
> /** @info.has_llc: Device has a shared CPU+GPU last level cache */
> u8 has_llc:1;
> /** @info.has_mbx_power_limits: Device has support to manage power limits using
>@@ -557,6 +560,9 @@ struct xe_device {
> /** @nvm: discrete graphics non-volatile memory */
> struct intel_dg_nvm_dev *nvm;
>
>+ /** @late_bind: xe mei late bind interface */
>+ struct xe_late_bind late_bind;
>+
> /** @oa: oa observation subsystem */
> struct xe_oa oa;
>
>diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>new file mode 100644
>index 000000000000..152f3e58de94
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>@@ -0,0 +1,84 @@
>+// SPDX-License-Identifier: MIT
>+/*
>+ * Copyright © 2025 Intel Corporation
>+ */
>+
>+#include <linux/component.h>
>+#include <linux/delay.h>
>+
>+#include <drm/drm_managed.h>
>+#include <drm/intel/i915_component.h>
>+#include <drm/intel/intel_lb_mei_interface.h>
>+#include <drm/drm_print.h>
>+
>+#include "xe_device.h"
>+#include "xe_late_bind_fw.h"
>+
>+static struct xe_device *
>+late_bind_to_xe(struct xe_late_bind *late_bind)
>+{
>+ return container_of(late_bind, struct xe_device, late_bind);
>+}
>+
>+static int xe_late_bind_component_bind(struct device *xe_kdev,
>+ struct device *mei_kdev, void *data)
>+{
>+ struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>+ struct xe_late_bind *late_bind = &xe->late_bind;
>+
>+ late_bind->component.ops = data;
>+ late_bind->component.mei_dev = mei_kdev;
>+
>+ return 0;
>+}
>+
>+static void xe_late_bind_component_unbind(struct device *xe_kdev,
>+ struct device *mei_kdev, void *data)
>+{
>+ struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>+ struct xe_late_bind *late_bind = &xe->late_bind;
>+
>+ late_bind->component.ops = NULL;
>+}
>+
>+static const struct component_ops xe_late_bind_component_ops = {
>+ .bind = xe_late_bind_component_bind,
>+ .unbind = xe_late_bind_component_unbind,
>+};
>+
>+static void xe_late_bind_remove(void *arg)
>+{
>+ struct xe_late_bind *late_bind = arg;
>+ struct xe_device *xe = late_bind_to_xe(late_bind);
>+
>+ component_del(xe->drm.dev, &xe_late_bind_component_ops);
>+}
>+
>+/**
>+ * xe_late_bind_init() - add xe mei late binding component
>+ * @late_bind: pointer to late bind structure.
>+ *
>+ * Return: 0 if the initialization was successful, a negative errno otherwise.
>+ */
>+int xe_late_bind_init(struct xe_late_bind *late_bind)
>+{
>+ struct xe_device *xe = late_bind_to_xe(late_bind);
>+ int err;
>+
>+ if (!xe->info.has_late_bind)
>+ return 0;
>+
>+ if (!IS_ENABLED(CONFIG_INTEL_MEI_LATE_BIND) || !IS_ENABLED(CONFIG_INTEL_MEI_GSC)) {
>+ drm_info(&xe->drm, "Can't init xe mei late bind missing mei component\n");
>+ return -ENODEV;
>+ }
>+
>+ err = component_add_typed(xe->drm.dev, &xe_late_bind_component_ops,
>+ INTEL_COMPONENT_LB);
>+ if (err < 0) {
>+ drm_info(&xe->drm, "Failed to add mei late bind component (%pe)\n", ERR_PTR(err));
>+ return err;
if you are going to return an error to stop the probe, then make this a
drm_err()
>+ }
>+
>+ return devm_add_action_or_reset(xe->drm.dev, xe_late_bind_remove, late_bind);
>+}
>diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>new file mode 100644
>index 000000000000..4c73571c3e62
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>@@ -0,0 +1,15 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2025 Intel Corporation
>+ */
>+
>+#ifndef _XE_LATE_BIND_FW_H_
>+#define _XE_LATE_BIND_FW_H_
>+
>+#include <linux/types.h>
>+
>+struct xe_late_bind;
>+
>+int xe_late_bind_init(struct xe_late_bind *late_bind);
>+
>+#endif
>diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>new file mode 100644
>index 000000000000..f79e5aefed94
>--- /dev/null
>+++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>@@ -0,0 +1,33 @@
>+/* SPDX-License-Identifier: MIT */
>+/*
>+ * Copyright © 2025 Intel Corporation
>+ */
>+
>+#ifndef _XE_LATE_BIND_TYPES_H_
>+#define _XE_LATE_BIND_TYPES_H_
>+
>+#include <linux/iosys-map.h>
>+#include <linux/mutex.h>
>+#include <linux/types.h>
>+
>+/**
>+ * struct xe_late_bind_component - Late Binding services component
>+ * @mei_dev: device that provide Late Binding service.
>+ * @ops: Ops implemented by Late Binding driver, used by Xe driver.
>+ *
>+ * Communication between Xe and MEI drivers for Late Binding services
>+ */
>+struct xe_late_bind_component {
>+ struct device *mei_dev;
>+ const struct late_bind_component_ops *ops;
>+};
>+
>+/**
>+ * struct xe_late_bind
>+ */
>+struct xe_late_bind {
>+ /** @component: struct for communication with mei component */
>+ struct xe_late_bind_component component;
>+};
>+
>+#endif
>diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>index ffd6ad569b7c..69b8ead9ca59 100644
>--- a/drivers/gpu/drm/xe/xe_pci.c
>+++ b/drivers/gpu/drm/xe/xe_pci.c
>@@ -332,6 +332,7 @@ static const struct xe_device_desc bmg_desc = {
> .has_gsc_nvm = 1,
> .has_heci_cscfi = 1,
> .max_gt_per_tile = 2,
>+ .has_late_bind = true,
again, boring is good: all the has_xxxx flags should be sorted
together.
Lucas De Marchi
> .needs_scratch = true,
> };
>
>@@ -578,6 +579,7 @@ static int xe_info_init_early(struct xe_device *xe,
> xe->info.has_gsc_nvm = desc->has_gsc_nvm;
> xe->info.has_heci_gscfi = desc->has_heci_gscfi;
> xe->info.has_heci_cscfi = desc->has_heci_cscfi;
>+ xe->info.has_late_bind = desc->has_late_bind;
> xe->info.has_llc = desc->has_llc;
> xe->info.has_pxp = desc->has_pxp;
> xe->info.has_sriov = desc->has_sriov;
>diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
>index 4de6f69ed975..51a607d323fb 100644
>--- a/drivers/gpu/drm/xe/xe_pci_types.h
>+++ b/drivers/gpu/drm/xe/xe_pci_types.h
>@@ -39,6 +39,7 @@ struct xe_device_desc {
> u8 has_gsc_nvm:1;
> u8 has_heci_gscfi:1;
> u8 has_heci_cscfi:1;
>+ u8 has_late_bind:1;
> u8 has_llc:1;
> u8 has_mbx_power_limits:1;
> u8 has_pxp:1;
>--
>2.49.0
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH 2/9] mei: late_bind: add late binding component driver
2025-07-16 14:45 ` Greg KH
@ 2025-07-27 14:46 ` Usyskin, Alexander
2025-08-13 9:51 ` Nilawar, Badal
0 siblings, 1 reply; 22+ messages in thread
From: Usyskin, Alexander @ 2025-07-27 14:46 UTC (permalink / raw)
To: Greg KH
Cc: Vivi, Rodrigo, intel-xe@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
Ceraolo Spurio, Daniele, Gupta, Anshuman, Nilawar, Badal
> Subject: Re: [PATCH 2/9] mei: late_bind: add late binding component driver
>
> On Wed, Jul 16, 2025 at 02:26:26PM +0000, Usyskin, Alexander wrote:
> > > > > > + if (bytes < sizeof(rsp)) {
> > > > > > + dev_err(dev, "bad response from the firmware: size
> %zd <
> > > > > %zu\n",
> > > > > > + bytes, sizeof(rsp));
> > > > > > + ret = -EPROTO;
> > > > > > + goto end;
> > > > > > + }
> > > > >
> > > > > Why not check this above when you check against the size of the
> header?
> > > > > You only need one size check, not 2.
> > > > Firmware may return only header with result field set without the data.
> > >
> > > Then the firmware is broken :)
> > >
> > > > We are parsing the header first and then starting to parse data.
> > > > If we check for whole message size at the beginning we'll miss the result
> > > data.
> > >
> > > You mean you will make it harder to debug the firmware, as you will not
> > > be printing out the header information? Or something else? The
> > > bytes variable HAS to match the full structure size, not just the header
> > > size, according to this code. So just test for that and be done with
> > > it!
> >
> > The CSME firmware returns only command header if, like, command is not
> recognised.
> > This may happen because of firmware bug or for firmware is
> configured/compiled
> > that way.
> > This seems reasonable for the complex protocols where firmware may not be
> > aware of this particular command at all and have no idea what the data size
> it
> > should send in reply.
> > Printing result from the header will allow us to understand either this is the
> firmware
> > problem or driver sent something wrong.
>
> Then make it obvious in your checking and in your error messages as to
> what you are doing here. Checking the size of the buffer in two
> different places, with different values is very odd, and deserves a lot
> of explaination.
>
Is this addition
/*
* Received message size may be smaller than the full message size when
* reply contains only MKHI header with result field set to the error code.
* Check the header size and content first to output exact error, if needed,
* and then process to the whole message.
*/
and remodelling error messages like "received less then header size from the firmware"
made it clean for people not involved with our firmware?
I'm too deep in this to judge the wording.
- -
Thanks,
Sasha
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/9] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw
2025-07-25 21:38 ` Lucas De Marchi
@ 2025-07-30 11:47 ` Nilawar, Badal
0 siblings, 0 replies; 22+ messages in thread
From: Nilawar, Badal @ 2025-07-30 11:47 UTC (permalink / raw)
To: Lucas De Marchi, Rodrigo Vivi
Cc: intel-xe, dri-devel, linux-kernel, gregkh, daniele.ceraolospurio,
anshuman.gupta, alexander.usyskin
On 26-07-2025 03:08, Lucas De Marchi wrote:
> On Thu, Jul 10, 2025 at 11:08:34AM -0400, Rodrigo Vivi wrote:
>> From: Badal Nilawar <badal.nilawar@intel.com>
>>
>> Introducing xe_late_bind_fw to enable firmware loading for the devices,
>
> here and in the subject: no gerund, please.
Sure.
>
>> such as the fan controller, during the driver probe. Typically,
>> firmware for such devices are part of IFWI flash image but can be
>> replaced at probe after OEM tuning.
>> This patch binds mei late binding component to enable firmware loading.
>>
>> v2:
>> - Add devm_add_action_or_reset to remove the component (Daniele)
>> - Add INTEL_MEI_GSC check in xe_late_bind_init() (Daniele)
>> v3:
>> - Fail driver probe if late bind initialization fails,
>> add has_late_bind flag (Daniele)
>> v4:
>> - %S/I915_COMPONENT_LATE_BIND/INTEL_COMPONENT_LATE_BIND/
>> v6:
>> - rebased
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>> drivers/gpu/drm/xe/Makefile | 1 +
>> drivers/gpu/drm/xe/xe_device.c | 5 ++
>> drivers/gpu/drm/xe/xe_device_types.h | 6 ++
>> drivers/gpu/drm/xe/xe_late_bind_fw.c | 84 ++++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_late_bind_fw.h | 15 ++++
>> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 33 +++++++++
>> drivers/gpu/drm/xe/xe_pci.c | 2 +
>> drivers/gpu/drm/xe/xe_pci_types.h | 1 +
>> 8 files changed, 147 insertions(+)
>> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.c
>> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw.h
>> create mode 100644 drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>>
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index 1d97e5b63f4e..97e2b1368a6e 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -76,6 +76,7 @@ xe-y += xe_bb.o \
>> xe_hw_fence.o \
>> xe_irq.o \
>> xe_lrc.o \
>> + xe_late_bind_fw.o \
>
> almost there, but still not sorted as it should be
It seems the rebase messed up the order—it was correctly placed in the
previous revision. I’ll fix it.
>
>> xe_migrate.o \
>> xe_mmio.o \
>> xe_mocs.o \
>> diff --git a/drivers/gpu/drm/xe/xe_device.c
>> b/drivers/gpu/drm/xe/xe_device.c
>> index 0b73cb72bad1..cb595bae5f55 100644
>> --- a/drivers/gpu/drm/xe/xe_device.c
>> +++ b/drivers/gpu/drm/xe/xe_device.c
>> @@ -44,6 +44,7 @@
>> #include "xe_hw_engine_group.h"
>> #include "xe_hwmon.h"
>> #include "xe_irq.h"
>> +#include "xe_late_bind_fw.h"
>> #include "xe_mmio.h"
>> #include "xe_module.h"
>> #include "xe_nvm.h"
>> @@ -866,6 +867,10 @@ int xe_device_probe(struct xe_device *xe)
>> if (err)
>> return err;
>>
>> + err = xe_late_bind_init(&xe->late_bind);
>> + if (err && err != -ENODEV)
>
> let's not be creative in xe_device_probe(). Boring is good.
>
> All the other parts in this function handle ENODEV internally. It's up
> to them to decide if ENODEV -> return 0, not here.
sure, will update accordingly.
>
>
>> + return err;
>> +
>> err = xe_oa_init(xe);
>> if (err)
>> return err;
>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h
>> b/drivers/gpu/drm/xe/xe_device_types.h
>> index 78c4acafd268..a8891833f980 100644
>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>> @@ -16,6 +16,7 @@
>> #include "xe_devcoredump_types.h"
>> #include "xe_heci_gsc.h"
>> #include "xe_lmtt_types.h"
>> +#include "xe_late_bind_fw_types.h"
>
> should be sorted
Sure.
>
>> #include "xe_memirq_types.h"
>> #include "xe_oa_types.h"
>> #include "xe_platform_types.h"
>> @@ -325,6 +326,8 @@ struct xe_device {
>> u8 has_heci_cscfi:1;
>> /** @info.has_heci_gscfi: device has heci gscfi */
>> u8 has_heci_gscfi:1;
>> + /** @info.has_late_bind: Device has firmware late binding
>> support */
>> + u8 has_late_bind:1;
>> /** @info.has_llc: Device has a shared CPU+GPU last level
>> cache */
>> u8 has_llc:1;
>> /** @info.has_mbx_power_limits: Device has support to manage
>> power limits using
>> @@ -557,6 +560,9 @@ struct xe_device {
>> /** @nvm: discrete graphics non-volatile memory */
>> struct intel_dg_nvm_dev *nvm;
>>
>> + /** @late_bind: xe mei late bind interface */
>> + struct xe_late_bind late_bind;
>> +
>> /** @oa: oa observation subsystem */
>> struct xe_oa oa;
>>
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> new file mode 100644
>> index 000000000000..152f3e58de94
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
>> @@ -0,0 +1,84 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2025 Intel Corporation
>> + */
>> +
>> +#include <linux/component.h>
>> +#include <linux/delay.h>
>> +
>> +#include <drm/drm_managed.h>
>> +#include <drm/intel/i915_component.h>
>> +#include <drm/intel/intel_lb_mei_interface.h>
>> +#include <drm/drm_print.h>
>> +
>> +#include "xe_device.h"
>> +#include "xe_late_bind_fw.h"
>> +
>> +static struct xe_device *
>> +late_bind_to_xe(struct xe_late_bind *late_bind)
>> +{
>> + return container_of(late_bind, struct xe_device, late_bind);
>> +}
>> +
>> +static int xe_late_bind_component_bind(struct device *xe_kdev,
>> + struct device *mei_kdev, void *data)
>> +{
>> + struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>> + struct xe_late_bind *late_bind = &xe->late_bind;
>> +
>> + late_bind->component.ops = data;
>> + late_bind->component.mei_dev = mei_kdev;
>> +
>> + return 0;
>> +}
>> +
>> +static void xe_late_bind_component_unbind(struct device *xe_kdev,
>> + struct device *mei_kdev, void *data)
>> +{
>> + struct xe_device *xe = kdev_to_xe_device(xe_kdev);
>> + struct xe_late_bind *late_bind = &xe->late_bind;
>> +
>> + late_bind->component.ops = NULL;
>> +}
>> +
>> +static const struct component_ops xe_late_bind_component_ops = {
>> + .bind = xe_late_bind_component_bind,
>> + .unbind = xe_late_bind_component_unbind,
>> +};
>> +
>> +static void xe_late_bind_remove(void *arg)
>> +{
>> + struct xe_late_bind *late_bind = arg;
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> +
>> + component_del(xe->drm.dev, &xe_late_bind_component_ops);
>> +}
>> +
>> +/**
>> + * xe_late_bind_init() - add xe mei late binding component
>> + * @late_bind: pointer to late bind structure.
>> + *
>> + * Return: 0 if the initialization was successful, a negative errno
>> otherwise.
>> + */
>> +int xe_late_bind_init(struct xe_late_bind *late_bind)
>> +{
>> + struct xe_device *xe = late_bind_to_xe(late_bind);
>> + int err;
>> +
>> + if (!xe->info.has_late_bind)
>> + return 0;
>> +
>> + if (!IS_ENABLED(CONFIG_INTEL_MEI_LATE_BIND) ||
>> !IS_ENABLED(CONFIG_INTEL_MEI_GSC)) {
>> + drm_info(&xe->drm, "Can't init xe mei late bind missing mei
>> component\n");
>> + return -ENODEV;
>> + }
>> +
>> + err = component_add_typed(xe->drm.dev, &xe_late_bind_component_ops,
>> + INTEL_COMPONENT_LB);
>> + if (err < 0) {
>> + drm_info(&xe->drm, "Failed to add mei late bind component
>> (%pe)\n", ERR_PTR(err));
>> + return err;
>
> if you are going to return an error to stop the probe, then make this a
> drm_err()
Sure.
>
>> + }
>> +
>> + return devm_add_action_or_reset(xe->drm.dev,
>> xe_late_bind_remove, late_bind);
>> +}
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> new file mode 100644
>> index 000000000000..4c73571c3e62
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2025 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_LATE_BIND_FW_H_
>> +#define _XE_LATE_BIND_FW_H_
>> +
>> +#include <linux/types.h>
>> +
>> +struct xe_late_bind;
>> +
>> +int xe_late_bind_init(struct xe_late_bind *late_bind);
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> new file mode 100644
>> index 000000000000..f79e5aefed94
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
>> @@ -0,0 +1,33 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2025 Intel Corporation
>> + */
>> +
>> +#ifndef _XE_LATE_BIND_TYPES_H_
>> +#define _XE_LATE_BIND_TYPES_H_
>> +
>> +#include <linux/iosys-map.h>
>> +#include <linux/mutex.h>
>> +#include <linux/types.h>
>> +
>> +/**
>> + * struct xe_late_bind_component - Late Binding services component
>> + * @mei_dev: device that provide Late Binding service.
>> + * @ops: Ops implemented by Late Binding driver, used by Xe driver.
>> + *
>> + * Communication between Xe and MEI drivers for Late Binding services
>> + */
>> +struct xe_late_bind_component {
>> + struct device *mei_dev;
>> + const struct late_bind_component_ops *ops;
>> +};
>> +
>> +/**
>> + * struct xe_late_bind
>> + */
>> +struct xe_late_bind {
>> + /** @component: struct for communication with mei component */
>> + struct xe_late_bind_component component;
>> +};
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>> index ffd6ad569b7c..69b8ead9ca59 100644
>> --- a/drivers/gpu/drm/xe/xe_pci.c
>> +++ b/drivers/gpu/drm/xe/xe_pci.c
>> @@ -332,6 +332,7 @@ static const struct xe_device_desc bmg_desc = {
>> .has_gsc_nvm = 1,
>> .has_heci_cscfi = 1,
>> .max_gt_per_tile = 2,
>> + .has_late_bind = true,
>
> again, boring is good: all the has_xxxx flags should be sorted
> together.
Looks like the re-base affected this part as well. It was correctly
grouped in the previous revision — I’ll fix this.
Thanks,
Badal
>
> Lucas De Marchi
>
>> .needs_scratch = true,
>> };
>>
>> @@ -578,6 +579,7 @@ static int xe_info_init_early(struct xe_device *xe,
>> xe->info.has_gsc_nvm = desc->has_gsc_nvm;
>> xe->info.has_heci_gscfi = desc->has_heci_gscfi;
>> xe->info.has_heci_cscfi = desc->has_heci_cscfi;
>> + xe->info.has_late_bind = desc->has_late_bind;
>> xe->info.has_llc = desc->has_llc;
>> xe->info.has_pxp = desc->has_pxp;
>> xe->info.has_sriov = desc->has_sriov;
>> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h
>> b/drivers/gpu/drm/xe/xe_pci_types.h
>> index 4de6f69ed975..51a607d323fb 100644
>> --- a/drivers/gpu/drm/xe/xe_pci_types.h
>> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
>> @@ -39,6 +39,7 @@ struct xe_device_desc {
>> u8 has_gsc_nvm:1;
>> u8 has_heci_gscfi:1;
>> u8 has_heci_cscfi:1;
>> + u8 has_late_bind:1;
>> u8 has_llc:1;
>> u8 has_mbx_power_limits:1;
>> u8 has_pxp:1;
>> --
>> 2.49.0
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 8/9] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding
2025-07-10 15:08 ` [PATCH 8/9] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding Rodrigo Vivi
@ 2025-07-31 20:03 ` Summers, Stuart
2025-08-01 15:17 ` Rodrigo Vivi
0 siblings, 1 reply; 22+ messages in thread
From: Summers, Stuart @ 2025-07-31 20:03 UTC (permalink / raw)
To: intel-xe@lists.freedesktop.org, Vivi, Rodrigo,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Cc: Nilawar, Badal, gregkh@linuxfoundation.org, Usyskin, Alexander,
Ceraolo Spurio, Daniele, Gupta, Anshuman
On Thu, 2025-07-10 at 11:08 -0400, Rodrigo Vivi wrote:
> From: Badal Nilawar <badal.nilawar@intel.com>
>
> Introduce a debug filesystem node to disable late binding fw reload
> during the system or runtime resume. This is intended for situations
> where the late binding fw needs to be loaded from user mode,
> perticularly for validation purpose.
> Note that xe kmd doesn't participate in late binding flow from user
> space. Binary loaded from the userspace will be lost upon entering to
> D3 cold hence user space app need to handle this situation.
>
> v2:
> - s/(uval == 1) ? true : false/!!uval/ (Daniele)
> v3:
> - Refine the commit message (Daniele)
>
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Bit of a flyby comment here, but would it be better if this was
configfs rather than debugfs in case we wanted to skip this on the
first probe?
Thanks,
Stuart
> ---
> drivers/gpu/drm/xe/xe_debugfs.c | 41
> ++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 3 ++
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 2 ++
> 3 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c
> b/drivers/gpu/drm/xe/xe_debugfs.c
> index d83cd6ed3fa8..d1f6f556efa2 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -226,6 +226,44 @@ static const struct file_operations
> atomic_svm_timeslice_ms_fops = {
> .write = atomic_svm_timeslice_ms_set,
> };
>
> +static ssize_t disable_late_binding_show(struct file *f, char __user
> *ubuf,
> + size_t size, loff_t *pos)
> +{
> + struct xe_device *xe = file_inode(f)->i_private;
> + struct xe_late_bind *late_bind = &xe->late_bind;
> + char buf[32];
> + int len;
> +
> + len = scnprintf(buf, sizeof(buf), "%d\n", late_bind-
> >disable);
> +
> + return simple_read_from_buffer(ubuf, size, pos, buf, len);
> +}
> +
> +static ssize_t disable_late_binding_set(struct file *f, const char
> __user *ubuf,
> + size_t size, loff_t *pos)
> +{
> + struct xe_device *xe = file_inode(f)->i_private;
> + struct xe_late_bind *late_bind = &xe->late_bind;
> + u32 uval;
> + ssize_t ret;
> +
> + ret = kstrtouint_from_user(ubuf, size, sizeof(uval), &uval);
> + if (ret)
> + return ret;
> +
> + if (uval > 1)
> + return -EINVAL;
> +
> + late_bind->disable = !!uval;
> + return size;
> +}
> +
> +static const struct file_operations disable_late_binding_fops = {
> + .owner = THIS_MODULE,
> + .read = disable_late_binding_show,
> + .write = disable_late_binding_set,
> +};
> +
> void xe_debugfs_register(struct xe_device *xe)
> {
> struct ttm_device *bdev = &xe->ttm;
> @@ -249,6 +287,9 @@ void xe_debugfs_register(struct xe_device *xe)
> debugfs_create_file("atomic_svm_timeslice_ms", 0600, root,
> xe,
> &atomic_svm_timeslice_ms_fops);
>
> + debugfs_create_file("disable_late_binding", 0600, root, xe,
> + &disable_late_binding_fops);
> +
> for (mem_type = XE_PL_VRAM0; mem_type <= XE_PL_VRAM1;
> ++mem_type) {
> man = ttm_manager_type(bdev, mem_type);
>
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> index df43523e9043..88355adce1d0 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> @@ -167,6 +167,9 @@ int xe_late_bind_fw_load(struct xe_late_bind
> *late_bind)
> if (!late_bind->component_added)
> return -ENODEV;
>
> + if (late_bind->disable)
> + return 0;
> +
> for (fw_id = 0; fw_id < XE_LB_FW_MAX_ID; fw_id++) {
> lbfw = &late_bind->late_bind_fw[fw_id];
> if (lbfw->payload) {
> diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> index 5c0574aff7b9..158dc1abe072 100644
> --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> @@ -65,6 +65,8 @@ struct xe_late_bind {
> struct workqueue_struct *wq;
> /** @component_added: whether the component has been added */
> bool component_added;
> + /** @disable: to block late binding reload during pm resume
> flow*/
> + bool disable;
> };
>
> #endif
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 8/9] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding
2025-07-31 20:03 ` Summers, Stuart
@ 2025-08-01 15:17 ` Rodrigo Vivi
0 siblings, 0 replies; 22+ messages in thread
From: Rodrigo Vivi @ 2025-08-01 15:17 UTC (permalink / raw)
To: Summers, Stuart
Cc: intel-xe@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, Nilawar, Badal,
gregkh@linuxfoundation.org, Usyskin, Alexander,
Ceraolo Spurio, Daniele, Gupta, Anshuman
On Thu, Jul 31, 2025 at 08:03:36PM +0000, Summers, Stuart wrote:
> On Thu, 2025-07-10 at 11:08 -0400, Rodrigo Vivi wrote:
> > From: Badal Nilawar <badal.nilawar@intel.com>
> >
> > Introduce a debug filesystem node to disable late binding fw reload
> > during the system or runtime resume. This is intended for situations
> > where the late binding fw needs to be loaded from user mode,
> > perticularly for validation purpose.
> > Note that xe kmd doesn't participate in late binding flow from user
> > space. Binary loaded from the userspace will be lost upon entering to
> > D3 cold hence user space app need to handle this situation.
> >
> > v2:
> > - s/(uval == 1) ? true : false/!!uval/ (Daniele)
> > v3:
> > - Refine the commit message (Daniele)
> >
> > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> > Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Bit of a flyby comment here, but would it be better if this was
> configfs rather than debugfs in case we wanted to skip this on the
> first probe?
Indeed. Configfs seems the best alternative for this. Otherwise
by the time we get to have the debugfs, the fw was already
attempted to get loaded at least once.
>
> Thanks,
> Stuart
>
> > ---
> > drivers/gpu/drm/xe/xe_debugfs.c | 41
> > ++++++++++++++++++++++
> > drivers/gpu/drm/xe/xe_late_bind_fw.c | 3 ++
> > drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 2 ++
> > 3 files changed, 46 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_debugfs.c
> > b/drivers/gpu/drm/xe/xe_debugfs.c
> > index d83cd6ed3fa8..d1f6f556efa2 100644
> > --- a/drivers/gpu/drm/xe/xe_debugfs.c
> > +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> > @@ -226,6 +226,44 @@ static const struct file_operations
> > atomic_svm_timeslice_ms_fops = {
> > .write = atomic_svm_timeslice_ms_set,
> > };
> >
> > +static ssize_t disable_late_binding_show(struct file *f, char __user
> > *ubuf,
> > + size_t size, loff_t *pos)
> > +{
> > + struct xe_device *xe = file_inode(f)->i_private;
> > + struct xe_late_bind *late_bind = &xe->late_bind;
> > + char buf[32];
> > + int len;
> > +
> > + len = scnprintf(buf, sizeof(buf), "%d\n", late_bind-
> > >disable);
> > +
> > + return simple_read_from_buffer(ubuf, size, pos, buf, len);
> > +}
> > +
> > +static ssize_t disable_late_binding_set(struct file *f, const char
> > __user *ubuf,
> > + size_t size, loff_t *pos)
> > +{
> > + struct xe_device *xe = file_inode(f)->i_private;
> > + struct xe_late_bind *late_bind = &xe->late_bind;
> > + u32 uval;
> > + ssize_t ret;
> > +
> > + ret = kstrtouint_from_user(ubuf, size, sizeof(uval), &uval);
> > + if (ret)
> > + return ret;
> > +
> > + if (uval > 1)
> > + return -EINVAL;
> > +
> > + late_bind->disable = !!uval;
> > + return size;
> > +}
> > +
> > +static const struct file_operations disable_late_binding_fops = {
> > + .owner = THIS_MODULE,
> > + .read = disable_late_binding_show,
> > + .write = disable_late_binding_set,
> > +};
> > +
> > void xe_debugfs_register(struct xe_device *xe)
> > {
> > struct ttm_device *bdev = &xe->ttm;
> > @@ -249,6 +287,9 @@ void xe_debugfs_register(struct xe_device *xe)
> > debugfs_create_file("atomic_svm_timeslice_ms", 0600, root,
> > xe,
> > &atomic_svm_timeslice_ms_fops);
> >
> > + debugfs_create_file("disable_late_binding", 0600, root, xe,
> > + &disable_late_binding_fops);
> > +
> > for (mem_type = XE_PL_VRAM0; mem_type <= XE_PL_VRAM1;
> > ++mem_type) {
> > man = ttm_manager_type(bdev, mem_type);
> >
> > diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> > b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> > index df43523e9043..88355adce1d0 100644
> > --- a/drivers/gpu/drm/xe/xe_late_bind_fw.c
> > +++ b/drivers/gpu/drm/xe/xe_late_bind_fw.c
> > @@ -167,6 +167,9 @@ int xe_late_bind_fw_load(struct xe_late_bind
> > *late_bind)
> > if (!late_bind->component_added)
> > return -ENODEV;
> >
> > + if (late_bind->disable)
> > + return 0;
> > +
> > for (fw_id = 0; fw_id < XE_LB_FW_MAX_ID; fw_id++) {
> > lbfw = &late_bind->late_bind_fw[fw_id];
> > if (lbfw->payload) {
> > diff --git a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> > b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> > index 5c0574aff7b9..158dc1abe072 100644
> > --- a/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> > +++ b/drivers/gpu/drm/xe/xe_late_bind_fw_types.h
> > @@ -65,6 +65,8 @@ struct xe_late_bind {
> > struct workqueue_struct *wq;
> > /** @component_added: whether the component has been added */
> > bool component_added;
> > + /** @disable: to block late binding reload during pm resume
> > flow*/
> > + bool disable;
> > };
> >
> > #endif
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/9] mei: late_bind: add late binding component driver
2025-07-27 14:46 ` Usyskin, Alexander
@ 2025-08-13 9:51 ` Nilawar, Badal
2025-08-13 12:13 ` Greg KH
0 siblings, 1 reply; 22+ messages in thread
From: Nilawar, Badal @ 2025-08-13 9:51 UTC (permalink / raw)
To: Usyskin, Alexander, Greg KH
Cc: Vivi, Rodrigo, intel-xe@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
Ceraolo Spurio, Daniele, Gupta, Anshuman
[-- Attachment #1: Type: text/plain, Size: 4250 bytes --]
Hi Greg,
On 27-07-2025 20:16, Usyskin, Alexander wrote:
>> Subject: Re: [PATCH 2/9] mei: late_bind: add late binding component driver
>>
>> On Wed, Jul 16, 2025 at 02:26:26PM +0000, Usyskin, Alexander wrote:
>>>>>>> + if (bytes < sizeof(rsp)) {
>>>>>>> + dev_err(dev, "bad response from the firmware: size
>> %zd <
>>>>>> %zu\n",
>>>>>>> + bytes, sizeof(rsp));
>>>>>>> + ret = -EPROTO;
>>>>>>> + goto end;
>>>>>>> + }
>>>>>> Why not check this above when you check against the size of the
>> header?
>>>>>> You only need one size check, not 2.
>>>>> Firmware may return only header with result field set without the data.
>>>> Then the firmware is broken :)
>>>>
>>>>> We are parsing the header first and then starting to parse data.
>>>>> If we check for whole message size at the beginning we'll miss the result
>>>> data.
>>>>
>>>> You mean you will make it harder to debug the firmware, as you will not
>>>> be printing out the header information? Or something else? The
>>>> bytes variable HAS to match the full structure size, not just the header
>>>> size, according to this code. So just test for that and be done with
>>>> it!
>>> The CSME firmware returns only command header if, like, command is not
>> recognised.
>>> This may happen because of firmware bug or for firmware is
>> configured/compiled
>>> that way.
>>> This seems reasonable for the complex protocols where firmware may not be
>>> aware of this particular command at all and have no idea what the data size
>> it
>>> should send in reply.
>>> Printing result from the header will allow us to understand either this is the
>> firmware
>>> problem or driver sent something wrong.
>> Then make it obvious in your checking and in your error messages as to
>> what you are doing here. Checking the size of the buffer in two
>> different places, with different values is very odd, and deserves a lot
>> of explaination.
>>
> Is this addition
> /*
> * Received message size may be smaller than the full message size when
> * reply contains only MKHI header with result field set to the error code.
> * Check the header size and content first to output exact error, if needed,
> * and then process to the whole message.
> */
>
> and remodelling error messages like "received less then header size from the firmware"
> made it clean for people not involved with our firmware?
> I'm too deep in this to judge the wording.
I'm planning to include the following code in the next revision. Does
this change align with your recommendation?
+ /*
+ * Received message size may be smaller than the full
message size when
+ * reply contains only MKHI header with result field set to
the error code.
+ * Check the header size and content first to output exact
error, if needed,
+ * and then process to the whole message.
+ */
if (bytes < sizeof(rsp.header)) {
- dev_err(dev, "bad response header from the firmware:
size %zd < %zu\n",
+ dev_err(dev, "received less than header size from
the firmware: %zd < %zu\n",
bytes, sizeof(rsp.header));
ret = -EPROTO;
goto end;
}
if (mei_lb_check_response(dev, &rsp.header)) {
- dev_err(dev, "bad result response from the firmware:
0x%x\n",
+ dev_err(dev, "bad response from the firmware:
header: 0x%x\n",
*(uint32_t *)&rsp.header);
ret = -EPROTO;
goto end;
}
if (bytes < sizeof(rsp)) {
- dev_err(dev, "bad response from the firmware: size
%zd < %zu\n",
+ dev_err(dev, "received less than message size from
the firmware: %zd < %zu\n",
bytes, sizeof(rsp));
ret = -EPROTO;
goto end;
Thanks,
Badal
>
> - -
> Thanks,
> Sasha
>
>
[-- Attachment #2: Type: text/html, Size: 9573 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/9] mei: late_bind: add late binding component driver
2025-08-13 9:51 ` Nilawar, Badal
@ 2025-08-13 12:13 ` Greg KH
0 siblings, 0 replies; 22+ messages in thread
From: Greg KH @ 2025-08-13 12:13 UTC (permalink / raw)
To: Nilawar, Badal
Cc: Usyskin, Alexander, Vivi, Rodrigo, intel-xe@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
Ceraolo Spurio, Daniele, Gupta, Anshuman
On Wed, Aug 13, 2025 at 03:21:09PM +0530, Nilawar, Badal wrote:
> Hi Greg,
<snip>
For some reason this was in html format :(
Also, please follow the rules that Intel kernel developers must follow,
which means not using me as your first-line of review. Please work with
your internal developers on this first, before resubmitting.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2025-08-13 12:13 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-10 15:08 [PATCH 0/9] Introducing firmware late binding Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 1/9] mei: bus: add mei_cldev_mtu interface Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 2/9] mei: late_bind: add late binding component driver Rodrigo Vivi
2025-07-16 11:48 ` Greg KH
2025-07-16 11:58 ` Usyskin, Alexander
2025-07-16 12:07 ` Greg KH
2025-07-16 14:26 ` Usyskin, Alexander
2025-07-16 14:45 ` Greg KH
2025-07-27 14:46 ` Usyskin, Alexander
2025-08-13 9:51 ` Nilawar, Badal
2025-08-13 12:13 ` Greg KH
2025-07-10 15:08 ` [PATCH 3/9] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw Rodrigo Vivi
2025-07-25 21:38 ` Lucas De Marchi
2025-07-30 11:47 ` Nilawar, Badal
2025-07-10 15:08 ` [PATCH 4/9] drm/xe/xe_late_bind_fw: Initialize late binding firmware Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 5/9] drm/xe/xe_late_bind_fw: Load " Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 6/9] drm/xe/xe_late_bind_fw: Reload late binding fw in rpm resume Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 7/9] drm/xe/xe_late_bind_fw: Reload late binding fw during system resume Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 8/9] drm/xe/xe_late_bind_fw: Introduce debug fs node to disable late binding Rodrigo Vivi
2025-07-31 20:03 ` Summers, Stuart
2025-08-01 15:17 ` Rodrigo Vivi
2025-07-10 15:08 ` [PATCH 9/9] drm/xe/xe_late_bind_fw: Extract and print version info Rodrigo Vivi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).