RE: Re: Re: Re: Re: [PATCH] RDMA/siw: Fix compiler warnings on 32-bit due to u64/pointer abuse

[Bernard Metzler]

-----"Jason Gunthorpe" <jgg@xxxxxxxx> wrote: -----

>To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx>
>From: "Jason Gunthorpe" <jgg@xxxxxxxx>
>Date: 08/19/2019 06:35PM
>Cc: "Geert Uytterhoeven" <geert@xxxxxxxxxxxxxx>, "Doug Ledford"
><dledford@xxxxxxxxxx>, linux-rdma@xxxxxxxxxxxxxxx,
>linux-kernel@xxxxxxxxxxxxxxx
>Subject: [EXTERNAL] Re: Re: Re: Re: Re: [PATCH] RDMA/siw: Fix
>compiler warnings on 32-bit due to u64/pointer abuse
>
>On Mon, Aug 19, 2019 at 04:29:11PM +0000, Bernard Metzler wrote:
>> 
>> >To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx>
>> >From: "Jason Gunthorpe" <jgg@xxxxxxxx>
>> >Date: 08/19/2019 06:05PM
>> >Cc: "Geert Uytterhoeven" <geert@xxxxxxxxxxxxxx>, "Doug Ledford"
>> ><dledford@xxxxxxxxxx>, linux-rdma@xxxxxxxxxxxxxxx,
>> >linux-kernel@xxxxxxxxxxxxxxx
>> >Subject: [EXTERNAL] Re: Re: Re: Re: [PATCH] RDMA/siw: Fix compiler
>> >warnings on 32-bit due to u64/pointer abuse
>> >
>> >On Mon, Aug 19, 2019 at 03:54:56PM +0000, Bernard Metzler wrote:
>> >
>> >> Absolutely. But these addresses are conveyed through the
>> >> API as unsigned 64 during post_send(), and land in the siw
>> >> send queue as is. During send queue processing, these addresses
>> >> must be interpreted according to its context and transformed
>> >> (casted) back to the callers intention. I frankly do not
>> >> know what we can do differently... The representation of
>> >> all addresses as unsigned 64 is given. Sorry for the confusion.
>> >
>> >send work does not have pointers in it, so I'm confused what this
>is
>> >about. Does siw allow userspace to stick an ordinary pointer for
>the
>> >SG list?
>> 
>> Right a user references a buffer by address and local key it
>> got during reservation of that buffer. The user can provide any
>> VA between start of that buffer and registered length. 
>
>Oh gross, it overloads the IOVA in the WR with a kernel void * ??

Oh no. The user library writes the buffer address into
the 64bit address field of the WR. This is nothing siw
has invented.


>
>> >The code paths here must be totally different, so there should be
>> >different types and functions for each case.
>> 
>> Yes, there is a function to process application memory
>(siw_rx_umem),
>> to process a kernel PBL (siw_rx_pbl), and one to process kernel
>> addresses (siw_rx_kva). Before running that function, the API
>> representation of the current SGE gets translated into target
>> buffer representation.
>
>Why does siw_pbl_get_buffer not return a void *??
>

I think, in fact, it should be dma_addr_t, since this is
what PBL's are described with. Makes sense?


>Still looks like two types have been crammed together.
>
>The kernel can't ever store anything into the user WQE buffer, so I
>would think it would copy the buffer to kernel space, transform it
>properly and then refer to it as a kernel buffer. Kernel sourced
>buffers just skip the transofmration.

This is in fact what happens. siw just does not copy immediately
during post send, but maintains a mmapped shared send queue.
The kernel driver copies the current element from the send queue
and processes it. Only then the current element gets transformed
into the right buffer representation, since it is not being
accessed before.


Thanks
Bernard.
>
>JAson
>
>