Hi Jerin,
> -----Original Message-----
> From: Jerin Jacob <jerinjac...@gmail.com>
> Sent: Tuesday, December 17, 2019 7:14 PM
> To: Gavin Hu <gavin...@arm.com>
> Cc: Sunil Kumar Kori <sk...@marvell.com>; jer...@marvell.com; Nithin
> Dabilpuram <ndabilpu...@marvell.com>; Vamsi Attunuru
> <vattun...@marvell.com>; dev@dpdk.org; Harman Kalra
> <hka...@marvell.com>; Honnappa Nagarahalli
> <honnappa.nagaraha...@arm.com>; nd <n...@arm.com>
> 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.
