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, 8 Dec 2020 19:14:47 +0000
From:   David Brazdil <dbrazdil@google.com>
To:     Marc Zyngier <maz@kernel.org>
Cc:     kvmarm@lists.cs.columbia.edu,
        Catalin Marinas <catalin.marinas@arm.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        linux-kernel@vger.kernel.org, Will Deacon <will@kernel.org>,
        Tejun Heo <tj@kernel.org>, Dennis Zhou <dennis@kernel.org>,
        linux-doc@vger.kernel.org, Jonathan Corbet <corbet@lwn.net>,
        kernel-team@android.com, linux-arm-kernel@lists.infradead.org,
        Christoph Lameter <cl@linux.com>,
        Sudeep Holla <sudeep.holla@arm.com>
Subject: Re: [PATCH v4 00/26] Opt-in always-on nVHE hypervisor
Message-ID: <20201208191447.47idqf7n2v3hvrdg@google.com>
References: <20201202184122.26046-1-dbrazdil@google.com>
 <160702322202.1501317.9696987088711766533.b4-ty@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <160702322202.1501317.9696987088711766533.b4-ty@kernel.org>
Precedence: bulk

Hey Marc,

On Thu, Dec 03, 2020 at 07:23:19PM +0000, Marc Zyngier wrote:
> On Wed, 2 Dec 2020 18:40:56 +0000, David Brazdil wrote:
> > As we progress towards being able to keep guest state private to the
> > host running nVHE hypervisor, this series allows the hypervisor to
> > install itself on newly booted CPUs before the host is allowed to run
> > on them.
> > 
> > All functionality described below is opt-in, guarded by an early param
> > 'kvm-arm.mode=protected'. Future patches specific to the new protected
> > mode should be hidden behind the same param.
> > 
> > [...]
> 
> Applied to kvm-arm64/psci-relay, thanks!
> 
> Note that although I pushed it to -next, I still need people to
> eyeball it and give it some Acks. The commit-IDs below will
> thus change as I apply tags, if any.
> 

I'm looking at -next and I think the merge with Mark Rutland's el2_setup
refactor didn't go as planned.

The `#ifdef CONFIG_ARM64_VHE` section needs to cover everything between
init_el2 and init_el2_nvhe. Currently the code falls through into VHE init
when CONFIG_ARM64_VHE is not set.

Here's the snippet:

SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
#ifdef CONFIG_ARM64_VHE
	/*
	 * Check for VHE being present. x2 being non-zero indicates that we
	 * do have VHE, and that the kernel is intended to run at EL2.
	 */
	mrs	x2, id_aa64mmfr1_el1
	ubfx	x2, x2, #ID_AA64MMFR1_VHE_SHIFT, #4
	cbz	x2, init_el2_nvhe
#endif						// <--- THIS

	<... initialize VHE ...>
	msr	elr_el2, lr
	mov	w0, #BOOT_CPU_MODE_EL2
	eret
						// <--- NEEDS TO MOVE HERE
SYM_INNER_LABEL(init_el2_nvhe, SYM_L_LOCAL)
	<... initialize nVHE ...>

-David