16/10/2024 10:18, Junlong Wang:
> Add msg chan enable implementation to support
> send msg to get infos.
Would be interesting to explain which module is receiving the message.
> +union VPORT {
Why uppercase? for differenciate with vport below?
In general we tend to use only lowercase.
> + uint16_t vport;
> + struct {
> + uint16_t vfid:8;
> + uint16_t pfid:3;
> + uint16_t vf_flag:1;
> + uint16_t epid:3;
> + uint16_t direct_flag:1;
> + };
> +};
I am curious about the spinlock function below.
What is it doing exactly?
> +static int32_t zxdh_spinlock_lock(uint32_t virt_lock_id, uint64_t virt_addr,
> + uint64_t label_addr, uint16_t
> primary_id)
> +{
> + uint32_t lock_rd_cnt = 0;
> +
> + do {
> + /* read to lock */
> + uint8_t spl_val = spinklock_read(virt_addr, virt_lock_id);
typo, it should be spinlock_read
> +
> + if (spl_val == 0) {
> + label_write((uint64_t)label_addr, virt_lock_id,
> primary_id);
> + break;
> + }
> + rte_delay_us_block(SPINLOCK_POLLING_SPAN_US);
> + lock_rd_cnt++;
> + } while (lock_rd_cnt < MAX_HARD_SPINLOCK_ASK_TIMES);
> + if (lock_rd_cnt >= MAX_HARD_SPINLOCK_ASK_TIMES)
> + return -1;
> +
> + return 0;
> +}
[...]
> +pthread_spinlock_t chan_lock;
> +static int zxdh_bar_chan_lock(uint8_t src, uint8_t dst, uint16_t src_pcieid,
> uint64_t virt_addr)
> +{
> + int ret = 0;
> + uint8_t src_index = zxdh_bar_msg_src_index_trans(src);
> + uint8_t dst_index = zxdh_bar_msg_dst_index_trans(dst);
> +
> + if (src_index == BAR_MSG_SRC_ERR || dst_index == BAR_MSG_DST_ERR) {
> + PMD_MSG_LOG(ERR, "lock ERR: chan doesn't exist.");
> + return BAR_MSG_ERR_TYPE;
> + }
> + uint16_t idx = lock_type_tbl[src_index][dst_index];
> +
> + if (idx == LOCK_TYPE_SOFT)
> + pthread_spin_lock(&chan_lock);
In general we avoid the pthread.h functions.
Do you know we have rte_spinlock_lock()?
> + else
> + ret = zxdh_bar_hard_lock(src_pcieid, dst, virt_addr);
> +
> + if (ret != 0)
> + PMD_MSG_LOG(ERR, "dev: 0x%x failed to lock.", src_pcieid);
> +
> + return ret;
> +}
[...]
> --- a/drivers/net/zxdh/zxdh_msg.h
> +++ b/drivers/net/zxdh/zxdh_msg.h
> @@ -13,6 +13,19 @@ extern "C" {
>
> #include <ethdev_driver.h>
>
> +#define ZXDH_MSIX_INTR_MSG_VEC_BASE 1
> +
You should continue using the prefix ZXDH_
These definitions below have a high risk of namespace conflict in future.
Using a namespace prefix is a good practice.
> +#define BAR_MSG_POLLING_SPAN 100
> +#define BAR_MSG_POLL_CNT_PER_MS (1 * 1000 / BAR_MSG_POLLING_SPAN)
> +#define BAR_MSG_POLL_CNT_PER_S (1 * 1000 * 1000 / BAR_MSG_POLLING_SPAN)
> +#define BAR_MSG_TIMEOUT_TH (10 * 1000 * 1000 / BAR_MSG_POLLING_SPAN)
> +
> +#define BAR_CHAN_MSG_SYNC 0
> +
> +#define BAR_MSG_ADDR_CHAN_INTERVAL (2 * 1024) /* channel size */
> +#define BAR_MSG_PLAYLOAD_OFFSET (sizeof(struct bar_msg_header))
> +#define BAR_MSG_PAYLOAD_MAX_LEN (BAR_MSG_ADDR_CHAN_INTERVAL -
> sizeof(struct bar_msg_header)) +
>
> enum DRIVER_TYPE {
>
> MSG_CHAN_END_MPF = 0,
> MSG_CHAN_END_PF,
>
> @@ -20,6 +33,13 @@ enum DRIVER_TYPE {
>
> MSG_CHAN_END_RISC,
>
> };
>
> +enum MSG_VEC {
> + MSIX_FROM_PFVF = ZXDH_MSIX_INTR_MSG_VEC_BASE,
> + MSIX_FROM_MPF,
> + MSIX_FROM_RISCV,
> + MSG_VEC_NUM,
> +};
> +
>
> enum BAR_MSG_RTN {
>
> BAR_MSG_OK = 0,
> BAR_MSG_ERR_MSGID,
>
> @@ -54,10 +74,117 @@ enum BAR_MSG_RTN {
>
> BAR_MSG_ERR_SOCKET, /* netlink sockte err */
>
> };
>
> +enum bar_module_id {
> + BAR_MODULE_DBG = 0, /* 0: debug */
> + BAR_MODULE_TBL, /* 1: resource table */
> + BAR_MODULE_MISX, /* 2: config msix */
> + BAR_MODULE_SDA, /* 3: */
> + BAR_MODULE_RDMA, /* 4: */
> + BAR_MODULE_DEMO, /* 5: channel test */
> + BAR_MODULE_SMMU, /* 6: */
> + BAR_MODULE_MAC, /* 7: mac rx/tx stats */
> + BAR_MODULE_VDPA, /* 8: vdpa live migration */
> + BAR_MODULE_VQM, /* 9: vqm live migration */
> + BAR_MODULE_NP, /* 10: vf msg callback np */
> + BAR_MODULE_VPORT, /* 11: get vport */
> + BAR_MODULE_BDF, /* 12: get bdf */
> + BAR_MODULE_RISC_READY, /* 13: */
> + BAR_MODULE_REVERSE, /* 14: byte stream reverse */
> + BAR_MDOULE_NVME, /* 15: */
> + BAR_MDOULE_NPSDK, /* 16: */
> + BAR_MODULE_NP_TODO, /* 17: */
> + MODULE_BAR_MSG_TO_PF, /* 18: */
> + MODULE_BAR_MSG_TO_VF, /* 19: */
> +
> + MODULE_FLASH = 32,
> + BAR_MODULE_OFFSET_GET = 33,
> + BAR_EVENT_OVS_WITH_VCB = 36,
> +
> + BAR_MSG_MODULE_NUM = 100,
> +};
> +
> +struct msix_para {
> + uint16_t pcie_id;
> + uint16_t vector_risc;
> + uint16_t vector_pfvf;
> + uint16_t vector_mpf;
> + uint64_t virt_addr;
> + uint16_t driver_type; /* refer to DRIVER_TYPE */
> +};
> +
> +struct msix_msg {
> + uint16_t pcie_id;
> + uint16_t vector_risc;
> + uint16_t vector_pfvf;
> + uint16_t vector_mpf;
> +};
> +
> +struct zxdh_pci_bar_msg {
> + uint64_t virt_addr; /* bar addr */
> + void *payload_addr;
> + uint16_t payload_len;
> + uint16_t emec;
> + uint16_t src; /* refer to BAR_DRIVER_TYPE */
> + uint16_t dst; /* refer to BAR_DRIVER_TYPE */
> + uint16_t module_id;
> + uint16_t src_pcieid;
> + uint16_t dst_pcieid;
> + uint16_t usr;
> +};
> +
> +struct bar_msix_reps {
> + uint16_t pcie_id;
> + uint16_t check;
> + uint16_t vport;
> + uint16_t rsv;
> +} __rte_packed;
> +
> +struct bar_offset_reps {
> + uint16_t check;
> + uint16_t rsv;
> + uint32_t offset;
> + uint32_t length;
> +} __rte_packed;
> +
> +struct bar_recv_msg {
> + uint8_t reps_ok;
> + uint16_t reps_len;
> + uint8_t rsv;
> + /* */
> + union {
> + struct bar_msix_reps msix_reps;
> + struct bar_offset_reps offset_reps;
> + } __rte_packed;
> +} __rte_packed;
> +
> +struct zxdh_msg_recviver_mem {
> + void *recv_buffer; /* first 4B is head, followed by payload */
> + uint64_t buffer_len;
> +};
> +
> +struct bar_msg_header {
> + uint8_t valid : 1; /* used by __bar_chan_msg_valid_set/get */
> + uint8_t sync : 1;
> + uint8_t emec : 1; /* emergency? */
> + uint8_t ack : 1; /* ack msg? */
> + uint8_t poll : 1;
> + uint8_t usr : 1;
> + uint8_t rsv;
> + uint16_t module_id;
> + uint16_t len;
> + uint16_t msg_id;
> + uint16_t src_pcieid;
> + uint16_t dst_pcieid; /* used in PF-->VF */
