Re: [PATCH v3 0/2] hw/nvme: Add shadow doorbell buffer support

[Klaus Jensen <its@irrelevant.dk>]

[-- Attachment #1: Type: text/plain, Size: 2813 bytes --]

On Jun 16 20:34, Jinhao Fan wrote:
> This patch adds shadow doorbell buffer support in NVMe 1.3 to QEMU
> NVMe. The Doorbell Buffer Config admin command is implemented for the
> guest to enable shadow doobell buffer. When this feature is enabled, each
> SQ/CQ is associated with two buffers, i.e., Shadow Doorbell buffer and
> EventIdx buffer. According to the Spec, each queue's doorbell register
> is only updated when the Shadow Doorbell buffer value changes from being
> less than or equal to the value of the corresponding EventIdx buffer
> entry to being greater than that value. Therefore, the number of MMIO's
> on the doorbell registers is greatly reduced.
> 
> This patch is adapted from Huaicheng Li's patch[1] in 2018.
> 
> [1] https://patchwork.kernel.org/project/qemu-devel/patch/20180305194906.GA3630@gmail.com/
> 
> IOPS comparison with FIO:
> 
> iodepth    1      2      4      8
>   QEMU   25.1k  25.9k  24.5k  24.0k
>  +dbbuf  29.1k  60.1k  99.8k  82.5k
> 
> MMIO's per IO measured by perf-kvm:
> 
> iodepth    1      2      4      8
>   QEMU   2.01   1.99   1.99   1.99
>  +dbbuf  1.00   0.52   0.27   0.46
> 
> The tests are done on Ubuntu 22.04 with 5.15.0-33 kernel with Intel(R) 
> Xeon(R) Gold 6248R CPU @ 3.00GHz.
> 
> QEMU set up:
> 
> bin/x86_64-softmmu/qemu-system-x86_64 \
>     -name "nvme-test" \
>     -machine accel=kvm \
>     -cpu host \
>     -smp 4 \
>     -m 8G \
>     -daemonize \
>     -device virtio-scsi-pci,id=scsi0 \
>     -device scsi-hd,drive=hd0 \
>     -drive file=$OSIMGF,if=none,aio=native,cache=none,format=qcow2,id=hd0,snapshot=on \
>     -drive "id=nvm,if=none,file=null-co://,file.read-zeroes=on,format=raw" \
>     -device nvme,serial=deadbeef,drive=nvm \
>     -net user,hostfwd=tcp::8080-:22 \
>     -net nic,model=virtio
> 
> FIO configuration:
> 
> [global]
> ioengine=libaio
> filename=/dev/nvme0n1
> thread=1
> group_reporting=1
> direct=1
> verify=0
> time_based=1
> ramp_time=0
> runtime=30
> ;size=1G
> ;iodepth=1
> rw=randread
> bs=4k
> 
> [test]
> numjobs=1
> 
> Changes since v2:
>   - Do not ignore admin queue updates in nvme_process_db and nvme_post_cqes
>   - Calculate db_addr and ei_addr in hard-coded way
> 
> Changes since v1:
>   - Add compatibility with hosts that do not use admin queue shadow doorbell
> 
> Jinhao Fan (2):
>   hw/nvme: Implement shadow doorbell buffer support
>   hw/nvme: Add trace events for shadow doorbell buffer
> 
>  hw/nvme/ctrl.c       | 118 ++++++++++++++++++++++++++++++++++++++++++-
>  hw/nvme/nvme.h       |   8 +++
>  hw/nvme/trace-events |   5 ++
>  include/block/nvme.h |   2 +
>  4 files changed, 132 insertions(+), 1 deletion(-)
> 
> -- 
> 2.25.1
> 

LGTM,

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]