On 10/9/24 04:28, Nikunj A Dadhania wrote:
> The SNP command mutex is used to serialize access to the shared buffer,
> command handling, and message sequence number.
>
> All shared buffer, command handling, and message sequence updates are done
> within snp_send_guest_request(), so moving the mutex to this function is
> appropriate and maintains the critical section.
>
> Since the mutex is now taken at a later point in time, remove the lockdep
> checks that occur before taking the mutex.
>
> Signed-off-by: Nikunj A Dadhania <nikunj@xxxxxxx>
Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> ---
> drivers/virt/coco/sev-guest/sev-guest.c | 35 ++++++-------------------
> 1 file changed, 8 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index 2a1b542168b1..1bddef822446 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -345,6 +345,14 @@ static int snp_send_guest_request(struct snp_guest_dev *snp_dev, struct snp_gues
> u64 seqno;
> int rc;
>
> + guard(mutex)(&snp_cmd_mutex);
> +
> + /* Check if the VMPCK is not empty */
> + if (is_vmpck_empty(snp_dev)) {
> + dev_err_ratelimited(snp_dev->dev, "VMPCK is disabled\n");
> + return -ENOTTY;
> + }
> +
> /* Get message sequence and verify that its a non-zero */
> seqno = snp_get_msg_seqno(snp_dev);
> if (!seqno)
> @@ -401,8 +409,6 @@ static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_io
> struct snp_guest_req req = {};
> int rc, resp_len;
>
> - lockdep_assert_held(&snp_cmd_mutex);
> -
> if (!arg->req_data || !arg->resp_data)
> return -EINVAL;
>
> @@ -449,8 +455,6 @@ static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_reque
> /* Response data is 64 bytes and max authsize for GCM is 16 bytes. */
> u8 buf[64 + 16];
>
> - lockdep_assert_held(&snp_cmd_mutex);
> -
> if (!arg->req_data || !arg->resp_data)
> return -EINVAL;
>
> @@ -501,8 +505,6 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
> int ret, npages = 0, resp_len;
> sockptr_t certs_address;
>
> - lockdep_assert_held(&snp_cmd_mutex);
> -
> if (sockptr_is_null(io->req_data) || sockptr_is_null(io->resp_data))
> return -EINVAL;
>
> @@ -598,15 +600,6 @@ static long snp_guest_ioctl(struct file *file, unsigned int ioctl, unsigned long
> if (!input.msg_version)
> return -EINVAL;
>
> - mutex_lock(&snp_cmd_mutex);
> -
> - /* Check if the VMPCK is not empty */
> - if (is_vmpck_empty(snp_dev)) {
> - dev_err_ratelimited(snp_dev->dev, "VMPCK is disabled\n");
> - mutex_unlock(&snp_cmd_mutex);
> - return -ENOTTY;
> - }
> -
> switch (ioctl) {
> case SNP_GET_REPORT:
> ret = get_report(snp_dev, &input);
> @@ -628,8 +621,6 @@ static long snp_guest_ioctl(struct file *file, unsigned int ioctl, unsigned long
> break;
> }
>
> - mutex_unlock(&snp_cmd_mutex);
> -
> if (input.exitinfo2 && copy_to_user(argp, &input, sizeof(input)))
> return -EFAULT;
>
> @@ -744,8 +735,6 @@ static int sev_svsm_report_new(struct tsm_report *report, void *data)
> man_len = SZ_4K;
> certs_len = SEV_FW_BLOB_MAX_SIZE;
>
> - guard(mutex)(&snp_cmd_mutex);
> -
> if (guid_is_null(&desc->service_guid)) {
> call_id = SVSM_ATTEST_CALL(SVSM_ATTEST_SERVICES);
> } else {
> @@ -880,14 +869,6 @@ static int sev_report_new(struct tsm_report *report, void *data)
> if (!buf)
> return -ENOMEM;
>
> - guard(mutex)(&snp_cmd_mutex);
> -
> - /* Check if the VMPCK is not empty */
> - if (is_vmpck_empty(snp_dev)) {
> - dev_err_ratelimited(snp_dev->dev, "VMPCK is disabled\n");
> - return -ENOTTY;
> - }
> -
> cert_table = buf + report_size;
> struct snp_ext_report_req ext_req = {
> .data = { .vmpl = desc->privlevel },
