On Tue, May 28, 2019 at 06:41:28PM +0000, Michal Kalderon wrote:
> > From: Jason Gunthorpe <jgg@xxxxxxxx>
> > Sent: Tuesday, May 28, 2019 7:16 PM
> >
> > On Tue, May 28, 2019 at 02:24:00PM +0300, Michal Kalderon wrote:
> >
> > > +static int qedr_init_user_db_rec(struct ib_udata *udata,
> > > + struct qedr_dev *dev, struct qedr_userq *q,
> > > + u64 db_rec_addr, int access, int dmasync) {
> > > + /* Aborting for non doorbell userqueue (SRQ) */
> > > + if (db_rec_addr == 0)
> > > + return 0;
> > > +
> > > + q->db_rec_addr = db_rec_addr;
> > > + q->db_rec_umem = ib_umem_get(udata, q->db_rec_addr,
> > PAGE_SIZE,
> > > + access, dmasync);
> > > +
> > > + if (IS_ERR(q->db_rec_umem)) {
> > > + DP_ERR(dev,
> > > + "create user queue: failed db_rec ib_umem_get, error
> > was %ld, db_rec_addr was %llx\n",
> > > + PTR_ERR(q->db_rec_umem), db_rec_addr);
> > > + return PTR_ERR(q->db_rec_umem);
> > > + }
> > > +
> > > + q->db_rec_page = sg_page(q->db_rec_umem->sg_head.sgl);
> > > + q->db_rec_virt = kmap(q->db_rec_page);
> >
> > Is this something new? You are much better to use user-triggered mmap to
> > get a shared page than to use long term kmap.
>
> This was the fix for previously using sg_virt which as you stated won't always work.
> Just to make sure I understand, by user-triggered mmap do you mean allocating the
> memory in kernel and passing the physical pointer to user to mmap it
> ?
Yes, if the ABI allows for it, this is a better choice for this kind
of long lived usage.
> > > cq->ibcq.cqe = chain_entries;
> > > + cq->q.db_addr = (void __iomem *)(uintptr_t)ctx->dpi_addr +
> > > + db_offset;
> >
> > Seems like something has gone wrong here if you have to type __iomem like
> > this
> The dpi_addr is an io address to the doorbell-bar received from the qed module,
> the qed/qedr interface passes it as a u64 ( it is casted from u8 __iomem * to u64)
> so I need to cast it back.
Don't cast __iommem * to u8. Make a patch to fix it.
Jason
