Skip to content

Commit

Permalink
hw: replace most qemu_bh_new calls with qemu_bh_new_guarded
Browse files Browse the repository at this point in the history
This protects devices from bh->mmio reentrancy issues.

Thanks: Thomas Huth <thuth@redhat.com> for diagnosing OS X test failure.
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20230427211013.2994127-5-alxndr@bu.edu>
Signed-off-by: Thomas Huth <thuth@redhat.com>
(cherry picked from commit f63192b)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
  • Loading branch information
a1xndr authored and Michael Tokarev committed Sep 11, 2023
1 parent bb3522b commit ae96dce
Show file tree
Hide file tree
Showing 25 changed files with 66 additions and 33 deletions.
5 changes: 4 additions & 1 deletion hw/9pfs/xen-9p-backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ typedef struct Xen9pfsDev {

int num_rings;
Xen9pfsRing *rings;
MemReentrancyGuard mem_reentrancy_guard;
} Xen9pfsDev;

static void xen_9pfs_disconnect(struct XenLegacyDevice *xendev);
Expand Down Expand Up @@ -448,7 +449,9 @@ static int xen_9pfs_connect(struct XenLegacyDevice *xendev)
xen_9pdev->rings[i].ring.out = xen_9pdev->rings[i].data +
XEN_FLEX_RING_SIZE(ring_order);

xen_9pdev->rings[i].bh = qemu_bh_new(xen_9pfs_bh, &xen_9pdev->rings[i]);
xen_9pdev->rings[i].bh = qemu_bh_new_guarded(xen_9pfs_bh,
&xen_9pdev->rings[i],
&xen_9pdev->mem_reentrancy_guard);
xen_9pdev->rings[i].out_cons = 0;
xen_9pdev->rings[i].out_size = 0;
xen_9pdev->rings[i].inprogress = false;
Expand Down
3 changes: 2 additions & 1 deletion hw/block/dataplane/virtio-blk.c
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
} else {
s->ctx = qemu_get_aio_context();
}
s->bh = aio_bh_new(s->ctx, notify_guest_bh, s);
s->bh = aio_bh_new_guarded(s->ctx, notify_guest_bh, s,
&DEVICE(vdev)->mem_reentrancy_guard);
s->batch_notify_vqs = bitmap_new(conf->num_queues);

*dataplane = s;
Expand Down
5 changes: 3 additions & 2 deletions hw/block/dataplane/xen-block.c
Original file line number Diff line number Diff line change
Expand Up @@ -632,8 +632,9 @@ XenBlockDataPlane *xen_block_dataplane_create(XenDevice *xendev,
} else {
dataplane->ctx = qemu_get_aio_context();
}
dataplane->bh = aio_bh_new(dataplane->ctx, xen_block_dataplane_bh,
dataplane);
dataplane->bh = aio_bh_new_guarded(dataplane->ctx, xen_block_dataplane_bh,
dataplane,
&DEVICE(xendev)->mem_reentrancy_guard);

return dataplane;
}
Expand Down
3 changes: 2 additions & 1 deletion hw/char/virtio-serial-bus.c
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,8 @@ static void virtser_port_device_realize(DeviceState *dev, Error **errp)
return;
}

port->bh = qemu_bh_new(flush_queued_data_bh, port);
port->bh = qemu_bh_new_guarded(flush_queued_data_bh, port,
&dev->mem_reentrancy_guard);
port->elem = NULL;
}

Expand Down
9 changes: 6 additions & 3 deletions hw/display/qxl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2223,11 +2223,14 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error **errp)

qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);

qxl->update_irq = qemu_bh_new(qxl_update_irq_bh, qxl);
qxl->update_irq = qemu_bh_new_guarded(qxl_update_irq_bh, qxl,
&DEVICE(qxl)->mem_reentrancy_guard);
qxl_reset_state(qxl);

qxl->update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl);
qxl->ssd.cursor_bh = qemu_bh_new(qemu_spice_cursor_refresh_bh, &qxl->ssd);
qxl->update_area_bh = qemu_bh_new_guarded(qxl_render_update_area_bh, qxl,
&DEVICE(qxl)->mem_reentrancy_guard);
qxl->ssd.cursor_bh = qemu_bh_new_guarded(qemu_spice_cursor_refresh_bh, &qxl->ssd,
&DEVICE(qxl)->mem_reentrancy_guard);
}

static void qxl_realize_primary(PCIDevice *dev, Error **errp)
Expand Down
6 changes: 4 additions & 2 deletions hw/display/virtio-gpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -1356,8 +1356,10 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)

