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: <0f39c75d-11f3-45ef-b0b5-d11c9f1720ac@app.fastmail.com>
Date: Mon, 17 Jun 2024 15:23:53 +0200
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 Mon, Jun 17, 2024, at 14:55, Vamsi Krishna Attunuru wrote:
>> On Thu, Jun 6, 2024, at 18:42, Vamsi Krishna Attunuru wrote:
>> >> -----Original Message-----
>> >> > -----Original Message-----
>> 
>> >> > 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.
>> >>
>> >
>> > Hi Greg, the ARM64 cores on the Octeon CN10K hardware platform always
>> > run in LE mode and this CN10K DPI PF driver is only supported on
>> > Octeon CN10K platforms as the DPI PF device is an onboard PCIe device.
>> > Can I remove the BE format and only define the LE format for the
>> > dpi_mbox_message structure?, other HW device drivers of Octeon CN10K
>> > platform also only support LE format.
>> 
>> Isn't this a regular Neoverse-N2 core? That means the hardware does
>> support big-endian in principle, though it's usually only used in VM guests,
>> not on bare bare metal and the driver is fairly safe.
>> 
>> In general, I would always suggest writing portable code, as you never know
>> who is going to copy from your driver into something else. Writing this
>> portably is not that hard or less readable than using bit fields.
>> 
>  Yes Arnd, understood your point. I am thing le64_get_bits() would 
> solve the problem here. Can you please confirm.?, I will avoid bit 
> fields and use mask scheme to extract the fields.

I think FIELD_GET() is more appropriate here. Note that readq()
already swaps the data from little-endian into CPU native word
order, so you have a 64-bit number with the bits in a particular
place starting from the low bit, so you don't have to worry about
how the fields inside that word line up with byte boundaries.

You can also just use an open-coded bit shift and mask value.

     Arnd