Re: [PATCH v1] drm/xe: Don't expose device attributes for VFs

[Raag Jadav <raag.jadav@intel.com>]

On Sat, Jul 12, 2025 at 02:19:50AM +0530, Summers, Stuart wrote:
> On Sat, 2025-07-12 at 00:38 +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 when xe->info.skip_pcode is set. Since
> > this
> > is true for IS_SRIOV_VF() cases, a much robust fix would be to not
> > expose
> > device attributes for VFs, which covers wider pcode mailbox users.
> 
> I'm not fully understanding your reasoning here. Why can't we just use
> a check against skip_pcode in that case? VF might be the main user
> there, but it isn't inconceivable that we will use that in other
> scenarios too and then hit a similar problem of accessing an
> uninitialized variable.

This also covers non-mailbox attributes which would be unusable for VFs.
So we can either fix the commit message or use skip_pcode check against
mailbox attributes, whichever makes more sense.

Raag

> > Fixes: a7f87deac229 ("drm/xe: Default auto_link_downgrade status to
> > false")
> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_device_sysfs.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c
> > b/drivers/gpu/drm/xe/xe_device_sysfs.c
> > index e5fd0cd537bc..8250aa4620ee 100644
> > --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> > +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> > @@ -259,8 +259,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;
> >         int ret;
> >  
> >         xe_pm_runtime_get(xe);
> > @@ -297,6 +296,9 @@ int xe_device_sysfs_init(struct xe_device *xe)
> >         struct device *dev = xe->drm.dev;
> >         int ret;
> >  
> > +       if (IS_SRIOV_VF(xe))
> > +               return 0;
> > +
> >         if (xe->d3cold.capable) {
> >                 ret = sysfs_create_file(&dev->kobj,
> > &dev_attr_vram_d3cold_threshold.attr);
> >                 if (ret)
>