Re: [PATCH v2 1/3] memory: Track whether a Device is engaged in IO

[Darren Kenny]

Hi Alex,

I don't know this code well enough to be certain, but is a flag
sufficient here given the intent, or should it be using a more
thread-safe method like a rwlock or condition variable?

Maybe the device state structure is already protected at some level
with a mutex - just not obvious to me from these changes...

Thanks,

Darren.

On Friday, 2022-05-27 at 12:19:35 -04, Alexander Bulekov wrote:
> Add a flag to the DeviceState, when a device is engaged in PIO/MMIO/DMA.
> This flag should be set/checked prior to calling a device's MemoryRegion
> handlers, and set when device code initiates DMA.  The purpose of this
> flag is to prevent DMA reentrancy issues. E.g.:
> sdhci pio -> dma write -> sdhci mmio
> nvme bh -> dma write -> nvme mmio
>
> These issues have led to problems such as stack-exhaustion and
> use-after-frees.
>
> Assumptions:
>  * Devices do not interact with their own PIO/MMIO memory-regions using
>    DMA.
>
>  * There is now way for there to be multiple simultaneous accesses to a
>    device's PIO/MMIO memory-regions, or for multiple threads to perform
>    DMA accesses simultaneously on behalf of a single device.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  include/hw/qdev-core.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 92c3d65208..6474dc51fa 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -193,6 +193,9 @@ struct DeviceState {
>      int instance_id_alias;
>      int alias_required_for_version;
>      ResettableState reset;
> +
> +    /* Is the device currently in mmio/pio/dma? Used to prevent re-entrancy 
> */
> +    int engaged_in_io;
>  };
>  
>  struct DeviceListener {
> -- 
> 2.33.0