Message ID | 20230718091310.119672-2-mlevitsk@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c923:0:b0:3e4:2afc:c1 with SMTP id j3csp1619138vqt; Tue, 18 Jul 2023 02:41:34 -0700 (PDT) X-Google-Smtp-Source: APBJJlFgzHQtABmmHIkeWLQPJlPZDYLtEZVdoRWhLCavWvklo/tq6JKMmtvNGw7g2BWu+bNtqEN7 X-Received: by 2002:aa7:cfc3:0:b0:51b:cee4:fc21 with SMTP id r3-20020aa7cfc3000000b0051bcee4fc21mr13674459edy.39.1689673294561; Tue, 18 Jul 2023 02:41:34 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689673294; cv=none; d=google.com; s=arc-20160816; b=sXWEgt21Mo0GRg/ilsTtIXYCfhn7//OC3ohtQAQr+Y2HVhOTWc8d5mKbhng/I8zDRN kUiEzQ2DW2SwBG3e/dyLo1oaGBl3nI52HqPnklLV+5sxVfgemNZ1/5WvPA3JZjKamM6x GRYE7CB0goxoRnzj43LWmNuqbepF8NGQIhuWDJR9brTxWlSEV6ib9erNiIdZPVinBwCJ wtlgk3iTUBK2AkjdZoCSUu50hmWdKTpRGextyfBm8wYcnOSPZqotZs/vl15eizpYRrwZ lBVrxwsPJILxVbcRW3dIsCaSHHzDhNEhfHOvDrwXKxwsRd/DDg+kV/QEwhVTvt97iQ5y QUVw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=4WlIkr5Df/8dGsk772CbAon6bnUBTqjKXV9qoX9zRiI=; fh=h+09CC2YFXYXA44jAN2W/XszUAPEuokN58P13gOP3bQ=; b=F3rHug9xTYqQdtypSB/dm0SHhfT7Ciij4flfQTlawmzd0Zn05MUGMGs1YO/9xPy8lw +NDpoJ3CJjSxbiRuUf11SPdhkksgU8bZhr0LYRS4q+5okg7DotWEBXSvcjIWGILwIQci nA0KHP47e2vqPFZ/UPQaIsZZb4dFgf+BFvRctwQkNgZ8yKWviAfLdNi7ub2i9JNJL1yt Oz7AfTT+GklD+Aerk+nI5lXn7PtmgP0eKk11zfhxbxy1wEslywkY3jxljwr7x7xgbIeT pWfSumWO+sbw+V/ls1jZcUArAh80lv9jClh6oLQ2NJcIJCUclR72SRzwCtiaonMVO8rq gHNA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=XuWIn0EF; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id q4-20020aa7d444000000b0051e576dbb63si954162edr.534.2023.07.18.02.41.11; Tue, 18 Jul 2023 02:41:34 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=XuWIn0EF; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232131AbjGRJOV (ORCPT <rfc822;assdfgzxcv4@gmail.com> + 99 others); Tue, 18 Jul 2023 05:14:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53192 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230230AbjGRJOK (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 18 Jul 2023 05:14:10 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C837EE60 for <linux-kernel@vger.kernel.org>; Tue, 18 Jul 2023 02:13:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1689671602; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=4WlIkr5Df/8dGsk772CbAon6bnUBTqjKXV9qoX9zRiI=; b=XuWIn0EFCjTct5C0MhoQUZ8EgY/Cqus0cPO7OCBjJ4YZjZtK8EhrJBCoWifu5jRJufvjww KR2F1S8yFBmuAvg274kvv+53b1zv6cz9nRjybyq5UO2XRWS2Gb7bUCvTS/MunGlX/LUOzR a6j3z8FsyaPYdHAIiBPu4Ysbmok1tCI= Received: from mimecast-mx02.redhat.com (66.187.233.73 [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-14-dSfb6Q6kOYqqc21jhdQidA-1; Tue, 18 Jul 2023 05:13:18 -0400 X-MC-Unique: dSfb6Q6kOYqqc21jhdQidA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 685113803916; Tue, 18 Jul 2023 09:13:18 +0000 (UTC) Received: from localhost.localdomain (unknown [10.45.224.28]) by smtp.corp.redhat.com (Postfix) with ESMTP id ECCAC1454143; Tue, 18 Jul 2023 09:13:15 +0000 (UTC) From: Maxim Levitsky <mlevitsk@redhat.com> To: kvm@vger.kernel.org Cc: "H. Peter Anvin" <hpa@zytor.com>, Sean Christopherson <seanjc@google.com>, linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>, Paolo Bonzini <pbonzini@redhat.com>, Ingo Molnar <mingo@redhat.com>, Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org, Borislav Petkov <bp@alien8.de>, Maxim Levitsky <mlevitsk@redhat.com> Subject: [PATCH 1/3] KVM: x86: VMX: __kvm_apic_update_irr must update the IRR atomically Date: Tue, 18 Jul 2023 12:13:08 +0300 Message-Id: <20230718091310.119672-2-mlevitsk@redhat.com> In-Reply-To: <20230718091310.119672-1-mlevitsk@redhat.com> References: <20230718091310.119672-1-mlevitsk@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.1 on 10.11.54.7 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1771750864557575131 X-GMAIL-MSGID: 1771750864557575131 |
Series |
Fix 'Spurious APIC interrupt (vector 0xFF) on CPU#n' issue
|
|
Commit Message
Maxim Levitsky
July 18, 2023, 9:13 a.m. UTC
If APICv is inhibited, then IPIs from peer vCPUs are done by
atomically setting bits in IRR.
This means, that when __kvm_apic_update_irr copies PIR to IRR,
it has to modify IRR atomically as well.
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
arch/x86/kvm/lapic.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
Comments
On 7/18/23 11:13, Maxim Levitsky wrote: > + irr_val = READ_ONCE(*((u32 *)(regs + APIC_IRR + i * 0x10))); Let's separate out the complicated arithmetic, as it recurs below too: u32 *p_irr = (u32 *)(regs + APIC_IRR + i * 0x10); > + while (!try_cmpxchg(((u32 *)(regs + APIC_IRR + i * 0x10)), > + &irr_val, irr_val | pir_val)); > + > prev_irr_val = irr_val; > - irr_val |= xchg(&pir[i], 0); > - *((u32 *)(regs + APIC_IRR + i * 0x10)) = irr_val; > - if (prev_irr_val != irr_val) { > - max_updated_irr = > - __fls(irr_val ^ prev_irr_val) + vec; > - } > + irr_val |= pir_val; > + > + if (prev_irr_val != irr_val) > + max_updated_irr = __fls(irr_val ^ prev_irr_val) + vec; We can write this a bit more cleanly too, and avoid unnecessary try_cmpxchg too: prev_irr_val = irr_val; do irr_val = prev_irr_val | pir_val; while (prev_irr_val != irr_val && !try_cmpxchg(p_irr, &prev_irr_val, irr_val)); if (prev_irr_val != irr_val) max_updated_irr = __fls(irr_val ^ prev_irr_val) + vec; If this looks okay to you, I'll queue the patches for -rc3 and also Cc them for inclusion in stable kernels. Paolo
У вт, 2023-07-18 у 13:41 +0200, Paolo Bonzini пише: > On 7/18/23 11:13, Maxim Levitsky wrote: > > + irr_val = READ_ONCE(*((u32 *)(regs + APIC_IRR + i * 0x10))); > > Let's separate out the complicated arithmetic, as it recurs below too: > > u32 *p_irr = (u32 *)(regs + APIC_IRR + i * 0x10); No objections at all for this change, I wanted to have a minimal patch. > > > + while (!try_cmpxchg(((u32 *)(regs + APIC_IRR + i * 0x10)), > > + &irr_val, irr_val | pir_val)); > > + > > prev_irr_val = irr_val; > > - irr_val |= xchg(&pir[i], 0); > > - *((u32 *)(regs + APIC_IRR + i * 0x10)) = irr_val; > > - if (prev_irr_val != irr_val) { > > - max_updated_irr = > > - __fls(irr_val ^ prev_irr_val) + vec; > > - } > > + irr_val |= pir_val; > > + > > + if (prev_irr_val != irr_val) > > + max_updated_irr = __fls(irr_val ^ prev_irr_val) + vec; > > We can write this a bit more cleanly too, and avoid unnecessary To be honest as far as I see, no matter what to do with this function, it is still a bit complicated IMHO: The root cause of the complexity in this function is that it does two things at the same time - copies both the new bits to IRR and also counts the max_irr. It would be so much cleaner to first copy new bits from PIR to irr (and that can be done with 'lock or' or even by setting each bit with atomic bit set (in this way the setting of the bits will be pretty much the same as what other users of IRR do (set bit atomically + set irr_pending). And then let the common code count the max_irr. I doubt this will affect performance in any way, but I don't have a good way to measure it, so I won't be arguing about it. On the other hand, I am thinking now that maybe I should make the cmpxchg conditional on apicv beeing inhibited, as otherwise it works for nothing and actually might affect performance. This though might in theory cause a race if a sender incorrectly thinks that this's vCPU APICv is inhibited or not. It probalby doesn't matter as the only reason for APICv to be inhibited is that AutoEOI thing which should happen just once when the guest boots. I also have another idea - I can make the IPI senders still set bits in the PIR even if APICv is inhibited, then there is no race to worry about although then the bits will always have to be copied from PIR to IRR (but then again APICv inhibition is rare). > try_cmpxchg too: > > prev_irr_val = irr_val; > do > irr_val = prev_irr_val | pir_val; > while (prev_irr_val != irr_val && > !try_cmpxchg(p_irr, &prev_irr_val, irr_val)); > > if (prev_irr_val != irr_val) > max_updated_irr = __fls(irr_val ^ prev_irr_val) + vec; > > If this looks okay to you, I'll queue the patches for -rc3 and also Cc > them for inclusion in stable kernels. No objections for this change as well. Best regards, Maxim Levitsky > > Paolo >
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index e542cf285b5184..b3f57e0f0d64ae 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -627,15 +627,19 @@ bool __kvm_apic_update_irr(u32 *pir, void *regs, int *max_irr) for (i = vec = 0; i <= 7; i++, vec += 32) { pir_val = READ_ONCE(pir[i]); - irr_val = *((u32 *)(regs + APIC_IRR + i * 0x10)); + irr_val = READ_ONCE(*((u32 *)(regs + APIC_IRR + i * 0x10))); + if (pir_val) { + pir_val = xchg(&pir[i], 0); + + while (!try_cmpxchg(((u32 *)(regs + APIC_IRR + i * 0x10)), + &irr_val, irr_val | pir_val)); + prev_irr_val = irr_val; - irr_val |= xchg(&pir[i], 0); - *((u32 *)(regs + APIC_IRR + i * 0x10)) = irr_val; - if (prev_irr_val != irr_val) { - max_updated_irr = - __fls(irr_val ^ prev_irr_val) + vec; - } + irr_val |= pir_val; + + if (prev_irr_val != irr_val) + max_updated_irr = __fls(irr_val ^ prev_irr_val) + vec; } if (irr_val) *max_irr = __fls(irr_val) + vec;