On Thu, Jan 31, 2019 at 02:17:41PM +0000, Bernard Metzler wrote:
> >> +struct siw_cqe {
> >> + __u64 id;
> >> + __u8 flags;
> >> + __u8 opcode;
> >> + __u16 status;
> >> + __u32 bytes;
> >> + __u64 imm_data;
> >> + /* QP number or QP pointer */
> >> + union {
> >> + void *qp;
> >> + __u64 qp_id;
> >
> >void * in this header is almost certainly wrong
>
> Depending on application location (user/kernel),
> we keep the QP identity of that CQE here. User land
> gets a unique QP number in the WC, Kernel user get a
> pointer to the QP.
This seems kind of crazy..
> Would that be OK/better?
>
> struct siw_qp;
>
> struct siw_cqe {
> __aligned_u64 id;
> __u8 flags;
> __u8 opcode;
> __u16 status;
> __u32 bytes;
> __aligned_u64 imm_data;
> /* QP number or QP pointer */
> union {
> struct siw_qp *qp;
Maybe put this around a #ifdef KERNEL?
> >> +/*
> >> + * Shared structure between user and kernel
> >> + * to control CQ arming.
> >> + */
> >> +struct siw_cq_ctrl {
> >> + enum siw_notify_flags notify;
> >
> >Do not use 'enum' in any of these structs. Only fixed size memory.
> >
> Right. Will fix that.
>
> >Is this shared memory?
> >
> Yes, it is shared between provider and user land library
> in case of user land app. This avoids trapping into the kernel
> for CQ re-arming.
Any access to shared memory must be done via READ_ONCE/WRITE_ONCE in
the kernel and stdatomic in userspace. See the still unfinished rvt
patches that are supposed to be fixing their driver..
Jason
