On 11/19/2019 7:55 PM, Horia Geanta wrote: > On 11/18/2019 12:31 AM, Iuliana Prodan wrote: >> Added a new struct - caam_jr_request_entry, to keep each request >> information. This has a crypto_async_request, used to determine >> the request type, and a bool to check if the request has backlog >> flag or not. >> This struct is passed to CAAM, via enqueue function - caam_jr_enqueue. >> >> The new added caam_jr_enqueue_no_bklog function is used to enqueue a job >> descriptor head for cases like caamrng, key_gen, digest_key, where we > Enqueuing terminology: either generic "job" or more HW-specific > "job descriptor". > Job descriptor *head* has no meaning. > >> don't have backlogged requests. >> > ...because the "requests" are not crypto requests - they are either coming > from hwrng (caamrng's case) or are driver-internal (key_gen, digest_key - > used for key hashing / derivation during .setkey callback). > Right, I'll update the patch description in v2. >> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c >> index 21b6172..abebcfc 100644 >> --- a/drivers/crypto/caam/caamalg.c >> +++ b/drivers/crypto/caam/caamalg.c > [...] >> @@ -1416,7 +1424,7 @@ static inline int chachapoly_crypt(struct aead_request *req, bool encrypt) >> DUMP_PREFIX_ADDRESS, 16, 4, desc, desc_bytes(desc), >> 1); >> >> - ret = caam_jr_enqueue(jrdev, desc, aead_crypt_done, req); >> + ret = caam_jr_enqueue(jrdev, desc, aead_crypt_done, &edesc->jrentry); >> if (ret != -EINPROGRESS) { >> aead_unmap(jrdev, edesc, req); >> kfree(edesc); >> @@ -1440,6 +1448,7 @@ static inline int aead_crypt(struct aead_request *req, bool encrypt) >> struct aead_edesc *edesc; >> struct crypto_aead *aead = crypto_aead_reqtfm(req); >> struct caam_ctx *ctx = crypto_aead_ctx(aead); >> + struct caam_jr_request_entry *jrentry; >> struct device *jrdev = ctx->jrdev; >> bool all_contig; >> u32 *desc; >> @@ -1459,7 +1468,9 @@ static inline int aead_crypt(struct aead_request *req, bool encrypt) >> desc_bytes(edesc->hw_desc), 1); >> >> desc = edesc->hw_desc; >> - ret = caam_jr_enqueue(jrdev, desc, aead_crypt_done, req); >> + jrentry = &edesc->jrentry; >> + >> + ret = caam_jr_enqueue(jrdev, desc, aead_crypt_done, jrentry); > Let's avoid adding a new local variable by using &edesc->jrentry directly, > like in chachapoly_crypt(). > Similar for the other places. > I've removed jrentry and req (as mentioned below) in patch #11 of this series, but I'll remove them, from this patch, in v2. >> diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c >> index baf4ab1..d9de3dc 100644 >> --- a/drivers/crypto/caam/caamhash.c >> +++ b/drivers/crypto/caam/caamhash.c > [...] >> @@ -933,11 +943,13 @@ static int ahash_final_ctx(struct ahash_request *req) >> DUMP_PREFIX_ADDRESS, 16, 4, desc, desc_bytes(desc), >> 1); >> >> - ret = caam_jr_enqueue(jrdev, desc, ahash_done_ctx_src, req); >> + jrentry = &edesc->jrentry; >> + >> + ret = caam_jr_enqueue(jrdev, desc, ahash_done_ctx_src, jrentry); >> if (ret == -EINPROGRESS) >> return ret; >> >> - unmap_ctx: >> +unmap_ctx: > That's correct, however whitespace fixing should be done separately. > Should I make a separate patch for these two whitespaces? >> @@ -1009,11 +1022,13 @@ static int ahash_finup_ctx(struct ahash_request *req) >> DUMP_PREFIX_ADDRESS, 16, 4, desc, desc_bytes(desc), >> 1); >> >> - ret = caam_jr_enqueue(jrdev, desc, ahash_done_ctx_src, req); >> + jrentry = &edesc->jrentry; >> + >> + ret = caam_jr_enqueue(jrdev, desc, ahash_done_ctx_src, jrentry); >> if (ret == -EINPROGRESS) >> return ret; >> >> - unmap_ctx: >> +unmap_ctx: > Again, unrelated whitespace fix. > >> diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c >> index 7f7ea32..bb0e4b9 100644 >> --- a/drivers/crypto/caam/caampkc.c >> +++ b/drivers/crypto/caam/caampkc.c > [...] >> @@ -315,6 +317,8 @@ static struct rsa_edesc *rsa_edesc_alloc(struct akcipher_request *req, >> edesc->mapped_src_nents = mapped_src_nents; >> edesc->mapped_dst_nents = mapped_dst_nents; >> >> + edesc->jrentry.base = &req->base; >> + >> edesc->sec4_sg_dma = dma_map_single(dev, edesc->sec4_sg, >> sec4_sg_bytes, DMA_TO_DEVICE); >> if (dma_mapping_error(dev, edesc->sec4_sg_dma)) { > [...] >> @@ -633,7 +638,10 @@ static int caam_rsa_enc(struct akcipher_request *req) >> /* Initialize Job Descriptor */ >> init_rsa_pub_desc(edesc->hw_desc, &edesc->pdb.pub); >> >> - ret = caam_jr_enqueue(jrdev, edesc->hw_desc, rsa_pub_done, req); >> + jrentry = &edesc->jrentry; >> + jrentry->base = &req->base; > This field is already set in rsa_edesc_alloc(). > >> @@ -666,7 +675,10 @@ static int caam_rsa_dec_priv_f1(struct akcipher_request *req) >> /* Initialize Job Descriptor */ >> init_rsa_priv_f1_desc(edesc->hw_desc, &edesc->pdb.priv_f1); >> >> - ret = caam_jr_enqueue(jrdev, edesc->hw_desc, rsa_priv_f_done, req); >> + jrentry = &edesc->jrentry; >> + jrentry->base = &req->base; > The same here. > >> @@ -699,7 +712,10 @@ static int caam_rsa_dec_priv_f2(struct akcipher_request *req) >> /* Initialize Job Descriptor */ >> init_rsa_priv_f2_desc(edesc->hw_desc, &edesc->pdb.priv_f2); >> >> - ret = caam_jr_enqueue(jrdev, edesc->hw_desc, rsa_priv_f_done, req); >> + jrentry = &edesc->jrentry; >> + jrentry->base = &req->base; > And here. > >> @@ -732,7 +749,10 @@ static int caam_rsa_dec_priv_f3(struct akcipher_request *req) >> /* Initialize Job Descriptor */ >> init_rsa_priv_f3_desc(edesc->hw_desc, &edesc->pdb.priv_f3); >> >> - ret = caam_jr_enqueue(jrdev, edesc->hw_desc, rsa_priv_f_done, req); >> + jrentry = &edesc->jrentry; >> + jrentry->base = &req->base; > Also here. > >> diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h >> index c7c10c9..58be66c 100644 >> --- a/drivers/crypto/caam/intern.h >> +++ b/drivers/crypto/caam/intern.h > [...] >> @@ -104,6 +105,15 @@ struct caam_drv_private { >> #endif >> }; >> >> +/* >> + * Storage for tracking each request that is processed by a ring >> + */ >> +struct caam_jr_request_entry { >> + /* Common attributes for async crypto requests */ >> + struct crypto_async_request *base; >> + bool bklog; /* Stored to determine if the request needs backlog */ >> +}; >> + > Could we use kernel-doc here? Sure, will do in v2. Thanks, Iulia > > Horia >