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:   Tue, 21 Sep 2021 13:50:09 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Ashish Kalra <ashish.kalra@amd.com>
Cc:     Steve Rutherford <srutherford@google.com>, pbonzini@redhat.com,
        tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
        joro@8bytes.org, bp@alien8.de, thomas.lendacky@amd.com,
        x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        brijesh.singh@amd.com, dovmurik@linux.ibm.com, tobin@linux.ibm.com,
        jejb@linux.ibm.com, dgilbert@redhat.com
Subject: Re: [PATCH v6 1/5] x86/kvm: Add AMD SEV specific Hypercall3
Message-ID: <YUnjEU+1icuihmbR@google.com>
References: <cover.1629726117.git.ashish.kalra@amd.com>
 <6fd25c749205dd0b1eb492c60d41b124760cc6ae.1629726117.git.ashish.kalra@amd.com>
 <CABayD+fnZ+Ho4qoUjB6YfWW+tFGUuftpsVBF3d=-kcU0-CEu0g@mail.gmail.com>
 <YUixqL+SRVaVNF07@google.com>
 <20210921095838.GA17357@ashkalra_ubuntu_server>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210921095838.GA17357@ashkalra_ubuntu_server>
Precedence: bulk

On Tue, Sep 21, 2021, Ashish Kalra wrote:
> This is simply a Hack, i don't think this is a good approach to take forward.

But a clever hack ;-)

> > Unless there's some fundamental technical hurdle I'm overlooking, if pv_ops can
> > be configured early enough to handle this, then so can alternatives.  
> 
> Now, as i mentioned earlier, apply_alternatives() is only called boot
> CPU identification has been done which is a lot of support code which
> may be dependent on earlier setup_arch() code and then it does CPU
> mitigtion selections before patching alternatives, again which may have
> dependencies on previous code paths in setup_arch(), so i am not sure if
> we can call apply_alternatives() earlier. 

apply_alternatives() is a generic helper that can work on any struct alt_instr
array, e.g. KVM_HYPERCALL can put its alternative into a different section that's
patched as soon as the VMM is identified.

> Maybe for a guest kernel and virtualized boot enviroment, CPU
> identification may not be as complicated as for a physical host, but
> still it may have dependencies on earlier architecture specific boot
> code.
> 
> > Adding notify_page_enc_status_changed() may be necessary in the future, e.g. for TDX
> > or SNP, but IMO that is orthogonal to adding a generic, 100% redundant helper.
> 
> If we have to do this in the future and as Sean mentioned ealier that
> vmcall needs to be fixed for TDX (as it will cause a #VE), then why not
> add this abstraction right now ?

I'm not objecting to adding a PV op, I'm objecting to kvm_sev_hypercall3().  If
others disagree and feel it's the way forward, I certainly won't stand in the way,
but IMO it's unnecessary code duplication.