All of lore.kernel.org
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	drjones@redhat.com, Marcelo Tosatti <mtosatti@redhat.com>,
	qemu-devel@nongnu.org, Eduardo Habkost <ehabkost@redhat.com>
Subject: Re: [PATCH v4 16/21] i386: track explicit 'hv-*' features enablement/disablement
Date: Mon, 15 Feb 2021 16:55:02 +0100	[thread overview]
Message-ID: <20210215165502.64e1e629@redhat.com> (raw)
In-Reply-To: <87blcllnf0.fsf@vitty.brq.redhat.com>

On Mon, 15 Feb 2021 09:56:19 +0100
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > On Fri, 12 Feb 2021 16:26:03 +0100
> > Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> >  
> >> Vitaly Kuznetsov <vkuznets@redhat.com> writes:
> >>   
> >> > Igor Mammedov <imammedo@redhat.com> writes:
> >> >    
> >> >>
> >> >> Please try reusing scratch CPU approach, see
> >> >>   kvm_arm_get_host_cpu_features()
> >> >> for an example. You will very likely end up with simpler series,
> >> >> compared to reinventing wheel.    
> >> >
> >> > Even if I do that (and I serioulsy doubt it's going to be easier than
> >> > just adding two 'u64's, kvm_arm_get_host_cpu_features() alone is 200
> >> > lines long) this is not going to give us what we need to distinguish
> >> > between
> >> >
> >> > 'hv-passthrough,hv-evmcs'
> >> >
> >> > and 
> >> >
> >> > 'hv-passthrough'
> >> >
> >> > when 'hv-evmcs' *is* supported by the host. When guest CPU lacks VMX we
> >> > don't want to enable it unless it was requested explicitly (former but
> >> > not the later).    
> >> 
> >> ... and if for whatever reason we decide that this is also bad/not
> >> needed, I can just drop patches 16-18 from the series (leaving
> >> 'hv-passthrough,hv-feature=off' problem to better times).  
> > that's also an option,
> > we would need to make sure that hv-passthrough is mutually exclusive
> > with ''all'' other hv- properties to avoid above combination being
> > ever (mis)used.  
> 
> That's an option to finally get these patches merged, not a good option
> for end users. 
> 
> 'hv-passthrough,hv-feature' works today and it's useful. Should we drop
> it?
well,
try suggested idea about using scratch CPU and it might get merged sooner.
(it's not like I'm suggesting you to rewrite half of QEMU, just some of
patches, which most likely would simplify series from my point of view
and would be easier to maintain)

> 
> 'hv-passthrough/hv-default' and 'hv-passthrough/hv-default,hv-evmcs'
> should give us sane results.
> 
> 'hv-passthrough,hv-feature=off' is convenient.
> 
> Why droppping this all? To save 9 (nine) lines of code in the parser? 
it's doing what generic property parsing is capable off, provided you
fish out hv-passthrough value in advance like arm/virt does (I think ppc
does similar hing also), so I consider it as unnecessary code duplication/
complication and maintenance burden.

If it were a hotfix during hard-freeze may be I'd agree (with promise to
rework it later to something more palatable), but it's not, for patches in
state they are now I'm not confident enough to ACK them.



  reply	other threads:[~2021-02-15 15:56 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-10 16:40 [PATCH v4 00/19] i386: KVM: expand Hyper-V features early and provide simple 'hv-default=on' option Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 01/21] i386: keep hyperv_vendor string up-to-date Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 02/21] i386: invert hyperv_spinlock_attempts setting logic with hv_passthrough Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 03/21] i386: always fill Hyper-V CPUID feature leaves from X86CPU data Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 04/21] i386: stop using env->features[] for filling Hyper-V CPUIDs Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 05/21] i386: introduce hyperv_feature_supported() Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 06/21] i386: introduce hv_cpuid_get_host() Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 07/21] i386: drop FEAT_HYPERV feature leaves Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 08/21] i386: introduce hv_cpuid_cache Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 09/21] i386: split hyperv_handle_properties() into hyperv_expand_features()/hyperv_fill_cpuids() Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 10/21] i386: move eVMCS enablement to hyperv_init_vcpu() Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 11/21] i386: switch hyperv_expand_features() to using error_setg() Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 12/21] i386: adjust the expected KVM_GET_SUPPORTED_HV_CPUID array size Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 13/21] i386: prefer system KVM_GET_SUPPORTED_HV_CPUID ioctl over vCPU's one Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 14/21] i386: use global kvm_state in hyperv_enabled() check Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 15/21] i386: expand Hyper-V features during CPU feature expansion time Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 16/21] i386: track explicit 'hv-*' features enablement/disablement Vitaly Kuznetsov
2021-02-11 17:35   ` Igor Mammedov
2021-02-12  8:45     ` Vitaly Kuznetsov
2021-02-12 14:12       ` Igor Mammedov
2021-02-12 15:19         ` Vitaly Kuznetsov
2021-02-12 15:26           ` Vitaly Kuznetsov
2021-02-12 16:05             ` Igor Mammedov
2021-02-15  8:56               ` Vitaly Kuznetsov
2021-02-15 15:55                 ` Igor Mammedov [this message]
2021-02-15 17:05                   ` Igor Mammedov
2021-02-15 18:12                   ` Vitaly Kuznetsov
2021-02-12 16:01           ` Igor Mammedov
2021-02-15  8:53             ` Vitaly Kuznetsov
2021-02-15 10:48               ` Andrew Jones
2021-02-15 17:01               ` Igor Mammedov
2021-02-15 18:11                 ` Vitaly Kuznetsov
2021-02-22 10:20                   ` Vitaly Kuznetsov
2021-02-23 15:19                     ` Igor Mammedov
2021-02-23 15:46                       ` Vitaly Kuznetsov
2021-02-23 17:48                         ` Igor Mammedov
2021-02-23 18:08                           ` Vitaly Kuznetsov
2021-02-24 16:06                             ` Igor Mammedov
2021-02-24 17:00                               ` Vitaly Kuznetsov
2021-03-01 15:32                                 ` Igor Mammedov
2021-03-01 16:22                                   ` Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 17/21] i386: support 'hv-passthrough, hv-feature=off' on the command line Vitaly Kuznetsov
2021-02-11 17:14   ` Igor Mammedov
2021-02-12  8:49     ` Vitaly Kuznetsov
2021-02-12  9:29       ` David Edmondson
2021-02-12 13:52       ` Igor Mammedov
2021-02-10 16:40 ` [PATCH v4 18/21] i386: be more picky about implicit 'hv-evmcs' enablement Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 19/21] i386: introduce kvm_hv_evmcs_available() Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 20/21] i386: provide simple 'hv-default=on' option Vitaly Kuznetsov
2021-02-11 17:23   ` Igor Mammedov
2021-02-12  8:52     ` Vitaly Kuznetsov
2021-02-10 16:40 ` [PATCH v4 21/21] qtest/hyperv: Introduce a simple hyper-v test Vitaly Kuznetsov
2021-02-10 16:56 ` [PATCH v4 00/19] i386: KVM: expand Hyper-V features early and provide simple 'hv-default=on' option Daniel P. Berrangé
2021-02-10 17:46   ` Eduardo Habkost
2021-02-11  8:30     ` Vitaly Kuznetsov
2021-02-11  9:14       ` Daniel P. Berrangé
2021-02-11  9:34         ` Vitaly Kuznetsov
2021-02-11 10:14           ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210215165502.64e1e629@redhat.com \
    --to=imammedo@redhat.com \
    --cc=drjones@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=vkuznets@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.