From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CD01BC32771 for ; Wed, 28 Sep 2022 20:45:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234191AbiI1UpJ (ORCPT ); Wed, 28 Sep 2022 16:45:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44328 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234134AbiI1Uov (ORCPT ); Wed, 28 Sep 2022 16:44:51 -0400 Received: from mail-pj1-x1032.google.com (mail-pj1-x1032.google.com [IPv6:2607:f8b0:4864:20::1032]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 833BB10E7 for ; Wed, 28 Sep 2022 13:44:48 -0700 (PDT) Received: by mail-pj1-x1032.google.com with SMTP id v10-20020a17090a634a00b00205e48cf845so3861513pjs.4 for ; Wed, 28 Sep 2022 13:44:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=Vdbd3ulUXBGBc17CxMn5J69DULA+Of7+9gUX+29F9hg=; b=rRgYmA36A88TMq0xkAhH2g3RVfk7DGY20N2SAcxV76lx/Td1UXHyEj0WvIFdcrSVwO qS3yRzGMyD+kkje1aIUlU3+mf8anLceaq8CC9Oa92Ub67hFzqpRFpfdUpiQj/J01t+Q8 P630ne57RF9w5e0sidUK3rBoFfAadHl0NX9NAkRIChI5x1iDfRooe/ullrP1wK/SfFo6 +vX+TkZ4yo2d9ESnik1Mb5jbCaWGxMBCkdPIfxSb6oegrgtCjHnVI2YeQbTNcAsMi5Mq AJvIcEVPIf80ETgnDPNdpf9TfhZUUbjRS4aWpCGuhumkXJkEt1WCS3nHaLN2VBB3TN0J 3plA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date; bh=Vdbd3ulUXBGBc17CxMn5J69DULA+Of7+9gUX+29F9hg=; b=aKmDnJKhw6jJKGC81VQjp6dcU6+5Ub00feRITQxnmdfBKE5NSHnbW44jxfJicLt9hd YEdqZZd6vfGHL7zf+s1xpSMPYiRyM1s8JDU58DnpOr8qSqyCfPlHBYNHghDPltIRHMW2 +PHgFEZlSsIa6eSveAbssiR2tVBGOcSj3PkWVmKieqgezfeFX2k/jFH+50oH8GFWnjvJ K9k7SHY/JkRVR1LeQJpO5ncr/kf3lgg/B6LM/C+k4J62snNhqaL47TZchXuKhKxVuLlH vWeKFI5QRQRfiMmgpd544TA6R1sAAeGPFI0HI8S8NsAILwCgn7M+yr8t+8+AZk+w0p5I oeCg== X-Gm-Message-State: ACrzQf1PY721Mf2uoHjWUFaDD0oRt/dpAQeqhA8pSkt0wJ6cAsScK11T SirajegLzvVdoI9BavHZjp2XrA== X-Google-Smtp-Source: AMsMyM4ivm1mIffq+6jbSHGX7GyxSYZ5r/9aMnzoa8fPG673WUexpILjoZ7Bo3Su0MuOfE2tDm8VQQ== X-Received: by 2002:a17:90a:2f43:b0:205:f4dc:a47b with SMTP id s61-20020a17090a2f4300b00205f4dca47bmr4555625pjd.62.1664397888386; Wed, 28 Sep 2022 13:44:48 -0700 (PDT) Received: from google.com (7.104.168.34.bc.googleusercontent.com. [34.168.104.7]) by smtp.gmail.com with ESMTPSA id v11-20020a170902d68b00b0016ed8af2ec0sm1096143ply.29.2022.09.28.13.44.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Sep 2022 13:44:47 -0700 (PDT) Date: Wed, 28 Sep 2022 20:44:44 +0000 From: Sean Christopherson To: Maxim Levitsky Cc: Alejandro Jimenez , Paolo Bonzini , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Suravee Suthikulpanit , Li RongQing Subject: Re: [PATCH v3 05/28] KVM: x86: Don't inhibit APICv/AVIC if xAPIC ID mismatch is due to 32-bit ID Message-ID: References: <20220920233134.940511-1-seanjc@google.com> <20220920233134.940511-6-seanjc@google.com> <1aea43e831cd7ed90c325b2c90bc6f3f9a1805b5.camel@redhat.com> <6fcec4adb331452c64adf255c367af86072bd9d3.camel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6fcec4adb331452c64adf255c367af86072bd9d3.camel@redhat.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 28, 2022, Maxim Levitsky wrote: > On Wed, 2022-09-28 at 18:03 +0000, Sean Christopherson wrote: > > On Wed, Sep 28, 2022, Maxim Levitsky wrote: > > > On Wed, 2022-09-28 at 16:51 +0000, Sean Christopherson wrote: > > > > > > It happens regardless of vCPU count (tested with 2, 32, 255, 380, and > > > > > > 512 vCPUs). This state persists for all subsequent reboots, until the VM > > > > > > is terminated. For vCPU counts < 256, when x2apic is disabled the > > > > > > problem does not occur, and AVIC continues to work properly after reboots. > > > > > > > > Bit of a shot in the dark, but does the below fix the issue? There are two > > > > issues with calling kvm_lapic_xapic_id_updated() from kvm_apic_state_fixup(): > > > > > > > > 1. The xAPIC ID should only be refreshed on "set". > > > True - I didn't bother to fix it yet because it shouldn't cause harm, but > > > sure this needs to be fixed. > > > > It's probably benign on its own, but with the missing "hardware enabled" check, > > it could be problematic if userspace does KVM_GET_LAPIC while the APIC is hardware > > disabled, after the APIC was previously in x2APIC mode. I'm guessing QEMU does > > KVM_GET_LAPIC state when emulating reboot, hence the potential for being involved > > in the bug Alejandro is seeing. > > > > > > 2. The refresh needs to be noted after memcpy(vcpu->arch.apic->regs, s->regs, sizeof(*s)); > > > Are you sure? The check is first because if it fails, then error is returned to userspace > > > and the KVM's state left unchanged. > > > > > > I assume you are talking about > > > > > > .... > > > r = kvm_apic_state_fixup(vcpu, s, true); > > > if (r) { > > > kvm_recalculate_apic_map(vcpu->kvm); > > > return r; > > > } > > > memcpy(vcpu->arch.apic->regs, s->regs, sizeof(*s)); > > > > This isn't a failure path though, it's purely a "take note of the update", and > > KVM needs to do that processing _after_ the actual update. Specifically, > > kvm_lapic_xapic_id_updated() consumes the internal APIC state: > > Yes, I somehow blindly assumed that kvm_apic_state_fixup actually checks > the new state and not the existing state. > > Probably because my original code did that, I think it just checked the 'id' > variable.. Oh well. > Thanks for catching this bug! This was indeed the bug Alejandro hit. QEMU does KVM_SET_LAPIC and KVM checks the stale vAPIC state. Because the vCPU was in x2APIC mode, the 32-bit ID is in bits 31:0, not bit 31:24, and so kvm_xapic_id() returned '0' instead of the correct ID.