Re: crypto: x86/crct10dif-pcl - cleanup and optimizations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 17, 2019 at 11:06:21AM -0700, Nick Desaulniers wrote:
> On Mon, Jun 17, 2019 at 6:35 AM Sedat Dilek <sedat.dilek@xxxxxxxxx> wrote:
> >
> > Hi,
> >
> > while digging through a ClangBuiltLinux issue when linking with LLD
> > linker on x86-64 I checked the settings for...
> >
> > .rodata.cst16 and .rodata.cst32
> >
> > ...in crypto tree and fell over this change in...
> >
> > commit "crypto: x86/crct10dif-pcl - cleanup and optimizations":
> >
> > -.section .rodata.cst16.SHUF_MASK, "aM", @progbits, 16
> > +.section .rodata.cst32.byteshift_table, "aM", @progbits, 32
> > .align 16
> >
> > Is that a typo?
> > I would have expected...
> > .rodata.cst32.XXX -> .align 32
> > or
> > rodata.cst16.XXX -> .align 16
> >
> > But I might be wrong as I am no expert for crypto and x86/asm.
> >
> > Thanks in advance.
> >
> > Regards,
> > - Sedat -
> >
> > [1] https://github.com/ClangBuiltLinux/linux/issues/431
> > [2] https://bugs.llvm.org/show_bug.cgi?id=42289
> 
> > [3] https://git.kernel.org/linus/0974037fc55c
> 
> + Peter, Fangrui (who have looked at this, and started looking into
> this from LLD's perspective)
> 
> In fact, looking closer at that diff, the section in question
> previously had 32b alignment.  Eric, was that change intentional?  It
> seems funny to have a 32b entity size but a 16b alignment.
> 
> PDF page 81 / printed page 67 of this doc:
> https://web.eecs.umich.edu/~prabal/teaching/resources/eecs373/Assembler.pdf
> says:
> 
> "The linker may remove duplicates within sections with the
> same name, same entity size and same flags. "
> 
> So for us, LLD is NOT merging these sections due to differing
> alignments, which is producing warnings when loading such kernel
> modules that link against these object files.
> -- 
> Thanks,
> ~Nick Desaulniers

It was an intentional change since actually no alignment is required for this.
It's an array of 32 bytes and the code loads 16 bytes starting from some byte
offset in the range 1...16, so it has to use movdqu anyway.

There's no problem with changing it back to 32, but I don't fully understand
what's going on here.  Where is it documented how alignment specifiers interact
with the section merging?  Also, there are already other mergeable sections in
the x86 crypto code with alignment smaller than their entity size, e.g.

	.rodata.cst16.aegis128_const
	.rodata.cst16.aegis128l_const
	.rodata.cst16.aegis256_const
	.rodata.cst16.morus640_const
	.rodata.cst256.K256

Do those need to be changed too?

- Eric



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux