Re: [PATCH v2 1/2] drm/xe: Expose retired VRAM pages via drm-ras

[Aravind Iddamsetty <aravind.iddamsetty@linux.intel.com>]

Hi Riana,

On 30-10-2026 11:46, Tauro, Riana wrote:
> Hi Aravind/Rodrigo
>
> How about using a generic telemetry node for this series from
> https://lore.kernel.org/dri-devel/20261029203000.1763447-1-zachary.mckevitt@oss.qualcomm.com/T/#t
> instead of a dedicated node for vram-retired-pages?
I don't think the telemetry node is a good fit for retired-resources,
the two solve
fairly different problems:

Telemetry models a fixed, compile-time-known set of named scalar
readings (temperature, voltage, power, ...), each a single s64
value, read/written one field at a time via doit. The dump op only
enumerates the field schema (field id + field-name), it does not
bulk-return values.

A retired VRAM page is a multi-attribute record (address, size,
status), discovered at runtime when a page actually faults. There's
no way to pre-enumerate a fixed "field id" per page the way
QAIC_DEVINFO_* is enumerated at build time, and the count is
unbounded  and grows over the life of the device.

Trying to map retired pages onto telemetry fields would mean either:
(a) registering a field per page per attribute ahead of time, which
isn't possible since addresses aren't known until the fault
happens, or
(b) doing N doit round-trips (one per field) to read back what is
already a single structured dump today.

Our current get-retired-resources dump returns the full list
(address/size/status, plus a leading max-pages summary) in one
cursor-paginated sweep, with the cursor keyed by address so it stays
correct if the list mutates concurrently (pages retiring mid-dump).
Telemetry's dumpit walks a static array with an index cursor, which
doesn't give us that property.

So I'd prefer to keep DRM_RAS_NODE_TYPE_RETIRED_RESOURCES as its own
node type rather than reusing telemetry for it.

Also, RETIRED_RESOURCES is generic node not just limited to VRAM bad pages
that can extended to support a new type of resource.

Thanks,
Aravind. 
>
> Thanks
> Riana
>
> On 19-10-2026 17:26, Rodrigo Vivi wrote:
>> On Fri, Oct 16, 2026 at 03:49:13PM -0700, Jakub Kicinski wrote:
>>> On Wed,  7 Oct 2026 13:33:53 +0530 Aravind Iddamsetty wrote:
>>>> +  -
>>>> +    name: retired-resource-attrs
>>>> +    attributes:
>>>> +      -
>>>> +        name: node-id
>>>> +        type: u32
>>>> +        doc: Node ID targeted by this retired resource operation.
>>>> +      -
>>>> +        name: vram-page
>>>> +        type: nest
>>>> +        nested-attributes: vram-page-attrs
>>> why wrap attrs in a nest? Are you planning to add other resource types
>>> (than a page)? It's a common anti-pattern in Netlink to over-wrap
>>> stuff.
>> we should indeed remove the wrap and have only the page info.
>>
>>>> +        doc: >-
>>>> +             Type-specific payload. The resource type is
>>>> identified by the
>>>> +             node (its node-name); each node reports a single
>>>> resource type.
>>>> +  -
>>>> +    name: vram-page-attrs
>>>> +    attributes:
>>>> +      -
>>>> +        name: address
>>>> +        type: u64
>>>> +        doc: Device address of the retired VRAM page (e.g. DPA).
>>>> +      -
>>>> +        name: size
>>>> +        type: u64
>>> prefer uint
>>>
>>>> +        doc: Size of the retired VRAM page in bytes.
>>>> +      -
>>>> +        name: max-pages
>>>> +        type: u32
>>>> +        doc: >-
>>>> +             Maximum VRAM pages that can be retired. Present only
>>>> in the
>>>> +             leading summary entry of the dump (no
>>>> address/size/status).
>>>> +      -
>>>> +        name: status
>>>> +        type: u32
>>>> +        doc: Retirement status of the retired VRAM page.
>>>> +        enum: retired-resource-status
>>>> +      -
>>>> +        name: pad
>>>> +        type: pad
>>> And then you won't need pad
>> I noticed this in the review prompts, but I have some reservations here.
>> We know for sure our value is always u64 because it is our hw
>> definition.
>> I don't see a good reason why not use the exact size that matches our
>> hw.