On Jul 4 11:26, Jinhao Fan wrote:
> Add property "ioeventfd" which is enabled by default. When this is
> enabled, updates on the doorbell registers will cause KVM to signal
> an event to the QEMU main loop to handle the doorbell updates.
> Therefore, instead of letting the vcpu thread run both guest VM and
> IO emulation, we now use the main loop thread to do IO emulation and
> thus the vcpu thread has more cycles for the guest VM.
>
> Since ioeventfd does not tell us the exact value that is written, it is
> only useful when shadow doorbell buffer is enabled, where we check
> for the value in the shadow doorbell buffer when we get the doorbell
> update event.
>
> IOPS comparison on Linux 5.19-rc2: (Unit: KIOPS)
>
> qd 1 4 16 64
> qemu 35 121 176 153
> ioeventfd 41 133 258 313
>
> Changes since v2:
> - Add memory_region_del_eventfd() when freeing queues to avoid leak
>
> Signed-off-by: Jinhao Fan <fanjinhao...@ict.ac.cn>
> ---
> hw/nvme/ctrl.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++-
> hw/nvme/nvme.h | 5 +++
> 2 files changed, 112 insertions(+), 1 deletion(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index c952c34f94..9ceedf0c29 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> static void nvme_free_sq(NvmeSQueue *sq, NvmeCtrl *n)
> {
> + uint16_t offset = sq->sqid << 3;
> +
> n->sq[sq->sqid] = NULL;
> timer_free(sq->timer);
> + memory_region_del_eventfd(&n->iomem,
> + 0x1000 + offset, 4, false, 0, &sq->notifier);
> + event_notifier_cleanup(&sq->notifier);
Need to guard this with an `if (sq->ioeventfd_enabled)`.
> g_free(sq->io_req);
> if (sq->sqid) {
> g_free(sq);
> @@ -4271,6 +4348,12 @@ static void nvme_init_sq(NvmeSQueue *sq, NvmeCtrl *n,
> uint64_t dma_addr,
> if (n->dbbuf_enabled) {
> sq->db_addr = n->dbbuf_dbs + (sqid << 3);
> sq->ei_addr = n->dbbuf_eis + (sqid << 3);
> +
> + if (n->params.ioeventfd && sq->sqid != 0) {
> + if (!nvme_init_sq_ioeventfd(sq)) {
> + sq->ioeventfd_enabled = true;
> + }
> + }
> }
>
> assert(n->cq[cqid]);
> @@ -4575,8 +4658,13 @@ static uint16_t nvme_get_log(NvmeCtrl *n, NvmeRequest
> *req)
>
> static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n)
> {
> + uint16_t offset = (cq->cqid << 3) + (1 << 2);
> +
> n->cq[cq->cqid] = NULL;
> timer_free(cq->timer);
> + memory_region_del_eventfd(&n->iomem,
> + 0x1000 + offset, 4, false, 0, &cq->notifier);
> + event_notifier_cleanup(&cq->notifier);
Same, should also be guarded.
Otherwise, looks good to me!
signature.asc
Description: PGP signature
