[PATCH v10 0/8] memory: prevent dma-reentracy issues

Alexander Bulekov posted 8 patches 1 year ago
Failed in applying to current master (apply log)
Maintainers: Stefano Stabellini <sstabellini@kernel.org>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul@xen.org>, Stefan Hajnoczi <stefanha@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Amit Shah <amit@kernel.org>, "Michael S. Tsirkin" <mst@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, John Snow <jsnow@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Jason Wang <jasowang@redhat.com>, Keith Busch <kbusch@kernel.org>, Klaus Jensen <its@irrelevant.dk>, "Hervé Poussineau" <hpoussin@reactos.org>, Fam Zheng <fam@euphon.net>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, David Hildenbrand <david@redhat.com>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Peter Xu <peterx@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
docs/devel/multiple-iothreads.txt |  7 +++++++
hw/9pfs/xen-9p-backend.c          |  5 ++++-
hw/block/dataplane/virtio-blk.c   |  3 ++-
hw/block/dataplane/xen-block.c    |  5 +++--
hw/char/virtio-serial-bus.c       |  3 ++-
hw/display/qxl.c                  |  9 ++++++---
hw/display/virtio-gpu.c           |  6 ++++--
hw/ide/ahci.c                     |  3 ++-
hw/ide/ahci_internal.h            |  1 +
hw/ide/core.c                     |  4 +++-
hw/intc/apic.c                    |  7 +++++++
hw/misc/bcm2835_property.c        |  7 +++++++
hw/misc/imx_rngc.c                |  6 ++++--
hw/misc/macio/mac_dbdma.c         |  2 +-
hw/net/virtio-net.c               |  3 ++-
hw/nvme/ctrl.c                    |  6 ++++--
hw/pci-host/raven.c               |  7 +++++++
hw/scsi/lsi53c895a.c              |  6 ++++++
hw/scsi/mptsas.c                  |  3 ++-
hw/scsi/scsi-bus.c                |  3 ++-
hw/scsi/vmw_pvscsi.c              |  3 ++-
hw/usb/dev-uas.c                  |  3 ++-
hw/usb/hcd-dwc2.c                 |  3 ++-
hw/usb/hcd-ehci.c                 |  3 ++-
hw/usb/hcd-uhci.c                 |  2 +-
hw/usb/host-libusb.c              |  6 ++++--
hw/usb/redirect.c                 |  6 ++++--
hw/usb/xen-usb.c                  |  3 ++-
hw/virtio/virtio-balloon.c        |  5 +++--
hw/virtio/virtio-crypto.c         |  3 ++-
include/block/aio.h               | 18 ++++++++++++++++--
include/exec/memory.h             |  5 +++++
include/hw/qdev-core.h            |  7 +++++++
include/qemu/main-loop.h          |  7 +++++--
scripts/checkpatch.pl             |  8 ++++++++
softmmu/memory.c                  | 16 ++++++++++++++++
tests/unit/ptimer-test-stubs.c    |  3 ++-
util/async.c                      | 18 +++++++++++++++++-
util/main-loop.c                  |  5 +++--
util/trace-events                 |  1 +
40 files changed, 180 insertions(+), 41 deletions(-)
[PATCH v10 0/8] memory: prevent dma-reentracy issues
Posted by Alexander Bulekov 1 year ago
v8-> v9:
    - Replace trace-events and attempt at making re-entrancy fatal with
      a warn_report message. This message should only be printed if a
      device is broken (and needs to be marked re-entrancy safe), or if
      something in the guest is attempting to trigger unintentional
      re-entrancy.
    - Added APIC change to the series

v7 -> v8:
    - Disable reentrancy checks for bcm2835_property's iomem (Patch 7)
    - Cache DeviceState* in the MemoryRegion to avoid dynamic cast for
      each MemoryRegion access. (Patch 1)
    - Make re-entrancy fatal for debug-builds (Patch 8)

v6 -> v7:
    - Fix bad qemu_bh_new_guarded calls found by Thomas (Patch 4)
    - Add an MR-specific flag to disable reentrancy (Patch 5)
    - Disable reentrancy checks for lsi53c895a's RAM-like MR (Patch 6)
    
    Patches 5 and 6 need review. I left the review-tags for Patch 4,
    however a few of the qemu_bh_new_guarded calls have changed.
  
v5 -> v6:
    - Only apply checkpatch checks to code in paths containing "/hw/"
      (/hw/ and include/hw/)
    - Fix a bug in a _guarded call added to hw/block/virtio-blk.c
v4-> v5:
    - Add corresponding checkpatch checks
    - Save/restore reentrancy-flag when entering/exiting BHs
    - Improve documentation
    - Check object_dynamic_cast return value
    
