Re: [PATCH] NVMe: init nvme queue before enabling irq

[Sagi Grimberg <sagig@dev.mellanox.co.il>]

> During reset process, the nvme_dev->bar (ioremapped) may change,
> so nvmeq->q_db shall be also updated by nvme_init_queue().
>
> Currently nvmeq irq is enabled before queue init, so a spurious
> interrupt triggered nvme_process_cq may access nvmeq->q_db just
> before it is updated, this could cause kernel panic.
>
> Signed-off-by: Wenbo Wang <wenbo.wang@memblaze.com>
> Reviewed-by: Wenwei Tao <wenwei.tao@memblaze.com>

This patch makes sense to me.

> ---
>   drivers/nvme/host/pci.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index f5c0e26..df55f28 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1590,11 +1590,17 @@ static int nvme_create_queue(struct nvme_queue *nvmeq, int qid)
>   	if (result < 0)
>   		goto release_cq;
>
> +	/*
> +	 * Init queue door bell ioremap address before enabling irq, if not,
> +	 * a spurious interrupt triggered nvme_process_cq may access invalid
> +	 * address
> +	 */

Not sure we need an explicit comment on the fix here. But I don't
mind keeping it.

> +	nvme_init_queue(nvmeq, qid);
> +

I think that this makes the duplication in nvme_alloc_queue redundant.
Can you remove it?

Other then that, looks good to me,

Reviewed-by: Sagi Grimberg <sagig@mellanox.com>