Re: [libvirt] [PATCH v3 2/6] libxl: do not enable nested HVM by mere presence of <cpu> element

On Tue, Dec 19, 2017 at 01:45:57PM +0000, Daniel P. Berrange wrote: > On Tue, Dec 19, 2017 at 01:43:24PM +0000, Joao Martins wrote: > > On 12/19/2017 01:13 PM, Daniel P. Berrange wrote: > > > On Tue, Dec 19, 2017 at 01:01:36PM +0000, Joao Martins wrote: > > >> [Sorry for double posting, but I mistakenly forgot to include libvirt list) > > >> > > >> +WimT +Daniel > > >> > > >> On 12/10/2017 02:10 AM, Marek Marczykowski-Górecki wrote: > > >>> <cpu mode='host-passthrough'> element may be used to configure other > > >>> features, like NUMA, or CPUID. Do not enable nested HVM (which is in > > >>> "preview" state after all) by mere presence of > > >>> <cpu mode='host-passthrough'> element, but require explicit <feature > > >>> policy='force' name='vmx'/> (or 'svm'). > > >>> Also, adjust xenconfig driver to appropriately translate to/from > > >>> nestedhvm=1. > > >>> > > >>> While at it, adjust xenconfig driver to not override def->cpu if already > > >>> set elsewhere. This will help with adding cpuid support. > > >> > > >> I agree with this and it was what we came up in the first version of nested hvm > > >> support[0]. Although Daniel suggested there to use the same semantics of qemu > > >> driver such that host-passthrough enables nested hvm without the use of: > > >> > > >> <feature policy='require' name='vmx'/> > > > > > > Yes, the key point of libvirt is to apply consistent semantics across different > > > drivers, so we should not diverge betweeen QEMU & Xen in this regard. > > > > > > > /nods > > > > > 'host-passthrough' and 'host-model' are supposed to expose *every* feature that > > > the host CPUs support (except for those few which the hypervisor may block due > > > to ability to virtualize them). > > > > > > So 'host-passthrough' is correct to automatically expose vmx/svm, without > > > requiring any extra <feature> element, and I don't think we can accept > > > this patch. My point is you can use <cpu> element to configure various features, like mentioned above (NUMA etc). As discussed previously, in libxl driver only 'host-passthrough' mode makes sense, because that's what libxl allows (enabled/disable various features, not define the whole CPU). So, you can use something like: <cpu mode='host-passthrough'> <numa> <cell id='0' cpus='0-3' memory='512000' unit='KiB'/> <cell id='1' cpus='4-7' memory='512000' unit='KiB' memAccess='shared'/> </numa> </cpu> Now, this is _very not obvious_ you've just enabled potentially dangerous feature. Quoting https://wiki.xenproject.org/wiki/Nested_Virtualization_in_Xen: This means an L1 admin can DOS the L0 hypervisor. This is a potential security issue; for this reason, we do not recommend running nested virtualization in production yet. Enabling potentially harmful features without explicit consent is not something that I'd expect from a project meant to be used in production environment...

Generally I think this is bad idea that placing just <cpu mode='host-passthrough'>, without any specific setting, change anything (compared to no <cpu/> at all). At least in context of libxl driver.

> You could conceivably replicate the host-level control KVM has by using an > /etc/libvirt/libxl.conf driver level config option to indicate whether > nested-virt is permitted or not. That could work. Is 'nestedhvm' ok for parameter name (disabled by default)?