Re: [PATCH v8 01/69] arm64: Add ARM64_HAS_NESTED_VIRT cpufeature

[Marc Zyngier <maz@kernel.org>]

On Tue, 31 Jan 2023 17:34:39 +0000,
Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> 
> Hi Marc,
> 
> On 31/01/2023 14:00, Marc Zyngier wrote:
> > Hi Suzuki,
> > 
> > On Tue, 31 Jan 2023 13:47:31 +0000,
> > Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> >> 
> >> Hi Marc
> >> 
> >> On 31/01/2023 09:23, Marc Zyngier wrote:
> >>> From: Jintack Lim <jintack.lim@linaro.org>
> >>> 
> >>> Add a new ARM64_HAS_NESTED_VIRT feature to indicate that the
> >>> CPU has the ARMv8.3 nested virtualization capability, together
> >>> with the 'kvm-arm.mode=nested' command line option.
> >>> 
> >>> This will be used to support nested virtualization in KVM.
> >>> 
> >>> Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> >>> Signed-off-by: Jintack Lim <jintack.lim@linaro.org>
> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >>> Signed-off-by: Christoffer Dall <christoffer.dall@arm.com>
> >>> [maz: moved the command-line option to kvm-arm.mode]
> >> 
> >> Should this be separate kvm-arm mode ? Or can this be tied to
> >> is_kernel_in_hyp_mode() ? Given this mode (from my limited
> >> review) doesn't conflict with normal VHE mode (and RME support),
> >> adding this explicit mode could confuse the user.
> > 
> > What is exactly the objection here? NV is more or less a VHE++ mode,
> > but is also completely experimental and incomplete.
> 
> I am all in for making this an "optional", only enabled it when "I know
> what I want".
> 
> kvm-arm.mode=nv kind of seems that the KVM driver is conditioned
> mainly for running NV (comparing with the other existing options
> for kvm-arm.mode).
> 
> In reality, as you confirmed, NV is an *additional* capability
> of a VHE hypervisor. So it would be good to "opt" in for "nv" capability
> support.
> 
> e.g,
> 
>    kvm-arm.nv=on
> 
> Thinking more about it, either is fine.

We had something like this for a long while, but it gave the bad
impression what NV and all the other options were orthogonal. But they
aren't, really. NV+nVHE is not a thing (at least, as far as I am
concerned), and I'm not even mentioning NV+protected.

The same reasoning applies to 'protected'. We don't have

	kvm-arm.protected=on

because it only makes sense with nVHE, so we have

	kvm-arm.mode=protected

which is nVHE + weird stuff, just like NV is VHE + even weirder stuff.

> >> In case we need a command line to turn the NV mode on/off,
> >> we could always use the id-override and simply leave this out ?
> > 
> > I really want an explicit user buy-in. There is absolutely no way this
> > can be enabled by default, the risks are way too high. Just look at
> > the x86 story: it took them 10 years to enable NV by default. I don't
> > expect to do any better.
> 
> Of course, I am with you on that. I realise that we may not be able
> to disable nv by default using idreg-override (i.e., without any kernel
> command line option). So we may need something outside of that, like
> the option mentioned above.

The override would indeed need the user to *add* something to disable
NV, and we want the opposite. If we really want to allow support for
something like NV + protected (to mention the worse possible case),
I'd rather we have something like:

	kvm-arm.mode=protected,nested

which would describe the expected behaviour in a compact way, without
adding a ton of extra options.

But to be honest, I'm not expecting to support any of that within the
foreseeable future!

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel