On Fri, Mar 13, 2020 at 05:05:35PM +0000, Barani Muthukumaran wrote:
> Hi Eric,
>
> The crypto_variant_ops exposed were not a guess, we had worked with Satya to
> add the functionality that is required.
As far as I can tell, my patch works fine with just the new ->program_key()
variant op.
Note that I'm also taking advantage of some of the existing, non-crypto-specific
variant ops. For example, I didn't need to add a ->crypto_resume() operation
because there's already ufs_hba_variant_ops::resume().
I understand that the old downstream ICE code defined and used a lot of crypto
variant ops, which did give the impression that a lot more would be needed. But
ultimately I found that most of them were unnecessary or could be replaced with
the existing non-crypto-specific variant ops.
> Can we define the below crypto_variant_ops that exposes the same functionality
> you have added, this allows us to extend this in the future in a seamless
> manner. As an example QC implementation uses 'debug', 'suspend' and we can add
> that when we upstream the implementation in the next few weeks once our
> validation is complete. Thanks.
>
> struct ufs_hba_crypto_variant_ops {
> int (*hba_init_crypto)(struct ufs_hba *hba,
> const struct keyslot_mgmt_ll_ops *ksm_ops);
> void (*enable)(struct ufs_hba *hba);
> int (*resume)(struct ufs_hba *hba);
> int (*program_key(struct ufs_hba *hba,
> const union ufs_crypto_cfg_entry *cfg, int slot);
> };
To re-iterate, upstream doesn't add hooks with no in-tree users.
It's great to hear that you'll be sending out a patchset soon. Just send out
the hooks you need along with them, so that they can all be properly reviewed.
- Eric
