[Intel-xe] [PATCH] drm/xe: remove header variable from parse_g2h_msg

[Matthew Brost]

On Mon, Jul 31, 2023 at 02:08:06PM -0400, Rodrigo Vivi wrote:
> On Fri, Jul 28, 2023 at 09:02:11PM -0700, Matthew Brost wrote:
> > The header variable is unused, remove it.
> > 
> > Suggested-by: Oded Gabbay <ogabbay at kernel.org>
> > Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_guc_ct.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> > index d322eadbe75a..6c757efa88b8 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> > @@ -829,12 +829,11 @@ static int parse_g2h_response(struct xe_guc_ct *ct, u32 *msg, u32 len)
> >  static int parse_g2h_msg(struct xe_guc_ct *ct, u32 *msg, u32 len)
> >  {
> >  	struct xe_device *xe = ct_to_xe(ct);
> > -	u32 header, hxg, origin, type;
> > +	u32 hxg, origin, type;
> >  	int ret;
> >  
> >  	lockdep_assert_held(&ct->lock);
> >  
> > -	header = msg[0];
> 
> Shouldn't we be parsing the header and checking for the len?
> against corrupted messages?
> 

Yep and we do. From g2h_read:

 955         /* Read header */
 956         xe_map_memcpy_from(xe, msg, &g2h->cmds, sizeof(u32) * g2h->info.head,
 957                            sizeof(u32));
 958         len = FIELD_GET(GUC_CTB_MSG_0_NUM_DWORDS, msg[0]) + GUC_CTB_MSG_MIN_LEN;
 959         if (len > avail) {
 960                 drm_err(&xe->drm,
 961                         "G2H channel broken on read, avail=%d, len=%d, reset required\n",
 962                         avail, len);
 963                 g2h->info.broken = true;
 964
 965                 return -EPROTO;
 966         }

Matt

> something like on i915's intel_guc_ct.c:
> 
>   /* message len with header */
>         len = FIELD_GET(GUC_CTB_MSG_0_NUM_DWORDS, header) + GUC_CTB_MSG_MIN_LEN;
>  	if (unlikely(len > (u32)available)) {
>                 CT_ERROR(ct, "Incomplete message %*ph %*ph %*ph\n",
> 	                 4, &header,
>                          4 * (head + available - 1 > size ?
>                               size - head : available - 1), &cmds[head],
> 	                 4 * (head + available - 1 > size ?
>                               available - 1 - size + head : 0), &cmds[0]);
>                 desc->status |= GUC_CTB_STATUS_UNDERFLOW;
> 	        goto corrupted;
>         }
> 
> 
> >  	hxg = msg[1];
> >  
> >  	origin = FIELD_GET(GUC_HXG_MSG_0_ORIGIN, hxg);
> > -- 
> > 2.34.1
> >