[PATCH] drm/xe: Skip creation of pcode sysfs files when pcode is disabled

[PATCH] drm/xe: Skip creation of pcode sysfs files when pcode is disabled

[Andi Shyti]

From: Andi Shyti <andi.shyti@linux.intel.com>

Coverity warns that 'cap' may be used uninitialised. If pcode
is disabled there is no need to go through the hassle of a
pcode read or taking a PM reference.

Check skip_pcode early in the function and return if it is set.

No change for platforms where pcode is enabled.

Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
---
 drivers/gpu/drm/xe/xe_device_sysfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
index bd9015761aa0..3a083c215891 100644
--- a/drivers/gpu/drm/xe/xe_device_sysfs.c
+++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
@@ -156,6 +156,9 @@ static int late_bind_create_files(struct device *dev)
 	u32 cap;
 	int ret;
 
+	if (xe->info.skip_pcode)
+		return 0;
+
 	xe_pm_runtime_get(xe);
 
 	ret = xe_pcode_read(root, PCODE_MBOX(PCODE_LATE_BINDING, GET_CAPABILITY_STATUS, 0),
-- 
2.50.0

Re: [PATCH] drm/xe: Skip creation of pcode sysfs files when pcode is disabled

[Rodrigo Vivi]

On Tue, Aug 19, 2025 at 04:55:29PM -0100, Andi Shyti wrote:
> From: Andi Shyti <andi.shyti@linux.intel.com>
> 
> Coverity warns that 'cap' may be used uninitialised. If pcode
> is disabled there is no need to go through the hassle of a
> pcode read or taking a PM reference.

Please mark it as false positive!
We will only get here for BMG which has pcode for sure.

> 
> Check skip_pcode early in the function and return if it is set.
> 
> No change for platforms where pcode is enabled.
> 
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device_sysfs.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
> index bd9015761aa0..3a083c215891 100644
> --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> @@ -156,6 +156,9 @@ static int late_bind_create_files(struct device *dev)
>  	u32 cap;
>  	int ret;
>  
> +	if (xe->info.skip_pcode)
> +		return 0;
> +
>  	xe_pm_runtime_get(xe);
>  
>  	ret = xe_pcode_read(root, PCODE_MBOX(PCODE_LATE_BINDING, GET_CAPABILITY_STATUS, 0),
> -- 
> 2.50.0
> 

Re: [PATCH] drm/xe: Skip creation of pcode sysfs files when pcode is disabled

[Andi Shyti]

Hi Rodrigo,

On Fri, Aug 22, 2025 at 12:30:02PM -0400, Rodrigo Vivi wrote:
> On Tue, Aug 19, 2025 at 04:55:29PM -0100, Andi Shyti wrote:
> > From: Andi Shyti <andi.shyti@linux.intel.com>
> > 
> > Coverity warns that 'cap' may be used uninitialised. If pcode
> > is disabled there is no need to go through the hassle of a
> > pcode read or taking a PM reference.
> 
> Please mark it as false positive!

this patch is not for fixing the Coverity warning, but I saw it
useless to step any further if there is skip pcode.

The same check is done later in the function, but in the meantime
we have done a few things that we could have spared.

Andi

> We will only get here for BMG which has pcode for sure.
> 
> > 
> > Check skip_pcode early in the function and return if it is set.
> > 
> > No change for platforms where pcode is enabled.
> > 
> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_device_sysfs.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
> > index bd9015761aa0..3a083c215891 100644
> > --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> > +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> > @@ -156,6 +156,9 @@ static int late_bind_create_files(struct device *dev)
> >  	u32 cap;
> >  	int ret;
> >  
> > +	if (xe->info.skip_pcode)
> > +		return 0;
> > +
> >  	xe_pm_runtime_get(xe);
> >  
> >  	ret = xe_pcode_read(root, PCODE_MBOX(PCODE_LATE_BINDING, GET_CAPABILITY_STATUS, 0),
> > -- 
> > 2.50.0
> > 

Re: [PATCH] drm/xe: Skip creation of pcode sysfs files when pcode is disabled

[Raag Jadav]

On Mon, Aug 25, 2025 at 04:31:23PM +0200, Andi Shyti wrote:
> Hi Rodrigo,
> 
> On Fri, Aug 22, 2025 at 12:30:02PM -0400, Rodrigo Vivi wrote:
> > On Tue, Aug 19, 2025 at 04:55:29PM -0100, Andi Shyti wrote:
> > > From: Andi Shyti <andi.shyti@linux.intel.com>
> > > 
> > > Coverity warns that 'cap' may be used uninitialised. If pcode
> > > is disabled there is no need to go through the hassle of a
> > > pcode read or taking a PM reference.
> > 
> > Please mark it as false positive!
> 
> this patch is not for fixing the Coverity warning, but I saw it
> useless to step any further if there is skip pcode.
> 
> The same check is done later in the function, but in the meantime
> we have done a few things that we could have spared.

I tried something similar a few days ago, but perhaps not very convincingly
I presume.

[1] https://lore.kernel.org/intel-xe/20250806152256.748057-1-raag.jadav@intel.com/

Raag

> > We will only get here for BMG which has pcode for sure.
> > 
> > > 
> > > Check skip_pcode early in the function and return if it is set.
> > > 
> > > No change for platforms where pcode is enabled.
> > > 
> > > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_device_sysfs.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
> > > index bd9015761aa0..3a083c215891 100644
> > > --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> > > +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> > > @@ -156,6 +156,9 @@ static int late_bind_create_files(struct device *dev)
> > >  	u32 cap;
> > >  	int ret;
> > >  
> > > +	if (xe->info.skip_pcode)
> > > +		return 0;
> > > +
> > >  	xe_pm_runtime_get(xe);
> > >  
> > >  	ret = xe_pcode_read(root, PCODE_MBOX(PCODE_LATE_BINDING, GET_CAPABILITY_STATUS, 0),
> > > -- 
> > > 2.50.0
> > > 

Re: [PATCH] drm/xe: Skip creation of pcode sysfs files when pcode is disabled

[Rodrigo Vivi]

On Mon, Aug 25, 2025 at 04:48:28PM +0200, Raag Jadav wrote:
> On Mon, Aug 25, 2025 at 04:31:23PM +0200, Andi Shyti wrote:
> > Hi Rodrigo,
> > 
> > On Fri, Aug 22, 2025 at 12:30:02PM -0400, Rodrigo Vivi wrote:
> > > On Tue, Aug 19, 2025 at 04:55:29PM -0100, Andi Shyti wrote:
> > > > From: Andi Shyti <andi.shyti@linux.intel.com>
> > > > 
> > > > Coverity warns that 'cap' may be used uninitialised. If pcode
> > > > is disabled there is no need to go through the hassle of a
> > > > pcode read or taking a PM reference.
> > > 
> > > Please mark it as false positive!
> > 
> > this patch is not for fixing the Coverity warning, but I saw it
> > useless to step any further if there is skip pcode.
> > 
> > The same check is done later in the function, but in the meantime
> > we have done a few things that we could have spared.
> 
> I tried something similar a few days ago, but perhaps not very convincingly
> I presume.
> 
> [1] https://lore.kernel.org/intel-xe/20250806152256.748057-1-raag.jadav@intel.com/

Cc a bunch of folks...

Well, we have a mess to fix here indeed.

We should not be mixing platform checks with info checks that are coming from
platform definition...

Look comment in xe_info_init_early:
"Initialize device info content that only depends on static driver_data
passed to the driver at probe time from PCI ID table."

However this thread made me to realize that we are not respecting that
and we are indeed changing the info at runtime and not only based
on the platform:

sriov_update_device_info()

Hence these kind of patches poping up.

Jani, I remember you did a very good organization in i915 with the static
info vs info that can change in runtime. Any advice, guidance here?

Thanks,
Rodrigo.

> 
> Raag
> 
> > > We will only get here for BMG which has pcode for sure.
> > > 
> > > > 
> > > > Check skip_pcode early in the function and return if it is set.
> > > > 
> > > > No change for platforms where pcode is enabled.
> > > > 
> > > > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/xe/xe_device_sysfs.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
> > > > index bd9015761aa0..3a083c215891 100644
> > > > --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> > > > +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> > > > @@ -156,6 +156,9 @@ static int late_bind_create_files(struct device *dev)
> > > >  	u32 cap;
> > > >  	int ret;
> > > >  
> > > > +	if (xe->info.skip_pcode)
> > > > +		return 0;
> > > > +
> > > >  	xe_pm_runtime_get(xe);
> > > >  
> > > >  	ret = xe_pcode_read(root, PCODE_MBOX(PCODE_LATE_BINDING, GET_CAPABILITY_STATUS, 0),
> > > > -- 
> > > > 2.50.0
> > > > 

Re: [PATCH] drm/xe: Skip creation of pcode sysfs files when pcode is disabled

[Michal Wajdeczko]

On 8/25/2025 5:26 PM, Rodrigo Vivi wrote:
> On Mon, Aug 25, 2025 at 04:48:28PM +0200, Raag Jadav wrote:
>> On Mon, Aug 25, 2025 at 04:31:23PM +0200, Andi Shyti wrote:
>>> Hi Rodrigo,
>>>
>>> On Fri, Aug 22, 2025 at 12:30:02PM -0400, Rodrigo Vivi wrote:
>>>> On Tue, Aug 19, 2025 at 04:55:29PM -0100, Andi Shyti wrote:
>>>>> From: Andi Shyti <andi.shyti@linux.intel.com>
>>>>>
>>>>> Coverity warns that 'cap' may be used uninitialised. If pcode
>>>>> is disabled there is no need to go through the hassle of a
>>>>> pcode read or taking a PM reference.
>>>>
>>>> Please mark it as false positive!
>>>
>>> this patch is not for fixing the Coverity warning, but I saw it
>>> useless to step any further if there is skip pcode.
>>>
>>> The same check is done later in the function, but in the meantime
>>> we have done a few things that we could have spared.
>>
>> I tried something similar a few days ago, but perhaps not very convincingly
>> I presume.
>>
>> [1] https://lore.kernel.org/intel-xe/20250806152256.748057-1-raag.jadav@intel.com/
> 
> Cc a bunch of folks...
> 
> Well, we have a mess to fix here indeed.
> 
> We should not be mixing platform checks with info checks that are coming from
> platform definition...
> 
> Look comment in xe_info_init_early:
> "Initialize device info content that only depends on static driver_data
> passed to the driver at probe time from PCI ID table."
> 
> However this thread made me to realize that we are not respecting that
> and we are indeed changing the info at runtime and not only based
> on the platform:
> 
> sriov_update_device_info()

this was due to not having two static sets of .info (one for native, other for VF)
and it shall be called ASAP (and only once) right after we detect that we are a VF

and all of this was true at the time I've added this helper but over the time
it looks that more and more code movement was happening, so I'm not sure now

> 
> Hence these kind of patches poping up.

hmm, or maybe the reason is much simpler - pcode was not honoring that flag
in all cases, like the one listed below ?

btw, late_bind_remove_files() seems to be also (very) broken

> 
> Jani, I remember you did a very good organization in i915 with the static
> info vs info that can change in runtime. Any advice, guidance here?
> 
> Thanks,
> Rodrigo.
> 
>>
>> Raag
>>
>>>> We will only get here for BMG which has pcode for sure.
>>>>
>>>>>
>>>>> Check skip_pcode early in the function and return if it is set.
>>>>>
>>>>> No change for platforms where pcode is enabled.
>>>>>
>>>>> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
>>>>> ---
>>>>>  drivers/gpu/drm/xe/xe_device_sysfs.c | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
>>>>> index bd9015761aa0..3a083c215891 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
>>>>> @@ -156,6 +156,9 @@ static int late_bind_create_files(struct device *dev)
>>>>>  	u32 cap;
>>>>>  	int ret;
>>>>>  
>>>>> +	if (xe->info.skip_pcode)
>>>>> +		return 0;
>>>>> +
>>>>>  	xe_pm_runtime_get(xe);
>>>>>  
>>>>>  	ret = xe_pcode_read(root, PCODE_MBOX(PCODE_LATE_BINDING, GET_CAPABILITY_STATUS, 0),
>>>>> -- 
>>>>> 2.50.0
>>>>>