Re: [PATCH] KVM : Fix read/write to IA32_FEATURE_CONTROL MSR in nested virt

[Gleb Natapov <gleb@redhat.com>]

On Thu, Jul 04, 2013 at 02:34:11PM +0200, Paolo Bonzini wrote:
> Il 04/07/2013 13:31, Gleb Natapov ha scritto:
> > On Thu, Jul 04, 2013 at 01:21:51PM +0200, Paolo Bonzini wrote:
> >> Il 04/07/2013 13:12, Gleb Natapov ha scritto:
> >>>> I don't like that it requires a firmware change in order to use nested
> >>>> VMX (at least for hypervisors that read the MSR).  "Worse emulation" and
> >>>> "better emulation + new firmware" are indistiguishable from the point of
> >>>> view of anyone except the firmware.
> >>>>
> >>>> IMO there is no reason for a better emulation that no one would care
> >>>> about _and_ could look like a regression when updating to a newer kernel.
> >>>
> >>> That is why now is the good time to do that since nested vmx is not
> >>> widely used. When it will be widely used the change will be impossible
> >>> to do for reason you age giving. So it is now or never.
> >>
> >> I think it is a can of worms.  For example, should this be
> >> conditionalized on running under QEMU?  Under UEFI, TianoCore should be
> >> doing it, not SeaBIOS.  And for CoreBoot, should it be done by CoreBoot
> >> or SeaBIOS?  (How do people use KVM together with CoreBoot?)
> >>
> > This is not the first thing that firmware need to initialize. I let
> > firmware guys fight over who is doing it, we just model HW. FWIW for
> > Seabios patch would be trivial.
> 
> Trivial but still depending on the question "who is doing it".  If
> CoreBoot should (also) be doing it, the SeaBIOS patch should be
> conditional on CONFIG_QEMU.  Also, should it be unconditional or depend
> on some external configuration knob (as on a bare-metal firmware)?
> 
Let firmware developers solve firmware problems. They have all the same
problems when running on real HW and they will have to figure out a
solution regardless. Making things different on virt will only cause
people to treat virt differently (remember irqbalance?).

> Actually KVM probes MSR_IA32_FEATURE_CONTROL itself and sets the bits,
> so we can sidestep the whole firmware thing, and go with a fixed version
> of Nadav's patch.
> 
Indeed, so no regression will be seen even temporary.

> >> So I still prefer never... :)
> >
> > This is a "can of worms" IMO. What we decide to init in KVM next to
> > relieve firmware from its duty? This is "other hypervisor" way, in KVM
> > we just model HW.
> 
> FWIW, I now checked Xen nested VMX and it just returns 5, but this has
> nothing to do with paravirtualization).
> 
> Paolo

--
			Gleb.