Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 1 Sep 2022 20:02:53 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Maxim Levitsky <mlevitsk@redhat.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
        Li RongQing <lirongqing@baidu.com>
Subject: Re: [PATCH 08/19] KVM: SVM: Remove redundant cluster calculation
 that also creates a shadow
Message-ID: <YxEP7ZBRIuFWhnYJ@google.com>
References: <20220831003506.4117148-1-seanjc@google.com>
 <20220831003506.4117148-9-seanjc@google.com>
 <7281e42323b53a016cf8545b7a4547d70d87efce.camel@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <7281e42323b53a016cf8545b7a4547d70d87efce.camel@redhat.com>
Precedence: bulk

On Wed, Aug 31, 2022, Maxim Levitsky wrote:
> On Wed, 2022-08-31 at 00:34 +0000, Sean Christopherson wrote:
> > Drop the redundant "cluster" calculation and its horrific shadowing in
> > the x2avic logical mode path.  The "cluster" that is declared at an outer
> > scope is derived using the exact same calculation and has performed the
> > left-shift.
> 
> Actually I think we should just revert the commit 
> 'KVM: SVM: Use target APIC ID to complete x2AVIC IRQs when possible'
> 
> 
> I know that the patch that intially introduced the the avic_kick_target_vcpus_fast had
> x2apic/x2avic code, and then it was split to avoid adding x2avic code before it was merged,
> resulting in this patch to add the x2apic specific code.
> 
> But when I fixed most issues of avic_kick_target_vcpus_fast in my 
> 'KVM: x86: SVM: fix avic_kick_target_vcpus_fast', I added back x2apic code because
> it was just natural to do since I had to calculate cluster/bitmap masks anyway.
> 
> I expected this patch to be dropped because of this but I guess it was not noticed,
> or patches were merged in the wrong order.
> 
> This is the reason of shadowing, duplicate calculations/etc.

Ooooh, I completely missed that x2AVIC was already supported.  I saw the code, but
between the fixing the first bug and unwinding everything everything else I didn't
see that the end result ended up being a full revert.

So yeah, a full revert is definitely in order.

Thanks!