Re: [PATCH] ufs: crypto: add host_sem lock in ufshcd_program_key

[ZhangHui]

On Thu, Mar 20, 2025 at 09:44:55PM -0700, Eric Biggers wrote:
> It seems broken that the filesystem doesn't get unmounted until after the UFS is
> shut down.  It would be helpful to get a clearer picture of exactly why things
> are happening in that order.
> 
> But disregarding that, it's indeed logical for blk_crypto_evict_key() to return
> an error if it cannot fulfill the request.
> 
> But I'm wondering if this needs to be solved in the UFS driver itself or whether
> the blk-crypto framework should handle this (so that it also gets fixed for
> other drivers that may have the same problem).  In block/blk-crypto-profile.c,
> pm_runtime_get_sync() is already called before ->keyslot_evict.  So
> ->keyslot_evict is supposed to be called only when the device is resumed.
> 
> The blk-crypto code (in blk_crypto_hw_enter()) doesn't check the return value of
> pm_runtime_get_sync(), though.  That seems like a bug.  Is it possible this
> issue would be fixed if it checked the return value?
> 

hi Eric,

I have checked the device_shutdown process and it seems only wait for the resume
that has not been processed to be completed, and then continue. It does not seem
to cause pm_runtime_get_sync to return an error.

> Or does the UFS driver still need to check ufshcd_is_user_access_allowed() too?
> If that's the case, I'm also wondering whether it's okay to nest host_sem inside
> pm_runtime_get_sync().  Elsewhere in the UFS driver they are called in the
> opposite order.

I found that ufshcd_is_user_access_allowed is used in many places in the ufs driver
code. What is the historical reason for this?

thanks
zhanghui