[PATCH v2 1/3] drm/xe: Add functions and sysfs for boot survivability
Riana Tauro
riana.tauro at intel.com
Thu Jan 16 09:47:47 UTC 2025
On 1/16/2025 1:12 AM, Rodrigo Vivi wrote:
> On Wed, Jan 15, 2025 at 09:47:53PM +0530, Riana Tauro wrote:
>> Hi Rodrigo
>>
>> On 1/10/2025 8:51 PM, Rodrigo Vivi wrote:
>>> On Wed, Jan 08, 2025 at 04:09:57PM +0530, Riana Tauro wrote:
>>>> Boot Survivability is a software based workflow for recovering a system
>>>> in a failed boot state. Here system recoverability is concerned with
>>>> recovering the firmware responsible for boot.
>>>>
>>>> This is implemented by loading the driver with bare minimum (no drm card)
>>>> to allow the firmware to be flashed through mei-gsc and collect telemetry.
>>>> The driver's probe flow is modified such that it enters survivability mode
>>>> when pcode initialization is incomplete and boot status denotes a failure.
>>>> In this mode, drm card is not exposed and presence of survivability_mode
>>>> entry in PCI sysfs is used to indicate survivability mode and
>>>> provide additional information required for debug
>>>>
>>>> This patch adds initialization functions and exposes admin
>>>> readable sysfs entries
>>>>
>>>> The new sysfs will have the below layout
>>>>
>>>> /sys/bus/.../bdf
>>>> ├── survivability_mode
>>>>
>>>> v2: reorder headers
>>>> fix doc
>>>> remove survivability info and use mode to display information
>>>> use separate function for logging survivability information
>>>> for critical error (Rodrigo)
>>>>
>>>> Signed-off-by: Riana Tauro <riana.tauro at intel.com>
>>>> ---
>>>> drivers/gpu/drm/xe/Makefile | 1 +
>>>> drivers/gpu/drm/xe/xe_device_types.h | 4 +
>>>> drivers/gpu/drm/xe/xe_pcode_api.h | 14 ++
>>>> drivers/gpu/drm/xe/xe_survivability_mode.c | 231 ++++++++++++++++++
>>>> drivers/gpu/drm/xe/xe_survivability_mode.h | 17 ++
>>>> .../gpu/drm/xe/xe_survivability_mode_types.h | 35 +++
>>>> 6 files changed, 302 insertions(+)
>>>> create mode 100644 drivers/gpu/drm/xe/xe_survivability_mode.c
>>>> create mode 100644 drivers/gpu/drm/xe/xe_survivability_mode.h
>>>> create mode 100644 drivers/gpu/drm/xe/xe_survivability_mode_types.h
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>>>> index 5c97ad6ed738..fb1cb98ce891 100644
>>>> --- a/drivers/gpu/drm/xe/Makefile
>>>> +++ b/drivers/gpu/drm/xe/Makefile
>>>> @@ -95,6 +95,7 @@ xe-y += xe_bb.o \
>>>> xe_sa.o \
>>>> xe_sched_job.o \
>>>> xe_step.o \
>>>> + xe_survivability_mode.o \
>>>> xe_sync.o \
>>>> xe_tile.o \
>>>> xe_tile_sysfs.o \
>>>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>>>> index 8a7b15972413..0f5a052150c9 100644
>>>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>>>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>>>> @@ -21,6 +21,7 @@
>>>> #include "xe_pt_types.h"
>>>> #include "xe_sriov_types.h"
>>>> #include "xe_step_types.h"
>>>> +#include "xe_survivability_mode_types.h"
>>>> #if IS_ENABLED(CONFIG_DRM_XE_DEBUG)
>>>> #define TEST_VM_OPS_ERROR
>>>> @@ -341,6 +342,9 @@ struct xe_device {
>>>> u8 skip_pcode:1;
>>>> } info;
>>>> + /** @survivability: survivability information for device */
>>>> + struct xe_survivability survivability;
>>>> +
>>>> /** @irq: device interrupt state */
>>>> struct {
>>>> /** @irq.lock: lock for processing irq's on this device */
>>>> diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h b/drivers/gpu/drm/xe/xe_pcode_api.h
>>>> index f153ce96f69a..4e373b8199ca 100644
>>>> --- a/drivers/gpu/drm/xe/xe_pcode_api.h
>>>> +++ b/drivers/gpu/drm/xe/xe_pcode_api.h
>>>> @@ -49,6 +49,20 @@
>>>> /* Domain IDs (param2) */
>>>> #define PCODE_MBOX_DOMAIN_HBM 0x2
>>>> +#define PCODE_SCRATCH_ADDR(x) XE_REG(0x138320 + ((x) * 4))
>>>> +/* PCODE_SCRATCH0 */
>>>> +#define AUXINFO_REG_OFFSET REG_GENMASK(17, 15)
>>>> +#define OVERFLOW_REG_OFFSET REG_GENMASK(14, 12)
>>>> +#define HISTORY_TRACKING REG_BIT(11)
>>>> +#define OVERFLOW_SUPPORT REG_BIT(10)
>>>> +#define AUXINFO_SUPPORT REG_BIT(9)
>>>> +#define BOOT_STATUS REG_GENMASK(3, 1)
>>>> +#define CRITICAL_FAILURE 4
>>>> +#define NON_CRITICAL_FAILURE 7
>>>> +
>>>> +/* Auxillary info bits */
>>>> +#define AUXINFO_HISTORY_OFFSET REG_GENMASK(31, 29)
>>>> +
>>>> struct pcode_err_decode {
>>>> int errno;
>>>> const char *str;
>>>> diff --git a/drivers/gpu/drm/xe/xe_survivability_mode.c b/drivers/gpu/drm/xe/xe_survivability_mode.c
>>>> new file mode 100644
>>>> index 000000000000..077422ae009d
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/xe/xe_survivability_mode.c
>>>> @@ -0,0 +1,231 @@
>>>> +// SPDX-License-Identifier: MIT
>>>> +/*
>>>> + * Copyright © 2025 Intel Corporation
>>>> + */
>>>> +
>>>> +#include "xe_survivability_mode.h"
>>>> +#include "xe_survivability_mode_types.h"
>>>> +
>>>> +#include <drm/drm_managed.h>
>>>> +#include <linux/kobject.h>
>>>> +#include <linux/pci.h>
>>>> +#include <linux/sysfs.h>
>>>> +
>>>> +#include "xe_device.h"
>>>> +#include "xe_gt.h"
>>>> +#include "xe_mmio.h"
>>>> +#include "xe_pcode_api.h"
>>>> +
>>>> +#define MAX_SCRATCH_MMIO 8
>>>> +
>>>> +/**
>>>> + * DOC: Xe Boot Survivability
>>>> + *
>>>> + * Boot Survivability is a software based workflow for recovering a system in a failed boot state
>>>> + * Here system recoverability is concerned with recovering the firmware responsible for boot.
>>>> + *
>>>> + * This is implemented by loading the driver with bare minimum (no drm card) to allow the firmware
>>>> + * to be flashed through mei and collect telemetry. The driver's probe flow is modified
>>>> + * such that it enters survivability mode when pcode initialization is incomplete and boot status
>>>> + * denotes a failure. The driver then populates the survivability_mode PCI sysfs indicating
>>>> + * survivability mode and provides additional information required for debug
>>>> + *
>>>> + * KMD exposes below admin-only readable sysfs in survivability mode
>>>> + *
>>>> + * device/survivability_mode: The presence of this file indicates that the card is in survivability
>>>> + * mode. Also, provides additional information on why the driver entered
>>>> + * survivability mode.
>>>> + *
>>>> + * Capability Information - Provides boot status
>>>> + * Postcode Information - Provides information about the failure
>>>> + * Overflow Information - Provides history of previous failures
>>>> + * Auxillary Information - Certain failures may have information in
>>>> + * addition to postcode information
>>>> + */
>>>> +
>>>> +static void set_survivability_info(struct xe_device *xe, struct xe_survivability_info *info,
>>>> + int id, char *name)
>>>> +{
>>>> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>>>> +
>>>> + strscpy(info[id].name, name, sizeof(info[id].name));
>>>> + info[id].reg = PCODE_SCRATCH_ADDR(id).raw;
>>>> + info[id].value = xe_mmio_read32(mmio, PCODE_SCRATCH_ADDR(id));
>>>> +}
>>>> +
>>>> +static int populate_survivability_info(struct xe_device *xe)
>>>> +{
>>>> + struct xe_survivability *survivability = &xe->survivability;
>>>> + struct xe_survivability_info *info = survivability->info;
>>>> + u32 capability_info;
>>>> + int id = 0;
>>>> +
>>>> + set_survivability_info(xe, info, id, "Capability Info");
>>>> + capability_info = info[id].value;
>>>> +
>>>> + if (capability_info & HISTORY_TRACKING) {
>>>> + id++;
>>>> + set_survivability_info(xe, info, id, "Postcode Info");
>>>> +
>>>> + if (capability_info & OVERFLOW_SUPPORT) {
>>>> + id = REG_FIELD_GET(OVERFLOW_REG_OFFSET, capability_info);
>>>> + /* ID should be within MAX_SCRATCH_MMIO */
>>>> + if (id >= MAX_SCRATCH_MMIO)
>>>> + return -EINVAL;
>>>> + set_survivability_info(xe, info, id, "Overflow Info");
>>>> + }
>>>> + }
>>>> +
>>>> + if (capability_info & AUXINFO_SUPPORT) {
>>>> + u32 aux_info;
>>>> + int index = 0;
>>>> + char name[NAME_MAX];
>>>> +
>>>> + id = REG_FIELD_GET(AUXINFO_REG_OFFSET, capability_info);
>>>> + if (id >= MAX_SCRATCH_MMIO)
>>>> + return -EINVAL;
>>>> +
>>>> + snprintf(name, NAME_MAX, "Auxiliary Info %d", index);
>>>> + set_survivability_info(xe, info, id, name);
>>>> + aux_info = info[id].value;
>>>> +
>>>> + while ((id = REG_FIELD_GET(AUXINFO_HISTORY_OFFSET, aux_info)) &&
>>>> + (id < MAX_SCRATCH_MMIO)) {
>>>
>>> This is a clear case where 'for' is better. But also, generally here we
>>> try to limit while usages...
>> This is similar to linked list with the address of prev aux registers in the
>> AUXINFO_HISTORY_OFFSET. So used while.
>>
>> Using for would be like below
>>
>> for (id = REG_FIELD_GET(AUXINFO_HISTORY_OFFSET, aux_info);
>> aux_info && id < MAX_SCRATCH_MMIO; id
>> =REG_FIELD_GET(AUXINFO_HISTORY_OFFSET, aux_info))
>
> I believe the right way is something like:
>
> if (capability_info & AUXINFO_SUPPORT) {
> //you could move all declarations to upper scope, or move this to a separate function
>
> id = REG_FIELD_GET(AUXINFO_REG_OFFSET, capability_info);
> if (id >= MAX_SCRATCH_MMIO)
> return -EINVAL;
>
> snprintf(name, NAME_MAX, "Auxiliary Info %d", index);
> set_survivability_info(xe, info, id, name);
>
> for (index = 1, aux_info = info[id].value;
> aux_info && && id < MAX_SCRATCH_MMIO;
> aux_info = info[id].value,
> id = REG_FIELD_GET(AUXINFO_HISTORY_OFFSET, aux_info),
> index++) {
> snprintf(name, NAME_MAX, "Prev Auxiliary Info %d", index);
> set_survivability_info(xe, info, id, name);
> }
> }
>
>>
>> Isn't while better?
>
> just by removing the duplication of aux_info = info[id].value
> and by making it clear what is the start, what is the condition and what
> is the iteration fields, I do believe 'for' is better than while...
with the index also moved for is better. Thank you.
will fix this
>
>>>
>>>> + index++;
>>>> + snprintf(name, NAME_MAX, "Prev Auxiliary Info %d", index);
>>>> + set_survivability_info(xe, info, id, name);
>>>> + aux_info = info[id].value;
>>>> + }
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void log_survivability_info(struct xe_device *xe)
>>>> +{
>>>> + struct xe_survivability *survivability = &xe->survivability;
>>>> + struct xe_survivability_info *info = survivability->info;
>>>> + int id;
>>>> +
>>>> + drm_info(&xe->drm, "Survivability Boot Status : Critical Failure (%d)\n",
>>>> + survivability->boot_status);
>>>
>>> hmm, since we are avoiding the drm, should we really use drm variants here?
>>> or the pci/dev ones?!
>>
>> drm variants use the dev ones and prints the prefix if drm is not null.
>> Will change the drm_info in this file but the logs in mei and vsec
>> initialization would have to be retained.
>
> ack
>
>>>
>>>> + for (id = 0; id < MAX_SCRATCH_MMIO; id++) {
>>>> + if (info[id].reg)
>>>> + drm_info(&xe->drm, "%s: 0x%x - 0x%x\n", info[id].name,
>>>> + info[id].reg, info[id].value);
>>>> + }
>>>> +}
>>>> +
>>>> +static ssize_t survivability_mode_show(struct device *dev,
>>>> + struct device_attribute *attr, char *buff)
>>>> +{
>>>> + struct pci_dev *pdev = to_pci_dev(dev);
>>>> + struct xe_device *xe = pdev_to_xe_device(pdev);
>>>> + struct xe_survivability *survivability = &xe->survivability;
>>>> + struct xe_survivability_info *info = survivability->info;
>>>> + int index = 0, count = 0;
>>>> +
>>>> + for (index = 0; index < MAX_SCRATCH_MMIO; index++) {
>>>> + if (info[index].reg)
>>>> + count += sysfs_emit_at(buff, count, "%s: 0x%x - 0x%x\n", info[index].name,
>>>> + info[index].reg, info[index].value);
>>>> + }
>>>> +
>>>> + return count;
>>>> +}
>>>> +
>>>> +static DEVICE_ATTR_ADMIN_RO(survivability_mode);
>>>> +
>>>> +static void enable_survivability_mode(struct xe_device *xe)
>>>> +{
>>>> + struct xe_survivability *survivability = &xe->survivability;
>>>> + struct device *dev = xe->drm.dev;
>>>
>>> do we really have this pointer valid at this point?!
>> This is allocated in xe_device_create. Registration is done later in
>> xe_device_probe so the prints and xe->drm.dev will be valid
>
> cool then, thanks for the confirmation
>
>>
>> Thanks
>> Riana
>>>
>>>> + int ret = 0;
>>>> +
>>>> + /* set survivability mode */
>>>> + survivability->mode = true;
>>>> + drm_info(&xe->drm, "In Survivability Mode\n");
>>>
>>> same here...
>>>
>>>> +
>>>> + /* create survivability mode sysfs */
>>>> + ret = sysfs_create_file(&dev->kobj, &dev_attr_survivability_mode.attr);
>>>> + if (ret) {
>>>> + drm_warn(&xe->drm, "Failed to create survivability sysfs files\n");
>>>> + return;
>>>> + }
>>>> +}
>>>> +
>>>> +/**
>>>> + * xe_survivability_mode_required- checks if survivability mode is required
>>>> + * @xe: xe device instance
>>>> + *
>>>> + * This function reads the boot status of Pcode capability register
>>>> + *
>>>> + * Return: true if boot status indicates failure, false otherwise
>>>> + */
>>>> +bool xe_survivability_mode_required(struct xe_device *xe)
>>>> +{
>>>> + struct xe_survivability *survivability = &xe->survivability;
>>>> + struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>>>> + u32 data;
>>>> +
>>>> + data = xe_mmio_read32(mmio, PCODE_SCRATCH_ADDR(0));
>>>> + survivability->boot_status = REG_FIELD_GET(BOOT_STATUS, data);
>>>> +
>>>> + return (survivability->boot_status == NON_CRITICAL_FAILURE ||
>>>> + survivability->boot_status == CRITICAL_FAILURE);
>>>> +}
>>>> +
>>>> +/**
>>>> + * xe_survivability_mode_remove - remove survivability mode
>>>> + * @xe: xe device instance
>>>> + *
>>>> + * clean up sysfs entries of survivability mode
>>>> + */
>>>> +void xe_survivability_mode_remove(struct xe_device *xe)
>>>> +{
>>>> + struct xe_survivability *survivability = &xe->survivability;
>>>> + struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>>> +
>>>> + sysfs_remove_file(&xe->drm.dev->kobj, &dev_attr_survivability_mode.attr);
>>>> + kfree(survivability->info);
>>>> + pci_set_drvdata(pdev, NULL);
>>>> +}
>>>> +
>>>> +/**
>>>> + * xe_survivability_mode_init - Initialize the survivability mode
>>>> + * @xe: xe device instance
>>>> + *
>>>> + * Initializes the sysfs and required actions to enter survivability mode
>>>> + */
>>>> +void xe_survivability_mode_init(struct xe_device *xe)
>>>> +{
>>>> + struct xe_survivability *survivability = &xe->survivability;
>>>> + struct xe_survivability_info *info;
>>>> + int ret = 0;
>>>> +
>>>> + survivability->size = MAX_SCRATCH_MMIO;
>>>> +
>>>> + info = kcalloc(survivability->size, sizeof(*info), GFP_KERNEL);
>>>> + if (!info) {
>>>> + ret = -ENOMEM;
>>>> + goto err;
>>>> + }
>>>> +
>>>> + survivability->info = info;
>>>> +
>>>> + ret = populate_survivability_info(xe);
>>>> + if (ret)
>>>> + goto err;
>>>> +
>>>> + /* Only log debug information and exit if it is a critical failure */
>>>> + if (survivability->boot_status == CRITICAL_FAILURE) {
>>>> + log_survivability_info(xe);
>>>> + kfree(survivability->info);
>>>> + return;
>>>> + }
>>>> +
>>>> + enable_survivability_mode(xe);
>>>> +err:
>>>> + if (ret)
>>>> + drm_warn(&xe->drm, "%s failed, err: %d\n", __func__, ret);
>>>
>>> same...
>>>
>>>> +}
>>>> diff --git a/drivers/gpu/drm/xe/xe_survivability_mode.h b/drivers/gpu/drm/xe/xe_survivability_mode.h
>>>> new file mode 100644
>>>> index 000000000000..410e3ee5f5d1
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/xe/xe_survivability_mode.h
>>>> @@ -0,0 +1,17 @@
>>>> +/* SPDX-License-Identifier: MIT */
>>>> +/*
>>>> + * Copyright © 2025 Intel Corporation
>>>> + */
>>>> +
>>>> +#ifndef _XE_SURVIVABILITY_MODE_H_
>>>> +#define _XE_SURVIVABILITY_MODE_H_
>>>> +
>>>> +#include <linux/types.h>
>>>> +
>>>> +struct xe_device;
>>>> +
>>>> +void xe_survivability_mode_init(struct xe_device *xe);
>>>> +void xe_survivability_mode_remove(struct xe_device *xe);
>>>> +bool xe_survivability_mode_required(struct xe_device *xe);
>>>> +
>>>> +#endif /* _XE_SURVIVABILITY_MODE_H_ */
>>>> diff --git a/drivers/gpu/drm/xe/xe_survivability_mode_types.h b/drivers/gpu/drm/xe/xe_survivability_mode_types.h
>>>> new file mode 100644
>>>> index 000000000000..19d433e253df
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/xe/xe_survivability_mode_types.h
>>>> @@ -0,0 +1,35 @@
>>>> +/* SPDX-License-Identifier: MIT */
>>>> +/*
>>>> + * Copyright © 2025 Intel Corporation
>>>> + */
>>>> +
>>>> +#ifndef _XE_SURVIVABILITY_MODE_TYPES_H_
>>>> +#define _XE_SURVIVABILITY_MODE_TYPES_H_
>>>> +
>>>> +#include <linux/limits.h>
>>>> +#include <linux/types.h>
>>>> +
>>>> +struct xe_survivability_info {
>>>> + char name[NAME_MAX];
>>>> + u32 reg;
>>>> + u32 value;
>>>> +};
>>>> +
>>>> +/**
>>>> + * struct xe_survivability: Contains survivability mode information
>>>> + */
>>>> +struct xe_survivability {
>>>> + /** @info: struct that holds survivability info from scratch registers */
>>>> + struct xe_survivability_info *info;
>>>> +
>>>> + /** @size: number of scratch registers */
>>>> + u32 size;
>>>> +
>>>> + /** @boot_status: indicates critical/non critical boot failure */
>>>> + u8 boot_status;
>>>> +
>>>> + /** @mode: boolean to indicate survivability mode */
>>>> + bool mode;
>>>> +};
>>>> +
>>>
>>> I believe the only blocker is the while-vs-for loop. I believe the 'drm'
>>> could be avoided, but not a big deal if it is really working...
>>>
>>>> +#endif /* _XE_SURVIVABILITY_MODE_TYPES_H_ */
>>>> --
>>>> 2.47.1
>>>>
>>
More information about the Intel-xe
mailing list