> -----Original Message-----
> From: zhenwei pi [mailto:pizhen...@bytedance.com]
> Sent: Tuesday, March 1, 2022 6:26 PM
> To: Gonglei (Arei) <arei.gong...@huawei.com>
> Cc: jasow...@redhat.com; m...@redhat.com;
> virtualization@lists.linux-foundation.org; linux-cry...@vger.kernel.org;
> linux-ker...@vger.kernel.org; helei.si...@bytedance.com;
> herb...@gondor.apana.org.au; kernel test robot <l...@intel.com>
> Subject: PING: [PATCH v2 3/3] virtio-crypto: implement RSA algorithm
>
> PING!
>
> Hi, Lei
> I also take a look at other crypto drivers qat/ccp/hisilicon, they separate
> akcipher/skcipher algo. If you consider that reusing
> virtio_crypto_algs_register/unregister seems better, I will try to merge them
> into a single function.
>
I'm fine with separating them in different c files. Then should we rename
virtio_crypto_algs.c
to virtio_crypto_skcipher_algo.c?
Regards,
-Gonglei
> On 2/23/22 6:17 PM, zhenwei pi wrote:
> >
> > On 2/18/22 11:12 AM, zhenwei pi wrote:
> >>>> +void virtio_crypto_akcipher_algs_unregister(struct virtio_crypto
> >>>> +*vcrypto) {
> >>>> + int i = 0;
> >>>> +
> >>>> + mutex_lock(&algs_lock);
> >>>> +
> >>>> + for (i = 0; i < ARRAY_SIZE(virtio_crypto_akcipher_algs); i++)
> >>>> +{
> >>>> + uint32_t service = virtio_crypto_akcipher_algs[i].service;
> >>>> + uint32_t algonum = virtio_crypto_akcipher_algs[i].algonum;
> >>>> +
> >>>> + if (virtio_crypto_akcipher_algs[i].active_devs == 0 ||
> >>>> + !virtcrypto_algo_is_supported(vcrypto, service,
> >>>> +algonum))
> >>>> + continue;
> >>>> +
> >>>> + if (virtio_crypto_akcipher_algs[i].active_devs == 1)
> >>>> +
> >>>>
> >>>> crypto_unregister_akcipher(&virtio_crypto_akcipher_algs[i].algo);
> >>>> +
> >>>> + virtio_crypto_akcipher_algs[i].active_devs--;
> >>>> + }
> >>>> +
> >>>> + mutex_unlock(&algs_lock);
> >>>> +}
> >>>
> >>> Why don't you reuse the virtio_crypto_algs_register/unregister
> >>> functions?
> >>> The current code is too repetitive. Maybe we don't need create the
> >>> new file virtio_crypto_akcipher_algo.c because we had
> >>> virtio_crypto_algs.c which includes all algorithms.
> >>>
> >>
> >> Yes, this looks similar to virtio_crypto_algs_register/unregister.
> >>
> >> Let's look at the difference:
> >> struct virtio_crypto_akcipher_algo {
> >> uint32_t algonum;
> >> uint32_t service;
> >> unsigned int active_devs;
> >> struct akcipher_alg algo;
> >> };
> >>
> >> struct virtio_crypto_algo {
> >> uint32_t algonum;
> >> uint32_t service;
> >> unsigned int active_devs;
> >> struct skcipher_alg algo; /* akcipher_alg VS skcipher_alg */
> >> };
> >>
> >> If reusing virtio_crypto_algs_register/unregister, we need to modify
> >> the data structure like this:
> >> struct virtio_crypto_akcipher_algo {
> >> uint32_t algonum;
> >> uint32_t service; /* use service to distinguish
> >> akcipher/skcipher */
> >> unsigned int active_devs;
> >> union {
> >> struct skcipher_alg skcipher;
> >> struct akcipher_alg akcipher;
> >> } alg;
> >> };
> >>
> >> int virtio_crypto_akcipher_algs_register(struct virtio_crypto
> >> *vcrypto) {
> >> ...
> >> for (i = 0; i < ARRAY_SIZE(virtio_crypto_akcipher_algs);
> >> i++) {
> >> uint32_t service =
> >> virtio_crypto_akcipher_algs[i].service;
> >> ...
> >> /* test service type then call
> >> crypto_register_akcipher/crypto_register_skcipher */
> >> if (service == VIRTIO_CRYPTO_SERVICE_AKCIPHER)
> >> crypto_register_akcipher(&virtio_crypto_akcipher_algs[i].algo.akciphe
> >> r);
> >> else
> >> crypto_register_skcipher(&virtio_crypto_skcipher_algs[i].algo.skciphe
> >> r);
> >> ...
> >> }
> >> ...
> >> }
> >>
> >> Also test service type and call
> >> crypto_unregister_skcipher/crypto_unregister_akcipher.
> >>
> >> This gets unclear from current v2 version.
> >>
> >> On the other hand, the kernel side prefers to separate skcipher and
> >> akcipher(separated header files and implementations).
> >>
> >
>
> --
> zhenwei pi
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
