On Jun 14 08:41, Keith Busch 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. >
No, I love this hack! :D I have tested your hack against a dbbuf enabled driver that enables shadow doorbells on the admin queue by default. I can confirm that this works as well as on "broken" (or, lets call them "reasonable") drivers. > By the way, I noticed that the patch never updates the cq's ei_addr value. Is > that on purpose? Yeah, I also mentioned this previously[1] and I still think we need to update the event index. Otherwise (and my testing confirms this), we end up in a situation where the driver skips the mmio, leaving a completion queue entry "in use" on the device until some other completion comes along. I have folded these changes into a patch for testing[2]. Note, your patch was missing equivalent changes in nvme_post_cqes(), so I added that as well as updating of the event index. [1]: https://lore.kernel.org/qemu-devel/YqEMwsclktptJvQI@apples/ [2]: http://git.infradead.org/qemu-nvme.git/commitdiff/60712930e441b684490a792b00ef6698cc85f116 Cheers, Klaus
signature.asc
Description: PGP signature
