Received: by 2002:ab2:620c:0:b0:1ef:ffd0:ce49 with SMTP id o12csp1059626lqt; Tue, 19 Mar 2024 11:22:18 -0700 (PDT) X-Forwarded-Encrypted: i=3; AJvYcCW8raNKA1HMIfoqkUUcwbNK0uslH9B+MQqv28gmW49Yuk6LstEIxxWJZALrJonwAdGb2E3kPFGZ/BF8oZ8fqcwr6FDaYueGkDiZH4SgSQ== X-Google-Smtp-Source: AGHT+IFChYVDye62tpbwfRzcfY4Ye9o7pRFsjgRJoYDtbA2zjAerchtK0yMdBwdg/XAM4P+1cmeW X-Received: by 2002:a17:906:bc46:b0:a46:b4c7:341c with SMTP id s6-20020a170906bc4600b00a46b4c7341cmr6001883ejv.58.1710872538813; Tue, 19 Mar 2024 11:22:18 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1710872538; cv=pass; d=google.com; s=arc-20160816; b=Hqgycuj8976c0W/entD2iL7BwEslXWpIrxbvYIuafKa2vSWJegoGjax1x5DLRGBg+6 8gJ2q1AnDFK4Kw9eL854BYYyD3td9B1igy99+0u/idH/CSu3wHWBbwc5/iggMcJ2K0hr Brr0jlfMHopf/TvliaB/r6Mo6H3TXjKHvZNXawtREgT0CRRIUDNR0/wceaoN4kFZfEAp qb6BycUFgMiSRhHSmnH1vHxpjeGOoNouZ4NkCwcMKHsBvqNi1kK1mXBvRL2dQgLF/d3y Cy/JOm5iLf1O8cQmaY1hCX65AuCBAo9C9xAvNl9L5bpalAi79phjXJv63Ovs6OV/+hoi 9OtA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=user-agent:in-reply-to:content-disposition:mime-version :list-unsubscribe:list-subscribe:list-id:precedence:references :message-id:subject:cc:to:from:date:dkim-signature; bh=EfJD980t6zGVKH04G9oNk3ciAa+P3hbVg0d6cdriU34=; fh=zVY+X32okTgS4QW9mjDYQAFCqUEsWLr+G0fguLDm7HA=; b=uMH8f57fwfLsNp2h0f+5zo/Qvyw+S1rK4XShCbCGCTrszAQwLUkeQwyg8BZwUZ0FuG LW3rMjhUgblF8S5g1rfvg/wGRAOxfu9gV/12ORm4wam2yhha89EM5Nf8SzzzyddrLD/G u8/aK0TzhHgdMCTKCifKg9tuQtJhM+JoqJYovqWb7yunvZ6fodD4E7cDc+VRSR1eSipM SnHIzHjhu/t/2/fd8FYC17jWRpgyXCmva2pDh/UqY3+IzNQstkM8Q1Z0q/b7/bAfbTXb eB0fZMQEK2i80LrtYC/d5tnxSzTUOlUMfcIGnuFIFhcpVvg4Ddh2K96OcFnHChusdvgp gosw==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=HOjkjRAp; arc=pass (i=1 dkim=pass dkdomain=kernel.org); spf=pass (google.com: domain of linux-kernel+bounces-108001-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-108001-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [147.75.80.249]) by mx.google.com with ESMTPS id v26-20020a170906489a00b00a461af7a1efsi5368708ejq.1045.2024.03.19.11.22.18 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Mar 2024 11:22:18 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel+bounces-108001-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=HOjkjRAp; arc=pass (i=1 dkim=pass dkdomain=kernel.org); spf=pass (google.com: domain of linux-kernel+bounces-108001-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-108001-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 42A811F2109A for ; Tue, 19 Mar 2024 18:21:53 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 8AA6A3A8CE; Tue, 19 Mar 2024 18:21:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="HOjkjRAp" Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 AC06F3A1B7; Tue, 19 Mar 2024 18:21:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710872491; cv=none; b=Wnya0KRiSokjHTSCHYdvYVZ//k9niTwd+ApHJolSrllZ8jQjEk0ykGUU6qeZh8xdhOdyNSgNyEKRrVy3a1x0VZop8pWvFHmmHWhhxiQsNhhwex3Jcy5u5wfh3NhlXWOhXdLUBHiA8lasfui9xSoZByVUDxtPtjitueAEe5TAgac= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710872491; c=relaxed/simple; bh=aD/VhLpbiA3vcXTWj5tF6nefydh1cXCm5/9qP5S2BSw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=pp6sky/Rj5J5NZluqB7LFnLLzBoF6LdXf8NpBx3AocuLEdNij4vNanS8OAM16a9J2U3mEIj3eIflXGk7Zat4HYeoTQ/wWoFmh4URFbJvDYM3biIOjQ7GwRp6WaWxe4Opa+NDZCPheDsymE5wuM/lwFJErWQvySOY0Aa+W89JgDk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=HOjkjRAp; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 015D4C43390; Tue, 19 Mar 2024 18:21:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1710872490; bh=aD/VhLpbiA3vcXTWj5tF6nefydh1cXCm5/9qP5S2BSw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=HOjkjRApQsEMcD8/EU3bOuLQ1KoiBSQRcgsqPtCeo/Q6ZNDK9itubzQ5b9FpZ4nY1 EHSL6xcECUV0XhH7M/0BjqFgHUqKniHneY9HV8ThzLOM/ZFTDacutQuQkNyvQ8hDpg hpGKQhN6pROh1QbqET4OSotLbs6GWi0+pJhcmpAKi8dAqiqIwlQm1kRq2pxthrqURQ NgZ/GaJ803xu0ym2EvzF2M14UPC578wY04EDBbQS5WRv0ts4RDcVyIhVeMBHAS8U83 QsTasv/u14Nf47VNM1QyZjktx3npaNIO5+gEZ/EKn9RcuJHH6B8/uvcAIV5seBfXE7 QmqqkqiQwZG7g== Date: Tue, 19 Mar 2024 18:21:25 +0000 From: Will Deacon To: "Michael S. Tsirkin" Cc: Gavin Shan , virtualization@lists.linux.dev, linux-kernel@vger.kernel.org, jasowang@redhat.com, xuanzhuo@linux.alibaba.com, yihyu@redhat.com, shan.gavin@gmail.com Subject: Re: [PATCH] virtio_ring: Fix the stale index in available ring Message-ID: <20240319182125.GA3121@willie-the-truck> References: <20240314074923.426688-1-gshan@redhat.com> <20240318165924.GA1824@willie-the-truck> <20240319033016-mutt-send-email-mst@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240319033016-mutt-send-email-mst@kernel.org> User-Agent: Mutt/1.10.1 (2018-07-13) On Tue, Mar 19, 2024 at 03:36:31AM -0400, Michael S. Tsirkin wrote: > On Mon, Mar 18, 2024 at 04:59:24PM +0000, Will Deacon wrote: > > On Thu, Mar 14, 2024 at 05:49:23PM +1000, Gavin Shan wrote: > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > > > index 49299b1f9ec7..7d852811c912 100644 > > > --- a/drivers/virtio/virtio_ring.c > > > +++ b/drivers/virtio/virtio_ring.c > > > @@ -687,9 +687,15 @@ static inline int virtqueue_add_split(struct virtqueue *_vq, > > > avail = vq->split.avail_idx_shadow & (vq->split.vring.num - 1); > > > vq->split.vring.avail->ring[avail] = cpu_to_virtio16(_vq->vdev, head); > > > > > > - /* Descriptors and available array need to be set before we expose the > > > - * new available array entries. */ > > > - virtio_wmb(vq->weak_barriers); > > > + /* > > > + * Descriptors and available array need to be set before we expose > > > + * the new available array entries. virtio_wmb() should be enough > > > + * to ensuere the order theoretically. However, a stronger barrier > > > + * is needed by ARM64. Otherwise, the stale data can be observed > > > + * by the host (vhost). A stronger barrier should work for other > > > + * architectures, but performance loss is expected. > > > + */ > > > + virtio_mb(false); > > > vq->split.avail_idx_shadow++; > > > vq->split.vring.avail->idx = cpu_to_virtio16(_vq->vdev, > > > vq->split.avail_idx_shadow); > > > > Replacing a DMB with a DSB is _very_ unlikely to be the correct solution > > here, especially when ordering accesses to coherent memory. > > > > In practice, either the larger timing different from the DSB or the fact > > that you're going from a Store->Store barrier to a full barrier is what > > makes things "work" for you. Have you tried, for example, a DMB SY > > (e.g. via __smb_mb()). > > > > We definitely shouldn't take changes like this without a proper > > explanation of what is going on. > > Just making sure: so on this system, how do > smp_wmb() and wmb() differ? smb_wmb is normally for synchronizing > with kernel running on another CPU and we are doing something > unusual in virtio when we use it to synchronize with host > as opposed to the guest - e.g. CONFIG_SMP is special cased > because of this: > > #define virt_wmb() do { kcsan_wmb(); __smp_wmb(); } while (0) > > Note __smp_wmb not smp_wmb which would be a NOP on UP. I think that should be fine (as long as the NOP is a barrier() for the compiler). wmb() uses a DSB, but that is only really relevant to endpoint ordering with real I/O devices, e.g. if for some reason you need to guarantee that a write has made it all the way to the device before proceeding. Even then you're slightly at the mercy of the memory type and the interconnect not giving an early acknowledgement, so the extra ordering is rarely needed in practice and we don't even provide it for our I/O accessors (e.g. writel() and readl()). So for virtio between two CPUs using coherent memory, DSB is a red herring. Will