| Message ID | 20240501163400.15838-2-will@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Tweaks to the kvm-arm.mode= early cmdline parsing | expand |
On Wed, May 01, 2024 at 05:33:59PM +0100, Will Deacon wrote: > Booting a kernel with "arm64_sw.hvhe=1 kvm-arm.mode=nvhe" on the > command-line results in KVM initialising using hVHE, whereas one might > expect the latter option to override the former. > > Fix this by adding "arm64_sw.hvhe=0" to the alias expansion for > "kvm-arm.mode=nvhe". Hmm, I wonder if it'd be better to just evaluate the sanitised VH field in hvhe_possible(). Otherwise I worry about keeping aliases in sync when new command line options come along. This is similar to what we had before commit 35876f35f482 ("arm64: cpufeature: Add helper to test for CPU feature overrides") w/ the added use of the sanitised reg. Thoughts? diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 56583677c1f2..3bd5f00a8db3 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -2138,6 +2138,12 @@ static bool has_nested_virt_support(const struct arm64_cpu_capabilities *cap, static bool hvhe_possible(const struct arm64_cpu_capabilities *entry, int __unused) { + u64 val = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1); + + /* No VHE? Then no hVHE for you either. */ + if (!SYS_FIELD_GET(ID_AA64MMFR1_EL1, VH, val)) + return false; + return arm64_test_sw_feature_override(ARM64_SW_FEATURE_OVERRIDE_HVHE); }
Hey Oliver, On Wed, May 01, 2024 at 05:44:57PM +0000, Oliver Upton wrote: > On Wed, May 01, 2024 at 05:33:59PM +0100, Will Deacon wrote: > > Booting a kernel with "arm64_sw.hvhe=1 kvm-arm.mode=nvhe" on the > > command-line results in KVM initialising using hVHE, whereas one might > > expect the latter option to override the former. > > > > Fix this by adding "arm64_sw.hvhe=0" to the alias expansion for > > "kvm-arm.mode=nvhe". > > Hmm, I wonder if it'd be better to just evaluate the sanitised VH field > in hvhe_possible(). Otherwise I worry about keeping aliases in sync when > new command line options come along. > > This is similar to what we had before commit 35876f35f482 ("arm64: > cpufeature: Add helper to test for CPU feature overrides") w/ the added > use of the sanitised reg. > > Thoughts? I think that goes wonky when you have the arguments the other way around: "kvm-arm.mode=nvhe arm64_sw.hvhe=1" would end up using nVHE. Will
On Thu, May 02, 2024 at 11:20:30AM +0100, Will Deacon wrote: > Hey Oliver, > > On Wed, May 01, 2024 at 05:44:57PM +0000, Oliver Upton wrote: > > On Wed, May 01, 2024 at 05:33:59PM +0100, Will Deacon wrote: > > > Booting a kernel with "arm64_sw.hvhe=1 kvm-arm.mode=nvhe" on the > > > command-line results in KVM initialising using hVHE, whereas one might > > > expect the latter option to override the former. > > > > > > Fix this by adding "arm64_sw.hvhe=0" to the alias expansion for > > > "kvm-arm.mode=nvhe". > > > > Hmm, I wonder if it'd be better to just evaluate the sanitised VH field > > in hvhe_possible(). Otherwise I worry about keeping aliases in sync when > > new command line options come along. > > > > This is similar to what we had before commit 35876f35f482 ("arm64: > > cpufeature: Add helper to test for CPU feature overrides") w/ the added > > use of the sanitised reg. > > > > Thoughts? > > I think that goes wonky when you have the arguments the other way around: > > "kvm-arm.mode=nvhe arm64_sw.hvhe=1" > > would end up using nVHE. Right... What I was hoping to get at is a simple set of arguments to test protected + nVHE, even on systems that support VHE. Although I suppose: "kvm-arm.mode=protected id_aa64mmfr1.vh=0 arm64_sw.hvhe=0" isn't that bad and would preserve precedence of later args. So, FWIW: Acked-by: Oliver Upton <oliver.upton@linux.dev>
diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c index aad399796e81..39c9253fcf23 100644 --- a/arch/arm64/kernel/pi/idreg-override.c +++ b/arch/arm64/kernel/pi/idreg-override.c @@ -209,7 +209,7 @@ static const struct { char alias[FTR_ALIAS_NAME_LEN]; char feature[FTR_ALIAS_OPTION_LEN]; } aliases[] __initconst = { - { "kvm_arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, + { "kvm_arm.mode=nvhe", "arm64_sw.hvhe=0 id_aa64mmfr1.vh=0" }, { "kvm_arm.mode=protected", "id_aa64mmfr1.vh=0" }, { "arm64.nosve", "id_aa64pfr0.sve=0" }, { "arm64.nosme", "id_aa64pfr1.sme=0" },
Booting a kernel with "arm64_sw.hvhe=1 kvm-arm.mode=nvhe" on the command-line results in KVM initialising using hVHE, whereas one might expect the latter option to override the former. Fix this by adding "arm64_sw.hvhe=0" to the alias expansion for "kvm-arm.mode=nvhe". Signed-off-by: Will Deacon <will@kernel.org> --- arch/arm64/kernel/pi/idreg-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)