On Aug 2 16:31, Jinhao Fan wrote: > at 2:02 PM, Klaus Jensen <i...@irrelevant.dk> wrote: > > > On Jul 28 16:07, Jinhao Fan wrote: > >> With the introduction of shadow doorbell and ioeventfd, we need to do > >> frequent conversion between qid and its doorbell offset. The original > >> hard-coded calculation is confusing and error-prone. Add several helper > >> functions to do this task. > >> > >> Signed-off-by: Jinhao Fan <fanjinhao...@ict.ac.cn> > >> --- > >> hw/nvme/ctrl.c | 61 ++++++++++++++++++++++++++++++++------------------ > >> 1 file changed, 39 insertions(+), 22 deletions(-) > >> > >> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c > >> index 533ad14e7a..6116c0e660 100644 > >> --- a/hw/nvme/ctrl.c > >> +++ b/hw/nvme/ctrl.c > >> @@ -487,6 +487,29 @@ static int nvme_check_cqid(NvmeCtrl *n, uint16_t cqid) > >> { > >> return cqid < n->conf_ioqpairs + 1 && n->cq[cqid] != NULL ? 0 : -1; > >> } > >> +static inline bool nvme_db_offset_is_cq(NvmeCtrl *n, hwaddr offset) > >> +{ > >> + hwaddr stride = 4 << NVME_CAP_DSTRD(ldq_le_p(&n->bar.cap)); > >> + return (offset / stride) & 1; > >> +} > > > > This can be changed morphed into `(offset >> (2 + dstrd)) & 1` if I am not > > mistaken. > > > > Yes. But my current code looks more readable to me. Is it necessary to > change to `(offset >> (2 + dstrd)) & 1`. >
I am unsure if the compiler will transform that division into the shift if it can infer that the divisor is a power of two (it most likely will be able to). But I see no reason to have a potential division here when we can do without and to me it is just as readable when you know the definition of DSTRD is `2 ^ (2 + DSTRD)`. > >> + > >> +static inline uint16_t nvme_db_offset_to_qid(NvmeCtrl *n, hwaddr offset) > >> +{ > >> + hwaddr stride = 4 << NVME_CAP_DSTRD(ldq_le_p(&n->bar.cap)); > >> + return offset / (2 * stride); > >> +} > > > > Same, should be able to do `offset >> (2 * dstrd + 1)`, no? > > Same as above. > -- One of us - No more doubt, silence or taboo about mental illness.
signature.asc
Description: PGP signature