On 26-Feb-19 19:56, Steve Wise wrote:
>> +/* EFA admin queue opcodes */
>> +enum efa_admin_aq_opcode {
>> + /* starting opcode of efa admin commands */
>> + EFA_ADMIN_START_CMD_RANGE = 1,
>> + /* Create QP */
>> + EFA_ADMIN_CREATE_QP = EFA_ADMIN_START_CMD_RANGE,
>> + /* Modify QP */
>> + EFA_ADMIN_MODIFY_QP = 2,
>> + /* Query QP */
>> + EFA_ADMIN_QUERY_QP = 3,
>> + /* Destroy QP */
>> + EFA_ADMIN_DESTROY_QP = 4,
>> + /* Create Address Handle */
>> + EFA_ADMIN_CREATE_AH = 5,
>> + /* Destroy Address Handle */
>> + EFA_ADMIN_DESTROY_AH = 6,
>> + /* Register Memory Region */
>> + EFA_ADMIN_REG_MR = 7,
>> + /* Deregister Memory Region */
>> + EFA_ADMIN_DEREG_MR = 8,
>> + /* Create Completion Q */
>> + EFA_ADMIN_CREATE_CQ = 9,
>> + /* Destroy Completion Q */
>
>
> Are the above comments really useful?
Will remove.
>
>
>> + EFA_ADMIN_DESTROY_CQ = 10,
>> + EFA_ADMIN_GET_FEATURE = 11,
>> + EFA_ADMIN_SET_FEATURE = 12,
>> + EFA_ADMIN_GET_STATS = 13,
>> + EFA_ADMIN_ALLOC_PD = 14,
>> + EFA_ADMIN_DEALLOC_PD = 15,
>> + EFA_ADMIN_ALLOC_UAR = 16,
>> + EFA_ADMIN_DEALLOC_UAR = 17,
>> + EFA_ADMIN_MAX_OPCODE = 17,
>> +};
>> +
>> +/* QP state */
>> +enum efa_admin_qp_state {
>> + /* Reset queue */
>> + EFA_ADMIN_QP_STATE_RESET = 0,
>> + /* Init queue */
>> + EFA_ADMIN_QP_STATE_INIT = 1,
>> + /* Ready to receive */
>> + EFA_ADMIN_QP_STATE_RTR = 2,
>> + /* Ready to send */
>> + EFA_ADMIN_QP_STATE_RTS = 3,
>> + /* Send queue drain */
>> + EFA_ADMIN_QP_STATE_SQD = 4,
>> + /* Send queue error */
>> + EFA_ADMIN_QP_STATE_SQE = 5,
>> + /* Queue in error state */
>> + EFA_ADMIN_QP_STATE_ERR = 6,
>> +};
>
>
> Again, not sure these comments help.
Will remove.
>
>
>> +
>> +/* Create QP command. */
>> +struct efa_admin_create_qp_cmd {
>> + /* Common Admin Queue descriptor */
>> + struct efa_admin_aq_common_desc aq_common_desc;
>> +
>> + /* Protection Domain associated with this QP */
>> + u16 pd;
>> +
>> + /* QP type */
>> + u8 qp_type;
>> +
>> + /*
>> + * 0 : sq_virt - If set, SQ ring base address is
>> + * virtual (IOVA returned by MR registration)
>> + * 1 : rq_virt - If set, RQ ring base address is
>> + * virtual (IOVA returned by MR registration)
>> + * 7:2 : reserved - MBZ
>> + */
>> + u8 flags;
>
>
> Should you have an enum for these bits?
You found them at the end :).
>
>
>> +/*
>> + * Registration of MemoryRegion, required for QP working with Virtual
>> + * Addresses. In standard verbs semantics, region length is limited to 2GB
>> + * space, but EFA offers larger MR support for large memory space, to ease
>> + * on users working with very large datasets (i.e. full GPU memory mapping).
>> + */
>> +struct efa_admin_reg_mr_cmd {
>> + /* Common Admin Queue descriptor */
>> + struct efa_admin_aq_common_desc aq_common_desc;
>> +
>> + /* Protection Domain */
>> + u16 pd;
>> +
>> + /* MBZ */
>> + u16 reserved16_w1;
>> +
>> + /* Physical Buffer List, each element is page-aligned. */
>> + union {
>> + /*
>> + * Inline array of guest-physical page addresses of user
>> + * memory pages (optimization for short region
>> + * registrations)
>> + */
>> + u64 inline_pbl_array[4];
>> +
>> + /* points to PBL (direct or indirect, chained if needed) */
>> + struct efa_admin_ctrl_buff_info pbl;
>> + } pbl;
>> +
>> + /* Memory region length, in bytes. */
>> + u64 mr_length;
>> +
>> + /*
>> + * flags and page size
>> + * 4:0 : phys_page_size_shift - page size is (1 <<
>> + * phys_page_size_shift). Page size is used for
>> + * building the Virtual to Physical address mapping
>> + * 6:5 : reserved - MBZ
>> + * 7 : mem_addr_phy_mode_en - Enable bit for physical
>> + * memory registration (no translation), can be used
>> + * only by privileged clients. If set, PBL must
>> + * contain a single entry.
>> + */
>> + u8 flags;
>> +
>> + /*
>> + * permissions
>> + * 0 : local_write_enable - Write permissions: value
>> + * of 1 needed for RQ buffers and for RDMA write
>> + * 7:1 : reserved1 - remote access flags, etc
>> + */
>> + u8 permissions;
>> +
>
>
> enums for flags and permissions bits? If you decide enums are useful,
> do them for all the bits in this file...
Same.
>
>
>> + u16 reserved16_w5;
>> +
>> + /* number of pages in PBL (redundant, could be calculated) */
>> + u32 page_num;
>> +
>> + /*
>> + * IO Virtual Address associated with this MR. If
>> + * mem_addr_phy_mode_en is set, contains the physical address of
>> + * the region.
>> + */
>> + u64 iova;
>> +};
>> +
>> +
>> +/* get_set_feature_common_desc */
>> +#define EFA_ADMIN_GET_SET_FEATURE_COMMON_DESC_SELECT_MASK GENMASK(1, 0)
>> +
>
>
> Oh I guess these are the bit defines for those fields. :)
Indeed.
>
>
>> +
>> +/* mmio_reg_read register */
>> +#define EFA_REGS_MMIO_REG_READ_REQ_ID_MASK 0xffff
>> +#define EFA_REGS_MMIO_REG_READ_REG_OFF_SHIFT 16
>> +#define EFA_REGS_MMIO_REG_READ_REG_OFF_MASK 0xffff0000
>> +
>> +#endif /*_EFA_REGS_H_ */
>
>
> Reviewed-by: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
Thanks Steve!
