From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-011.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-011.esa.us-west-2.outbound.mail-perimeter.amazon.com [52.35.192.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6A7F837475C for ; Sun, 9 Aug 2026 18:20:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=52.35.192.45 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299622; cv=none; b=ANR3TaklDzsZ/bpIuMOlhyizEYbo9F1ggl0pLTG44KgEQjSKyMD6YYD8h3zM1D/cemcLwtcf16PRqabCT+ghLONNAHIFUPznXcgjX4VzXp8Y1oz+GYA5OtdE+fDlTdE9fBeNeOXZFvepLChaaH/IVxboN95YbspUgiy9/9K6Yiw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299622; c=relaxed/simple; bh=zdpFl4MtmzbUgJL7zsohL1qYs3UZChN8OMZanTHyZXo=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=S7ro/h39c89NqgGoUedSjEbC3aGVAb53fA4piEDVXLU+o+HYFawpCUvsfpGEcYTH+6zxA2jKh67H8yxNiACStMrtjPU5ijfvDJ8rXRyMWr7ticHrBARmfLl1GoYv2ZIHBdCcLbKEkMXTBnADp/jwbuTUO1XjiIuQUSPZvFg5/Js= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=e6AaKOmK; arc=none smtp.client-ip=52.35.192.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="e6AaKOmK" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299621; x=1817835621; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=KDl7UEafOpm77w6dRDFK3GgIysZX3WlQtTfDhkiiqiQ=; b=e6AaKOmKZNPFBlnI1yaXRei5S8hnVrN40Q+Nkgy5SQ27VdE4kbyaL85/ DON2EBa7mjLkQ8BVAySaf8lKhF7Bx1mBGJ6dU76zFBfDgFnDTyfo0seSE XWPV2dgyCRY0kmU5pC1ZcT7ieVQAokxUbinO/P0Mg/P6Imm2hm/E/k3gW DIztQ7mwSsjCz7CPgQygIhEWR7gGc4CcE3Ayo6G4xs3L7DuBt9M02iGfE HzJ48yX1UdfFhd3zqFNNnjhtl48FQnldhK8vP/A0qQYLDZKaJhcIQNsDH I5JroD7I4XVok0UGqEN4vlSymNdt1hgmTOvlR+m6YPQVA0mpgfdAqZnRf A==; X-CSE-ConnectionGUID: +AWlV5KeSQeAZ9+skUf8pg== X-CSE-MsgGUID: zZ5u1sG0ToqbMtHSGfqZRA== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25283671" Received: from ip-10-5-6-203.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.6.203]) by internal-pdx-out-011.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:20:18 +0000 Received: from EX19MTAUWA002.ant.amazon.com [205.251.233.234:7862] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.58.172:2525] with esmtp (Farcaster) id 0b959277-6977-4a99-b734-84217a11f39c; Sun, 9 Aug 2026 18:20:18 +0000 (UTC) X-Farcaster-Flow-ID: 0b959277-6977-4a99-b734-84217a11f39c Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWA002.ant.amazon.com (10.250.64.202) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:20:17 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:20:15 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , Yishai Hadas , Alex Williamson , Feng Liu , Jiri Pirko , , , Parav Pandit , , Stefan Hajnoczi , "Paolo Bonzini" Subject: [RFC PATCH 01/12] vdpa: correct the VIRTIO_DEVICE_F_MASK example value Date: Sun, 9 Aug 2026 18:19:59 +0000 Message-ID: <20260809182010.32931-2-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D043UWC001.ant.amazon.com (10.13.139.202) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" The comment above VIRTIO_DEVICE_F_MASK spells the mask out by hand as 0xfffffc000fffffff, which was correct while VIRTIO_TRANSPORT_F_END was 41. The bound is 42 now, so the macro expands to 0xfffff8000fffffff. The comment is the only place we write the expansion out, so someone checking a provisioned feature mask against it reads the wrong boundary between transport and per-device bits. The macro derives its value from the bound and needs no change, so correct the comment. Fixes: 838bebb4c926 ("virtio: Define feature bit for administration virtque= ue") Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- drivers/vdpa/vdpa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index 47c6c3d23f5c..1757a33f298f 100644 --- a/drivers/vdpa/vdpa.c +++ b/drivers/vdpa/vdpa.c @@ -548,7 +548,7 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff *msg, str= uct netlink_callback *cb) =20 /* * Bitmask for all per-device features: feature bits VIRTIO_TRANSPORT_F_ST= ART - * through VIRTIO_TRANSPORT_F_END are unset, i.e. 0xfffffc000fffffff for + * through VIRTIO_TRANSPORT_F_END are unset, i.e. 0xfffff8000fffffff for * all 64bit features. If the features are extended beyond 64 bits, or new * "holes" are reserved for other type of features than per-device, this * macro would have to be updated. From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-010.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-010.esa.us-west-2.outbound.mail-perimeter.amazon.com [52.12.53.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3EA45366074 for ; Sun, 9 Aug 2026 18:20:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=52.12.53.23 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299623; cv=none; b=c4xMb6HSv53dywW0YEzpQ7/QG62G46NkqURDv1c7sk5ZIeI2op4jeFjbP0uTQlVRHEkL+nrTV8oi0bTBmOtTJxPDMEUKhpushRzQFS/kMdNOB/dDnav0/t0sXS3IFn4h1GEm2iUtbdjM7nIaGjAXWuX6YKN55bc5jneEhuTwd2Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299623; c=relaxed/simple; bh=gjlLwSNnFDEPAFVzQhoA8ipTHI1t2/ZcdBup/Ck6aaE=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=jxibJ4WJHIGayc2fIs/p0WYYN5gTklimEK5jBhHDqBJ2HuFRO0A5dRDLE3IU07Cn9Q/oLtfmKBoe2vEOUnuPPU/3la+kpbms7G58Hrn7JZdfn1SJHTmlaaWDA976prIDev7UJyYTJR5fnGsH6edXeemFoIyiM7FCWhlUD3jy0V4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=UMwXBGTH; arc=none smtp.client-ip=52.12.53.23 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="UMwXBGTH" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299622; x=1817835622; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=84/xp2Y+X18kdGch+tpdJM7HA+HBrJiXOp2jp+iO4aU=; b=UMwXBGTHDm2xLOXlXL0CMzKgXOaerRLEZDd9m4UO4u7TMyPryoTZY7a4 sTDbTRN5rXMVNHVuWuEDY3Oa6arqJDIDrMF+l1p/0ZqiqG5I78K8u49BW ywdloXcJ7ikUk85x41roBitUQRzAutMJXDYvBOF2PKrrVGTMItCS/7uP3 GL5r6hV1mYs6NaSJ2fcvYD3v8CRaWAtmNmaIlgVX5YY+OPtBivCT4Etyw Ru6DlI3WO/Gwf6AMSgnIdAifzPeWouDtQ9kn5P4IH0qSTThJ/ssjMSAq4 uhhdNquJgLmBmniKaw9Gsp0TwyXvAJHgtnRRw7WNSS1ZiHWZk4aymMpjK w==; X-CSE-ConnectionGUID: dhgwElU8R/ShSAX2NjCLzg== X-CSE-MsgGUID: 1vXdf49BQsifaZJnFixo+w== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25395366" Received: from ip-10-5-9-48.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.9.48]) by internal-pdx-out-010.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:20:20 +0000 Received: from EX19MTAUWB001.ant.amazon.com [205.251.233.51:11000] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.19.171:2525] with esmtp (Farcaster) id e378ef4f-fe59-44b5-b2e4-eab231faa2d1; Sun, 9 Aug 2026 18:20:19 +0000 (UTC) X-Farcaster-Flow-ID: e378ef4f-fe59-44b5-b2e4-eab231faa2d1 Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWB001.ant.amazon.com (10.250.64.248) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:20:19 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:20:17 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , , , , Stefan Hajnoczi , Paolo Bonzini Subject: [RFC PATCH 02/12] virtio_ring: validate premapped addresses through the device's map Date: Sun, 9 Aug 2026 18:20:00 +0000 Message-ID: <20260809182010.32931-3-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D043UWC001.ant.amazon.com (10.13.139.202) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" Callers that forget to handle a mapping failure may accidentally pass DMA_MAPPING_ERROR as target map address and we don't error out for it. The mapped path in vring_map_one_sg() checks the result, but the premapped path takes the address as given. That makes it more difficult to identify accidental API misuse. Run the address through the existing vring_mapping_error(), which asks the device's virtio_map_ops mapping_error op or dma_mapping_error(), warn once and return -ENOMEM when it rejects the address. Document where the premapped helpers expect their addresses to come from, since an address from any other source is indistinguishable from a valid one. That way an API misuse shows up at the call that made it. Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- drivers/virtio/virtio_ring.c | 40 ++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index b438dc2ce1b8..9caa4f96204f 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -499,6 +499,19 @@ static int vring_map_one_sg(const struct vring_virtque= ue *vq, struct scatterlist if (premapped) { *addr =3D sg_dma_address(sg); *len =3D sg_dma_len(sg); + + /* + * The caller mapped this itself, so the map it used is the + * only thing that can judge the result. Ask it rather than + * skipping the check the mapped path performs: a caller that + * ignored a failed mapping would otherwise publish the + * reserved error value to the device. + */ + if (dev_WARN_ONCE(&vq->vq.vdev->dev, + vring_mapping_error(vq, *addr), + "premapped buffer holds no valid mapping\n")) + return -ENOMEM; + return 0; } =20 @@ -2910,6 +2923,14 @@ EXPORT_SYMBOL_GPL(virtqueue_add_outbuf); * @data: the token identifying the buffer. * @gfp: how to do memory allocations (if necessary). * + * Each entry of @sg must carry an address the caller obtained for this + * virtqueue: from the DMA API when virtqueue_dma_dev() returns a device, = and + * from virtqueue_map_page_attrs() when it returns NULL, because the device + * then interprets every address published to it in its own terms. Only an + * address the map itself rejects is caught here; an address from any other + * source is indistinguishable from a valid one and reaches the device + * unchanged. + * * Caller must ensure we don't call this with other virtqueue operations * at the same time (except where noted). * @@ -3008,6 +3029,14 @@ EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_ctx); * @ctx: extra context for the token * @gfp: how to do memory allocations (if necessary). * + * Each entry of @sg must carry an address the caller obtained for this + * virtqueue: from the DMA API when virtqueue_dma_dev() returns a device, = and + * from virtqueue_map_page_attrs() when it returns NULL, because the device + * then interprets every address published to it in its own terms. Only an + * address the map itself rejects is caught here; an address from any other + * source is indistinguishable from a valid one and reaches the device + * unchanged. + * * Caller must ensure we don't call this with other virtqueue operations * at the same time (except where noted). * @@ -3025,10 +3054,17 @@ int virtqueue_add_inbuf_premapped(struct virtqueue = *vq, EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_premapped); =20 /** - * virtqueue_dma_dev - get the dma dev + * virtqueue_dma_dev - get the device to use for DMA API calls * @_vq: the struct virtqueue we're talking about. * - * Returns the dma dev. That can been used for dma api. + * A NULL return means this virtqueue publishes no DMA addresses: either it + * needs no mapping at all, or the device supplies its own virtio_map_ops = and + * interprets every address published to it in its own terms. A caller th= at + * maps buffers itself must therefore check for NULL before using the DMA = API + * on this virtqueue's behalf, and use virtqueue_map_page_attrs() when it = is, + * which maps through whichever of the two the device uses. + * + * Return: the device to use for DMA API calls, or NULL when there is none. */ struct device *virtqueue_dma_dev(struct virtqueue *_vq) { From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-004.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-004.esa.us-west-2.outbound.mail-perimeter.amazon.com [44.246.77.92]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E8F89361DAB for ; Sun, 9 Aug 2026 18:20:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=44.246.77.92 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299647; cv=none; b=dg/0tT8v/Vpg64zVLu/AA9ehHMjJPucj4Y2/Z2dBImLGjHDFg0Vp2mmh+o/i/7q6sthMNLVYLu9O7XXoP4dzYkmqfIt3jmEtk5yhquGsLnqcmD6RnArAm23CSgHkRcX/o9xRTdN5FTSEI4kmBlpR2mfJUNoJqWQK44AdC2KqrBw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299647; c=relaxed/simple; bh=js1xNr3lPht0sT8p2tMOogwLbJAFyOPBU4L3h/xMZsg=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=PCMd7lCa946QtgVC7B6AicjevSAECV17M+7CF9frY/7DigZ+TQ5zVOqMjooy90BdE8ZyfY8nE3wEJh1qLXTOzbkhp1YJvLw98kvj5Tp9F0aImBB7s0eb3aK6ylcbtidiv/d7xF448IzIwKLkdtzHkxUTobUILWR5n9pa0La3zpM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=suZbrFJg; arc=none smtp.client-ip=44.246.77.92 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="suZbrFJg" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299645; x=1817835645; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=iybfP7f6LBxlJmNoFYXHpDFTtr6xuR9T27xQFliCRME=; b=suZbrFJg0l7J4gQyxQau/JYXCmWQz8eLdzv5auB54/BAmo3i/ZSEW3IY GnlOsR4iAr90tWTqAreSmAsQbnj53RYJZpEHxnXwcZ5ACurs35uTmCp2C s0KFLrtZeg0gcejEuq/pyHopYoH61UntAex7TdNgsWdSf0ttyH0dew65y CyxzleM8smbYkW6Zr/VKRFwwp+RHGdwgp+946SuwjIF4yTDjLjCSDw/MO tN0/WT6kJ23qBqqZTCCC4S3xTmH19pMUGKGYnd8VB4ZpcjTk2dwqZeP0Z RHAhb88iciGZpGELd1bKLzurm2JgHTFHiaqHxQGj9kks5/2Bm44fiUOBG w==; X-CSE-ConnectionGUID: twIFBTtiR/SM3xbFn8maPA== X-CSE-MsgGUID: hqi5oq8vSgyPPpZD2NEHfw== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25518625" Received: from ip-10-5-0-115.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.0.115]) by internal-pdx-out-004.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:20:43 +0000 Received: from EX19MTAUWC002.ant.amazon.com [205.251.233.51:26162] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.10.57:2525] with esmtp (Farcaster) id 9bf96873-3dbd-42a7-96b8-9d58065dc8bd; Sun, 9 Aug 2026 18:20:43 +0000 (UTC) X-Farcaster-Flow-ID: 9bf96873-3dbd-42a7-96b8-9d58065dc8bd Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWC002.ant.amazon.com (10.250.64.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:20:42 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:20:41 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , , , , Stefan Hajnoczi , Paolo Bonzini Subject: [RFC PATCH 03/12] virtio: add the VIRTIO_F_DMB feature bit Date: Sun, 9 Aug 2026 18:20:01 +0000 Message-ID: <20260809182010.32931-4-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D031UWC004.ant.amazon.com (10.13.139.246) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" In preparation to support VIRTIO_F_DMB, define the feature bit. A device that negotiates it holds its virtqueues and the buffers they reference in a Device Memory Buffer, a shared memory region the device owns. Every address the driver publishes to such a device is an offset into that region, so the device can only reach memory the driver placed there. Define VIRTIO_F_DMB as 44 and raise VIRTIO_TRANSPORT_F_END to 45, so virtio_dev_probe() preserves the bit into finalize_features() where a transport can act on it. VIRTIO_DEVICE_F_MASK derives from the same bound, and the expansion drivers/vdpa/vdpa.c writes out by hand moves with it. The feature is a proposal before the virtio Technical Committee and has no allocated number, so 44 is provisional. Link: https://lore.kernel.org/virtio-comment/20260804161202.38619-1-graf@am= azon.com/ Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- drivers/vdpa/vdpa.c | 2 +- include/uapi/linux/virtio_config.h | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index 1757a33f298f..ae9326394c2e 100644 --- a/drivers/vdpa/vdpa.c +++ b/drivers/vdpa/vdpa.c @@ -548,7 +548,7 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff *msg, str= uct netlink_callback *cb) =20 /* * Bitmask for all per-device features: feature bits VIRTIO_TRANSPORT_F_ST= ART - * through VIRTIO_TRANSPORT_F_END are unset, i.e. 0xfffff8000fffffff for + * through VIRTIO_TRANSPORT_F_END are unset, i.e. 0xffffc0000fffffff for * all 64bit features. If the features are extended beyond 64 bits, or new * "holes" are reserved for other type of features than per-device, this * macro would have to be updated. diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio= _config.h index 2445f365bce7..986e2b5fb3c8 100644 --- a/include/uapi/linux/virtio_config.h +++ b/include/uapi/linux/virtio_config.h @@ -52,7 +52,7 @@ * rest are per-device feature bits. */ #define VIRTIO_TRANSPORT_F_START 28 -#define VIRTIO_TRANSPORT_F_END 42 +#define VIRTIO_TRANSPORT_F_END 45 =20 #ifndef VIRTIO_CONFIG_NO_LEGACY /* Do we get callbacks when the ring is completely used, even if we've @@ -120,4 +120,19 @@ */ #define VIRTIO_F_ADMIN_VQ 41 =20 +/* + * This feature indicates that the device offers a Device Memory Buffer: a + * shared memory region, owned by the device, that holds the virtqueues and + * the buffers they reference. When this feature is negotiated, every add= ress + * that the driver places in a virtqueue is an offset into the Device Memo= ry + * Buffer region rather than a physical address or a bus address. The dev= ice + * is thereby confined to memory the driver has published in that region. + * + * The feature is a specification proposal before the virtio Technical + * Committee, not a ratified feature. The number below is provisional: the + * committee has allocated none for it, and it will change if the committee + * allocates a different one. Do not treat it as a stable interface yet. + */ +#define VIRTIO_F_DMB 44 + #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */ From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-010.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-010.esa.us-west-2.outbound.mail-perimeter.amazon.com [52.12.53.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 39EA53016E0 for ; Sun, 9 Aug 2026 18:20:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=52.12.53.23 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299646; cv=none; b=mwSAXJ3FfOv+LtFIHxsRgjCM/slepwmlxR7NvXx3qsdsS634fGs6mYq23pp3XeCEIHM+Vy5rEOmF+3DnzlC20HotA5NfbWwbFuoetFI5ID0K+feZ3ZJPXkpp3S0qPd9lTbNvNfvEedu443hZ0ZbI6lIDDMBiA7ynoylBWTlnKJY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299646; c=relaxed/simple; bh=VQxsBCBv3cflFMsN0lZ7zQ5TL/y8JS8pQ3kpAcS9A80=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=BrsK74E03Gj66EhqYqSuYXXfBK6BEXG92NorfZd1DC/kXJXCaVHhwpHnkDZov8JzvOz+pTSLfbGKfTKpB7dlJyG/M+JReuCAQeuHh9Ady8KCMZUdERZopmhZZ9Stt7T2hqxS/36voDveBvjsU3ENl2U1B9+UgsKgdw/+DVNtbps= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=cITf8BgQ; arc=none smtp.client-ip=52.12.53.23 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="cITf8BgQ" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299645; x=1817835645; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=c5qlgOuPfQrZFw2BxM5Ltl1DEUaFpcSxDb9rLnEjs6E=; b=cITf8BgQKvsGl9toh/R8M+0EalQX9OOmBrIaWKNuVn6Q7CwoASkokcUz mD90gfkvQTP67VuBZI90tbcdpVckOv1A0MP/GwWlFzyXDxruRs6BAJwXx zKranprRnc43o1OAbJRJvjRPby8ybnAXxkOSYCGbpE5V5lIUWflP2QSZv FLkotuXqLu7EvgCRdUA/Ncb3MTmUNegHkx0E9KMrOEWO3F38diMVU0RJ7 PUmHCarIQm7+2BZPEB2qbJYKDQv3oCzQ6nBCUHjkowR+vO32pXhwpbt1H jHbwfn+ZLILo+kd8Ber5jOZ9bKUMYEokBuJYo4YhtoL+JjsKTbTA7Xq69 Q==; X-CSE-ConnectionGUID: 0pNN5iPJTCmBu/xl52gJ4Q== X-CSE-MsgGUID: ztppbCuCT/2miS+w/ePxQA== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25395390" Received: from ip-10-5-0-115.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.0.115]) by internal-pdx-out-010.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:20:45 +0000 Received: from EX19MTAUWA001.ant.amazon.com [205.251.233.236:9195] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.19.171:2525] with esmtp (Farcaster) id eb4c9b36-36ab-44c4-8e42-937775d9e01d; Sun, 9 Aug 2026 18:20:44 +0000 (UTC) X-Farcaster-Flow-ID: eb4c9b36-36ab-44c4-8e42-937775d9e01d Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWA001.ant.amazon.com (10.250.64.204) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:20:44 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:20:42 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , , , , Stefan Hajnoczi , Paolo Bonzini Subject: [RFC PATCH 04/12] virtio_pci: read the device memory buffer shared memory id Date: Sun, 9 Aug 2026 18:20:02 +0000 Message-ID: <20260809182010.32931-5-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D031UWC004.ant.amazon.com (10.13.139.246) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" In preparation to support VIRTIO_F_DMB, read the shared memory id of the Device Memory Buffer. A device that negotiates VIRTIO_F_DMB reports it in a read-only le16 at the end of the common configuration structure, and it names the VIRTIO_PCI_CAP_SHARED_MEMORY_CFG capability that virtio_pci_find_shm_cap() has to find to map the region. Add the dmb_shm_id field, VIRTIO_PCI_COMMON_DMB_SHM_ID for the offset check_offsets() asserts it against, and vp_modern_get_dmb_shm_id() to read the register. vp_modern_probe() capped the common cfg mapping at the end of admin_queue_num, exactly where the new field starts, so extend it to the end of dmb_shm_id. vp_modern_map_capability() takes that as an upper bound, so a device with a shorter common cfg maps what it has and mdev->common_len records how much. The offset and VIRTIO_F_DMB are provisional until the virtio Technical Committee allocates them. Link: https://lore.kernel.org/virtio-comment/20260804161202.38619-1-graf@am= azon.com/ Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- drivers/virtio/virtio_pci_modern_dev.c | 23 ++++++++++++++++++++++- include/linux/virtio_pci_modern.h | 1 + include/uapi/linux/virtio_pci.h | 10 ++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio= _pci_modern_dev.c index 413a8c353463..7fbfcb305a62 100644 --- a/drivers/virtio/virtio_pci_modern_dev.c +++ b/drivers/virtio/virtio_pci_modern_dev.c @@ -211,6 +211,8 @@ static inline void check_offsets(void) offsetof(struct virtio_pci_modern_common_cfg, admin_queue_index)); BUILD_BUG_ON(VIRTIO_PCI_COMMON_ADM_Q_NUM !=3D offsetof(struct virtio_pci_modern_common_cfg, admin_queue_num)); + BUILD_BUG_ON(VIRTIO_PCI_COMMON_DMB_SHM_ID !=3D + offsetof(struct virtio_pci_modern_common_cfg, dmb_shm_id)); } =20 /* @@ -300,7 +302,7 @@ int vp_modern_probe(struct virtio_pci_modern_device *md= ev) mdev->common =3D vp_modern_map_capability(mdev, common, sizeof(struct virtio_pci_common_cfg), 4, 0, offsetofend(struct virtio_pci_modern_common_cfg, - admin_queue_num), + dmb_shm_id), &mdev->common_len, NULL); if (!mdev->common) goto err_map_common; @@ -752,6 +754,25 @@ u16 vp_modern_avq_index(struct virtio_pci_modern_devic= e *mdev) } EXPORT_SYMBOL_GPL(vp_modern_avq_index); =20 +/* + * vp_modern_get_dmb_shm_id - read the Device Memory Buffer shared memory = id + * @mdev: the modern virtio-pci device + * + * The value identifies the VIRTIO_PCI_CAP_SHARED_MEMORY_CFG capability th= at + * describes the Device Memory Buffer region. Only valid once VIRTIO_F_DMB + * has been negotiated. + * + * Returns the shared memory id. + */ +u16 vp_modern_get_dmb_shm_id(struct virtio_pci_modern_device *mdev) +{ + struct virtio_pci_modern_common_cfg __iomem *cfg; + + cfg =3D (struct virtio_pci_modern_common_cfg __iomem *)mdev->common; + return vp_ioread16(&cfg->dmb_shm_id); +} +EXPORT_SYMBOL_GPL(vp_modern_get_dmb_shm_id); + MODULE_VERSION("0.1"); MODULE_DESCRIPTION("Modern Virtio PCI Device"); MODULE_AUTHOR("Jason Wang "); diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_m= odern.h index 9a3f2fc53bd6..c396f8e6055b 100644 --- a/include/linux/virtio_pci_modern.h +++ b/include/linux/virtio_pci_modern.h @@ -162,4 +162,5 @@ int vp_modern_get_queue_reset(struct virtio_pci_modern_= device *mdev, u16 index); void vp_modern_set_queue_reset(struct virtio_pci_modern_device *mdev, u16 = index); u16 vp_modern_avq_num(struct virtio_pci_modern_device *mdev); u16 vp_modern_avq_index(struct virtio_pci_modern_device *mdev); +u16 vp_modern_get_dmb_shm_id(struct virtio_pci_modern_device *mdev); #endif diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pc= i.h index e732e3456e27..052a1047f48f 100644 --- a/include/uapi/linux/virtio_pci.h +++ b/include/uapi/linux/virtio_pci.h @@ -193,6 +193,15 @@ struct virtio_pci_modern_common_cfg { =20 __le16 admin_queue_index; /* read-only */ __le16 admin_queue_num; /* read-only */ + + /* + * Reports the shmid of the Device Memory Buffer region. The feature + * is a specification proposal before the virtio Technical Committee, + * and both this offset and the VIRTIO_F_DMB feature bit are + * provisional: neither has been allocated, so a device must not rely + * on either until they have been. + */ + __le16 dmb_shm_id; /* read-only */ }; =20 /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */ @@ -235,6 +244,7 @@ struct virtio_pci_cfg_cap { #define VIRTIO_PCI_COMMON_Q_RESET 58 #define VIRTIO_PCI_COMMON_ADM_Q_IDX 60 #define VIRTIO_PCI_COMMON_ADM_Q_NUM 62 +#define VIRTIO_PCI_COMMON_DMB_SHM_ID 64 =20 #endif /* VIRTIO_PCI_NO_MODERN */ From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-009.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-009.esa.us-west-2.outbound.mail-perimeter.amazon.com [35.155.198.111]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DACBA37B021 for ; Sun, 9 Aug 2026 18:20:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=35.155.198.111 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299650; cv=none; b=gwFnLGoyydnjEg3fXChSSXDLIdboIq7aC+dNppt2/ji3/QfrO5PGXYMh6m9raT/iPkbPEOfQLaFy2bNLc4H847tAGjARkMPAvbeWqfgyCth+LY9YLyy3hPuJ6S1Jw2CbxP7WjZ2aE47qV5yC1BpVIb6beGt4BNBbR9IyYcT0DlI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299650; c=relaxed/simple; bh=UpSklniRYvM1jOwXC9qKeQR408LhTPkVknKi5vZdDBM=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=pE8b9xGblhmgAN/zsgUFnnJButOk5qKMUJUcWUEu0zCurYdh1MMQiiQDtyIR5cLggL89TrrBjxko3DwGea15FQRbedhXjWDpSTUc/OP68z7VbiYmrUuzKla4N+lUdvdQHAMZv/glOyahB0ZZphnZBelNL/kEWH8QM84LnxFmhOI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=OI8HKH09; arc=none smtp.client-ip=35.155.198.111 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="OI8HKH09" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299648; x=1817835648; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=o47xSUAJjSkI+7xfGGQqd+W9hTYM04zaBrXdkO5FaB4=; b=OI8HKH09E8gnKhpXlokfJ1ktvvUl7NHnrNfHufJy9/ImsYeS3dLX737M TfIdb7LeLrIcw5XlDthGRjDeFRYEx3M0UzJTxQaspDKZnSMZoBAMWUWWK wywx8DnTusr/ByICvGZ49oa26uF+EzYbnJ7xFNyp0NbGBQi4KG7CH2qjK U8R+FLIZGwhlyJASTbcOJEbXHbGbU/oKipXHp34F3o4khOHTB0ya8WGt9 LA2kqYpF9zO6EUgL3lDPCPTffE1hskx/ftFrbvbOiKhuZR87N5Qq+W7nq u1vEjZygkYRr74iBRYrrlXAvS0ckyxNFoHFM0pm6TH4yVo1oclYoKOkgr Q==; X-CSE-ConnectionGUID: KfApvsFoTF2O23nk8nFfag== X-CSE-MsgGUID: Y3OG7g5RTJGC1nVjhhq4ug== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25415435" Received: from ip-10-5-6-203.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.6.203]) by internal-pdx-out-009.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:20:46 +0000 Received: from EX19MTAUWC001.ant.amazon.com [205.251.233.53:25043] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.60.103:2525] with esmtp (Farcaster) id 69ba2a84-8ff1-4856-af59-f9de74bbe5fd; Sun, 9 Aug 2026 18:20:46 +0000 (UTC) X-Farcaster-Flow-ID: 69ba2a84-8ff1-4856-af59-f9de74bbe5fd Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWC001.ant.amazon.com (10.250.64.174) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:20:46 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:20:44 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , , , , Stefan Hajnoczi , Paolo Bonzini Subject: [RFC PATCH 05/12] virtio_pci: create virtqueues with the device's mapping token Date: Sun, 9 Aug 2026 18:20:03 +0000 Message-ID: <20260809182010.32931-6-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D031UWC004.ant.amazon.com (10.13.139.246) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" In preparation to support VIRTIO_F_DMB, create the modern PCI transport's virtqueues with the mapping token held on the virtio_device. Every mapping call for a virtqueue is handed a union virtio_map token, and vring_create_virtqueue() derives that token as {.dma_dev =3D vdev->dev.parent} without letting its callers choose. Set vdev->vmap.dma_dev in virtio_pci_modern_probe() and create the virtqueues through vring_create_virtqueue_map() instead. That covers both the virtqueues from vp_modern_find_vqs() and the admin virtqueue, which go through the same setup_vq(). virtio_pci_probe() assigns vdev->dev.parent first, so the token holds the pointer vring_create_virtqueue() would have derived. The legacy transport keeps its own vring_create_virtqueue() call: vp_legacy_get_features() reads 32 feature bits, so VIRTIO_F_DMB cannot reach it. Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- drivers/virtio/virtio_pci_modern.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci= _modern.c index 6d8ae2a6a8ca..565d37b630b3 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -715,10 +715,11 @@ static struct virtqueue *setup_vq(struct virtio_pci_d= evice *vp_dev, info->msix_vector =3D msix_vec; =20 /* create the vring */ - vq =3D vring_create_virtqueue(index, num, - SMP_CACHE_BYTES, &vp_dev->vdev, - true, true, ctx, - notify, callback, name); + vq =3D vring_create_virtqueue_map(index, num, + SMP_CACHE_BYTES, &vp_dev->vdev, + true, true, ctx, + notify, callback, name, + vp_dev->vdev.vmap); if (!vq) return ERR_PTR(-ENOMEM); =20 @@ -1289,6 +1290,14 @@ int virtio_pci_modern_probe(struct virtio_pci_device= *vp_dev) vp_dev->isr =3D mdev->isr; vp_dev->vdev.id =3D mdev->id; =20 + /* + * The mapping token every virtqueue of this device is created with. + * This is the same value vring_create_virtqueue() would derive from + * vdev->dev.parent, kept here so that a transport feature can replace + * it in one place. + */ + vp_dev->vdev.vmap.dma_dev =3D &pci_dev->dev; + spin_lock_init(&vp_dev->admin_vq.lock); return 0; } From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-010.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-010.esa.us-west-2.outbound.mail-perimeter.amazon.com [52.12.53.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A77BA3016E0 for ; Sun, 9 Aug 2026 18:21:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=52.12.53.23 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299674; cv=none; b=vE/KnQVRzmZAlTvNZkXxUhWZHWCYidpTmiiZjWc2U1Vqn9HAbPs6IRhBFEltauce1+bSD3w3zsdbYjRJlUZQ07PE9TnpPjWkV3CdXxl9uarq3FppAnIyKPZTHU7RhCHmTFnzEaGsfftEjed2QxYKage0XVKJJ8RjkOcCvbBef88= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299674; c=relaxed/simple; bh=hebyaLjc9gMauOlzkq6qsrLYEkUF2vwN+ac6VRQ+JMU=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=CLoE4ubOPRPjqYxwjTiCt1sPtJNeid+fnuboEk5pr0dQ7qGQauI2ipqpj3Nm0xPrsRocTqwzkZzXMyxliJRtwDTQ3NIp+fRVWhTGB6InmHRD+3FvogAP+HYdcVr0Jayzf/nM/HlqHumUZFg5fbMSlXT37XcADXn1mXT09XKl2eY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=TNkCfvch; arc=none smtp.client-ip=52.12.53.23 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="TNkCfvch" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299670; x=1817835670; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=u4wNonYwFOiOvenEfMQaGFydeHgig3hpTURXNX/0ROI=; b=TNkCfvch4PLrag7AsuzN0NwxpFKW605DDn8ApmDKZF4fZJ8OQwlKL+gF DqADfgG6hkaf9qHYq+ZMtVX/RovKN7kBYEhD5K6ATETpiqwllHBQOFRO8 p0od/9tazNW1dLdC54ai9L4wUUemRV6+UY7T1o///EMgLsnIwPzVgZHdq 2LHXztzjYzcL5BCGS2ysWAFKmyHyQAFmPCL5mAkkkunjTK8mHBvOhFwtp SZkzfXBMFDThUaznF08R3WRAFv18N/SjBW8EwWBJB/BvYTGjqpok2w2E/ j5MqLOyFfD0A/cHG8TUgRQ07535Of45mXOz32JLNm0pMYbebhFx6sp+qY g==; X-CSE-ConnectionGUID: 77SPrsrKQlaYr1mTZdlUYQ== X-CSE-MsgGUID: AoXTTJDLR1uMgpwIUYqTRQ== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25395416" Received: from ip-10-5-0-115.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.0.115]) by internal-pdx-out-010.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:21:10 +0000 Received: from EX19MTAUWB002.ant.amazon.com [205.251.233.48:7752] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.5.32:2525] with esmtp (Farcaster) id cfb5edc4-4673-44e3-8a36-31ce97be2ede; Sun, 9 Aug 2026 18:21:10 +0000 (UTC) X-Farcaster-Flow-ID: cfb5edc4-4673-44e3-8a36-31ce97be2ede Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWB002.ant.amazon.com (10.250.64.231) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:09 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:07 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , , , , "Stefan Hajnoczi" , Paolo Bonzini Subject: [RFC PATCH 06/12] virtio: add a device memory buffer region allocator Date: Sun, 9 Aug 2026 18:20:04 +0000 Message-ID: <20260809182010.32931-7-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D035UWB004.ant.amazon.com (10.13.138.104) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" In preparation to support VIRTIO_F_DMB, create a mechanism to allocate and map memory from the Device Memory Buffer (DMB). The DMB is a shared memory region a device exposes and owns. A device that negotiates the feature expects its virtqueues and all the buffers we hand it to live in that region, and every address we publish to it is a byte offset into the region. Add virtio_dmb_init(), which locates the region by the shared memory id the device reports and builds a page-granular allocator over it, and virtio_dmb_destroy() to tear that down. Add virtio_dmb_map_ops, a struct virtio_map_ops implementation that hands out allocations from that allocator as region offsets: alloc() places a virtqueue area in the region, map_page() copies a buffer that lives elsewhere into it and copies it back on unmap. The map operations reach that allocator through a new dmb member of union virtio_map. The shared memory id is transport specific, so add a get_dmb_shm_id() callback to struct virtio_config_ops for a transport to report it. A transport that does not implement it must not accept VIRTIO_F_DMB. Add CONFIG_VIRTIO_DMB to enable this support. It defaults to y, and a kernel that will never meet such a device can turn it off to leave the allocator and its bookkeeping out. Link: https://lore.kernel.org/virtio-comment/20260804161202.38619-1-graf@am= azon.com/ Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- drivers/virtio/Kconfig | 15 + drivers/virtio/Makefile | 3 +- drivers/virtio/virtio_dmb.c | 1317 +++++++++++++++++++++++++++++++++ drivers/virtio/virtio_dmb.h | 28 + include/linux/virtio.h | 3 + include/linux/virtio_config.h | 8 + 6 files changed, 1373 insertions(+), 1 deletion(-) create mode 100644 drivers/virtio/virtio_dmb.c create mode 100644 drivers/virtio/virtio_dmb.h diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index ce5bc0d9ea28..b6b5a36c3c21 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -188,6 +188,21 @@ config VIRTIO_DEBUG =20 If unsure, say N. =20 +config VIRTIO_DMB + bool "Device Memory Buffer support" + depends on VIRTIO + default y + help + Support devices that place their virtqueues and buffers in a shared + memory region they own, rather than in memory the driver allocates. + + Enabling this adds a page allocator and per-page bookkeeping, both + of which are set up only for a device that negotiates the feature. + When disabled the feature is never accepted and such a device is + driven as an ordinary one. + + If unsure, say Y. + config VIRTIO_RTC tristate "Virtio RTC driver" depends on VIRTIO diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile index eefcfe90d6b8..ba785ff44a16 100644 --- a/drivers/virtio/Makefile +++ b/drivers/virtio/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 -obj-$(CONFIG_VIRTIO) +=3D virtio.o virtio_ring.o +virtio-dmb-$(CONFIG_VIRTIO_DMB) :=3D virtio_dmb.o +obj-$(CONFIG_VIRTIO) +=3D virtio.o virtio_ring.o $(virtio-dmb-y) obj-$(CONFIG_VIRTIO_ANCHOR) +=3D virtio_anchor.o obj-$(CONFIG_VIRTIO_PCI_LIB) +=3D virtio_pci_modern_dev.o obj-$(CONFIG_VIRTIO_PCI_LIB_LEGACY) +=3D virtio_pci_legacy_dev.o diff --git a/drivers/virtio/virtio_dmb.c b/drivers/virtio/virtio_dmb.c new file mode 100644 index 000000000000..42126f928bc0 --- /dev/null +++ b/drivers/virtio/virtio_dmb.c @@ -0,0 +1,1317 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Device Memory Buffer support for virtio devices. + * + * A device that negotiates VIRTIO_F_DMB owns one shared memory region, the + * Device Memory Buffer, that holds its virtqueues and the buffers they + * reference. Every address the driver publishes to such a device is a by= te + * offset from the start of that region. + * + * This file provides an allocator over the region and the virtio_map_ops + * implementation that turns allocations into those offsets. A mapping ha= ndle + * belonging to a DMB device is a region offset and nothing else: no code + * outside these operations may treat it as a DMA address. + * + * The region is shared with the device, which may read or write any of it= at + * any time. Nothing this file reads back from the region is used to comp= ute + * a kernel address, a length or an index. Handles and their sizes arrive + * from the ring's own bookkeeping in kernel memory, and are range-checked + * anyway so that a bug there cannot reach outside the arrays below. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "virtio_dmb.h" + +/* No source recorded for this slot: it holds no bounced mapping. */ +#define DMB_SRC_NONE ((phys_addr_t)-1) + +/** + * struct virtio_dmb_slot - what one PAGE_SIZE slot of the pool records + * @src: physical address this slot bounces, DMB_SRC_NONE for a virtqueue = area + * @end: one past the last slot of the allocation this slot belongs to, or + * zero when the slot is free + * @tail: how many bytes of slot @end - 1 the allocation covers, PAGE_SIZE= when + * its length is a whole number of pages, zero when the slot is free + * + * Every slot of an allocation records the same @end and the same @tail, s= o a + * handle that points into the middle of one still yields both the allocat= ion's + * extent in slots and its end in bytes from a single read. + * + * @tail is what makes the length a mapping is bounced against a byte count + * rather than a page count. It is bounded by PAGE_SIZE, so u32 holds it = on + * every configuration, which a whole length would not: virtio_dmb_op_allo= c() + * bounds a request by the entire pool and a pool may exceed 4 GiB. + */ +struct virtio_dmb_slot { + phys_addr_t src; + u32 end; + u32 tail; +}; + +/* + * Bounds on how many slots one area covers. + * + * The floor is one cacheline of bitmap, so that no two areas contend on t= he + * line their separate locks exist to keep apart, and at least 512 slots. = On a + * 64-byte line those coincide; a wider line raises the floor, which is + * correct. Either way the floor is a power of two of at least BITS_PER_L= ONG, + * which is what makes an area own whole bitmap words and what makes the a= rea + * of a slot a shift; virtio_dmb_init() asserts both rather than leaving t= hem + * to inspection. + * + * The ceiling has no counterpart in kernel/dma/swiotlb.c, which this geom= etry + * otherwise follows, and it is needed because the search differs: swiotlb + * finds a run through a per-slot free-run list, while this sweeps a bitma= p, + * so the cost of one search here is linear in the size of an area. Witho= ut a + * ceiling that cost grows with the region, which is the thing being bound= ed. + */ +#define DMB_AREA_CACHELINE_SLOTS ((unsigned int)L1_CACHE_BYTES * BITS_PER_= BYTE) +#define DMB_AREA_MIN_SLOTS (DMB_AREA_CACHELINE_SLOTS > 512u ? \ + DMB_AREA_CACHELINE_SLOTS : 512u) +#define DMB_AREA_MAX_SLOTS 4096u + +/** + * struct virtio_dmb_area - one independently locked range of the pool + * @used: slots of this area that are allocated; exact under @lock + * @index: slot this area's next search starts from, relative to the area = base + * @lock: covers this area's bits of the pool bitmap, @used and @index + * + * Cacheline-aligned where that means anything, so that two areas' locks d= o not + * share a line; the alignment compiles away on !SMP, where nothing conten= ds. + */ +struct virtio_dmb_area { + unsigned int used; + unsigned int index; + /* Serialises this area's bits of the pool bitmap, @used and @index. */ + spinlock_t lock; +} ____cacheline_aligned_in_smp; + +/** + * struct virtio_dmb - driver-side state for one Device Memory Buffer + * @vdev: the device that owns the region, for message context + * @map_va: what memremap() returned, for memunmap() + * @map_phys: physical base of the region, for release_mem_region() + * @map_len: length of the claimed and mapped part of the region + * @map_claimed: whether request_mem_region() succeeded for that range + * @prev_map: map operations the transport had installed, restored on tear= down + * @prev_vmap: mapping token that went with @prev_map + * @base_va: kernel address the pool starts at, inside the mapping + * @base_off: pool start as an offset from the start of the region + * @nslots: pool size in PAGE_SIZE slots + * @bitmap: @nslots bits, set when the slot is allocated + * @slots: @nslots slot records + * @areas: the @nareas ranges the pool is divided into + * @nareas: how many areas the pool is divided into + * @area_slots: slots one area covers, a power of two; the last area covers + * fewer when @nslots is not a multiple of it + * @area_shift: ilog2(@area_slots), so slot >> @area_shift names its area + * @shm_id: shared memory id the device reported for the region + */ +struct virtio_dmb { + struct virtio_device *vdev; + void *map_va; + phys_addr_t map_phys; + size_t map_len; + bool map_claimed; + const struct virtio_map_ops *prev_map; + union virtio_map prev_vmap; + void *base_va; + u64 base_off; + unsigned int nslots; + unsigned long *bitmap; + struct virtio_dmb_slot *slots; + struct virtio_dmb_area *areas; + unsigned int nareas; + unsigned int area_slots; + unsigned int area_shift; + u16 shm_id; +}; + +/* First slot of area @i. */ +static unsigned int virtio_dmb_area_base(const struct virtio_dmb *dmb, + unsigned int i) +{ + return i << dmb->area_shift; +} + +/* Slots area @i covers. The last area is short unless nslots divides. */ +static unsigned int virtio_dmb_area_len(const struct virtio_dmb *dmb, + unsigned int i) +{ + return min(dmb->area_slots, + dmb->nslots - virtio_dmb_area_base(dmb, i)); +} + +static unsigned int virtio_dmb_slots(size_t size) +{ + return DIV_ROUND_UP(size, PAGE_SIZE); +} + +static size_t virtio_dmb_pool_size(const struct virtio_dmb *dmb) +{ + return (size_t)dmb->nslots << PAGE_SHIFT; +} + +/* + * The largest buffer mapping this pool will serve: the smaller of an eigh= th of + * the pool and half of one area, but never less than one page. That floo= r is + * what governs a region at the four-slot minimum, since an eighth of four + * pages rounds down to nothing. + * + * The eighth is a choice, not a derived value: it bounds the capacity one + * mapping can deny the rest of the device to seven eighths of the pool, so + * that a device with several virtqueues can still make forward progress w= hile + * one large mapping is outstanding. + * + * The half-area is derived, and it is the reason this function has to be + * consulted rather than the eighth alone. An allocation has to lie insid= e one + * area, because that is what lets one lock cover it and what lets the rel= ease + * path find that lock from the slot index. Half rather than all of an ar= ea is + * headroom: a request the size of a whole area could only ever be satisfi= ed by + * a completely empty one, and an area may be as small as + * DMB_AREA_MIN_SLOTS, so there is no expectation that one is empty. The + * figure is half of the nominal area size, so a request at the cap can ex= ceed + * the short last area outright; that costs a claim in one area out of nar= eas + * and the walk tries the others. + * + * The cap applies to map_page() only. An alloc() is a virtqueue area, wh= ich + * is structural rather than in-flight: it lives for as long as the queue + * does and no back-pressure can defer it, so a cap on it could only shrin= k a + * queue on a region that is too small, or refuse one outright where the r= ing + * layout cannot be shrunk. Sizing the region for the areas as well as the + * buffers is the device's obligation. + */ +static size_t virtio_dmb_max_mapping(const struct virtio_dmb *dmb) +{ + size_t eighth =3D ALIGN_DOWN(virtio_dmb_pool_size(dmb) / 8, PAGE_SIZE); + size_t half_area =3D ((size_t)dmb->area_slots / 2) << PAGE_SHIFT; + + return max_t(size_t, min(eighth, half_area), PAGE_SIZE); +} + +/* + * Claim @nr contiguous slots from area @i, or -ENOMEM when that one area + * cannot satisfy the request. Takes and drops that area's lock and touch= es + * no other area's state, so no path ever holds two of these locks and the= re + * is no ordering between them to get right. + */ +static long virtio_dmb_area_claim(struct virtio_dmb *dmb, unsigned int i, + unsigned int nr) +{ + struct virtio_dmb_area *area =3D &dmb->areas[i]; + unsigned int base =3D virtio_dmb_area_base(dmb, i); + unsigned int end =3D base + virtio_dmb_area_len(dmb, i); + unsigned long flags, slot; + + spin_lock_irqsave(&area->lock, flags); + + /* + * Exact, and inside the lock. Written as a subtraction from the + * area's own length rather than as len - used < nr, which underflows. + */ + if (nr > (end - base) - area->used) + goto not_found; + + /* + * Both sweeps are bounded at the area end, so an allocation cannot + * span two areas and the release path can find one lock from the slot + * index. bitmap_find_next_zero_area() returns a value whose sum with + * @nr exceeds the size it was given when it finds nothing, so that sum + * is the test; the whole-pool "slot >=3D nslots" form does not transfer. + */ + slot =3D bitmap_find_next_zero_area(dmb->bitmap, end, + base + area->index, nr, 0); + if (slot + nr > end && area->index) + slot =3D bitmap_find_next_zero_area(dmb->bitmap, end, base, + nr, 0); + if (slot + nr > end) + goto not_found; + + bitmap_set(dmb->bitmap, slot, nr); + area->used +=3D nr; + area->index =3D slot + nr < end ? slot + nr - base : 0; + + spin_unlock_irqrestore(&area->lock, flags); + + return slot; + +not_found: + spin_unlock_irqrestore(&area->lock, flags); + + return -ENOMEM; +} + +/* + * Claim @nr contiguous slots. Returns the first slot, or -ENOMEM when no + * area can satisfy the request. Exhaustion is a routine condition: the + * region's length bounds how much virtqueue data can be in flight. What a + * caller makes of it is the caller's, and it is not always back-pressure:= a + * network receive fill has nothing to push back on when it cannot post a + * buffer, and repolls instead. Reporting it at any level a working device + * would print would therefore be a log flood, and it is the only signal an + * undersized region produces at all, so it is reported through dynamic de= bug + * where it costs nothing until somebody asks for it. + * + * Next fit within one area, from a hint that advances past each claim and + * rewinds to each release, beginning in the area belonging to the running= CPU + * and then trying each other area in turn. So a multi-slot request can f= ail + * while the total free count would have satisfied it: a map_page() reques= t is + * bounded by virtio_dmb_max_mapping(), which keeps it inside one area, but + * fragmentation within that area can still cost capacity and fail an + * individual mapping. Nothing is moved to recover: handles are live in + * descriptors the device is reading. + * + * The walk visits every area and tests capacity inside that area's lock, = so a + * refusal is a true statement about the pool rather than about one area. = It + * releases the lock and restores interrupts between areas, which is what + * bounds the interrupts-off window to a single area's sweep; the total wo= rk in + * the failing case is a whole-pool sweep either way. + */ +static long virtio_dmb_claim(struct virtio_dmb *dmb, unsigned int nr) +{ + unsigned int i, start; + long ret; + + /* + * raw_smp_processor_id() and not smp_processor_id(): the index is + * computed before any lock is taken, so preemption or migration + * between the read and the claim only changes which area is tried + * first. kernel/dma/swiotlb.c picks its home area on the same + * reasoning. + */ + start =3D raw_smp_processor_id() % dmb->nareas; + i =3D start; + do { + ret =3D virtio_dmb_area_claim(dmb, i, nr); + if (ret >=3D 0) + return ret; + + if (++i >=3D dmb->nareas) + i =3D 0; + } while (i !=3D start); + + /* + * The geometry rather than a free count: there is no instant at which + * a total free count is true under per-area locking, so printing one + * would mean either a walk taking every lock or a torn read. + */ + dev_dbg_ratelimited(&dmb->vdev->dev, + "device memory buffer has no run of %u pages in any of %u areas of = %u pages\n", + nr, dmb->nareas, dmb->area_slots); + + return -ENOMEM; +} + +static void virtio_dmb_release(struct virtio_dmb *dmb, unsigned int slot, + unsigned int nr) +{ + struct virtio_dmb_area *area; + unsigned long flags; + unsigned int i, a; + + if (dev_WARN_ONCE(&dmb->vdev->dev, + !nr || slot >=3D dmb->nslots || nr > dmb->nslots - slot, + "bad device memory buffer slot range %u+%u\n", + slot, nr)) + return; + + /* + * One area holds the whole allocation, so one lock covers it. The + * allocator establishes that by bounding both of its sweeps at an area + * end; enforce it here rather than inherit it, because this is the path + * that depends on it to pick a lock at all, and picking the wrong one + * would clear bits and adjust a count under a lock that does not cover + * either. + */ + a =3D slot >> dmb->area_shift; + if (dev_WARN_ONCE(&dmb->vdev->dev, + ((slot + nr - 1) >> dmb->area_shift) !=3D a, + "device memory buffer allocation %u+%u spans two areas\n", + slot, nr)) + return; + area =3D &dmb->areas[a]; + + spin_lock_irqsave(&area->lock, flags); + + /* + * Releasing a range that is not wholly allocated would put slots that + * a different allocation now owns back on the free list, which is what + * a second release of one handle does. The bitmap is the only record + * that can answer whether that is happening, and the test costs less + * than the bitmap_clear() it guards. + */ + if (dev_WARN_ONCE(&dmb->vdev->dev, + find_next_zero_bit(dmb->bitmap, slot + nr, slot) < + slot + nr, + "device memory buffer double release %u+%u\n", + slot, nr)) + goto out; + + /* + * Clear the records before the bits, so that a slot reachable from the + * free list never carries an extent that virtio_dmb_resolve() would + * trust. Doing it here rather than in the callers covers every + * release path with one copy of the invariant. + * + * WRITE_ONCE() because virtio_dmb_resolve() reads these three fields + * without the lock, which is the pattern + * tools/memory-model/Documentation/access-marking.txt calls + * "Lock-Protected Writes With Lockless Reads" and asks to be marked on + * both sides. + */ + for (i =3D slot; i < slot + nr; i++) { + WRITE_ONCE(dmb->slots[i].src, DMB_SRC_NONE); + WRITE_ONCE(dmb->slots[i].end, 0); + WRITE_ONCE(dmb->slots[i].tail, 0); + } + + bitmap_clear(dmb->bitmap, slot, nr); + area->used -=3D nr; + area->index =3D slot - virtio_dmb_area_base(dmb, a); + +out: + spin_unlock_irqrestore(&area->lock, flags); +} + +/** + * struct virtio_dmb_ref - a handle resolved against the pool + * @slot: the slot the handle lands in + * @nr: slots the allocation still holds from @slot on + * @src: the physical address @slot bounces, or DMB_SRC_NONE for a queue a= rea + * + * @nr and @src are derived from the same read of the record that validate= d the + * handle, so the extent a caller releases and the pages the copy touches = are + * the ones that were checked and not a later re-read of a field another C= PU may + * meanwhile have cleared. + */ +struct virtio_dmb_ref { + unsigned int slot; + unsigned int nr; + phys_addr_t src; +}; + +/* Extra conditions virtio_dmb_resolve() enforces for particular callers. = */ +#define DMB_RESOLVE_BOUNCED BIT(0) /* must be a mapping, not a queue area = */ +#define DMB_RESOLVE_WHOLE BIT(1) /* must be the allocation's first slot */ + +/* + * Turn a handle and a length into a slot index and an extent, rejecting + * anything that does not lie wholly inside the pool. The bound is exclus= ive, + * so a handle at the end of the pool and a zero length are both refused. = The + * arithmetic is done in u64 so that it cannot wrap where dma_addr_t is + * narrower. + * + * A handle may point into the middle of a mapping, because + * virtqueue_map_sync_single_range_for_cpu() and its counterpart pass + * handle + offset, so the extent is taken from the slot the handle lands = in + * rather than from the first slot of the allocation. That rejects a hand= le + * released twice, a length that runs past the end of its allocation, and a + * range that would continue into a neighbouring one. + * + * The length is checked in bytes, not in pages. A page-granular check wo= uld + * pass a length ending anywhere inside the allocation's last slot, which = is up + * to PAGE_SIZE - 1 bytes past what was mapped, and virtio_dmb_copy() would + * then touch the page after the source run. swiotlb_bounce() keeps the s= ame + * bound in alloc_size (kernel/dma/swiotlb.c:890) and clamps an over-long + * mapping to it, because it has callers it cannot refuse. Every caller h= ere + * passes a length the map side recorded, so an over-long one is a caller = bug + * and the copy is refused outright rather than truncated. + * + * DMB_RESOLVE_BOUNCED additionally requires the allocation to be one this= file + * bounced rather than a virtqueue area. DMB_RESOLVE_WHOLE requires the h= andle + * to be the start of its allocation, which is what the callers that relea= se it + * need: mid-extent tolerance exists for the sync ops alone, and a mid-ext= ent + * release would free the tail of an allocation and leak its head for good. + * + * The resolved slot is read without the lock, which is legitimate for a c= aller + * that owns it: virtio_dmb_claim() published it exclusively to this calle= r, so + * nothing else writes it. The DMB_RESOLVE_WHOLE test also reads the prec= eding + * slot, which the caller does not own, and that read rests on a different + * argument, stated where it is made. + */ +static bool virtio_dmb_resolve(struct virtio_dmb *dmb, dma_addr_t handle, + size_t size, unsigned int rflags, + struct virtio_dmb_ref *ref) +{ + u64 off, end, limit; + phys_addr_t src; + u32 slot_end, tail; + + if (!size || (u64)handle < dmb->base_off) + goto bad_handle; + + off =3D (u64)handle - dmb->base_off; + if (check_add_overflow(off, (u64)size, &end)) + goto bad_handle; + if (end > (u64)virtio_dmb_pool_size(dmb)) + goto bad_handle; + + ref->slot =3D off >> PAGE_SHIFT; + + /* + * Snapshot the record here, once. Every test below, the extent the + * caller goes on to release and the pages virtio_dmb_copy() touches are + * taken from these three reads rather than from a second look at the + * entry, so what was validated is what gets used. Reading the slot + * without the lock is legitimate for a caller that owns it, as above, + * but the callers these tests exist to catch are exactly the ones that + * do not own it, and for those another CPU may be writing the entry. + */ + slot_end =3D READ_ONCE(dmb->slots[ref->slot].end); + tail =3D READ_ONCE(dmb->slots[ref->slot].tail); + src =3D READ_ONCE(dmb->slots[ref->slot].src); + + if (dev_WARN_ONCE(&dmb->vdev->dev, !slot_end, + "device memory buffer handle %pad holds no allocation\n", + &handle)) + return false; + + /* The allocation's exclusive end in bytes, reachable from any slot. */ + limit =3D ((u64)(slot_end - 1) << PAGE_SHIFT) + tail; + + if (dev_WARN_ONCE(&dmb->vdev->dev, end > limit, + "device memory buffer handle %pad length %zu leaves its allocation\n", + &handle, size)) + return false; + + /* + * Allocations are disjoint and every slot of one records the same end, + * so the preceding slot shares that end if and only if this slot is not + * the first of the allocation. + * + * That slot may belong to another caller, which is the read the + * ownership argument above does not cover. Disjointness covers it + * instead: a neighbouring allocation ends at or before this slot and a + * free slot records zero, so whichever of the two a concurrent + * virtio_dmb_record() or virtio_dmb_release() leaves visible, neither + * can equal slot_end. + */ + if ((rflags & DMB_RESOLVE_WHOLE) && + dev_WARN_ONCE(&dmb->vdev->dev, + !IS_ALIGNED(off, PAGE_SIZE) || + (ref->slot && + READ_ONCE(dmb->slots[ref->slot - 1].end) =3D=3D slot_end), + "device memory buffer handle %pad is not the start of its allocation\= n", + &handle)) + return false; + + if ((rflags & DMB_RESOLVE_BOUNCED) && + dev_WARN_ONCE(&dmb->vdev->dev, src =3D=3D DMB_SRC_NONE, + "device memory buffer handle %pad holds no mapping\n", + &handle)) + return false; + + ref->nr =3D slot_end - ref->slot; + ref->src =3D src; + return true; + +bad_handle: + dev_WARN_ONCE(&dmb->vdev->dev, 1, + "device memory buffer handle %pad length %zu out of range\n", + &handle, size); + return false; +} + +/* + * Copy between the region and the pages a mapping bounces. @ref is what + * virtio_dmb_resolve() validated for this handle, or what the map side ha= s just + * recorded; every source address is derived from its snapshot rather than= from + * dmb->slots[], which this function does not read at all, so validation a= nd use + * cannot disagree about where the source is. Walk the source one page at= a + * time through kmap_local_page(): the source may be highmem, and the per-= slot + * record is a physical address precisely so that no assumption about a li= near + * kernel mapping across the mapping's pages is needed. The region side n= eeds + * no such split, being one contiguous mapping. + */ +static void virtio_dmb_copy(struct virtio_dmb *dmb, + const struct virtio_dmb_ref *ref, + dma_addr_t handle, size_t size, bool to_region) +{ + size_t off =3D (size_t)((u64)handle - dmb->base_off); + phys_addr_t base =3D ref->src - ((phys_addr_t)ref->slot << PAGE_SHIFT); + size_t done =3D 0; + + while (done < size) { + size_t pos =3D off + done; + phys_addr_t src =3D base + pos; + unsigned int in_src =3D offset_in_page(src); + void *region =3D dmb->base_va + pos; + size_t n; + void *va; + + n =3D min(size - done, (size_t)PAGE_SIZE - in_src); + + va =3D kmap_local_page(pfn_to_page(PHYS_PFN(src))); + if (to_region) + memcpy(region, va + in_src, n); + else + memcpy(va + in_src, region, n); + kunmap_local(va); + + done +=3D n; + } +} + +/* + * Record @nr slots from @slot as one allocation of @size bytes bouncing @= src. + * + * Each slot carries the source address of its own page rather than only t= he + * first slot carrying the address of the mapping. That is what makes + * virtio_dmb_copy() independent of which slot of the allocation a handle + * resolved to: taking the slot index back off the recorded address yields= the + * same base from any slot of the mapping. The sync ops do resolve to a s= lot + * in the middle of one, being handed handle + offset. + * + * The tail is recorded on every slot for the same reason, so that the byt= e end + * of the allocation is derivable from a mid-extent handle without also kn= owing + * which slot the allocation starts at. + */ +static void virtio_dmb_record(struct virtio_dmb *dmb, unsigned int slot, + unsigned int nr, size_t size, phys_addr_t src) +{ + unsigned int i; + u32 tail; + + /* + * @nr is virtio_dmb_slots(size) at both call sites, so the last slot + * carries between 1 and PAGE_SIZE bytes and the cast cannot truncate. + */ + tail =3D (u32)(size - ((size_t)(nr - 1) << PAGE_SHIFT)); + + for (i =3D 0; i < nr; i++) { + WRITE_ONCE(dmb->slots[slot + i].src, + src =3D=3D DMB_SRC_NONE ? + DMB_SRC_NONE : src + ((phys_addr_t)i << PAGE_SHIFT)); + WRITE_ONCE(dmb->slots[slot + i].end, slot + nr); + WRITE_ONCE(dmb->slots[slot + i].tail, tail); + } +} + +static void *virtio_dmb_op_alloc(union virtio_map map, size_t size, + dma_addr_t *map_handle, gfp_t gfp) +{ + struct virtio_dmb *dmb =3D map.dmb; + unsigned int nr, slot; + void *va; + long ret; + + /* + * The allocation-behaviour bits of gfp are ignored, because claiming + * slots neither sleeps nor allocates; __GFP_NOWARN is honoured, for + * the reason the failure path below gives. The result is zeroed + * because this stands in for dma_alloc_coherent(), whose callers rely + * on that. + */ + if (!size) + return NULL; + + /* + * Bound the request before a slot count is derived from it, as + * map_page() does. The bound is the whole pool rather than the + * fraction of it virtio_dmb_max_mapping() reports, for the reason + * that function gives. It is not the effective limit: every search + * is bounded at one pool area, so an allocation larger than + * area_slots pages fails even on an empty pool. + */ + if (size > virtio_dmb_pool_size(dmb)) + goto no_room; + + nr =3D virtio_dmb_slots(size); + ret =3D virtio_dmb_claim(dmb, nr); + if (ret < 0) + goto no_room; + slot =3D ret; + + va =3D dmb->base_va + ((size_t)slot << PAGE_SHIFT); + memset(va, 0, (size_t)nr << PAGE_SHIFT); + virtio_dmb_record(dmb, slot, nr, size, DMB_SRC_NONE); + + *map_handle =3D dmb->base_off + ((u64)slot << PAGE_SHIFT); + return va; + +no_room: + /* + * A buffer that does not fit is back-pressure and stays quiet, but + * this is a virtqueue area: no back-pressure can defer it, and a + * region sized for the buffers but not for the areas otherwise fails + * queue setup with nothing to tell it apart from every other reason + * find_vqs() can fail, and it is the one of those a larger region + * fixes. + * + * Which is why __GFP_NOWARN has to be honoured rather than ignored. + * vring_alloc_queue_split() walks the queue size down from the size + * the device asked for and marks every attempt but the last with the + * flag, so warning regardless would print a line for every attempt + * but the last, for a probe that then succeeds. Dynamic debug still car= ries the + * message, which is what a packed ring has to rely on: none of its + * three areas can be made smaller and all three set the flag. + */ + if (gfp & __GFP_NOWARN) + dev_dbg(&dmb->vdev->dev, + "no room for a %zu-byte virtqueue area in %u pages\n", + size, dmb->nslots); + else + dev_warn(&dmb->vdev->dev, + "no room for a %zu-byte virtqueue area in %u pages\n", + size, dmb->nslots); + return NULL; +} + +/* + * DMB_RESOLVE_WHOLE alone: there is deliberately no converse of + * DMB_RESOLVE_BOUNCED insisting that the allocation is a virtqueue area. = A + * caller reaching this with a mapping has called free() on something it g= ot + * from map_page(), and would lose the copy-out that unmap_page() does; the + * bytes it loses are its own, and the only in-tree caller of the exported + * virtqueue_map_free_coherent() is vring_free_queue(), which frees an are= a. + * Refusing here would trade that for a leak of the slots, which is worse. + */ +static void virtio_dmb_op_free(union virtio_map map, size_t size, void *va= ddr, + dma_addr_t map_handle, unsigned long attrs) +{ + struct virtio_dmb *dmb =3D map.dmb; + struct virtio_dmb_ref ref; + + if (!virtio_dmb_resolve(dmb, map_handle, size, DMB_RESOLVE_WHOLE, &ref)) + return; + + virtio_dmb_release(dmb, ref.slot, ref.nr); +} + +static dma_addr_t virtio_dmb_op_map_page(union virtio_map map, + struct page *page, + unsigned long offset, size_t size, + enum dma_data_direction dir, + unsigned long attrs) +{ + struct virtio_dmb *dmb =3D map.dmb; + phys_addr_t src =3D page_to_phys(page) + offset; + struct virtio_dmb_ref ref; + unsigned int nr, slot; + dma_addr_t handle; + long ret; + + if (!size || size > virtio_dmb_max_mapping(dmb)) + return DMA_MAPPING_ERROR; + + nr =3D virtio_dmb_slots(size); + ret =3D virtio_dmb_claim(dmb, nr); + if (ret < 0) + return DMA_MAPPING_ERROR; + slot =3D ret; + + virtio_dmb_record(dmb, slot, nr, size, src); + + handle =3D dmb->base_off + ((u64)slot << PAGE_SHIFT); + + ref.slot =3D slot; + ref.nr =3D nr; + ref.src =3D src; + + /* + * Copy the caller's buffer in whatever the direction is, and without + * honouring DMA_ATTR_SKIP_CPU_SYNC. swiotlb_tbl_map_single() bounces + * unconditionally for the same two reasons: a device that writes less + * than the whole buffer must leave the rest of the caller's bytes + * intact, and the mapped bytes must not reach the device as whatever + * the slot held before. + * + * Those bytes and no others. size need not be a multiple of + * PAGE_SIZE, and [size, nr << PAGE_SHIFT) keeps what the slots held + * before: a freed virtqueue area, an earlier mapping of this device, + * or what the device left there itself. Nothing outside a mapping's + * own length is ever copied in, so the device reads nothing there + * that the region did not already hold for it, and the descriptor + * carries a length. swiotlb leaves the remainder of its last slot + * the same way. + */ + virtio_dmb_copy(dmb, &ref, handle, size, true); + + return handle; +} + +static void virtio_dmb_op_unmap_page(union virtio_map map, + dma_addr_t map_handle, size_t size, + enum dma_data_direction dir, + unsigned long attrs) +{ + struct virtio_dmb *dmb =3D map.dmb; + struct virtio_dmb_ref ref; + + if (!virtio_dmb_resolve(dmb, map_handle, size, + DMB_RESOLVE_BOUNCED | DMB_RESOLVE_WHOLE, &ref)) + return; + + if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) && + (dir =3D=3D DMA_FROM_DEVICE || dir =3D=3D DMA_BIDIRECTIONAL)) + virtio_dmb_copy(dmb, &ref, map_handle, size, false); + + /* + * The slot count comes from what map_page() recorded rather than from + * the caller's size, so a mismatched size cannot release a different + * number of slots than were claimed. + */ + virtio_dmb_release(dmb, ref.slot, ref.nr); +} + +static void virtio_dmb_op_sync_single_for_cpu(union virtio_map map, + dma_addr_t map_handle, + size_t size, + enum dma_data_direction dir) +{ + struct virtio_dmb *dmb =3D map.dmb; + struct virtio_dmb_ref ref; + + /* A zero-length sync is a no-op and not a bad handle. */ + if (!size) + return; + + if (!virtio_dmb_resolve(dmb, map_handle, size, DMB_RESOLVE_BOUNCED, + &ref)) + return; + + if (dir =3D=3D DMA_FROM_DEVICE || dir =3D=3D DMA_BIDIRECTIONAL) + virtio_dmb_copy(dmb, &ref, map_handle, size, false); +} + +static void virtio_dmb_op_sync_single_for_device(union virtio_map map, + dma_addr_t map_handle, + size_t size, + enum dma_data_direction dir) +{ + struct virtio_dmb *dmb =3D map.dmb; + struct virtio_dmb_ref ref; + + /* A zero-length sync is a no-op and not a bad handle. */ + if (!size) + return; + + if (!virtio_dmb_resolve(dmb, map_handle, size, DMB_RESOLVE_BOUNCED, + &ref)) + return; + + if (dir =3D=3D DMA_TO_DEVICE || dir =3D=3D DMA_BIDIRECTIONAL) + virtio_dmb_copy(dmb, &ref, map_handle, size, true); +} + +static bool virtio_dmb_op_need_sync(union virtio_map map, dma_addr_t map_h= andle) +{ + /* Every mapping is a bounce, so every sync is a real copy. */ + return true; +} + +static int virtio_dmb_op_mapping_error(union virtio_map map, + dma_addr_t map_handle) +{ + /* + * DMA_MAPPING_ERROR is the value virtio_ring reserves. Offset 0 is + * reserved too, by the proposal and by every device implementation + * that reads a queue address of zero as a queue that was never + * programmed, so it is not an address this driver may publish either. + * Nothing allocated here yields it, because the pool starts after the + * first byte of the region for the reason virtio_dmb_init() gives, but + * a premapped buffer carries an address its caller obtained and + * vring_map_one_sg() asks this operation to judge that one. Whether + * such an address came from this map cannot be answered here, and a + * containment test would answer a different question, but zero can be + * answered: it is the one value the proposal rules out outright. + */ + if (map_handle =3D=3D DMA_MAPPING_ERROR || !map_handle) + return -ENOMEM; + + return 0; +} + +static size_t virtio_dmb_op_max_mapping_size(union virtio_map map) +{ + return virtio_dmb_max_mapping(map.dmb); +} + +static const struct virtio_map_ops virtio_dmb_map_ops =3D { + .map_page =3D virtio_dmb_op_map_page, + .unmap_page =3D virtio_dmb_op_unmap_page, + .sync_single_for_cpu =3D virtio_dmb_op_sync_single_for_cpu, + .sync_single_for_device =3D virtio_dmb_op_sync_single_for_device, + .alloc =3D virtio_dmb_op_alloc, + .free =3D virtio_dmb_op_free, + .need_sync =3D virtio_dmb_op_need_sync, + .mapping_error =3D virtio_dmb_op_mapping_error, + .max_mapping_size =3D virtio_dmb_op_max_mapping_size, +}; + +/* + * Whether the device still has virtqueues. vqs_list_lock is what protects + * that list against a concurrent adder. No caller here can race one, bec= ause + * every path that reaches this runs under the device lock and before or a= fter + * the driver's find_vqs(), but the invariant is worth enforcing rather th= an + * inheriting from callers this file does not control. + */ +static bool virtio_dmb_vqs_live(struct virtio_device *vdev) +{ + bool live; + + spin_lock(&vdev->vqs_list_lock); + live =3D !list_empty(&vdev->vqs); + spin_unlock(&vdev->vqs_list_lock); + + return live; +} + +/** + * virtio_dmb_destroy - release the Device Memory Buffer state of a device + * @vdev: the device + * + * Does nothing unless @vdev is currently using a Device Memory Buffer, and + * refuses if the device still has virtqueues: they would be left pointing + * into a region that is no longer mapped. The mapping and the physical + * region claim then stay behind until something deletes those virtqueues = and + * calls again, which unbinding the driver does: virtio_dev_remove() calls= the + * driver's remove() before this. + */ +void virtio_dmb_destroy(struct virtio_device *vdev) +{ + struct virtio_dmb *dmb; + + /* + * vdev->map identifies which member of vdev->vmap is live, so it is + * also the test for whether the union holds a Device Memory Buffer. + */ + if (vdev->map !=3D &virtio_dmb_map_ops) + return; + + /* + * A virtqueue keeps the mapping token it was created with, while + * vdev->map is consulted afresh on every dispatch. Clearing vdev->map + * therefore does not disarm a live virtqueue, it redirects that + * virtqueue's copy of the token into the DMA API, where the pointer + * this frees would be used as a struct device. Refuse instead and + * leak the mapping, which is unconditionally better than a + * use-after-free. + * + * Reported rather than warned about, because a driver that left its + * virtqueues in place is not the only way to get here. A device that + * fails to report its region on the way back from a suspend takes + * virtio_device_restore() to its error path, which calls this, and a + * driver with no freeze callback still has its virtqueues at that + * point, correctly. A condition a correct driver can satisfy must + * not taint the kernel. + */ + if (virtio_dmb_vqs_live(vdev)) { + dev_warn(&vdev->dev, + "device memory buffer not released, virtqueues are still live\n"); + return; + } + + dmb =3D vdev->vmap.dmb; + + /* Put back exactly what the transport had installed. */ + vdev->map =3D dmb->prev_map; + vdev->vmap =3D dmb->prev_vmap; + + memunmap(dmb->map_va); + if (dmb->map_claimed) + release_mem_region(dmb->map_phys, dmb->map_len); + kvfree(dmb->slots); + kfree(dmb->areas); + bitmap_free(dmb->bitmap); + kfree(dmb); +} +EXPORT_SYMBOL_GPL(virtio_dmb_destroy); + +/** + * virtio_dmb_init - make a device's Device Memory Buffer state current + * @vdev: the device, with feature negotiation complete + * + * Reads the shared memory id the device reports, locates the region, buil= ds + * an allocator over it and routes every mapping of the device through it. + * When the feature is not negotiated, releases any state a previous + * negotiation left behind. + * + * The operation is "make the state match what the device reports now", an= d it + * is reached again from resume and from reset completion. A device that + * reports the region it reported last time keeps the state it already has= , so + * handles held by a virtqueue that outlived the transition stay valid. A + * device that reports a different region has the state rebuilt when no + * virtqueue is live, and is refused otherwise: a virtqueue holds kernel + * addresses inside the mapping and cannot be redirected into a new one. + * + * A caller that gets an error must set the FAILED device status bit, and = must + * not touch the device for anything else before it does. The device has + * already confirmed the feature by the time this runs, so it is entitled = to + * assume the driver will address it through the region; the bit is what t= ells + * it the driver gave up instead. + * + * Return: 0 on success, or a negative errno. + */ +int virtio_dmb_init(struct virtio_device *vdev) +{ + struct virtio_shm_region region; + struct virtio_dmb *dmb; + unsigned int nslots, skew; + unsigned int area_slots, nareas, target, i; + u64 base_off, slots =3D 0; + size_t map_len; + u16 shm_id; + int err; + + if (!virtio_has_feature(vdev, VIRTIO_F_DMB)) { + /* + * The feature may have been withdrawn across re-negotiation. + * + * virtio_dmb_destroy() refuses under live virtqueues, and + * returning 0 after a refusal would leave this file's map + * operations installed for a device that has not negotiated the + * feature, so every later mapping would resolve a handle + * against a region the device no longer agrees it has. Report + * the refusal to the caller instead, which sets the FAILED + * device status bit. No path reaches this today: it needs map + * operations an earlier negotiation installed, which unbinding + * destroys, so only the restore path can find them, and that + * path hands finalize_features() the word already accepted + * rather than the offer, so a transport could drop the feature + * there only in reaction to a device that changed what it + * offers after the driver bound, and no reset does that. + */ + virtio_dmb_destroy(vdev); + if (vdev->map =3D=3D &virtio_dmb_map_ops) + return -EBUSY; + return 0; + } + + /* + * Without both of these the region cannot be located at all, which is + * what keeps a transport that does not implement them from offering + * the feature in the first place. + */ + if (!vdev->config->get_dmb_shm_id || !vdev->config->get_shm_region) { + dev_warn(&vdev->dev, + "transport cannot locate a device memory buffer\n"); + return -EINVAL; + } + + /* The feature is only defined together with VIRTIO_F_ACCESS_PLATFORM. */ + if (!virtio_has_feature(vdev, VIRTIO_F_ACCESS_PLATFORM)) { + dev_warn(&vdev->dev, + "device memory buffer without VIRTIO_F_ACCESS_PLATFORM\n"); + return -EINVAL; + } + + err =3D vdev->config->get_dmb_shm_id(vdev, &shm_id); + if (err) + return err; + + /* A region is looked up by a u8 id. */ + if (shm_id > U8_MAX) { + dev_warn(&vdev->dev, + "device memory buffer id %u out of range\n", shm_id); + return -EINVAL; + } + + if (!virtio_get_shm_region(vdev, ®ion, shm_id)) { + dev_warn(&vdev->dev, + "cannot locate device memory buffer region %u\n", + shm_id); + return -ENODEV; + } + + /* + * The region base carries no alignment guarantee, but every virtqueue + * layout requires one of the areas placed in it. Start the pool at a + * PAGE_SIZE-aligned address and record the skew, so that page-granular + * allocation makes every absolute address aligned. + * + * PAGE_SIZE - skew is the distance from the start of the region to the + * first aligned address strictly after it, so the pool never begins at + * the region's first byte and no handle is ever 0. The proposal + * reserves offset 0: it is not the address of any structure the driver + * places in the region, and a device may treat it as an error. A device + * that predates the reservation reads a queue address of 0 as the queue + * never having been programmed and ignores it, so the value is unusable + * either way. Keeping it out of the pool costs one page of an + * already-aligned region and nothing at all of a misaligned one, whose + * leading partial page was unusable regardless. + */ + skew =3D offset_in_page(region.addr); + base_off =3D PAGE_SIZE - skew; + + if (region.len > base_off) + slots =3D (region.len - base_off) >> PAGE_SHIFT; + + /* + * The least a region could hold: one minimally-sized virtqueue plus + * one buffer in flight against it. A packed queue costs three + * allocations, a descriptor ring and two event structures, and a split + * queue up to two when the transport aligns its areas to PAGE_SIZE, so + * four slots is the floor for either layout. + * + * Four slots is four pages of pool, which is five pages of region for + * a region whose base is already aligned, since the page the pool + * starts after is not part of it. + * + * That derivation counts one virtqueue. A device that also offers an + * administration virtqueue has its areas allocated from the same region + * through the same path, and one administration command occupies + * several slots more, so four pages is a floor such a device is + * misconfigured to sit on rather than a size it can work at. + * + * This floor is enforced, but it is a floor and not a sufficiency + * check. How much a device actually needs depends on how many + * virtqueues its driver creates and how deep they are, neither of + * which is known here: this runs during feature negotiation, before + * find_vqs(). A region above this floor but still too small fails + * there instead, which for a split ring reduces the queue depth and + * for a packed ring fails the queue. + * + * The upper bounds are what a slot index, a mapping length and a + * published handle can each represent. The mapping length bound is + * exclusive because the length mapped is base_off larger than the + * pool, and base_off is a whole page where the region base is + * aligned: at the last representable slot count that sum would wrap + * to zero on a 32-bit size_t. + */ + if (slots < 4 || slots > UINT_MAX || + slots >=3D (u64)(SIZE_MAX >> PAGE_SHIFT) || + base_off + (slots << PAGE_SHIFT) - 1 > + DMA_BIT_MASK(BITS_PER_TYPE(dma_addr_t))) { + dev_warn(&vdev->dev, + "device memory buffer region holds %llu usable pages\n", + slots); + return -EINVAL; + } + nslots =3D slots; + + /* Nothing outside the pool and the bytes ahead of it is used. */ + map_len =3D base_off + ((size_t)nslots << PAGE_SHIFT); + + /* + * The transport reports the region in 64 bits while a resource is + * addressed in resource_size_t. Refuse a region that does not fit + * rather than claim and map a truncated one. + */ + if (region.addr > (u64)(resource_size_t)-1 - map_len) { + dev_warn(&vdev->dev, + "device memory buffer region at 0x%llx is not addressable\n", + region.addr); + return -EINVAL; + } + + /* + * Everything that identifies the region is known now and nothing has + * been touched yet, so an unchanged region can be adopted instead of + * being torn down and rebuilt identically. That is what lets a + * virtqueue which outlived a suspend or a reset keep handles that are + * still valid, and it is why no separate freeze-time teardown is + * needed. A region that moved can be neither adopted nor replaced + * under live virtqueues, so refuse without a warning: a device that + * moves its region while its driver still has virtqueues is + * misbehaving, which is not evidence of a kernel bug. + */ + if (vdev->map =3D=3D &virtio_dmb_map_ops) { + dmb =3D vdev->vmap.dmb; + + if (dmb->shm_id =3D=3D shm_id && dmb->map_phys =3D=3D region.addr && + dmb->map_len =3D=3D map_len) + return 0; + + if (virtio_dmb_vqs_live(vdev)) + return -EBUSY; + + virtio_dmb_destroy(vdev); + } + + dmb =3D kzalloc(sizeof(*dmb), GFP_KERNEL); + if (!dmb) + return -ENOMEM; + + dmb->vdev =3D vdev; + dmb->shm_id =3D shm_id; + dmb->base_off =3D base_off; + dmb->map_phys =3D region.addr; + dmb->map_len =3D map_len; + + /* + * A shared memory region need not lie in a BAR the transport already + * claimed, so record a claim on the range here. The claim is + * advisory: where the region does lie in such a BAR the transport's + * own claim already covers it, and that is not a conflict with + * anything, so it must not fail the device. A foreign driver cannot + * own another device's BAR range either, so a refusal here is not + * evidence that anything is wrong. + */ + dmb->map_claimed =3D request_mem_region(region.addr, map_len, + "virtio-dmb") !=3D NULL; + if (!dmb->map_claimed) + dev_dbg(&vdev->dev, + "device memory buffer region at 0x%llx already reserved\n", + region.addr); + + /* + * MEMREMAP_DEC because the region is memory shared with the device, + * which is what the proposal requires of a driver wherever the platform + * distinguishes that from memory private to the driver. Without it + * x86's arch_memremap_wb() applies the guest's own encryption to the + * mapping, and the device would see ciphertext wherever a guest encrypts + * its memory. x86 is the only architecture that reads the flag; arm64 + * ignores it and reaches ioremap_prot() instead, where a realm guest's + * hook finds the region is not protected memory and shares the mapping. + * Anywhere else that draws the distinction, the proposal forbids the + * device from offering the feature at all. + */ + dmb->map_va =3D memremap(region.addr, map_len, + MEMREMAP_WB | MEMREMAP_DEC); + if (!dmb->map_va) { + /* + * memremap() is silent, and every sibling failure in this + * function names itself. Without this the device is left with + * the FAILED status bit set and nothing saying why. + */ + dev_warn(&vdev->dev, + "cannot map device memory buffer region at 0x%llx\n", + region.addr); + err =3D -ENOMEM; + goto err_unclaim; + } + + /* + * The skew was derived from the physical base, so the pool is aligned + * in the mapping only if the mapping kept that page offset. Check it + * rather than assume it. + */ + if (offset_in_page(dmb->map_va) !=3D skew) { + dev_warn(&vdev->dev, + "device memory buffer mapping is not page-congruent\n"); + err =3D -EINVAL; + goto err_unmap; + } + + dmb->base_va =3D dmb->map_va + (size_t)base_off; + + dmb->bitmap =3D bitmap_zalloc(nslots, GFP_KERNEL); + if (!dmb->bitmap) { + err =3D -ENOMEM; + goto err_unmap; + } + + /* A zeroed record has end =3D=3D 0, which is what marks a slot free. */ + dmb->slots =3D kvcalloc(nslots, sizeof(*dmb->slots), GFP_KERNEL); + if (!dmb->slots) { + err =3D -ENOMEM; + goto err_free_bitmap; + } + + /* + * Divide the pool into independently locked areas, so that mappings on + * different CPUs do not serialise on one lock and the interrupts-off + * window of one search does not grow with the region. This is the + * structure kernel/dma/swiotlb.c adopted in commit 20347fca71a3 + * ("swiotlb: split up the global swiotlb lock"), for the same reason. + * + * area_slots is the power of two and nareas is derived from it, which + * is the reverse of swiotlb. swiotlb indexes areas with a mask and + * rounds its pool size up to suit; a region's length is the device's + * and cannot be rounded up, and dividing a power-of-two area count into + * it would leave area_slots neither a power of two nor a multiple of + * BITS_PER_LONG. That matters for correctness rather than for tuning: + * bitmap_set() and bitmap_clear() are non-atomic read-modify-write on + * an unsigned long, so two areas sharing a bitmap word under separate + * locks would lose updates. A power-of-two area_slots at least + * BITS_PER_LONG makes every area own whole words, and makes the area of + * a slot a shift. The cost is one division per claim where swiotlb has + * a mask, which is a fraction of the two copies every mapping already + * performs. + * + * num_possible_cpus() and not num_online_cpus(), so that the division + * is sized for the CPUs that can run rather than for the ones running + * when the region is installed, which on a guest that onlines the rest + * later would divide the pool for one. Nothing is allocated per CPU + * and no area belongs to one, so a CPU going away strands no capacity + * and there is no hotplug callback. A pool too small to divide that + * far yields fewer areas than CPUs, which then share. + */ + BUILD_BUG_ON(DMB_AREA_MIN_SLOTS > DMB_AREA_MAX_SLOTS); + BUILD_BUG_ON(DMB_AREA_MIN_SLOTS < BITS_PER_LONG); + BUILD_BUG_ON(!is_power_of_2(DMB_AREA_MIN_SLOTS)); + + target =3D nslots / roundup_pow_of_two(num_possible_cpus()); + if (target < DMB_AREA_MIN_SLOTS) + /* rounddown_pow_of_two(0) is undefined. */ + area_slots =3D DMB_AREA_MIN_SLOTS; + else + area_slots =3D clamp_t(unsigned int, + rounddown_pow_of_two(target), + DMB_AREA_MIN_SLOTS, DMB_AREA_MAX_SLOTS); + + nareas =3D DIV_ROUND_UP(nslots, area_slots); + + dmb->areas =3D kcalloc(nareas, sizeof(*dmb->areas), GFP_KERNEL); + if (!dmb->areas) { + err =3D -ENOMEM; + goto err_free_slots; + } + + for (i =3D 0; i < nareas; i++) + spin_lock_init(&dmb->areas[i].lock); + + dmb->nslots =3D nslots; + dmb->nareas =3D nareas; + dmb->area_slots =3D area_slots; + dmb->area_shift =3D ilog2(area_slots); + + /* Published last: until now nothing routes a mapping here. */ + dmb->prev_map =3D vdev->map; + dmb->prev_vmap =3D vdev->vmap; + vdev->vmap.dmb =3D dmb; + vdev->map =3D &virtio_dmb_map_ops; + + /* + * The feature moves every virtqueue of this device into a region and + * changes what every address published to it means, and it activates + * from a value the device supplies that nothing else records. Report + * the three facts about it that are recoverable nowhere else, on the + * device that negotiated it, and the derived area geometry with them so + * that it is visible without debugfs. + */ + dev_info(&vdev->dev, + "device memory buffer %u at %pa, %u usable pages in %u areas of %u page= s\n", + shm_id, &dmb->map_phys, nslots, dmb->nareas, + dmb->area_slots); + + return 0; + +err_free_slots: + kvfree(dmb->slots); +err_free_bitmap: + bitmap_free(dmb->bitmap); +err_unmap: + memunmap(dmb->map_va); +err_unclaim: + if (dmb->map_claimed) + release_mem_region(dmb->map_phys, dmb->map_len); + kfree(dmb); + return err; +} +EXPORT_SYMBOL_GPL(virtio_dmb_init); + +MODULE_DESCRIPTION("Virtio device memory buffer allocator"); +MODULE_LICENSE("GPL"); diff --git a/drivers/virtio/virtio_dmb.h b/drivers/virtio/virtio_dmb.h new file mode 100644 index 000000000000..69fbcbb9c2c0 --- /dev/null +++ b/drivers/virtio/virtio_dmb.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Device Memory Buffer support for virtio devices. + */ +#ifndef _DRIVERS_VIRTIO_VIRTIO_DMB_H +#define _DRIVERS_VIRTIO_VIRTIO_DMB_H + +struct virtio_device; + +#if IS_ENABLED(CONFIG_VIRTIO_DMB) + +int virtio_dmb_init(struct virtio_device *vdev); +void virtio_dmb_destroy(struct virtio_device *vdev); + +#else + +static inline int virtio_dmb_init(struct virtio_device *vdev) +{ + return 0; +} + +static inline void virtio_dmb_destroy(struct virtio_device *vdev) +{ +} + +#endif /* CONFIG_VIRTIO_DMB */ + +#endif /* _DRIVERS_VIRTIO_VIRTIO_DMB_H */ diff --git a/include/linux/virtio.h b/include/linux/virtio.h index 93e573c56563..cd3fe82ff328 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -44,12 +44,15 @@ struct virtqueue { }; =20 struct vduse_vq_group; +struct virtio_dmb; =20 union virtio_map { /* Device that performs DMA */ struct device *dma_dev; /* VDUSE specific virtqueue group for doing map */ struct vduse_vq_group *group; + /* Device Memory Buffer holding the virtqueues and their buffers */ + struct virtio_dmb *dmb; }; =20 int virtqueue_add_outbuf(struct virtqueue *vq, diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index 69f84ea85d71..a6780aa85966 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -96,6 +96,13 @@ struct virtqueue_info { * @set_vq_affinity: set the affinity for a virtqueue (optional). * @get_vq_affinity: get the affinity for a virtqueue (optional). * @get_shm_region: get a shared memory region based on the index. + * @get_dmb_shm_id: get the shared memory id of the Device Memory Buffer + * (optional). + * vdev: the device + * id: where to store the shared memory id + * Returns 0 on success or error status + * Only valid once VIRTIO_F_DMB has been negotiated. A transport that + * does not implement this must not accept VIRTIO_F_DMB. * @disable_vq_and_reset: reset a queue individually (optional). * vq: the virtqueue * Returns 0 on success or error status @@ -135,6 +142,7 @@ struct virtio_config_ops { int index); bool (*get_shm_region)(struct virtio_device *vdev, struct virtio_shm_region *region, u8 id); + int (*get_dmb_shm_id)(struct virtio_device *vdev, u16 *id); int (*disable_vq_and_reset)(struct virtqueue *vq); int (*enable_vq_after_reset)(struct virtqueue *vq); }; From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-002.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-002.esa.us-west-2.outbound.mail-perimeter.amazon.com [44.246.1.125]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3B66436F91F for ; Sun, 9 Aug 2026 18:21:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=44.246.1.125 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299675; cv=none; b=jAEPIYC6J3G21XMfc3o1x8oCi9OQsnkdWN9wp+WyZqVQmwbpVJaNup1CEA2Hd9hBT+MybJNgoyqeiXgm6u8WVq8OTTMYeZt2VkglM4BUElhcxmMpTG1h2CBEE0ZvjveIoVwbGOyewt+7BYaZn7Gt2Bkd81eKNd806M+wBAdxaok= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299675; c=relaxed/simple; bh=DwdVTtlzCwZuXZ5o0jAjWpgCQVfr1NQNncmvuqraSiQ=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=JTW6MBh8k6CWdV5qcadGh1U1udkQ86e1BmLllxMxDVX+q3mhv4s+403K+s9/FkmYL9uSEsPnd0JIKObVKlFWwt8712QbUGhwbk6w5M4kjEDtk1ZCRFVFIwxnyWMLCy9XDFmCpYG5vY3Gind42BdE1CNEbcz3NNKZveE5mtL6rZA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=oWkF6xv5; arc=none smtp.client-ip=44.246.1.125 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="oWkF6xv5" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299674; x=1817835674; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=bpQw20GqOuyWGdONApxpPST+oYpdx2tqLi8GETl/AjY=; b=oWkF6xv5QS5IlrZcINdW9anrezFMTLfUOlr7qCF23MIakjsNixuBZ5ZR s00rNB0jswCSxBLEMzB0rMWqCDOGyaajBa0Mua0f7nZ2fWcZdIEVE+9it yX5GGGr3xjY8QP6bIEXZTO5INMCkf8zeFK0yJO+akiYb2wB0oInwgJEJ2 mYqUbWlVSeo7CnhxECbWziSyyO1+tkrgXgchjPhdKQbXKDcl8whT9yd8q QOkHrRl5ON82eVG5hT+gAaihqi7uM5p0qdY0uQt/R+tisgzXZD9EAetnH 0k3zOP0/sblSrRZ3K1JuwGxMX8F4++b0H+cJTl3GevDqCuWruJqgKxt8v w==; X-CSE-ConnectionGUID: BEW7+jOIS+ifnG2NDroeVg== X-CSE-MsgGUID: fWZ+dlnpQsyaFJ8h2NSJHQ== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25521004" Received: from ip-10-5-0-115.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.0.115]) by internal-pdx-out-002.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:21:11 +0000 Received: from EX19MTAUWA002.ant.amazon.com [205.251.233.234:28835] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.5.32:2525] with esmtp (Farcaster) id 6cd55a81-4f58-4ea8-a5da-8c84cfc45e04; Sun, 9 Aug 2026 18:21:11 +0000 (UTC) X-Farcaster-Flow-ID: 6cd55a81-4f58-4ea8-a5da-8c84cfc45e04 Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWA002.ant.amazon.com (10.250.64.202) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:11 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:09 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , , , , Stefan Hajnoczi , Paolo Bonzini Subject: [RFC PATCH 07/12] virtio: locate the device memory buffer after feature negotiation Date: Sun, 9 Aug 2026 18:20:05 +0000 Message-ID: <20260809182010.32931-8-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D035UWB004.ant.amazon.com (10.13.138.104) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" In preparation to support VIRTIO_F_DMB, locate a device's Device Memory Buffer once feature negotiation is complete. The shared memory id that locates the region may only be read after the device has confirmed FEATURES_OK, and finalize_features() runs before that. virtio_features_ok() is the one place in the core that has just read FEATURES_OK back, so locate the region from there, and release it from virtio_dev_remove() and from the error paths of probe and restore. Restore and reset completion reach it too, where virtio_dmb_init() keeps the state of a device reporting the region it had. Suspend and reset need no handling of their own. Neither virtio_device_freeze() nor virtio_reset_device() deletes a virtqueue, and a live virtqueue holds addresses inside the region, so we keep it across both. Link: https://lore.kernel.org/virtio-comment/20260804161202.38619-1-graf@am= azon.com/ Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- drivers/virtio/virtio.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index 299fa83be1d5..6f112593566c 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -8,6 +8,8 @@ #include #include =20 +#include "virtio_dmb.h" + /* Unique numbering for virtio devices. */ static DEFINE_IDA(virtio_index_ida); =20 @@ -231,7 +233,14 @@ static int virtio_features_ok(struct virtio_device *de= v) status); return -ENODEV; } - return 0; + + /* + * Negotiation is complete, so a Device Memory Buffer may now be + * located. Reached from probe, from resume and from reset + * completion, all of which have to end with the state matching what + * the device reports now. + */ + return virtio_dmb_init(dev); } =20 /** @@ -361,6 +370,7 @@ static int virtio_dev_probe(struct device *_d) =20 err: virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED); + virtio_dmb_destroy(dev); return err; =20 } @@ -377,6 +387,8 @@ static void virtio_dev_remove(struct device *_d) /* Driver should have reset device. */ WARN_ON_ONCE(dev->config->get_status(dev)); =20 + virtio_dmb_destroy(dev); + /* Acknowledge the device's existence again. */ virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE); =20 @@ -650,6 +662,7 @@ static int virtio_device_restore_priv(struct virtio_dev= ice *dev, bool restore) =20 err: virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED); + virtio_dmb_destroy(dev); return ret; } From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-005.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-005.esa.us-west-2.outbound.mail-perimeter.amazon.com [52.13.214.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DD12B37204E for ; Sun, 9 Aug 2026 18:21:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=52.13.214.179 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299678; cv=none; b=d53ag/PiMe6f/0TyEYvVAdthtxo+VTTGQJ3sIF2uNoOjFMgabsr5z25ezb3wUIy4F/q9PNtZPv0qyJ9WdQywTRT8acBooz4WQ7OHYC5VAVWVRr1TfiEgpxiSELcg6wsmBl3HW44UDW+a9MuQXCfFmC7RD+J15D4arHsdrPazIQg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299678; c=relaxed/simple; bh=M9pTWXVyco0cyJDI0/8rpL6FKpSOxS9abIbnWnICVuY=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=BKEz68C/ve1zrCDPFhLgx+GQ8ElhmhOCg0r8Gbw+ubv7WwmCPVTjwN3h6scNn1BKwtccexIDwvPpWHxBpGxb5hMcTD3uR3ZqfQg8SeQkUIgeQwPnyoh1cpJMVuF2IvKHqxiG5+vrUOhEhkJmigAN29yVO+/cmR5IWO32o84rKKw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=txafXlwk; arc=none smtp.client-ip=52.13.214.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="txafXlwk" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299675; x=1817835675; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=TZ+BlLRrqjruaH4UmOKdQsXVbVROnHD2B9q6LIv1REc=; b=txafXlwki9k/S9lxV6Pqanm2plrEepDvAUcfSFAR8EEgv10JeAUzTSFp 00pkaTAe0M21SyHge2ktyhOlA2PRc+BW4cYriUnsld4DhwtqOjU/py9qt 9giq6owdHlB4x6IaP28lqfxTHzv49GhtDTBPrZBhMKVQrPIp3+t799ezx 5Q7x6iJZ9aDakf815sQHeW4bVodAEmISQoND9iXTRd0lsCuoyEXK5TMNJ 0eUz2ht59eI2ZYVErewa/FTEtulgDrdNXQ0KDsBAtpxMaR5QhQqU+mA3+ JqmK//pM8Yzq17SqRlS4GolRTLQmqJB7/2OPs5JsNm/JkMk+7EKxlab/3 g==; X-CSE-ConnectionGUID: TnbV6CP9TsulSH/X73M7EA== X-CSE-MsgGUID: 5SEToIxtSBiMT0U9Xrkxjg== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25498741" Received: from ip-10-5-12-219.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.12.219]) by internal-pdx-out-005.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:21:13 +0000 Received: from EX19MTAUWB001.ant.amazon.com [205.251.233.51:5771] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.37.134:2525] with esmtp (Farcaster) id 54e962e6-146d-4c73-80ea-8517520215c4; Sun, 9 Aug 2026 18:21:13 +0000 (UTC) X-Farcaster-Flow-ID: 54e962e6-146d-4c73-80ea-8517520215c4 Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWB001.ant.amazon.com (10.250.64.248) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:13 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:11 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , , , , Stefan Hajnoczi , Paolo Bonzini Subject: [RFC PATCH 08/12] virtio_pci: support VIRTIO_F_DMB Date: Sun, 9 Aug 2026 18:20:06 +0000 Message-ID: <20260809182010.32931-9-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D035UWB004.ant.amazon.com (10.13.138.104) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" Let a modern virtio-pci device place its virtqueues and the buffers they reference in a Device Memory Buffer of its own: accept VIRTIO_F_DMB from vp_transport_features(), and implement the get_dmb_shm_id config op on top of vp_modern_get_dmb_shm_id(). Refuse a device whose common configuration is too short to hold dmb_shm_id, which would put that read outside what vp_modern_probe() mapped. The accept commits before the id can be read, so a region we fail to locate afterwards fails virtio_features_ok() and probe sets the FAILED status bit. Four conditions gate the accept: 1) VIRTIO_F_ACCESS_PLATFORM, because the feature is only defined together with it. 2) VIRTIO_F_ORDER_PLATFORM where the device offers it. Without it the ring emits the weaker barriers that assume the device sees memory the way another CPU does, and a region that is not ordinary host memory breaks that assumption. 3) CONFIG_VIRTIO_DMB, so a device offering the feature to a kernel built without it is driven as an ordinary device. 4) VIRTIO_F_VERSION_1, because virtio_features_ok() returns early without it, which would leave the feature negotiated and the region never built. This transport refuses such a device anyway. vp_dmb_ordering_ok() asks the device with vp_modern_get_features() instead of reading the feature word vp_transport_features() is handed. That word holds what the driver accepts, so a device offering VIRTIO_F_ORDER_PLATFORM to a driver that declined it would read there as a device that never offered it. Link: https://lore.kernel.org/virtio-comment/20260804161202.38619-1-graf@am= azon.com/ Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- drivers/virtio/virtio_pci_modern.c | 65 ++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci= _modern.c index 565d37b630b3..c43c1fc6e843 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -364,6 +364,36 @@ static void vp_modern_avq_cleanup(struct virtio_device= *vdev) } } =20 +/* + * The proposal makes accepting VIRTIO_F_DMB conditional on accepting + * VIRTIO_F_ORDER_PLATFORM where the device offers it. Without it the bar= riers + * the ring emits order accesses only as seen by a device that can be assu= med + * to run on identical CPUs in an SMP configuration, which a device whose + * region is not ordinary host memory is not. Accepting + * VIRTIO_F_ORDER_PLATFORM is otherwise only a SHOULD, so nothing else cou= ples + * the two. + * + * The offer is read back from the device rather than taken from the featu= re + * word vp_transport_features() is given, because that word is what the dr= iver + * still wants rather than what the device offered. The two differ exactly + * where this has to hold: virtio_dev_probe() calls finalize_features() a + * second time when a driver's validate() changed the set, and a validate() + * that declined VIRTIO_F_ORDER_PLATFORM leaves the bit absent from the wo= rd + * as well, which would read as an offer that never happened. virtio_ball= oon + * declines VIRTIO_F_ACCESS_PLATFORM from validate() today, so the shape is + * not hypothetical. + */ +static bool vp_dmb_ordering_ok(struct virtio_device *vdev) +{ + struct virtio_pci_device *vp_dev =3D to_vp_device(vdev); + + if (__virtio_test_bit(vdev, VIRTIO_F_ORDER_PLATFORM)) + return true; + + return !(vp_modern_get_features(&vp_dev->mdev) & + BIT_ULL(VIRTIO_F_ORDER_PLATFORM)); +} + static void vp_transport_features(struct virtio_device *vdev, u64 features) { struct virtio_pci_device *vp_dev =3D to_vp_device(vdev); @@ -378,6 +408,27 @@ static void vp_transport_features(struct virtio_device= *vdev, u64 features) =20 if (features & BIT_ULL(VIRTIO_F_ADMIN_VQ)) __virtio_set_bit(vdev, VIRTIO_F_ADMIN_VQ); + + /* + * VIRTIO_F_DMB is only defined together with + * VIRTIO_F_ACCESS_PLATFORM, so accept it only when the driver accepts + * that too. vring_transport_features() has already run, so the bit in + * vdev is the one the driver accepts rather than the one the device + * offered, and the proposal words the requirement against what the + * driver accepts. VIRTIO_F_ORDER_PLATFORM is required where the device + * offers it, for the reason vp_dmb_ordering_ok() gives. + * VIRTIO_F_VERSION_1 is required because the core locates and releases + * the region from virtio_features_ok(), which returns before it gets + * that far for a device without VERSION_1, so accepting the feature + * without it would leave the feature negotiated and the region never + * built. + */ + if (IS_ENABLED(CONFIG_VIRTIO_DMB) && + (features & BIT_ULL(VIRTIO_F_DMB)) && + __virtio_test_bit(vdev, VIRTIO_F_ACCESS_PLATFORM) && + (features & BIT_ULL(VIRTIO_F_VERSION_1)) && + vp_dmb_ordering_ok(vdev)) + __virtio_set_bit(vdev, VIRTIO_F_DMB); } =20 static int __vp_check_common_size_one_feature(struct virtio_device *vdev, = u32 fbit, @@ -413,6 +464,9 @@ static int vp_check_common_size(struct virtio_device *v= dev) if (vp_check_common_size_one_feature(vdev, VIRTIO_F_ADMIN_VQ, admin_queue= _num)) return -EINVAL; =20 + if (vp_check_common_size_one_feature(vdev, VIRTIO_F_DMB, dmb_shm_id)) + return -EINVAL; + return 0; } =20 @@ -878,6 +932,15 @@ static bool vp_get_shm_region(struct virtio_device *vd= ev, return true; } =20 +static int vp_get_dmb_shm_id(struct virtio_device *vdev, u16 *id) +{ + struct virtio_pci_device *vp_dev =3D to_vp_device(vdev); + + *id =3D vp_modern_get_dmb_shm_id(&vp_dev->mdev); + + return 0; +} + /* * virtio_pci_admin_has_dev_parts - Checks whether the device parts * functionality is supported @@ -1241,6 +1304,7 @@ static const struct virtio_config_ops virtio_pci_conf= ig_nodev_ops =3D { .set_vq_affinity =3D vp_set_vq_affinity, .get_vq_affinity =3D vp_get_vq_affinity, .get_shm_region =3D vp_get_shm_region, + .get_dmb_shm_id =3D vp_get_dmb_shm_id, .disable_vq_and_reset =3D vp_modern_disable_vq_and_reset, .enable_vq_after_reset =3D vp_modern_enable_vq_after_reset, }; @@ -1261,6 +1325,7 @@ static const struct virtio_config_ops virtio_pci_conf= ig_ops =3D { .set_vq_affinity =3D vp_set_vq_affinity, .get_vq_affinity =3D vp_get_vq_affinity, .get_shm_region =3D vp_get_shm_region, + .get_dmb_shm_id =3D vp_get_dmb_shm_id, .disable_vq_and_reset =3D vp_modern_disable_vq_and_reset, .enable_vq_after_reset =3D vp_modern_enable_vq_after_reset, }; From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-012.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-012.esa.us-west-2.outbound.mail-perimeter.amazon.com [35.162.73.231]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1CFB8366074; Sun, 9 Aug 2026 18:21:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=35.162.73.231 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299700; cv=none; b=TmXhkV0BFomm1hvFBfc6jX9ApNLzm02yZ96aR3CTJ4Tf3Za4CihVyKj1Qt9aM5HQO9xh0wRmrpx7G+8R0N+QzmQvcm0i43B+9UeyDbSZBDkbQzdNI6PWdj8NyfjI0Rj8MXejratiCZEV5nvUGPJW6wBKYAu2SXKvjD3xItDMcF8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299700; c=relaxed/simple; bh=n7fs/p5czBunTdAK26sqhtSC0lGNR3zWUGF6I7lJt0w=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=svWTL06p9r1vaszzWQCDRUAJ5eK3Z7iOQ6xqOkNh+qvQnq5ggVkYa+039DrFZUL6t1RLXk5j2cb3uTYW4LuHRLhN6nCP5M9HYdaQgL/rNlFzZWf4rEfdSH0bxoiOkvYsZSK30zNPxmslwxNnBRCiPiMnf0xGDC+oXMynfsg8XiM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=PkgwfJqE; arc=none smtp.client-ip=35.162.73.231 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="PkgwfJqE" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299698; x=1817835698; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=JxvA8OD0SoqRG0nlP6CghfuVBYrE9PQ1h06QIsNoT1A=; b=PkgwfJqEShVbheX4t9IwzIS/uUGYtqtFJ3zbNhhh6vVPsgzX5yFFqS2W BWntKCH+j4+oMOyvwzGu1WAO98DsFWNQn6hUu5l66AKI04Xg/GPy3JeGy gfipl6ND9i00e5vUsVUfhHcV3o3lKd1FY2etpkMcFv2hpajM7AXdYVXJC L6S7Q8sEGoC9Df8UjiyYPnxkuuhXbJUtlqJL3rZ5OvzyhSchrUv8PlIdM As0vEgSkI4fmhfTxjBlGDlrbBaZFqiD3rtBH58c+qcb+NoKg/hS1uA9De glu+aXjsmq0FsswzTPBwYL+Cv1cNaPem7R8XE3THIgBt3Zr+z9v+RZpzY g==; X-CSE-ConnectionGUID: eUCnMNFcRG+mU0otmNMZeg== X-CSE-MsgGUID: VcFi4MRRRhavN9EDHJOujg== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25319039" Received: from ip-10-5-6-203.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.6.203]) by internal-pdx-out-012.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:21:37 +0000 Received: from EX19MTAUWC002.ant.amazon.com [205.251.233.51:5262] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.60.103:2525] with esmtp (Farcaster) id 91f52dfb-26ce-4d39-95cd-a3256dd97d9b; Sun, 9 Aug 2026 18:21:37 +0000 (UTC) X-Farcaster-Flow-ID: 91f52dfb-26ce-4d39-95cd-a3256dd97d9b Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWC002.ant.amazon.com (10.250.64.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:37 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:34 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , Jonathan Corbet , Shuah Khan , , , , , Stefan Hajnoczi , "Paolo Bonzini" Subject: [RFC PATCH 09/12] Documentation: virtio: describe the device memory buffer Date: Sun, 9 Aug 2026 18:20:07 +0000 Message-ID: <20260809182010.32931-10-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D042UWA002.ant.amazon.com (10.13.139.17) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" A Device Memory Buffer device hands the driver a dma_addr_t that is an offset into the region and not an address the DMA API accepts. The next person to write a virtio_map_ops implementation has nowhere in the tree to read that, and the sizing rules a device has to follow live only in the code that enforces them. Document both, along with what the driver builds over the region and what it assumes about coherency. Link: https://lore.kernel.org/virtio-comment/20260804161202.38619-1-graf@am= azon.com/ Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- Documentation/driver-api/virtio/index.rst | 1 + .../driver-api/virtio/virtio-dmb.rst | 614 ++++++++++++++++++ drivers/virtio/Kconfig | 1 + 3 files changed, 616 insertions(+) create mode 100644 Documentation/driver-api/virtio/virtio-dmb.rst diff --git a/Documentation/driver-api/virtio/index.rst b/Documentation/driv= er-api/virtio/index.rst index 528b14b291e3..8c885d189991 100644 --- a/Documentation/driver-api/virtio/index.rst +++ b/Documentation/driver-api/virtio/index.rst @@ -9,3 +9,4 @@ Virtio =20 virtio writing_virtio_drivers + virtio-dmb diff --git a/Documentation/driver-api/virtio/virtio-dmb.rst b/Documentation= /driver-api/virtio/virtio-dmb.rst new file mode 100644 index 000000000000..5975948165c9 --- /dev/null +++ b/Documentation/driver-api/virtio/virtio-dmb.rst @@ -0,0 +1,614 @@ +.. SPDX-License-Identifier: GPL-2.0 + +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D +Device Memory Buffer +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +A device that offers ``VIRTIO_F_DMB`` presents one shared memory region, +the Device Memory Buffer, that it owns. When the feature is negotiated +the region holds that device's virtqueues and the buffers its descriptors +reference, and every address the driver places in a virtqueue is an +offset from the start of the region rather than a physical or a bus +address. + +Because each device has its own region, devices do not contend for one +shared bounce buffer, and the implementation of one device can be torn down +without touching the memory another device is using. A consequence is +confinement of the virtqueue data path: to process virtqueues the device +can only reach memory the driver has published in the region. It says +nothing about other shared memory regions the device exposes, or about +DMA the device performs outside a virtqueue. + +.. note:: + + ``VIRTIO_F_DMB`` is a specification proposal before the virtio + Technical Committee, not a ratified feature: + + https://lore.kernel.org/virtio-comment/20260804161202.38619-1-graf@am= azon.com/ + + The feature bit number and the PCI register the driver reads the + ``shmid`` from are both provisional, pending allocation by that + committee, and the definitions of both in the uapi headers say so. A + device implementing this must expect either to change. + +What lives in the region +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +For every virtqueue of the device: its Descriptor Area, Driver Area and +Device Area. For every buffer made available to the device: the buffer +itself. With ``VIRTIO_F_INDIRECT_DESC`` also negotiated: every indirect +descriptor table, and the buffers the descriptors in it name. + +Not in the region, and not changed by this feature: available and used +buffer notifications, configuration change notifications, the +configuration space, and everything else the transport conveys. The +memory ordering requirements of the three virtqueue areas are unchanged. +What did change is that the driver accepts ``VIRTIO_F_DMB`` only together +with ``VIRTIO_F_ORDER_PLATFORM`` where the device offers it, because the +areas are now in memory the device supplies rather than memory the driver +allocated. + +What the device must guarantee +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D + +* It must not offer ``VIRTIO_F_DMB`` unless the transport defines both a + shared memory region discovery mechanism and a way to report which + region is the Device Memory Buffer. It must also offer + ``VIRTIO_F_ACCESS_PLATFORM``. Of the transports Linux implements only + PCI defines the second, and only PCI accepts the feature bit at all: on + every other transport it is cleared during transport feature negotiation + and never reaches the device's driver. + +* It must not offer ``VIRTIO_F_DMB`` unless the platform permits the + driver to access the region as memory shared with the device, and unless + a write by either side becomes visible to the other without any cache + maintenance by the driver. It must offer ``VIRTIO_F_ORDER_PLATFORM`` if + it needs the driver to use barriers suitable for a device described by + the platform, and the driver then accepts ``VIRTIO_F_DMB`` only together + with it. + +* It must expose exactly one shared memory region for the Device Memory + Buffer, and report that region's ``shmid`` through the + transport-specific mechanism. On the PCI transport that is the + ``dmb_shm_id`` field of the common configuration structure, whose value + is confined to 8 bits because a ``virtio_pci_cap`` id is 8 bits wide, + and a device offering the feature must present a common configuration + structure long enough to contain the field. + +* It must not report a ``dmb_shm_id`` equal to any ``shmid`` its device + type defines. The driver looks up only the region the device points it + at, but the device-type driver bound to the same device looks up the ids + its own specification reserves, so a collision has the two look up the + same region. The Device Memory Buffer claims the range first, because it + is installed during feature negotiation and before the device-type driver + probes, so a virtio-fs device reporting ``dmb_shm_id`` 0 fails to probe + rather than sharing the region, and the failure names neither the feature + nor the collision. + +* It must interpret every address the driver supplies in a virtqueue as + an offset from the start of the region, and must not interpret any of + them as a physical or bus address. That covers the three area + addresses, the ``addr`` field of every descriptor, the address of every + indirect descriptor table, and the ``addr`` field of every descriptor + inside one. + +* Before accessing anything a driver-supplied offset refers to, it must + check that the offset together with the length of that structure lies + within the region and does not overflow. If the check fails it must + not perform the access, and must set the ``DEVICE_NEEDS_RESET`` device + status bit. + +* It must not access memory outside the region in order to process + virtqueues. + +* A device that cannot function at all without a Device Memory Buffer + must refuse ``FEATURES_OK`` when the driver has not accepted + ``VIRTIO_F_DMB``. Nothing else can catch that case: a driver that did + not accept the feature proceeds with ordinary host-memory addresses, + which such a device cannot reach, and it has no way to know that. + +Exposing other shared memory regions alongside the Device Memory Buffer +is allowed; the driver looks up only the reported ``shmid`` and ignores +every other region. + +The proposal does not state two of the obligations above: the +``dmb_shm_id`` collision rule, and the ``FEATURES_OK`` refusal by a +device that cannot work without a region. Both follow from what the +driver does rather than from the standard, and the driver cannot check +either, so treat them as requirements anyway. + +Coherency, and what this driver assumes +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +The driver maps the region as ordinary cacheable memory shared with the +device and uses plain loads and stores on it, including for the virtqueue +areas themselves. It asks for a mapping that does not apply the guest's +own memory encryption, because the device has to be able to read what the +driver writes there; that is what lets a guest whose memory is encrypted +use the feature at all. + +Plain loads and stores are only correct if the region is coherent with +CPU caches as seen by the device, which is satisfied by a device whose +region is host memory. A device whose region is not host memory must +additionally offer ``VIRTIO_F_ORDER_PLATFORM``: without it the ring's +barriers are the inner-shareable ones, which do not order accesses as +seen by a device outside that domain, and the available ring and the +notification would be reordered silently. + +The driver accepts ``VIRTIO_F_DMB`` only if it also accepts +``VIRTIO_F_ACCESS_PLATFORM``, and, where the device offers +``VIRTIO_F_ORDER_PLATFORM``, only if it accepts that as well. What it +cannot do is refuse the feature when ``VIRTIO_F_ORDER_PLATFORM`` is simply +absent from the offer: which barriers are needed depends on where the +region's memory is, the transport reports a ``shmid``, a base and a length +and nothing about the memory behind them, and refusing unconditionally +would reject the emulated device whose region is ordinary host memory and +for which the inner-shareable barriers are correct. So a device whose +region needs the platform's barriers has to say so by offering the +feature, and one that does not misbehaves in a way that looks like a ring +bug. + +The proposal states both requirements, and states the coupling the driver +implements. + +A device whose region is not coherent with CPU caches at all is out of +scope here. Supporting one would need two things that do not exist: a +way for the device to declare the memory type of its region, and explicit +flush points in the virtqueue code. Mapping the region write-combining +instead is not a substitute, because on some architectures draining a +write-combining buffer needs an explicit barrier that virtio's ring +barriers do not issue, which would break available-ring and notification +ordering silently. + +Sizing the region +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +The region has to hold everything at once: every virtqueue's virtqueue +areas, and every buffer in flight. Allocation is page-granular, so a +device sizing its region counts pages and not bytes. + +The pool does not start at the start of the region. The driver places it +at the first page-aligned address after the region base, so the bytes +ahead of it are lost: a whole page where the region base is page-aligned, +and the leading partial page, which was never usable, where it is not. A +device must count that in what it offers rather than in what it expects to +be used, and since it cannot know the guest's ``PAGE_SIZE`` it cannot know +which of the two cases it is in, so it has to budget a whole page. That +page is a page of the guest's ``PAGE_SIZE``, not of the device's own +granularity, and like every other figure here it has to be sized for the +largest page size the device expects a driver to run with. + +Offset zero is never published, and that is why the page goes. The +proposal reserves it: it is not the address of any structure the driver +places in the region, and a device may treat an offset of zero as an +error. Device implementations would not accept it in any case: a device +that has ever driven a virtqueue uses a queue address of zero as its +"this queue was never programmed" sentinel, and QEMU's generic virtio +core and its vhost path both do, so a queue published at offset zero is +dropped silently, with no error reported anywhere and no way for the +driver to tell. The driver keeps the value out of its pool rather than +rely on being an exception to that. + +Which pages is the guest's business, and the device cannot find out. +Every count and every byte figure below is a multiple of the guest's +``PAGE_SIZE``, and the figures given are for a 4 KiB page; a guest with +64 KiB pages needs up to sixteen times the bytes for the same queue depth +and the same traffic. So a device must size its region for the largest +page size it expects a driver to run with, or state the page size it was +sized for. + +One buffer costs ``ceil(len / PAGE_SIZE)`` slots per scatterlist entry:: + + slots(request) =3D sum over sg entries of ceil(len / PAGE_SIZE) + + ceil(nr_sg * 16 / PAGE_SIZE) [1] + + [1] with VIRTIO_F_INDIRECT_DESC and for a chain of more than one entry, + for the indirect descriptor table; one slot for up to 256 entries + +The term that surprises people is neither the payload nor the indirect +table: it is the small scatterlist entries, because each costs a whole +page. A virtio_blk read of 4 KiB, with ``VIRTIO_F_INDIRECT_DESC`` +negotiated, spends one slot on the 16-byte header, one on the 1-byte +status, one on the indirect table and one on the data, so 16 KiB of +region carries 4 KiB of payload. The 12 KiB of metadata is the same for +a 4 KiB request and for a 128 KiB one, so the amplification is worst for +small requests: 4x for that 4 KiB read, under 1.1x for a 128 KiB one. + +How much a driver pays therefore depends on how it lays a request out, +and a virtio_net transmit costs one slot rather than the four above. +``VIRTIO_F_VERSION_1``, which a device offering this feature has to offer +anyway, lets virtio_net push its header into the skb headroom; a linear +skb then yields a single scatterlist entry, and +``virtqueue_use_indirect()`` wants more than one entry before it builds a +table. One slot for the whole transmit, then: about 2.7x for a full-MTU +frame and about 57x for a 60-byte one, counting the pushed header as part +of what the slot carries. An skb the driver cannot push into, or one +carrying fragments, pays a slot per entry and one more for the table. + +How the pool is divided +----------------------- + +The pool is not one flat range. It is divided into *areas*, each with its +own lock, and a claim is served from one of them: the area belonging to the +CPU that asked, or the next area that has room. This is the structure +``kernel/dma/swiotlb.c`` uses, and it exists so that mappings on different +CPUs do not serialise on a single lock, and so that the time one search +spends with interrupts disabled does not grow with the region. + +.. note:: + + In this document *area* on its own always means a pool area: one of the + allocator's lock partitions, the quantity ``areas`` counts and + ``area_pages`` sizes. The Descriptor, Driver and Device Areas the + specification names are always written out as *virtqueue areas*, and the + pages one virtqueue's three areas occupy are ``ring_pages(num)``. + +A pool area covers a power-of-two number of pages, never fewer than 512 or +one cacheline of the allocator's bitmap, whichever is larger, and never mo= re +than 4096, and the count follows from that:: + + area_pages =3D clamp(rounddown_pow2(pages / roundup_pow2(possible_cpus)), + max(512, L1_CACHE_BYTES * 8), 4096) + areas =3D ceil(pages / area_pages) + +The last area is short unless ``area_pages`` divides ``pages``. A region +small enough for one area to cover behaves exactly as a single lock over +the whole pool does, which includes every region up to 2 MiB on a 4 KiB +page, and more than that on a guest with few possible CPUs. +Both values are printed at probe time, so a device implementer can read +back what a guest derived from the region it offered. + +A device cannot influence the count and should not try to: it is derived +from the guest's page size, cacheline size and possible-CPU count, none of +which the device knows. What it costs the guest is one cacheline per pool +area, against one bit of bitmap and one slot record of ``sizeof(phys_addr_= t) ++ 8`` bytes for every page of pool. On a 512 MiB region on a guest with +sixty-five or more possible CPUs, where the pool comes to 256 areas of 512 +pages, that is 16 KiB of areas against 2 MiB of slot records. + +No single buffer mapping may exceed an eighth of the pool or half of one +pool area, whichever is smaller, rounded down to a page; and never less +than one page, which is the floor that governs a region at the four-page +minimum, where an eighth rounds down to nothing. That is what +``max_mapping_size`` reports, through ``virtio_max_dma_size()``. Only +virtio_blk consults it, as its segment-size limit; on a driver that does +not, an over-cap mapping is refused rather than split, and that refusal is +permanent rather than back-pressure. The eighth bounds the capacity one +mapping can deny the rest of the device. The half-area is not a choice: an +allocation has to lie inside one pool area so that a single lock covers it, +and half rather than all of an area is headroom, because a request the size +of an area could only ever be satisfied by a completely empty one. Which = of +the two governs follows from the geometry rather than from the region size +alone, because ``area_pages`` is itself derived from the possible-CPU coun= t: +the half-area becomes the smaller only once ``area_pages`` has fallen belo= w a +quarter of ``pages``. On a 16 MiB region with three or more possible CPUs= it +has, and the cap is 1048576 bytes rather than the 2093056 an eighth alone +would give; on a one- or two-CPU guest the eighth still governs a 16 MiB +region. Virtqueue areas are allocated from the region too and are subject= to +neither cap, though an ``alloc()`` still has to fit inside one pool area. = A +split ring costs one allocation for all three virtqueue areas together, of +``vring_size(num, align)`` bytes, which at the cache-line alignment the +PCI transport passes is one slot at a queue depth of 128, two at 256, +seven at 1024 and twenty-seven at 4096. A packed ring costs three +separate allocations, the descriptor ring and two event structures, and +the two event structures are four bytes each: a packed queue therefore +always spends two whole pages on eight bytes. + +So a device must offer at least:: + + region_bytes >=3D PAGE_SIZE + + sum over virtqueues of + [ ring_pages(num) + num * slots(max_request) ] + * PAGE_SIZE + + where the leading PAGE_SIZE covers the bytes ahead of the pool, and is + a whole page because a device cannot know the guest's page size and so + cannot know whether its region base is aligned to one, + slots(max_request) is the largest slots(request) value from the + formula above the driver can produce for one request on that queue, + and ring_pages(num) is the pages one virtqueue's Descriptor, Driver and + Device Areas occupy: ceil(vring_size(num, align) / PAGE_SIZE) for a + split ring, and ceil(num * 16 / PAGE_SIZE) + 2 for a packed one + +A device that also offers an administration virtqueue pays for it out of +the same region: its virtqueue areas are allocated through the same path, +and one +administration command occupies several slots more. + +A driver that resizes a queue needs the region to hold both copies of its +virtqueue areas at once. ``virtqueue_resize()``, which ``VIRTIO_F_RING_RE= SET`` +makes reachable and which ``ethtool -G`` reaches on virtio_net, allocates +the new virtqueue areas before it frees the old, so on a region sized for +one copy +it is the new allocation that fails. What happens then differs by ring +layout, and only one of the two is a refusal. A packed ring cannot be +made smaller, so the resize fails and the queue keeps the depth it had. +A split ring on the PCI transport is created with ``may_reduce_num`` set, +so it halves the requested depth until the virtqueue areas fit and then +reports success at that smaller depth: a driver asking for a deeper queue +can get a shallower one instead, and the depth the queue goes on to report +is the only thing that says so. The halving stops once they fit in a sing= le +page, which at 4 KiB pages and cache-line alignment is a depth of 128, so a +region with no free page at all fails the resize as a packed ring would. + +A device must offer a region of at least four usable pages, which is five +pages of region where the region base is page-aligned. The driver refuses +a smaller one and sets the ``FAILED`` device status bit. That floor is +derived from one minimally-sized virtqueue and one buffer in +flight against it, so it is a lower bound on any working configuration +and not a size a device with several queues can sit on. Above that floor +a device that offers less than the formula asks for still probes, and the +shortfall appears as reduced effective queue depth: the ring reports a +mapping failure and the driver applies back-pressure. One case is not +back-pressure: a mapping longer than ``max_mapping_size`` is refused howev= er +empty the pool is, and retrying it never succeeds. The allocator is next = fit +within one pool area, from a hint that advances past each claim and rewind= s to +each release, beginning in the area belonging to the running CPU and then +trying each other area in turn, and nothing is moved, so a region that mee= ts +the formula can still fail an individual mapping to fragmentation. Either= is +legal, and either should be a deliberate choice rather than a surprise. + +A region too small even for the virtqueue areas is a different case. A +split ring halves the queue size and retries, so such a region can still +produce a working queue shorter than the device asked for, down to the +depth whose virtqueue areas fit in one page and no further; a packed ring +cannot be made smaller at all, so queue creation fails. The driver logs t= he +size of the virtqueue area that did not fit and how many pages the region +has: at +warning level for an attempt the ring did not mark as retryable, and at +debug level for the ones it did, which for a packed ring is all of them. + +Sizing the region for several virtqueues +---------------------------------------- + +The formula above is per virtqueue, and the sum over a multi-queue device +is what matters. A driver creates every virtqueue the device offers, not +the ones it goes on to use: ``virtnet_find_vqs()`` creates +``2 * max_queue_pairs`` virtqueues plus a control virtqueue, whatever +``curr_queue_pairs`` ends up being. So:: + + V =3D 2 * Q + C Q =3D max_queue_pairs, C =3D 1 with + VIRTIO_NET_F_CTRL_VQ + rings =3D 2 * Q * ring_pages(num) + C * ring_pages(num_ctrl) + the table below takes num_ctrl =3D num, + which over-counts a device giving its + control virtqueue a shallower ring + rx_fill =3D Q * num * B_rx B_rx =3D 1 with VIRTIO_NET_F_MRG_RXBUF + (or small buffers), MAX_SKB_FRAGS + 3 + without it + tx_min =3D Q * (MAX_SKB_FRAGS + 3) one worst-case segmented packet + per transmit queue, each of whose + scatterlist entries is assumed to + fit in one page + ctrl =3D 5 header, status, up to two data entries = and + the indirect table they take, since + virtqueue_use_indirect() builds one for + more than a single entry; each entry is + assumed to fit in one page + + floor_slots =3D rings + Q * B_rx + tx_min + ctrl multi-queue works: one + receive buffer per queue + work_slots =3D rings + rx_fill + tx_min + ctrl receive rings full + +Every term above assumes ``VIRTIO_F_INDIRECT_DESC``, which is what the +``+ 3`` and the ``ctrl`` table account for. Without it a chain occupies o= ne +descriptor per entry instead of one, so a queue of depth ``num`` posts few= er +buffers and ``rx_fill`` falls rather than rises. + +``try_fill_recv()`` fills a receive queue until it has no free descriptors, +so ``rx_fill`` and not ``floor_slots`` is the working figure. For a split +ring, x86_64, 4 KiB pages, ``MAX_SKB_FRAGS`` 17, mergeable receive buffers +and ``C =3D 1``: + +=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D= =3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D +Q num rings rx_fill work_slots work MiB +=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D= =3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D +1 256 6 256 287 1.1 +4 256 18 1024 1127 4.4 +8 256 34 2048 2247 8.8 +8 1024 119 8192 8476 33.1 +16 1024 231 16384 16940 66.2 +32 1024 455 32768 33868 132.3 +64 1024 903 65536 67724 264.5 +=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D= =3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D + +Two things to take from it. ``rx_fill`` dominates: it is 65536 slots of t= he +67724 sixty-four queue pairs come to, so sizing a region for multiple queu= es +is "how many receive buffers will be posted" to within three and a quarter +percent, and the virtqueue rings are the smaller part of what is left -- 9= 03 +slots against ``tx_min``'s 1280. And the pool areas of the previous +subsection do not appear at all, because they cost guest memory rather than +region pages. + +The receive buffer mode is a twenty-fold multiplier the device cannot +predict. A driver that negotiates any of ``VIRTIO_NET_F_GUEST_TSO4``, +``VIRTIO_NET_F_GUEST_TSO6``, ``VIRTIO_NET_F_GUEST_ECN`` or +``VIRTIO_NET_F_GUEST_UFO`` *without* ``VIRTIO_NET_F_MRG_RXBUF`` uses big +receive buffers, which cost ``MAX_SKB_FRAGS + 3`` slots each instead of +one: the eight-queue, depth-256 row above goes from 2247 slots to 41159, +which is 160.8 MiB instead of 8.8. A device offering guest segmentation +offload without ``MRG_RXBUF`` must size for that. Offering ``MRG_RXBUF`` +is the better answer. + +An undersized multi-queue region fails in two ways, and both are +properties of the network driver's existing behaviour rather than of the +region. + +First, cross-queue starvation. ``virtnet_open()`` pre-fills the receive +queues in index order and discards the result, so queue 0 takes what it +needs before queue 1 asks. A region that cannot hold every queue's fill +leaves the later queues with no buffers at all; the device's receive +steering then drops whatever it sends to them, and transmit fails on every +queue with ``tx_fifo_errors``, ``tx_dropped`` and a rate-limited +``Unexpected TXQ`` message. Pool areas do not help: at +``virtnet_open()`` every fill runs on whichever CPU brought the link up, so +they all share one home area. Areas give preference, never reservation. + +Second, a receive queue that cannot refill spends softirq time without +making progress. ``try_fill_recv()`` reports failure, ``virtnet_receive()= `` +returns the full budget to force a repoll, and ``virtnet_poll()`` therefore +never completes NAPI. There is no delayed worker behind that: commit +1e7b90aa7988 ("virtio-net: remove unused delayed refill worker") removed it +deliberately, "since we switched to retry refilling receive buffer in NAPI +poll instead of delayed worker". The repoll always recovers once capacity +is released, and it burns a CPU until then. + +What the driver does +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +Feature negotiation comes first. ``VIRTIO_F_DMB`` is accepted only when +the same offer carries ``VIRTIO_F_ACCESS_PLATFORM`` and +``VIRTIO_F_VERSION_1``, and only when the kernel was built with +``CONFIG_VIRTIO_DMB``. Once the device has +confirmed ``FEATURES_OK``, and not before, the driver reads the +``shmid``, locates the region through the ordinary shared memory +enumeration, and maps it. That happens during feature negotiation and so +before the device-type driver's own probe, and it records a claim on the +range, which appears in ``/proc/iomem``. The claim is advisory: a range +something else already owns is mapped anyway and nothing fails. If it +cannot locate the region it sets the ``FAILED`` device status bit: it has +already committed to the feature by accepting it, so refusing at that point +is not available to it. + +The driver then places every virtqueue area, indirect table and buffer +inside the +region and publishes offsets from the region start. Allocation is +page-granular, which is what makes each absolute address satisfy the +alignment the virtqueue layout in use requires, since the region base +carries no alignment guarantee of its own. Buffers that were not +allocated from the region are copied into it, and copied back out on +unmap for any mapping the device may have written, so a driver needs no +change to its data path to work with such a device. + +That copy in happens for every mapping, in either direction and whether +or not the caller asked for the CPU sync to be skipped, because a device +that writes only part of a buffer has to leave the rest of the caller's +bytes as they were. It covers the length of that mapping and nothing +beyond it, so where a buffer does not end on a page boundary the rest of +its last page still holds what that page held before: an earlier mapping +of this device, a freed virtqueue area, or what the device itself left +there. A device is given the length of every buffer it is offered, and +nothing outside that length is part of the buffer. + +A driver bound to such a device must delete its virtqueues before it +creates them again across a suspend or a reset. When the region is +unchanged the allocator is kept as it was, so a virtqueue that survived +still holds slots the pool has recorded as in use, and a second +``find_vqs()`` claims fresh slots alongside them that nothing will +release. + +The region is located after every feature negotiation, so its lifetime +is the lifetime of the negotiation and not of the device. A negotiation +that reports the same shmid, base and usable length as last time keeps +the region's kernel mapping that is already there, so an offset +published before it stays valid. A negotiation that reports a different +region replaces that mapping only when no virtqueue is live; nothing the +device left in the old one is reachable afterwards. A different region +under live virtqueues is refused and the device is marked ``FAILED``, so +a device must not move its region, or resize it by a whole page or more, +across a suspend or a reset it expects the driver's virtqueues to +survive. Either way, a device must not treat anything it left in the +region as still valid once the driver has reset it. + +Keeping the existing kernel mapping is also the whole of what happens +across hibernation, and that carries a limitation worth stating. That +mapping and the pool that describes it are ordinary kernel memory, so +they are part of the hibernation image, and on the way back up the +driver adopts them rather than establishing it again. Whether the +kernel address it sits at still reaches the region depends on the kernel +page tables being restored from the image along with everything else, +which is the same thing every driver carrying a ``memremap()`` or +``ioremap()`` across hibernation relies on, and nothing here checks it. +Establishing it again instead is not available while virtqueues are +live: a virtqueue holds the kernel addresses of its own virtqueue areas, +which are addresses inside it, and this code cannot rewrite them. +Tearing the region down at freeze time in order to make that safe is the +one thing it must not do, because a driver with no freeze callback +cannot be torn down, and a device that refuses to be frozen refuses +suspend for the whole system. + +The region's length bounds how much virtqueue data can be in flight at +once. Running out of room is therefore an ordinary condition and not an +error: a mapping fails and the driver applies the back-pressure it +already has for a full queue. The errno is whichever the ring already +reports for a failed mapping, which is ``-ENOMEM`` from a split ring and +from a packed indirect table, and ``-EIO`` from a packed ring using +direct descriptors. A driver must therefore treat any error from +``virtqueue_add_*()`` as back-pressure, and must not treat a particular +errno as the only indication of exhaustion. + +Under this feature the virtqueue data path calls no DMA mapping function +at all, so no bus address is ever passed to the device and the +``VIRTIO_F_ACCESS_PLATFORM`` obligation to translate one has nothing to +act on. The IOMMU is neither disabled nor programmed by this path. DMA +the device performs that does not go through a virtqueue, such as MSI-X +writes, is programmed by the transport as usual and is unaffected. + +``virtqueue_dma_dev()`` returns NULL for such a device, which every +driver bound to it inherits whether or not it asked for anything. A +driver that maps buffers itself loses that ability, and in virtio_net the +consequences are worth stating precisely because they are narrower than +"page_pool is unavailable". ``virtnet_create_page_pools()`` branches on +``virtqueue_dma_dev()`` and still creates the pool, without +``PP_FLAG_DMA_MAP`` and ``PP_FLAG_DMA_SYNC_DEV``: allocation and recycling +both keep working, and only the mapping the pool would have done is lost, +so the ring maps each buffer instead. Plain XDP works, being gated on +headroom rather than on a mapping device. ``AF_XDP`` does not: +``virtnet_xsk_pool_enable()`` refuses outright without a mapping device. + +The cost is that a receive buffer is mapped and copied on every post and +unmapped and copied on every completion, where a driver that could map for +itself would map once and recycle. Nothing here removes that, and the +reasons are structural rather than a matter of scheduling. Making +``virtqueue_dma_dev()`` return the parent device would be untrue: page_pool +would call ``dma_map_page()`` on it and produce addresses the device cannot +use, since a device using this feature reads offsets into its region. A +region-backed page_pool would have to allocate its pages inside the region, +which is a larger change than this. And eliding the copy on a receive +mapping needs ``DMA_ATTR_SKIP_CPU_SYNC`` both honoured on the map side and +produced by a driver, and no producer of it exists in the ring or in any +driver today. VDUSE already imposes the same downgrade for the same +reason, so drivers handle it, but it is a capability the device removes +from the driver rather than one the driver declines. + +Observing a region +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +A device that negotiates the feature reports what it got, the first time +the driver installs the region and again only if the device later reports +a different one:: + + virtio_net virtio5: device memory buffer 2 at 0x00000000c0000000, 4095 u= sable pages in 8 areas of 512 pages + +That names the ``shmid`` the device reported, where the region turned out +to be, how many pages the allocator can hand out, which is the region +length less the bytes ahead of the pool and less any trailing partial +page, and the pool-area geometry the guest derived. The count multiplied +by the page size is therefore smaller than +the length the transport reported for the region, and a device sizing +itself against the count rather than the length is the one that matches. +The eight areas of 512 pages in the example are what 4095 pages come to on +a guest with three or more possible CPUs; a guest with one or two derives +fewer, larger areas from the same region. +The claimed range also appears in ``/proc/iomem`` as +``virtio-dmb``, but only where the claim was granted, and the ``shmid`` +appears nowhere else at all. + +Notes for kernel code +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D + +A ``dma_addr_t`` belonging to a device using a Device Memory Buffer is a +byte offset into that region. It is not a DMA address, it is not valid +for any DMA API call, and no code outside the device's +``virtio_map_ops`` may treat it as either. It is never zero: the pool +starts after the first byte of the region, so that no address handed to +the device can be mistaken by it for a queue it was never given, and the +handle validation rejects zero along with every other offset below the +pool. + +The contents of the region are readable and writable by the device at +all times, including the descriptor table, the available ring and every +buffer. No value read back from the region may be used to compute a +kernel address, a length or an index without being validated first. In +particular, a header the driver has already validated may have been +rewritten by the time it is read a second time. diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index b6b5a36c3c21..67a7dc1a87df 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -195,6 +195,7 @@ config VIRTIO_DMB help Support devices that place their virtqueues and buffers in a shared memory region they own, rather than in memory the driver allocates. + See Documentation/driver-api/virtio/virtio-dmb.rst. =20 Enabling this adds a page allocator and per-page bookkeeping, both of which are set up only for a device that negotiates the feature. From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-002.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-002.esa.us-west-2.outbound.mail-perimeter.amazon.com [44.246.1.125]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2C5023016E0; Sun, 9 Aug 2026 18:21:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=44.246.1.125 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299703; cv=none; b=LJSIE1ldoauy/uBqnsntrzc7mdriu9ifKjpy2QhKSrw7gP9H+uCOXdDLNcxVkThp5WDMTNWG6q4W2F03Pngb00xWoqfyJFp9t+1OmD4U8M7r2MsUeBiw+2CnTxLcf+5+MXNdghc75YGH/9K6Ggd9z/Sl+rKCgOzewOXKnKnla5I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299703; c=relaxed/simple; bh=DRAZsgOl1qqzf65YiDOTALAb6txnLdZU6YpHhbUgAyo=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=KON4SDdPTjlQ3nPZDLFnYVBILnZJq/IDD/oaCcgJcZS3dTeGHbRJ5wWJm/VyQn7XD4Wn8wux3E1O0uEwbsyI9XgF25e+/vIq9lBC8OJnMwXhJ66Pq5LsHKWKdxijOvvCPXH5KTZxSjV8fZ0XGIjeOwecrdbHKyeoTPFd2/bxSbQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=eue8AQ6i; arc=none smtp.client-ip=44.246.1.125 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="eue8AQ6i" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299700; x=1817835700; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=vzL6Syiexp2mLHc2NqdKms5lj+Y2ja6OvvHtC7EZF74=; b=eue8AQ6iXm1jCi8/hIqrxaK+K5ImnAM1C6YYKIVfiZHzJJe1jpyU/1sv y/lwQA+OLw51fZKfnsZ0anhXiBk83IWQ/QYq+TVouP5+YJyyMFz76SEYA Wbk1Ur/EY88Pddcml5+JPgA2EjKW7zGxanhs3taMeXVAzo/bTHWCtaBO7 BWF8QySLJf1hAaSCayB7b9Fd2SyB0tFvNRl0fzGJYic7o7cKWjgxk1HNx P48zAW9du04PbKZcVs9I/IOD3TD4Y/NoNKRA+igytTTQ5nBFF40at1nLD yNq22B/Fr7hzm1xg4v76gY+JA/OTxM0cT+M8N6xJk6UrwD10e4F++zCoI A==; X-CSE-ConnectionGUID: nja4c6HQQfC1CmUCia0OCQ== X-CSE-MsgGUID: IXJovujMQG6d9o7eBMnL+A== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25521012" Received: from ip-10-5-6-203.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.6.203]) by internal-pdx-out-002.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:21:39 +0000 Received: from EX19MTAUWA001.ant.amazon.com [205.251.233.236:26930] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.19.171:2525] with esmtp (Farcaster) id f61689f6-416b-4ee2-ace5-56cf2a8bcbb8; Sun, 9 Aug 2026 18:21:39 +0000 (UTC) X-Farcaster-Flow-ID: f61689f6-416b-4ee2-ace5-56cf2a8bcbb8 Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWA001.ant.amazon.com (10.250.64.204) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:39 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:37 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , Jonathan Corbet , Shuah Khan , Halil Pasic , , , , , Stefan Hajnoczi , Paolo Bonzini Subject: [RFC PATCH 10/12] virtio_ring: report a bounded pool's exhaustion as -ENOSPC Date: Sun, 9 Aug 2026 18:20:08 +0000 Message-ID: <20260809182010.32931-11-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D042UWA002.ant.amazon.com (10.13.139.17) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" A failed mapping reaches the caller as -ENOMEM from virtqueue_add_split() and as -EIO from the two packed paths. -EIO is a fatal I/O error that virtblk_fail_to_queue() has no case for, so a packed-ring bounce that cannot be satisfied fails the I/O to the filesystem, where the same failure on a split ring is only back-pressure. -ENOMEM is little better: a map implementation's pool belongs to one device and only that device's completions free it, so a caller we tell -ENOMEM retries against a pool nothing else refills. Report -ENOSPC from vring_map_errno() when the failure came from a map implementation and num_free is not vring_num(), so the completion the caller waits for provably exists. include/linux/blk_types.h states that precondition for the BLK_STS_DEV_RESOURCE virtio_blk maps -ENOSPC to, and reporting it with nothing outstanding would put nd_virtio into a non-killable wait_event() that only a host completion wakes. virtio_map_ops.mapping_error() is handed no virtqueue, so a map implementation cannot draw the distinction for us. VDUSE reports -ENOSPC for its own mapping failures as a result, and drm/virtio's cursor queue, which waits for num_free to rise, retries in a tight loop because a failed mapping does not move it. The kerneldoc of virtqueue_add_sgs() said only a full queue reports -ENOSPC, so correct that as well. That way a bounce that could not be satisfied arrives at a block driver as back-pressure it can retry on either ring layout. Fixes: f7728002c1c7 ("virtio_ring: fix return code on DMA mapping fails") Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- .../driver-api/virtio/virtio-dmb.rst | 49 +++++++-- drivers/virtio/virtio_ring.c | 100 ++++++++++++++---- 2 files changed, 118 insertions(+), 31 deletions(-) diff --git a/Documentation/driver-api/virtio/virtio-dmb.rst b/Documentation= /driver-api/virtio/virtio-dmb.rst index 5975948165c9..4cd23e25901f 100644 --- a/Documentation/driver-api/virtio/virtio-dmb.rst +++ b/Documentation/driver-api/virtio/virtio-dmb.rst @@ -444,14 +444,19 @@ queue with ``tx_fifo_errors``, ``tx_dropped`` and a r= ate-limited ``virtnet_open()`` every fill runs on whichever CPU brought the link up, so they all share one home area. Areas give preference, never reservation. =20 -Second, a receive queue that cannot refill spends softirq time without -making progress. ``try_fill_recv()`` reports failure, ``virtnet_receive()= `` -returns the full budget to force a repoll, and ``virtnet_poll()`` therefore -never completes NAPI. There is no delayed worker behind that: commit +Second, a receive queue that holds no buffers and cannot refill spends +softirq time without making progress. ``try_fill_recv()`` reports failure +for the ``-ENOMEM`` such a queue gets, ``virtnet_receive()`` returns the +full budget to force a repoll, and ``virtnet_poll()`` therefore never +completes NAPI. There is no delayed worker behind that: commit 1e7b90aa7988 ("virtio-net: remove unused delayed refill worker") removed it deliberately, "since we switched to retry refilling receive buffer in NAPI poll instead of delayed worker". The repoll always recovers once capacity -is released, and it burns a CPU until then. +is released, and it burns a CPU until then. That is the only recovery a +queue with nothing posted can have, because the device cannot signal a used +buffer on it. A queue that does hold buffers gets ``-ENOSPC`` instead, +completes NAPI, and is woken by its own completions, so it stops cleanly; +the errno section below has the whole rule. =20 What the driver does =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D @@ -530,12 +535,34 @@ suspend for the whole system. The region's length bounds how much virtqueue data can be in flight at once. Running out of room is therefore an ordinary condition and not an error: a mapping fails and the driver applies the back-pressure it -already has for a full queue. The errno is whichever the ring already -reports for a failed mapping, which is ``-ENOMEM`` from a split ring and -from a packed indirect table, and ``-EIO`` from a packed ring using -direct descriptors. A driver must therefore treat any error from -``virtqueue_add_*()`` as back-pressure, and must not treat a particular -errno as the only indication of exhaustion. +already has for a full queue. Which errno the ring reports for it +depends on whether the failing virtqueue has a chain outstanding: + +* ``-ENOSPC`` when it has. This is the value a full ring already + reports, and it means what it means there: capacity that a completion + on this virtqueue will return. A caller may stop the queue and wait, + because the completion it waits for provably exists. + +* ``-ENOMEM`` when it has not. Nothing is outstanding to free capacity, + so there is no completion to wait for, and the caller must retry rather + than stop. A shortage the caller cannot wait out is reported the same + way whatever its origin, which is also what a failed + ``dma_map_page()`` on an ordinary device reports. + +The distinction matters because the two demand opposite responses, and a +caller that stops on the second never restarts. ``virtio_blk`` already +keys on exactly this: ``-ENOSPC`` stops the hardware queue, which +``virtblk_done()`` restarts on any completion, while ``-ENOMEM`` leaves it +running for the block layer to re-run after a delay. For a virtio_net +receive fill the split is the same one: a queue holding buffers stops +cleanly and its own completions wake it, and a queue holding none keeps +the NAPI repoll described above, which costs softirq time but recovers +without needing an interrupt the device cannot send. + +A driver must still treat any error from ``virtqueue_add_*()`` as +back-pressure and must not treat a particular errno as the only +indication of exhaustion. A mapping failure no longer produces +``-EIO``, on any ring layout. =20 Under this feature the virtqueue data path calls no DMA mapping function at all, so no bus address is ever passed to the device and the diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 9caa4f96204f..641ab07be931 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -337,6 +337,13 @@ static inline bool virtqueue_is_packed(const struct vr= ing_virtqueue *vq) vq->layout =3D=3D VQ_LAYOUT_PACKED_IN_ORDER; } =20 +/* Descriptors this virtqueue's ring holds, whatever its layout. */ +static inline u32 vring_num(const struct vring_virtqueue *vq) +{ + return virtqueue_is_packed(vq) ? vq->packed.vring.num : + vq->split.vring.num; +} + static inline bool virtqueue_is_in_order(const struct vring_virtqueue *vq) { return vq->layout =3D=3D VQ_LAYOUT_SPLIT_IN_ORDER || @@ -491,6 +498,35 @@ static int vring_mapping_error(const struct vring_virt= queue *vq, return dma_mapping_error(vring_dma_dev(vq), addr); } =20 +/* + * The errno a failed mapping reports to the caller. + * + * A map implementation owns a bounded pool that belongs to this device, s= o a + * failure to map into it while this virtqueue has a descriptor chain + * outstanding means capacity that this virtqueue's own completions will + * return: -ENOSPC, the value a full ring reports, which a caller that wai= ts + * for a completion already answers correctly. With nothing outstanding t= here is no + * such guarantee -- the pool may be held entirely by other virtqueues, or= by + * structural allocations no completion frees -- and a shortage the caller + * cannot wait out is -ENOMEM whatever its origin. That is the preconditi= on + * include/linux/blk_types.h states for BLK_STS_DEV_RESOURCE, which + * virtio_blk maps -ENOSPC to. + * + * num_free !=3D vring_num() is this tree's own definition of a non-empty = ring; + * both teardown paths assert the equality as the definition of an empty o= ne. + * All four virtqueue_add_*() paths decrement num_free after their mapping + * loop, below the last goto to their unmap_release label, so the value re= ad + * here counts chains already published and not the one being built. A + * refactor that moved either would break this silently. + */ +static int vring_map_errno(const struct vring_virtqueue *vq) +{ + if (vq->vq.vdev->map && vq->vq.num_free !=3D vring_num(vq)) + return -ENOSPC; + + return -ENOMEM; +} + /* Map one sg entry. */ static int vring_map_one_sg(const struct vring_virtqueue *vq, struct scatt= erlist *sg, enum dma_data_direction direction, dma_addr_t *addr, @@ -510,6 +546,12 @@ static int vring_map_one_sg(const struct vring_virtque= ue *vq, struct scatterlist if (dev_WARN_ONCE(&vq->vq.vdev->dev, vring_mapping_error(vq, *addr), "premapped buffer holds no valid mapping\n")) + /* + * Deliberately not vring_map_errno(): no capacity is + * involved, the caller published an invalid address, + * and reporting that as back-pressure would have the + * caller wait for a completion that will not fix it. + */ return -ENOMEM; =20 return 0; @@ -538,7 +580,7 @@ static int vring_map_one_sg(const struct vring_virtqueu= e *vq, struct scatterlist direction, attr); =20 if (vring_mapping_error(vq, *addr)) - return -ENOMEM; + return vring_map_errno(vq); =20 return 0; } @@ -677,6 +719,7 @@ static inline int virtqueue_add_split(struct vring_virt= queue *vq, unsigned int total_in_len =3D 0; int head; bool indirect; + int err; =20 START_USE(vq); =20 @@ -739,8 +782,9 @@ static inline int virtqueue_add_split(struct vring_virt= queue *vq, if (++sg_count !=3D total_sg) flags |=3D VRING_DESC_F_NEXT; =20 - if (vring_map_one_sg(vq, sg, DMA_TO_DEVICE, &addr, &len, - premapped, attr)) + err =3D vring_map_one_sg(vq, sg, DMA_TO_DEVICE, &addr, + &len, premapped, attr); + if (err) goto unmap_release; =20 /* Note that we trust indirect descriptor @@ -759,8 +803,9 @@ static inline int virtqueue_add_split(struct vring_virt= queue *vq, if (++sg_count !=3D total_sg) flags |=3D VRING_DESC_F_NEXT; =20 - if (vring_map_one_sg(vq, sg, DMA_FROM_DEVICE, &addr, &len, - premapped, attr)) + err =3D vring_map_one_sg(vq, sg, DMA_FROM_DEVICE, &addr, + &len, premapped, attr); + if (err) goto unmap_release; =20 /* Note that we trust indirect descriptor @@ -777,8 +822,10 @@ static inline int virtqueue_add_split(struct vring_vir= tqueue *vq, dma_addr_t addr =3D vring_map_single( vq, desc, total_sg * sizeof(struct vring_desc), DMA_TO_DEVICE); - if (vring_mapping_error(vq, addr)) + if (vring_mapping_error(vq, addr)) { + err =3D vring_map_errno(vq); goto unmap_release; + } =20 virtqueue_add_desc_split(vq, vq->split.vring.desc, vq->split.desc_extra, @@ -850,7 +897,7 @@ static inline int virtqueue_add_split(struct vring_virt= queue *vq, kfree(desc); =20 END_USE(vq); - return -ENOMEM; + return err; } =20 static bool virtqueue_kick_prepare_split(struct vring_virtqueue *vq) @@ -1665,6 +1712,17 @@ static int virtqueue_add_indirect_packed(struct vrin= g_virtqueue *vq, kfree(desc); =20 END_USE(vq); + + /* + * Not vring_map_errno(), unlike the three add paths a driver can + * reach. This value never leaves virtio_ring: both callers treat + * anything but -ENOMEM as final and return it, and use -ENOMEM as + * the signal to retry the same chain with direct descriptors. The + * direct attempt maps the same scatterlist and reports the errno for + * it, so a caller still learns that the pool is exhausted -- while a + * chain that failed only because the indirect table did not fit is + * still published, which is what it can be. + */ return -ENOMEM; } =20 @@ -1739,9 +1797,10 @@ static inline int virtqueue_add_packed(struct vring_= virtqueue *vq, for (sg =3D sgs[n]; sg; sg =3D sg_next(sg)) { dma_addr_t addr; =20 - if (vring_map_one_sg(vq, sg, n < out_sgs ? - DMA_TO_DEVICE : DMA_FROM_DEVICE, - &addr, &len, premapped, attr)) + err =3D vring_map_one_sg(vq, sg, n < out_sgs ? + DMA_TO_DEVICE : DMA_FROM_DEVICE, + &addr, &len, premapped, attr); + if (err) goto unmap_release; =20 flags =3D cpu_to_le16(vq->packed.avail_used_flags | @@ -1823,7 +1882,7 @@ static inline int virtqueue_add_packed(struct vring_v= irtqueue *vq, } =20 END_USE(vq); - return -EIO; + return err; } =20 static inline int virtqueue_add_packed_in_order(struct vring_virtqueue *vq, @@ -1900,9 +1959,10 @@ static inline int virtqueue_add_packed_in_order(stru= ct vring_virtqueue *vq, if (n >=3D out_sgs) flags |=3D cpu_to_le16(VRING_DESC_F_WRITE); =20 - if (vring_map_one_sg(vq, sg, n < out_sgs ? - DMA_TO_DEVICE : DMA_FROM_DEVICE, - &addr, &len, premapped, attr)) + err =3D vring_map_one_sg(vq, sg, n < out_sgs ? + DMA_TO_DEVICE : DMA_FROM_DEVICE, + &addr, &len, premapped, attr); + if (err) goto unmap_release; =20 flags |=3D cpu_to_le16(vq->packed.avail_used_flags); @@ -1979,7 +2039,7 @@ static inline int virtqueue_add_packed_in_order(struc= t vring_virtqueue *vq, } =20 END_USE(vq); - return -EIO; + return err; } =20 static bool virtqueue_kick_prepare_packed(struct vring_virtqueue *vq) @@ -2868,9 +2928,10 @@ static inline int virtqueue_add(struct virtqueue *_v= q, * * Returns zero or a negative error (ie. ENOSPC, ENOMEM, EIO). * - * NB: ENOSPC is a special code that is only returned on an attempt to add= a - * buffer to a full VQ. It indicates that some buffers are outstanding and= that - * the operation can be retried after some buffers have been used. + * NB: ENOSPC indicates that some buffers are outstanding and that the + * operation can be retried after some buffers have been used. A full VQ + * reports it, and so does a failure to map into a bounded pool a map + * implementation owns while this virtqueue has a chain outstanding. */ int virtqueue_add_sgs(struct virtqueue *_vq, struct scatterlist *sgs[], @@ -3623,8 +3684,7 @@ unsigned int virtqueue_get_vring_size(const struct vi= rtqueue *_vq) =20 const struct vring_virtqueue *vq =3D to_vvq(_vq); =20 - return virtqueue_is_packed(vq) ? vq->packed.vring.num : - vq->split.vring.num; + return vring_num(vq); } EXPORT_SYMBOL_GPL(virtqueue_get_vring_size); From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-015.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-015.esa.us-west-2.outbound.mail-perimeter.amazon.com [50.112.246.219]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B7960373C1A; Sun, 9 Aug 2026 18:21:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=50.112.246.219 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299708; cv=none; b=s/PKJBT0FVL93hPj4FsQbpL/BJmKzUEkddK6hB1+Rf99lURdMBNWVpEmjIhq4oslJn6AH3HvXQz5QWTsihvBr55Zn3mBM/f9lIbuFKWnBy72ITj2f5gsArbCPfcRFjj6kzcZRchIu5885npfiSWeSINwWDkky3huPJlBPZ/b1bs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299708; c=relaxed/simple; bh=J+30Nzlnc59X91Bo0RHuiOtOVXbqmP/Ow6OyZxvLSFI=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=pTYHwZQywUMu+oPk8iRVtOTZDoiOwvbwcZyM4LU7V9e1JRQwvVgheAWXrgf9kf/6vAhJaoboIXejoRBcgG0m5w3IaUU4ZqKV4E3YoiiY4XZbHM/dgEKi3XY7542MlfqbBFXioa8hROH2b24B/OPLNnBIDt5s8i8TpFHfuOdOUR0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=XF9w49Jg; arc=none smtp.client-ip=50.112.246.219 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="XF9w49Jg" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299706; x=1817835706; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=3ZohG5gy9QOsMsldJtnm8SZT4TUIYoDQX1T75IzNTaY=; b=XF9w49Jg0TUatCyo0JtIexDXOEdK/ZRpaenKjeAn87qrOXmaoJm1bbl7 Ojpgw5a1pSOhP8BZ7GultCHpBq9RuvySKIdUkXM+6MWcrfVOMEdIfRF1N ipubHqzACmzv2H4vtZHXMbwe3oOZxtBObRo6ROIeSTxv7uPuuFIrRX1mH ZE5QayMST0npRvA61ikyeNmV7DG4AFagesSh9xvbrMQTOmja780ODKLXm BWyZBbP2dFdBpkjGGa8kNE4DLqOu4Dq4MB/gZGK8fHl7ivdKB8SgsVHAW Bf6ArCDR7mYZbQTy9mBYp4p+rILBzSqyHOBgEa9cIX/JvxNcXq4ST9dqJ Q==; X-CSE-ConnectionGUID: MAxwrWlMTqK30MRFk/cRJw== X-CSE-MsgGUID: p2nOzCqUTeSBkNvs1LVtow== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25317679" Received: from ip-10-5-6-203.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.6.203]) by internal-pdx-out-015.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:21:42 +0000 Received: from EX19MTAUWC001.ant.amazon.com [205.251.233.53:26917] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.58.172:2525] with esmtp (Farcaster) id cdbce74b-36db-44c6-840a-130baee747a8; Sun, 9 Aug 2026 18:21:42 +0000 (UTC) X-Farcaster-Flow-ID: cdbce74b-36db-44c6-840a-130baee747a8 Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWC001.ant.amazon.com (10.250.64.174) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:42 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:21:39 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , Jonathan Corbet , Shuah Khan , , , , Jens Axboe , , David Airlie , Gerd Hoffmann , Dmitry Osipenko , , "Pankaj Gupta" , , , Stefan Hajnoczi , "Paolo Bonzini" Subject: [RFC PATCH 11/12] virtio: expose device memory buffer occupancy over debugfs Date: Sun, 9 Aug 2026 18:20:09 +0000 Message-ID: <20260809182010.32931-12-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D042UWA002.ant.amazon.com (10.13.139.17) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" Running out of room in the pool is routine for a device with a bounded one: vring_map_errno() reports it as the -ENOSPC a full queue reports, so the driver retries and carries on. The allocator must not log that at any level a working device prints, or an undersized region turns into a log flood. That leaves an operator nothing but unexplained throughput loss to go on. swiotlb can afford dev_warn_ratelimited() because exhaustion there is a misconfiguration, and it exports io_tlb_used and io_tlb_used_hiwater through debugfs, which a log line cannot do for a sampled quantity. Follow it, plus one file swiotlb has no need for: dmb/pages pages the allocator can hand out dmb/areas pool areas the pool is divided into dmb/area_pages pages one pool area covers dmb/used_pages pages allocated now dmb/used_pages_hiwater the largest used_pages has been dmb/alloc_failed buffer mappings the pool had no room for used_pages_hiwater is the number to size a region against, because a burst that fills the pool between two samples of used_pages leaves no other trace. Writing 0 restarts the measurement from the occupancy now, so a peak never reads below the used_pages read alongside it. alloc_failed counts only the mapping path, so that a virtqueue area rejected during vring_alloc_queue_split()'s search for a size that fits does not make a correctly sized region look undersized. There is no sysfs file, because a sysfs attribute is ABI and nothing here should be. Every file and every counter behind one sits under CONFIG_VIRTIO_DEBUG, so a production build carries none of them. Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- .../driver-api/virtio/virtio-dmb.rst | 52 ++++- drivers/virtio/virtio_dmb.c | 215 +++++++++++++++++- 2 files changed, 262 insertions(+), 5 deletions(-) diff --git a/Documentation/driver-api/virtio/virtio-dmb.rst b/Documentation= /driver-api/virtio/virtio-dmb.rst index 4cd23e25901f..610db080dee5 100644 --- a/Documentation/driver-api/virtio/virtio-dmb.rst +++ b/Documentation/driver-api/virtio/virtio-dmb.rst @@ -255,8 +255,10 @@ The last area is short unless ``area_pages`` divides `= `pages``. A region small enough for one area to cover behaves exactly as a single lock over the whole pool does, which includes every region up to 2 MiB on a 4 KiB page, and more than that on a guest with few possible CPUs. -Both values are printed at probe time, so a device implementer can read -back what a guest derived from the region it offered. +Both values are printed at probe time and are readable under +``CONFIG_VIRTIO_DEBUG`` in ``dmb/areas`` and ``dmb/area_pages``, so a +device implementer can read back what a guest derived from the region it +offered. =20 A device cannot influence the count and should not try to: it is derived from the guest's page size, cacheline size and possible-CPU count, none of @@ -619,7 +621,51 @@ a guest with three or more possible CPUs; a guest with= one or two derives fewer, larger areas from the same region. The claimed range also appears in ``/proc/iomem`` as ``virtio-dmb``, but only where the claim was granted, and the ``shmid`` -appears nowhere else at all. +appears nowhere else at all. The three counts appear again in +``dmb/pages``, ``dmb/areas`` and ``dmb/area_pages`` below. + +With ``CONFIG_VIRTIO_DEBUG`` the state of the region is also available +under the device's virtio debugfs directory, in ``dmb/``. The directory +exists only while the device has a region, so a device that did not +negotiate the feature has no ``dmb/`` at all. These files are +diagnostics and not ABI: their names, their contents and the one write +they accept may change or go away. + +``pages`` + how many pages the allocator can hand out. + +``areas`` + how many pool areas the pool is divided into. + +``area_pages`` + how many pages one pool area covers. ``areas`` is + ``ceil(pages / area_pages)``, so the last area covers fewer unless + ``area_pages`` divides ``pages``. Both are fixed when the region is + installed and are the same two values the probe-time message prints. + +``used_pages`` + how many of them are allocated. One counter maintained across all areas + rather than a sum of per-area figures read at different moments, so it + never reports a torn total; it is raised just outside the area lock, so a + read taken during a claim or a release can lag the bitmap by that claim. + +``used_pages_hiwater`` + the largest ``used_pages`` has been. This, rather than a sample of + ``used_pages``, is what a region should be sized against: a burst + that fills the region between two samples leaves no other trace. + Writing ``0`` restarts the measurement from the current occupancy. + +``alloc_failed`` + how many buffer mappings the pool had no room for. Because running + out of room is an ordinary condition it is not logged above debug + level, so this is the cheapest indication that a region is too small + for what is running on it: a non-zero value means the pool could not + fit a mapping the workload asked for, either because it is smaller + than the workload needs or because no single pool area held a long + enough run of free pages. A virtqueue area that did not fit is not + counted here, since it is logged instead, and neither is a mapping + refused for exceeding the per-mapping cap, which ``max_mapping_size`` + advertises. =20 Notes for kernel code =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D diff --git a/drivers/virtio/virtio_dmb.c b/drivers/virtio/virtio_dmb.c index 42126f928bc0..fe556d585f24 100644 --- a/drivers/virtio/virtio_dmb.c +++ b/drivers/virtio/virtio_dmb.c @@ -20,10 +20,12 @@ */ =20 #include +#include #include #include #include #include +#include #include #include #include @@ -126,7 +128,12 @@ struct virtio_dmb_area { * @area_slots: slots one area covers, a power of two; the last area covers * fewer when @nslots is not a multiple of it * @area_shift: ilog2(@area_slots), so slot >> @area_shift names its area + * @total_used: slots allocated across every area, exact; CONFIG_VIRTIO_DE= BUG + * @used_hiwater: the largest @total_used has been since the last reset th= rough + * debugfs, or since init; CONFIG_VIRTIO_DEBUG + * @alloc_failed: buffer mappings the pool had no room for; CONFIG_VIRTIO_= DEBUG * @shm_id: shared memory id the device reported for the region + * @debugfs_dir: directory holding this region's debugfs files */ struct virtio_dmb { struct virtio_device *vdev; @@ -145,7 +152,20 @@ struct virtio_dmb { unsigned int nareas; unsigned int area_slots; unsigned int area_shift; +#ifdef CONFIG_VIRTIO_DEBUG + /* + * Exact occupancy for the two debugfs files, kept outside the area + * locks. A production build has neither: summing the per-area counts + * would need every lock, and the hot path is what this file exists to + * make cheap. kernel/dma/swiotlb.c draws the same line at + * CONFIG_DEBUG_FS. + */ + atomic_long_t total_used; + atomic_long_t used_hiwater; + atomic_long_t alloc_failed; +#endif u16 shm_id; + struct dentry *debugfs_dir; }; =20 /* First slot of area @i. */ @@ -210,6 +230,61 @@ static size_t virtio_dmb_max_mapping(const struct virt= io_dmb *dmb) return max_t(size_t, min(eighth, half_area), PAGE_SIZE); } =20 +#ifdef CONFIG_VIRTIO_DEBUG + +/* + * Exact global occupancy, kept outside every area lock. + * + * Summing the per-area counts would be imprecise, because no two of them = are + * read under the same lock, and taking every lock to read one debugfs file + * would put a whole-pool serialisation back into a file whose only purpos= e is + * to observe. An atomic add-return instead yields a value @total_used + * genuinely held, so raising the high-water mark to the largest such value + * makes both figures exact rather than approximate: two racing claims each + * observe a distinct real total and the larger wins. + * + * This is kernel/dma/swiotlb.c's inc_used_and_hiwater()/dec_used() pair, = for + * the same reason and with the same empty stubs when the option is off. + */ +static void virtio_dmb_inc_used(struct virtio_dmb *dmb, unsigned int nr) +{ + long old_hiwater, new_used; + + new_used =3D atomic_long_add_return(nr, &dmb->total_used); + old_hiwater =3D atomic_long_read(&dmb->used_hiwater); + do { + if (new_used <=3D old_hiwater) + break; + } while (!atomic_long_try_cmpxchg(&dmb->used_hiwater, &old_hiwater, + new_used)); +} + +static void virtio_dmb_dec_used(struct virtio_dmb *dmb, unsigned int nr) +{ + atomic_long_sub(nr, &dmb->total_used); +} + +static void virtio_dmb_inc_alloc_failed(struct virtio_dmb *dmb) +{ + atomic_long_inc(&dmb->alloc_failed); +} + +#else /* !CONFIG_VIRTIO_DEBUG */ + +static void virtio_dmb_inc_used(struct virtio_dmb *dmb, unsigned int nr) +{ +} + +static void virtio_dmb_dec_used(struct virtio_dmb *dmb, unsigned int nr) +{ +} + +static void virtio_dmb_inc_alloc_failed(struct virtio_dmb *dmb) +{ +} + +#endif /* CONFIG_VIRTIO_DEBUG */ + /* * Claim @nr contiguous slots from area @i, or -ENOMEM when that one area * cannot satisfy the request. Takes and drops that area's lock and touch= es @@ -254,6 +329,12 @@ static long virtio_dmb_area_claim(struct virtio_dmb *d= mb, unsigned int i, =20 spin_unlock_irqrestore(&area->lock, flags); =20 + /* + * Outside the lock, which is where swiotlb_search_pool_area() does it + * too: it touches no area state, so holding one buys nothing. + */ + virtio_dmb_inc_used(dmb, nr); + return slot; =20 not_found: @@ -271,7 +352,9 @@ static long virtio_dmb_area_claim(struct virtio_dmb *dm= b, unsigned int i, * buffer, and repolls instead. Reporting it at any level a working device * would print would therefore be a log flood, and it is the only signal an * undersized region produces at all, so it is reported through dynamic de= bug - * where it costs nothing until somebody asks for it. + * where it costs nothing until somebody asks for it. The map_page() call= er + * counts it as well, so that a debug build offers a sampled reader as wel= l as + * a log. * * Next fit within one area, from a hint that advances past each claim and * rewinds to each release, beginning in the area belonging to the running= CPU @@ -390,6 +473,18 @@ static void virtio_dmb_release(struct virtio_dmb *dmb,= unsigned int slot, area->used -=3D nr; area->index =3D slot - virtio_dmb_area_base(dmb, a); =20 + spin_unlock_irqrestore(&area->lock, flags); + + /* + * Outside the lock, and deliberately not below the label: the guard + * above rejects a range that is not wholly allocated, and only slots + * that were counted in are counted out, so the total tracks the bitmap + * rather than the caller's arithmetic. + */ + virtio_dmb_dec_used(dmb, nr); + + return; + out: spin_unlock_irqrestore(&area->lock, flags); } @@ -717,8 +812,22 @@ static dma_addr_t virtio_dmb_op_map_page(union virtio_= map map, =20 nr =3D virtio_dmb_slots(size); ret =3D virtio_dmb_claim(dmb, nr); - if (ret < 0) + if (ret < 0) { + /* + * Counted here rather than in virtio_dmb_claim(), which + * virtio_dmb_op_alloc() reaches as well. A virtqueue area + * that does not fit is a step of vring_alloc_queue_split()'s + * search for a size that does, so counting it would have a + * correctly sized region boot with a failure for every + * attempt but the last, in the one file whose purpose is to + * answer whether the region is too small for the traffic. A + * request over the per-mapping cap is not counted either: the + * cap is what max_mapping_size() advertises, so exceeding it + * is a caller bug rather than a property of the region. + */ + virtio_dmb_inc_alloc_failed(dmb); return DMA_MAPPING_ERROR; + } slot =3D ret; =20 virtio_dmb_record(dmb, slot, nr, size, src); @@ -860,6 +969,105 @@ static const struct virtio_map_ops virtio_dmb_map_ops= =3D { .max_mapping_size =3D virtio_dmb_op_max_mapping_size, }; =20 +#ifdef CONFIG_VIRTIO_DEBUG + +static int virtio_dmb_used_get(void *data, u64 *val) +{ + struct virtio_dmb *dmb =3D data; + + *val =3D atomic_long_read(&dmb->total_used); + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(virtio_dmb_used_fops, virtio_dmb_used_get, NULL, + "%llu\n"); + +static int virtio_dmb_hiwater_get(void *data, u64 *val) +{ + struct virtio_dmb *dmb =3D data; + + *val =3D atomic_long_read(&dmb->used_hiwater); + + return 0; +} + +static int virtio_dmb_hiwater_set(void *data, u64 val) +{ + struct virtio_dmb *dmb =3D data; + + /* Restarting the measurement is the only meaningful write. */ + if (val) + return -EINVAL; + + /* + * Restart from what is allocated now rather than from zero, so that + * the file never reports a peak below the occupancy it is read + * alongside. + */ + atomic_long_set(&dmb->used_hiwater, + atomic_long_read(&dmb->total_used)); + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(virtio_dmb_hiwater_fops, virtio_dmb_hiwater_get, + virtio_dmb_hiwater_set, "%llu\n"); + +static int virtio_dmb_alloc_failed_get(void *data, u64 *val) +{ + struct virtio_dmb *dmb =3D data; + + *val =3D atomic_long_read(&dmb->alloc_failed); + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(virtio_dmb_alloc_failed_fops, + virtio_dmb_alloc_failed_get, NULL, "%llu\n"); + +/* + * The files go under the device's existing virtio debugfs directory, and = exist + * only while the device has a region. They describe one, so their presen= ce is + * also the answer to whether the device is using the feature. + * + * Every file is served through DEFINE_DEBUGFS_ATTRIBUTE, either directly = or by + * a debugfs_create_*() helper that uses it, so each read takes a reference + * that debugfs_remove_recursive() waits for. That is what lets the caller + * free the state these files point at once the directory is gone. + */ +static void virtio_dmb_debugfs_init(struct virtio_dmb *dmb) +{ + struct dentry *dir; + + dir =3D debugfs_create_dir("dmb", dmb->vdev->debugfs_dir); + dmb->debugfs_dir =3D dir; + + debugfs_create_u32("pages", 0400, dir, &dmb->nslots); + debugfs_create_u32("areas", 0400, dir, &dmb->nareas); + debugfs_create_u32("area_pages", 0400, dir, &dmb->area_slots); + debugfs_create_file("used_pages", 0400, dir, dmb, + &virtio_dmb_used_fops); + debugfs_create_file("used_pages_hiwater", 0600, dir, dmb, + &virtio_dmb_hiwater_fops); + debugfs_create_file("alloc_failed", 0400, dir, dmb, + &virtio_dmb_alloc_failed_fops); +} + +static void virtio_dmb_debugfs_exit(struct virtio_dmb *dmb) +{ + debugfs_remove_recursive(dmb->debugfs_dir); +} + +#else /* !CONFIG_VIRTIO_DEBUG */ + +static void virtio_dmb_debugfs_init(struct virtio_dmb *dmb) +{ +} + +static void virtio_dmb_debugfs_exit(struct virtio_dmb *dmb) +{ +} + +#endif /* CONFIG_VIRTIO_DEBUG */ + /* * Whether the device still has virtqueues. vqs_list_lock is what protects * that list against a concurrent adder. No caller here can race one, bec= ause @@ -929,6 +1137,7 @@ void virtio_dmb_destroy(struct virtio_device *vdev) vdev->map =3D dmb->prev_map; vdev->vmap =3D dmb->prev_vmap; =20 + virtio_dmb_debugfs_exit(dmb); memunmap(dmb->map_va); if (dmb->map_claimed) release_mem_region(dmb->map_phys, dmb->map_len); @@ -1278,6 +1487,8 @@ int virtio_dmb_init(struct virtio_device *vdev) dmb->area_slots =3D area_slots; dmb->area_shift =3D ilog2(area_slots); =20 + virtio_dmb_debugfs_init(dmb); + /* Published last: until now nothing routes a mapping here. */ dmb->prev_map =3D vdev->map; dmb->prev_vmap =3D vdev->vmap; From nobody Tue Sep 29 09:45:52 2026 Received: from pdx-out-008.esa.us-west-2.outbound.mail-perimeter.amazon.com (pdx-out-008.esa.us-west-2.outbound.mail-perimeter.amazon.com [52.42.203.116]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3103E3559E1; Sun, 9 Aug 2026 18:22:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=52.42.203.116 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299732; cv=none; b=S0w+QWC0RMEdPaR4QhGhY5zivgIgk/lhKiGnrbo/r1k2iZaMKYGGPvj5IVZx47/YpAdmRD4EauopO0cqiDlhHl00po6a8ZBwJrbLk7Kxe0dTrmre5Et7WIQF0hT6DAIxLxIKzXV9cCAQndjJfl5de3wrdbostxlqgM0G7yd5fV8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786299732; c=relaxed/simple; bh=bnJKTG0nidINrA6BoUVNxu1I36m5Sz+bZD32xqDmMaM=; h=From:To:CC:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=Vhxeqx89NUP84R6sXI0rXHlGYP84NZuu38gdvJ8BoR5qt8EiJoIgLN5rj1QMEO/R0YPd6NOYe16WVmLcvhw0HJXKJRrvrWD3X0y43/o6kLebcFEOTgzYhmpTEGg/rk6BYD4J7m06RclqDXQvkoFs7xKhg9YWi6TYZn4s+blJ3mY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com; spf=pass smtp.mailfrom=amazon.de; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b=Gpo0Hgmh; arc=none smtp.client-ip=52.42.203.116 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=amazon.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=amazon.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=amazon.com header.i=@amazon.com header.b="Gpo0Hgmh" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazoncorp2; t=1786299729; x=1817835729; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=fDY8Nt5HeTNm8g1LUKnWtoj0HVblP9xXqUYeTyk5B68=; b=Gpo0Hgmh0EwAdBu4PJTBG78Zltj7RwPD2nOY4q5Prt1jDdHUTbBRBVsy aHpelr5Owb8y9DsHF6I3jUGt8+ruFeWbCGuF5grXICbXY7Djka8Bba5cD 8yV3Ui6Vcc6w2qoA2hPFFp8ru7qXBP9kKFhoPWN+3R53pQmi1Xz7tO5pc 3ghN9g+XbYLX1nG/uhDxDaqIPlxbza9jG1nWyq2YCZdDbeoDfsaIQKAk2 204ZTReO4A4ttNy0VxrUjFdW36WwSnbML7I1t8YlxA/aSBaVEQMzQU2uW eRssaZ+N4gxY8v1DA2C/V5gMHskFyuv91zS1m8uZqGCTxAST0By4xYEao g==; X-CSE-ConnectionGUID: Tk2v1b/aRBqEtjwI4GfRXA== X-CSE-MsgGUID: Ji5GrQl5Q8ybRfP7A8biSg== X-IronPort-AV: E=Sophos;i="6.25,214,1779148800"; d="scan'208";a="25540693" Received: from ip-10-5-0-115.us-west-2.compute.internal (HELO smtpout.naws.us-west-2.prod.farcaster.email.amazon.dev) ([10.5.0.115]) by internal-pdx-out-008.esa.us-west-2.outbound.mail-perimeter.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Aug 2026 18:22:06 +0000 Received: from EX19MTAUWB002.ant.amazon.com [205.251.233.48:17781] by smtpin.naws.us-west-2.prod.farcaster.email.amazon.dev [10.0.37.134:2525] with esmtp (Farcaster) id da160e83-2e0b-4447-91e0-182d7df7fbce; Sun, 9 Aug 2026 18:22:06 +0000 (UTC) X-Farcaster-Flow-ID: da160e83-2e0b-4447-91e0-182d7df7fbce Received: from EX19D001UWA001.ant.amazon.com (10.13.138.214) by EX19MTAUWB002.ant.amazon.com (10.250.64.231) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:22:06 +0000 Received: from ip-10-253-83-51.amazon.com (172.19.99.218) by EX19D001UWA001.ant.amazon.com (10.13.138.214) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.2562.45; Sun, 9 Aug 2026 18:22:03 +0000 From: Alexander Graf To: "Michael S. Tsirkin" , Jason Wang CC: Xuan Zhuo , =?UTF-8?q?Eugenio=20P=C3=A9rez?= , Jonathan Corbet , Shuah Khan , , , , , Stefan Hajnoczi , "Paolo Bonzini" Subject: [RFC PATCH 12/12] virtio: guarantee a virtqueue can publish its first descriptor chain Date: Sun, 9 Aug 2026 18:20:10 +0000 Message-ID: <20260809182010.32931-13-graf@amazon.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20260809182010.32931-1-graf@amazon.com> References: <20260809182010.32931-1-graf@amazon.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: EX19D037UWC001.ant.amazon.com (10.13.139.197) To EX19D001UWA001.ant.amazon.com (10.13.138.214) Content-Type: text/plain; charset="utf-8" A device's pool is shared by every virtqueue it has and nothing partitions it between them, so a virtqueue that asks late can get nothing at all. virtnet_open() pre-fills the receive queues in index order and discards the result, and a queue that came up with nothing is one the device cannot signal a completion on. Withhold a range of the pool from ordinary claims and let a virtqueue whose ring is empty draw on it for one descriptor chain. vring_map_reserve_attr() marks those mappings with a new attrs bit, VIRTIO_MAP_ATTR_RESERVE, which asks a bounded map implementation to serve them out of the withheld range and which it may ignore. VIRTIO_MAP_ATTR_MASK collects the attrs bits virtio owns, which virtqueue_map_page_attrs() strips before any dma_map_*() call. VIRTIO_MAP_RESERVE_PAGES bounds what one chain may draw at 32 pages, enough for a page-granular chain at the default CONFIG_MAX_SKB_FRAGS. Size the withheld range as one chain's allowance plus one page for each virtqueue beyond the one drawing the chain; an allowance per virtqueue would be a permanent shortage, since only an empty ring may draw. virtio_dmb_note_vqs() takes the count from vp_modern_find_vqs(), which counts the administration virtqueue too, and the size it arrives at appears in a second probe-time message and in dmb/reserved_pages. The new VIRTIO_DMB_KUNIT_TEST option covers the range's geometry and the withdrawal of the feature, neither of which we reach on a real device. That way a virtqueue that comes up against a full pool publishes its first chain and gets a completion to wait for. Assisted-by: Kiro:claude-opus-5 checkpatch sparse Signed-off-by: Alexander Graf --- .../driver-api/virtio/virtio-dmb.rst | 178 +++++++++-- drivers/virtio/Kconfig | 13 + drivers/virtio/virtio_dmb.c | 247 ++++++++++++++-- drivers/virtio/virtio_dmb.h | 6 + drivers/virtio/virtio_dmb_test.c | 279 ++++++++++++++++++ drivers/virtio/virtio_pci_modern.c | 18 +- drivers/virtio/virtio_ring.c | 84 +++++- include/linux/virtio_config.h | 33 +++ 8 files changed, 784 insertions(+), 74 deletions(-) create mode 100644 drivers/virtio/virtio_dmb_test.c diff --git a/Documentation/driver-api/virtio/virtio-dmb.rst b/Documentation= /driver-api/virtio/virtio-dmb.rst index 610db080dee5..d84f0771f010 100644 --- a/Documentation/driver-api/virtio/virtio-dmb.rst +++ b/Documentation/driver-api/virtio/virtio-dmb.rst @@ -302,15 +302,18 @@ So a device must offer at least:: + sum over virtqueues of [ ring_pages(num) + num * slots(max_request) ] * PAGE_SIZE + + reserved * PAGE_SIZE =20 where the leading PAGE_SIZE covers the bytes ahead of the pool, and is a whole page because a device cannot know the guest's page size and so cannot know whether its region base is aligned to one, slots(max_request) is the largest slots(request) value from the formula above the driver can produce for one request on that queue, - and ring_pages(num) is the pages one virtqueue's Descriptor, Driver and + ring_pages(num) is the pages one virtqueue's Descriptor, Driver and Device Areas occupy: ceil(vring_size(num, align) / PAGE_SIZE) for a - split ring, and ceil(num * 16 / PAGE_SIZE) + 2 for a packed one + split ring, and ceil(num * 16 / PAGE_SIZE) + 2 for a packed one, + and reserved is the range withheld from ordinary claims, described + under "The withheld range" below =20 A device that also offers an administration virtqueue pays for it out of the same region: its virtqueue areas are allocated through the same path, @@ -391,6 +394,13 @@ the ones it goes on to use: ``virtnet_find_vqs()`` cre= ates floor_slots =3D rings + Q * B_rx + tx_min + ctrl multi-queue works: one receive buffer per queue work_slots =3D rings + rx_fill + tx_min + ctrl receive rings full + reserved =3D min(32 + V - 1, pages / 2) withheld, and zero below = 256 + pages; a short last pool + area can add to it, see + below. The V there counts + an administration virtqueue + too where a device has one + region_slots =3D work_slots + reserved what a device must offer =20 Every term above assumes ``VIRTIO_F_INDIRECT_DESC``, which is what the ``+ 3`` and the ``ctrl`` table account for. Without it a chain occupies o= ne @@ -402,25 +412,33 @@ so ``rx_fill`` and not ``floor_slots`` is the working= figure. For a split ring, x86_64, 4 KiB pages, ``MAX_SKB_FRAGS`` 17, mergeable receive buffers and ``C =3D 1``: =20 -=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D= =3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D -Q num rings rx_fill work_slots work MiB -=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D= =3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D -1 256 6 256 287 1.1 -4 256 18 1024 1127 4.4 -8 256 34 2048 2247 8.8 -8 1024 119 8192 8476 33.1 -16 1024 231 16384 16940 66.2 -32 1024 455 32768 33868 132.3 -64 1024 903 65536 67724 264.5 -=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D= =3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D - -Two things to take from it. ``rx_fill`` dominates: it is 65536 slots of t= he -67724 sixty-four queue pairs come to, so sizing a region for multiple queu= es -is "how many receive buffers will be posted" to within three and a quarter -percent, and the virtqueue rings are the smaller part of what is left -- 9= 03 -slots against ``tx_min``'s 1280. And the pool areas of the previous -subsection do not appear at all, because they cost guest memory rather than -region pages. +=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D= =3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D +Q num rings rx_fill work_slots work MiB reserved regi= on +=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D= =3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D +1 256 6 256 287 1.1 34 321 +4 256 18 1024 1127 4.4 40 1167 +8 256 34 2048 2247 8.8 48 2295 +8 1024 119 8192 8476 33.1 48 8524 +16 1024 231 16384 16940 66.2 64 17004 +32 1024 455 32768 33868 132.3 96 33964 +64 1024 903 65536 67724 264.5 160 67884 +=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D= =3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D =3D=3D=3D=3D=3D=3D=3D=3D + +``region`` is ``region_slots``, the figure to offer; ``work MiB`` is +``work_slots`` in mebibytes and excludes ``reserved``. The +sixty-four-queue-pair row is 265.2 MiB of region against the 264.5 +``work_slots`` alone comes to. + +Three things to take from it. ``rx_fill`` dominates: it is 65536 slots of +the 67884 a sixty-four-queue-pair region comes to, so sizing a region for +multiple queues is "how many receive buffers will be posted" to within thr= ee +and a half percent, and the virtqueue rings are the smaller part of what is +left -- 903 slots against ``tx_min``'s 1280. ``reserved`` is small and gr= ows +with the virtqueue count rather than with the region -- 160 slots at +sixty-four queue pairs, under a quarter of one percent -- but it is not +optional, because it is withheld rather than merely spent. And the pool +areas of the previous subsection do not appear at all, because they cost +guest memory rather than region pages. =20 The receive buffer mode is a twenty-fold multiplier the device cannot predict. A driver that negotiates any of ``VIRTIO_NET_F_GUEST_TSO4``, @@ -432,6 +450,83 @@ which is 160.8 MiB instead of 8.8. A device offering = guest segmentation offload without ``MRG_RXBUF`` must size for that. Offering ``MRG_RXBUF`` is the better answer. =20 +The withheld range +------------------ + +Part of the pool is withheld from ordinary claims so that a virtqueue +whose ring is empty can publish a chain whatever the others have mapped:: + + reserved =3D 0 if pages < 256 + =3D min(32 + V - 1, pages / 2) otherwise, plus the last pool + area's page count where that + area is short enough to split + the range -- see below + +One page for each of the ``V`` virtqueues, and a chain's allowance of 32 +pages on top, less the one page already counted for whichever virtqueue +draws the chain. A virtqueue may draw on the range only while its ring is +empty, and only for one chain of up to 32 pages, which the ring asks for by +setting ``VIRTIO_MAP_ATTR_RESERVE`` on the mapping. That is the case worth +protecting: a virtqueue with nothing published has no completion of its own +to be woken by, because the device cannot signal a used buffer on a queue +with no buffers posted, so it depends entirely on its owner retrying. A +virtqueue that has published a chain does not need the guarantee, and does +not get it. + +Both terms follow from that restriction. Because a running virtqueue +cannot draw, the range is contended only by the virtqueues that have +published nothing; there are at most ``V`` of those, and what each of them +needs is the one page that takes its ring from empty to non-empty. The 31 +pages left over, plus that virtqueue's own one, are then a whole chain for +whichever of them is publishing more than a single page. + +So the guarantee is: every virtqueue can obtain one page that no running +virtqueue can take, and one virtqueue at a time can obtain a whole chain. +For virtio_net that is the whole of it at any queue count, because a +mergeable receive buffer is one slot and a linear transmit is one slot: the +resident draw is ``2 * Q + 5`` pages against the ``31 + V`` withheld, and +``2 * Q + 5 <=3D 2 * Q + 32`` holds for every ``Q``. What is *not* +guaranteed is ``V`` simultaneous multi-page first chains. A second +virtqueue whose first chain needs more than its one page competes for the +32 and, losing, gets ``-ENOMEM`` -- the value it got before the range +existed, and one every caller answers by retrying later rather than by +waiting for a completion it has not got. Sizing the range for ``V`` whole +chains instead would withhold ``32 * V``, which on a small region is a +fixed fraction of the pool and is capacity the transmit path then does not +have; that trades a rare mapping failure for a permanent shortage, so the +quantity is deliberately not the product of the two worst cases. + +The range is the tail of the pool, and an allocation has to lie inside one +pool area, so a whole chain needs 32 of the withheld pages contiguous with= in +one area and not merely free. Where the last pool area is shorter than 32 +pages the range straddles the boundary below it, and where the piece above +that boundary and the piece below are both shorter than a chain, the range +is widened by the short area's length so that the piece below it is a whole +``31 + V`` pages. That costs under 32 pages and only where ``pages`` is j= ust +above a multiple of ``area_pages``; no row of the table above reaches it, = and +neither does any region whose page count is a multiple of ``area_pages`` o= r at +least 32 above one. + +The guarantee holds for every virtqueue on any pool of 256 pages or more, = and +below that the range is inert altogether, because it would withhold more t= han +it protects. Above ninety-seven virtqueues the half-of-the-pool bound is = the +later of the two conditions, and the pool has to be ``62 + 2 * V`` pages or +more instead. Every row of the table above is far clear of both: the +tightest is 287 pages against the 68 three virtqueues need. That bound is +not a policy, only what keeps the ordinary range non-empty; where it does +bind, the pool has fewer pages than the virtqueues alone want and the +guarantee covers as many of them as it has pages for. + +This is a floor and not a fair share. A single busy virtqueue may still +use everything outside the withheld range: on a 16 MiB region with eight +queue pairs and depth-1024 rings that is 3928 of 4095 pages, the region +less the 48 withheld and the 119 the virtqueue areas hold. The range is +sized from every virtqueue the transport creates, which is the driver's +count plus an administration virtqueue where the device offers one, and +which over-counts a device that offers more queue pairs than the driver +uses; over-counting withholds one page per unused virtqueue and never +denies capacity. + An undersized multi-queue region fails in two ways, and both are properties of the network driver's existing behaviour rather than of the region. @@ -439,12 +534,16 @@ region. First, cross-queue starvation. ``virtnet_open()`` pre-fills the receive queues in index order and discards the result, so queue 0 takes what it needs before queue 1 asks. A region that cannot hold every queue's fill -leaves the later queues with no buffers at all; the device's receive -steering then drops whatever it sends to them, and transmit fails on every -queue with ``tx_fifo_errors``, ``tx_dropped`` and a rate-limited -``Unexpected TXQ`` message. Pool areas do not help: at -``virtnet_open()`` every fill runs on whichever CPU brought the link up, so -they all share one home area. Areas give preference, never reservation. +leaves the later queues with far fewer buffers than the earlier ones; the +device's receive steering then drops most of what it sends to them, and +transmit fails on those queues with ``tx_fifo_errors``, ``tx_dropped`` and +a rate-limited ``Unexpected TXQ`` message. Pool areas do not help: at +``virtnet_open()`` every fill runs on whichever CPU brought the link up, +so they all share one home area, and areas give preference rather than +reservation. The withheld range bounds how bad this gets -- no virtqueue +is left unable to publish anything at all -- but one buffer per queue is a +floor for forward progress, not a working receive ring. Sizing the region +for ``region_slots`` is what avoids it. =20 Second, a receive queue that holds no buffers and cannot refill spends softirq time without making progress. ``try_fill_recv()`` reports failure @@ -624,6 +723,14 @@ The claimed range also appears in ``/proc/iomem`` as appears nowhere else at all. The three counts appear again in ``dmb/pages``, ``dmb/areas`` and ``dmb/area_pages`` below. =20 +A second line follows when the driver creates its virtqueues, because the +count they come to is not known any earlier:: + + virtio_net virtio5: device memory buffer withholds 48 of 4095 pages for = 17 virtqueues + +That is the withheld range of the previous section, and the virtqueue count +it was sized from. It appears again in ``dmb/reserved_pages``. + With ``CONFIG_VIRTIO_DEBUG`` the state of the region is also available under the device's virtio debugfs directory, in ``dmb/``. The directory exists only while the device has a region, so a device that did not @@ -643,11 +750,20 @@ they accept may change or go away. ``area_pages`` divides ``pages``. Both are fixed when the region is installed and are the same two values the probe-time message prints. =20 +``reserved_pages`` + how many pages are withheld from ordinary claims so that a virtqueue who= se + ring is empty can publish a chain. Zero until the driver asks for its + virtqueues, and zero for good on a pool too small for the mechanism to + mean anything. Not subtracted from ``pages``: the withheld pages are + part of the pool and are counted in ``used_pages`` when a virtqueue draws + on them. + ``used_pages`` - how many of them are allocated. One counter maintained across all areas - rather than a sum of per-area figures read at different moments, so it - never reports a torn total; it is raised just outside the area lock, so a - read taken during a claim or a release can lag the bitmap by that claim. + how many of the pool's pages are allocated. One counter maintained + across all areas rather than a sum of per-area figures read at different + moments, so it never reports a torn total; it is raised just outside the + area lock, so a read taken during a claim or a release can lag the bitmap + by that claim. =20 ``used_pages_hiwater`` the largest ``used_pages`` has been. This, rather than a sample of diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index 67a7dc1a87df..628ce92edf81 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -204,6 +204,19 @@ config VIRTIO_DMB =20 If unsure, say Y. =20 +config VIRTIO_DMB_KUNIT_TEST + bool "Device Memory Buffer allocator tests" if !KUNIT_ALL_TESTS + depends on VIRTIO_DMB && KUNIT=3Dy + default KUNIT_ALL_TESTS + help + Tests for the geometry of the range the allocator withholds so that + a virtqueue with an empty ring can publish its first descriptor + chain, and for what becomes of a region when a negotiation drops + the feature. Neither case arises on a device anyone is likely to + build, so a test rather than a measurement is what keeps both true. + + If unsure, say N. + config VIRTIO_RTC tristate "Virtio RTC driver" depends on VIRTIO diff --git a/drivers/virtio/virtio_dmb.c b/drivers/virtio/virtio_dmb.c index fe556d585f24..773b5bb38dc6 100644 --- a/drivers/virtio/virtio_dmb.c +++ b/drivers/virtio/virtio_dmb.c @@ -128,6 +128,7 @@ struct virtio_dmb_area { * @area_slots: slots one area covers, a power of two; the last area covers * fewer when @nslots is not a multiple of it * @area_shift: ilog2(@area_slots), so slot >> @area_shift names its area + * @nvqs: virtqueues the transport created, which sizes the withheld range * @total_used: slots allocated across every area, exact; CONFIG_VIRTIO_DE= BUG * @used_hiwater: the largest @total_used has been since the last reset th= rough * debugfs, or since init; CONFIG_VIRTIO_DEBUG @@ -152,6 +153,7 @@ struct virtio_dmb { unsigned int nareas; unsigned int area_slots; unsigned int area_shift; + unsigned int nvqs; #ifdef CONFIG_VIRTIO_DEBUG /* * Exact occupancy for the two debugfs files, kept outside the area @@ -286,26 +288,116 @@ static void virtio_dmb_inc_alloc_failed(struct virti= o_dmb *dmb) #endif /* CONFIG_VIRTIO_DEBUG */ =20 /* - * Claim @nr contiguous slots from area @i, or -ENOMEM when that one area - * cannot satisfy the request. Takes and drops that area's lock and touch= es - * no other area's state, so no path ever holds two of these locks and the= re - * is no ordering between them to get right. + * Slots withheld from ordinary claims so that a virtqueue with an empty r= ing + * can publish a chain even when every other virtqueue has filled the rest= of + * the pool. A virtqueue with nothing published has no completion of its = own + * to be woken by, because the device cannot signal a used buffer on a que= ue + * with no buffers posted, so it depends entirely on its owner retrying; o= ne + * that has published a chain does not. + * + * One page for every virtqueue, and the rest of one chain's allowance on = top. + * Both terms follow from who can draw: only a virtqueue whose ring is emp= ty + * may, so the range is contended by the virtqueues that have published + * nothing and never by the ones that are running. At most nvqs of those + * exist, each needing the one page that takes its ring from empty to + * non-empty, and VIRTIO_MAP_RESERVE_PAGES less that one page is then what= is + * left for whichever of them is publishing a chain longer than a single p= age. + * + * Withholding a whole chain's allowance for every virtqueue instead would= be + * sizing for a state the device cannot be in, and it costs + * nvqs * VIRTIO_MAP_RESERVE_PAGES -- on a small pool a fixed fraction of = it, + * which is capacity the transmit path then does not have. That trades a + * mapping failure which is rare for a shortage which is permanent, so the + * quantity is deliberately not a product of the two worst cases. + * + * The range is the tail of the pool, but an allocation lies inside one ar= ea, + * so a whole chain needs VIRTIO_MAP_RESERVE_PAGES of the range contiguous + * within one area rather than merely free. Where the last area is short = the + * range straddles the boundary below it and the two pieces are the last + * area's length and the remainder; when both fall short of a chain, no ar= ea + * holds one however much of the range is free. Widening the range by the + * short area's length in that case moves its start down to the boundary, + * which gives the piece below a full VIRTIO_MAP_RESERVE_PAGES + nvqs - 1 + * slots. It costs the short area's length, which is under + * VIRTIO_MAP_RESERVE_PAGES because that is the case being tested for, and= it + * is reached only where nslots is just above a multiple of area_slots, so= no + * geometry in Documentation/driver-api/virtio/virtio-dmb.rst pays for it. + * + * Sized for every virtqueue the transport creates, which over-counts a de= vice + * offering more queue pairs than the driver uses. Over-counting withhold= s one + * page per unused virtqueue and never denies capacity. The half-of-the-p= ool + * bound is not part of the policy: it is what keeps the ordinary range + * non-empty, and where it binds the pool has fewer pages than the virtque= ues + * alone want, so the guarantee covers as many of them as it has pages for. + * Zero on a pool too small for the mechanism to mean anything, where it is + * inert rather than crippling, and zero until the count is known. + */ +static unsigned int virtio_dmb_reserved(const struct virtio_dmb *dmb) +{ + unsigned int nvqs =3D READ_ONCE(dmb->nvqs); + unsigned int last, reserved; + + if (!nvqs || dmb->nslots < 8 * VIRTIO_MAP_RESERVE_PAGES) + return 0; + + reserved =3D min(VIRTIO_MAP_RESERVE_PAGES + nvqs - 1, dmb->nslots / 2); + + /* + * reserved is at least VIRTIO_MAP_RESERVE_PAGES here, so the + * subtraction cannot wrap: the branch is taken only where last is + * below it. Both pieces short of a chain bounds reserved below 2 * + * VIRTIO_MAP_RESERVE_PAGES, and nslots is at least 8 of them, so the + * widened range is still inside the half-of-the-pool bound and leaves + * the ordinary range a whole area less the reserve. + */ + last =3D virtio_dmb_area_len(dmb, dmb->nareas - 1); + if (last < VIRTIO_MAP_RESERVE_PAGES && + reserved - last < VIRTIO_MAP_RESERVE_PAGES) + reserved +=3D last; + + return reserved; +} + +/* + * virtio_dmb_note_vqs() records the count that sizes the range above. It= is + * defined further down, next to the other entry points, because it has to= test + * vdev->map against this file's operations. + */ + +/* + * Claim @nr contiguous slots from area @i, bounded above at @end_max so t= hat + * an ordinary claim cannot reach the withheld range. Returns the first s= lot, + * or -ENOMEM when that one area cannot satisfy the request. Takes and dr= ops + * that area's lock and touches no other area's state, so no path ever hol= ds + * two of these locks and there is no ordering between them to get right. */ static long virtio_dmb_area_claim(struct virtio_dmb *dmb, unsigned int i, - unsigned int nr) + unsigned int nr, unsigned int end_max) { struct virtio_dmb_area *area =3D &dmb->areas[i]; unsigned int base =3D virtio_dmb_area_base(dmb, i); - unsigned int end =3D base + virtio_dmb_area_len(dmb, i); + unsigned int len =3D virtio_dmb_area_len(dmb, i); + unsigned int end =3D min(base + len, end_max); unsigned long flags, slot; =20 + if (end <=3D base) + return -ENOMEM; + spin_lock_irqsave(&area->lock, flags); =20 /* * Exact, and inside the lock. Written as a subtraction from the - * area's own length rather than as len - used < nr, which underflows. + * length being searched rather than as len - used < nr, which + * underflows. + * + * @area->used counts the whole area, including any withheld slots in + * use, so for the one area that straddles @end_max this is + * conservative: it can refuse an ordinary claim that area could have + * satisfied, and the walk then tries the next one. It cannot admit a + * claim the area could not satisfy. That is the only approximation + * here, and it is confined to at most one area out of @nareas. */ - if (nr > (end - base) - area->used) + if (area->used >=3D end - base || nr > (end - base) - area->used) goto not_found; =20 /* @@ -314,9 +406,13 @@ static long virtio_dmb_area_claim(struct virtio_dmb *d= mb, unsigned int i, * index. bitmap_find_next_zero_area() returns a value whose sum with * @nr exceeds the size it was given when it finds nothing, so that sum * is the test; the whole-pool "slot >=3D nslots" form does not transfer. + * + * @area->index is relative to the whole area, so it can point past a + * clamped @end; starting the sweep there would search nothing, hence + * the min(). The second sweep from the base then covers the range. */ slot =3D bitmap_find_next_zero_area(dmb->bitmap, end, - base + area->index, nr, 0); + min(base + area->index, end), nr, 0); if (slot + nr > end && area->index) slot =3D bitmap_find_next_zero_area(dmb->bitmap, end, base, nr, 0); @@ -325,7 +421,7 @@ static long virtio_dmb_area_claim(struct virtio_dmb *dm= b, unsigned int i, =20 bitmap_set(dmb->bitmap, slot, nr); area->used +=3D nr; - area->index =3D slot + nr < end ? slot + nr - base : 0; + area->index =3D slot + nr < base + len ? slot + nr - base : 0; =20 spin_unlock_irqrestore(&area->lock, flags); =20 @@ -343,9 +439,39 @@ static long virtio_dmb_area_claim(struct virtio_dmb *d= mb, unsigned int i, return -ENOMEM; } =20 +/* One pass over every area, each bounded at @end_max. */ +static long virtio_dmb_walk(struct virtio_dmb *dmb, unsigned int nr, + unsigned int end_max) +{ + unsigned int i, start; + long ret; + + /* + * raw_smp_processor_id() and not smp_processor_id(): the index is + * computed before any lock is taken, so preemption or migration + * between the read and the claim only changes which area is tried + * first. kernel/dma/swiotlb.c picks its home area on the same + * reasoning. + */ + start =3D raw_smp_processor_id() % dmb->nareas; + i =3D start; + do { + ret =3D virtio_dmb_area_claim(dmb, i, nr, end_max); + if (ret >=3D 0) + return ret; + + if (++i >=3D dmb->nareas) + i =3D 0; + } while (i !=3D start); + + return -ENOMEM; +} + /* * Claim @nr contiguous slots. Returns the first slot, or -ENOMEM when no - * area can satisfy the request. Exhaustion is a routine condition: the + * area can satisfy the request. @reserved permits the withheld tail of t= he + * pool, and is set only for a mapping of the first chain a virtqueue is + * publishing. Exhaustion is a routine condition: the * region's length bounds how much virtqueue data can be in flight. What a * caller makes of it is the caller's, and it is not always back-pressure:= a * network receive fill has nothing to push back on when it cannot post a @@ -371,28 +497,32 @@ static long virtio_dmb_area_claim(struct virtio_dmb *= dmb, unsigned int i, * bounds the interrupts-off window to a single area's sweep; the total wo= rk in * the failing case is a whole-pool sweep either way. */ -static long virtio_dmb_claim(struct virtio_dmb *dmb, unsigned int nr) +static long virtio_dmb_claim(struct virtio_dmb *dmb, unsigned int nr, + bool reserved) { - unsigned int i, start; long ret; =20 /* - * raw_smp_processor_id() and not smp_processor_id(): the index is - * computed before any lock is taken, so preemption or migration - * between the read and the claim only changes which area is tried - * first. kernel/dma/swiotlb.c picks its home area on the same - * reasoning. + * The bitmap is its own accounting for the withheld range: bounding + * the search is what bounds the sum, so no counter is added to a + * production build's hot path. */ - start =3D raw_smp_processor_id() % dmb->nareas; - i =3D start; - do { - ret =3D virtio_dmb_area_claim(dmb, i, nr); + ret =3D virtio_dmb_walk(dmb, nr, dmb->nslots - virtio_dmb_reserved(dmb)); + if (ret >=3D 0) + return ret; + + /* + * The second walk runs only once the first has failed in every area, + * so the withheld range is a last resort rather than a second pool. + * That is not the same as "only when the pool is full": next fit can + * fail on fragmentation while capacity remains, and this reaches the + * withheld range then too. + */ + if (reserved) { + ret =3D virtio_dmb_walk(dmb, nr, dmb->nslots); if (ret >=3D 0) return ret; - - if (++i >=3D dmb->nareas) - i =3D 0; - } while (i !=3D start); + } =20 /* * The geometry rather than a free count: there is no instant at which @@ -733,7 +863,13 @@ static void *virtio_dmb_op_alloc(union virtio_map map,= size_t size, goto no_room; =20 nr =3D virtio_dmb_slots(size); - ret =3D virtio_dmb_claim(dmb, nr); + /* + * false: a virtqueue area is structural, claimed when a queue is + * created or resized and never under back-pressure, so letting it into + * the withheld range would consume the reserve for the life of the + * queue rather than for one chain. + */ + ret =3D virtio_dmb_claim(dmb, nr, false); if (ret < 0) goto no_room; slot =3D ret; @@ -811,7 +947,7 @@ static dma_addr_t virtio_dmb_op_map_page(union virtio_m= ap map, return DMA_MAPPING_ERROR; =20 nr =3D virtio_dmb_slots(size); - ret =3D virtio_dmb_claim(dmb, nr); + ret =3D virtio_dmb_claim(dmb, nr, attrs & VIRTIO_MAP_ATTR_RESERVE); if (ret < 0) { /* * Counted here rather than in virtio_dmb_claim(), which @@ -1023,6 +1159,21 @@ static int virtio_dmb_alloc_failed_get(void *data, u= 64 *val) DEFINE_DEBUGFS_ATTRIBUTE(virtio_dmb_alloc_failed_fops, virtio_dmb_alloc_failed_get, NULL, "%llu\n"); =20 +/* + * A getter rather than debugfs_create_u32(), because the value is derived + * from @nvqs and the pool size rather than stored. + */ +static int virtio_dmb_reserved_get(void *data, u64 *val) +{ + struct virtio_dmb *dmb =3D data; + + *val =3D virtio_dmb_reserved(dmb); + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(virtio_dmb_reserved_fops, virtio_dmb_reserved_get, + NULL, "%llu\n"); + /* * The files go under the device's existing virtio debugfs directory, and = exist * only while the device has a region. They describe one, so their presen= ce is @@ -1049,6 +1200,8 @@ static void virtio_dmb_debugfs_init(struct virtio_dmb= *dmb) &virtio_dmb_hiwater_fops); debugfs_create_file("alloc_failed", 0400, dir, dmb, &virtio_dmb_alloc_failed_fops); + debugfs_create_file("reserved_pages", 0400, dir, dmb, + &virtio_dmb_reserved_fops); } =20 static void virtio_dmb_debugfs_exit(struct virtio_dmb *dmb) @@ -1068,6 +1221,40 @@ static void virtio_dmb_debugfs_exit(struct virtio_dm= b *dmb) =20 #endif /* CONFIG_VIRTIO_DEBUG */ =20 +/** + * virtio_dmb_note_vqs - record how many virtqueues the transport created + * @vdev: the device + * @nvqs: virtqueues about to be created + * + * Sizes the range withheld so that a virtqueue whose ring is empty can + * publish a chain. Does nothing unless @vdev is using a Device Memory + * Buffer. + * + * Called before the virtqueues exist, so the ring allocations that follow= are + * ordinary claims and cannot land in the withheld range; and @nvqs theref= ore + * only ever changes while the device has no virtqueues and so no mappings. + * WRITE_ONCE() because the claim path reads it without any lock. + */ +void virtio_dmb_note_vqs(struct virtio_device *vdev, unsigned int nvqs) +{ + struct virtio_dmb *dmb; + + if (vdev->map !=3D &virtio_dmb_map_ops) + return; + + dmb =3D vdev->vmap.dmb; + WRITE_ONCE(dmb->nvqs, nvqs); + + /* + * Reported here rather than from virtio_dmb_init(), which runs during + * feature negotiation and cannot know the count. One line per probe. + */ + dev_info(&vdev->dev, + "device memory buffer withholds %u of %u pages for %u virtqueues\n", + virtio_dmb_reserved(dmb), dmb->nslots, nvqs); +} +EXPORT_SYMBOL_GPL(virtio_dmb_note_vqs); + /* * Whether the device still has virtqueues. vqs_list_lock is what protects * that list against a concurrent adder. No caller here can race one, bec= ause @@ -1524,5 +1711,9 @@ int virtio_dmb_init(struct virtio_device *vdev) } EXPORT_SYMBOL_GPL(virtio_dmb_init); =20 +#if IS_ENABLED(CONFIG_VIRTIO_DMB_KUNIT_TEST) +#include "virtio_dmb_test.c" +#endif + MODULE_DESCRIPTION("Virtio device memory buffer allocator"); MODULE_LICENSE("GPL"); diff --git a/drivers/virtio/virtio_dmb.h b/drivers/virtio/virtio_dmb.h index 69fbcbb9c2c0..38e4c3ecc9cb 100644 --- a/drivers/virtio/virtio_dmb.h +++ b/drivers/virtio/virtio_dmb.h @@ -11,6 +11,7 @@ struct virtio_device; =20 int virtio_dmb_init(struct virtio_device *vdev); void virtio_dmb_destroy(struct virtio_device *vdev); +void virtio_dmb_note_vqs(struct virtio_device *vdev, unsigned int nvqs); =20 #else =20 @@ -23,6 +24,11 @@ static inline void virtio_dmb_destroy(struct virtio_devi= ce *vdev) { } =20 +static inline void virtio_dmb_note_vqs(struct virtio_device *vdev, + unsigned int nvqs) +{ +} + #endif /* CONFIG_VIRTIO_DMB */ =20 #endif /* _DRIVERS_VIRTIO_VIRTIO_DMB_H */ diff --git a/drivers/virtio/virtio_dmb_test.c b/drivers/virtio/virtio_dmb_t= est.c new file mode 100644 index 000000000000..d9dff81a8eb3 --- /dev/null +++ b/drivers/virtio/virtio_dmb_test.c @@ -0,0 +1,279 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Tests for the geometry of the withheld range, and for what becomes of a + * region when a negotiation drops the feature. + * + * Included by virtio_dmb.c rather than compiled on its own, so that the t= ests + * reach its static functions and its private structure without widening + * either into a header for their benefit. + * + * The geometry cases test a property rather than a value: that whatever t= he + * pool size, the area size and the virtqueue count, the withheld range ho= lds + * VIRTIO_MAP_RESERVE_PAGES slots that are contiguous *within one area*. = That + * qualifier is the whole point. An allocation cannot span two areas, so a + * range that is merely large enough does not guarantee a chain, and a pool + * whose last area is short can withhold a range whose two pieces are each= too + * small. A region has to be just above a multiple of the area size to re= ach + * it, which no plausible device offers and so no test on real hardware + * exercises. + */ +#include + +/* Geometry only: virtio_dmb_reserved() reads no more of the structure. */ +static void dmb_test_shape(struct kunit *test, struct virtio_dmb *dmb, + unsigned int nslots, unsigned int area_slots, + unsigned int nvqs) +{ + KUNIT_ASSERT_TRUE(test, is_power_of_2(area_slots)); + + dmb->nslots =3D nslots; + dmb->area_slots =3D area_slots; + dmb->area_shift =3D ilog2(area_slots); + dmb->nareas =3D DIV_ROUND_UP(nslots, area_slots); + dmb->nvqs =3D nvqs; +} + +static struct virtio_dmb *dmb_test_pool(struct kunit *test) +{ + struct virtio_dmb *dmb =3D kunit_kzalloc(test, sizeof(*dmb), GFP_KERNEL); + + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dmb); + + return dmb; +} + +/* + * The longest run of withheld slots that lies inside a single area, which= is + * the largest chain the range can serve. + */ +static unsigned int dmb_test_longest_run(const struct virtio_dmb *dmb, + unsigned int reserved) +{ + unsigned int i, best =3D 0; + unsigned int start; + + if (!reserved) + return 0; + + start =3D dmb->nslots - reserved; + + for (i =3D 0; i < dmb->nareas; i++) { + unsigned int base =3D virtio_dmb_area_base(dmb, i); + unsigned int end =3D base + virtio_dmb_area_len(dmb, i); + unsigned int lo =3D max(start, base); + + if (end > lo) + best =3D max(best, end - lo); + } + + return best; +} + +static void dmb_test_assert_guarantees(struct kunit *test, + const struct virtio_dmb *dmb) +{ + unsigned int reserved =3D virtio_dmb_reserved(dmb); + unsigned int run; + + if (!reserved) + return; + + /* One virtqueue at a time can obtain a whole chain. */ + run =3D dmb_test_longest_run(dmb, reserved); + KUNIT_EXPECT_GE_MSG(test, run, VIRTIO_MAP_RESERVE_PAGES, + "nslots=3D%u area_slots=3D%u nareas=3D%u nvqs=3D%u reserved=3D%u", + dmb->nslots, dmb->area_slots, dmb->nareas, + dmb->nvqs, reserved); + + /* Every virtqueue can obtain one page no running virtqueue can take. */ + KUNIT_EXPECT_GE_MSG(test, reserved, + min(dmb->nvqs, dmb->nslots / 2), + "nslots=3D%u nvqs=3D%u reserved=3D%u", + dmb->nslots, dmb->nvqs, reserved); + + /* The range never takes more than half the pool, nor all of it. */ + KUNIT_EXPECT_LE_MSG(test, reserved, dmb->nslots / 2, + "nslots=3D%u nvqs=3D%u reserved=3D%u", + dmb->nslots, dmb->nvqs, reserved); +} + +/* + * Every area size the allocator can derive, against pool sizes that put t= he + * last area at every length a straddle needs, and virtqueue counts either + * side of a chain's allowance. + */ +static void dmb_test_reserve_geometry(struct kunit *test) +{ + static const unsigned int area_sizes[] =3D { 512, 1024, 2048, 4096 }; + struct virtio_dmb *dmb =3D dmb_test_pool(test); + unsigned int a, k, tail, nvqs; + + for (a =3D 0; a < ARRAY_SIZE(area_sizes); a++) { + unsigned int area_slots =3D area_sizes[a]; + + for (k =3D 1; k <=3D 3; k++) { + for (tail =3D 0; tail <=3D 2 * VIRTIO_MAP_RESERVE_PAGES; + tail++) { + unsigned int nslots =3D k * area_slots + tail; + + for (nvqs =3D 1; nvqs <=3D 40; nvqs++) { + dmb_test_shape(test, dmb, nslots, + area_slots, nvqs); + dmb_test_assert_guarantees(test, dmb); + } + } + } + } +} + +/* + * The geometries that showed the guarantee was conditional. Named so tha= t a + * revision that reintroduces the pool-tail bound fails here rather than in + * the sweep, where the reason is harder to read off. + */ +static void dmb_test_reserve_straddle(struct kunit *test) +{ + static const struct { + unsigned int nslots, area_slots, nvqs; + } cases[] =3D { + { 528, 512, 3 }, /* last area 16, pieces 18 and 16 */ + { 527, 512, 1 }, /* last area 15, pieces 17 and 15 */ + { 4114, 4096, 7 }, /* last area 18, pieces 20 and 18 */ + { 4114, 1024, 7 }, /* same tail, more areas */ + { 1048600, 512, 17 }, /* last area 24, pieces 24 and 24 */ + { 4097, 4096, 17 }, /* last area 1, but 47 below it: fine */ + { 2100, 512, 7 }, /* last area 52: whole range fits it */ + }; + struct virtio_dmb *dmb =3D dmb_test_pool(test); + unsigned int i; + + for (i =3D 0; i < ARRAY_SIZE(cases); i++) { + dmb_test_shape(test, dmb, cases[i].nslots, cases[i].area_slots, + cases[i].nvqs); + dmb_test_assert_guarantees(test, dmb); + } +} + +/* + * The sizing table in Documentation/driver-api/virtio/virtio-dmb.rst stat= es a + * withheld count for each of its rows, and a device implementer sizes aga= inst + * it. Every row's last area is long enough to hold the range, so the area + * term must not change any of them. + */ +static void dmb_test_reserve_documented_sizing(struct kunit *test) +{ + static const struct { + unsigned int nslots, nvqs, reserved; + } rows[] =3D { + { 321, 3, 34 }, + { 1167, 9, 40 }, + { 2295, 17, 48 }, + { 8524, 17, 48 }, + { 17004, 33, 64 }, + { 33964, 65, 96 }, + { 67884, 129, 160 }, + }; + static const unsigned int area_sizes[] =3D { 512, 1024, 2048, 4096 }; + struct virtio_dmb *dmb =3D dmb_test_pool(test); + unsigned int i, a; + + for (i =3D 0; i < ARRAY_SIZE(rows); i++) { + for (a =3D 0; a < ARRAY_SIZE(area_sizes); a++) { + dmb_test_shape(test, dmb, rows[i].nslots, + area_sizes[a], rows[i].nvqs); + + KUNIT_EXPECT_EQ_MSG(test, virtio_dmb_reserved(dmb), + rows[i].reserved, + "nslots=3D%u nvqs=3D%u area_slots=3D%u", + rows[i].nslots, rows[i].nvqs, + area_sizes[a]); + dmb_test_assert_guarantees(test, dmb); + } + } +} + +/* Inert below the threshold, and before the transport reports a count. */ +static void dmb_test_reserve_inert(struct kunit *test) +{ + struct virtio_dmb *dmb =3D dmb_test_pool(test); + unsigned int nslots; + + dmb_test_shape(test, dmb, 4096, 512, 0); + KUNIT_EXPECT_EQ(test, virtio_dmb_reserved(dmb), 0); + + for (nslots =3D 4; nslots < 8 * VIRTIO_MAP_RESERVE_PAGES; nslots++) { + dmb_test_shape(test, dmb, nslots, 512, 3); + KUNIT_EXPECT_EQ_MSG(test, virtio_dmb_reserved(dmb), 0, + "nslots=3D%u", nslots); + } + + dmb_test_shape(test, dmb, 8 * VIRTIO_MAP_RESERVE_PAGES, 512, 1); + KUNIT_EXPECT_EQ(test, virtio_dmb_reserved(dmb), + VIRTIO_MAP_RESERVE_PAGES); +} + +/* + * What virtio_dmb_init() does with state an earlier negotiation left behi= nd + * when this one did not accept the feature: releases it, and refuses to + * release it under a live virtqueue, which holds kernel addresses inside = the + * mapping. The refusal is the only error it returns for a device it is t= aking + * the region away from, so it is the one a caller turns into the FAILED s= tatus + * bit; releasing it is not an error and sets nothing. Nothing can reach = the + * refusal, for the reason the function itself gives, so a test is what co= vers + * it. Its dev_warn() is expected output. + */ +static void dmb_test_withdrawn_feature(struct kunit *test) +{ + struct virtio_device *vdev; + const struct virtio_map_ops *prev; + struct virtqueue *vq; + struct virtio_dmb *dmb; + + vdev =3D kunit_kzalloc(test, sizeof(*vdev), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vdev); + vq =3D kunit_kzalloc(test, sizeof(*vq), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vq); + prev =3D kunit_kzalloc(test, sizeof(*prev), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, prev); + + spin_lock_init(&vdev->vqs_list_lock); + INIT_LIST_HEAD(&vdev->vqs); + + /* Nothing installed: nothing to release, and not an error. */ + KUNIT_EXPECT_EQ(test, virtio_dmb_init(vdev), 0); + + /* Not kunit_kzalloc(): the last call below frees this. */ + dmb =3D kzalloc_obj(*dmb, GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dmb); + dmb->prev_map =3D prev; + vdev->map =3D &virtio_dmb_map_ops; + vdev->vmap.dmb =3D dmb; + list_add(&vq->list, &vdev->vqs); + + /* Installed, under a virtqueue: refused, and left addressable. */ + KUNIT_EXPECT_EQ(test, virtio_dmb_init(vdev), -EBUSY); + KUNIT_EXPECT_PTR_EQ(test, vdev->map, &virtio_dmb_map_ops); + KUNIT_EXPECT_PTR_EQ(test, vdev->vmap.dmb, dmb); + + /* The same withdrawal with no virtqueue left: released, and no error. */ + list_del(&vq->list); + KUNIT_EXPECT_EQ(test, virtio_dmb_init(vdev), 0); + KUNIT_EXPECT_PTR_EQ(test, vdev->map, prev); +} + +static struct kunit_case virtio_dmb_test_cases[] =3D { + /* Slow: the sweep is tens of thousands of geometries. */ + KUNIT_CASE_SLOW(dmb_test_reserve_geometry), + KUNIT_CASE(dmb_test_reserve_straddle), + KUNIT_CASE(dmb_test_reserve_documented_sizing), + KUNIT_CASE(dmb_test_reserve_inert), + KUNIT_CASE(dmb_test_withdrawn_feature), + {} +}; + +static struct kunit_suite virtio_dmb_test_suite =3D { + .name =3D "virtio_dmb", + .test_cases =3D virtio_dmb_test_cases, +}; + +kunit_test_suite(virtio_dmb_test_suite); diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci= _modern.c index c43c1fc6e843..635ff8012f4c 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -19,6 +19,7 @@ #define VIRTIO_PCI_NO_LEGACY #define VIRTIO_RING_NO_LEGACY #include "virtio_pci_common.h" +#include "virtio_dmb.h" =20 #define VIRTIO_AVQ_SGS_MAX 4 =20 @@ -803,8 +804,23 @@ static int vp_modern_find_vqs(struct virtio_device *vd= ev, unsigned int nvqs, { struct virtio_pci_device *vp_dev =3D to_vp_device(vdev); struct virtqueue *vq; - int rc =3D vp_find_vqs(vdev, nvqs, vqs, vqs_info, desc); + int rc; =20 + /* + * Before vp_find_vqs(), so that no virtqueue exists yet: the ring + * allocations it makes are ordinary claims and cannot land in the + * range this withholds. + * + * nvqs counts the virtqueues the driver asked for. vp_find_vqs() + * creates one more when the device offers an administration + * virtqueue, and that one draws on the guarantee like any other -- its + * only response to a refusal is a cpu_relax() spin -- so count it here + * rather than leave it as the one virtqueue without a floor. + */ + virtio_dmb_note_vqs(vdev, nvqs + + virtio_has_feature(vdev, VIRTIO_F_ADMIN_VQ)); + + rc =3D vp_find_vqs(vdev, nvqs, vqs, vqs_info, desc); if (rc) return rc; =20 diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 641ab07be931..6cd82d418c37 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -527,10 +527,44 @@ static int vring_map_errno(const struct vring_virtque= ue *vq) return -ENOMEM; } =20 +/* + * Whether this mapping may draw on capacity a bounded map implementation + * withholds, and account it against the chain's allowance if so. + * + * Only while the ring is empty, which is exactly the window in which this + * virtqueue has no completion of its own to be woken by: the device cannot + * signal a used buffer on a queue with no buffers posted. Every + * virtqueue_add_*() path decrements num_free after its mapping loop, so t= he + * test is true for every mapping of a chain going into an empty ring and + * false for every mapping after it, with nothing to keep in step. + * + * @chain_pages accumulates within one virtqueue_add_*() call and dies with + * it. It is charged whether or not the mapping later fails, and a chain = that + * fails half way therefore over-counts its own allowance and nothing else= , so + * no bookkeeping can leak. + */ +static unsigned long vring_map_reserve_attr(const struct vring_virtqueue *= vq, + unsigned int *chain_pages, + size_t size) +{ + unsigned int pages =3D DIV_ROUND_UP(size, PAGE_SIZE); + + if (*chain_pages + pages > VIRTIO_MAP_RESERVE_PAGES) + return 0; + + if (vq->vq.num_free !=3D vring_num(vq)) + return 0; + + *chain_pages +=3D pages; + + return VIRTIO_MAP_ATTR_RESERVE; +} + /* Map one sg entry. */ static int vring_map_one_sg(const struct vring_virtqueue *vq, struct scatt= erlist *sg, enum dma_data_direction direction, dma_addr_t *addr, - u32 *len, bool premapped, unsigned long attr) + u32 *len, bool premapped, unsigned long attr, + unsigned int *chain_pages) { if (premapped) { *addr =3D sg_dma_address(sg); @@ -577,7 +611,9 @@ static int vring_map_one_sg(const struct vring_virtqueu= e *vq, struct scatterlist */ *addr =3D virtqueue_map_page_attrs(&vq->vq, sg_page(sg), sg->offset, sg->length, - direction, attr); + direction, + attr | vring_map_reserve_attr(vq, chain_pages, + sg->length)); =20 if (vring_mapping_error(vq, *addr)) return vring_map_errno(vq); @@ -587,13 +623,15 @@ static int vring_map_one_sg(const struct vring_virtqu= eue *vq, struct scatterlist =20 static dma_addr_t vring_map_single(const struct vring_virtqueue *vq, void *cpu_addr, size_t size, - enum dma_data_direction direction) + enum dma_data_direction direction, + unsigned int *chain_pages) { if (!vq->use_map_api) return (dma_addr_t)virt_to_phys(cpu_addr); =20 - return virtqueue_map_single_attrs(&vq->vq, cpu_addr, - size, direction, 0); + return virtqueue_map_single_attrs(&vq->vq, cpu_addr, size, direction, + vring_map_reserve_attr(vq, chain_pages, + size)); } =20 static void virtqueue_init(struct vring_virtqueue *vq, u32 num) @@ -717,6 +755,7 @@ static inline int virtqueue_add_split(struct vring_virt= queue *vq, unsigned int i, n, avail, descs_used, err_idx, sg_count =3D 0; /* Total length for in-order */ unsigned int total_in_len =3D 0; + unsigned int chain_pages =3D 0; int head; bool indirect; int err; @@ -783,7 +822,8 @@ static inline int virtqueue_add_split(struct vring_virt= queue *vq, flags |=3D VRING_DESC_F_NEXT; =20 err =3D vring_map_one_sg(vq, sg, DMA_TO_DEVICE, &addr, - &len, premapped, attr); + &len, premapped, attr, + &chain_pages); if (err) goto unmap_release; =20 @@ -804,7 +844,8 @@ static inline int virtqueue_add_split(struct vring_virt= queue *vq, flags |=3D VRING_DESC_F_NEXT; =20 err =3D vring_map_one_sg(vq, sg, DMA_FROM_DEVICE, &addr, - &len, premapped, attr); + &len, premapped, attr, + &chain_pages); if (err) goto unmap_release; =20 @@ -821,7 +862,8 @@ static inline int virtqueue_add_split(struct vring_virt= queue *vq, /* Now that the indirect table is filled in, map it. */ dma_addr_t addr =3D vring_map_single( vq, desc, total_sg * sizeof(struct vring_desc), - DMA_TO_DEVICE); + DMA_TO_DEVICE, + &chain_pages); if (vring_mapping_error(vq, addr)) { err =3D vring_map_errno(vq); goto unmap_release; @@ -1601,6 +1643,7 @@ static int virtqueue_add_indirect_packed(struct vring= _virtqueue *vq, struct vring_packed_desc *desc; struct scatterlist *sg; unsigned int i, n, err_idx, len, total_in_len =3D 0; + unsigned int chain_pages =3D 0; u16 head; dma_addr_t addr; =20 @@ -1624,7 +1667,8 @@ static int virtqueue_add_indirect_packed(struct vring= _virtqueue *vq, for (sg =3D sgs[n]; sg; sg =3D sg_next(sg)) { if (vring_map_one_sg(vq, sg, n < out_sgs ? DMA_TO_DEVICE : DMA_FROM_DEVICE, - &addr, &len, premapped, attr)) + &addr, &len, premapped, attr, + &chain_pages)) goto unmap_release; =20 desc[i].flags =3D cpu_to_le16(n < out_sgs ? @@ -1647,7 +1691,8 @@ static int virtqueue_add_indirect_packed(struct vring= _virtqueue *vq, /* Now that the indirect table is filled in, map it. */ addr =3D vring_map_single(vq, desc, total_sg * sizeof(struct vring_packed_desc), - DMA_TO_DEVICE); + DMA_TO_DEVICE, + &chain_pages); if (vring_mapping_error(vq, addr)) goto unmap_release; =20 @@ -1740,6 +1785,7 @@ static inline int virtqueue_add_packed(struct vring_v= irtqueue *vq, struct vring_packed_desc *desc; struct scatterlist *sg; unsigned int i, n, c, descs_used, err_idx, len; + unsigned int chain_pages =3D 0; __le16 head_flags, flags; u16 head, id, prev, curr, avail_used_flags; int err; @@ -1799,7 +1845,8 @@ static inline int virtqueue_add_packed(struct vring_v= irtqueue *vq, =20 err =3D vring_map_one_sg(vq, sg, n < out_sgs ? DMA_TO_DEVICE : DMA_FROM_DEVICE, - &addr, &len, premapped, attr); + &addr, &len, premapped, attr, + &chain_pages); if (err) goto unmap_release; =20 @@ -1899,6 +1946,7 @@ static inline int virtqueue_add_packed_in_order(struc= t vring_virtqueue *vq, struct vring_packed_desc *desc; struct scatterlist *sg; unsigned int i, n, sg_count, err_idx, total_in_len =3D 0; + unsigned int chain_pages =3D 0; __le16 head_flags, flags; u16 head, avail_used_flags; bool avail_wrap_counter; @@ -1961,7 +2009,8 @@ static inline int virtqueue_add_packed_in_order(struc= t vring_virtqueue *vq, =20 err =3D vring_map_one_sg(vq, sg, n < out_sgs ? DMA_TO_DEVICE : DMA_FROM_DEVICE, - &addr, &len, premapped, attr); + &addr, &len, premapped, attr, + &chain_pages); if (err) goto unmap_release; =20 @@ -3880,9 +3929,15 @@ dma_addr_t virtqueue_map_page_attrs(const struct vir= tqueue *_vq, page, offset, size, dir, attrs); =20 + /* + * Strip the bits virtio owns before the DMA API sees them: it defines + * DMA_ATTR_* over the same word, and a bit outside that set has no + * defined meaning there. A map implementation is the only reader of + * them. + */ return dma_map_page_attrs(vring_dma_dev(vq), page, offset, size, - dir, attrs); + dir, attrs & ~VIRTIO_MAP_ATTR_MASK); } EXPORT_SYMBOL_GPL(virtqueue_map_page_attrs); =20 @@ -3907,7 +3962,8 @@ void virtqueue_unmap_page_attrs(const struct virtqueu= e *_vq, map_handle, size, dir, attrs); else dma_unmap_page_attrs(vring_dma_dev(vq), map_handle, - size, dir, attrs); + size, dir, + attrs & ~VIRTIO_MAP_ATTR_MASK); } EXPORT_SYMBOL_GPL(virtqueue_unmap_page_attrs); =20 diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index a6780aa85966..a25552862ec4 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -3,6 +3,7 @@ #define _LINUX_VIRTIO_CONFIG_H =20 #include +#include #include #include #include @@ -219,6 +220,38 @@ struct virtio_map_ops { size_t (*max_mapping_size)(union virtio_map map); }; =20 +/* + * Pages one virtqueue at a time is guaranteed to be able to map through a= map + * implementation with a bounded pool, whatever the other virtqueues of the + * device have mapped. Enough for one page-granular descriptor chain at t= he + * default CONFIG_MAX_SKB_FRAGS: a network receive buffer in the non-merge= able + * case is MAX_SKB_FRAGS + 2 scatterlist entries plus an indirect table, w= hich + * is 20 pages where a page is 4 KiB and MAX_SKB_FRAGS is 17. A chain lar= ger + * than this -- a raised CONFIG_MAX_SKB_FRAGS, or entries spanning more th= an a + * page each -- draws on the reserve for as much of itself as fits and is = not + * guaranteed. Neither is a second chain of more than one page concurrent + * with the first: what every virtqueue is guaranteed is the single page t= hat + * takes its ring from empty to non-empty. Both hold on any pool of eight + * times this many pages or more, which is where an implementation withhol= ds + * anything at all; below that there is no range and no guarantee. + */ +#define VIRTIO_MAP_RESERVE_PAGES 32u + +/* + * map_page() attrs bits owned by virtio rather than by the DMA API. + * DMA_ATTR_* occupies bits 1 to 13 today; these sit above it, and + * virtqueue_map_page_attrs() masks them off before any dma_map_*() call, = so + * the DMA API never sees one. The gap is deliberate headroom rather than= a + * partition: whoever grows either range has to check the other. + * + * VIRTIO_MAP_ATTR_RESERVE: this mapping is part of the first descriptor c= hain + * a virtqueue is publishing, and an implementation with a bounded pool sh= ould + * satisfy it from capacity withheld for that purpose if it has no other. + * An implementation that ignores the bit behaves exactly as before. + */ +#define VIRTIO_MAP_ATTR_RESERVE BIT(24) +#define VIRTIO_MAP_ATTR_MASK VIRTIO_MAP_ATTR_RESERVE + /* If driver didn't advertise the feature, it will never appear. */ void virtio_check_driver_offered_feature(const struct virtio_device *vdev, unsigned int fbit);