Re: [PATCH v3 2/3] lib/crypto: x86/sha1: PHE Extensions optimized SHA1 transform function

[AlanSong-oc]

On 3/12/26 12:03, Eric Biggers wrote:
> On Wed, Mar 11, 2026 at 07:37:39PM +0800, AlanSong-oc wrote:
>>> I also have to ask: are you sure you need SHA-1 to be optimized at all?
>>> SHA-1 has been deprecated for a long time.  Most users have moved to
>>> SHA-256 and other stronger algorithms, and those that haven't need to
>>> move very soon.  There's little value in adding new optimized code for
>>> SHA-1.
>>>
>>> How about simplifying your patch to just SHA-256?  Then we can focus on
>>> the one that's actually important and not on the deprecated SHA-1.
>>
>> It is true that SHA-1 is rarely used by most users today. However, it
>> may still be needed in certain scenarios. For those cases, we would like
>> to add support for the XHSA1 instruction to accelerate SHA-1.
>>
>> Does the crypto community have any plans to remove SHA-1 support in
>> recent kernel versions?
> 
> It's already possible to build a kernel without SHA-1 support.  SHA-1
> has been cryptographically broken and is considered obsolete.
> Performance-critical hashing in the kernel already tends to use SHA-256.
> 
> These patches already feel marginal, as they are being pushed without
> QEMU support, so the community will be unable to test them.  The only
> reason I would consider accepting them without QEMU support is because
> there was already code in drivers/crypto/ that used these instructions.

Sorry for the inconvenience caused by the inability to test provided
patches, as QEMU currently does not support emulation of the XSHA1 and
XSHA256 instructions.

Besides, since the previous patch adding XSHA384 and XSHA512 instruction
support was not accepted, I would like to ask whether adding emulation
support for XSHA384 and XSHA512 instructions in QEMU would help the
crypto community evaluate and accept the corresponding kernel patches.

> It also helps that they are just single instructions.  Though, even with
> that I still found a bug in the proposed code as well as errors in the
> CPU documentation, as mentioned.  And the drivers/crypto/ implementation
> that uses these instructions is broken too, as you're aware of.
> 
> Overall, it's clear that platform-specific routines like this are very
> risky to maintain without adequate testing.  Yet, correctness is the
> first priority in cryptographic code.
> 
> So I would suggest that to reduce the risk, we focus on just one
> algorithm, SHA-256.  Note that this makes your job easier, as well.

Thanks for your suggestions. I will only add XSHA256 instruction support
for the SHA-256 algorithm in the next version of the patch.

Best Regards
AlanSong-oc