From mboxrd@z Thu Jan  1 00:00:00 1970
Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au
Authentication-Results: passt.top;
	dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202408 header.b=esxmGsMB;
	dkim-atps=neutral
Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76])
	by passt.top (Postfix) with ESMTPS id 21B705A004E
	for <passt-dev@passt.top>; Tue, 27 Aug 2024 06:43:08 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
	d=gibson.dropbear.id.au; s=202408; t=1724733783;
	bh=pIho8i5j1tx090Fbji9wrn/Rt1B2errg/Xo7JJQXJEU=;
	h=Date:From:To:Cc:Subject:References:In-Reply-To:From;
	b=esxmGsMBQIXxCn2svHOvKpqoTssFpnbVAVxal7e6hyPU6UuKEXAFF6kFxVL+phXlJ
	 ruclN4+VDrQq7AD69w25I0Bin6LQSOXntNZd0VN64U2mzpU0J1mS6edaj2nqou/6ze
	 Db+GbjjwJq9KLkqW3oPkRXfpzc4mq3m8mhyuATWlKHIvcBN5AAYHvENsGT1HPskVLA
	 +JqX/mPxWhKi2HX2w6pRCp2sOd8AlualDeY0XtnfCk9c32WHqb+f+XRxPHPSxFqdHv
	 UctmrmhAeFEU9lD0g7iIO9f+7O1KTAYGLNs72LmLtzdeaNfqBkqeN8h3GczUd47bgk
	 ZW1Ygl0wm5azA==
Received: by gandalf.ozlabs.org (Postfix, from userid 1007)
	id 4WtFKM6SXmz4wcF; Tue, 27 Aug 2024 14:43:03 +1000 (AEST)
Date: Tue, 27 Aug 2024 14:42:47 +1000
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>
Subject: Re: [PATCH v3 3/4] vhost-user: introduce vhost-user API
Message-ID: <Zs1ZR8-NktdTdfbr@zatzit.fritz.box>
References: <20240815155024.827956-1-lvivier@redhat.com>
 <20240815155024.827956-4-lvivier@redhat.com>
 <ZswSFOJsSsRmXABP@zatzit.fritz.box>
 <20240827001420.1f895c7d@elisabeth>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="MCXjgfeeTGSfrzW1"
Content-Disposition: inline
In-Reply-To: <20240827001420.1f895c7d@elisabeth>
Message-ID-Hash: ZKHOH37NVNU5NW47IKLVR6DVXMXBOG72
X-Message-ID-Hash: ZKHOH37NVNU5NW47IKLVR6DVXMXBOG72
X-MailFrom: dgibson@gandalf.ozlabs.org
X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header
CC: Laurent Vivier <lvivier@redhat.com>, passt-dev@passt.top
X-Mailman-Version: 3.3.8
Precedence: list
List-Id: Development discussion and patches for passt <passt-dev.passt.top>
Archived-At: <https://archives.passt.top/passt-dev/Zs1ZR8-NktdTdfbr@zatzit.fritz.box/>
Archived-At: <https://passt.top/hyperkitty/list/passt-dev@passt.top/message/ZKHOH37NVNU5NW47IKLVR6DVXMXBOG72/>
List-Archive: <https://archives.passt.top/passt-dev/>
List-Archive: <https://passt.top/hyperkitty/list/passt-dev@passt.top/>
List-Help: <mailto:passt-dev-request@passt.top?subject=help>
List-Owner: <mailto:passt-dev-owner@passt.top>
List-Post: <mailto:passt-dev@passt.top>
List-Subscribe: <mailto:passt-dev-join@passt.top>
List-Unsubscribe: <mailto:passt-dev-leave@passt.top>


