| 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: <6edjvu7hnzq7rzmr4dybfxderiwfozlvdbptho2z7sgkhy5eka@jndenohiutrc>
Date: Fri, 25 Jul 2025 18:38:51 -0300
From: Lucas De Marchi <lucas.demarchi@...el.com>
To: Rodrigo Vivi <rodrigo.vivi@...el.com>
CC: <intel-xe@...ts.freedesktop.org>, <dri-devel@...ts.freedesktop.org>,
<linux-kernel@...r.kernel.org>, <gregkh@...uxfoundation.org>,
<daniele.ceraolospurio@...el.com>, <anshuman.gupta@...el.com>,
<alexander.usyskin@...el.com>, Badal Nilawar <badal.nilawar@...el.com>
Subject: Re: [PATCH 3/9] drm/xe/xe_late_bind_fw: Introducing xe_late_bind_fw
On Thu, Jul 10, 2025 at 11:08:34AM -0400, Rodrigo Vivi wrote:
>From: Badal Nilawar <badal.nilawar@...el.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@...el.com>
>Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@...el.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@...el.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
>
Powered by blists - more mailing lists