v3 -> v4: Instead of changing all of the DMA APIs, instead add an
    optional reentrancy guard to the BH API.
    
v2 -> v3: Bite the bullet and modify the DMA APIs, rather than
    attempting to guess DeviceStates in BHs.
    
These patches aim to solve two types of DMA-reentrancy issues:

1.) mmio -> dma -> mmio case
To solve this, we track whether the device is engaged in io by
checking/setting a reentrancy-guard within APIs used for MMIO access.

2.) bh -> dma write -> mmio case
This case is trickier, since we dont have a generic way to associate a
bh with the underlying Device/DeviceState. Thus, this version allows a
device to associate a reentrancy-guard with a bh, when creating it.
(Instead of calling qemu_bh_new, you call qemu_bh_new_guarded)

I replaced most of the qemu_bh_new invocations with the guarded analog,
except for the ones where the DeviceState was not trivially accessible.

Alexander Bulekov (8):
  memory: prevent dma-reentracy issues
  async: Add an optional reentrancy guard to the BH API
  checkpatch: add qemu_bh_new/aio_bh_new checks
  hw: replace most qemu_bh_new calls with qemu_bh_new_guarded
  lsi53c895a: disable reentrancy detection for script RAM
  bcm2835_property: disable reentrancy detection for iomem
  raven: disable reentrancy detection for iomem
  apic: disable reentrancy detection for apic-msi

 docs/devel/multiple-iothreads.txt |  7 +++++++
 hw/9pfs/xen-9p-backend.c          |  5 ++++-
 hw/block/dataplane/virtio-blk.c   |  3 ++-
 hw/block/dataplane/xen-block.c    |  5 +++--
 hw/char/virtio-serial-bus.c       |  3 ++-
 hw/display/qxl.c                  |  9 ++++++---
 hw/display/virtio-gpu.c           |  6 ++++--
 hw/ide/ahci.c                     |  3 ++-
 hw/ide/ahci_internal.h            |  1 +
 hw/ide/core.c                     |  4 +++-
 hw/intc/apic.c                    |  7 +++++++
 hw/misc/bcm2835_property.c        |  7 +++++++
 hw/misc/imx_rngc.c                |  6 ++++--
 hw/misc/macio/mac_dbdma.c         |  2 +-
 hw/net/virtio-net.c               |  3 ++-
 hw/nvme/ctrl.c                    |  6 ++++--
 hw/pci-host/raven.c               |  7 +++++++
 hw/scsi/lsi53c895a.c              |  6 ++++++
 hw/scsi/mptsas.c                  |  3 ++-
 hw/scsi/scsi-bus.c                |  3 ++-
 hw/scsi/vmw_pvscsi.c              |  3 ++-
 hw/usb/dev-uas.c                  |  3 ++-
 hw/usb/hcd-dwc2.c                 |  3 ++-
 hw/usb/hcd-ehci.c                 |  3 ++-
 hw/usb/hcd-uhci.c                 |  2 +-
 hw/usb/host-libusb.c              |  6 ++++--
 hw/usb/redirect.c                 |  6 ++++--
 hw/usb/xen-usb.c                  |  3 ++-
 hw/virtio/virtio-balloon.c        |  5 +++--
 hw/virtio/virtio-crypto.c         |  3 ++-
 include/block/aio.h               | 18 ++++++++++++++++--
 include/exec/memory.h             |  5 +++++
 include/hw/qdev-core.h            |  7 +++++++
 include/qemu/main-loop.h          |  7 +++++--
 scripts/checkpatch.pl             |  8 ++++++++
 softmmu/memory.c                  | 16 ++++++++++++++++
 tests/unit/ptimer-test-stubs.c    |  3 ++-
 util/async.c                      | 18 +++++++++++++++++-
 util/main-loop.c                  |  5 +++--
 util/trace-events                 |  1 +
 40 files changed, 180 insertions(+), 41 deletions(-)

-- 
2.39.0
Re: [PATCH v10 0/8] memory: prevent dma-reentracy issues
Posted by Michael Tokarev 11 months, 4 weeks ago
28.04.2023 00:10, Alexander Bulekov wrote:
..
> These patches aim to solve two types of DMA-reentrancy issues:
> 
> 1.) mmio -> dma -> mmio case
> To solve this, we track whether the device is engaged in io by
> checking/setting a reentrancy-guard within APIs used for MMIO access.
> 
> 2.) bh -> dma write -> mmio case
> This case is trickier, since we dont have a generic way to associate a
> bh with the underlying Device/DeviceState. Thus, this version allows a
> device to associate a reentrancy-guard with a bh, when creating it.
> (Instead of calling qemu_bh_new, you call qemu_bh_new_guarded)

Should this patchset (with any follow-up fixes) go to 8.0-stable?

Thanks,

/mjt