RE: [PATCH 4/6] tools: hv: Add vmbus_bufring

Long Li
2024-03-13T19:12:25Z

> +
> +#define rte_compiler_barrier() ({ asm volatile ("" : : : "memory"); })
> +
> +#define rte_smp_rwmb() ({ asm volatile ("" : : :
> "memory"); })
> +
> +#define VMBUS_RQST_ERROR 0xFFFFFFFFFFFFFFFF
> +#define ALIGN(val, align) ((typeof(val))((val) & (~((typeof(val))((align) -
> 1)))))
> +
> +void *vmbus_uio_map(int *fd, int size)
> +{
> + void *map;
> +
> + map = mmap(NULL, 2 * size, PROT_READ | PROT_WRITE, MAP_SHARED,
> *fd, 0);
> + if (map == MAP_FAILED)
> + return NULL;
> +
> + return map;
> +}
> +
> +/* Increase bufring index by inc with wraparound */ static inline
> +uint32_t vmbus_br_idxinc(uint32_t idx, uint32_t inc, uint32_t sz) {
> + idx += inc;
> + if (idx >= sz)
> + idx -= sz;
> +
> + return idx;
> +}
> +
> +void vmbus_br_setup(struct vmbus_br *br, void *buf, unsigned int blen)
> +{
> + br->vbr = buf;
> + br->windex = br->vbr->windex;
> + br->dsize = blen - sizeof(struct vmbus_bufring); }
> +
> +static inline __always_inline void
> +rte_smp_mb(void)
> +{
> + asm volatile("lock addl $0, -128(%%rsp); " ::: "memory"); }
> +
> +static inline int
> +rte_atomic32_cmpset(volatile uint32_t *dst, uint32_t exp, uint32_t src)
> +{
> + uint8_t res;
> +
> + asm volatile("lock ; "
> + "cmpxchgl %[src], %[dst];"
> + "sete %[res];"
> + : [res] "=a" (res), /* output */
> + [dst] "=m" (*dst)
> + : [src] "r" (src), /* input */
> + "a" (exp),
> + "m" (*dst)
> + : "memory"); /* no-clobber list */
> + return res;
> +}

Those helper functions (rte_*) are difficult to read for someone without DPDK background. Can you add comments to those helper functions? Maybe just copy the comments over from DPDK.

> +
> +static inline uint32_t
> +vmbus_txbr_copyto(const struct vmbus_br *tbr, uint32_t windex,
> + const void *src0, uint32_t cplen)
> +{
> + uint8_t *br_data = tbr->vbr->data;
> + uint32_t br_dsize = tbr->dsize;
> + const uint8_t *src = src0;
> +
> + /* XXX use double mapping like Linux kernel? */
> + if (cplen > br_dsize - windex) {
> + uint32_t fraglen = br_dsize - windex;
> +
> + /* Wrap-around detected */
> + memcpy(br_data + windex, src, fraglen);
> + memcpy(br_data, src + fraglen, cplen - fraglen);
> + } else {
> + memcpy(br_data + windex, src, cplen);
> + }
> +
> + return vmbus_br_idxinc(windex, cplen, br_dsize); }
> +
> +/*
> + * Write scattered channel packet to TX bufring.
> + *
> + * The offset of this channel packet is written as a 64bits value
> + * immediately after this channel packet.
> + *
> + * The write goes through three stages:
> + * 1. Reserve space in ring buffer for the new data.
> + * Writer atomically moves priv_write_index.
> + * 2. Copy the new data into the ring.
> + * 3. Update the tail of the ring (visible to host) that indicates
> + * next read location. Writer updates write_index
> + */
> +static int
> +vmbus_txbr_write(struct vmbus_br *tbr, const struct iovec iov[], int iovlen,
> + bool *need_sig)

Do you need "need_sig"? It's probably not for non-network/storage devices.