Re: [PATCH rdma-next] RDMA/cxgb4: Remove unnecessary conversion from __be32 to cpu format

[Tom Talpey <tom-CLs1Zie5N5HQT0dZR+AlfA@public.gmane.org>]

On 10/25/2017 7:46 AM, Steve Wise wrote:
>>>> @@ -234,7 +234,7 @@ struct t4_cqe {
>>>>
>>>>   /* used for SQ completion processing */
>>>>   #define CQE_WRID_SQ_IDX(x)	((x)->u.scqe.cidx)
>>>> -#define CQE_WRID_FR_STAG(x)     (be32_to_cpu((x)->u.scqe.stag))
>>>> +#define CQE_WRID_FR_STAG(x)     ((x)->u.scqe.stag)
>>>
>>> This is incorrect.  The stag is filled in by HW which is BE.  The
> declaration of
>>> scqe.stag needs to be __be32.
>>
>> So why do you declare stag as u32?
> 
> I'm saying it is a bug that stag is declared as u32.  t4_cqe.u.scqe.stag should
> be declared as __be32.
> 
> So the fix for the sparse warning should be something like this:
> 
> diff --git a/drivers/infiniband/hw/cxgb4/t4.h b/drivers/infiniband/hw/cxgb4/t4.h
> index e765c00..bcb80ca6 100644
> --- a/drivers/infiniband/hw/cxgb4/t4.h
> +++ b/drivers/infiniband/hw/cxgb4/t4.h
> @@ -171,7 +171,7 @@ struct t4_cqe {
>                          __be32 msn;
>                  } rcqe;
>                  struct {
> -                       u32 stag;
> +                       __be32 stag;

Technically speaking, the stag is opaque to software and should be
declared as a non-integer type, e.g. u8 stag[4]. However, most
code treats it as a native 32-bit type, and performs integer
stores to pass it in the WR. If declaring as __be32 achieves
that without manipulating byte order, well, ok, but it's not
perfectly accurate, type-wise.

Tom.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html