Received-SPF: pass (google.com: domain of linux-kernel+bounces-102407-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Long Li <longli@microsoft.com>
To: Saurabh Sengar <ssengar@linux.microsoft.com>, KY Srinivasan
	<kys@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>,
	"wei.liu@kernel.org" <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC: Saurabh Singh Sengar <ssengar@microsoft.com>
Subject: RE: [PATCH 4/6] tools: hv: Add vmbus_bufring
Thread-Topic: [PATCH 4/6] tools: hv: Add vmbus_bufring
Thread-Index: AQHaYcvMTJRAmQtKYEuIgdJTlJTrs7E2LsEQ
Date: Wed, 13 Mar 2024 19:12:23 +0000
Message-ID:
 <SJ1PR21MB34578EF2FD0F5BA29D0CB95ECE2A2@SJ1PR21MB3457.namprd21.prod.outlook.com>
References: <1708193020-14740-1-git-send-email-ssengar@linux.microsoft.com>
 <1708193020-14740-5-git-send-email-ssengar@linux.microsoft.com>
In-Reply-To: <1708193020-14740-5-git-send-email-ssengar@linux.microsoft.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?zRTdhH9lrGb49sZQOiiC7rPQ3vEjRhN8f21d5tjBYC5rMTnR/tFxvi+mPsZf?=
 =?us-ascii?Q?xfZ8e0gCg4bFqOFnW+0XOiAyMtyjpNl+QeASP7lzdonTyfx+3jo1z/G7F9Xc?=
 =?us-ascii?Q?Phjcn6boxf6kcTr+NiwZ60B1f+9T9FFrng241MldGM7z780znKe/z5iHyfvR?=
 =?us-ascii?Q?bx5Klu3mrNUQAyXbKcaa6v4sHpf7xBzzliTEUaELWlBLKPOzTFQr3C9oybeN?=
 =?us-ascii?Q?vjsYCN3vgeWGkQDJuc4dcrdU57VEgpJtSQB9E1XU3ySLuo5z/w01O6T7ePCj?=
 =?us-ascii?Q?iBUjAv5IsXUng6e41M1b90KrQUusYAEX69n4gU8tIuMTTV6zUafQXcf+ElSO?=
 =?us-ascii?Q?hTq1esYd7SzgtYaflY9GN2vPbjc4WpZWhb3r90czpEI4vw/lAQ4+w47/Wq5C?=
 =?us-ascii?Q?O+sJgfOQfVxUc/tOpF5wi0eE6dRnldewr05lV42PkdDZ0c1xKqQo1RumQfvq?=
 =?us-ascii?Q?jvM9UAd7J0jFj65RHo6p1aFzHj3PuUJG4MqJtroBaw127Xmv6lusZKxoQ2W2?=
 =?us-ascii?Q?fKYY2nQeim+DadNdibgnGTBC44R9Z0VBFbi/Hj+0epitLq817vOGTgXbPcpX?=
 =?us-ascii?Q?cWNDQF4LP1dSd4IH/MM2AK5i7bd/4FioxkafDytfCZjrQ7sVon6kk7ecoI6s?=
 =?us-ascii?Q?uA2Mv8PshmUyoKVuaKz3g/SFoXq+Rafqya+LS8hxc1yOJw36WKmgUoy/a4I9?=
 =?us-ascii?Q?WGx8Cu8IIQHL5iroM3JoyQ4shqZJ5k2AYgu/rZqRTgBSuGf+AKc+iSchgkwL?=
 =?us-ascii?Q?nn4QNCWdMvL0GV7AbkQhiECUb0kPEhc4RmpV1YJX8p72DxMhviaPEmqtM6gz?=
 =?us-ascii?Q?rGtxMTcZ3Uuxtx6bb/So/t+IN+NKTpfRcI4JfNwiAm32WraaUC8diCQ3Tf2o?=
 =?us-ascii?Q?LG/xne3xlyduwXmfnl9JJ1OWt+VyxS6tYmEdap2f8kWANiHR0kFBpIrY9OKW?=
 =?us-ascii?Q?awHsMdPPsSHB86ALYoe1wT0ZEL5er+l3W1ioej88Seu0OHiUtgimo46o3PRP?=
 =?us-ascii?Q?J4KGlEnNTfaIL8ZKVba1akRA5zTrEUQTEBbdap4CXmJiE4WG5jMZSZArQiRm?=
 =?us-ascii?Q?bKfJ8CS/4+iCEjmk1+IpHWZdX5/tYHWP5glGsFRFYeQR6sM90ng8zMwoC5bj?=
 =?us-ascii?Q?75WpIOlxIFc1swqma2S7p2rEI5wvVeLW6PikUHCCXBaEPAhc1/CRVrtQW6Yy?=
 =?us-ascii?Q?kPWg1OawocJ2OJA+gSgS/AnHlWDobRV2mr4hZRMbS0f5dMMOi8faT5itDVa3?=
 =?us-ascii?Q?yl8D0NX4+q9EI3zh3W+YSTM2iapYUHqUF0HMl9ahpgPtwFxJhRZLpKhbi9A3?=
 =?us-ascii?Q?FahEQkuv+nF6VOiwDkmarYSrqWDU+CWRsIJ/xybquAfHQKQnN0qttRPhZL+6?=
 =?us-ascii?Q?l8yhlU/UyAAXbISGOwNtJhz1tR7D61+NMoBgEtOcEzz/AHQBP0XtzD5tPifs?=
 =?us-ascii?Q?zEo9/5oxVKhACWFgZi6HPnsNs5378SFy1iPRKACoDQYI31hyR47yF9LWVMF5?=
 =?us-ascii?Q?wdch6bpwxv4UICoZVcFbM69VTjzy4UAKKK59ZvnJK5Y3GyD8fC0EOGXkyGfp?=
 =?us-ascii?Q?kgwBtnwXaDM0+VYRtZukGT1EqA0rctg4jclzdeqL?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: microsoft.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SJ1PR21MB3457.namprd21.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 58a43b57-07a2-4af5-ee8a-08dc4391837e
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Mar 2024 19:12:23.4603
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ngfyXcrZ/Bf6CL/m3eoVBeWCJ81jhWh3kNJ9Y/zQZCYEvjEDO+r49uqYemuMQdyHPIEALZ1sjIFpsvU051byBA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR21MB3026

> +
> +#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 =3D mmap(NULL, 2 * size, PROT_READ | PROT_WRITE, MAP_SHARED,
> *fd, 0);
> +	if (map =3D=3D 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 +=3D inc;
> +	if (idx >=3D sz)
> +		idx -=3D sz;
> +
> +	return idx;
> +}
> +
> +void vmbus_br_setup(struct vmbus_br *br, void *buf, unsigned int blen)
> +{
> +	br->vbr =3D buf;
> +	br->windex =3D br->vbr->windex;
> +	br->dsize =3D 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] "=3Da" (res),     /* output */
> +		     [dst] "=3Dm" (*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 DP=
DK background. Can you add comments to those helper functions? Maybe just c=
opy 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 =3D tbr->vbr->data;
> +	uint32_t br_dsize =3D tbr->dsize;
> +	const uint8_t *src =3D src0;
> +
> +	/* XXX use double mapping like Linux kernel? */
> +	if (cplen > br_dsize - windex) {
> +		uint32_t fraglen =3D 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 iov=
len,
> +		 bool *need_sig)

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