PING: [PATCH v2 3/3] virtio-crypto: implement RSA algorithm

[zhenwei pi]

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.

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.akcipher);
> >          else
> > crypto_register_skcipher(&virtio_crypto_skcipher_algs[i].algo.skcipher);
> >          ...
> >          }
> >      ...
> > }
> >
> > 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