Hi Jerin,
> -----Original Message----- > From: Jerin Jacob <[email protected]> > Sent: Tuesday, December 17, 2019 7:14 PM > To: Gavin Hu <[email protected]> > Cc: Sunil Kumar Kori <[email protected]>; [email protected]; Nithin > Dabilpuram <[email protected]>; Vamsi Attunuru > <[email protected]>; [email protected]; Harman Kalra > <[email protected]>; Honnappa Nagarahalli > <[email protected]>; nd <[email protected]> > Subject: Re: [dpdk-dev] [PATCH v3 2/2] common/octeontx2: add polling > based response mbox message > > > > +mbox_poll(struct otx2_mbox *mbox, uint32_t wait) > > > +{ > > > + uint32_t timeout = 0, sleep = 1; > > > + uint64_t rsp_reg = 0; > > > + uintptr_t reg_addr; > > > + > > > + reg_addr = mbox->reg_base + mbox->intr_offset; > > > + while (!rsp_reg) { > > The first iteration of (!rsp_reg) always evaluate to 'true'. > > Why not use do .. while to save one iteration? > > > + rte_rmb(); > > Rte_rmb is overkill, how is reg_addr mapped(what is the memory attribute? > WC? cacheable? ) > > If it is a PCI mmaped region, then rte_io_barrier is okay, I am proposing to > relax the io barrier for aarch64 as a compiler barrier. > > http://patches.dpdk.org/patch/61662/ > > It is the device memory and Linux kernel(_CPU_ not IO) is updating the > memory on the other side. I think, > we would need rte_smp_rmb() here. Since it was slow-path code, added > the full barrier here. > I think it is OK to change rte_smp_rmb() not to rte_io_* version. Correct? I want to summarize as follows: rte_io_* should be used in this case as it is the device memory. rte_smp_* is for normal memory, including DMA buffer resides in the normal memory and updated by devices. Rte_cio_* should be used for ordering between normal memory and device memory, for example, updates to descriptors in the normal memory and a doorbell write.
