linux-kernel - Re: [PATCH v3 07/28] KVM: x86: Inhibit APIC memslot if x2APIC and AVIC are enabled

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 28 Sep 2022 22:35:31 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Maxim Levitsky <mlevitsk@...hat.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Alejandro Jimenez <alejandro.j.jimenez@...cle.com>,
        Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
        Li RongQing <lirongqing@...du.com>
Subject: Re: [PATCH v3 07/28] KVM: x86: Inhibit APIC memslot if x2APIC and
 AVIC are enabled

On Wed, Sep 28, 2022, Maxim Levitsky wrote:
> On Wed, 2022-09-28 at 16:33 +0000, Sean Christopherson wrote:
> > Similarly, is there a good reason for having nested_svm_vmexit() invoke
> > kvm_vcpu_update_apicv() directly?  I'm confused by the "so that other vCPUs can
> > start to benefit from it right away".  The nested inhibit is per-vCPU and so
> > should only affect the current vCPU, no?  I.e. for all intents and purposes, using
> > a request should be functionally equivalent.
> 
> It is kind of the other way around:
> 
> The mere fact of switching to vmcb02 *inhibits* the AVIC on the current vCPU,
> but the AVIC inhibit is there only to set the is_running bits in the physid table
> and in IOMMU to prevent its *peers* to try and send interrupts to it via AVIC.
> 
> It is the reason why APICv doesn't need it - the posted interrupts still work
> just fine when a vCPU doens't use APICv, or uses a different posted interrupt vector
> when it uses the nested APICv.

Gotcha, the "other vCPUs" part is where I got confused.

> So it makes sense to remove that inhibit as soon as possible that the peers
> could stop getting 'unaccellerated IPI' vmexits for nothing.

But practically speaking, the delay between the nested VM-Exit and servicing the
request is minimal.  Might be a moot point if nested AVIC is supported, i.e. an
inline update may be "required" at that point.

Not a sticking point by any means, but if possible, it would be nice to have a
single call site for the per-vCPU APICv update.

> However back to the discussion, I don't think this is a problem.
> 
> We can just call both the kvm_vcpu_update_apicv() and a new function that
> does the memslot disable from KVM_REQ_APICV_UPDATE, then 
> plain kvm_vcpu_update_apicv() won't need to drop the srcu lock.
> 
> It is pretty much the same that you proposed, just instead of piggybacking on 
> KVM_REQ_UNBLOCK, I proposed to piggyback on KVM_REQ_APICV_UPDATE.

Yep, easy to do after converting the x2APIC toggling to use a request.

Thanks!