On Wed, Jun 17, 2015 at 08:02:30PM +0300, Horia Geantă wrote:
> >
> > -#define DESC_MAX_USED_BYTES (DESC_RFC4543_GIVENC_LEN + \
> > - CAAM_MAX_KEY_SIZE)
> > -#define DESC_MAX_USED_LEN (DESC_MAX_USED_BYTES / CAAM_CMD_SZ)
> > +#define DESC_MAX_USED_LEN (CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN)
>
> This is going to increase the size of caam_ctx struct, but I agree
> previous approach was error-prone.
The problem with the previous code is that it doesn't take into
account the size of the inline key should the key fit into the
64 entries.
However, it appears that I incorrectly removed DESC_MAX_USED_BYTES
and thus made it 4 times bigger than necessary. I'll fix that up.
> > + /* skip assoc data */
> > + append_seq_fifo_store(desc, 0, FIFOST_TYPE_SKIP | FIFOLDST_VLF);
>
> This wasn't previously needed. I assume it's related to your comment:
> "This series converts various GCM implementations to the new AEAD
> interface. The main changes [...] both src/dst now contain space at the
> head equal to assoclen, but only src has the actual AD."
Right. The new interface always includes assoclen bytes in both
src and dst SG lists. req->assoc is gone.
> > +static void init_gcm_job(struct aead_request *req,
> > + struct aead_edesc *edesc,
> > + bool all_contig, bool encrypt)
> > +{
> > + struct crypto_aead *aead = crypto_aead_reqtfm(req);
> > + struct caam_ctx *ctx = crypto_aead_ctx(aead);
> > + unsigned int ivsize = crypto_aead_ivsize(aead);
> > + u32 *desc = edesc->hw_desc;
> > + bool generic_gcm = (ivsize == 12);
> > + unsigned int last;
> > +
> > + init_aead_job(req, edesc, all_contig, encrypt);
> > +
> > + /* BUG This should not be specific to generic GCM. */
>
> AFAICT, for non-generic GCM uses (RFC4106, RFC4543), cryptlen and/or
> assoclen are always > 0. That's why the descriptors do not address these
> cases.
Of course. But with the algif_aead interface you need to at least
ensure that you don't crash or do something silly should the user
give you such an input. So my question is what happens when it is
zero? Does the hardware simply emit an error and recover, or does it
hang/lock up/do something worse?
> Now that GCM is handled separately, is_gcm logic should be removed from
> all old_aead_* functions.
I haven't touched the old_aead_* path at all because there are more
conversions to come. Once it's all done we can kill all of the
old_aead_* functions.
> > - sg_to_sec4_sg_last(req->src,
> > - src_nents,
> > - edesc->sec4_sg +
> > - sec4_sg_index, 0);
> > + sg_to_sec4_sg(req->src, src_nents,
> > + edesc->sec4_sg + sec4_sg_index, 0);
>
> Need to mark end of input S/G, use sg_to_sec4_sg_last() instead.
Thanks I'll fix that up too.
BTW does this actually work on your hardware now?
Cheers,
--
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
