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

[Michał Winiarski]

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?

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
> >