Linux cryptographic layer development
 help / color / mirror / Atom feed
* [cryptodev:master 43/44] arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr tt,=crypto_ft_tab'
From: kbuild test robot @ 2017-01-12 23:28 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: kbuild-all, linux-crypto, Herbert Xu

[-- Attachment #1: Type: text/plain, Size: 8989 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
head:   1abee99eafab67fb1c98f9ecfc43cd5735384a86
commit: 81edb42629758bacdf813dd5e4542ae26e3ad73a [43/44] crypto: arm/aes - replace scalar AES cipher
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 81edb42629758bacdf813dd5e4542ae26e3ad73a
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/crypto/aes-cipher-core.S: Assembler messages:
   arch/arm/crypto/aes-cipher-core.S:21: Error: selected processor does not support `tt .req ip' in ARM mode
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr tt,=crypto_ft_tab'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r8,[tt,r8,lsl#2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t0,[tt,t0,lsr#(8*1)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r9,[tt,r9,lsl#2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t1,[tt,t1,lsr#(8*1)-2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t2,[tt,t2,lsr#(8*2)-2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r10,[tt,r10,lsr#(8*2)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t0,[tt,t0,lsr#(8*3)-2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r11,[tt,r11,lsr#(8*3)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r10,[tt,r10,lsl#2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t0,[tt,t0,lsr#(8*1)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r11,[tt,r11,lsl#2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t1,[tt,t1,lsr#(8*1)-2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t2,[tt,t2,lsr#(8*2)-2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r5,[tt,r5,lsr#(8*2)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t0,[tt,t0,lsr#(8*3)-2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r6,[tt,r6,lsr#(8*3)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r4,[tt,r4,lsl#2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t0,[tt,t0,lsr#(8*1)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r5,[tt,r5,lsl#2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t1,[tt,t1,lsr#(8*1)-2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t2,[tt,t2,lsr#(8*2)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r6,[tt,r6,lsr#(8*2)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t0,[tt,t0,lsr#(8*3)-2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r7,[tt,r7,lsr#(8*3)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r6,[tt,r6,lsl#2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t0,[tt,t0,lsr#(8*1)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r7,[tt,r7,lsl#2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t1,[tt,t1,lsr#(8*1)-2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t2,[tt,t2,lsr#(8*2)-2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r9,[tt,r9,lsr#(8*2)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t0,[tt,t0,lsr#(8*3)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r10,[tt,r10,lsr#(8*3)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r8,[tt,r8,lsl#2]'
>> arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr t0,[tt,t0,lsr#(8*1)-2]'
   arch/arm/crypto/aes-cipher-core.S:174: Error: ARM register expected -- `ldr r9,[tt,r9,lsl#2]'

vim +174 arch/arm/crypto/aes-cipher-core.S

    15		.align		5
    16	
    17		rk		.req	r0
    18		rounds		.req	r1
    19		in		.req	r2
    20		out		.req	r3
  > 21		tt		.req	ip
    22	
    23		t0		.req	lr
    24		t1		.req	r2
    25		t2		.req	r3
    26	
    27		.macro		__select, out, in, idx
    28		.if		__LINUX_ARM_ARCH__ < 7
    29		and		\out, \in, #0xff << (8 * \idx)
    30		.else
    31		ubfx		\out, \in, #(8 * \idx), #8
    32		.endif
    33		.endm
    34	
    35		.macro		__load, out, in, idx
    36		.if		__LINUX_ARM_ARCH__ < 7 && \idx > 0
    37		ldr		\out, [tt, \in, lsr #(8 * \idx) - 2]
    38		.else
    39		ldr		\out, [tt, \in, lsl #2]
    40		.endif
    41		.endm
    42	
    43		.macro		__hround, out0, out1, in0, in1, in2, in3, t3, t4, enc
    44		__select	\out0, \in0, 0
    45		__select	t0, \in1, 1
    46		__load		\out0, \out0, 0
    47		__load		t0, t0, 1
    48	
    49		.if		\enc
    50		__select	\out1, \in1, 0
    51		__select	t1, \in2, 1
    52		.else
    53		__select	\out1, \in3, 0
    54		__select	t1, \in0, 1
    55		.endif
    56		__load		\out1, \out1, 0
    57		__select	t2, \in2, 2
    58		__load		t1, t1, 1
    59		__load		t2, t2, 2
    60	
    61		eor		\out0, \out0, t0, ror #24
    62	
    63		__select	t0, \in3, 3
    64		.if		\enc
    65		__select	\t3, \in3, 2
    66		__select	\t4, \in0, 3
    67		.else
    68		__select	\t3, \in1, 2
    69		__select	\t4, \in2, 3
    70		.endif
    71		__load		\t3, \t3, 2
    72		__load		t0, t0, 3
    73		__load		\t4, \t4, 3
    74	
    75		eor		\out1, \out1, t1, ror #24
    76		eor		\out0, \out0, t2, ror #16
    77		ldm		rk!, {t1, t2}
    78		eor		\out1, \out1, \t3, ror #16
    79		eor		\out0, \out0, t0, ror #8
    80		eor		\out1, \out1, \t4, ror #8
    81		eor		\out0, \out0, t1
    82		eor		\out1, \out1, t2
    83		.endm
    84	
    85		.macro		fround, out0, out1, out2, out3, in0, in1, in2, in3
    86		__hround	\out0, \out1, \in0, \in1, \in2, \in3, \out2, \out3, 1
    87		__hround	\out2, \out3, \in2, \in3, \in0, \in1, \in1, \in2, 1
    88		.endm
    89	
    90		.macro		iround, out0, out1, out2, out3, in0, in1, in2, in3
    91		__hround	\out0, \out1, \in0, \in3, \in2, \in1, \out2, \out3, 0
    92		__hround	\out2, \out3, \in2, \in1, \in0, \in3, \in1, \in0, 0
    93		.endm
    94	
    95		.macro		__rev, out, in
    96		.if		__LINUX_ARM_ARCH__ < 6
    97		lsl		t0, \in, #24
    98		and		t1, \in, #0xff00
    99		and		t2, \in, #0xff0000
   100		orr		\out, t0, \in, lsr #24
   101		orr		\out, \out, t1, lsl #8
   102		orr		\out, \out, t2, lsr #8
   103		.else
   104		rev		\out, \in
   105		.endif
   106		.endm
   107	
   108		.macro		__adrl, out, sym, c
   109		.if		__LINUX_ARM_ARCH__ < 7
   110		ldr\c		\out, =\sym
   111		.else
   112		movw\c		\out, #:lower16:\sym
   113		movt\c		\out, #:upper16:\sym
   114		.endif
   115		.endm
   116	
   117		.macro		do_crypt, round, ttab, ltab
   118		push		{r3-r11, lr}
   119	
   120		ldr		r4, [in]
   121		ldr		r5, [in, #4]
   122		ldr		r6, [in, #8]
   123		ldr		r7, [in, #12]
   124	
   125		ldm		rk!, {r8-r11}
   126	
   127	#ifdef CONFIG_CPU_BIG_ENDIAN
   128		__rev		r4, r4
   129		__rev		r5, r5
   130		__rev		r6, r6
   131		__rev		r7, r7
   132	#endif
   133	
   134		eor		r4, r4, r8
   135		eor		r5, r5, r9
   136		eor		r6, r6, r10
   137		eor		r7, r7, r11
   138	
   139		__adrl		tt, \ttab
   140	
   141		tst		rounds, #2
   142		bne		1f
   143	
   144	0:	\round		r8, r9, r10, r11, r4, r5, r6, r7
   145		\round		r4, r5, r6, r7, r8, r9, r10, r11
   146	
   147	1:	subs		rounds, rounds, #4
   148		\round		r8, r9, r10, r11, r4, r5, r6, r7
   149		__adrl		tt, \ltab, ls
   150		\round		r4, r5, r6, r7, r8, r9, r10, r11
   151		bhi		0b
   152	
   153	#ifdef CONFIG_CPU_BIG_ENDIAN
   154		__rev		r4, r4
   155		__rev		r5, r5
   156		__rev		r6, r6
   157		__rev		r7, r7
   158	#endif
   159	
   160		ldr		out, [sp]
   161	
   162		str		r4, [out]
   163		str		r5, [out, #4]
   164		str		r6, [out, #8]
   165		str		r7, [out, #12]
   166	
   167		pop		{r3-r11, pc}
   168	
   169		.align		3
   170		.ltorg
   171		.endm
   172	
   173	ENTRY(__aes_arm_encrypt)
 > 174		do_crypt	fround, crypto_ft_tab, crypto_fl_tab
   175	ENDPROC(__aes_arm_encrypt)
   176	
   177	ENTRY(__aes_arm_decrypt)
 > 178		do_crypt	iround, crypto_it_tab, crypto_il_tab
   179	ENDPROC(__aes_arm_decrypt)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 60405 bytes --]

^ permalink raw reply

* Re: x86-64: Maintain 16-byte stack alignment
From: Andy Lutomirski @ 2017-01-13  1:46 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Linus Torvalds, Herbert Xu, Linux Kernel Mailing List,
	Linux Crypto Mailing List, Ingo Molnar, Thomas Gleixner,
	Andy Lutomirski, Ard Biesheuvel
In-Reply-To: <20170112201511.yj5ekqmj76r2yv6t@treble>

On Thu, Jan 12, 2017 at 12:15 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Thu, Jan 12, 2017 at 12:08:07PM -0800, Andy Lutomirski wrote:
>> On Thu, Jan 12, 2017 at 11:51 AM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>> > On Thu, Jan 12, 2017 at 6:02 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>> >>
>> >> Just to clarify, I think you're asking if, for versions of gcc which
>> >> don't support -mpreferred-stack-boundary=3, objtool can analyze all C
>> >> functions to ensure their stacks are 16-byte aligned.
>> >>
>> >> It's certainly possible, but I don't see how that solves the problem.
>> >> The stack will still be misaligned by entry code.  Or am I missing
>> >> something?
>> >
>> > I think the argument is that we *could* try to align things, if we
>> > just had some tool that actually then verified that we aren't missing
>> > anything.
>> >
>> > I'm not entirely happy with checking the generated code, though,
>> > because as Ingo says, you have a 50:50 chance of just getting it right
>> > by mistake. So I'd much rather have some static tool that checks
>> > things at a code level (ie coccinelle or sparse).
>>
>> What I meant was checking the entry code to see if it aligns stack
>> frames, and good luck getting sparse to do that.  Hmm, getting 16-byte
>> alignment for real may actually be entirely a lost cause.  After all,
>> I think we have some inline functions that do asm volatile ("call
>> ..."), and I don't see any credible way of forcing alignment short of
>> generating an entirely new stack frame and aligning that.
>
> Actually we already found all such cases and fixed them by forcing a new
> stack frame, thanks to objtool.  For example, see 55a76b59b5fe.

What I mean is: what guarantees that the stack is properly aligned for
the subroutine call?  gcc promises to set up a stack frame, but does
it promise that rsp will be properly aligned to call a C function?

^ permalink raw reply

* RE: [PATCH v8 1/1] crypto: add virtio-crypto driver
From: Gonglei (Arei) @ 2017-01-13  1:56 UTC (permalink / raw)
  To: Michael S. Tsirkin, Christian Borntraeger
  Cc: linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
	virtio-dev@lists.oasis-open.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org, davem@davemloft.net,
	herbert@gondor.apana.org.au, Huangweidong (C), Claudio Fontana,
	Luonengjun, Hanweidong (Randy), Xuquan (Quan Xu),
	Wanzongshun (Vincent), stefanha@redhat.com,
	"Zhoujian (jay, Euler)" <
In-Reply-To: <20170112161729-mutt-send-email-mst@kernel.org>

> 
> On Thu, Jan 12, 2017 at 03:10:25PM +0100, Christian Borntraeger wrote:
> > On 01/10/2017 01:56 PM, Christian Borntraeger wrote:
> > > On 01/10/2017 01:36 PM, Gonglei (Arei) wrote:
> > >> Hi,
> > >>
> > >>>
> > >>> On 12/15/2016 03:03 AM, Gonglei wrote:
> > >>> [...]
> > >>>> +
> > >>>> +static struct crypto_alg virtio_crypto_algs[] = { {
> > >>>> +	.cra_name = "cbc(aes)",
> > >>>> +	.cra_driver_name = "virtio_crypto_aes_cbc",
> > >>>> +	.cra_priority = 501,
> > >>>
> > >>>
> > >>> This is still higher than the hardware-accelerators (like intel aesni or the
> > >>> s390 cpacf functions or the arm hw). aesni and s390/cpacf are supported
> by the
> > >>> hardware virtualization and available to the guests. I do not see a way
> how
> > >>> virtio
> > >>> crypto can be faster than that (in the end it might be cpacf/aesni +
> overhead)
> > >>> instead it will very likely be slower.
> > >>> So we should use a number that is higher than software implementations
> but
> > >>> lower than the hw ones.
> > >>>
> > >>> Just grepping around, the software ones seem be be around 100 and the
> > >>> hardware
> > >>> ones around 200-400. So why was 150 not enough?
> > >>>
> > >> I didn't find a documentation about how we use the priority, and I assumed
> > >> people use virtio-crypto will configure hardware accelerators in the
> > >> host. So I choosed the number which bigger than aesni's priority.
> > >
> > > Yes, but the aesni driver will only bind if there is HW support in the guest.
> > > And if aesni is available in the guest (or the s390 aes function from cpacf)
> > > it will always be faster than the same in the host via virtio.So your priority
> > > should be smaller.
> >
> >
> > any opinion on this?
> 
> Going forward, we might add an emulated aesni device and that might
> become slower than virtio. OTOH if or when this happens, we can solve it
> by adding a priority or a feature flag to virtio to raise its priority.
> 
> So I think I agree with Christian here, let's lower the priority.
> Gonglei, could you send a patch like this?
> 
OK, will do.

Thanks,
-Gonglei

^ permalink raw reply

* Re: x86-64: Maintain 16-byte stack alignment
From: Josh Poimboeuf @ 2017-01-13  3:11 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Linus Torvalds, Herbert Xu, Linux Kernel Mailing List,
	Linux Crypto Mailing List, Ingo Molnar, Thomas Gleixner,
	Andy Lutomirski, Ard Biesheuvel
In-Reply-To: <CALCETrXom8aY2XhpAyOtAwQQYF7wftBHJE_px1xr0iRmcYEJoA@mail.gmail.com>

On Thu, Jan 12, 2017 at 05:46:55PM -0800, Andy Lutomirski wrote:
> On Thu, Jan 12, 2017 at 12:15 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Thu, Jan 12, 2017 at 12:08:07PM -0800, Andy Lutomirski wrote:
> >> On Thu, Jan 12, 2017 at 11:51 AM, Linus Torvalds
> >> <torvalds@linux-foundation.org> wrote:
> >> > On Thu, Jan 12, 2017 at 6:02 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >> >>
> >> >> Just to clarify, I think you're asking if, for versions of gcc which
> >> >> don't support -mpreferred-stack-boundary=3, objtool can analyze all C
> >> >> functions to ensure their stacks are 16-byte aligned.
> >> >>
> >> >> It's certainly possible, but I don't see how that solves the problem.
> >> >> The stack will still be misaligned by entry code.  Or am I missing
> >> >> something?
> >> >
> >> > I think the argument is that we *could* try to align things, if we
> >> > just had some tool that actually then verified that we aren't missing
> >> > anything.
> >> >
> >> > I'm not entirely happy with checking the generated code, though,
> >> > because as Ingo says, you have a 50:50 chance of just getting it right
> >> > by mistake. So I'd much rather have some static tool that checks
> >> > things at a code level (ie coccinelle or sparse).
> >>
> >> What I meant was checking the entry code to see if it aligns stack
> >> frames, and good luck getting sparse to do that.  Hmm, getting 16-byte
> >> alignment for real may actually be entirely a lost cause.  After all,
> >> I think we have some inline functions that do asm volatile ("call
> >> ..."), and I don't see any credible way of forcing alignment short of
> >> generating an entirely new stack frame and aligning that.
> >
> > Actually we already found all such cases and fixed them by forcing a new
> > stack frame, thanks to objtool.  For example, see 55a76b59b5fe.
> 
> What I mean is: what guarantees that the stack is properly aligned for
> the subroutine call?  gcc promises to set up a stack frame, but does
> it promise that rsp will be properly aligned to call a C function?

Yes, I did an experiment and you're right.  I had naively assumed that
all stack frames would be aligned.

-- 
Josh

^ permalink raw reply

* Re: [RFC PATCH 5/6] crypto: aesni-intel - Add bulk request support
From: Eric Biggers @ 2017-01-13  3:19 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: Herbert Xu, linux-crypto, dm-devel, Mike Snitzer, Milan Broz,
	Mikulas Patocka, Binoy Jayan
In-Reply-To: <c32a28630157c619ac2a7c851be586e72f193c68.1484215956.git.omosnacek@gmail.com>

On Thu, Jan 12, 2017 at 01:59:57PM +0100, Ondrej Mosnacek wrote:
> This patch implements bulk request handling in the AES-NI crypto drivers.
> The major advantage of this is that with bulk requests, the kernel_fpu_*
> functions (which are usually quite slow) are now called only once for the whole
> request.
> 

Hi Ondrej,

To what extent does the performance benefit of this patchset result from just
the reduced numbers of calls to kernel_fpu_begin() and kernel_fpu_end()?

If it's most of the benefit, would it make any sense to optimize
kernel_fpu_begin() and kernel_fpu_end() instead?

And if there are other examples besides kernel_fpu_begin/kernel_fpu_end where
the bulk API would provide a significant performance boost, can you mention
them?

Interestingly, the arm64 equivalent to kernel_fpu_begin()
(kernel_neon_begin_partial() in arch/arm64/kernel/fpsimd.c) appears to have an
optimization where the SIMD registers aren't saved if they were already saved.
I wonder why something similar isn't done on x86.

Eric

^ permalink raw reply

* Re: x86-64: Maintain 16-byte stack alignment
From: Andy Lutomirski @ 2017-01-13  3:23 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Linus Torvalds, Herbert Xu, Linux Kernel Mailing List,
	Linux Crypto Mailing List, Ingo Molnar, Thomas Gleixner,
	Andy Lutomirski, Ard Biesheuvel
In-Reply-To: <20170113031107.mgitq54fmjnrvi6f@treble>

On Thu, Jan 12, 2017 at 7:11 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Thu, Jan 12, 2017 at 05:46:55PM -0800, Andy Lutomirski wrote:
>> On Thu, Jan 12, 2017 at 12:15 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>> > On Thu, Jan 12, 2017 at 12:08:07PM -0800, Andy Lutomirski wrote:
>> >> On Thu, Jan 12, 2017 at 11:51 AM, Linus Torvalds
>> >> <torvalds@linux-foundation.org> wrote:
>> >> > On Thu, Jan 12, 2017 at 6:02 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>> >> >>
>> >> >> Just to clarify, I think you're asking if, for versions of gcc which
>> >> >> don't support -mpreferred-stack-boundary=3, objtool can analyze all C
>> >> >> functions to ensure their stacks are 16-byte aligned.
>> >> >>
>> >> >> It's certainly possible, but I don't see how that solves the problem.
>> >> >> The stack will still be misaligned by entry code.  Or am I missing
>> >> >> something?
>> >> >
>> >> > I think the argument is that we *could* try to align things, if we
>> >> > just had some tool that actually then verified that we aren't missing
>> >> > anything.
>> >> >
>> >> > I'm not entirely happy with checking the generated code, though,
>> >> > because as Ingo says, you have a 50:50 chance of just getting it right
>> >> > by mistake. So I'd much rather have some static tool that checks
>> >> > things at a code level (ie coccinelle or sparse).
>> >>
>> >> What I meant was checking the entry code to see if it aligns stack
>> >> frames, and good luck getting sparse to do that.  Hmm, getting 16-byte
>> >> alignment for real may actually be entirely a lost cause.  After all,
>> >> I think we have some inline functions that do asm volatile ("call
>> >> ..."), and I don't see any credible way of forcing alignment short of
>> >> generating an entirely new stack frame and aligning that.
>> >
>> > Actually we already found all such cases and fixed them by forcing a new
>> > stack frame, thanks to objtool.  For example, see 55a76b59b5fe.
>>
>> What I mean is: what guarantees that the stack is properly aligned for
>> the subroutine call?  gcc promises to set up a stack frame, but does
>> it promise that rsp will be properly aligned to call a C function?
>
> Yes, I did an experiment and you're right.  I had naively assumed that
> all stack frames would be aligned.

Just to check: did you do your experiment with -mpreferred-stack-boundary=4?

--Andy

^ permalink raw reply

* Re: x86-64: Maintain 16-byte stack alignment
From: Josh Poimboeuf @ 2017-01-13  4:27 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Linus Torvalds, Herbert Xu, Linux Kernel Mailing List,
	Linux Crypto Mailing List, Ingo Molnar, Thomas Gleixner,
	Andy Lutomirski, Ard Biesheuvel
In-Reply-To: <CALCETrUFCFn-rKnr+NG3SU7J78ree9siJC=Kz8f_Bk6eG2HyPA@mail.gmail.com>

On Thu, Jan 12, 2017 at 07:23:18PM -0800, Andy Lutomirski wrote:
> On Thu, Jan 12, 2017 at 7:11 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Thu, Jan 12, 2017 at 05:46:55PM -0800, Andy Lutomirski wrote:
> >> On Thu, Jan 12, 2017 at 12:15 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >> > On Thu, Jan 12, 2017 at 12:08:07PM -0800, Andy Lutomirski wrote:
> >> >> On Thu, Jan 12, 2017 at 11:51 AM, Linus Torvalds
> >> >> <torvalds@linux-foundation.org> wrote:
> >> >> > On Thu, Jan 12, 2017 at 6:02 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >> >> >>
> >> >> >> Just to clarify, I think you're asking if, for versions of gcc which
> >> >> >> don't support -mpreferred-stack-boundary=3, objtool can analyze all C
> >> >> >> functions to ensure their stacks are 16-byte aligned.
> >> >> >>
> >> >> >> It's certainly possible, but I don't see how that solves the problem.
> >> >> >> The stack will still be misaligned by entry code.  Or am I missing
> >> >> >> something?
> >> >> >
> >> >> > I think the argument is that we *could* try to align things, if we
> >> >> > just had some tool that actually then verified that we aren't missing
> >> >> > anything.
> >> >> >
> >> >> > I'm not entirely happy with checking the generated code, though,
> >> >> > because as Ingo says, you have a 50:50 chance of just getting it right
> >> >> > by mistake. So I'd much rather have some static tool that checks
> >> >> > things at a code level (ie coccinelle or sparse).
> >> >>
> >> >> What I meant was checking the entry code to see if it aligns stack
> >> >> frames, and good luck getting sparse to do that.  Hmm, getting 16-byte
> >> >> alignment for real may actually be entirely a lost cause.  After all,
> >> >> I think we have some inline functions that do asm volatile ("call
> >> >> ..."), and I don't see any credible way of forcing alignment short of
> >> >> generating an entirely new stack frame and aligning that.
> >> >
> >> > Actually we already found all such cases and fixed them by forcing a new
> >> > stack frame, thanks to objtool.  For example, see 55a76b59b5fe.
> >>
> >> What I mean is: what guarantees that the stack is properly aligned for
> >> the subroutine call?  gcc promises to set up a stack frame, but does
> >> it promise that rsp will be properly aligned to call a C function?
> >
> > Yes, I did an experiment and you're right.  I had naively assumed that
> > all stack frames would be aligned.
> 
> Just to check: did you do your experiment with -mpreferred-stack-boundary=4?

Yes, but it's too late for me to be doing hard stuff and I think my
first experiment was bogus.  I didn't use all the other kernel-specific
gcc options.

I tried again with all the kernel gcc options, except with
-mpreferred-stack-boundary=4 instead of 3, and actually came up with the
opposite conclusion.

I used the following code:

void otherfunc(void);

static inline void bar(long *f)
{
	asm volatile("call otherfunc" : : "m" (f) : );
}

void foo(void)
{
	long buf[3] = {0, 0, 0};
	bar(buf);
}

The stack frame was always 16-byte aligned regardless of whether the
buf array size was even or odd.

So my half-asleep brain is telling me that my original assumption was
right.

-- 
Josh

^ permalink raw reply

* Re: x86-64: Maintain 16-byte stack alignment
From: Josh Poimboeuf @ 2017-01-13  5:07 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux Crypto Mailing List, Thomas Gleixner, Herbert Xu,
	Andy Lutomirski, Ingo Molnar, Andy Lutomirski,
	Linux Kernel Mailing List, Ard Biesheuvel
In-Reply-To: <CA+55aFzRrSwGxxfZk-RUEnsz=xhcSmOwE1CenfCPBWtsS9MwDw@mail.gmail.com>

On Thu, Jan 12, 2017 at 08:37:18PM -0800, Linus Torvalds wrote:
> On Jan 12, 2017 8:28 PM, "Josh Poimboeuf" <jpoimboe@redhat.com> wrote:
> 
> 
> The stack frame was always 16-byte aligned regardless of whether the
> buf array size was even or odd.
> 
> 
> Including with -fomit-frame-pointer?
> 
> With frame pointers, stack frames really are naturally 16 bytes, and then
> keeping the frame 16-byte aligned is just a matter of making any extra
> frame allocations or push/pop sequences that you do also be a multiple of
> 16 bytes.
> 
> But *without* frame pointers, the"native" frame size is just 8 bytes, and a
> function that doesn't need any other local storage and then calls another
> function (think various trivial wrapper functions that just add an argument
> and then munge the return value) would thus naturally cause the frame to
> become misaligned.
> 
> So then the compiler actually needs to start adding useless instructions
> just to keep the stack 16-byte aligned.

Disabling frame pointers didn't seem to help, but I finally got it to
misalign with a different test case.  I think it had been aligning the
array, so instead I made it push a register.


void otherfunc(void);

static inline void bar(int f)
{
	register void *__sp asm(_ASM_SP);
	asm volatile("call otherfunc" : "+r" (__sp) : "b"(f));
}

void foo(void)
{
	bar(5);
}


00000000000020f0 <foo>:
    20f0:	55                   	push   %rbp
    20f1:	48 89 e5             	mov    %rsp,%rbp
    20f4:	53                   	push   %rbx
    20f5:	bb 05 00 00 00       	mov    $0x5,%ebx
    20fa:	e8 00 00 00 00       	callq  20ff <foo+0xf>
			20fb: R_X86_64_PC32	otherfunc-0x4
    20ff:	5b                   	pop    %rbx
    2100:	5d                   	pop    %rbp
    2101:	c3                   	retq   
    2102:	0f 1f 40 00          	nopl   0x0(%rax)
    2106:	66 2e 0f 1f 84 00 00 	nopw   %cs:0x0(%rax,%rax,1)
    210d:	00 00 00 

-- 
Josh

^ permalink raw reply

* [PATCH] virtio-crypto: adjust priority of algorithm
From: Gonglei @ 2017-01-13  6:25 UTC (permalink / raw)
  To: virtualization, linux-crypto, linux-kernel
  Cc: mst, herbert, borntraeger, Gonglei

Some hardware accelerators (like intel aseni or the s390
cpacf functions) have lower priorities than virtio
crypto, and those drivers are faster than the same in
the host via virtio. So let's lower the priority of
virtio-crypto's algorithm, make it's higher than sofeware
implimentations but lower than the hardware ones.

Suggested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 drivers/crypto/virtio/virtio_crypto_algs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c
index 6f40a42..4de4740 100644
--- a/drivers/crypto/virtio/virtio_crypto_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_algs.c
@@ -498,7 +498,7 @@ void virtio_crypto_ablkcipher_finalize_req(
 static struct crypto_alg virtio_crypto_algs[] = { {
 	.cra_name = "cbc(aes)",
 	.cra_driver_name = "virtio_crypto_aes_cbc",
-	.cra_priority = 501,
+	.cra_priority = 150,
 	.cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC,
 	.cra_blocksize = AES_BLOCK_SIZE,
 	.cra_ctxsize  = sizeof(struct virtio_crypto_ablkcipher_ctx),
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH] crypto: testmgr - don't DMA map IV from stack in test_skcipher()
From: Horia Geantă @ 2017-01-13  6:59 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David S. Miller, linux-crypto

Fix the "DMA-API: device driver maps memory from stack" warning
generated when crypto accelerators map the IV.

Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
---
 crypto/testmgr.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 500a5277cc22..64245aeef634 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -1081,12 +1081,16 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
 	const char *e, *d;
 	struct tcrypt_result result;
 	void *data;
-	char iv[MAX_IVLEN];
+	char *iv;
 	char *xbuf[XBUFSIZE];
 	char *xoutbuf[XBUFSIZE];
 	int ret = -ENOMEM;
 	unsigned int ivsize = crypto_skcipher_ivsize(tfm);
 
+	iv = kmalloc(MAX_IVLEN, GFP_KERNEL);
+	if (!iv)
+		return ret;
+
 	if (testmgr_alloc_buf(xbuf))
 		goto out_nobuf;
 
@@ -1328,6 +1332,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
 out_nooutbuf:
 	testmgr_free_buf(xbuf);
 out_nobuf:
+	kfree(iv);
 	return ret;
 }
 
-- 
2.4.4

^ permalink raw reply related

* Re: [PATCH] virtio-crypto: adjust priority of algorithm
From: Christian Borntraeger @ 2017-01-13  8:28 UTC (permalink / raw)
  To: Gonglei, virtualization, linux-crypto, linux-kernel; +Cc: herbert, mst
In-Reply-To: <1484288741-31100-1-git-send-email-arei.gonglei@huawei.com>

ACK. Whoever takes this patch might want to fixup 3 typos.

On 01/13/2017 07:25 AM, Gonglei wrote:

> Some hardware accelerators (like intel aseni or the s390
					aesni
> cpacf functions) have lower priorities than virtio
> crypto, and those drivers are faster than the same in
> the host via virtio. So let's lower the priority of
> virtio-crypto's algorithm, make it's higher than sofeware
                                                   software
> implimentations but lower than the hardware ones.
  implementations
> 
> Suggested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  drivers/crypto/virtio/virtio_crypto_algs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c
> index 6f40a42..4de4740 100644
> --- a/drivers/crypto/virtio/virtio_crypto_algs.c
> +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
> @@ -498,7 +498,7 @@ void virtio_crypto_ablkcipher_finalize_req(
>  static struct crypto_alg virtio_crypto_algs[] = { {
>  	.cra_name = "cbc(aes)",
>  	.cra_driver_name = "virtio_crypto_aes_cbc",
> -	.cra_priority = 501,
> +	.cra_priority = 150,
>  	.cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC,
>  	.cra_blocksize = AES_BLOCK_SIZE,
>  	.cra_ctxsize  = sizeof(struct virtio_crypto_ablkcipher_ctx),
> 

^ permalink raw reply

* [PATCH] crypto: arm/aes - avoid reserved 'tt' mnemonic in asm code
From: Ard Biesheuvel @ 2017-01-13  8:33 UTC (permalink / raw)
  To: linux-crypto; +Cc: linux-arm-kernel, herbert, arnd, Ard Biesheuvel

The ARMv8-M architecture introduces 'tt' and 'ttt' instructions,
which means we can no longer use 'tt' as a register alias on recent
versions of binutils for ARM. So replace the alias with 'ttab'.

Fixes: 81edb4262975 ("crypto: arm/aes - replace scalar AES cipher")
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/crypto/aes-cipher-core.S | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/crypto/aes-cipher-core.S b/arch/arm/crypto/aes-cipher-core.S
index b04261e1e068..c817a86c4ca8 100644
--- a/arch/arm/crypto/aes-cipher-core.S
+++ b/arch/arm/crypto/aes-cipher-core.S
@@ -18,7 +18,7 @@
 	rounds		.req	r1
 	in		.req	r2
 	out		.req	r3
-	tt		.req	ip
+	ttab		.req	ip
 
 	t0		.req	lr
 	t1		.req	r2
@@ -34,9 +34,9 @@
 
 	.macro		__load, out, in, idx
 	.if		__LINUX_ARM_ARCH__ < 7 && \idx > 0
-	ldr		\out, [tt, \in, lsr #(8 * \idx) - 2]
+	ldr		\out, [ttab, \in, lsr #(8 * \idx) - 2]
 	.else
-	ldr		\out, [tt, \in, lsl #2]
+	ldr		\out, [ttab, \in, lsl #2]
 	.endif
 	.endm
 
@@ -136,7 +136,7 @@
 	eor		r6, r6, r10
 	eor		r7, r7, r11
 
-	__adrl		tt, \ttab
+	__adrl		ttab, \ttab
 
 	tst		rounds, #2
 	bne		1f
@@ -146,7 +146,7 @@
 
 1:	subs		rounds, rounds, #4
 	\round		r8, r9, r10, r11, r4, r5, r6, r7
-	__adrl		tt, \ltab, ls
+	__adrl		ttab, \ltab, ls
 	\round		r4, r5, r6, r7, r8, r9, r10, r11
 	bhi		0b
 
-- 
2.7.4

^ permalink raw reply related

* Re: x86-64: Maintain 16-byte stack alignment
From: Herbert Xu @ 2017-01-13  8:36 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Linus Torvalds, Josh Poimboeuf, Linux Kernel Mailing List,
	Linux Crypto Mailing List, Ingo Molnar, Thomas Gleixner,
	Andy Lutomirski, Ard Biesheuvel
In-Reply-To: <CALCETrVz-wEFVUwrpS8-Ln9SWnsF5KxkqJC-Br6wJ+e0LGM9UA@mail.gmail.com>

On Thu, Jan 12, 2017 at 12:08:07PM -0800, Andy Lutomirski wrote:
>
> I think we have some inline functions that do asm volatile ("call
> ..."), and I don't see any credible way of forcing alignment short of
> generating an entirely new stack frame and aligning that.  Ick.  This

A straight asm call from C should always work because gcc keeps
the stack aligned in the prologue.

The only problem with inline assembly is when you start pushing
things onto the stack directly.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: x86-64: Maintain 16-byte stack alignment
From: Herbert Xu @ 2017-01-13  8:38 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Josh Poimboeuf, Andy Lutomirski, Linux Kernel Mailing List,
	Linux Crypto Mailing List, Ingo Molnar, Thomas Gleixner,
	Andy Lutomirski, Ard Biesheuvel
In-Reply-To: <CA+55aFwkodRf3QM+GLzq5G4F7fp+=ds-cHJRtCgJczMZgVx6Ug@mail.gmail.com>

On Thu, Jan 12, 2017 at 01:40:54PM -0800, Linus Torvalds wrote:
> 
> The 8-byte alignment mainly makes sense when the basic call sequence
> just adds 8 bytes, and you have functions without frames (that still
> call other functions).

The question is does it really make sense to save those 8 bytes
of padding on x86-64 when arm64 apparently also requires 16-byte
stack alignment.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: x86-64: Maintain 16-byte stack alignment
From: Herbert Xu @ 2017-01-13  8:39 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Josh Poimboeuf, Linus Torvalds, Linux Kernel Mailing List,
	Linux Crypto Mailing List, Ingo Molnar, Thomas Gleixner,
	Andy Lutomirski, Ard Biesheuvel
In-Reply-To: <CALCETrXom8aY2XhpAyOtAwQQYF7wftBHJE_px1xr0iRmcYEJoA@mail.gmail.com>

On Thu, Jan 12, 2017 at 05:46:55PM -0800, Andy Lutomirski wrote:
> 
> What I mean is: what guarantees that the stack is properly aligned for
> the subroutine call?  gcc promises to set up a stack frame, but does
> it promise that rsp will be properly aligned to call a C function?

Yes, as long as you don't go behind its back and start directly
pushing things onto the stack with inline asm.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: x86-64: Maintain 16-byte stack alignment
From: Herbert Xu @ 2017-01-13  8:42 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Josh Poimboeuf, Linux Crypto Mailing List, Thomas Gleixner,
	Andy Lutomirski, Ingo Molnar, Andy Lutomirski,
	Linux Kernel Mailing List, Ard Biesheuvel
In-Reply-To: <CA+55aFzRrSwGxxfZk-RUEnsz=xhcSmOwE1CenfCPBWtsS9MwDw@mail.gmail.com>

On Thu, Jan 12, 2017 at 08:37:18PM -0800, Linus Torvalds wrote:
>
> So then the compiler actually needs to start adding useless instructions
> just to keep the stack 16-byte aligned.

Which it does.  Of course most of the time no extra instructions
are required because there are stack variables, so it's just matter
of adding 8 to the value you're subtracting from rsp.  But it is
probably why gcc assumes that the stack is 16-byte aligned which
triggered my original crash.

Here is an example from the function that was involved in the crash,
without frame pointers:

00000000000001b0 <chacha20_simd>:
 1b0:   41 54                   push   %r12
 1b2:   55                      push   %rbp
 1b3:   48 81 ec f8 00 00 00    sub    $0xf8,%rsp

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: x86-64: Maintain 16-byte stack alignment
From: Herbert Xu @ 2017-01-13  8:43 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Linus Torvalds, Linux Crypto Mailing List, Thomas Gleixner,
	Andy Lutomirski, Ingo Molnar, Andy Lutomirski,
	Linux Kernel Mailing List, Ard Biesheuvel
In-Reply-To: <20170113050709.yrdtfet5d4sebubi@treble>

On Thu, Jan 12, 2017 at 11:07:09PM -0600, Josh Poimboeuf wrote:
> 
> Disabling frame pointers didn't seem to help, but I finally got it to
> misalign with a different test case.  I think it had been aligning the
> array, so instead I made it push a register.

Right.  If you start manipulating the stack directly then all bets
are off.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] crypto: testmgr - don't DMA map IV from stack in test_skcipher()
From: Herbert Xu @ 2017-01-13  8:46 UTC (permalink / raw)
  To: Horia Geantă; +Cc: David S. Miller, linux-crypto
In-Reply-To: <1484290756-20868-1-git-send-email-horia.geanta@nxp.com>

On Fri, Jan 13, 2017 at 08:59:16AM +0200, Horia Geantă wrote:
> Fix the "DMA-API: device driver maps memory from stack" warning
> generated when crypto accelerators map the IV.
> 
> Signed-off-by: Horia Geantă <horia.geanta@nxp.com>

Hmm, the IV comes in as a pointer.  So you should not assume that
it can be DMAed at all.

Perhaps we should change the API so that it gets passed in as an
SG list.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* RE: [PATCH] virtio-crypto: adjust priority of algorithm
From: Gonglei (Arei) @ 2017-01-13  9:24 UTC (permalink / raw)
  To: Christian Borntraeger, virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: herbert@gondor.apana.org.au, mst@redhat.com
In-Reply-To: <874ee338-4dd6-8a2c-fb95-59bd784de026@de.ibm.com>

>
> From: Christian Borntraeger [mailto:borntraeger@de.ibm.com]
> Sent: Friday, January 13, 2017 4:28 PM
> To: Gonglei (Arei); virtualization@lists.linux-foundation.org;
> linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: mst@redhat.com; herbert@gondor.apana.org.au
> Subject: Re: [PATCH] virtio-crypto: adjust priority of algorithm
> 
> ACK. Whoever takes this patch might want to fixup 3 typos.
> 
Thanks, I'd better send v2 IMHO.  :)

Regards,
-Gonglei

> On 01/13/2017 07:25 AM, Gonglei wrote:
> 
> > Some hardware accelerators (like intel aseni or the s390
> 					aesni
> > cpacf functions) have lower priorities than virtio
> > crypto, and those drivers are faster than the same in
> > the host via virtio. So let's lower the priority of
> > virtio-crypto's algorithm, make it's higher than sofeware
>                                                    software
> > implimentations but lower than the hardware ones.
>   implementations
> >
> > Suggested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  drivers/crypto/virtio/virtio_crypto_algs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c
> b/drivers/crypto/virtio/virtio_crypto_algs.c
> > index 6f40a42..4de4740 100644
> > --- a/drivers/crypto/virtio/virtio_crypto_algs.c
> > +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
> > @@ -498,7 +498,7 @@ void virtio_crypto_ablkcipher_finalize_req(
> >  static struct crypto_alg virtio_crypto_algs[] = { {
> >  	.cra_name = "cbc(aes)",
> >  	.cra_driver_name = "virtio_crypto_aes_cbc",
> > -	.cra_priority = 501,
> > +	.cra_priority = 150,
> >  	.cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC,
> >  	.cra_blocksize = AES_BLOCK_SIZE,
> >  	.cra_ctxsize  = sizeof(struct virtio_crypto_ablkcipher_ctx),
> >

^ permalink raw reply

* [PATCH v2] virtio-crypto: adjust priority of algorithm
From: Gonglei @ 2017-01-13  9:34 UTC (permalink / raw)
  To: virtualization, linux-crypto, linux-kernel
  Cc: mst, herbert, borntraeger, Gonglei

Some hardware accelerators (like intel aesni or the s390
cpacf functions) have lower priorities than virtio
crypto, and those drivers are faster than the same in
the host via virtio. So let's lower the priority of
virtio-crypto's algorithm, make it's higher than software
implementations but lower than the hardware ones.

Suggested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
v2:
    fix three typos. [Christian]
---
 drivers/crypto/virtio/virtio_crypto_algs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c
index 6f40a42..4de4740 100644
--- a/drivers/crypto/virtio/virtio_crypto_algs.c
+++ b/drivers/crypto/virtio/virtio_crypto_algs.c
@@ -498,7 +498,7 @@ void virtio_crypto_ablkcipher_finalize_req(
 static struct crypto_alg virtio_crypto_algs[] = { {
 	.cra_name = "cbc(aes)",
 	.cra_driver_name = "virtio_crypto_aes_cbc",
-	.cra_priority = 501,
+	.cra_priority = 150,
 	.cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC,
 	.cra_blocksize = AES_BLOCK_SIZE,
 	.cra_ctxsize  = sizeof(struct virtio_crypto_ablkcipher_ctx),
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH 1/2] crypto: aead AF_ALG - overhaul memory management
From: Herbert Xu @ 2017-01-13 10:21 UTC (permalink / raw)
  To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <5211147.RjDSfvrhhz@tauon.atsec.com>

On Thu, Jan 12, 2017 at 05:19:57PM +0100, Stephan Müller wrote:
>
> > I don't understand, what's wrong with:
> > 
> > sendmsg(fd, ...)
> > aio_read(iocb1)
> > sendmsg(fd, ...)
> > aio_read(iocb2)
> 
> Sure, that works. But here you limit yourself to one IOCB per aio_read. But 
> aio_read supports multiple IOCBs in one invocation. And this is the issue I am 
> considering.

Not really.  You just lay it out in the same way with lio_listio.
That is, a write followed by read, etc.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 00/13] crypto: copy AAD during encrypt for AEAD ciphers
From: Herbert Xu @ 2017-01-13 10:23 UTC (permalink / raw)
  To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <3463059.TVfkfLOfNf@tauon.atsec.com>

On Thu, Jan 12, 2017 at 05:37:33PM +0100, Stephan Müller wrote:
>
> I would not understand that statement.
> 
> With the patch mentioned above that I provided some weeks ago, we have the 
> following scenario for an encryption (in case of decryption, it is almost 
> identical, just the tag location is reversed):
> 
> user calls sendmsg with data buffer/IOVEC: AAD || PT
> 	-> algif_aead turns this into the src SGL
> 
> user calls recvmsg with data buffer/IOVEC: CT || Tag
> 	-> algif_aead creates the first SG entry in the dst SGL pointing to the 
> AAD from the src SGL
> 	-> algif_aead appends the user buffers to the dst SGL
> 
> 	-> algif_aead performs its operation and during that operation, only the 
> CT and Tag parts are changed
> 
> I.e. with the pre-pending of the SG pointing to the AAD from the src SGL to 
> the dst SGL we have a clean invocation of the kernel API.

But that means you can never invoke the in-place path of the kernel
API, which is the most optimised code path.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v2 0/7] crypto: ARM/arm64 - AES and ChaCha20 updates for v4.11
From: Herbert Xu @ 2017-01-13 10:28 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAKv+Gu9a5ZsWtT0XYczPBf54L7d5Whu5gG1eUOt8T-wms0orbg@mail.gmail.com>

On Thu, Jan 12, 2017 at 04:48:08PM +0000, Ard Biesheuvel wrote:
>
> Actually, patch #6 was the huge one not #7, and I don't see it in your tree yet.
> 
> https://git.kernel.org/cgit/linux/kernel/git/ardb/linux.git/commit/?h=crypto-arm-v4.11&id=cbf03b255f7c
> 
> The order does not matter, though, so could you please put it on top? Thanks.

OK I've applied it now and will push out soon.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [RFC PATCH 0/6] Add bulk skcipher requests to crypto API and dm-crypt
From: Herbert Xu @ 2017-01-13 10:41 UTC (permalink / raw)
  To: Ondrej Mosnacek
  Cc: linux-crypto, dm-devel, Mike Snitzer, Milan Broz, Mikulas Patocka,
	Binoy Jayan
In-Reply-To: <cover.1484215956.git.omosnacek@gmail.com>

On Thu, Jan 12, 2017 at 01:59:52PM +0100, Ondrej Mosnacek wrote:
> 
> the goal of this patchset is to allow those skcipher API users that need to
> process batches of small messages (especially dm-crypt) to do so efficiently.

Please explain why this can't be done with the existing framework
using IV generators similar to the ones used for IPsec.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 1/2] crypto: aead AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-01-13 10:49 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170113102145.GA23349@gondor.apana.org.au>

Am Freitag, 13. Januar 2017, 18:21:45 CET schrieb Herbert Xu:

Hi Herbert,

> On Thu, Jan 12, 2017 at 05:19:57PM +0100, Stephan Müller wrote:
> > > I don't understand, what's wrong with:
> > > 
> > > sendmsg(fd, ...)
> > > aio_read(iocb1)
> > > sendmsg(fd, ...)
> > > aio_read(iocb2)
> > 
> > Sure, that works. But here you limit yourself to one IOCB per aio_read.
> > But
> > aio_read supports multiple IOCBs in one invocation. And this is the issue
> > I am considering.
> 
> Not really.  You just lay it out in the same way with lio_listio.
> That is, a write followed by read, etc.

According to the man page of lio_listio(3) the provided AIO operations are 
executed in an unspecified order. I would infer from that statement that even 
if an order of write / read / write / read is defined by the caller, this 
order may not be followed by the kernel. Thus we would need to consider the 
case that in the end, algif has to process the order of write / write / read / 
read or any other order.

Besides, the crashes I reported for the current AIO implementation in 
algif_aead and algif_skcipher are always triggered when invoking an aio_read 
with two or more IOCBs. The most important aspect I want to cover with the 
patch set is to stop crashing the kernel.

Ciao
Stephan

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox