Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Bandan Das <bsd@redhat.com>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/apic: reset LDR in clear_local_APIC
References: <jpga7ccl7la.fsf@linux.bootlegged.copy>
        <alpine.DEB.2.21.1908192259390.4008@nanos.tec.linutronix.de>
        <jpgk1b8g69t.fsf@linux.bootlegged.copy>
        <alpine.DEB.2.21.1908200052281.4008@nanos.tec.linutronix.de>
        <jpgwof6plkv.fsf@linux.bootlegged.copy>
        <alpine.DEB.2.21.1908212008500.1983@nanos.tec.linutronix.de>
Date:   Wed, 21 Aug 2019 15:54:34 -0400
In-Reply-To: <alpine.DEB.2.21.1908212008500.1983@nanos.tec.linutronix.de>
        (Thomas Gleixner's message of "Wed, 21 Aug 2019 20:22:21 +0200
        (CEST)")
Message-ID: <jpgimqq70qt.fsf@linux.bootlegged.copy>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Thomas Gleixner <tglx@linutronix.de> writes:

> Bandan,
>
> On Wed, 21 Aug 2019, Bandan Das wrote:
>> Thomas Gleixner <tglx@linutronix.de> writes:
>> So, in KVM: if we make sure that the logical destination map isn't filled up if the virtual
>> apic is not enabled by software, it really doesn't matter whether the LDR for an inactive CPU
>> has a stale value.
>>
>> In x86/apic: if we make sure that the LDR is 0 or reset,
>> recalculate_apic_map() will never consider including this cpu in the
>> logical map.
> ?
>> In short, as I mentioned in the patch description, this is really a KVM
>> bug but it doesn't hurt to clear out the LDR in the guest and then, it
>> wouldn't need a hypervisor fix.
>
> I still needs a hypervisor fix. Taking disabled APICs into account is a bug
> which has also other consequeces than that particular one. So please don't
> claim that. It's wrong.
>
> If that prevents the APIC bug from triggering on unfixed hypervisors, then
> this is a nice side effect, but not a solution.
>
Agreed and fwiw, the kvm fix has been queued already.

>> Is this better ?
>
> That's way better.
>
> So can you please create two patches:
>
>    1) Make that bogus bigsmp ldr init empty
>
>       That one wants a changelog along these lines:
>
>       - Setting LDR for physical destination mode is pointless
>       - Setting multiple bits in the LDR is wrong
>
>       Mention how this was discovered and caused the KVM APIC bug to be
>       triggered. Also mention that the change is not there to paper over
>       the KVM APIC bug. The change fixes a bug in the bigsmp APIC code.
>
>    2) Clear LDR in in that apic reset function
>
>       That one wants a changelog along these lines:
>
>       - Except for x2apic the LDR should be cleared as any other APIC
>       	register
>
>       Mention how this was discovered. Again the change is not there to
>       paper over the KVM APIC bug. It's for correctness sake and valid on
>       its own.
>
> Thanks,
>
Will do as you suggested. Thank you for the review.

Bandan
> 	tglx
>
>