Re: [PATCH] i386/kvm: add NoNonArchitecturalCoreSharing Hyper-V enlightenment

[Eduardo Habkost <ehabkost@redhat.com>]

On Wed, Oct 23, 2019 at 01:16:38PM +0200, Vitaly Kuznetsov wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Mon, Oct 21, 2019 at 06:26:14PM +0200, Paolo Bonzini wrote:
> >> On 21/10/19 16:09, Vitaly Kuznetsov wrote:
> >> >>> +    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
> >> >>> +        env->features[FEAT_HV_RECOMM_EAX] |= HV_NO_NONARCH_CORESHARING;
> >> >>> +    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
> >> >> Do you want to make auto the default if "-cpu host,migratable=off"?  It
> >> >> can be done on top so I started queueing this patch.
> >> > Hm, one thing is that CPUID 0x40000004 doesn't exist if no Hyper-V
> >> > enlightenments are passed so we'll probably have to modify your idea to
> >> > "-cpu host,migratable=off,+any-hyperv-enlightenment" but then the
> >> > question is how conservative are we, like if QEMU command line doesn't
> >> > change can new CPUID flags appear or not? And we'll probably need a way
> >> > to explicitly disable HV_NO_NONARCH_CORESHARING if needed.
> >> 
> >> I would defer to Eduardo on whether "migratable=off" would allow adding
> >> new CPUID flags.  The follow-up question however is whether we would
> >> benefit from a "+hyperv" option that enables all known Hyper-V
> >> enlightenment for a given machine type.
> >
> > I'm not sure what "adding new CPUID flags" means exactly, but on
> > both cases, the answer is yes:
> >
> > If you mean having new flags appear with the same QEMU command
> > line, this is 100% OK with "-cpu host".  Doubly so with
> > "migratable=off".  "-cpu host" doesn't guarantee a stable guest
> > ABI, and migratable=off doesn't guarantee the ability to live
> > migrate.
> >
> > If you just mean the ability to write "-cpu
> > host,migratable=off,+some-extra-flag", that's OK too.
> >
> > I would try to make "-cpu host,migratable=off" enable all
> > features out of the box (because users probably expect that).
> > But we you have a compelling reason to not enable the hyperv
> > flags by default (do we?), it's OK to require something like
> > "-cpu host,...,+hyperv".
> 
> I'm not sure if the reason is compelling enough but I remember some
> Linux tools were only looking at the first hypervisor signature and
> reporting that we're now running on Hyper-V. Also, more features you
> enable larger the atack surface...
> 
> Actually, we already '-cpu host,hv_passthrough' option which implies
> 'migratable=off', not sure if another one is needed.

So, if I understood correctly, Paolo's "+hyperv" suggestion above
is already implemented by "hv_passthrough"?  Sounds good enough
to me.

-- 
Eduardo