[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support
From: |
Klaus Jensen |
Subject: |
Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support |
Date: |
Wed, 15 Jun 2022 11:38:00 +0200 |
On Jun 15 11:58, Jinhao Fan wrote:
>
> > On Jun 14, 2022, at 11:41 PM, Keith Busch <kbusch@kernel.org> wrote:
> >
> > It's a pretty nasty hack, and definitely not in compliance with the spec:
> > the
> > db_addr is supposed to be read-only from the device side, though I do think
> > it's safe for this environment. Unless Klaus or anyone finds something I'm
> > missing, I feel this is an acceptable compromise to address this odd
> > discrepency.
>
> :) In my next patch I will check the performance numbers with this hack. Not
> sure if updating db_addr value from the host will have any performance
> implications but I guess it should be OK.
>
I prefer we use the NVMe terminology to minimize misunderstandings, so
"host" means the driver and "device" means the qemu side of things
> > By the way, I noticed that the patch never updates the cq's ei_addr value.
> > Is
> > that on purpose?
>
> Klaus also raised a similar question in a prior comment. I think we need to
> figure
> this out before we move on to the v2 patch. I did this because the original
> Google
> extension patch did not update cq’s ei_addr. This seems to make sense because
> the purpose of cq’s ei_addr is for the guest to notify the host about cq head
> changes when necessary. However, the host does not need this notification
> because we let the host proactively check for cq’s db_addr value when it wants
> to post a new cqe.
> This is also the only point where the host uses the cq’s
> db_addr. Therefore, it is OK to postpone the check for cq’s db_addr to this
> point,
> instead of getting timely but not useful notifications by updating cq’s
> ei_addr.
> This helps to reduce the number of MMIO’s on the cq’s doorbell register.
>
True, it does reduce it, but it may leave CQEs "lingering" on the device
side (since the device has not been notified that the host has consumed
them).
> Klaus, Keith, do you think this design makes sense?
As I mentioned in my reply to John, I can see why this is a perfectly
reasonable optimization, we don't really care about the lingering CQEs
since we read the head anyway prior to posting completions. I jumped the
gun here in my eagerness to be "spec compliant" ;)
signature.asc
Description: PGP signature
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, (continued)
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Jinhao Fan, 2022/06/08
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Keith Busch, 2022/06/09
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, John Levon, 2022/06/09
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Klaus Jensen, 2022/06/09
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, John Levon, 2022/06/09
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Jinhao Fan, 2022/06/12
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Keith Busch, 2022/06/13
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Jinhao Fan, 2022/06/14
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Keith Busch, 2022/06/14
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Jinhao Fan, 2022/06/14
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support,
Klaus Jensen <=
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Jinhao Fan, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Klaus Jensen, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, John Levon, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Klaus Jensen, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, John Levon, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, Jinhao Fan, 2022/06/15
- Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support, John Levon, 2022/06/15
[PATCH 2/2] hw/nvme: Add trace events for shadow doorbell buffer, Jinhao Fan, 2022/06/07