Received-SPF: pass (google.com: domain of linux-crypto+bounces-4919-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 12 Jun 2024 08:27:07 -0700
From: Eric Biggers <ebiggers@kernel.org>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org, fsverity@lists.linux.dev,
	dm-devel@lists.linux.dev, x86@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Ard Biesheuvel <ardb@kernel.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	Bart Van Assche <bvanassche@acm.org>
Subject: Re: [PATCH v5 04/15] crypto: x86/sha256-ni - add support for finup_mb
Message-ID: <20240612152707.GB1170@sol.localdomain>
References: <20240611034822.36603-1-ebiggers@kernel.org>
 <20240611034822.36603-5-ebiggers@kernel.org>
 <ZmltfGWFTj2Kq7hN@gondor.apana.org.au>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZmltfGWFTj2Kq7hN@gondor.apana.org.au>

On Wed, Jun 12, 2024 at 05:42:20PM +0800, Herbert Xu wrote:
> On Mon, Jun 10, 2024 at 08:48:11PM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Add an implementation of finup_mb to sha256-ni, using an interleaving
> > factor of 2.  It interleaves a finup operation for two equal-length
> > messages that share a common prefix.  dm-verity and fs-verity will take
> 
> I think the limitation on equal length is artificial.  There is
> no reason why the code couldn't handle two messages with different
> lengths.  Simply execute in dual mode up until the shorter message
> runs out.  Then carry on as if you have a single message.

Sure, as I mentioned the algorithm could fall back to single-buffer hashing once
the messages get out of sync.  This would actually have to be implemented and
tested, of course, which gets especially tricky with your proposal to support
arbitrary scatterlists.  And there's no actual use case for adding that
complexity yet.

> In fact, there is no reason why the two hashes have to start from
> the same initial state either.  It has no bearing on the performance
> of the actual hashing as far as I can see.

The SHA-256 inner loop would indeed be the same, but the single state has
several advantages:

- The caller only needs to allocate and prepare a single state.  This saves
  per-IO memory and reduces overhead.
- The glue code doesn't need to check that the number of internally buffered
  bytes are synced up.
- The assembly code only needs to load from the one state.

All of this simplifies the code slightly and boosts performance slightly.

These advantages aren't *too* large, of course, and if a use case for supporting
update arose, then support for multiple states would be added.  But it doesn't
make sense to add this functionality prematurely before it actually has a user.

- Eric