g->ctrl_vq = virtio_get_queue(vdev, 0);
g->cursor_vq = virtio_get_queue(vdev, 1);
g->ctrl_bh = qemu_bh_new(virtio_gpu_ctrl_bh, g);
g->cursor_bh = qemu_bh_new(virtio_gpu_cursor_bh, g);
g->ctrl_bh = qemu_bh_new_guarded(virtio_gpu_ctrl_bh, g,
&qdev->mem_reentrancy_guard);
g->cursor_bh = qemu_bh_new_guarded(virtio_gpu_cursor_bh, g,
&qdev->mem_reentrancy_guard);
QTAILQ_INIT(&g->reslist);
QTAILQ_INIT(&g->cmdq);
QTAILQ_INIT(&g->fenceq);
Expand Down
3 changes: 2 additions & 1 deletion hw/ide/ahci.c
Original file line number Diff line number Diff line change
Expand Up @@ -1508,7 +1508,8 @@ static void ahci_cmd_done(const IDEDMA *dma)
ahci_write_fis_d2h(ad);

if (ad->port_regs.cmd_issue && !ad->check_bh) {
ad->check_bh = qemu_bh_new(ahci_check_cmd_bh, ad);
ad->check_bh = qemu_bh_new_guarded(ahci_check_cmd_bh, ad,
&ad->mem_reentrancy_guard);
qemu_bh_schedule(ad->check_bh);
}
}
Expand Down
1 change: 1 addition & 0 deletions hw/ide/ahci_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ struct AHCIDevice {
bool init_d2h_sent;
AHCICmdHdr *cur_cmd;
NCQTransferState ncq_tfs[AHCI_MAX_CMDS];
MemReentrancyGuard mem_reentrancy_guard;
};

struct AHCIPCIState {
Expand Down
4 changes: 3 additions & 1 deletion hw/ide/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -512,14 +512,16 @@ BlockAIOCB *ide_issue_trim(
BlockCompletionFunc *cb, void *cb_opaque, void *opaque)
{
IDEState *s = opaque;
IDEDevice *dev = s->unit ? s->bus->slave : s->bus->master;
TrimAIOCB *iocb;

/* Paired with a decrement in ide_trim_bh_cb() */
blk_inc_in_flight(s->blk);

iocb = blk_aio_get(&trim_aiocb_info, s->blk, cb, cb_opaque);
iocb->s = s;
iocb->bh = qemu_bh_new(ide_trim_bh_cb, iocb);
iocb->bh = qemu_bh_new_guarded(ide_trim_bh_cb, iocb,
&DEVICE(dev)->mem_reentrancy_guard);
iocb->ret = 0;
iocb->qiov = qiov;
iocb->i = -1;
Expand Down
6 changes: 4 additions & 2 deletions hw/misc/imx_rngc.c
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,10 @@ static void imx_rngc_realize(DeviceState *dev, Error **errp)
sysbus_init_mmio(sbd, &s->iomem);

sysbus_init_irq(sbd, &s->irq);
s->self_test_bh = qemu_bh_new(imx_rngc_self_test, s);
s->seed_bh = qemu_bh_new(imx_rngc_seed, s);
s->self_test_bh = qemu_bh_new_guarded(imx_rngc_self_test, s,
&dev->mem_reentrancy_guard);
s->seed_bh = qemu_bh_new_guarded(imx_rngc_seed, s,
&dev->mem_reentrancy_guard);
}

static void imx_rngc_reset(DeviceState *dev)
Expand Down
2 changes: 1 addition & 1 deletion hw/misc/macio/mac_dbdma.c
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,7 @@ static void mac_dbdma_realize(DeviceState *dev, Error **errp)
{
DBDMAState *s = MAC_DBDMA(dev);

s->bh = qemu_bh_new(DBDMA_run_bh, s);
s->bh = qemu_bh_new_guarded(DBDMA_run_bh, s, &dev->mem_reentrancy_guard);
}

static void mac_dbdma_class_init(ObjectClass *oc, void *data)
Expand Down
3 changes: 2 additions & 1 deletion hw/net/virtio-net.c
Original file line number Diff line number Diff line change
Expand Up @@ -2875,7 +2875,8 @@ static void virtio_net_add_queue(VirtIONet *n, int index)
n->vqs[index].tx_vq =
virtio_add_queue(vdev, n->net_conf.tx_queue_size,
virtio_net_handle_tx_bh);
n->vqs[index].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[index]);
n->vqs[index].tx_bh = qemu_bh_new_guarded(virtio_net_tx_bh, &n->vqs[index],
&DEVICE(vdev)->mem_reentrancy_guard);
}

n->vqs[index].tx_waiting = 0;
Expand Down
6 changes: 4 additions & 2 deletions hw/nvme/ctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -4318,7 +4318,8 @@ static void nvme_init_sq(NvmeSQueue *sq, NvmeCtrl *n, uint64_t dma_addr,
QTAILQ_INSERT_TAIL(&(sq->req_list), &sq->io_req[i], entry);
}

