Re: [PATCH v2 0/2] net: Update MemReentrancyGuard for NIC - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: Akihiko Odaki <akihiko.odaki@daynix.com>
Cc: "Mauro Matteo Cascella" <mcascell@redhat.com>,
	"P J P" <pj.pandit@yahoo.co.in>,
	"Alexander Bulekov" <alxndr@bu.edu>,
	"Dmitry Fleytman" <dmitry.fleytman@gmail.com>,
	"Beniamino Galvani" <b.galvani@gmail.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Strahinja Jankovic" <strahinja.p.jankovic@gmail.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	"Alistair Francis" <alistair@alistair23.me>,
	"Stefan Weil" <sw@weilnetz.de>, "Cédric Le Goater" <clg@kaod.org>,
	"Andrew Jeffery" <andrew@aj.id.au>,
	"Joel Stanley" <joel@jms.id.au>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Helge Deller" <deller@gmx.de>,
	"Sriram Yagnaraman" <sriram.yagnaraman@est.tech>,
	"Thomas Huth" <huth@tuxfamily.org>,
	"Aleksandar Rikalo" <aleksandar.rikalo@syrmia.com>,
	"Subbaraya Sundeep" <sundeep.lkml@gmail.com>,
	"Jan Kiszka" <jan.kiszka@web.de>,
	"Tyrone Ting" <kfting@nuvoton.com>,
	"Hao Wu" <wuhaotsh@google.com>,
	"Max Filippov" <jcmvbkbc@gmail.com>,
	"Jiri Pirko" <jiri@resnulli.us>,
	"Daniel Henrique Barboza" <danielhb413@gmail.com>,
	"David Gibson" <david@gibson.dropbear.id.au>,
	"Greg Kurz" <groug@kaod.org>,
	"Harsh Prateek Bora" <harshpb@linux.ibm.com>,
	"Sven Schnelle" <svens@stackframe.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Anthony Perard" <anthony.perard@citrix.com>,
	"Paul Durrant" <paul@xen.org>, "Rob Herring" <robh@kernel.org>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	qemu-arm@nongnu.org, qemu-devel@nongnu.org, qemu-ppc@nongnu.org,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v2 0/2] net: Update MemReentrancyGuard for NIC
Date: Thu, 1 Jun 2023 09:16:03 +0200	[thread overview]
Message-ID: <72ccd4c2-7c60-e015-2322-721d09a8334b@linaro.org> (raw)
In-Reply-To: <20230601031859.7115-1-akihiko.odaki@daynix.com>

On 1/6/23 05:18, Akihiko Odaki wrote:
> Recently MemReentrancyGuard was added to DeviceState to record that the
> device is engaging in I/O. The network device backend needs to update it
> when delivering a packet to a device.
> 
> This implementation follows what bottom half does, but it does not add
> a tracepoint for the case that the network device backend started
> delivering a packet to a device which is already engaging in I/O. This
> is because such reentrancy frequently happens for
> qemu_flush_queued_packets() and is insignificant.
> 
> This series consists of two patches. The first patch makes a bulk change to
> add a new parameter to qemu_new_nic() and does not contain behavioral changes.
> The second patch actually implements MemReentrancyGuard update.

/me look at the 'net' API.

So the NetReceive* handlers from NetClientInfo process the HW NIC
data flow, independently from the CPUs.

IIUC MemReentrancyGuard is supposed to protect reentrancy abuse from
CPUs.

NetReceive* handlers aren't restricted to any particular API, they
just consume blob of data. Looking at e1000_receive_iov(), this data
is filled into memory using the pci_dma_rw() API. pci_dma_rw() gets
the AddressSpace to use calling pci_get_address_space(), which returns
PCIDevice::bus_master_as. Then we use the dma_memory_rw(), followed
by address_space_rw(). Beh, I fail to see why there is reentrancy
checks from this NIC DMA HW path.

Maybe the MemoryRegion API isn't the correct place to check for
reentrancy abuse and we should do that at the AddressSpace level,
keeping DMA ASes clear and only protecting CPU ASes?