linux-kernel - Re: [BUG?] crypto: caam: little/big endianness on ARM vs PPC

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: <20150615220506.GA15134@gondor.apana.org.au>
Date:	Tue, 16 Jun 2015 06:05:06 +0800
From:	Herbert Xu <herbert@...dor.apana.org.au>
To:	Steffen Trumtrar <s.trumtrar@...gutronix.de>
Cc:	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-crypto@...r.kernel.org,
	Ruchika Gupta <ruchika.gupta@...escale.com>,
	kernel@...gutronix.de,
	Horia Geantă <horia.geanta@...escale.com>,
	Kim Phillips <kim.phillips@...escale.com>
Subject: Re: [BUG?] crypto: caam: little/big endianness on ARM vs PPC

On Mon, Jun 15, 2015 at 05:59:07PM +0200, Steffen Trumtrar wrote:
> Hi!
> 
> I'm working on CAAM support for the ARM-based i.MX6 SoCs. The current
> drivers/crypto/caam driver only works for PowerPC AFAIK.
> Actually, there isn't that much to do, to get support for the i.MX6 but
> one patch breaks the driver severely:
> 
> 	commit ef94b1d834aace7101de77c3a7c2631b9ae9c5f6
> 	crypto: caam - Add definition of rd/wr_reg64 for little endian platform
> 
> This patch adds
> 
> +#ifdef __LITTLE_ENDIAN
> +static inline void wr_reg64(u64 __iomem *reg, u64 data)
> +{
> +	wr_reg32((u32 __iomem *)reg + 1, (data & 0xffffffff00000000ull) >> 32);
> +	wr_reg32((u32 __iomem *)reg, data & 0x00000000ffffffffull);
> +}
> 
> The wr_reg64 function is only used in one place in the drivers/crypto/caam/jr.c
> driver: to write the dma_addr_t to the register. Without that patch everything
> works fine on ARM (little endian, 32bit), with that patch the driver will write
> 0's into the register that holds the DMA address (the numerically-higher) -> kernel hangs.
> Also, from my understanding, the comment above the defines, stating that you
> have to first write the numerically-lower and then the numerically-higher address
> on 32bit systems doesn't match with the implementation.
> 
> What I don't know/understand is if this makes any sense for any PowerPC implementation.
> 
> So, the question is, how to fix this? I'd prefer to do it directly in the jr driver
> instead of the ifdef-ery.
> 
> Something like
> 	if (sizeof(dma_addr_t) == sizeof(u32))
> 		wr_reg32(&jrp->rregs->inpring_base + 1, inpbusaddr);
> 	else if (sizeof(dma_addr_t) == sizeof(u64))
> 		wr_reg64(...)
> 
> or just go by DT compatible and then remove the inline function definitions.
> 
> As far as I can tell, the compatible wouldn't be needed for anything else in the
> jr driver, so maybe that is not optimal. On the other hand the sizeof(..)
> solution would only catch little endian on 32bit and not big endian (?!)
> I however don't know what combinations actually *have* to be caught, as I don't
> know, which exist.
> 
> So, what do you think people?

I'm adding a couple of CCs.

Thanks,
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/