Received-SPF: pass (google.com: domain of linux-kernel+bounces-154865-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 23 Apr 2024 12:28:29 +0200
From: Borislav Petkov <bp@alien8.de>
To: "Nikunj A. Dadhania" <nikunj@amd.com>
Cc: linux-kernel@vger.kernel.org, thomas.lendacky@amd.com, x86@kernel.org,
	kvm@vger.kernel.org, mingo@redhat.com, tglx@linutronix.de,
	dave.hansen@linux.intel.com, pgonda@google.com, seanjc@google.com,
	pbonzini@redhat.com
Subject: Re: [PATCH v8 06/16] virt: sev-guest: Move SNP Guest command mutex
Message-ID: <20240423102829.GCZieNTcHyuAYMcRf5@fat_crate.local>
References: <20240215113128.275608-1-nikunj@amd.com>
 <20240215113128.275608-7-nikunj@amd.com>
 <20240422130012.GAZiZfXM5Z2yRvw7Cx@fat_crate.local>
 <6a7a8892-bb8d-4f03-a802-d7eee48045b5@amd.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <6a7a8892-bb8d-4f03-a802-d7eee48045b5@amd.com>

On Tue, Apr 23, 2024 at 09:52:41AM +0530, Nikunj A. Dadhania wrote:
> SNP guest messaging will be moving as part of sev.c, and Secure TSC code
> will use this mutex.

No, this is all backwards.

You have a *static* function in sev-guest - snp_guest_ioctl- which takes
an exported lock - snp_guest_cmd_lock - in order to synchronize with
other callers which are only in that same sev-guest driver.

Why do you even need the guest messaging in sev.c?

I guess this: "Many of the required functions are implemented in the
sev-guest driver and therefore not available at early boot."

But then your API is misdesigned: the lock should be private to sev.c
and none of the callers should pay attention to grabbing it - the
callers simply call the functions and underneath the locking works
automatically for them - they don't care. Just like any other shared
resource, users see only the API they call and the actual
synchronization is done behind the scenes.

Sounds like you need to go back to the drawing board and think how this
thing should look like.

And when you have it, make sure to explain the commit messages *why* it
is done this way.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette