linux-kernel - Re: [PATCH v10 3/4] fpga: dfl: add basic support for DFHv1

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: <alpine.DEB.2.22.394.2301120732500.845139@rhweight-WRK1>
Date:   Thu, 12 Jan 2023 07:36:29 -0800 (PST)
From:   matthew.gerlach@...ux.intel.com
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
cc:     Xu Yilun <yilun.xu@...el.com>, hao.wu@...el.com,
        russell.h.weight@...el.com, basheer.ahmed.muddebihal@...el.com,
        trix@...hat.com, mdf@...nel.org, linux-fpga@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        tianfei.zhang@...el.com, corbet@....net,
        gregkh@...uxfoundation.org, linux-serial@...r.kernel.org,
        jirislaby@...nel.org, geert+renesas@...der.be,
        niklas.soderlund+renesas@...natech.se, macro@...am.me.uk,
        johan@...nel.org, lukas@...ner.de, ilpo.jarvinen@...ux.intel.com,
        marpagan@...hat.com, bagasdotme@...il.com
Subject: Re: [PATCH v10 3/4] fpga: dfl: add basic support for DFHv1



On Thu, 12 Jan 2023, Andy Shevchenko wrote:

> On Wed, Jan 11, 2023 at 10:13:31AM +0800, Xu Yilun wrote:
>> On 2023-01-10 at 14:07:16 -0800, matthew.gerlach@...ux.intel.com wrote:
>>> On Tue, 10 Jan 2023, Andy Shevchenko wrote:
>>>> On Mon, Jan 09, 2023 at 04:30:28PM -0800, matthew.gerlach@...ux.intel.com wrote:
>>>>> From: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
>
> ...
>
>>>>> v10: change dfh_find_param to return size of parameter data in bytes
>>>>
>>>> The problem that might occur with this approach is byte ordering.
>>>> When we have u64 items, we know that they all are placed in CPU
>>>> ordering by the bottom layer. What's the contract now? Can it be
>>>> a problematic? Please double check this (always keep in mind BE32
>>>> as most interesting case for u64/unsigned long representation and
>>>> other possible byte ordering outcomes).
>>>
>>> A number of u64 items certainly states explicit alignment of the memory, but
>>> I think byte ordering is a different issue.
>>>
>>> The bottom layer, by design, is still enforcing a number u64 items under the
>>> hood. So the contract has not changed. Changing units of size from u64s to
>>> bytes was suggested to match the general practice of size of memory being in
>>> bytes. I think the suggestion was made because the return type for
>>> dfh_find_param() changed from u64* to void* in version 9, when indirectly
>>> returning the size of the parameter data was introduced.  So a void * with a
>>> size in bytes makes sense. On the other hand, returning a u64 * is a more
>>> precise reflection of the data alignment. I think the API should be as
>>
>> I prefer (void *) + bytes. The properties in the parameter block are not
>> guarateed to be u64 for each, e.g. the REG_LAYOUT, so (void *) could better
>> indicate it is not. It is just a block of data unknown to DFL core and to
>> be parsed by drivers.
>
> If the hardware / protocol is capable of communicating the arbitrary lengths
> of parameters, then yes, bytes make sense. But this should be clear what byte
> ordering is there if the items can be words / dwords / qwords.

The hardware does communicate the arbitrary lengths of the parameter data; 
so bytes make sense.  I will update Documentation/fpga/dfl.rst to 
explicitly say that multi-byte quantities are little-endian.

>
> TL;DR: The Q is: Is the parameter block a byte stream? If yes, then your
> proposal is okay. If no, no void * should be used. In the latter it should
> be union of possible items or a like as defined by a protocol.

The parameter block is not a byte stream; so void * should be used.

Thanks,
Matthew Gerlach


>
>> And why users/drivers need to care about the alignment of the parameter
>> block?
>>
>>> follows:
>
> -- 
> With Best Regards,
> Andy Shevchenko
>
>
>