Re: [PATCH 1/2] hw/nvme: Implement shadow doorbell buffer support

[Jinhao Fan]

> On Jun 14, 2022, at 5:15 AM, Keith Busch <kbusch@kernel.org> wrote:
> 
> 
> @@ -6538,9 +6544,25 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, 
> int val)
> 
>         trace_pci_nvme_mmio_doorbell_sq(sq->sqid, new_tail);
> 
> -        if (!sq->db_addr) {
>         sq->tail = new_tail;
> +        if (sq->db_addr) {
> +            /*
> +             * The spec states "the host shall also update the controller's
> +             * corresponding doorbell property to match the value of that 
> entry
> +             * in the Shadow Doorbell buffer."
> +             *
> +             * Since this context is currently a VM trap, we can safely 
> enforce
> +             * the requirement from the device side in case the host is
> +             * misbehaving.
> +             *
> +             * Note, we shouldn't have to do this, but various drivers
> +             * including ones that run on Linux, are not updating Admin 
> Queues,
> +             * so we can't trust reading it for an appropriate sq tail.
> +             */
> +            pci_dma_write(&n->parent_obj, sq->db_addr, &sq->tail,
> +                    sizeof(sq->tail));
>         }
> +
>         timer_mod(sq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
>     }
> }
> --

Thanks Keith,

This is an interesting hack. I wonder how should I incorporate your changes in 
my patch. I guess I can modify the code in PATCH 1/2 and add a “Proposed-by” 
tag. Is this the correct way?

Regards,
Jinhao Fan