Re: [PATCH 1/2] crypto: vmx - Remove overly verbose printk from AES init routines

[Michael Ellerman]

Herbert Xu <herb...@gondor.apana.org.au> writes:

> On Thu, May 03, 2018 at 10:29:29PM +1000, Michael Ellerman wrote:
>> In the vmx AES init routines we do a printk(KERN_INFO ...) to report
>> the fallback implementation we're using.
>> 
>> However with a slow console this can significantly affect the speed of
>> crypto operations. Using 'cryptsetup benchmark' the removal of the
>> printk() leads to a ~5x speedup for aes-cbc decryption.
>> 
>> So remove them.
>> 
>> Fixes: 8676590a1593 ("crypto: vmx - Adding AES routines for VMX module")
>> Fixes: 8c755ace357c ("crypto: vmx - Adding CBC routines for VMX module")
>> Fixes: 4f7f60d312b3 ("crypto: vmx - Adding CTR routines for VMX module")
>> Fixes: cc333cd68dfa ("crypto: vmx - Adding GHASH routines for VMX module")
>> Cc: sta...@vger.kernel.org # v4.1+
>> Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
>> ---
>>  drivers/crypto/vmx/aes.c | 2 --
>>  drivers/crypto/vmx/aes_cbc.c | 3 ---
>>  drivers/crypto/vmx/aes_ctr.c | 2 --
>>  drivers/crypto/vmx/ghash.c   | 2 --
>>  4 files changed, 9 deletions(-)
>
> Patch applied.  Thanks.

Thanks.

>> If this is the wrong fix please let me know, I'm not a crypto expert.
>> 
>> What we see is 'cryptsetup benchmark' causing thousands of these printks() to
>> happen. The call trace is something like:
>> 
>> [c01e47867a60] [c09cf6b4] p8_aes_cbc_init+0x74/0xf0
>> [c01e47867ae0] [c0551a80] __crypto_alloc_tfm+0x1d0/0x2c0
>> [c01e47867b20] [c055aea4] crypto_skcipher_init_tfm+0x124/0x280
>> [c01e47867b60] [c055138c] crypto_create_tfm+0x9c/0x1a0
>> [c01e47867ba0] [c0552220] crypto_alloc_tfm+0xa0/0x140
>> [c01e47867c00] [c055b168] crypto_alloc_skcipher+0x48/0x70
>> [c01e47867c40] [c057af28] skcipher_bind+0x38/0x50
>> [c01e47867c80] [c057a07c] alg_bind+0xbc/0x220
>> [c01e47867d10] [c0a016a0] __sys_bind+0x90/0x100
>> [c01e47867df0] [c0a01750] sys_bind+0x40/0x60
>> [c01e47867e30] [c000b320] system_call+0x58/0x6c
>> 
>> 
>> Is it normal for init to be called on every system call?
>
> This is the tfm init function, so yes it is called every time
> you allocate a tfm.

OK thanks. So we just shouldn't be printk'ing in there in the non-error
path, good to know.

cheers

[PATCH 2/2] crypto: vmx - Remove overly verbose printk from AES XTS init

[Michael Ellerman]

In p8_aes_xts_init() we do a printk(KERN_INFO ...) to report the
fallback implementation we're using. However with a slow console this
can significantly affect the speed of crypto operations. So remove it.

Fixes: c07f5d3da643 ("crypto: vmx - Adding support for XTS")
Cc: sta...@vger.kernel.org # v4.8+
Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---
 drivers/crypto/vmx/aes_xts.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c
index 8cd6e62e4c90..8bd9aff0f55f 100644
--- a/drivers/crypto/vmx/aes_xts.c
+++ b/drivers/crypto/vmx/aes_xts.c
@@ -53,8 +53,6 @@ static int p8_aes_xts_init(struct crypto_tfm *tfm)
alg, PTR_ERR(fallback));
return PTR_ERR(fallback);
}
-   printk(KERN_INFO "Using '%s' as fallback implementation.\n",
-   crypto_skcipher_driver_name(fallback));
 
crypto_skcipher_set_flags(
fallback,
-- 
2.14.1

[PATCH 1/2] crypto: vmx - Remove overly verbose printk from AES init routines

[Michael Ellerman]

In the vmx AES init routines we do a printk(KERN_INFO ...) to report
the fallback implementation we're using.

However with a slow console this can significantly affect the speed of
crypto operations. Using 'cryptsetup benchmark' the removal of the
printk() leads to a ~5x speedup for aes-cbc decryption.

So remove them.

Fixes: 8676590a1593 ("crypto: vmx - Adding AES routines for VMX module")
Fixes: 8c755ace357c ("crypto: vmx - Adding CBC routines for VMX module")
Fixes: 4f7f60d312b3 ("crypto: vmx - Adding CTR routines for VMX module")
Fixes: cc333cd68dfa ("crypto: vmx - Adding GHASH routines for VMX module")
Cc: sta...@vger.kernel.org # v4.1+
Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---
 drivers/crypto/vmx/aes.c | 2 --
 drivers/crypto/vmx/aes_cbc.c | 3 ---
 drivers/crypto/vmx/aes_ctr.c | 2 --
 drivers/crypto/vmx/ghash.c   | 2 --
 4 files changed, 9 deletions(-)


If this is the wrong fix please let me know, I'm not a crypto expert.

What we see is 'cryptsetup benchmark' causing thousands of these printks() to
happen. The call trace is something like:

[c01e47867a60] [c09cf6b4] p8_aes_cbc_init+0x74/0xf0
[c01e47867ae0] [c0551a80] __crypto_alloc_tfm+0x1d0/0x2c0
[c01e47867b20] [c055aea4] crypto_skcipher_init_tfm+0x124/0x280
[c01e47867b60] [c055138c] crypto_create_tfm+0x9c/0x1a0
[c01e47867ba0] [c0552220] crypto_alloc_tfm+0xa0/0x140
[c01e47867c00] [c055b168] crypto_alloc_skcipher+0x48/0x70
[c01e47867c40] [c057af28] skcipher_bind+0x38/0x50
[c01e47867c80] [c057a07c] alg_bind+0xbc/0x220
[c01e47867d10] [c0a016a0] __sys_bind+0x90/0x100
[c01e47867df0] [c0a01750] sys_bind+0x40/0x60
[c01e47867e30] [c000b320] system_call+0x58/0x6c


Is it normal for init to be called on every system call?

cheers


diff --git a/drivers/crypto/vmx/aes.c b/drivers/crypto/vmx/aes.c
index 96072b9b55c4..d7316f7a3a69 100644
--- a/drivers/crypto/vmx/aes.c
+++ b/drivers/crypto/vmx/aes.c
@@ -48,8 +48,6 @@ static int p8_aes_init(struct crypto_tfm *tfm)
   alg, PTR_ERR(fallback));
return PTR_ERR(fallback);
}
-   printk(KERN_INFO "Using '%s' as fallback implementation.\n",
-  crypto_tfm_alg_driver_name((struct crypto_tfm *) fallback));
 
crypto_cipher_set_flags(fallback,
crypto_cipher_get_flags((struct
diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
index 7394d35d5936..5285ece4f33a 100644
--- a/drivers/crypto/vmx/aes_cbc.c
+++ b/drivers/crypto/vmx/aes_cbc.c
@@ -52,9 +52,6 @@ static int p8_aes_cbc_init(struct crypto_tfm *tfm)
   alg, PTR_ERR(fallback));
return PTR_ERR(fallback);
}
-   printk(KERN_INFO "Using '%s' as fallback implementation.\n",
-   crypto_skcipher_driver_name(fallback));
-
 
crypto_skcipher_set_flags(
fallback,
diff --git a/drivers/crypto/vmx/aes_ctr.c b/drivers/crypto/vmx/aes_ctr.c
index fc60d00a2e84..cd777c75291d 100644
--- a/drivers/crypto/vmx/aes_ctr.c
+++ b/drivers/crypto/vmx/aes_ctr.c
@@ -50,8 +50,6 @@ static int p8_aes_ctr_init(struct crypto_tfm *tfm)
   alg, PTR_ERR(fallback));
return PTR_ERR(fallback);
}
-   printk(KERN_INFO "Using '%s' as fallback implementation.\n",
-   crypto_skcipher_driver_name(fallback));
 
crypto_skcipher_set_flags(
fallback,
diff --git a/drivers/crypto/vmx/ghash.c b/drivers/crypto/vmx/ghash.c
index 27a94a119009..1c4b5b889fba 100644
--- a/drivers/crypto/vmx/ghash.c
+++ b/drivers/crypto/vmx/ghash.c
@@ -64,8 +64,6 @@ static int p8_ghash_init_tfm(struct crypto_tfm *tfm)
   alg, PTR_ERR(fallback));
return PTR_ERR(fallback);
}
-   printk(KERN_INFO "Using '%s' as fallback implementation.\n",
-  crypto_tfm_alg_driver_name(crypto_shash_tfm(fallback)));
 
