[PATCH v2] drm/xe: Don't expose mailbox attributes for skip_pcode cases

[Summers, Stuart]

On Wed, 2025-08-06 at 20:52 +0530, Raag Jadav wrote:
> Commit a7f87deac229 ("drm/xe: Default auto_link_downgrade status to
> false")
> tried to fix the side effect of xe_pcode_read() returning
> successfully
> without valid data-out value. Since this is true for cases where
> skip_pcode
> flag is set, a much robust fix would be to not expose mailbox
> attributes
> at all for such cases.
> 
> v2: Check against skip_pcode (Stuart)
> 
> Fixes: a7f87deac229 ("drm/xe: Default auto_link_downgrade status to
> false")
> Signed-off-by: Raag Jadav <raag.jadav at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device_sysfs.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c
> b/drivers/gpu/drm/xe/xe_device_sysfs.c
> index bd9015761aa0..75b6cd816fa8 100644
> --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> @@ -264,8 +264,7 @@ auto_link_downgrade_status_show(struct device
> *dev, struct device_attribute *att
>  {
>         struct pci_dev *pdev = to_pci_dev(dev);
>         struct xe_device *xe = pdev_to_xe_device(pdev);
> -       /* default the auto_link_downgrade status to 0 */
> -       u32 val = 0;
> +       u32 val;

Why are we making this change again? Looking at xe_pcode_read, which
calls pcode_mailbox_rw with this value, the first thing we're doing
with this is a write:
xe_mmio_write32(mmio, PCODE_DATA0, *data0);

There's no NULL check here like we have for the second parameter in
xe_pcode_read. So we need some initial value here so we don't write
garbage.

>         int ret;
>  
>         xe_pm_runtime_get(xe);
> @@ -291,7 +290,7 @@ static void xe_device_sysfs_fini(void *arg)
>         if (xe->d3cold.capable)
>                 sysfs_remove_file(&xe->drm.dev->kobj,
> &dev_attr_vram_d3cold_threshold.attr);
>  
> -       if (xe->info.platform == XE_BATTLEMAGE) {
> +       if (xe->info.platform == XE_BATTLEMAGE && !xe-
> >info.skip_pcode) {
>                 sysfs_remove_files(&xe->drm.dev->kobj,
> auto_link_downgrade_attrs);
>                 late_bind_remove_files(xe->drm.dev);
>         }
> @@ -308,7 +307,7 @@ int xe_device_sysfs_init(struct xe_device *xe)
>                         return ret;
>         }
>  
> -       if (xe->info.platform == XE_BATTLEMAGE) {
> +       if (xe->info.platform == XE_BATTLEMAGE && !xe-
> >info.skip_pcode) {

This part looks good. We have skip_pcode in the pcode_mailbox_rw
routine, but these files are checking values here that don't make sense
if we aren't reading them.

Thanks,
Stuart

>                 ret = sysfs_create_files(&dev->kobj,
> auto_link_downgrade_attrs);
>                 if (ret)
>                         return ret;