On Tue, 31 Jan 2023 17:34:39 +0000, Suzuki K Poulose <suzuki.poulose@xxxxxxx> 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@xxxxxxx> wrote: > >> > >> Hi Marc > >> > >> On 31/01/2023 09:23, Marc Zyngier wrote: > >>> From: Jintack Lim <jintack.lim@xxxxxxxxxx> > >>> > >>> 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@xxxxxxxxxxxxxxx> > >>> Signed-off-by: Jintack Lim <jintack.lim@xxxxxxxxxx> > >>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx> > >>> Signed-off-by: Christoffer Dall <christoffer.dall@xxxxxxx> > >>> [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.
