RE: [EXTERNAL] Re: [PATCH v7 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K DPI administrative driver

[Vamsi Krishna Attunuru]

> -----Original Message-----
> From: Arnd Bergmann <arnd@xxxxxxxx>
> Sent: Wednesday, June 5, 2024 5:27 PM
> To: Vamsi Krishna Attunuru <vattunuru@xxxxxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Jerin Jacob <jerinj@xxxxxxxxxxx>; Srujana Challa <schalla@xxxxxxxxxxx>;
> linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [EXTERNAL] Re: [PATCH v7 1/1] misc: mrvl-cn10k-dpi: add
> Octeon CN10K DPI administrative driver
> 
> On Tue, Jun 4, 2024, at 18:21, Vamsi Krishna Attunuru wrote:
> >> From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> 
> >> > +static inline void dpi_reg_write(struct dpipf *dpi, u64 offset,
> >> > +u64
> >> > +val) {
> >> > +	writeq(val, dpi->reg_base + offset);
> >>
> >> No read needed after a write to ensure the write made it to the
> >> hardware properly?
> >
> > Yes, as it's an onboard PCIe device, writes will happen properly. I
> > will modify it as write followed by a write barrier to avoid any
> > reordering.
> 
> I don't think a write barrier after the I/O is what you want here, they don't
> just make I/O finish before the next instruction or unlock.
> 
> When you have like
> 
> 	mutex_lock(&mbox->lock);
> 	msg.word[0] = readq(mbox->vf_pf_data_reg); ...
> 	writeq(DPI_MBOX_TYPE_RSP_ACK, mbox->pf_vf_data_reg);
> 	mutex_unlock(&mbox->lock);
> 
> there is no guarantee that the writeq() completes before the
> mutex_unlock(), regardless of what barriers you put after it:
> even if the device is synchronous, the CPU itself does not wait for the store
> on device memory to complete.
> 
> If you actually need the writeq() to be serialized with the lock, a dummy
> readq() is usually the safe option, though for arm64 specific code, you can
> use ioremap_np() in place of ioremap() to turn the readq() into a non-posted
> store.

Writeq() & readq() are already having the barriers internally (arch/arm64/include/asm/io.h). Below commit describes the requirement.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=22ec71615d824f4f11d38d0e55a88d8956b7e45f

If the driver needs to make sure writes are done then driver can read back at appropriate places.

> 
>      Arnd