Re: [PATCH v12 2/3] x86/sev: Change snp_guest_issue_request's fw_err

From: Sean Christopherson <seanjc@google.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Dionna Amalie Glaze <dionnaglaze@google.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	Tom Lendacky <Thomas.Lendacky@amd.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Joerg Roedel <jroedel@suse.de>, Peter Gonda <pgonda@google.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Venu Busireddy <venu.busireddy@oracle.com>,
	Michael Roth <michael.roth@amd.com>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Michael Sterritt <sterritt@google.com>
Subject: Re: [PATCH v12 2/3] x86/sev: Change snp_guest_issue_request's fw_err
Date: Tue, 24 Jan 2023 16:35:37 +0000	[thread overview]
Message-ID: <Y9AI2eZCCSo3+hYz@google.com> (raw)
In-Reply-To: <Y8/iYyGHiPow20NX@zn.tnic>

On Tue, Jan 24, 2023, Borislav Petkov wrote:
> On Mon, Jan 23, 2023 at 01:22:07PM -0800, Dionna Amalie Glaze wrote:
> > This isn't the primary problem that needs fixing, although it is part
> > of it.
> 
> I'm replying to the 2/3 patch which is addressing this part.
> 
> > The problem is that the host can provide a throttling error and
> > the guest will need to continue trying the exact same request or else
> > end up locking themself out of the vmpck due to the IV reuse patch
> > Peter sent.
> > 
> > I think Sean's request to keep throttling a host problem in user space
> 
> + Sean.

It was not a request, it was a question[*].  In the same paragraph I even asked
if the guest was expected to schedule out the blocked task.

 : Why does throttling just punt back to the guest?  E.g. why not exit to userspace
 : and let userspace stall the vCPU?  Is the guest expected to schedule out the task
 : that's trying to make the request?

[*] https://lore.kernel.org/all/Y8rEFpbMV58yJIKy@google.com

> > is not the right one in this case. That would avoid scheduling the
> > whole vCPU, but the guest code I'm proposing can do other useful work
> > while waiting. There will be no other code that depends on that
> > particular control flow.

Relying on host behavior for this sort of thing is also a bad idea.   There is no
contract between the guest and host beyond the host being able to say "you've been
throttled".

Similar to Boris's comments about splitting this patch to fix bugs, separate the
handling of SNP_GUEST_VMM_ERR_BUSY from the internal ratelimiting.  For all intents
and purpsoses, gracefully handling SNP_GUEST_VMM_ERR_BUSY is mandatory.  Ratelimiting
requests from the guest side is a completely different story.