| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f1bfbdf6-8f66-477d-92e3-50612b4c4b71@intel.com>
Date: Thu, 26 Jun 2025 14:06:10 -0700
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@...el.com>
To: Badal Nilawar <badal.nilawar@...el.com>, <intel-xe@...ts.freedesktop.org>,
<dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
CC: <anshuman.gupta@...el.com>, <rodrigo.vivi@...el.com>,
<alexander.usyskin@...el.com>, <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v4 04/10] drm/xe/xe_late_bind_fw: Initialize late binding
firmware
On 6/25/2025 10:00 AM, Badal Nilawar wrote:
> 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
>
> Signed-off-by: Badal Nilawar <badal.nilawar@...el.com>
> ---
> drivers/gpu/drm/xe/xe_late_bind_fw.c | 103 ++++++++++++++++++++-
> drivers/gpu/drm/xe/xe_late_bind_fw_types.h | 32 +++++++
> 2 files changed, 134 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 eaf12cfec848..32d1436e7191 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] = CSC_LATE_BINDING_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,92 @@ 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->valid = false;
> + lb_fw->id = fw_id;
> + lb_fw->type = fw_id_to_type[lb_fw->id];
> + lb_fw->flags &= ~CSC_LATE_BINDING_FLAGS_IS_PERSISTENT;
> +
> + if (lb_fw->type == CSC_LATE_BINDING_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 > 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, MAX_PAYLOAD_SIZE);
> + release_firmware(fw);
> + return -ENODATA;
> + }
> +
> + lb_fw->payload = drmm_kzalloc(&xe->drm, lb_fw->payload_size, GFP_KERNEL);
here you're using lb_fw->payload_size before assigning it.
> + if (!lb_fw->payload) {
> + release_firmware(fw);
> + return -ENOMEM;
> + }
> +
> + lb_fw->payload_size = fw->size;
> +
> + memcpy(lb_fw->payload, fw->data, lb_fw->payload_size);
> + release_firmware(fw);
> + lb_fw->valid = true;
You can now use lb_fw->payload to check if the FW is valid, no need for
a separate variable. not a blocker.
> +
> + 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)
> {
> @@ -86,5 +183,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 1156ef94f0d5..93abf4c51789 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,36 @@
> #include <linux/mutex.h>
> #include <linux/types.h>
>
> +#define 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 {
> + /** @late_bind_fw.valid: to check if fw is valid */
> + bool valid;
> + /** @late_bind_fw.id: firmware index */
> + u32 id;
> + /** @late_bind_fw.blob_path: firmware binary path */
> + char blob_path[PATH_MAX];
> + /** @late_bind_fw.type: firmware type */
> + u32 type;
> + /** @late_bind_fw.flags: firmware flags */
> + u32 flags;
> + /** @late_bind_fw.payload: to store the late binding blob */
> + u8 *payload;
Why a u8 pointer and not a void one?
Daniele
> + /** @late_bind_fw.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.
> @@ -32,6 +62,8 @@ struct xe_late_bind {
> struct xe_late_bind_component component;
> /** @late_bind.mutex: protects the component binding and usage */
> struct mutex mutex;
> + /** @late_bind.late_bind_fw: late binding firmware array */
> + struct xe_late_bind_fw late_bind_fw[XE_LB_FW_MAX_ID];
> };
>
> #endif
Powered by blists - more mailing lists