Re: [PATCH 2/2] drm/xe/i2c: Expose AMC Alert reason sysfs

["Nilawar, Badal" <badal.nilawar@intel.com>]

On 04-09-2026 17:38, Nilawar, Badal wrote:
>
> On 25-08-2026 00:56, Rodrigo Vivi wrote:
>> On Sun, Aug 23, 2026 at 04:05:26PM +0530, Badal Nilawar wrote:
>>> AMC raises an SMBUS alert before performing a power removal or
>>> power-cycle operation. The xe driver then places the device into
>>> vendor-specific wedge mode until the recovery is performed.
>>>
>>> Expose a read-only xe_amc_alert_reason sysfs attribute to help users
>>> identify the required recovery action.
>>>
>>> Assisted-by: Claude:claude-opus-4.8
>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>>> ---
>>>   .../ABI/testing/sysfs-driver-intel-xe-amc     | 21 ++++++
>>>   drivers/gpu/drm/xe/xe_amc.c                   | 65 
>>> ++++++++++++++++++-
>>>   2 files changed, 84 insertions(+), 2 deletions(-)
>>>   create mode 100644 
>>> Documentation/ABI/testing/sysfs-driver-intel-xe-amc
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-xe-amc 
>>> b/Documentation/ABI/testing/sysfs-driver-intel-xe-amc
>>> new file mode 100644
>>> index 000000000000..b3de933efe11
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-driver-intel-xe-amc
>>> @@ -0,0 +1,21 @@
>>> +What:        /sys/bus/pci/drivers/xe/.../xe_amc_alert_reason
>> Should we do <bdf> instead of ... ?!
> Sure
>>
>>> +Date:        August 2026
>>> +KernelVersion:    7.3
>> this will be 7.4
> I will fix this.
>>
>>> +Contact: intel-xe@lists.freedesktop.org
>>> +Description:
>>> +        This file exposes the reason for the most recent Add-In
>>> +        Management Controller (AMC) alert on Intel Xe platforms.
>>> +
>>> +        An AMC alert is delivered via an SMBUS interrupt and causes 
>>> the
>>> +        device to be wedged, requiring vendor-specific recovery. This
>>> +        attribute is created when such an alert is handled and is
>>> +        available to all users as read-only.
>>> +
>>> +        Read returns a single line containing one of the following
>>> +        alert reasons:
>>> +
>>> +            Firmware Download
>>> +            Thermal Trip
>>> +            OOB Request
>>> +            OOB Reset
>>> +            Catastrophic
>>> diff --git a/drivers/gpu/drm/xe/xe_amc.c b/drivers/gpu/drm/xe/xe_amc.c
>>> index 8ecadee6eea3..ceb2c4d618fe 100644
>>> --- a/drivers/gpu/drm/xe/xe_amc.c
>>> +++ b/drivers/gpu/drm/xe/xe_amc.c
>>> @@ -18,6 +18,7 @@
>>>   #include "xe_device.h"
>>>   #include "xe_i2c.h"
>>>   #include "xe_mmio.h"
>>> +#include "xe_printk.h"
>>>     /**
>>>    * DOC: Add-In Management Controller (AMC)
>>> @@ -56,6 +57,8 @@ static const char * const amc_alert[] = {
>>>   struct xe_amc {
>>>       struct xe_i2c *i2c;
>>>       struct work_struct work;
>>> +    u8 alert_reason;
>>> +    bool sysfs_created;
>>>   };
>>>     struct amc_header {
>>> @@ -104,6 +107,54 @@ static const struct amc_request 
>>> amc_get_alert_reason = {
>>>       },
>>>   };
>>>   +/**
>>> + * DOC: AMC Alert Reason
>>> + *
>>> + * On Intel Xe platforms, AMC sends an alert notification via an 
>>> SMBUS interrupt
>>> + * to notify events such as firmware download, thermal trip or a
>>> + * catastrophic error. See enum xe_amc_alert for the full list of 
>>> reasons.
>>> + * Upon an AMC alert the device is wedged and requires 
>>> vendor-specific recovery.
>>> + *
>>> + * The alert reason is exposed through
>>> + * /sys/bus/pci/drivers/xe/.../xe_amc_alert_reason
>> ditto
>>
>>> + *
>>> + * See Documentation/ABI/testing/sysfs-driver-intel-xe-amc for the ABI
>>> + * specification.
>>> + */
>>> +
>>> +static ssize_t xe_amc_alert_reason_show(struct device *dev,
>>> +                    struct device_attribute *attr, char *buff)
>>> +{
>>> +    struct xe_device *xe = pdev_to_xe_device(to_pci_dev(dev));
>>> +    struct xe_amc *amc = xe->i2c->amc;
>>> +
>>> +    return sysfs_emit(buff, "%s\n", amc_alert[amc->alert_reason]);
>>> +}
>>> +static DEVICE_ATTR_RO(xe_amc_alert_reason);
>>> +
>>> +static void xe_remove_amc_alert_sysfs(struct xe_device *xe)
>>> +{
>>> +    if (xe->i2c->amc->sysfs_created)
>>> +        device_remove_file(xe->drm.dev, 
>>> &dev_attr_xe_amc_alert_reason);
>>> +}
>>> +
>>> +static void xe_create_amc_alert_sysfs(struct xe_device *xe)
>>> +{
>>> +    struct device *dev = xe->drm.dev;
>>> +    int ret;
>>> +
>>> +    if (xe->i2c->amc->sysfs_created)
>>> +        return;
>> Why do you need to track this?!
>> if this can be called from multiple places shoulnd't you protect with 
>> lock?
>
> This will not be called from multiple places. Will drop this check.

Forgot to mention, I will create this sysfs probe time. When no valid 
alert reason it will give EINVAL.

Thanks,
Badal

>
> Thanks,
> Badal
>
>>
>> But I prefer that you ensure this is absolutely called only once and
>> remove this check.
>>
>>> +
>>> +    ret = device_create_file(dev, &dev_attr_xe_amc_alert_reason);
>>> +    if (ret)
>>> +        goto failed;
>>> +
>>> +    xe->i2c->amc->sysfs_created = true;
>>> +failed:
>>> +    dev_err(dev, "Failed to create sysfs file for amc alert 
>>> reason\n");
>>> +}
>>> +
>>>   static void xe_amc_work(struct work_struct *work)
>>>   {
>>>       const struct amc_request *request = &amc_get_alert_reason;
>>> @@ -158,10 +209,16 @@ static void xe_amc_work(struct work_struct *work)
>>>       case AMC_ALERT_THERMAL_TRIP:
>>>       case AMC_ALERT_OOB_REQUEST:
>>>       case AMC_ALERT_OOB_RESET:
>>> -    case AMC_ALERT_CATERR:
>>> +    case AMC_ALERT_CATERR: {
>>> +        struct xe_device *xe = i2c_client_to_xe_device(client);
>>> +
>>>           dev_warn(amc->i2c->drm_dev, "AMC Alert: %s\n", 
>>> amc_alert[alert_reason]);
>>> - xe_device_declare_wedged(i2c_client_to_xe_device(client));
>>> +        amc->alert_reason = alert_reason;
>>> +        xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_VENDOR);
>>> +        xe_device_declare_wedged(xe);
>>> +        xe_create_amc_alert_sysfs(xe);
>>>           break;
>>> +    }
>>>       default:
>>>           dev_warn(amc->i2c->drm_dev, "unknown AMC alert: %d\n", 
>>> alert_reason);
>>>           break;
>>> @@ -190,8 +247,12 @@ int xe_amc_init(struct xe_i2c *i2c)
>>>     void xe_amc_exit(struct xe_i2c *i2c)
>>>   {
>>> +    struct xe_device *xe;
>>> +
>>>       if (i2c->amc) {
>>>           cancel_work_sync(&i2c->amc->work);
>>> +        xe = i2c_client_to_xe_device(i2c->client[XE_I2C_CLIENT_AMC]);
>>> +        xe_remove_amc_alert_sysfs(xe);
>>>           kfree(i2c->amc);
>>>       }
>>>   }
>>> -- 
>>> 2.54.0
>>>