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]

Date: Tue, 4 Jun 2024 16:21:07 +0000
From: Vamsi Krishna Attunuru <vattunuru@...vell.com>
To: Greg KH <gregkh@...uxfoundation.org>
CC: Jerin Jacob <jerinj@...vell.com>, Srujana Challa <schalla@...vell.com>,
        "arnd@...db.de" <arnd@...db.de>,
        "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



> -----Original Message-----
> From: Greg KH <gregkh@...uxfoundation.org>
> Sent: Tuesday, June 4, 2024 9:23 PM
> To: Vamsi Krishna Attunuru <vattunuru@...vell.com>
> Cc: Jerin Jacob <jerinj@...vell.com>; Srujana Challa <schalla@...vell.com>;
> arnd@...db.de; linux-kernel@...r.kernel.org
> Subject: [EXTERNAL] Re: [PATCH v7 1/1] misc: mrvl-cn10k-dpi: add Octeon
> CN10K DPI administrative driver
> 
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
> 
> ----------------------------------------------------------------------
> On Mon, May 20, 2024 at 04:06:30AM -0700, Vamsi Attunuru wrote:
> > +union dpi_mbox_message {
> > +	u64 word[2];
> > +	struct {
> > +#if defined(__BIG_ENDIAN_BITFIELD)
> > +		/* SSO PF function */
> > +		u64 sso_pf_func  :16;
> > +		/* Aura of the command buffer */
> > +		u64 aura         :20;
> > +		/* Command buffer size in 8-byte words */
> > +		u64 csize        :16;
> > +		/* Command code */
> > +		u64 cmd          :4;
> > +		/* VF ID to configure */
> > +		u64 vfid         :8;
> > +		/* Reserved for future use */
> > +		u64 rsvd_85_127  :40;
> > +		/* Work queue completion status byte offset */
> > +		u64 wqecsoff     :7;
> > +		/* Work queue completion status enable */
> > +		u64 wqecs        :1;
> > +		/* NPA PF function */
> > +		u64 npa_pf_func  :16;
> > +#else
> > +		/* VF ID to configure */
> > +		u64 vfid         :8;
> > +		/* Command code */
> > +		u64 cmd          :4;
> > +		/* Command buffer size in 8-byte words */
> > +		u64 csize        :16;
> > +		/* Aura of the command buffer */
> > +		u64 aura         :20;
> > +		/* SSO PF function */
> > +		u64 sso_pf_func  :16;
> > +		/* NPA PF function */
> > +		u64 npa_pf_func  :16;
> > +		/* Work queue completion status enable */
> > +		u64 wqecs        :1;
> > +		/* Work queue completion status byte offset */
> > +		u64 wqecsoff     :7;
> > +		/* Reserved for future use */
> > +		u64 rsvd_85_127  :40;
> > +#endif
> > +	};
> > +};
> 
> The ifdef is cute, but not correct, sorry.  Please use bit shifts to handle this
> properly without any #ifdef needed at all.
> 
Ack, will fix it next version. Thanks for the suggestion.

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

Thanks
Vamsi
> 
> thanks,
> 
> greg k-h