On 1/7/19 10:59 AM, Jason Gunthorpe wrote:
> On Mon, Jan 07, 2019 at 06:53:24PM +0000, Adit Ranadive wrote:
>> From: Adit Ranadive <aditr@xxxxxxxxxx>
>>
>> Since the IB_WR_REG_MR opcode value changed, set some of the PVRDMA device
>> opcodes explicitly.
>>
>> Reported-by: Ruishuang Wang <ruishuangw@xxxxxxxxxx>
>> Fixes: 9a59739bd01f ("IB/rxe: Revise the ib_wr_opcode enum")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Reviewed-by: Bryan Tan <bryantan@xxxxxxxxxx>
>> Reviewed-by: Ruishuang Wang <ruishuangw@xxxxxxxxxx>
>> Reviewed-by: Vishnu Dasa <vdasa@xxxxxxxxxx>
>> Signed-off-by: Adit Ranadive <aditr@xxxxxxxxxx>
>> ---
>> drivers/infiniband/hw/vmw_pvrdma/pvrdma.h | 21 ++++++++++++++++++++-
>> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> So naughty!! How many other places in this driver are assuming the
> intenal IB constants are stable?
>
> Did you audit for other cases?
Lots of places. Though its the values shared with userspace, like QP types,
IB MTUs, etc. Unless you foresee changes to those ..
>
>> diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
>> index 42b8685c997e..c2ed09e66d2b 100644
>> --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
>> +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h
>> @@ -427,7 +427,26 @@ static inline enum ib_qp_state pvrdma_qp_state_to_ib(enum pvrdma_qp_state state)
>>
>> static inline enum pvrdma_wr_opcode ib_wr_opcode_to_pvrdma(enum ib_wr_opcode op)
>> {
>> - return (enum pvrdma_wr_opcode)op;
>> + switch (op) {
>> + case IB_WR_LSO:
>> + return PVRDMA_WR_LSO;
>> + case IB_WR_SEND_WITH_INV:
>> + return PVRDMA_WR_SEND_WITH_INV;
>> + case IB_WR_RDMA_READ_WITH_INV:
>> + return PVRDMA_WR_RDMA_READ_WITH_INV;
>> + case IB_WR_LOCAL_INV:
>> + return PVRDMA_WR_LOCAL_INV;
>> + case IB_WR_REG_MR:
>> + return PVRDMA_WR_FAST_REG_MR;
>> + case IB_WR_MASKED_ATOMIC_CMP_AND_SWP:
>> + return PVRDMA_WR_MASKED_ATOMIC_CMP_AND_SWP;
>> + case IB_WR_MASKED_ATOMIC_FETCH_AND_ADD:
>> + return PVRDMA_WR_MASKED_ATOMIC_FETCH_AND_ADD;
>> + case IB_WR_REG_SIG_MR:
>> + return PVRDMA_WR_REG_SIG_MR;
>> + default:
>> + return (enum pvrdma_wr_opcode)op;
>
> No default, lets write them all out please.
Fair enough.
>
> Jason
>
