linux-kernel - Re: [PATCH 3/3] staging: ccree: simplify ioread/iowrite

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:   Mon, 6 Nov 2017 16:46:54 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Gilad Ben-Yossef <gilad@...yossef.com>
Cc:     "Tobin C. Harding" <me@...in.cc>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        devel@...verdev.osuosl.org, driverdev-devel@...uxdriverproject.org,
        Linux kernel mailing list <linux-kernel@...r.kernel.org>,
        Ofir Drang <ofir.drang@....com>
Subject: Re: [PATCH 3/3] staging: ccree: simplify ioread/iowrite

On Mon, Nov 06, 2017 at 10:59:47AM +0200, Gilad Ben-Yossef wrote:
> On Mon, Nov 6, 2017 at 10:37 AM, Tobin C. Harding <me@...in.cc> wrote:
> > On Mon, Nov 06, 2017 at 06:55:52AM +0000, Gilad Ben-Yossef wrote:
> >> Registers ioread/iowrite operations were done via macros,
> >> sometime using a "magical" implicit parameter.
> >>
> >> Replace all register access with simple inline macros.
> >>
> >> Signed-off-by: Gilad Ben-Yossef <gilad@...yossef.com>
> >
> > Hi,
> >
> > Nice work. I had a little trouble following this one. Perhaps you are
> > doing more than one thing per patch, feel free to ignore me if I am
> > wrong but it seems you are moving the macro definition of CC_REG to a
> > different header, adding the new inline functions and doing some other
> > change that I can't grok (commented on below).
> >
> > Perhaps this patch could be broken up.
> 
> Thank you Tobin.
> 
> The original macro that I am replacing had an assumption of a variable void *
> cc_base being defined in the context of the macro being called, even though
> it was not listed in the explicit parameter list of the macro.
> 
> The inline function that replace it instead takes an explicit
> parameter a pointer to
> struct ssi_drive data * , who has said cc_base as one of the fields.
> 
> As a result several function that took a void * cc_base parameter
> (which is than only
> used implicitly via the macro without ever being visibly referenced), now take
> struct ssi_drive data * parameter instead which is passed explicitly
> to the inline
> function.
> 
> These seems to be the places you are referring to. They are cascading changes
> resulting from the change in API between the macro and the inline
> function that replaces it.
> 
> I imagine I can try to break that change to two patches but at least
> in my mind this is artificial
> and it is a single logical change.
> 
> Having said that, if you think otherwise and consider this
> none-reviewable even after this
> explanation let me know and  I'd be happy to break it down.

Nah, this is fine, I'll take it as-is.  Tobin, thanks for the review.

greg k-h