Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Wed, 16 Feb 2022 11:19:33 +0200
From:   Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
To:     Jani Nikula <jani.nikula@linux.intel.com>
Cc:     Jiapeng Chong <jiapeng.chong@linux.alibaba.com>,
        tvrtko.ursulin@linux.intel.com, airlied@linux.ie,
        intel-gfx@lists.freedesktop.org,
        Abaci Robot <abaci@linux.alibaba.com>,
        linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        christian.koenig@amd.com, linaro-mm-sig@lists.linaro.org,
        rodrigo.vivi@intel.com, sumit.semwal@linaro.org,
        linux-media@vger.kernel.org
Subject: Re: [PATCH] drm/i915/gt: fix unsigned integer to signed assignment
Message-ID: <YgzBpX2SKCh4cb19@intel.com>
References: <20220216083849.91239-1-jiapeng.chong@linux.alibaba.com>
 <87h78z5gmp.fsf@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <87h78z5gmp.fsf@intel.com>
Precedence: bulk

On Wed, Feb 16, 2022 at 11:02:06AM +0200, Jani Nikula wrote:
> On Wed, 16 Feb 2022, Jiapeng Chong <jiapeng.chong@linux.alibaba.com> wrote:
> > Eliminate the follow smatch warning:
> >
> > drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:4640
> > guc_create_virtual() warn: assigning (-2) to unsigned variable
> > 've->base.instance'.
> >
> > drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:4641
> > guc_create_virtual() warn: assigning (-2) to unsigned variable
> > 've->base.uabi_instance'.
> >
> > Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> > Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> 
> The report seems to be valid, but I don't think this is the fix.
> 
> Where do we even check for invalid instance/uabi_instance in code?

The whole thing seems rather poorly documented as there's a matching
uabi struct with __u16's and the negative values are defined right
there in the uapi header as well.

> 
> BR,
> Jani.
> 
> 
> > ---
> >  drivers/gpu/drm/i915/gt/intel_engine_types.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > index 36365bdbe1ee..dc7cc06c68e7 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> > @@ -328,10 +328,10 @@ struct intel_engine_cs {
> >  	intel_engine_mask_t logical_mask;
> >  
> >  	u8 class;
> > -	u8 instance;
> > +	s8 instance;
> >  
> >  	u16 uabi_class;
> > -	u16 uabi_instance;
> > +	s16 uabi_instance;
> >  
> >  	u32 uabi_capabilities;
> >  	u32 context_size;
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrj?l?
Intel