sq->bh = qemu_bh_new(nvme_process_sq, sq);
sq->bh = qemu_bh_new_guarded(nvme_process_sq, sq,
&DEVICE(sq->ctrl)->mem_reentrancy_guard);

if (n->dbbuf_enabled) {
sq->db_addr = n->dbbuf_dbs + (sqid << 3);
Expand Down Expand Up @@ -4705,7 +4706,8 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n, uint64_t dma_addr,
}
}
n->cq[cqid] = cq;
cq->bh = qemu_bh_new(nvme_post_cqes, cq);
cq->bh = qemu_bh_new_guarded(nvme_post_cqes, cq,
&DEVICE(cq->ctrl)->mem_reentrancy_guard);
}

static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeRequest *req)
Expand Down
3 changes: 2 additions & 1 deletion hw/scsi/mptsas.c
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,8 @@ static void mptsas_scsi_realize(PCIDevice *dev, Error **errp)
}
s->max_devices = MPTSAS_NUM_PORTS;

s->request_bh = qemu_bh_new(mptsas_fetch_requests, s);
s->request_bh = qemu_bh_new_guarded(mptsas_fetch_requests, s,
&DEVICE(dev)->mem_reentrancy_guard);

scsi_bus_init(&s->bus, sizeof(s->bus), &dev->qdev, &mptsas_scsi_info);
}
Expand Down
3 changes: 2 additions & 1 deletion hw/scsi/scsi-bus.c
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ static void scsi_dma_restart_cb(void *opaque, bool running, RunState state)
AioContext *ctx = blk_get_aio_context(s->conf.blk);
/* The reference is dropped in scsi_dma_restart_bh.*/
object_ref(OBJECT(s));
s->bh = aio_bh_new(ctx, scsi_dma_restart_bh, s);
s->bh = aio_bh_new_guarded(ctx, scsi_dma_restart_bh, s,
&DEVICE(s)->mem_reentrancy_guard);
qemu_bh_schedule(s->bh);
}
}
Expand Down
3 changes: 2 additions & 1 deletion hw/scsi/vmw_pvscsi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,8 @@ pvscsi_realizefn(PCIDevice *pci_dev, Error **errp)
pcie_endpoint_cap_init(pci_dev, PVSCSI_EXP_EP_OFFSET);
}

s->completion_worker = qemu_bh_new(pvscsi_process_completion_queue, s);
s->completion_worker = qemu_bh_new_guarded(pvscsi_process_completion_queue, s,
&DEVICE(pci_dev)->mem_reentrancy_guard);

scsi_bus_init(&s->bus, sizeof(s->bus), DEVICE(pci_dev), &pvscsi_scsi_info);
/* override default SCSI bus hotplug-handler, with pvscsi's one */
Expand Down
3 changes: 2 additions & 1 deletion hw/usb/dev-uas.c
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,8 @@ static void usb_uas_realize(USBDevice *dev, Error **errp)

QTAILQ_INIT(&uas->results);
QTAILQ_INIT(&uas->requests);
uas->status_bh = qemu_bh_new(usb_uas_send_status_bh, uas);
uas->status_bh = qemu_bh_new_guarded(usb_uas_send_status_bh, uas,
&d->mem_reentrancy_guard);

dev->flags |= (1 << USB_DEV_FLAG_IS_SCSI_STORAGE);
scsi_bus_init(&uas->bus, sizeof(uas->bus), DEVICE(dev), &usb_uas_scsi_info);
Expand Down
3 changes: 2 additions & 1 deletion hw/usb/hcd-dwc2.c
Original file line number Diff line number Diff line change
Expand Up @@ -1364,7 +1364,8 @@ static void dwc2_realize(DeviceState *dev, Error **errp)
s->fi = USB_FRMINTVL - 1;
s->eof_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, dwc2_frame_boundary, s);
s->frame_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, dwc2_work_timer, s);
s->async_bh = qemu_bh_new(dwc2_work_bh, s);
s->async_bh = qemu_bh_new_guarded(dwc2_work_bh, s,
&dev->mem_reentrancy_guard);

sysbus_init_irq(sbd, &s->irq);
}
Expand Down
3 changes: 2 additions & 1 deletion hw/usb/hcd-ehci.c
Original file line number Diff line number Diff line change
Expand Up @@ -2533,7 +2533,8 @@ void usb_ehci_realize(EHCIState *s, DeviceState *dev, Error **errp)
}

s->frame_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ehci_work_timer, s);
s->async_bh = qemu_bh_new(ehci_work_bh, s);
s->async_bh = qemu_bh_new_guarded(ehci_work_bh, s,
&dev->mem_reentrancy_guard);
s->device = dev;

