Received-SPF: pass (google.com: domain of linux-kernel+bounces-202447-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-497-g97f96844c-fm-20240526.001-g97f96844
Precedence: bulk
MIME-Version: 1.0
Message-Id: <93cf15d9-6709-4f64-880b-efb178b7bf01@app.fastmail.com>
In-Reply-To: 
 <SJ0PR18MB52462AA2A9F15103D5198CC5A6F82@SJ0PR18MB5246.namprd18.prod.outlook.com>
References: <2024042950-refurbish-duckbill-77b4@gregkh>
 <20240520110630.2568767-1-vattunuru@marvell.com>
 <2024060412-amulet-unflawed-f37a@gregkh>
 <SJ0PR18MB52462AA2A9F15103D5198CC5A6F82@SJ0PR18MB5246.namprd18.prod.outlook.com>
Date: Wed, 05 Jun 2024 11:57:01 +0000
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Vamsi Attunuru" <vattunuru@marvell.com>,
 "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Cc: "Jerin Jacob" <jerinj@marvell.com>,
 "Srujana Challa" <schalla@marvell.com>,
 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [EXTERNAL] Re: [PATCH v7 1/1] misc: mrvl-cn10k-dpi: add Octeon CN10K DPI
 administrative driver
Content-Type: text/plain

On Tue, Jun 4, 2024, at 18:21, Vamsi Krishna Attunuru wrote:
>> From: Greg KH <gregkh@linuxfoundation.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