linux-kernel - Re: [PATCH linux dev-4.10 3/6] drivers/misc: Add driver for Aspeed PECI and generic PECI headers

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: <538056d0-dacc-b27e-db37-9b956dc06496@linux.intel.com>
Date:   Thu, 11 Jan 2018 12:33:51 -0800
From:   Jae Hyun Yoo <jae.hyun.yoo@...ux.intel.com>
To:     Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Greg KH <gregkh@...uxfoundation.org>
Cc:     linux-hwmon@...r.kernel.org, devicetree@...r.kernel.org,
        jdelvare@...e.com, arnd@...db.de, linux-doc@...r.kernel.org,
        andrew@...id.au, openbmc@...ts.ozlabs.org,
        linux-kernel@...r.kernel.org, linux@...ck-us.net,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH linux dev-4.10 3/6] drivers/misc: Add driver for Aspeed
 PECI and generic PECI headers

On 1/11/2018 1:02 AM, Benjamin Herrenschmidt wrote:
> On Wed, 2018-01-10 at 11:18 +0100, Greg KH wrote:
>> On Tue, Jan 09, 2018 at 02:31:23PM -0800, Jae Hyun Yoo wrote:
>>> This commit adds driver implementation for Aspeed PECI. Also adds
>>> generic peci.h and peci_ioctl.h files to provide compatibility
>>> to peci drivers that can be implemented later e.g. Nuvoton's BMC
>>> SoC family.
>>
>> We don't add code that could be used "sometime in the future".  Only
>> include stuff that we use now.
>>
>> Please fix up this series based on that and resubmit.  There should not
>> be any need for any uapi file then, right?
> 
> No Greg, I think you misunderstood (unless I misread myself).
> 
> What Jae means is that since PECI is a standard and other drivers
> implementing the same ioctl interface and messages will eventually go
> upstream, instead of having the ioctl definitions in a driver specific
> locations, they go in a generic spot, as they define a generic API for
> all PECI drivers, including the one that's getting merged now.
> 
> IE. This doesn't add unused stuff, it just puts the API parts of it
> into a generic location.
> 
> At least that's my understanding from a, granted cursory, look at the
> patch.
> 
> That said, I do have a problem with the structure definitions of the
> various packet types as they use "long" which has a variable size and
> unclear alignment. It should be using __u8, __u16 and __u32...
> 
> Cheers,
> Ben.
> 

Thanks for your clear explanation. That is what I actually intended to. 
However, the structure definitions you and Greg pointed out need to be 
corrected. I will fix it.

Thanks,
Jae