[Intel-xe] [PATCH 1/2] drm/xe: Fix uninitialized variables

[Balasubramani Vivekanandan]

On 25.05.2023 00:04, Michał Winiarski wrote:
> On Wed, May 24, 2023 at 02:34:47PM +0530, Balasubramani Vivekanandan wrote:
> > On 23.05.2023 15:50, Michał Winiarski wrote:
> > > From: Michał Winiarski <michal.winiarski at intel.com>
> > > 
> > > Using uninitialized variables leads to undefined behavior.
> > > 
> > > Moreover, it causes the compiler to complain with:
> > > ../drivers/gpu/drm/xe/xe_vm.c:3265:40: error: variable 'vma' is uninitialized when used here [-Werror,-Wuninitialized]
> > > ../drivers/gpu/drm/xe/xe_rtp.c:118:36: error: variable 'i' is uninitialized when used here [-Werror,-Wuninitialized]
> > > ../drivers/gpu/drm/xe/xe_mocs.c:449:3: error: variable 'flags' is uninitialized when used here [-Werror,-Wuninitialized]
> > > 
> > > Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_mocs.c | 2 +-
> > >  drivers/gpu/drm/xe/xe_rtp.c  | 2 +-
> > >  drivers/gpu/drm/xe/xe_vm.c   | 2 +-
> > >  3 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_mocs.c b/drivers/gpu/drm/xe/xe_mocs.c
> > > index c7a9e733ef3b..ec82088b73f0 100644
> > > --- a/drivers/gpu/drm/xe/xe_mocs.c
> > > +++ b/drivers/gpu/drm/xe/xe_mocs.c
> > > @@ -373,7 +373,7 @@ static const struct xe_mocs_entry mtl_mocs_desc[] = {
> > >  static unsigned int get_mocs_settings(struct xe_device *xe,
> > >  				      struct xe_mocs_info *info)
> > >  {
> > > -	unsigned int flags;
> > > +	unsigned int flags = 0;
> > >  
> > >  	memset(info, 0, sizeof(struct xe_mocs_info));
> > >  
> > > diff --git a/drivers/gpu/drm/xe/xe_rtp.c b/drivers/gpu/drm/xe/xe_rtp.c
> > > index 0c6a23e14a71..86fd1025b931 100644
> > > --- a/drivers/gpu/drm/xe/xe_rtp.c
> > > +++ b/drivers/gpu/drm/xe/xe_rtp.c
> > > @@ -115,7 +115,7 @@ static void rtp_process_one(const struct xe_rtp_entry *entry, struct xe_gt *gt,
> > >  	if (!rule_matches(gt, hwe, entry))
> > >  		return;
> > >  
> > > -	for (action = &entry->actions[0]; i < entry->n_actions; action++, i++) {
> > > +	for (i = 0, action = &entry->actions[0]; i < entry->n_actions; action++, i++) {
> > >  		if ((entry->flags & XE_RTP_ENTRY_FLAG_FOREACH_ENGINE) ||
> > >  		    (action->flags & XE_RTP_ACTION_FLAG_ENGINE_BASE))
> > >  			mmio_base = hwe->mmio_base;
> > > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > > index a0306526b269..109a5d93be89 100644
> > > --- a/drivers/gpu/drm/xe/xe_vm.c
> > > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > > @@ -3262,7 +3262,7 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> > >  		if (!vmas[i])
> > >  			break;
> > >  
> > > -		list_for_each_entry_safe(vma, next, &vma->unbind_link,
> > > +		list_for_each_entry_safe(vma, next, &vmas[i]->unbind_link,
> > 
> > This seems to be an unrelated fix. Can this be separted into a different
> > commit if not already merged?

At first sight, change looked like fixing a bug which used variable vma
instead of vmas. So I thought this bugfix sneaked in by mistake.

I realize now it was also reported by the check for uninitialized
variable.

Regards,
Bala

> 
> Unrelated to what?
> All changes present in this commit fix the same "category" of a problem. 
> 
> The commit fixes all 3 occurences of using uninitialized variables.
> In this case, vma is used both as an iterator (which is fine) and
> container (vma->unbind_link - which is not fine, as vma is
> not initialized).
> 
> Are you asking to split it into 3 separate commits?
> Or are you saying that there's something special about vma in this
> particular case?
> 
> -Michał
> 
> > 
> > Regards,
> > Bala
> > 
> > >  					 unbind_link) {
> > >  			list_del_init(&vma->unbind_link);
> > >  			if (!vma->destroyed) {
> > > -- 
> > > 2.40.1
> > >