Received-SPF: pass (google.com: domain of linux-kernel+bounces-85444-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240227232100.478238-1-pbonzini@redhat.com> <20240227232100.478238-11-pbonzini@redhat.com>
 <Zd6T06Qghvutp8Qw@google.com>
In-Reply-To: <Zd6T06Qghvutp8Qw@google.com>
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Wed, 28 Feb 2024 18:32:37 +0100
Message-ID: <CABgObfaPodSSzArO99Hkn=vpGotO1wZ-0dZKEZHx9EqLZ7M_XQ@mail.gmail.com>
Subject: Re: [PATCH 10/21] KVM: SEV: Use a VMSA physical address variable for
 populating VMCB
To: Sean Christopherson <seanjc@google.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, michael.roth@amd.com, 
	isaku.yamahata@intel.com, thomas.lendacky@amd.com, 
	Ashish Kalra <ashish.kalra@amd.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 28, 2024 at 3:00=E2=80=AFAM Sean Christopherson <seanjc@google.=
com> wrote:
>
> On Tue, Feb 27, 2024, Paolo Bonzini wrote:
> > From: Tom Lendacky <thomas.lendacky@amd.com>
> >
> > In preparation to support SEV-SNP AP Creation, use a variable that hold=
s
> > the VMSA physical address rather than converting the virtual address.
> > This will allow SEV-SNP AP Creation to set the new physical address tha=
t
> > will be used should the vCPU reset path be taken.
>
> No, this patch belongs in the SNP series.  The hanlding of vmsa_pa is bro=
ken
> (KVM leaks the page set by the guest; I need to follow-up in the SNP seri=
es).
> On top of that, I detest duplicat variables, and I don't like that KVM ke=
eps its
> original VMSA (kernel allocation) after the guest creates its own.
>
> I can't possibly imagine why this needs to be pulled in early.  There's n=
o way
> TDX needs this, and while this patch is _small_, the functional change it=
 leads
> to is not.

Well, the point of this series (and there will be more if you agree)
is exactly to ask "why not" in a way that is more manageable than
through the huge TDX and SNP series. My reading of the above is that
you believe this is small enough that it can even be merged with "KVM:
SEV: Support SEV-SNP AP Creation NAE event" (with fixes), which I
don't disagree with.

Otherwise, if the approach was good there's no reason _not_ to get it
in early. It's just a refactoring.

Talking in general: I think I agree about keeping the gmem parts in a
kvm-coco-queue branch (and in the meanwhile involving the mm people if
mm/filemap.c changes are needed). #VE too, probably, but what I
_really_ want to avoid is that these series (the plural is not a typo)
become a new bottleneck for everybody. Basically these are meant to be
a "these seem good to go to me, please confirm or deny" between
comaintainers more than a real patch posting; having an extra branch
is extra protection against screwups but we should be mindful that
force pushes are painful for everyone.

If you think I'm misguided, please do speak out or feel free to ask me
to talk on voice.

Paolo