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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <93cf15d9-6709-4f64-880b-efb178b7bf01@app.fastmail.com>
Date: Wed, 05 Jun 2024 11:57:01 +0000
From: "Arnd Bergmann" <arnd@...db.de>
To: "Vamsi Attunuru" <vattunuru@...vell.com>,
 "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>
Cc: "Jerin Jacob" <jerinj@...vell.com>,
 "Srujana Challa" <schalla@...vell.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
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@...uxfoundation.org>

>> > +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.

     Arnd