linux-kernel - Re: [PATCH v2 3/3] crypto: caam: cleanup CONFIG

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:   Wed, 28 Jun 2017 10:51:15 -0600
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        linux-ntb@...glegroups.com, linux-crypto@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jyri Sarha <jsarha@...com>,
        Stephen Bates <sbates@...thlin.com>,
        Horia Geantă <horia.geanta@....com>,
        Dan Douglass <dan.douglass@....com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH v2 3/3] crypto: caam: cleanup CONFIG_64BIT ifdefs when
 using io{read|write}64



On 28/06/17 04:20 AM, Arnd Bergmann wrote:
> On Wed, Jun 28, 2017 at 1:02 AM, Logan Gunthorpe <logang@...tatee.com> wrote:
>>  #include <linux/types.h>
>>  #include <linux/bitops.h>
>> -#include <linux/io.h>
>> +#include <linux/io-64-nonatomic-hi-lo.h>
> 
> Here you include the hi-lo variant unconditionally.
> 
>> -#else /* CONFIG_64BIT */
>> -static inline void wr_reg64(void __iomem *reg, u64 data)
>> -{
>> -#ifndef CONFIG_CRYPTO_DEV_FSL_CAAM_IMX
>> -       if (caam_little_end) {
>> -               wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
>> -               wr_reg32((u32 __iomem *)(reg), data);
>> -       } else
>>  #endif
>> -       {
>> -               wr_reg32((u32 __iomem *)(reg), data >> 32);
>> -               wr_reg32((u32 __iomem *)(reg) + 1, data);
>> -       }
>> +               iowrite64be(data, reg);
>>  }
> 
> However, the #else path here uses lo-hi instead. I guess we have
> to decide how to define iowrite64be_lo_hi() first: it could
> either byteswap the 64-bit value first, then write the two halves,
> or it could write the two halves, doing a 32-bit byte swap on
> each.

Ok, I studied this a bit more:

The lo_hi/hi_lo functions seem to always refer to the data being written
or read not to the address operated on. So, in the v3 version of this
set, which I'm working on, I've defined:

static inline void iowrite64_hi_lo(u64 val, void __iomem *addr)
{
        iowrite32(val >> 32, addr + sizeof(u32));
        iowrite32(val, addr);
}

static inline void iowrite64be_hi_lo(u64 val, void __iomem *addr)
{
        iowrite32be(val >> 32, addr);
        iowrite32be(val, addr + sizeof(u32));
}

So the two hi_lo functions match both paths of the #if and thus, I
believe, the patch will be correct in v3 without changes.

Thanks,

Logan