Message ID | 20230205040737.3567731-1-alxndr@bu.edu (mailing list archive) |
---|---|
Headers | show |
Series | memory: prevent dma-reentracy issues | expand |
ping On 230204 2307, 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) > > I replaced most of the qemu_bh_new invocations with the guarded analog, > except for the ones where the DeviceState was not trivially accessible. > > 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. > > Alexander Bulekov (4): > 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 > > docs/devel/multiple-iothreads.txt | 7 +++++++ > hw/9pfs/xen-9p-backend.c | 4 +++- > 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/core.c | 3 ++- > 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/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/hw/qdev-core.h | 7 +++++++ > include/qemu/main-loop.h | 7 +++++-- > scripts/checkpatch.pl | 8 ++++++++ > softmmu/memory.c | 17 +++++++++++++++++ > softmmu/trace-events | 1 + > tests/unit/ptimer-test-stubs.c | 3 ++- > util/async.c | 18 +++++++++++++++++- > util/main-loop.c | 5 +++-- > util/trace-events | 1 + > 35 files changed, 147 insertions(+), 41 deletions(-) > > -- > 2.39.0 >
Hi Alex, Everything looks good to me, I don't have anything else to add: Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Thanks, Darren. On Saturday, 2023-02-04 at 23:07:33 -05, 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) > > I replaced most of the qemu_bh_new invocations with the guarded analog, > except for the ones where the DeviceState was not trivially accessible. > > 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. > > Alexander Bulekov (4): > 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 > > docs/devel/multiple-iothreads.txt | 7 +++++++ > hw/9pfs/xen-9p-backend.c | 4 +++- > 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/core.c | 3 ++- > 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/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/hw/qdev-core.h | 7 +++++++ > include/qemu/main-loop.h | 7 +++++-- > scripts/checkpatch.pl | 8 ++++++++ > softmmu/memory.c | 17 +++++++++++++++++ > softmmu/trace-events | 1 + > tests/unit/ptimer-test-stubs.c | 3 ++- > util/async.c | 18 +++++++++++++++++- > util/main-loop.c | 5 +++-- > util/trace-events | 1 + > 35 files changed, 147 insertions(+), 41 deletions(-) > > -- > 2.39.0
On Sun, Feb 12, 2023 at 09:11:41PM -0500, Alexander Bulekov wrote: > ping > > On 230204 2307, 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) > > > > I replaced most of the qemu_bh_new invocations with the guarded analog, > > except for the ones where the DeviceState was not trivially accessible. > > > > 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. As long as we are adding the new APIs virtio things look ok to me. Pls merge with rest of patches. Acked-by: Michael S. Tsirkin <mst@redhat.com> > > Alexander Bulekov (4): > > 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 > > > > docs/devel/multiple-iothreads.txt | 7 +++++++ > > hw/9pfs/xen-9p-backend.c | 4 +++- > > 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/core.c | 3 ++- > > 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/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/hw/qdev-core.h | 7 +++++++ > > include/qemu/main-loop.h | 7 +++++-- > > scripts/checkpatch.pl | 8 ++++++++ > > softmmu/memory.c | 17 +++++++++++++++++ > > softmmu/trace-events | 1 + > > tests/unit/ptimer-test-stubs.c | 3 ++- > > util/async.c | 18 +++++++++++++++++- > > util/main-loop.c | 5 +++-- > > util/trace-events | 1 + > > 35 files changed, 147 insertions(+), 41 deletions(-) > > > > -- > > 2.39.0 > >
On 13/02/2023 03.11, Alexander Bulekov wrote: > ping I think it would be really good to finally get these dma-reentrancy issues fixed! Who's supposed to pick up these patches? Paolo? David? Peter? Thomas > On 230204 2307, 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) >> >> I replaced most of the qemu_bh_new invocations with the guarded analog, >> except for the ones where the DeviceState was not trivially accessible. >> >> 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. >> >> Alexander Bulekov (4): >> 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 >> >> docs/devel/multiple-iothreads.txt | 7 +++++++ >> hw/9pfs/xen-9p-backend.c | 4 +++- >> 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/core.c | 3 ++- >> 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/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/hw/qdev-core.h | 7 +++++++ >> include/qemu/main-loop.h | 7 +++++-- >> scripts/checkpatch.pl | 8 ++++++++ >> softmmu/memory.c | 17 +++++++++++++++++ >> softmmu/trace-events | 1 + >> tests/unit/ptimer-test-stubs.c | 3 ++- >> util/async.c | 18 +++++++++++++++++- >> util/main-loop.c | 5 +++-- >> util/trace-events | 1 + >> 35 files changed, 147 insertions(+), 41 deletions(-) >> >> -- >> 2.39.0 >> >
On Sat, Feb 04, 2023 at 11:07:33PM -0500, 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) > > I replaced most of the qemu_bh_new invocations with the guarded analog, > except for the ones where the DeviceState was not trivially accessible. > > 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. > > Alexander Bulekov (4): > 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 > > docs/devel/multiple-iothreads.txt | 7 +++++++ > hw/9pfs/xen-9p-backend.c | 4 +++- > 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/core.c | 3 ++- > 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/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/hw/qdev-core.h | 7 +++++++ > include/qemu/main-loop.h | 7 +++++-- > scripts/checkpatch.pl | 8 ++++++++ > softmmu/memory.c | 17 +++++++++++++++++ > softmmu/trace-events | 1 + > tests/unit/ptimer-test-stubs.c | 3 ++- > util/async.c | 18 +++++++++++++++++- > util/main-loop.c | 5 +++-- > util/trace-events | 1 + > 35 files changed, 147 insertions(+), 41 deletions(-) > > -- > 2.39.0 > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On 230216 1214, Thomas Huth wrote: > On 13/02/2023 03.11, Alexander Bulekov wrote: > > ping > > I think it would be really good to finally get these dma-reentrancy issues > fixed! Who's supposed to pick up these patches? Paolo? David? Peter? Ping > > Thomas
On Tue, Feb 28, 2023 at 11:07:14AM -0500, Alexander Bulekov wrote: > On 230216 1214, Thomas Huth wrote: > > On 13/02/2023 03.11, Alexander Bulekov wrote: > > > ping > > > > I think it would be really good to finally get these dma-reentrancy issues > > fixed! Who's supposed to pick up these patches? Paolo? David? Peter? > > Ping Sorry to not have replied here - I talked to Paolo and I think Paolo has a plan to review it. Let's wait for another 2-3 days, perhaps? Otherwise from what I can tell this series already received enough R-b/A-bs from major maintainers, this should be mergeable material going via any tree (for memory, it was always Paolo who sends the PR before). Thanks,
On Sat, Feb 04, 2023 at 11:07:33PM -0500, 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) > > I replaced most of the qemu_bh_new invocations with the guarded analog, > except for the ones where the DeviceState was not trivially accessible. Acked-by: Michael S. Tsirkin <mst@redhat.com> > 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. > > Alexander Bulekov (4): > 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 > > docs/devel/multiple-iothreads.txt | 7 +++++++ > hw/9pfs/xen-9p-backend.c | 4 +++- > 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/core.c | 3 ++- > 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/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/hw/qdev-core.h | 7 +++++++ > include/qemu/main-loop.h | 7 +++++-- > scripts/checkpatch.pl | 8 ++++++++ > softmmu/memory.c | 17 +++++++++++++++++ > softmmu/trace-events | 1 + > tests/unit/ptimer-test-stubs.c | 3 ++- > util/async.c | 18 +++++++++++++++++- > util/main-loop.c | 5 +++-- > util/trace-events | 1 + > 35 files changed, 147 insertions(+), 41 deletions(-) > > -- > 2.39.0