crypto_shash_set_flags(fallback,
   crypto_shash_get_flags((struct crypto_shash
-- 
2.14.1

Re: [PATCH v15 0/9] Add io{read|write}64 to io-64-atomic headers

[Michael Ellerman]

Logan Gunthorpe <log...@deltatee.com> writes:
> On 4/4/2018 4:38 AM, Michael Ellerman wrote:
...
>> eg. It looks like I could take the two powerpc patches on their own for
>> 4.17, and then the rest could go via other trees?
>
> Yup! If you can take the powerpc patches I can keep trying to get the 
> rest in. They are largely independent and shouldn't really change 
> anything without the following patches.

OK, I'll take those then.

>> Is patch 1 stand alone?
>
> Essentially, yes, but patch 5 depends on it seeing it's changing the 
> same area and is trying to avoid creating the same kbuild warnings that 
> patch 1 suppresses.
>
>> The other option is to ask Andrew Morton to take it, as he often carries
>> these cross-tree type series.
>
> Thanks for the tip! I'll copy him when I repost it after the merge window.

Cool. If you want him to take it you should probably explicitly say so
when you repost.

cheers

Re: [PATCH v15 0/9] Add io{read|write}64 to io-64-atomic headers

[Michael Ellerman]

Logan Gunthorpe  writes:

> This is v14 of my cleanup series to push a number of instances of people
> defining their own io{read|write}64 functions into common headers seing
> they don't exist in non-64bit systems. This series adds inline functions to 
> the
> io-64-nonatomic headers and then cleans up the drivers that defined their
> own copies.
>
> This cleanup was originally requested by Greg after he reviewed my
> Switchtec NTB code. And I hope someone can pick it up or at least give
> feedback on it soon as it's been around relatively unchanged for a few
> cycles now and I'm getting a bit tired of resubmitting it with little to
> no interest.

Yeah fair enough.

What's the plan for getting it merged? Seems we don't have one?

Given it touches two arches and generic stuff and drivers and crypto,
it's a bit of a mess to merge. It's not really something I want to merge
via the powerpc tree.

Is there any way to split it? I couldn't immediately see what the hard
dependencies were between the patches.

eg. It looks like I could take the two powerpc patches on their own for
4.17, and then the rest could go via other trees?

Is patch 1 stand alone?

The other option is to ask Andrew Morton to take it, as he often carries
these cross-tree type series.

cheers

> Changes since v14:
> - Rebased onto v4.16-rc7
> - Replace the first two patches so that instead of correcting the
>   endianness annotations we change to using writeX() and readX() with
>   swabX() calls. This makes the big-endian functions more symmetric
>   with the little-endian versions (with respect to barriers that are
>   not included in the raw functions). As a side effect, it also fixes
>   the kbuild warnings that the first two patches tried to address.
>
> Changes since v13:
> - Changed the subject of patch 0001 to correct a nit pointed out by Luc
>
> Changes since v12:
> - Rebased onto v4.16-rc6
> - Split patch 0001 into two and reworked the commit log as requested
>   by Luc Van Oostenryck
>
> Changes since v11:
> - Rebased onto v4.16-rc5
> - Added a patch (0001) to fix some old and new sparse warnings
>   that the kbuild robot warned about this cycle. The latest version
>   of sparse was required to reproduce these.
> - Added a patch (0002) to add io{read|write}64 to parisc which the kbuild
>   robot also found errors for this cycle
>
> Changes since v10:
> - Rebased onto v4.16-rc4, this droped the drm/tilcdc patch which was
>   picked up by that tree and is already in 4.16.
>
> Changes since v9:
> - Rebased onto v4.15-rc6
> - Fixed a couple of issues in the new version of the CAAM patch as
>   pointed out by Horia
>
> Changes since v8:
> - Rebased onto v4.15-rc2, as a result rewrote patch 7 seeing someone did
>   some similar cleanup in that area.
> - Added a patch to clean up the Switchtec NTB driver which landed in
>   v4.15-rc1
>
> Changes since v7:
> - Fix minor nits from Andy Shevchenko
> - Rebased onto v4.14-rc1
>
> Changes since v6:
>  ** none **
>
> Changes since v5:
> - Added a fix to the tilcdc driver to ensure it doesn't use the
>   non-atomic operation. (This includes adding 
> io{read|write}64[be]_is_nonatomic
>   defines).
>
> Changes since v4:
> - Add functions so the powerpc implementation of iomap.c compiles. (As
>   noticed by Horia)
>
> Changes since v3:
>
> - I noticed powerpc didn't use the appropriate functions seeing
>   readq/writeq were not defined when iomap.h was included. Thus I've
>   included a patch to adjust this
> - Fixed some mistakes with a couple of the defines in io-64-nonatomic*
>   headers
> - Fixed a typo noticed by Horia.
>
> (earlier versions were drastically different)
>
> --
>
> Logan Gunthorpe (9):
>   iomap: Use non-raw io functions for io{read|write}XXbe
>   parisc: iomap: introduce io{read|write}64
>   powerpc: io.h: move iomap.h include so that it can use readq/writeq
> defs
>   powerpc: iomap.c: introduce io{read|write}64_{lo_hi|hi_lo}
>   iomap: introduce io{read|write}64_{lo_hi|hi_lo}
>   io-64-nonatomic: add io{read|write}64[be]{_lo_hi|_hi_lo} macros
>   ntb: ntb_hw_intel: use io-64-nonatomic instead of in-driver hacks
>   crypto: caam: cleanup CONFIG_64BIT ifdefs when using io{read|write}64
>   ntb: ntb_hw_switchtec: Cleanup 64bit IO defines to use the common
> header
>
>  arch/parisc/include/asm/io.h   |   9 +++
>  arch/parisc/lib/iomap.c|  64 +++
>  arch/powerpc/include/asm/io.h  |   6 +-
>  arch/powerpc/kernel/iomap.c|  40 ++
>  drivers/crypto/caam/regs.h |  30 +--
>  drivers/ntb/hw/intel/ntb_hw_intel.c|  30 +--
>  drivers/ntb/hw/mscc/ntb_hw_switchtec.c |  36 +
>  include/asm-generic/iomap.h|  26 --
>  include/linux/io-64-nonatomic-hi-lo.h  |  64 +++
>  include/linux/io-64-nonatomic-lo-hi.h  |  64 +++
>  lib/iomap.c| 140 
> -
>  11 files changed, 409 

Re: [1/2] crypto/nx: Use percpu send window for NX requests

[Michael Ellerman]

On Mon, 2017-09-25 at 06:43:02 UTC, Haren Myneni wrote:
> [PATCH 1/2] crypto/nx: Use percpu send window for NX requests
> 
> For P9 NX, the send window is opened for each crypto session and closed
> upon free. But VAS supports 64K windows per chip for all coprocessors
> including in user space support. So there is a possibility of not
> getting the window for kernel requests.
> 
> This patch reserves windows for each coprocessor type (NX842) and are
> available forever for kernel requests, Opens each window for each CPU
> on the corresponding chip during driver initialization. So then use the
> percpu txwin for NX requests depends on the CPU on which the process
> is executing.
> 
> Signed-off-by: Haren Myneni 

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/976dd6490b1b45727733a3ee1e25e1

cheers

Re: [V4,1/7] crypto/nx: Rename nx842_powernv_function as icswx function

[Michael Ellerman]

On Thu, 2017-08-31 at 07:11:29 UTC, Haren Myneni wrote:
> Rename nx842_powernv_function to nx842_powernv_exec.
> nx842_powernv_exec points to nx842_exec_icswx and
> will be point to VAS exec function which will be added later
> for P9 NX support.
> 
> Signed-off-by: Haren Myneni 

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/c97f8169fb227cae5adeac56cafa98

cheers

Re: [PATCH V3 6/6] crypto/nx: Add P9 NX support for 842 compression engine

[Michael Ellerman]

Haren Myneni <ha...@linux.vnet.ibm.com> writes:
>> On Mon, Aug 28, 2017 at 7:25 PM, Michael Ellerman <m...@ellerman.id.au> 
>> wrote:
>>> Hi Haren,
>>>
>>> Some comments inline ...
>>>
>>> Haren Myneni <ha...@linux.vnet.ibm.com> writes:
>>>
>>>> diff --git a/drivers/crypto/nx/nx-842-powernv.c 
>>>> b/drivers/crypto/nx/nx-842-powernv.c
>>>> index c0dd4c7e17d3..13089a0b9dfa 100644
>>>> --- a/drivers/crypto/nx/nx-842-powernv.c
>>>> +++ b/drivers/crypto/nx/nx-842-powernv.c
>>>> @@ -32,6 +33,9 @@ MODULE_ALIAS_CRYPTO("842-nx");
>>>>
>>>>  #define WORKMEM_ALIGN(CRB_ALIGN)
>>>>  #define CSB_WAIT_MAX (5000) /* ms */
>>>> +#define VAS_RETRIES  (10)
>>>
>>> Where does that number come from?
>
> Sometimes HW returns copy/paste failures. So we should retry the
> request again. With 10 retries, Test running 12 hours was successful
> for repeated compression/decompression requests with 1024 threads.

But why 10. Why not 5, or 100, or 1, or 10,000?

Presumably when we have to retry it means the NX is too busy to service the
request?

Do we have any way to find out how long it might be busy for?

Should we try an NX on another chip?

We should also take into account the size of our request, ie. are we
asking the NX to compress one page, or 1GB ?

If it's just one page maybe we should fall back to software immediately.

cheers

Re: [PATCH V3 6/6] crypto/nx: Add P9 NX support for 842 compression engine

[Michael Ellerman]

Hi Dan,

Thanks for reviewing this series.

Dan Streetman  writes:
> On Tue, Aug 29, 2017 at 5:54 PM, Haren Myneni  
> wrote:
>> On 08/29/2017 02:23 PM, Benjamin Herrenschmidt wrote:
>>> On Tue, 2017-08-29 at 09:58 -0400, Dan Streetman wrote:
> +
> +   ret = -EINVAL;
> +   if (coproc && coproc->vas.rxwin) {
> +   wmem->txwin = nx842_alloc_txwin(coproc);

 this is wrong.  the workmem is scratch memory that's valid only for
 the duration of a single operation.
>>
>> Correct, workmem is used until crypto_free is called.
>
> that's not a 'single operation'.  a single operation is compress() or
> decompress().
>

 do you actually need a txwin per crypto transform?  or do you need a
 txwin per coprocessor?  or txwin per processor?  either per-coproc or
 per-cpu should be created at driver init and held separately
 (globally) instead of a per-transform txwin.  I really don't see why
 you would need a txwin per transform, because the coproc should not
 care how many different transforms there are.
>>>
>>> We should only need a single window for the whole kernel really, plus
>>> one per user process who wants direct access but that's not relevant
>>> here.
>>
>> Opening send window for each crypto transform (crypto_alloc,
>> compression/decompression, ..., crypto_free) so that does not
>> have to wait for the previous copy/paste complete.
>> VAS will map send and receive windows, and can cache in send
>> windows (up to 128). So I thought using the same send window
>> (per chip) for more requests (say 1000) may be adding overhead.
>>
>> I will make changes if you prefer using 1 send window per chip.
>
> i don't have the spec, so i shouldn't be making the decision on it,
> but i do know putting a persistent field into the workmem is the wrong
> location.  If it's valid for the life of the transform, put it into
> the transform context.  The workmem buffer is intended to be used only
> during a single operation - it's "working memory" to perform each
> individual crypto transformation.

I agree workmem isn't the right place for the txwin. But I don't believe
it actually breaks anything to put txwin there.

So for now I'm going to merge this series as-is and I've asked Haren to
send fixes as soon as he can to clean it up.

cheers

Re: [PATCH V3 6/6] crypto/nx: Add P9 NX support for 842 compression engine

[Michael Ellerman]

Hi Haren,

Some comments inline ...

Haren Myneni  writes:

> diff --git a/drivers/crypto/nx/nx-842-powernv.c 
> b/drivers/crypto/nx/nx-842-powernv.c
> index c0dd4c7e17d3..13089a0b9dfa 100644
> --- a/drivers/crypto/nx/nx-842-powernv.c
> +++ b/drivers/crypto/nx/nx-842-powernv.c
> @@ -32,6 +33,9 @@ MODULE_ALIAS_CRYPTO("842-nx");
>  
>  #define WORKMEM_ALIGN(CRB_ALIGN)
>  #define CSB_WAIT_MAX (5000) /* ms */
> +#define VAS_RETRIES  (10)

Where does that number come from?

Do we have any idea what the trade off is between retrying vs just
falling back to doing the request in software?

> +/* # of requests allowed per RxFIFO at a time. 0 for unlimited */
> +#define MAX_CREDITS_PER_RXFIFO   (1024)
>  
>  struct nx842_workmem {
>   /* Below fields must be properly aligned */
> @@ -42,16 +46,27 @@ struct nx842_workmem {
>  
>   ktime_t start;
>  
> + struct vas_window *txwin;   /* Used with VAS function */

I don't understand how it makes sense to put txwin and start between the
fields above, and the padding.

If the workmem pointer you receive is not aligned, then PTR_ALIGN() will
advance it and mean you end up writing over start and txwin.

That's probably not your bug, the code is already like that.

>   char padding[WORKMEM_ALIGN]; /* unused, to allow alignment */
>  } __packed __aligned(WORKMEM_ALIGN);

> @@ -576,6 +690,198 @@ static inline void nx842_add_coprocs_list(struct 
> nx842_coproc *coproc,
>   list_add(>list, _coprocs);
>  }
>  
> +/*
> + * Identify chip ID for each CPU and save coprocesor adddress for the
> + * corresponding NX engine in percpu coproc_inst.
> + * coproc_inst is used in crypto_init to open send window on the NX instance
> + * for the corresponding CPU / chip where the open request is executed.
> + */
> +static void nx842_set_per_cpu_coproc(struct nx842_coproc *coproc)
> +{
> + unsigned int i, chip_id;
> +
> + for_each_possible_cpu(i) {
> + chip_id = cpu_to_chip_id(i);
> +
> + if (coproc->chip_id == chip_id)
> + per_cpu(coproc_inst, i) = coproc;
> + }
> +}
> +
> +
> +static struct vas_window *nx842_alloc_txwin(struct nx842_coproc *coproc)
> +{
> + struct vas_window *txwin = NULL;
> + struct vas_tx_win_attr txattr;
> +
> + /*
> +  * Kernel requests will be high priority. So open send
> +  * windows only for high priority RxFIFO entries.
> +  */
> + vas_init_tx_win_attr(, coproc->ct);
> + txattr.lpid = 0;/* lpid is 0 for kernel requests */
> + txattr.pid = mfspr(SPRN_PID);

Should we be using SPRN_PID here? That makes it appear as if it comes
from the current user process, which seems fishy.

> + /*
> +  * Open a VAS send window which is used to send request to NX.
> +  */
> + txwin = vas_tx_win_open(coproc->vas.id, coproc->ct, );
> + if (IS_ERR(txwin)) {
> + pr_err("ibm,nx-842: Can not open TX window: %ld\n",
> + PTR_ERR(txwin));
> + return NULL;
> + }
> +
> + return txwin;
> +}
> +
> +static int __init vas_cfg_coproc_info(struct device_node *dn, int chip_id,
> + int vasid)
> +{
> + struct vas_window *rxwin = NULL;
> + struct vas_rx_win_attr rxattr;
> + struct nx842_coproc *coproc;
> + u32 lpid, pid, tid, fifo_size;
> + u64 rx_fifo;
> + const char *priority;
> + int ret;
> +
> + ret = of_property_read_u64(dn, "rx-fifo-address", (void *)_fifo);
  
  Unnecessary cast.

> + if (ret) {
> + pr_err("Missing rx-fifo-address property\n");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(dn, "rx-fifo-size", _size);
> + if (ret) {
> + pr_err("Missing rx-fifo-size property\n");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(dn, "lpid", );
> + if (ret) {
> + pr_err("Missing lpid property\n");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(dn, "pid", );
> + if (ret) {
> + pr_err("Missing pid property\n");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(dn, "tid", );
> + if (ret) {
> + pr_err("Missing tid property\n");
> + return ret;
> + }
> +
> + ret = of_property_read_string(dn, "priority", );
> + if (ret) {
> + pr_err("Missing priority property\n");
> + return ret;
> + }
> +
> + coproc = kzalloc(sizeof(*coproc), GFP_KERNEL);
> + if (!coproc)
> + return -ENOMEM;
> +
> + if (!strcmp(priority, "High"))
> + coproc->ct = VAS_COP_TYPE_842_HIPRI;
> + else if (!strcmp(priority, "Normal"))
> + coproc->ct = VAS_COP_TYPE_842;
> + else {
> + pr_err("Invalid RxFIFO priority 

Re: [PATCH V3 2/6] crypto/nx: Create nx842_configure_crb function

[Michael Ellerman]

Herbert Xu  writes:

> On Fri, Jul 21, 2017 at 09:57:39PM -0700, Haren Myneni wrote:
>> 
>> Configure CRB is moved to nx842_configure_crb() so that it can
>> be used for icswx and VAS exec functions. VAS function will be
>> added later with P9 support.
>> 
>> Signed-off-by: Haren Myneni 
>
> Your patch does not apply against cryptodev.  Please rebase.

It depends on another series that will go via my tree, so it might be
better if this series goes via my tree too?

cheers

Re: [PATCH v4 1/5] powerpc: io.h: move iomap.h include so that it can use readq/writeq defs

[Michael Ellerman]

Logan Gunthorpe <log...@deltatee.com> writes:

> On 18/07/17 11:57 PM, Michael Ellerman wrote:
>> Seems fair enough, have you tested it at all?
>
> It's only been compile tested and the kbuild robot has beat up on it a bit.

OK. I don't think I see any way it can break anything, so feel free to
merge it, it'll get boot testing on powerpc once it's in linux-next.

Acked-by: Michael Ellerman <m...@ellerman.id.au>

cheers

Re: [PATCH v4 1/5] powerpc: io.h: move iomap.h include so that it can use readq/writeq defs

[Michael Ellerman]

Logan Gunthorpe <log...@deltatee.com> writes:

> Subsequent patches in this series makes use of the readq and writeq
> defines in iomap.h. However, as is, they get missed on the powerpc
> platform seeing the include comes before the define. This patch
> moves the include down to fix this.
>
> Signed-off-by: Logan Gunthorpe <log...@deltatee.com>
> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> Cc: Paul Mackerras <pau...@samba.org>
> Cc: Michael Ellerman <m...@ellerman.id.au>
> Cc: Nicholas Piggin <npig...@gmail.com>
> Cc: Suresh Warrier <warr...@linux.vnet.ibm.com>
> Cc: "Oliver O'Halloran" <ooh...@gmail.com>
> ---
>  arch/powerpc/include/asm/io.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Seems fair enough, have you tested it at all?

cheers

Re: [kernel-hardening] [PATCH] random: warn when kernel uses unseeded randomness

[Michael Ellerman]

"Jason A. Donenfeld"  writes:

> This enables an important dmesg notification about when drivers have
> used the crng without it being seeded first. Prior, these errors would
> occur silently, and so there hasn't been a great way of diagnosing these
> types of bugs for obscure setups. By adding this as a config option, we
> can leave it on by default, so that we learn where these issues happen,
> in the field, will still allowing some people to turn it off, if they
> really know what they're doing and do not want the log entries.
>
> However, we don't leave it _completely_ by default. An earlier version
> of this patch simply had `default y`. I'd really love that, but it turns
> out, this problem with unseeded randomness being used is really quite
> present and is going to take a long time to fix. Thus, as a compromise
> between log-messages-for-all and nobody-knows, this is `default y`,
> except it is also `depends on DEBUG_KERNEL`. This will ensure that the
> curious see the messages while others don't have to.

All the distro kernels I'm aware of have DEBUG_KERNEL=y.

Where all includes at least RHEL, SLES, Fedora, Ubuntu & Debian.

So it's still essentially default y.

Emitting *one* warning by default would be reasonable. That gives users
who are interested something to chase, they can then turn on the option
to get the full story.

Filling the dmesg buffer with repeated warnings is really not helpful.

cheers

Re: [kernel-hardening] Re: [PATCH] random: silence compiler warnings and fix race

[Michael Ellerman]

Theodore Ts'o  writes:

> On Mon, Jun 19, 2017 at 10:57:18PM +0200, Jason A. Donenfeld wrote:
>> 
>> With rc6 already released and rc7 coming up, I'd really appreciate you
>> stepping in here and either ACKing the above commit, or giving your
>> two cents about it in case I need to roll something different.
>
> I actually had set up an earlier version of your patch for on Saturday
> while I was in Beijing.  (Like Linus, I'm attending the LinuxCon China
> conference Monday and Tuesday.)  I had even created the signed tag,
> but I didn't send the pull request to Linus because I was waiting to
> see about how discussions over the locking strategy and the spammy log
> messages on PowerPC was going to get resolved.
>
> I've since respun the commit to reflect your newer patch (see the
> random_for_linus_stable tag on random.git) and rebased the dev branch
> on top of that.  Please take a look and comment.
>
> The other open issue I want to resolve before sending a pull request
> this week is whether we want to change the default for
> CONFIG_WARN_UNSEEDED_RANDOM so that the answer is 'n'.

Yes please.

> It *is* spammy for PowerPC, because they aren't getting their CRNG

*some* powerpc machines ...

> initialized quickly enough, so several userspace processes are getting
> fork/exec'ed with an uninitialized CRNG.  That being said, it is a
> valid warning because it means that the initial stack canary for the
> first couple of PowerPC processes are being created without a fully
> initialized CRNG, which may mean that an attacker might be able to
> circumvent the stack canary on the first couple of processes.  So that
> could potentially be a real security issue on Power.  OTOH, most Power
> users aren't going to be able to do anything about the fact the stack
> canaries of the system daemons started during early boot don't have
> strong randomness, so perhaps we should disable the warning by
> default.

powerpc supports a wide range of hardware platforms, some of which are
10-15 years old, and don't have a hardware RNG.

Is there anything we can do on those machines? Seems like our only
option would be to block the boot while some more "entropy" builds up,
but that's unlikely to be popular with users.

On our newer machines (>= Power8) we have a hardware RNG which we wire
up to arch_get_random_seed_long(), so on those machines the warnings
would be valid, because they'd indicate a bug.

So I think it should be up to arches to decide whether this is turned on
via their defconfigs, and the default should be 'n' because a lot of old
hardware won't be able to do anything useful with the warnings.

cheers

Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness

[Michael Ellerman]

Theodore Ts'o  writes:
> On Tue, Jun 06, 2017 at 07:48:04PM +0200, Jason A. Donenfeld wrote:
>> This enables an important dmesg notification about when drivers have
>> used the crng without it being seeded first. Prior, these errors would
>> occur silently, and so there hasn't been a great way of diagnosing these
>> types of bugs for obscure setups. By adding this as a config option, we
>> can leave it on by default, so that we learn where these issues happen,
>> in the field, will still allowing some people to turn it off, if they
>> really know what they're doing and do not want the log entries.
...
>
> This patch is pretty spammy.  On my KVM test kernel:
>
> random: bucket_table_alloc+0x15f/0x190 get_random_u32 called with crng_init = > 0
> random: bucket_table_alloc+0x15f/0x190 get_random_u32 called with crng_init = > 0
> random: bucket_table_alloc+0x15f/0x190 get_random_u32 called with crng_init = > 0
> random: bucket_table_alloc+0x15f/0x190 get_random_u32 called with crng_init = > 0
...
>
> At the very least we probably should do a logical "uniq" on the output
> (e.g., if we have complained about the previous callsite, don't whinge
> about it again).
>
> commit 9d9035bc6d7871a73d7f9aada4e63cb190874a68
> Author: Theodore Ts'o 
> Date:   Thu Jun 8 04:16:59 2017 -0400
>
> random: suppress duplicate crng_init=0 warnings
> 
> Suppress duplicate CONFIG_WARN_UNSEEDED_RANDOM warnings to avoid
> spamming dmesg.
> 
> Signed-off-by: Theodore Ts'o 

Even with this patch, it's still pretty spammy (today's linux-next):

 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0
 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0
 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0
 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0
 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0
 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0
 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0
 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0
 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0
 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0
 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0
 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0
 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0
 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0
 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0
 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0
 Initializing random number generator... random: arch_mmap_rnd+0x78/0xb0 
get_random_u64 called with crng_init=0
 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0
 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0
 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0
 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0
 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0
 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0
 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0
 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0


Do I need to be doing anything to fix these? (this is on powerpc)

cheers

Re: [PATCH v2 0/6] Appended signatures support for IMA appraisal

[Michael Ellerman]

Thiago Jung Bauermann <bauer...@linux.vnet.ibm.com> writes:

> Michael Ellerman <m...@ellerman.id.au> writes:
>
>> Thiago Jung Bauermann <bauer...@linux.vnet.ibm.com> writes:
>>
>>> On the OpenPOWER platform, secure boot and trusted boot are being
>>> implemented using IMA for taking measurements and verifying signatures.
>>
>> I still want you to implement arch_kexec_kernel_verify_sig() as well :)
>
> Yes, I will implement it! We are still working on loading the public
> keys for kernel signing from the firmware into a kernel keyring, so
> there's not much point in implementing arch_kexec_kernel_verify_sig
> without having that first.

OK. What's the ETA on those patches?

cheers

Re: [PATCH v2 0/6] Appended signatures support for IMA appraisal

[Michael Ellerman]

Thiago Jung Bauermann  writes:

> On the OpenPOWER platform, secure boot and trusted boot are being
> implemented using IMA for taking measurements and verifying signatures.

I still want you to implement arch_kexec_kernel_verify_sig() as well :)

cheers

[PATCH v2] powerpc/crypto/crct10dif-vpmsum: Fix missing preempt_disable()

[Michael Ellerman]

In crct10dif_vpmsum() we call enable_kernel_altivec() without first
disabling preemption, which is not allowed.

It used to be sufficient just to call pagefault_disable(), because that
also disabled preemption. But the two were decoupled in commit 8222dbe21e79
("sched/preempt, mm/fault: Decouple preemption from the page fault
logic") in mid 2015.

The crct10dif-vpmsum code inherited this bug from the crc32c-vpmsum code
on which it was modelled.

So add the missing preempt_disable/enable(). We should also call
disable_kernel_fp(), although it does nothing by default, there is a
debug switch to make it active and all enables should be paired with
disables.

Fixes: b01df1c16c9a ("crypto: powerpc - Add CRC-T10DIF acceleration")
Acked-by: Daniel Axtens <d...@axtens.net>
Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---
 arch/powerpc/crypto/crct10dif-vpmsum_glue.c | 3 +++
 1 file changed, 3 insertions(+)

v2: No change to the patch. Add dja's ack. Add linux-crypto to Cc.

diff --git a/arch/powerpc/crypto/crct10dif-vpmsum_glue.c 
b/arch/powerpc/crypto/crct10dif-vpmsum_glue.c
index bebfc329f746..02ea277863d1 100644
--- a/arch/powerpc/crypto/crct10dif-vpmsum_glue.c
+++ b/arch/powerpc/crypto/crct10dif-vpmsum_glue.c
@@ -44,10 +44,13 @@ static u16 crct10dif_vpmsum(u16 crci, unsigned char const 
*p, size_t len)
 
if (len & ~VMX_ALIGN_MASK) {
crc <<= 16;
+   preempt_disable();
pagefault_disable();
enable_kernel_altivec();
crc = __crct10dif_vpmsum(crc, p, len & ~VMX_ALIGN_MASK);
+   disable_kernel_altivec();
pagefault_enable();
+   preempt_enable();
crc >>= 16;
}
 
-- 
2.7.4

Re: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms

[Michael Ellerman]

Laurentiu Tudor <laurentiu.tu...@nxp.com> writes:

> On 04/05/2017 01:06 PM, Michael Ellerman wrote:
>> Laurentiu Tudor <laurentiu.tu...@nxp.com> writes:
>>
>>> Hi Michael,
>>>
>>> Just a couple of basic things to check:
>>>- was the dtb updated to the newest?
>>
>> Possibly not, it's an automated build/boot, I'll have to check what it
>> does with the dtb.
>>
>>>- is the qman node present? This should be easily visible in
>>> /proc/device-tree/soc@ffe00/qman@318000.
>>
>> No it's not there.
>>
>> That's running linux-next with:
>>
>> CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI=n
>>
>>
>> Does that mean I didn't update the device tree?
>
> I think so. Also, I just checked that the node is actually there by 
> compiling p5020ds.dts and then decompiling the dtb.

OK, I'll make sure I update the DTB.

It will still be good if the code was a bit more robust about the qman
being missing.

cheers

Re: [7/7] crypto: caam/qi - add ablkcipher and authenc algorithms

[Michael Ellerman]

Laurentiu Tudor  writes:

> Hi Michael,
>
> Just a couple of basic things to check:
>   - was the dtb updated to the newest?

Possibly not, it's an automated build/boot, I'll have to check what it
does with the dtb.

>   - is the qman node present? This should be easily visible in 
> /proc/device-tree/soc@ffe00/qman@318000.

No it's not there.

That's running linux-next with:

CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI=n


Does that mean I didn't update the device tree?

cheers

Re: [PATCH 3/5] crypto/nx: Create nx842_delete_coproc function

[Michael Ellerman]

Haren Myneni  writes:

> [PATCH 3/5] crypto/nx: Create nx842_delete_coproc function
>
> Move deleting coprocessor info upon exit or failure to
> nx842_delete_coproc().

Naming again, this deletes *all* the coprocs, so the name should be
plural.

cheers

Re: [PATCH 5/5] crypto/nx: Add P9 NX specific error codes for 842 engine

[Michael Ellerman]

Haren Myneni  writes:

> [PATCH 5/5] crypto/nx: Add P9 NX specific error codes for 842 engine
>
> This patch adds changes for checking P9 specific 842 engine
> error codes. These errros are reported in co-processor status
> block (CSB) for failures.

But you just enabled support on P9 in patch 4.

So you should reorder patch 4 and 5. ie. add the P9 error handling
before you enable P9 support.

cheers

Re: [PATCH 2/5] crypto/nx: Create nx842_cfg_crb function

[Michael Ellerman]

Haren Myneni  writes:

> [PATCH 2/5] crypto/nx: Create nx842_cfg_crb function
>
> Configure CRB is moved to nx842_cfg_crb() so that it can be
> used for icswx function and VAS function which will be added
> later.

Buy a vowel! :)

nx842_configure_crb() is fine.

cheers

Re: [PATCH 1/5] crypto/nx: Rename nx842_powernv_function as icswx function

[Michael Ellerman]

Haren Myneni  writes:

> [PATCH 1/5] crypto/nx: Rename nx842_powernv_function as icswx function
>
> nx842_powernv_function is points to nx842_icswx_function and
> will be point to VAS function which will be added later for
> P9 NX support.

I know it's nit-picking but can you give it a better name while you're
there.

I was thinking it should be called "send" or something, but it actually
synchronously sends and waits for a request.

So perhaps just nx842_exec(), for "execute a request", and then you can
have nx842_exec_icswx() and nx842_exec_vas().

cheers

Re: [PATCH 4/5] crypto/nx: Add P9 NX support for 842 compression engine.

[Michael Ellerman]

Hi Haren,

A few comments ...

Haren Myneni  writes:

> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index 4e5a470..7315621 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -19,6 +19,8 @@
>  #define VAS_RX_FIFO_SIZE_MIN (1 << 10)   /* 1KB */
>  #define VAS_RX_FIFO_SIZE_MAX (8 << 20)   /* 8MB */
>  
> +#define is_vas_available()   (cpu_has_feature(CPU_FTR_ARCH_300))

You shouldn't need that, it should all come from the device tree.

> diff --git a/drivers/crypto/nx/Kconfig b/drivers/crypto/nx/Kconfig
> index ad7552a..4ad7fdb 100644
> --- a/drivers/crypto/nx/Kconfig
> +++ b/drivers/crypto/nx/Kconfig
> @@ -38,6 +38,7 @@ config CRYPTO_DEV_NX_COMPRESS_PSERIES
>  config CRYPTO_DEV_NX_COMPRESS_POWERNV
>   tristate "Compression acceleration support on PowerNV platform"
>   depends on PPC_POWERNV
> + select VAS

Don't select symbols that are user visible. 

I'm not sure we actually want CONFIG_VAS to be user visible, but
currently it is so this should be 'depends on VAS'.

> diff --git a/drivers/crypto/nx/nx-842-powernv.c 
> b/drivers/crypto/nx/nx-842-powernv.c
> index 8737e90..66efd39 100644
> --- a/drivers/crypto/nx/nx-842-powernv.c
> +++ b/drivers/crypto/nx/nx-842-powernv.c
> @@ -554,6 +662,164 @@ static int nx842_powernv_decompress(const unsigned char 
> *in, unsigned int inlen,
> wmem, CCW_FC_842_DECOMP_CRC);
>  }
>  
> +
> +static int __init vas_cfg_coproc_info(struct device_node *dn, int chip_id,
> + int vasid, int ct)
> +{
> + struct vas_window *rxwin, *txwin = NULL;
> + struct vas_rx_win_attr rxattr;
> + struct vas_tx_win_attr txattr;
> + struct nx842_coproc *coproc;
> + u32 lpid, pid, tid;
> + u64 rx_fifo;
> + int ret;
> +#define RX_FIFO_SIZE 0x8000

Where's that come from?

> + if (of_property_read_u64(dn, "rx-fifo-address", (void *)_fifo)) {
> + pr_err("ibm,nx-842: Missing rx-fifo-address property\n");

The driver already declares pr_fmt(), so do you need the prefixes on
these pr_err()s ?

> + return -EINVAL;
> + }
> +
> + if (of_property_read_u32(dn, "lpid", )) {
> + pr_err("ibm,nx-842: Missing lpid property\n");
> + return -EINVAL;
> + }
> +
> + if (of_property_read_u32(dn, "pid", )) {
> + pr_err("ibm,nx-842: Missing pid property\n");
> + return -EINVAL;
> + }
> +
> + if (of_property_read_u32(dn, "tid", )) {
> + pr_err("ibm,nx-842: Missing tid property\n");
> + return -EINVAL;
> + }
> +
> + vas_init_rx_win_attr(, ct);
> + rxattr.rx_fifo = (void *)rx_fifo;
> + rxattr.rx_fifo_size = RX_FIFO_SIZE;
> + rxattr.lnotify_lpid = lpid;
> + rxattr.lnotify_pid = pid;
> + rxattr.lnotify_tid = tid;
> + rxattr.wcreds_max = 64;
> +
> + /*
> +  * Open a VAS receice window which is used to configure RxFIFO
> +  * for NX.
> +  */
> + rxwin = vas_rx_win_open(vasid, ct, );
> + if (IS_ERR(rxwin)) {
> + pr_err("ibm,nx-842: setting RxFIFO with VAS failed: %ld\n",
> + PTR_ERR(rxwin));
> + return PTR_ERR(rxwin);
> + }
> +
> + /*
> +  * Kernel requests will be high priority. So open send
> +  * windows only for high priority RxFIFO entries.
> +  */
> + if (ct == VAS_COP_TYPE_842_HIPRI) {

This if body looks like it should be a separate function to me.

> + vas_init_tx_win_attr(, ct);
> + txattr.lpid = 0;/* lpid is 0 for kernel requests */
> + txattr.pid = mfspr(SPRN_PID);
> +
> + /*
> +  * Open a VAS send window which is used to send request to NX.
> +  */
> + txwin = vas_tx_win_open(vasid, ct, );
> + if (IS_ERR(txwin)) {
> + pr_err("ibm,nx-842: Can not open TX window: %ld\n",
> + PTR_ERR(txwin));
> + ret = PTR_ERR(txwin);
> + goto err_out;
> + }
> + }
> +
> + coproc = kmalloc(sizeof(*coproc), GFP_KERNEL);
> + if (!coproc) {
> + ret = -ENOMEM;
> + goto err_out;
> + }

The error handling would be simpler if you did that earlier, before you
open the RX/TX windows.

> + coproc->chip_id = chip_id;
> + coproc->vas.rxwin = rxwin;
> + coproc->vas.txwin = txwin;
> +
> + INIT_LIST_HEAD(>list);
> + list_add(>list, _coprocs);

That duplicates logic in the non-vas probe, so ideally would be shared
or in a helper.

> +
> + return 0;
> +
> +err_out:
> + if (txwin)
> + vas_win_close(txwin);
> +
> + vas_win_close(rxwin);
> +
> + return ret;
> +}
> +
> +
> +static int __init nx842_powernv_probe_vas(struct device_node *dn)
> +{
> + struct device_node *nxdn, *np;

There's too many device nodes 

Re: [PATCH 1/4] crypto: powerpc - Factor out the core CRC vpmsum algorithm

[Michael Ellerman]

Daniel Axtens  writes:

> The core nuts and bolts of the crc32c vpmsum algorithm will
> also work for a number of other CRC algorithms with different
> polynomials. Factor out the function into a new asm file.
>
> To handle multiple users of the function, a user simply
> provides constants, defines the name of their CRC function,
> and then #includes the core algorithm file.
>
> Cc: Anton Blanchard 
> Signed-off-by: Daniel Axtens 
>
> --
>
> It's possible at this point to argue that the address
> of the constant tables should be passed in to the function,
> rather than doing this somewhat unconventional #include.
>
> However, we're about to add further #ifdef's back into the core
> that will be provided by the encapsulaing code, and which couldn't
> be done as a variable without performance loss.
> ---
>  arch/powerpc/crypto/crc32-vpmsum_core.S | 726 
> 
>  arch/powerpc/crypto/crc32c-vpmsum_asm.S | 714 +--
>  2 files changed, 729 insertions(+), 711 deletions(-)
>  create mode 100644 arch/powerpc/crypto/crc32-vpmsum_core.S

So although this sits in arch/powerpc, it's heavy on the crypto which is
not my area of expertise (to say the least!), so I think it should
probably go via Herbert and the crypto tree?

cheers

Re: [PATCH 2/4] crypto: powerpc - Re-enable non-REFLECTed CRCs

[Michael Ellerman]

Daniel Axtens  writes:

> When CRC32c was included in the kernel, Anton ripped out
> the #ifdefs around reflected polynomials, because CRC32c
> is always reflected. However, not all CRCs use reflection
> so we'd like to make it optional.
>
> Restore the REFLECT parts from Anton's original CRC32
> implementation (https://github.com/antonblanchard/crc32-vpmsum)
>
> That implementation is available under GPLv2+, so we're OK
> from a licensing point of view:
> https://github.com/antonblanchard/crc32-vpmsum/blob/master/LICENSE.TXT

It's also written by Anton and copyright IBM, so you (we (IBM)) could
always just relicense it anyway.

So doubly OK IMO.

cheers

Re: [PATCH] crypto: powerpc - Fix initialisation of crc32c context

[Michael Ellerman]

Daniel Axtens  writes:

> Turning on crypto self-tests on a POWER8 shows:
>
> alg: hash: Test 1 failed for crc32c-vpmsum
> : ff ff ff ff
>
> Comparing the code with the Intel CRC32c implementation on which
> ours is based shows that we are doing an init with 0, not ~0
> as CRC32c requires.
>
> This probably wasn't caught because btrfs does its own weird
> open-coded initialisation.
>
> Initialise our internal context to ~0 on init.
>
> This makes the self-tests pass, and btrfs continues to work.
>
> Fixes: 6dd7a82cc54e ("crypto: powerpc - Add POWER8 optimised crc32c")
> Cc: Anton Blanchard 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Daniel Axtens 
> ---
>  arch/powerpc/crypto/crc32c-vpmsum_glue.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This driver was originally merged via the crypto tree, so I'll assume
Herbert will pick up the fix. If he hasn't in a few days I'll take it.

cheers

Re: [PATCH] powerpc: crypto/vmx: various build fixes

[Michael Ellerman]

"Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> writes:

> First up, clean up the generated .S files properly on a 'make clean'.
> Secondly, force re-generation of these files when building for different
> endian-ness than what was built previously. Finally, generate the new
> files in the build tree, rather than the source tree.
>
> Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
> ---
> Michael,
> I've added your SOB here though you didn't explicitly include it in your
> previous mail. I hope that's fine from your end.

Yeah that's fine, thanks.

Crypto patches usually have a subject like:

crypto: vmx - Various build fixes


But maybe Herbert can fix it up for you when he applies this.

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] powerpc: crypto/vmx: clean up generated files

[Michael Ellerman]

"Naveen N. Rao"  writes:

> ..as stray .S files result in build errors, especially when using
> cross-compilers.

They should be cleaned on make clean, so adding them to clean-files
seems correct.

> More specifically, the generated .S files are endian-specific and will break
> subsequent builds targeting the other endian architecture.

But that indicates we're missing an if_changed somewhere, ie. switching
endian should cause them to be regenerated.

Also they should be generated into $(obj) not $(src) I think.

How about this?

diff --git a/drivers/crypto/vmx/Makefile b/drivers/crypto/vmx/Makefile
index de6e241..52f6ae9 100644
--- a/drivers/crypto/vmx/Makefile
+++ b/drivers/crypto/vmx/Makefile
@@ -10,10 +10,10 @@ endif
 quiet_cmd_perl = PERL $@
   cmd_perl = $(PERL) $(<) $(TARGET) > $(@)
 
-$(src)/aesp8-ppc.S: $(src)/aesp8-ppc.pl
-   $(call cmd,perl)
+$(obj)/aesp8-ppc.S: $(src)/aesp8-ppc.pl FORCE
+   $(call if_changed,perl)
   
-$(src)/ghashp8-ppc.S: $(src)/ghashp8-ppc.pl
-   $(call cmd,perl)
+$(obj)/ghashp8-ppc.S: $(src)/ghashp8-ppc.pl FORCE
+   $(call if_changed,perl)
 
 .PRECIOUS: $(obj)/aesp8-ppc.S $(obj)/ghashp8-ppc.S


cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: sha1-powerpc: little-endian support

[Michael Ellerman]

Marcelo Cerri  writes:

> [ Unknown signature status ]
> On Wed, Sep 28, 2016 at 09:20:15PM +0800, Herbert Xu wrote:
>> On Wed, Sep 28, 2016 at 10:15:51AM -0300, Marcelo Cerri wrote:
>> > Hi Herbert,
>> > 
>> > Any thoughts on this one?
>> 
>> Can this patch wait until the next merge window? On the broken
>> platforms it should just fail the self-test, right?
>
> Yes. It fails on any LE platform (including Ubuntu and RHEL 7.1).

How are you testing this? I thought I was running the crypto tests but
I've never seen this fail.

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hwrng: pasemi-rng - Use linux/io.h instead of asm/io.h

[Michael Ellerman]

Herbert Xu  writes:

> On Tue, Sep 06, 2016 at 01:58:39PM +0530, PrasannaKumar Muralidharan wrote:
>> Checkpatch.pl warns about usage of asm/io.h. Use linux/io.h instead.
>> 
>> Signed-off-by: PrasannaKumar Muralidharan 
>
> Patch applied.  Thanks.

Oops I merged it too, my bad.

Hopefully git will work out the resolution.

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: hwrng: pasemi-rng - Use linux/io.h instead of asm/io.h

[Michael Ellerman]

On Tue, 2016-06-09 at 08:28:39 UTC, PrasannaKumar Muralidharan wrote:
> Checkpatch.pl warns about usage of asm/io.h. Use linux/io.h instead.
> 
> Signed-off-by: PrasannaKumar Muralidharan 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/63019f3cab99c7acd27df5a5b8

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: crypto: crc32c-vpmsum - Convert to CPU feature based module autoloading

[Michael Ellerman]

On Thu, 2016-04-08 at 06:38:15 UTC, Anton Blanchard wrote:
> From: Anton Blanchard 
> 
> This patch utilises the GENERIC_CPU_AUTOPROBE infrastructure
> to automatically load the crc32c-vpmsum module if the CPU supports
> it.
> 
> Signed-off-by: Anton Blanchard 

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/d2cf5be07ff7c7cde8bef8551a

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] crypto: powerpc - CRYPT_CRC32C_VPMSUM should depend on ALTIVEC

[Michael Ellerman]

The optimised crc32c implementation depends on VMX (aka. Altivec)
instructions, so the kernel must be built with Altivec support in order
for the crc32c code to build.

Fixes: 6dd7a82cc54e ("crypto: powerpc - Add POWER8 optimised crc32c")
Acked-by: Anton Blanchard <an...@samba.org>
Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---
 crypto/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

v2: No change. Send to linux-crypto.

diff --git a/crypto/Kconfig b/crypto/Kconfig
index a9377bef25e3..84d71482bf08 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -439,7 +439,7 @@ config CRYPTO_CRC32C_INTEL
 
 config CRYPT_CRC32C_VPMSUM
tristate "CRC32c CRC algorithm (powerpc64)"
-   depends on PPC64
+   depends on PPC64 && ALTIVEC
select CRYPTO_HASH
select CRC32
help
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: crc32c-vpmsum - Convert to CPU feature based module autoloading

[Michael Ellerman]

Anton Blanchard  writes:

> Hi Michael,
>
>> Is VEC_CRYPTO the right feature?
>> 
>> That's new power8 crypto stuff.
>
> The vpmsum* instructions are part of the same pipeline as the vcipher*
> instructions, introduced in POWER8.

OK.

>> I thought this only used VMX? (but I haven't looked closely)
>
> Yes, vcipher* and vpmsum* are VMX instructions.

Right. The confusion is that we have PPC_FEATURE_HAS_ALTIVEC, but that
doesn't mean we have *these* VMX instructions.

This is actually an arch/powerpc patch, so I'll merge it unless Herbert
objects.

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: crc32c-vpmsum - Convert to CPU feature based module autoloading

[Michael Ellerman]

Anton Blanchard  writes:

> From: Anton Blanchard 
>
> This patch utilises the GENERIC_CPU_AUTOPROBE infrastructure
> to automatically load the crc32c-vpmsum module if the CPU supports
> it.
>
> Signed-off-by: Anton Blanchard 
> ---
>  arch/powerpc/crypto/crc32c-vpmsum_glue.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/crypto/crc32c-vpmsum_glue.c 
> b/arch/powerpc/crypto/crc32c-vpmsum_glue.c
> index bfe3d37..9fa046d 100644
> --- a/arch/powerpc/crypto/crc32c-vpmsum_glue.c
> +++ b/arch/powerpc/crypto/crc32c-vpmsum_glue.c
> @@ -4,6 +4,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #define CHKSUM_BLOCK_SIZE1
> @@ -157,7 +158,7 @@ static void __exit crc32c_vpmsum_mod_fini(void)
>   crypto_unregister_shash();
>  }
>  
> -module_init(crc32c_vpmsum_mod_init);
> +module_cpu_feature_match(PPC_MODULE_FEATURE_VEC_CRYPTO, 
> crc32c_vpmsum_mod_init);

Is VEC_CRYPTO the right feature?

That's new power8 crypto stuff.

I thought this only used VMX? (but I haven't looked closely)

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [V2,1/2] powerpc: Add module autoloading based on CPU features

[Michael Ellerman]

On Tue, 2016-19-07 at 04:03:52 UTC, Alastair D'Silva wrote:
> From: Alastair D'Silva 
> 
> This patch provides the necessary infrastructure to allow drivers
> to be automatically loaded via UDEV. It implements the minimum
> required to be able to use module_cpu_feature_match to trigger
> the GENERIC_CPU_AUTOPROBE mechanisms.
> 
> The features exposed are a mirror of the cpu_user_features
> (converted to an offset from a mask). This decision was made to
> ensure that the behavior between features for module loading and
> userspace are consistent.
> 
> Signed-off-by: Alastair D'Silva 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/4a1202765ddf4e5bb3143c0a85

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [V2, 2/2] crypto: vmx - Convert to CPU feature based module autoloading

[Michael Ellerman]

On Tue, 2016-19-07 at 04:03:53 UTC, Alastair D'Silva wrote:
> From: Alastair D'Silva 
> 
> This patch utilises the GENERIC_CPU_AUTOPROBE infrastructure
> to automatically load the vmx_crypto module if the CPU supports
> it.
> 
> Signed-off-by: Alastair D'Silva 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ccf5c442a1b82bf74105d72416

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/8] powerpc: add io{read,write}64 accessors

[Michael Ellerman]

On Tue, 2016-05-10 at 18:50 +, Scott Wood wrote:

> On 05/09/2016 03:20 AM, Horia Ioan Geanta Neag wrote:

> > On 5/5/2016 6:37 PM, Horia Geantă wrote:

> > > This will allow device drivers to consistently use io{read,write}XX
> > > also for 64-bit accesses.
> > > 
> > > Signed-off-by: Horia Geantă <horia.gea...@nxp.com>
> > 
> > It would be great if PPC maintainers could Ack this patch.
> > 
> > As stated in the cover letter: https://lkml.org/lkml/2016/5/5/340
> > I'd like to go with the whole patch set via cryptodev-2.6 tree.
> 
> It looks good to me.  Michael?

I didn't get the cover letter, or any of the rest of the series, so although I
saw the patch I had no context. And I didn't have time to chase it up.

At a glance it seems fine, so:

Acked-by: Michael Ellerman <m...@ellerman.id.au>

cheers

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH stable 1/2] powerpc: Uncomment and make enable_kernel_vsx() routine available

[Michael Ellerman]

From: Leonidas Da Silva Barbosa <leosi...@linux.vnet.ibm.com>

commit 72cd7b44bc99376b3f3c93cedcd052663fcdf705 upstream.

enable_kernel_vsx() function was commented since anything was using
it. However, vmx-crypto driver uses VSX instructions which are
only available if VSX is enable. Otherwise it rises an exception oops.

This patch uncomment enable_kernel_vsx() routine and makes it available.

Signed-off-by: Leonidas S. Barbosa <leosi...@linux.vnet.ibm.com>
Signed-off-by: Herbert Xu <herb...@gondor.apana.org.au>
Cc: sta...@vger.kernel.org # 4.2
Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---
 arch/powerpc/include/asm/switch_to.h | 1 +
 arch/powerpc/kernel/process.c| 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/switch_to.h 
b/arch/powerpc/include/asm/switch_to.h
index 58abeda64cb7..15cca17cba4b 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -29,6 +29,7 @@ static inline void save_early_sprs(struct thread_struct 
*prev) {}
 
 extern void enable_kernel_fp(void);
 extern void enable_kernel_altivec(void);
+extern void enable_kernel_vsx(void);
 extern int emulate_altivec(struct pt_regs *);
 extern void __giveup_vsx(struct task_struct *);
 extern void giveup_vsx(struct task_struct *);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 8005e18d1b40..64e6e9d9e656 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -204,8 +204,6 @@ EXPORT_SYMBOL_GPL(flush_altivec_to_thread);
 #endif /* CONFIG_ALTIVEC */
 
 #ifdef CONFIG_VSX
-#if 0
-/* not currently used, but some crazy RAID module might want to later */
 void enable_kernel_vsx(void)
 {
WARN_ON(preemptible());
@@ -220,7 +218,6 @@ void enable_kernel_vsx(void)
 #endif /* CONFIG_SMP */
 }
 EXPORT_SYMBOL(enable_kernel_vsx);
-#endif
 
 void giveup_vsx(struct task_struct *tsk)
 {
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH stable 2/2] crypto: vmx - Adding enable_kernel_vsx() to access VSX instructions

[Michael Ellerman]

From: Leonidas Da Silva Barbosa <leosi...@linux.vnet.ibm.com>

commit 2d6f0600b2cd755959527230ef5a6fba97bb762a upstream.

vmx-crypto driver make use of some VSX instructions which are
only available if VSX is enabled. Running in cases where VSX
are not enabled vmx-crypto fails in a VSX exception.

In order to fix this enable_kernel_vsx() was added to turn on
VSX instructions for vmx-crypto.

Signed-off-by: Leonidas S. Barbosa <leosi...@linux.vnet.ibm.com>
Signed-off-by: Herbert Xu <herb...@gondor.apana.org.au>
Fixes: 8676590a1593 ("crypto: vmx - Adding AES routines for VMX module")
Cc: sta...@vger.kernel.org # 4.2
Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---
 drivers/crypto/vmx/aes.c | 3 +++
 drivers/crypto/vmx/aes_cbc.c | 3 +++
 drivers/crypto/vmx/aes_ctr.c | 3 +++
 drivers/crypto/vmx/ghash.c   | 4 
 4 files changed, 13 insertions(+)

This fixes a kernel crash at boot.

diff --git a/drivers/crypto/vmx/aes.c b/drivers/crypto/vmx/aes.c
index e79e567e43aa..263af709e536 100644
--- a/drivers/crypto/vmx/aes.c
+++ b/drivers/crypto/vmx/aes.c
@@ -84,6 +84,7 @@ static int p8_aes_setkey(struct crypto_tfm *tfm, const u8 
*key,
preempt_disable();
pagefault_disable();
enable_kernel_altivec();
+   enable_kernel_vsx();
ret = aes_p8_set_encrypt_key(key, keylen * 8, >enc_key);
ret += aes_p8_set_decrypt_key(key, keylen * 8, >dec_key);
pagefault_enable();
@@ -103,6 +104,7 @@ static void p8_aes_encrypt(struct crypto_tfm *tfm, u8 *dst, 
const u8 *src)
preempt_disable();
pagefault_disable();
enable_kernel_altivec();
+   enable_kernel_vsx();
aes_p8_encrypt(src, dst, >enc_key);
pagefault_enable();
preempt_enable();
@@ -119,6 +121,7 @@ static void p8_aes_decrypt(struct crypto_tfm *tfm, u8 *dst, 
const u8 *src)
preempt_disable();
pagefault_disable();
enable_kernel_altivec();
+   enable_kernel_vsx();
aes_p8_decrypt(src, dst, >dec_key);
pagefault_enable();
preempt_enable();
diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
index 725c78ec..0b8fe2ec5315 100644
--- a/drivers/crypto/vmx/aes_cbc.c
+++ b/drivers/crypto/vmx/aes_cbc.c
@@ -85,6 +85,7 @@ static int p8_aes_cbc_setkey(struct crypto_tfm *tfm, const u8 
*key,
preempt_disable();
pagefault_disable();
enable_kernel_altivec();
+   enable_kernel_vsx();
ret = aes_p8_set_encrypt_key(key, keylen * 8, >enc_key);
ret += aes_p8_set_decrypt_key(key, keylen * 8, >dec_key);
pagefault_enable();
@@ -115,6 +116,7 @@ static int p8_aes_cbc_encrypt(struct blkcipher_desc *desc,
preempt_disable();
pagefault_disable();
enable_kernel_altivec();
+   enable_kernel_vsx();
 
blkcipher_walk_init(, dst, src, nbytes);
ret = blkcipher_walk_virt(desc, );
@@ -155,6 +157,7 @@ static int p8_aes_cbc_decrypt(struct blkcipher_desc *desc,
preempt_disable();
pagefault_disable();
enable_kernel_altivec();
+   enable_kernel_vsx();
 
blkcipher_walk_init(, dst, src, nbytes);
ret = blkcipher_walk_virt(desc, );
diff --git a/drivers/crypto/vmx/aes_ctr.c b/drivers/crypto/vmx/aes_ctr.c
index 7adae42a7b79..1e754ae4e850 100644
--- a/drivers/crypto/vmx/aes_ctr.c
+++ b/drivers/crypto/vmx/aes_ctr.c
@@ -82,6 +82,7 @@ static int p8_aes_ctr_setkey(struct crypto_tfm *tfm, const u8 
*key,
 
pagefault_disable();
enable_kernel_altivec();
+   enable_kernel_vsx();
ret = aes_p8_set_encrypt_key(key, keylen * 8, >enc_key);
pagefault_enable();
 
@@ -100,6 +101,7 @@ static void p8_aes_ctr_final(struct p8_aes_ctr_ctx *ctx,
 
pagefault_disable();
enable_kernel_altivec();
+   enable_kernel_vsx();
aes_p8_encrypt(ctrblk, keystream, >enc_key);
pagefault_enable();
 
@@ -131,6 +133,7 @@ static int p8_aes_ctr_crypt(struct blkcipher_desc *desc,
while ((nbytes = walk.nbytes) >= AES_BLOCK_SIZE) {
pagefault_disable();
enable_kernel_altivec();
+   enable_kernel_vsx();
aes_p8_ctr32_encrypt_blocks(walk.src.virt.addr,
walk.dst.virt.addr,
(nbytes &
diff --git a/drivers/crypto/vmx/ghash.c b/drivers/crypto/vmx/ghash.c
index b5e29002b666..2183a2e77641 100644
--- a/drivers/crypto/vmx/ghash.c
+++ b/drivers/crypto/vmx/ghash.c
@@ -119,6 +119,7 @@ static int p8_ghash_setkey(struct crypto_shash *tfm, const 
u8 *key,
preempt_disable();
pagefault_disa

[PATCH stable 1/2] powerpc: Uncomment and make enable_kernel_vsx() routine available

[Michael Ellerman]

From: Leonidas Da Silva Barbosa <leosi...@linux.vnet.ibm.com>

commit 72cd7b44bc99376b3f3c93cedcd052663fcdf705 upstream.

enable_kernel_vsx() function was commented since anything was using
it. However, vmx-crypto driver uses VSX instructions which are
only available if VSX is enable. Otherwise it rises an exception oops.

This patch uncomment enable_kernel_vsx() routine and makes it available.

Signed-off-by: Leonidas S. Barbosa <leosi...@linux.vnet.ibm.com>
Signed-off-by: Herbert Xu <herb...@gondor.apana.org.au>
Cc: sta...@vger.kernel.org # 4.1
Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---
 arch/powerpc/include/asm/switch_to.h | 1 +
 arch/powerpc/kernel/process.c| 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/switch_to.h 
b/arch/powerpc/include/asm/switch_to.h
index 58abeda64cb7..15cca17cba4b 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -29,6 +29,7 @@ static inline void save_early_sprs(struct thread_struct 
*prev) {}
 
 extern void enable_kernel_fp(void);
 extern void enable_kernel_altivec(void);
+extern void enable_kernel_vsx(void);
 extern int emulate_altivec(struct pt_regs *);
 extern void __giveup_vsx(struct task_struct *);
 extern void giveup_vsx(struct task_struct *);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index febb50dd5328..0596373cd1c3 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -204,8 +204,6 @@ EXPORT_SYMBOL_GPL(flush_altivec_to_thread);
 #endif /* CONFIG_ALTIVEC */
 
 #ifdef CONFIG_VSX
-#if 0
-/* not currently used, but some crazy RAID module might want to later */
 void enable_kernel_vsx(void)
 {
WARN_ON(preemptible());
@@ -220,7 +218,6 @@ void enable_kernel_vsx(void)
 #endif /* CONFIG_SMP */
 }
 EXPORT_SYMBOL(enable_kernel_vsx);
-#endif
 
 void giveup_vsx(struct task_struct *tsk)
 {
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH stable 2/2] crypto: vmx - Adding enable_kernel_vsx() to access VSX instructions

[Michael Ellerman]

commit 2d6f0600b2cd755959527230ef5a6fba97bb762a upstream.

vmx-crypto driver make use of some VSX instructions which are
only available if VSX is enabled. Running in cases where VSX
are not enabled vmx-crypto fails in a VSX exception.

In order to fix this enable_kernel_vsx() was added to turn on
VSX instructions for vmx-crypto.

Signed-off-by: Leonidas S. Barbosa <leosi...@linux.vnet.ibm.com>
Signed-off-by: Herbert Xu <herb...@gondor.apana.org.au>
Fixes: 8676590a1593 ("crypto: vmx - Adding AES routines for VMX module")
Cc: sta...@vger.kernel.org # 4.1
Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---
 drivers/crypto/vmx/aes.c | 3 +++
 drivers/crypto/vmx/aes_cbc.c | 3 +++
 drivers/crypto/vmx/aes_ctr.c | 3 +++
 drivers/crypto/vmx/ghash.c   | 4 
 4 files changed, 13 insertions(+)

diff --git a/drivers/crypto/vmx/aes.c b/drivers/crypto/vmx/aes.c
index ab300ea19434..41f93334cc44 100644
--- a/drivers/crypto/vmx/aes.c
+++ b/drivers/crypto/vmx/aes.c
@@ -80,6 +80,7 @@ static int p8_aes_setkey(struct crypto_tfm *tfm, const u8 
*key,
 
 pagefault_disable();
 enable_kernel_altivec();
+enable_kernel_vsx();
 ret = aes_p8_set_encrypt_key(key, keylen * 8, >enc_key);
 ret += aes_p8_set_decrypt_key(key, keylen * 8, >dec_key);
 pagefault_enable();
@@ -97,6 +98,7 @@ static void p8_aes_encrypt(struct crypto_tfm *tfm, u8 *dst, 
const u8 *src)
 } else {
 pagefault_disable();
 enable_kernel_altivec();
+enable_kernel_vsx();
 aes_p8_encrypt(src, dst, >enc_key);
 pagefault_enable();
 }
@@ -111,6 +113,7 @@ static void p8_aes_decrypt(struct crypto_tfm *tfm, u8 *dst, 
const u8 *src)
 } else {
 pagefault_disable();
 enable_kernel_altivec();
+enable_kernel_vsx();
 aes_p8_decrypt(src, dst, >dec_key);
 pagefault_enable();
 }
diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
index 1a559b7dddb5..c8e7f653e5d3 100644
--- a/drivers/crypto/vmx/aes_cbc.c
+++ b/drivers/crypto/vmx/aes_cbc.c
@@ -81,6 +81,7 @@ static int p8_aes_cbc_setkey(struct crypto_tfm *tfm, const u8 
*key,
 
 pagefault_disable();
 enable_kernel_altivec();
+enable_kernel_vsx();
 ret = aes_p8_set_encrypt_key(key, keylen * 8, >enc_key);
 ret += aes_p8_set_decrypt_key(key, keylen * 8, >dec_key);
 pagefault_enable();
@@ -108,6 +109,7 @@ static int p8_aes_cbc_encrypt(struct blkcipher_desc *desc,
 } else {
 pagefault_disable();
 enable_kernel_altivec();
+enable_kernel_vsx();
 
blkcipher_walk_init(, dst, src, nbytes);
 ret = blkcipher_walk_virt(desc, );
@@ -143,6 +145,7 @@ static int p8_aes_cbc_decrypt(struct blkcipher_desc *desc,
 } else {
 pagefault_disable();
 enable_kernel_altivec();
+enable_kernel_vsx();
 
blkcipher_walk_init(, dst, src, nbytes);
 ret = blkcipher_walk_virt(desc, );
diff --git a/drivers/crypto/vmx/aes_ctr.c b/drivers/crypto/vmx/aes_ctr.c
index 96dbee4bf4a6..266e708d63df 100644
--- a/drivers/crypto/vmx/aes_ctr.c
+++ b/drivers/crypto/vmx/aes_ctr.c
@@ -79,6 +79,7 @@ static int p8_aes_ctr_setkey(struct crypto_tfm *tfm, const u8 
*key,
 
 pagefault_disable();
 enable_kernel_altivec();
+enable_kernel_vsx();
 ret = aes_p8_set_encrypt_key(key, keylen * 8, >enc_key);
 pagefault_enable();
 
@@ -97,6 +98,7 @@ static void p8_aes_ctr_final(struct p8_aes_ctr_ctx *ctx,
 
 pagefault_disable();
 enable_kernel_altivec();
+enable_kernel_vsx();
 aes_p8_encrypt(ctrblk, keystream, >enc_key);
 pagefault_enable();
 
@@ -127,6 +129,7 @@ static int p8_aes_ctr_crypt(struct blkcipher_desc *desc,
 while ((nbytes = walk.nbytes) >= AES_BLOCK_SIZE) {
 pagefault_disable();
 enable_kernel_altivec();
+enable_kernel_vsx();
 aes_p8_ctr32_encrypt_blocks(walk.src.virt.addr, walk.dst.virt.addr,
 (nbytes & AES_BLOCK_MASK)/AES_BLOCK_SIZE, >enc_key, 
walk.iv);
 pagefault_enable();
diff --git a/drivers/crypto/vmx/ghash.c b/drivers/crypto/vmx/ghash.c
index d0ffe277af5c..917b3f09e724 100644
--- a/drivers/crypto/vmx/ghash.c
+++ b/drivers/crypto/vmx/ghash.c
@@ -116,6 +116,7 @@ static int p8_ghash_setkey(struct crypto_shash *tfm, const 
u8 *key,
 
 pagefault_disable();
 enable_kernel_altivec();
+enable_kernel_vsx();
 enable_kernel_fp();
 gcm_init_p8(ctx->htable, (const u64 *) key);
 pagefault_enable();
@@ -142,6 +143,7 @@ static int p8_ghash_update(struct shash_desc *desc,
 GHASH_DIGEST_SIZE - dctx->bytes);
 pagefault_disable();
 enable_kernel_altivec();
+enable_kernel_vsx();
 enable_kernel_fp();
 gcm_ghash_p8(dctx->shash, ctx->htable, dctx->buffer,
 GHASH_DIGEST_SIZE);
@@ -154,6 +156,7 @@ static int p8_ghash_updat

Re: [PATCH v2] crypto: vmx - VMX crypto should depend on CONFIG_VSX

[Michael Ellerman]

On Thu, 2015-09-10 at 17:26 +0800, Herbert Xu wrote:
> On Wed, Sep 09, 2015 at 06:22:35PM +1000, Michael Ellerman wrote:
> > This code uses FP (floating point), Altivec and VSX (Vector-Scalar
> > Extension). It can just depend on CONFIG_VSX though, because that
> > already depends on FP and Altivec.
> > 
> > Otherwise we get lots of link errors such as:
> > 
> >   drivers/built-in.o: In function `.p8_aes_setkey':
> >   aes.c:(.text+0x2d325c): undefined reference to `.enable_kernel_altivec'
> >   aes.c:(.text+0x2d326c): undefined reference to `.enable_kernel_vsx'
> > 
> > Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
> 
> Applied.

Thanks.

Is that targeted for 4.3 ?

cheers




--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] crypto: vmx - VMX crypto should depend on CONFIG_VSX

[Michael Ellerman]

This code uses FP (floating point), Altivec and VSX (Vector-Scalar
Extension). It can just depend on CONFIG_VSX though, because that
already depends on FP and Altivec.

Otherwise we get lots of link errors such as:

  drivers/built-in.o: In function `.p8_aes_setkey':
  aes.c:(.text+0x2d325c): undefined reference to `.enable_kernel_altivec'
  aes.c:(.text+0x2d326c): undefined reference to `.enable_kernel_vsx'

Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
---

v2: Spell out VSX, and CC linux-crypto.

 drivers/crypto/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 07bc7aa6b224..d234719065a5 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -461,7 +461,7 @@ config CRYPTO_DEV_QCE
 
 config CRYPTO_DEV_VMX
bool "Support for VMX cryptographic acceleration instructions"
-   depends on PPC64
+   depends on PPC64 && VSX
help
  Support for VMX cryptographic acceleration instructions.
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] crypto/nx-842-{powerpc,pseries}: reduce chattiness of platform drivers

[Michael Ellerman]

On Mon, 2015-07-06 at 10:06 -0700, Nishanth Aravamudan wrote:
 On 03.07.2015 [11:30:32 +1000], Michael Ellerman wrote:
  On Thu, 2015-07-02 at 15:40 -0700, Nishanth Aravamudan wrote:
   While we never would successfully load on the wrong machine type, there
   is extra output by default regardless of machine type.
   
 
 Thanks Michael!

Thanks for cleaning it up.


 While we never would successfully load on the wrong machine type, there
 is extra output by default regardless of machine type.
 
 For instance, on a PowerVM LPAR, we see the following:
 
 nx_compress_powernv: loading
 nx_compress_powernv: no coprocessors found
 
 even though those coprocessors could never be found.
 
 Signed-off-by: Nishanth Aravamudan n...@linux.vnet.ibm.com
 Cc: Dan Streetman ddstr...@us.ibm.com
 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Cc: linux-crypto@vger.kernel.org
 Cc: linuxppc-...@lists.ozlabs.org
 
 ---
 v2:
   Rather than not loading, just reduce the verbosity

Acked-by: Michael Ellerman m...@ellerman.id.au

I'm assuming Herbert will take this.

cheers





--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] crypto/nx-842-{powerpc,pseries}: only load on the appropriate machine type

[Michael Ellerman]

On Thu, 2015-07-02 at 15:40 -0700, Nishanth Aravamudan wrote:
 While we never would successfully load on the wrong machine type, there
 is extra output by default regardless of machine type.
 
 For instance, on a PowerVM LPAR, we see the following:
 
 nx_compress_powernv: loading
 nx_compress_powernv: no coprocessors found
 
 even though those coprocessors could never be found. Similar pseries
 messages are printed on powernv.

I know I've been converting init calls to machine_initcalls() to avoid these
sort of issues in platform code. But for a driver it should be trivial for it
to only probe when the hardware is found.

By which I mean I think we shouldn't need these.

 diff --git a/drivers/crypto/nx/nx-842-powernv.c 
 b/drivers/crypto/nx/nx-842-powernv.c
 index 33b3b0abf4ae..6b5e5143c95b 100644
 --- a/drivers/crypto/nx/nx-842-powernv.c
 +++ b/drivers/crypto/nx/nx-842-powernv.c
 @@ -594,6 +594,9 @@ static __init int nx842_powernv_init(void)
   BUILD_BUG_ON(DDE_BUFFER_ALIGN % DDE_BUFFER_SIZE_MULT);
   BUILD_BUG_ON(DDE_BUFFER_SIZE_MULT % DDE_BUFFER_LAST_MULT);
  
 + if (!machine_is(powernv))
 + return -ENODEV;
 +
   pr_info(loading\n);

This is just too chatty, drop it.

   for_each_compatible_node(dn, NULL, ibm,power-nx)

It shouldn't be printing anything unless it finds some devices in this loop.

And you should drop the print in here:

if (!nx842_ct) {
pr_err(no coprocessors found\n);
return -ENODEV;
}

And that should mean no output unless hardware is found I think?

 @@ -625,6 +628,9 @@ static void __exit nx842_powernv_exit(void)
  {
   struct nx842_coproc *coproc, *n;
  
 + if (!machine_is(powernv))
 + return;

You shouldn't need to touch the exit paths if the drivers were never loaded?

 diff --git a/drivers/crypto/nx/nx-842-pseries.c 
 b/drivers/crypto/nx/nx-842-pseries.c
 index b84b0ceeb46e..75a7bfdc160e 100644
 --- a/drivers/crypto/nx/nx-842-pseries.c
 +++ b/drivers/crypto/nx/nx-842-pseries.c
 @@ -1091,6 +1091,9 @@ static int __init nx842_pseries_init(void)
   struct nx842_devdata *new_devdata;
   int ret;
  
 + if (!machine_is(pseries))
 + return -ENODEV;
 +
   pr_info(Registering IBM Power 842 compression driver\n);

Again this is too chatty, just remove it.

   if (!of_find_compatible_node(NULL, NULL, ibm,compression))
return -ENODEV;

That should do the trick shouldn't it?

cheers


--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/8 v2] crypto: replace memset by memzero_explicit

[Michael Ellerman]

On Sun, 2014-11-30 at 18:03 +0100, Julia Lawall wrote:
 From: Julia Lawall julia.law...@lip6.fr
 
 Memset on a local variable may be removed when it is called just before the
 variable goes out of scope.  Using memzero_explicit defeats this
 optimization.  A simplified version of the semantic patch that makes this
 
 Signed-off-by: Julia Lawall julia.law...@lip6.fr
 
 ---
 Daniel Borkmann suggested that these patches could go through Herbert Xu's
 cryptodev tree.

That's fine by me:

Acked-by: Michael Ellerman m...@ellerman.id.au

cheers


--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] drivers/crypto/nx: saves chaining value from co-processor

[Michael Ellerman]

Hi Fin,

I don't know anything about crypto so I can only critique you on your
patch submission technique :)  ...

On Wed, Aug 07, 2013 at 06:15:50PM -0500, Fionnuala Gunter wrote:
 This patch fixes a bug that is triggered when cts(cbc(aes)) is used with
 nx-crypto driver on input larger than 32 bytes.
 
 The chaining value from co-processor was not being saved. This value is
 needed because it is used as the IV by cts(cbc(aes)).
 
 Signed-off-by: Fionnuala Gunter f...@linux.vnet.ibm.com
 Reviewed-by: Marcelo Cerri mhce...@linux.vnet.ibm.com
 ---
 v2. changed signed-off-by to reviewed-by and added more details to
 description
 
 This bug appeared in the original submission (v3.5)

Ideally this should identify the commit, so:

  This bug was introduced in the original submission (v3.5), commit
  856d673 powerpc/crypto: AES-CBC mode routines for nx encryption.

Including the subject of the commit is handy in case the patch has been
backported somewhere, in which case the commit sha will be different.

It should definitely be part of the commit message, not below the ---.

And Ben might disagree but I think with a clear cut bug fix like this it
should include the CC to stable, so:

Cc: sta...@vger.kernel.org # 3.5+

cheers
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] powerpc/crypto: Remove virt_to_abs() usage in nx-842.c

[Michael Ellerman]

virt_to_abs() is just a wrapper around __pa(), use __pa() directly.

We should be including asm/page.h to get __pa(). abs_addr.h will be
removed shortly so drop that.

We were getting of.h via abs_addr.h so we need to include that directly.

Having done all that, clean up the ordering of the includes.

Signed-off-by: Michael Ellerman mich...@ellerman.id.au
---
 drivers/crypto/nx/nx-842.c |   34 ++
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/crypto/nx/nx-842.c b/drivers/crypto/nx/nx-842.c
index 9da0fb2..0ce6257 100644
--- a/drivers/crypto/nx/nx-842.c
+++ b/drivers/crypto/nx/nx-842.c
@@ -21,13 +21,15 @@
  *  Seth Jennings sjenn...@linux.vnet.ibm.com
  */
 
+#include linux/kernel.h
 #include linux/module.h
-#include asm/vio.h
-#include asm/pSeries_reconfig.h
-#include linux/slab.h
-#include asm/abs_addr.h
 #include linux/nx842.h
-#include linux/kernel.h
+#include linux/of.h
+#include linux/slab.h
+
+#include asm/page.h
+#include asm/pSeries_reconfig.h
+#include asm/vio.h
 
 #include nx_csbcpb.h /* struct nx_csbcpb */
 
@@ -140,7 +142,7 @@ static unsigned long nx842_get_desired_dma(struct vio_dev 
*viodev)
 }
 
 struct nx842_slentry {
-   unsigned long ptr; /* Absolute address (use virt_to_abs()) */
+   unsigned long ptr; /* Real address (use __pa()) */
unsigned long len;
 };
 
@@ -167,7 +169,7 @@ static int nx842_build_scatterlist(unsigned long buf, int 
len,
 
entry = sl-entries;
while (len) {
-   entry-ptr = virt_to_abs(buf);
+   entry-ptr = __pa(buf);
nextpage = ALIGN(buf + 1, NX842_HW_PAGE_SIZE);
if (nextpage  buf + len) {
/* we aren't at the end yet */
@@ -369,8 +371,8 @@ int nx842_compress(const unsigned char *in, unsigned int 
inlen,
op.flags = NX842_OP_COMPRESS;
csbcpb = workmem-csbcpb;
memset(csbcpb, 0, sizeof(*csbcpb));
-   op.csbcpb = virt_to_abs(csbcpb);
-   op.out = virt_to_abs(slout.entries);
+   op.csbcpb = __pa(csbcpb);
+   op.out = __pa(slout.entries);
 
for (i = 0; i  hdr-blocks_nr; i++) {
/*
@@ -400,13 +402,13 @@ int nx842_compress(const unsigned char *in, unsigned int 
inlen,
 */
if (likely(max_sync_size == NX842_HW_PAGE_SIZE)) {
/* Create direct DDE */
-   op.in = virt_to_abs(inbuf);
+   op.in = __pa(inbuf);
op.inlen = max_sync_size;
 
} else {
/* Create indirect DDE (scatterlist) */
nx842_build_scatterlist(inbuf, max_sync_size, slin);
-   op.in = virt_to_abs(slin.entries);
+   op.in = __pa(slin.entries);
op.inlen = -nx842_get_scatterlist_size(slin);
}
 
@@ -564,7 +566,7 @@ int nx842_decompress(const unsigned char *in, unsigned int 
inlen,
op.flags = NX842_OP_DECOMPRESS;
csbcpb = workmem-csbcpb;
memset(csbcpb, 0, sizeof(*csbcpb));
-   op.csbcpb = virt_to_abs(csbcpb);
+   op.csbcpb = __pa(csbcpb);
 
/*
 * max_sync_size may have changed since compression,
@@ -596,12 +598,12 @@ int nx842_decompress(const unsigned char *in, unsigned 
int inlen,
if (likely((inbuf  NX842_HW_PAGE_MASK) ==
((inbuf + hdr-sizes[i] - 1)  NX842_HW_PAGE_MASK))) {
/* Create direct DDE */
-   op.in = virt_to_abs(inbuf);
+   op.in = __pa(inbuf);
op.inlen = hdr-sizes[i];
} else {
/* Create indirect DDE (scatterlist) */
nx842_build_scatterlist(inbuf, hdr-sizes[i] , slin);
-   op.in = virt_to_abs(slin.entries);
+   op.in = __pa(slin.entries);
op.inlen = -nx842_get_scatterlist_size(slin);
}
 
@@ -612,12 +614,12 @@ int nx842_decompress(const unsigned char *in, unsigned 
int inlen,
 */
if (likely(max_sync_size == NX842_HW_PAGE_SIZE)) {
/* Create direct DDE */
-   op.out = virt_to_abs(outbuf);
+   op.out = __pa(outbuf);
op.outlen = max_sync_size;
} else {
/* Create indirect DDE (scatterlist) */
nx842_build_scatterlist(outbuf, max_sync_size, slout);
-   op.out = virt_to_abs(slout.entries);
+   op.out = __pa(slout.entries);
op.outlen = -nx842_get_scatterlist_size(slout);
}
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http

Re: [PATCH] powerpc/crypto: Remove virt_to_abs() usage in nx-842.c

[Michael Ellerman]

Hi Herbert,

We're planning on removing virt_to_abs() from the powerpc tree this
cycle. So if you can take this patch in your tree everything should
continue building when the two trees merge.

cheers

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] powerpc/crypto: add 842 hardware compression driver

[Michael Ellerman]

On Fri, 2012-07-20 at 09:01 -0500, Seth Jennings wrote:
 On 07/20/2012 12:33 AM, Michael Ellerman wrote:
  On Thu, 2012-07-19 at 09:42 -0500, Seth Jennings wrote:
  This patch adds the driver for interacting with the 842
  compression accelerator on IBM Power7+ systems.
  
  ...
  
  +struct nx842_slentry {
  +  unsigned long ptr; /* Absolute address (use virt_to_abs()) */
  /+ unsigned long len;
  +};
  
  These days virt_to_abs() is just __pa() - ie. convert to a real address.
 
 Thanks, I'll make that change.
 
 Is it a blocker to the code being pulled in though? I'm
 hoping to get this in ASAP for the 3.6 merge window.  As
 this isn't a functional defect (I assume __pa() and
 virt_to_abs() still achieve the same result), can I get an
 OK from you that this isn't a blocker to the code being
 accepted?

Sorry I missed your reply. No it's not a blocker, just ugly.

I have sent a series to Ben which removes virt_to_abs() entirely, so
we'll want to make sure we fixup the nx driver before that goes in.

cheers

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] powerpc/crypto: add 842 hardware compression driver

[Michael Ellerman]

On Thu, 2012-07-19 at 09:42 -0500, Seth Jennings wrote:
 This patch adds the driver for interacting with the 842
 compression accelerator on IBM Power7+ systems.

...

 +struct nx842_slentry {
 + unsigned long ptr; /* Absolute address (use virt_to_abs()) */
 /+unsigned long len;
 +};

These days virt_to_abs() is just __pa() - ie. convert to a real address.

So you should just use __pa().

And you also need to be certain that you only call it on addresses where
that makes sense, ie. not vmalloc etc.

cheers

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html