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

[Jinhao Fan]

> On Jun 28, 2022, at 3:33 AM, Klaus Jensen <its@irrelevant.dk> wrote:
> 
> On Jun 27 13:17, Keith Busch wrote:
>> On Thu, Jun 16, 2022 at 08:34:07PM +0800, Jinhao Fan wrote:
>>>     }
>>>     sq->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, nvme_process_sq, sq);
>>> 
>>> +    if (n->dbbuf_enabled) {
>>> +        sq->db_addr = n->dbbuf_dbs + (sqid << 3);
>>> +        sq->ei_addr = n->dbbuf_eis + (sqid << 3);
>>> +    }
>>> +
>>>     assert(n->cq[cqid]);
>>>     cq = n->cq[cqid];
>>>     QTAILQ_INSERT_TAIL(&(cq->sq_list), sq, entry);
>>> @@ -4615,6 +4631,10 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl 
>>> *n, uint64_t dma_addr,
>>>     cq->head = cq->tail = 0;
>>>     QTAILQ_INIT(&cq->req_list);
>>>     QTAILQ_INIT(&cq->sq_list);
>>> +    if (n->dbbuf_enabled) {
>>> +        cq->db_addr = n->dbbuf_dbs + (cqid << 3) + (1 << 2);
>>> +        cq->ei_addr = n->dbbuf_eis + (cqid << 3) + (1 << 2);
>>> +    }
>>>     n->cq[cqid] = cq;
>>>     cq->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, nvme_post_cqes, cq);
>>> }
>> 
>> I just notice these address calculations changed from previous versions. What
>> happened to taking the doorbell stride into account? Spec says the shadows 
>> and
>> events follow the same stride spacing as the registers.
> 
> The stride is fixed at zero.

Yes. I noticed that nvme_process_db() uses a similar hard-coded approach to get
qid from doorbell register offset. So I hard coded here as well. I can submit a 
tiny
patch later to wrap this calculation.