Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 24 May 2021 17:03:29 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Paolo Bonzini <pbonzini@redhat.com>
Cc:     Tom Lendacky <thomas.lendacky@amd.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        Wanpeng Li <wanpengli@tencent.com>,
        Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Brijesh Singh <brijesh.singh@amd.com>,
        Ashish Kalra <Ashish.Kalra@amd.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, x86@kernel.org
Subject: Re: [PATCH] KVM: SVM: Assume a 64-bit hypercall for guests with
 protected state
Message-ID: <YKvcYeXaxTQ//87M@google.com>
References: <d0904f0d049300267665bd4abf96c3d7e7aa4825.1621701837.git.thomas.lendacky@amd.com>
 <87pmxg73h7.fsf@vitty.brq.redhat.com>
 <a947ee05-4205-fb3d-a1e6-f5df7275014e@amd.com>
 <87tums8cn0.fsf@vitty.brq.redhat.com>
 <211d5285-e209-b9ef-3099-8da646051661@amd.com>
 <c6864982-b30a-29b5-9a10-3cfdd331057e@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c6864982-b30a-29b5-9a10-3cfdd331057e@redhat.com>
Precedence: bulk

On Mon, May 24, 2021, Paolo Bonzini wrote:
> On 24/05/21 15:58, Tom Lendacky wrote:
> > > Would it hurt if we just move 'vcpu->arch.guest_state_protected' check
> > > to is_64_bit_mode() itself? It seems to be too easy to miss this
> > > peculiar detail about SEV in review if new is_64_bit_mode() users are to
> > > be added.
> > I thought about that, but wondered if is_64_bit_mode() was to be used in
> > other places in the future, if it would be a concern. I think it would be
> > safe since anyone adding it to a new section of code is likely to look at
> > what that function is doing first.
> > 
> > I'm ok with this. Paolo, I know you already queued this, but would you
> > prefer moving the check into is_64_bit_mode()?
> 
> Let's introduce a new wrapper is_64_bit_hypercall, and add a
> WARN_ON_ONCE(vcpu->arch.guest_state_protected) to is_64_bit_mode.

Can we introduce the WARN(s) in a separate patch, and deploy them much more
widely than just is_64_bit_mode()?  I would like to have them lying in wait at
every path that should be unreachable, e.g. get/set segments, get_cpl(), etc...

Side topic, kvm_get_cs_db_l_bits() should be moved to svm.c.  Functionally, it's
fine to have it as a vendor-agnostic helper, but practically speaking it should
never be called directly.