Re: [RFC 0/3] try to solve the DMA to MMIO issue

[Jason Wang <jasowang@redhat.com>]

On 2020/9/3 上午12:22, Li Qiang wrote:
> The qemu device fuzzer has found several DMA to MMIO issue.
> These issues is caused by the guest driver programs the DMA
> address, then in the device MMIO handler it trigger the DMA
> and as the DMA address is MMIO it will trigger another dispatch
> and reenter the MMIO handler again. However most of the device
> is not reentrant.
>
> DMA to MMIO will cause issues depend by the device emulator,
> mostly it will crash the qemu. Following is three classic
> DMA to MMIO issue.
>
> e1000e: https://bugs.launchpad.net/qemu/+bug/1886362
> xhci: https://bugs.launchpad.net/qemu/+bug/1891354
> virtio-gpu: https://bugs.launchpad.net/qemu/+bug/1888606
>
> The DMA to MMIO issue I think can be classified as following:
> 1. DMA to the device itself
> 2. device A DMA to device B and to device C
> 3. device A DMA to device B and to device A
>
> The first case of course should not be allowed.
> The second case I think it ok as the device IO handler has no
> assumption about the IO data came from no matter it come from
> device or other device. This is for P2P DMA.
> The third case I think it also should not be allowed.
>
> So our issue has been reduced by one case: not allowed the
> device's IO handler reenter.
>
> Paolo suggested that we can refactor the device emulation with
> BH. However it is a lot of work.
> I have thought several propose to address this, also discuss
> this with Jason Wang in private email.
>
> I have can solve this issue in core framework or in specific device.
> After try several methods I choose address it in per-device for
> following reason:
> 1. If we address it in core framwork we have to recored and check the
> device or MR info in MR dispatch write function. Unfortunally we have
> no these info in core framework.
> 2. The performance will also be decrease largely
> 3. Only the device itself know its IO


I think we still need to seek a way to address this issue completely.

How about adding a flag in MemoryRegionOps and detect the reentrancy 
through that flag?

Thanks


>
> The (most of the) device emulation is protected by BQL one time only
> a device emulation code can be run. We can add a flag to indicate the
> IO is running. The first two patches does this. For simplicity at the
> RFC stage I just set it while enter the IO callback and clear it exit
> the IO callback. It should be check/set/clean according the per-device's
> IO emulation.
> The second issue which itself suffers a race condition so I uses a
> atomic.
>
>
>
>
> Li Qiang (3):
>    e1000e: make the IO handler reentrant
>    xhci: make the IO handler reentrant
>    virtio-gpu: make the IO handler reentrant
>
>   hw/display/virtio-gpu.c        | 10 ++++++
>   hw/net/e1000e.c                | 35 +++++++++++++++++++-
>   hw/usb/hcd-xhci.c              | 60 ++++++++++++++++++++++++++++++++++
>   hw/usb/hcd-xhci.h              |  1 +
>   include/hw/virtio/virtio-gpu.h |  1 +
>   5 files changed, 106 insertions(+), 1 deletion(-)
>