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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 1/6/23 09:41, Akihiko Odaki wrote:
> 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.

OK, thanks for the explanation.