linux-kernel - Re: [PATCH v2 3/5] KVM: x86: Use common 'enable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YK5kQcTh8LmE0+8I@google.com>
Date:   Wed, 26 May 2021 15:07:45 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Maxim Levitsky <mlevitsk@...hat.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>, kvm@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Kechen Lu <kechenl@...dia.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/5] KVM: x86: Use common 'enable_apicv' variable for
 both APICv and AVIC

On Wed, May 26, 2021, Maxim Levitsky wrote:
> On Tue, 2021-05-18 at 16:43 +0200, Vitaly Kuznetsov wrote:
> > Unify VMX and SVM code by moving APICv/AVIC enablement tracking to common
> > 'enable_apicv' variable. Note: unlike APICv, AVIC is disabled by default.
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 9b6bca616929..23fdbba6b394 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -209,6 +209,9 @@ EXPORT_SYMBOL_GPL(host_efer);
> >  bool __read_mostly allow_smaller_maxphyaddr = 0;
> >  EXPORT_SYMBOL_GPL(allow_smaller_maxphyaddr);
> >  
> > +bool __read_mostly enable_apicv = true;
> 
> Nitpick: I don't like this asymmetry.
>
> VMX and the common code uses the enable_apicv module param and variable,
> while SVM uses avic, which sets the enable_apicv variable.
>  
> I'll prefer both VMX and SVM have their own private variable for their
> avic/apicv module param, which should set a common variable later.

I don't love the intermediate "avic" either, but there isn't a good alternative.
Forcing VMX to also use an intermediate doesn't make much sense, we'd be penalizing
ourselves in the form of unnecessary complexity just because AVIC needs to be
disabled by default for reasons KVM can't fix.

As for the asymmetry, I actually like it because it makes "avic" stand out and
highlights that there is weirdness with enabling AVIC.