--MCXjgfeeTGSfrzW1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Aug 27, 2024 at 12:14:20AM +0200, Stefano Brivio wrote:
> On Mon, 26 Aug 2024 15:26:44 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
>=20
> > On Thu, Aug 15, 2024 at 05:50:22PM +0200, Laurent Vivier wrote:
> > > Add vhost_user.c and vhost_user.h that define the functions needed
> > > to implement vhost-user backend.
> > >
> > > [...]
> > >=20
> > > +static int vu_message_read_default(int conn_fd, struct vhost_user_ms=
g *vmsg)
> > > +{
> > > +	char control[CMSG_SPACE(VHOST_MEMORY_BASELINE_NREGIONS *
> > > +		     sizeof(int))] =3D { 0 };
> > > +	struct iovec iov =3D {
> > > +		.iov_base =3D (char *)vmsg,
> > > +		.iov_len =3D VHOST_USER_HDR_SIZE,
> > > +	};
> > > +	struct msghdr msg =3D {
> > > +		.msg_iov =3D &iov,
> > > +		.msg_iovlen =3D 1,
> > > +		.msg_control =3D control,
> > > +		.msg_controllen =3D sizeof(control),
> > > +	};
> > > +	ssize_t ret, sz_payload;
> > > +	struct cmsghdr *cmsg;
> > > +	size_t fd_size;
> > > +
> > > +	ret =3D recvmsg(conn_fd, &msg, MSG_DONTWAIT);
> > > +	if (ret < 0) {
> > > +		if (errno =3D=3D EINTR || errno =3D=3D EAGAIN || errno =3D=3D EWOU=
LDBLOCK)
> > > +			return 0;
> > > +		return -1;
> > > +	}
> > > +
> > > +	vmsg->fd_num =3D 0;
> > > +	for (cmsg =3D CMSG_FIRSTHDR(&msg); cmsg !=3D NULL;
> > > +	     cmsg =3D CMSG_NXTHDR(&msg, cmsg)) {
> > > +		if (cmsg->cmsg_level =3D=3D SOL_SOCKET &&
> > > +		    cmsg->cmsg_type =3D=3D SCM_RIGHTS) {
> > > +			fd_size =3D cmsg->cmsg_len - CMSG_LEN(0);
> > > +			ASSERT(fd_size / sizeof(int) <=3D
> > > +			       VHOST_MEMORY_BASELINE_NREGIONS); =20
> >=20
> > IIUC, this could be tripped by a bug in the peer (qemu?) rather than
> > in our own code.  In which case I think a die() would be more
> > appropriate than an ASSERT().
>=20
> Ah, right, it wouldn't be our issue... what about neither, so that we
> don't crash if QEMU has an issue we could easily recover from?

It wasn't immediately obvious to me if we could reasily recover from
that or not.

[snip]
> > > +	vdev->nregions =3D memory->nregions;
> > > +
> > > +	debug("Nregions: %u", memory->nregions);
> > > +	for (i =3D 0; i < vdev->nregions; i++) {
> > > +		struct vhost_user_memory_region *msg_region =3D &memory->regions[i=
];
> > > +		struct vu_dev_region *dev_region =3D &vdev->regions[i];
> > > +		void *mmap_addr;
> > > +
> > > +		debug("Region %d", i);
> > > +		debug("    guest_phys_addr: 0x%016"PRIx64,
> > > +		      msg_region->guest_phys_addr);
> > > +		debug("    memory_size:     0x%016"PRIx64,
> > > +		      msg_region->memory_size);
> > > +		debug("    userspace_addr   0x%016"PRIx64,
> > > +		      msg_region->userspace_addr);
> > > +		debug("    mmap_offset      0x%016"PRIx64,
> > > +		      msg_region->mmap_offset);
> > > +
> > > +		dev_region->gpa =3D msg_region->guest_phys_addr;
> > > +		dev_region->size =3D msg_region->memory_size;
> > > +		dev_region->qva =3D msg_region->userspace_addr;
> > > +		dev_region->mmap_offset =3D msg_region->mmap_offset;
> > > +
> > > +		/* We don't use offset argument of mmap() since the
> > > +		 * mapped address has to be page aligned, and we use huge
> > > +		 * pages. =20
> >=20
> > We do what now?
>=20
> We do madvise(pkt_buf, TAP_BUF_BYTES, MADV_HUGEPAGE) in main(), but
> we're not using pkt_buf in this case, so I guess it's not relevant. I'm
> not sure if _passt_ calling madvise(..., MADV_HUGEPAGE) on the memory
> regions we get would have any effect, by the way.

Huh, I'd forgotten about that.  AIUI qemu allocates the memory and we
map it into passt, so I don't think our madvise() would have any
effect here.

--=20
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

--MCXjgfeeTGSfrzW1
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmbNWUYACgkQzQJF27ox
2Gdnsw//cPE6DSbvmaSxCRr3llaMCn5T372vRhMTQeST6cHgzifBE7yhuh9lwxoI
1o+WnZ+RgfVpdhRADzv1QsHZPC0rWzBCnJ2Pr/ZQIE8fjHZmoSudbudotOx2EyRK
0vNADCfVdhV6ph69SZqJo0n1DjAq9C4SBhUYB6vBpatWAXTfOIi7YDvQB2jxJll9
1D2E9tS0TPXHLuzci3u7kzAnvhTia+kuLWlg27ZBcf4Tu+kpbeNz2byOGgbMEtSW
77uElNAHrRDoUZrSGPwSB3EqV29d+JKynaJl6kPOk1+S9WPjqatjTPmsqejBKBMR
UuG04RAv+tBAcnGUkA6ncRvGog1CtoQ0a2fsczjWQ/H14wluKkdXaL5s+rnKtudJ
ReuvrVdD5WlVq6TNxnGrXLaIR1sxisTUInl9VrfSgHbfg2JXWAloU/zxzKd2Eg5Y
aSXZLO++n7yI4THDmj+AI6gNntPCmWHdPP8sGR9+0xLzJclp6SqkxV/MHnsSX9kr
Fe6Ovm1oBzIPl1GuFgUd1TXYgtJdHiwn6MpLkQvggp7y2+BIAveERZCNDQZil5iu
6gURHaDp6N8aGdgHnI8Xgtpz3joM9xPv1uqLD1i4Q4mXveRATW9eAV1dvYzAHPjV
CZ/vRt+jcP1DQ70DqYPyWm3ffQeRJKwmcJgptIhFxojLS/Evwv4=
=OdwB
-----END PGP SIGNATURE-----

--MCXjgfeeTGSfrzW1--