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

Tom Talpey
2017-10-26T05:00:49Z

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.