Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 27 Oct 2022 17:59:42 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Peter Gonda <pgonda@google.com>
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        marcorr@google.com, michael.roth@amd.com, thomas.lendacky@amd.com,
        joro@8bytes.org, mizhang@google.com, pbonzini@redhat.com,
        andrew.jones@linux.dev
Subject: Re: [V4 6/8] KVM: selftests: add library for creating/interacting
 with SEV guests
Message-ID: <Y1rHDlDskvSacLNp@google.com>
References: <20220829171021.701198-1-pgonda@google.com>
 <20220829171021.701198-7-pgonda@google.com>
 <Yz8dpB5+RFjEhA3n@google.com>
 <CAMkAt6oZQc4jqF7FOXOKkpbP3c4NXxPumVVjX9gXwPCh-zbtYg@mail.gmail.com>
 <Y02ZLFcDQbX6lP9z@google.com>
 <CAMkAt6q0g5Ua=PwLXa2oA4zCQUaHuEQ3pTXycD61HU6-dtQ5Gg@mail.gmail.com>
 <Y028WrU3pmEQqWDq@google.com>
 <CAMkAt6pvT15teuYWjz7r1vmUP5McDp76qjxQ26_oeg5mTnv5NA@mail.gmail.com>
 <Y1AnHwVtOFShRxQD@google.com>
 <CAMkAt6rP7KbgUqmK+aiooSLfvRrMsRmp99cL0YWKBwpOJZc82A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAMkAt6rP7KbgUqmK+aiooSLfvRrMsRmp99cL0YWKBwpOJZc82A@mail.gmail.com>
Precedence: bulk

On Thu, Oct 27, 2022, Peter Gonda wrote:
> On Wed, Oct 19, 2022 at 10:34 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Tue, Oct 18, 2022, Peter Gonda wrote:
> > > On Mon, Oct 17, 2022 at 2:34 PM Sean Christopherson <seanjc@google.com> wrote:
> > > >
> > > > On Mon, Oct 17, 2022, Peter Gonda wrote:
> > > > > I think this means we don't need to add VM_MODE_PXXV48_4K_SEV since we
> > > > > can set up the c-bit from inside of vm_sev_create_*(), thoughts?
> > > >
> > > > Configuring the C-bit inside vm_sev_create_*() won't work (at least not well).
> > > > The C-bit needs to be known before kvm_vm_elf_load(), i.e. can't be handled after
> > > > __vm_create(), and needs to be tracked inside the VM, i.e. can't be handled before
> > > > __vm_create().
> > > >
> > > > The proposed kvm_init_vm_address_properties() seems like the best fit since the
> > > > C-bit (and TDX's S-bit) is stolen from GPA space, i.e. directly affects the other
> > > > values computed in that path.
> > > >
> > > > As for the kvm_vm_arch allocation ugliness, when we talked off-list I didn't
> > > > consider the need to allocate in kvm_init_vm_address_properties().  That's quite
> > > > gross, especially since the pointer will be larger than the thing being allocated.
> > > >
> > > > With that in mind, adding .../include/<arch>/kvm_util.h so that "struct kvm_vm_arch"
> > > > can be defined and referenced directly doesn't seem so bad.  Having to stub in the
> > > > struct for the other architectures is annoying, but not the end of the world.
> > >
> > > I'll make "struct kvm_vm_arch" a non pointer member, so adding
> > > /include/<arch>/kvm_util.h files.
> > >
> > > But I think we do not need VM_MODE_PXXV48_4K_SEV, see:
> >
> > I really don't want to open code __vm_create() with a slight tweak.  E.g. the
> > below code will be broken by Ricardo's series to add memslot0 is moved out of
> > ____vm_create()[1], and kinda sorta be broken again by Vishal's series to add an
> > arch hook to __vm_create()[2].
> >
> > AFAICT, there is no requirement that KVM_SEV_INIT be called before computing the
> > C-Bit, the only requirement is that KVM_SEV_INIT is called before adding vCPUs.
> >
> > [1] https://lore.kernel.org/all/20221017195834.2295901-8-ricarkol@google.com
> > [2] https://lore.kernel.org/all/YzsC4ibDqGh5qaP9@google.com
> 
> Oh I misunderstood your suggestion above.
> 
> I should make KVM_SEV_INIT happen from kvm_arch_vm_post_create().  Add
> VM_MODE_PXXV48_4K_SEV for c-bit setting inside of
> kvm_init_vm_address_properties().
> 
> Inside of vm_sev_create_with_one_vcpu() I use
> __vm_create_with_vcpus(), then call KVM_SEV_LAUNCH_FINISH.
> 
> Is that correct?

Yep, I'm pretty sure that was what I was thinking.