On Fri, Jun 21, 2024 at 06:08:47PM +0530, Nikunj A Dadhania wrote:
> SNP command mutex is used to serialize access to the shared buffer, command
> handling and message sequence number races.
serialize access to ... races?
Needs re-formulation.
> As part of the preparation for moving SEV guest driver common code and
> making mutex private, take the mutex in snp_send_guest_request() instead of
> snp_guest_ioctl(). This will result in locking behavior change as detailed
> below:
>
> Current locking behaviour:
>
> snp_guest_ioctl()
> mutex_lock(&snp_cmd_mutex)
> get_report()/get_derived_key()/get_ext_report()
> snp_send_guest_request()
> ...
> mutex_unlock(&snp_cmd_mutex)
>
> New locking behaviour:
>
> snp_guest_ioctl()
> get_report()/get_derived_key()/get_ext_report()
> snp_send_guest_request()
> guard(mutex)(&snp_cmd_mutex)
> ...
Why is it ok to grab the mutex in snp_send_guest_request()?
Folks need to learn to stop spelling out what the patch does but WHY it
does it and WHY is it ok?!?
> Remove multiple lockdep check in the sev-guest driver as they are redundant
> now.
More "what" redundancy.
"The new locking region covers <bla> and that is ok because of <foo>."
This is what your commit message should say.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
