Re: [PATCH v2 0/2] net: Update MemReentrancyGuard for NIC

[Akihiko Odaki]

On 2023/06/01 16:16, Philippe Mathieu-Daudé wrote:
> 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?

The involvement of CPU is not essential in my understanding. A typical 
scenario of DMA reentrancy is like the following:
1) The guest configures the DMA destination address register the device 
has to the address of another device register.
2) The DMA gets triggered.
3) The device performs the DMA, writing its own register.
4) The write causes reentrancy.
5) The re-entered device code corrupts the device state.

I guess 2) is done by CPU in most cases, but sometimes it happen with 
another cause. In fact, the current reentrancy protection code covers 
the case that bottom half handlers triggers DMA. The intention of this 
series is to extend the coverage and handles the case that incoming 
network traffic triggers DMA.

The essence of DMA reentrancy is in 3). This happens when the DMA 
address space contains the MMIO region of the device and there is no 
involvement of CPU here.