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

[Eric Biggers <ebiggers@kernel.org>]

Hi Sedat,

On Wed, Jul 03, 2019 at 05:16:40PM +0200, Sedat Dilek wrote:
> 
> Hi Eric, Hi Nick,
> 
> I am building Linux v5.1.16 with a new llvm-toolchain including the fix for LLD:
> 
> "[ELF] Allow placing SHF_MERGE sections with different alignments into
> the same MergeSyntheticSection"
> 
> [ Alignment=16 before my patch ]
> 
> $ cd arch/x86/crypto/
> $ for o in $(ls *.o) ; do echo [ $o ] ; readelf -WS $o | grep
> rodata\.cst32 ; done
> 
> [ crct10dif-pcl-asm_64.o ]
>   [ 9] .rodata.cst32.byteshift_table PROGBITS        0000000000000000
> 0004e0 000020 20  AM  0   0 16
> 
> [ crct10dif-pclmul.o ]
>   [ 9] .rodata.cst32.byteshift_table PROGBITS        0000000000000000
> 000b40 000020 20  AM  0   0 16
> 
> [ Alignment=32 after my patch ]
> 
> [ crct10dif-pcl-asm_64.o ]
>   [ 9] .rodata.cst32.byteshift_table PROGBITS        0000000000000000
> 0004e0 000020 20  AM  0   0 32
> 
> [ crct10dif-pclmul.o ]
>   [ 9] .rodata.cst32.byteshift_table PROGBITS        0000000000000000
> 000b40 000020 20  AM  0   0 32
> 
> I am still building the Linux-kernel but first checks in [3] looks good.
> 
> I can send out a separate patch if you like for the issue I have reported.

Sorry, I am still confused.  Are you saying that something still needs to be
fixed in the kernel code, and if so, why?  To reiterate, the byteshift_table
doesn't actually *need* any particular alignment.  Would it avoid the confusion
if I changed it to no alignment?  Or is there some section merging related
reason it actually needs to be 32?

> 
> I can not say much to ...
> 
> >         .rodata.cst16.aegis128_const
> >         .rodata.cst16.aegis128l_const
> >         .rodata.cst16.aegis256_const
> >         .rodata.cst16.morus640_const
> >         .rodata.cst256.K256
> 
> ... as I am not a Linker or Linux/x86/crypto specialist.

Well those all seem to be the same issue; the needed alignment isn't the same as
the entity size.  So if the crct10dif one needs to be fixed, these need to be
too.  Am I missing something?

- Eric