s->vmstate = qemu_add_vm_change_state_handler(usb_ehci_vm_state_change, s);
Expand Down
2 changes: 1 addition & 1 deletion hw/usb/hcd-uhci.c
Original file line number Diff line number Diff line change
Expand Up @@ -1193,7 +1193,7 @@ void usb_uhci_common_realize(PCIDevice *dev, Error **errp)
USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL);
}
}
s->bh = qemu_bh_new(uhci_bh, s);
s->bh = qemu_bh_new_guarded(uhci_bh, s, &DEVICE(dev)->mem_reentrancy_guard);
s->frame_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, uhci_frame_timer, s);
s->num_ports_vmstate = NB_PORTS;
QTAILQ_INIT(&s->queues);
Expand Down
6 changes: 4 additions & 2 deletions hw/usb/host-libusb.c
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,8 @@ static void usb_host_nodev_bh(void *opaque)
static void usb_host_nodev(USBHostDevice *s)
{
if (!s->bh_nodev) {
s->bh_nodev = qemu_bh_new(usb_host_nodev_bh, s);
s->bh_nodev = qemu_bh_new_guarded(usb_host_nodev_bh, s,
&DEVICE(s)->mem_reentrancy_guard);
}
qemu_bh_schedule(s->bh_nodev);
}
Expand Down Expand Up @@ -1739,7 +1740,8 @@ static int usb_host_post_load(void *opaque, int version_id)
USBHostDevice *dev = opaque;

if (!dev->bh_postld) {
dev->bh_postld = qemu_bh_new(usb_host_post_load_bh, dev);
dev->bh_postld = qemu_bh_new_guarded(usb_host_post_load_bh, dev,
&DEVICE(dev)->mem_reentrancy_guard);
}
qemu_bh_schedule(dev->bh_postld);
dev->bh_postld_pending = true;
Expand Down
6 changes: 4 additions & 2 deletions hw/usb/redirect.c
Original file line number Diff line number Diff line change
Expand Up @@ -1441,8 +1441,10 @@ static void usbredir_realize(USBDevice *udev, Error **errp)
}
}

dev->chardev_close_bh = qemu_bh_new(usbredir_chardev_close_bh, dev);
dev->device_reject_bh = qemu_bh_new(usbredir_device_reject_bh, dev);
dev->chardev_close_bh = qemu_bh_new_guarded(usbredir_chardev_close_bh, dev,
&DEVICE(dev)->mem_reentrancy_guard);
dev->device_reject_bh = qemu_bh_new_guarded(usbredir_device_reject_bh, dev,
&DEVICE(dev)->mem_reentrancy_guard);
dev->attach_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, usbredir_do_attach, dev);

packet_id_queue_init(&dev->cancelled, dev, "cancelled");
Expand Down
3 changes: 2 additions & 1 deletion hw/usb/xen-usb.c
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,8 @@ static void usbback_alloc(struct XenLegacyDevice *xendev)

QTAILQ_INIT(&usbif->req_free_q);
QSIMPLEQ_INIT(&usbif->hotplug_q);
usbif->bh = qemu_bh_new(usbback_bh, usbif);
usbif->bh = qemu_bh_new_guarded(usbback_bh, usbif,
&DEVICE(xendev)->mem_reentrancy_guard);
}

static int usbback_free(struct XenLegacyDevice *xendev)
Expand Down
5 changes: 3 additions & 2 deletions hw/virtio/virtio-balloon.c
Original file line number Diff line number Diff line change
Expand Up @@ -910,8 +910,9 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
precopy_add_notifier(&s->free_page_hint_notify);

object_ref(OBJECT(s->iothread));
s->free_page_bh = aio_bh_new(iothread_get_aio_context(s->iothread),
virtio_ballloon_get_free_page_hints, s);
s->free_page_bh = aio_bh_new_guarded(iothread_get_aio_context(s->iothread),
virtio_ballloon_get_free_page_hints, s,
&dev->mem_reentrancy_guard);
}

if (virtio_has_feature(s->host_features, VIRTIO_BALLOON_F_REPORTING)) {
Expand Down
3 changes: 2 additions & 1 deletion hw/virtio/virtio-crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,8 @@ static void virtio_crypto_device_realize(DeviceState *dev, Error **errp)
vcrypto->vqs[i].dataq =
virtio_add_queue(vdev, 1024, virtio_crypto_handle_dataq_bh);
vcrypto->vqs[i].dataq_bh =
qemu_bh_new(virtio_crypto_dataq_bh, &vcrypto->vqs[i]);
qemu_bh_new_guarded(virtio_crypto_dataq_bh, &vcrypto->vqs[i],
&dev->mem_reentrancy_guard);
vcrypto->vqs[i].vcrypto = vcrypto;
}

Expand Down

0 comments on commit ae96dce

Please sign in to comment.