Message ID | 20230207002156.521736-3-seanjc@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp2548307wrn; Mon, 6 Feb 2023 16:25:35 -0800 (PST) X-Google-Smtp-Source: AK7set9UC7jRZbogSFj4SupHQjyAgDijtHGY1EX7R+6qBNt+pn34Lzpc1A+MOT6kwqw8coZILnVP X-Received: by 2002:a05:6a21:6713:b0:b8:89f5:4e96 with SMTP id wh19-20020a056a21671300b000b889f54e96mr1008459pzb.26.1675729535726; Mon, 06 Feb 2023 16:25:35 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675729535; cv=none; d=google.com; s=arc-20160816; b=SFr4FTyCwmQ1VEjMIxhG5jDszJNv85ovHKT9kwEN+ZfbtNoeIotd3Elq5n6/9dRB0S HMVPc7oK2Plz9z5e7/dgWhzVQpuVppfLX3qmT3w/m/4yL8xKmxjp+W+PHqbhBfij+bpN OGCSleYu//+yBDdca5ixsTKv6U2fT/FCdBnwbvO5Qy+LoXJd1q2ARl2vO5/+DLp1alUZ nCsx0MOtj0RmuM09OONcw7l5FdTq+ttyN7C6FXK/mqVrNOqkq6Bx47hZYoRp45cIcONc wgttkxQRiOgIWgmC1CxO6xqROaGROPOhkwZnMGwnUN0IER9+GiJxJx0P7y/m2lUXfOCD eXrw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:reply-to:dkim-signature; bh=I+Z+wnDCARltqljoL3uU/iOxjVMjPeIS4mK0SvgvmXM=; b=NiORDZa2B3xMvE5os0nbXpQjagPvE2wWlD2rAz4irY05SoIZWW7RKcPducH5cz4Ivh VMrOUhB9tg/QJLjKnuH4jHVGn70Te8WUdwtnzapUBDQBZFzh1USFAm6tpe46kV9+9EJp NMj3FyPr5Y17UkHVxFiBxrdrb2d5rT3zBL3ynHLgq4XnHERpnpOfDvo2Lqtx74WaT5Zt nGwFKWa0t2SxDqYHqb7TKdZ6vD6HzhAFVY5q8KLsgBQiY4gM01tldwnkJVpYZJI+/vjE lX52aesgOiGTNIbJBuF847iDn0Cc5RPWYZI+6qMQtpUyH5G3wxBdsQmfXXqdYlnVum+R ETMA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b="E+x/tX7H"; 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=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s135-20020a63778d000000b004fab8543f06si8825518pgc.687.2023.02.06.16.25.21; Mon, 06 Feb 2023 16:25:35 -0800 (PST) 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=@google.com header.s=20210112 header.b="E+x/tX7H"; 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=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230249AbjBGAWN (ORCPT <rfc822;kmanaouilinux@gmail.com> + 99 others); Mon, 6 Feb 2023 19:22:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53746 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230205AbjBGAWE (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 6 Feb 2023 19:22:04 -0500 Received: from mail-pl1-x64a.google.com (mail-pl1-x64a.google.com [IPv6:2607:f8b0:4864:20::64a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7946D31E3F for <linux-kernel@vger.kernel.org>; Mon, 6 Feb 2023 16:22:02 -0800 (PST) Received: by mail-pl1-x64a.google.com with SMTP id z8-20020a170902834800b001990ad8de5bso2954081pln.10 for <linux-kernel@vger.kernel.org>; Mon, 06 Feb 2023 16:22:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=I+Z+wnDCARltqljoL3uU/iOxjVMjPeIS4mK0SvgvmXM=; b=E+x/tX7HzgELHcR+Ij6x7Y0MW/bHTaFLnLIVqRX3JDz8e8Tvpq5w7lT+zejYXOjKcm 147j5MdceO1IYTLoomosRta3aOcWTzHdvvjyBYnXrX7C6rAQW7rNYFm78r9QSo3bt/Xt dA4RKuHph/QH7FmEfBbPnxJslYmLR+Wb1gLR4n8A7sYsnr88bxF6pumaDYSAXWQPrvLB krCdEp6vYemo605TU9sVcoJgE65NsIgGaYhamjmFukKJZCeiphVfdXJ9FMq7r/+j2Bju Js5/j0LA4vFOq1K36AfQvjeZNWvmodCjPYkpEiSsI+7M4gvjrqNWBv0lqedseD5pMUtn 5SXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=I+Z+wnDCARltqljoL3uU/iOxjVMjPeIS4mK0SvgvmXM=; b=w0Pm5SJA8HQ4o9fD1qtzl8PwmtEIXgEOldhAWp8C9+9yjdmqvvh0uLi7fBbcQ9Im4Z /45P7Q6HIfwDC8V9GFQ5RtPUUb6m71KpbbP2x3Q3d5eYQ74vHtqHYwtWNaKbr5+Yhoh+ zZD3+BlTTqXrcIpj/lOov98tfniLyFe/T2T1FpTbBf2iySwUfvy88rJK/necHn63J0zX uxUEsziHMvDVbFdB5Q4Uepm/xyI/0cImxudQLbzIA18RZo4b65xsmwUnbveQvEZKKmi0 XtFGhhzr3t5h1+/2AbHBl0W/161WAIzWrrbNECFcJ9jC5mxGcRRflOfG3AGoRJH1X9BF ydNA== X-Gm-Message-State: AO0yUKXdq7T/7vU9XINJvCOmw5OrmxP4uUPbL3yXKBOB/Oit4YDzKoun en4Su1fesGjD2bRRcKjk7fZ4Wq3kPcQ= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:902:f54b:b0:192:c7f1:977b with SMTP id h11-20020a170902f54b00b00192c7f1977bmr219867plf.1.1675729321872; Mon, 06 Feb 2023 16:22:01 -0800 (PST) Reply-To: Sean Christopherson <seanjc@google.com> Date: Tue, 7 Feb 2023 00:21:55 +0000 In-Reply-To: <20230207002156.521736-1-seanjc@google.com> Mime-Version: 1.0 References: <20230207002156.521736-1-seanjc@google.com> X-Mailer: git-send-email 2.39.1.519.gcb327c4b5f-goog Message-ID: <20230207002156.521736-3-seanjc@google.com> Subject: [PATCH v2 2/3] KVM: SVM: Modify AVIC GATag to support max number of 512 vCPUs From: Sean Christopherson <seanjc@google.com> To: Sean Christopherson <seanjc@google.com>, Paolo Bonzini <pbonzini@redhat.com> Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Alejandro Jimenez <alejandro.j.jimenez@oracle.com>, Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham 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: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1757129773868304486?= X-GMAIL-MSGID: =?utf-8?q?1757129773868304486?= |
Series |
KVM: SVM: Fix GATag bug for >256 vCPUs
|
|
Commit Message
Sean Christopherson
Feb. 7, 2023, 12:21 a.m. UTC
From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> Define AVIC_VCPU_ID_MASK based on AVIC_PHYSICAL_MAX_INDEX, i.e. the mask that effectively controls the largest guest physical APIC ID supported by x2AVIC, instead of hardcoding the number of bits to 8 (and the number of VM bits to 24). The AVIC GATag is programmed into the AMD IOMMU IRTE to provide a reference back to KVM in case the IOMMU cannot inject an interrupt into a non-running vCPU. In such a case, the IOMMU notifies software by creating a GALog entry with the corresponded GATag, and KVM then uses the GATag to find the correct VM+vCPU to kick. Dropping bit 8 from the GATag results in kicking the wrong vCPU when targeting vCPUs with x2APIC ID > 255. Fixes: 4d1d7942e36a ("KVM: SVM: Introduce logic to (de)activate x2AVIC mode") Cc: stable@vger.kernel.org Reported-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> Co-developed-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/svm/avic.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
Comments
On Tue, 7 Feb 2023 00:21:55 +0000 Sean Christopherson <seanjc@google.com> wrote: > From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > Define AVIC_VCPU_ID_MASK based on AVIC_PHYSICAL_MAX_INDEX, i.e. the mask > that effectively controls the largest guest physical APIC ID supported by > x2AVIC, instead of hardcoding the number of bits to 8 (and the number of > VM bits to 24). Is there any particular reason not to tie it to max supported by KVM KVM_MAX_VCPU_IDS? Another question: will guest fail to start when configured with more than 512 vCPUs or it will start broken? > > The AVIC GATag is programmed into the AMD IOMMU IRTE to provide a > reference back to KVM in case the IOMMU cannot inject an interrupt into a > non-running vCPU. In such a case, the IOMMU notifies software by creating > a GALog entry with the corresponded GATag, and KVM then uses the GATag to > find the correct VM+vCPU to kick. Dropping bit 8 from the GATag results > in kicking the wrong vCPU when targeting vCPUs with x2APIC ID > 255. > > Fixes: 4d1d7942e36a ("KVM: SVM: Introduce logic to (de)activate x2AVIC mode") > Cc: stable@vger.kernel.org > Reported-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > Co-developed-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/svm/avic.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index ca684979e90d..326341a22153 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -27,19 +27,29 @@ > #include "irq.h" > #include "svm.h" > > -/* AVIC GATAG is encoded using VM and VCPU IDs */ > -#define AVIC_VCPU_ID_BITS 8 > -#define AVIC_VCPU_ID_MASK ((1 << AVIC_VCPU_ID_BITS) - 1) > +/* > + * Encode the arbitrary VM ID and the vCPU's default APIC ID, i.e the vCPU ID, > + * into the GATag so that KVM can retrieve the correct vCPU from a GALog entry > + * if an interrupt can't be delivered, e.g. because the vCPU isn't running. > + * > + * For the vCPU ID, use however many bits are currently allowed for the max > + * guest physical APIC ID (limited by the size of the physical ID table), and > + * use whatever bits remain to assign arbitrary AVIC IDs to VMs. Note, the > + * size of the GATag is defined by hardware (32 bits), but is an opaque value > + * as far as hardware is concerned. > + */ > +#define AVIC_VCPU_ID_MASK AVIC_PHYSICAL_MAX_INDEX_MASK > > -#define AVIC_VM_ID_BITS 24 > -#define AVIC_VM_ID_NR (1 << AVIC_VM_ID_BITS) > -#define AVIC_VM_ID_MASK ((1 << AVIC_VM_ID_BITS) - 1) > +#define AVIC_VM_ID_SHIFT HWEIGHT32(AVIC_PHYSICAL_MAX_INDEX_MASK) > +#define AVIC_VM_ID_MASK (GENMASK(31, AVIC_VM_ID_SHIFT) >> AVIC_VM_ID_SHIFT) > > -#define AVIC_GATAG(x, y) (((x & AVIC_VM_ID_MASK) << AVIC_VCPU_ID_BITS) | \ > +#define AVIC_GATAG(x, y) (((x & AVIC_VM_ID_MASK) << AVIC_VM_ID_SHIFT) | \ > (y & AVIC_VCPU_ID_MASK)) > -#define AVIC_GATAG_TO_VMID(x) ((x >> AVIC_VCPU_ID_BITS) & AVIC_VM_ID_MASK) > +#define AVIC_GATAG_TO_VMID(x) ((x >> AVIC_VM_ID_SHIFT) & AVIC_VM_ID_MASK) > #define AVIC_GATAG_TO_VCPUID(x) (x & AVIC_VCPU_ID_MASK) > > +static_assert(AVIC_GATAG(AVIC_VM_ID_MASK, AVIC_VCPU_ID_MASK) == -1u); > + > static bool force_avic; > module_param_unsafe(force_avic, bool, 0444); >
On 07/02/2023 08:33, Igor Mammedov wrote: > On Tue, 7 Feb 2023 00:21:55 +0000 > Sean Christopherson <seanjc@google.com> wrote: > >> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> >> >> Define AVIC_VCPU_ID_MASK based on AVIC_PHYSICAL_MAX_INDEX, i.e. the mask >> that effectively controls the largest guest physical APIC ID supported by >> x2AVIC, instead of hardcoding the number of bits to 8 (and the number of >> VM bits to 24). > > Is there any particular reason not to tie it to max supported by KVM > KVM_MAX_VCPU_IDS? > > Another question: > will guest fail to start when configured with more than 512 vCPUs > or it will start broken? > I think the problem is not so much the GATag (which can really be anything at the resolution you want). It's more of an SVM limit AIUI. Provided you can't have GATAgs if you don't have guest-mode/AVIC active, then makes sense have the same limit on both. SVM seems to be limited to 256 vcpus in xAPIC mode or 512 vcpus in x2APIC mode[0]. IIUC You actually won't be able to create guests with more than 512vcpus as KVM bound checks those max limits very early in the vCPU init (see avic_init_vcpu()). I guess the alternative would an AVIC inhibit if vCPU count goes beyond those limits -- probably a must have once avic flips to 1 by default like Intel. [0] in APM Volume 2 15.29.4.3 Physical Address Pointer Restrictions, * All the addresses point to 4-Kbyte aligned data structures. Bits 11:0 are reserved (except for offset 0F8h) and should be set to zero. The lower 8 bits of offset 0F8h are used for the field AVIC_PHYSICAL_MAX_INDEX. VMRUN fails with #VMEXIT(VMEXIT_INVALID) if AVIC_PHYSICAL_MAX_INDEX is greater than 255 in xAVIC mode or greater than 511 in x2AVIC mode.
On Tue, Feb 07, 2023, Joao Martins wrote: > On 07/02/2023 08:33, Igor Mammedov wrote: > > On Tue, 7 Feb 2023 00:21:55 +0000 > > Sean Christopherson <seanjc@google.com> wrote: > > > >> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > >> > >> Define AVIC_VCPU_ID_MASK based on AVIC_PHYSICAL_MAX_INDEX, i.e. the mask > >> that effectively controls the largest guest physical APIC ID supported by > >> x2AVIC, instead of hardcoding the number of bits to 8 (and the number of > >> VM bits to 24). > > > > Is there any particular reason not to tie it to max supported by KVM > > KVM_MAX_VCPU_IDS? > > > > Another question: > > will guest fail to start when configured with more than 512 vCPUs > > or it will start broken? > > > > I think the problem is not so much the GATag (which can really be anything at > the resolution you want). It's more of an SVM limit AIUI. Provided you can't > have GATAgs if you don't have guest-mode/AVIC active, then makes sense have the > same limit on both. Yep. The physical ID table, which is needed to achieve full AVIC benefits for a vCPU, is a single 4KiB page that holds 512 64-bit entries. AIUI, the GATag is used if and only if the interrupt target is in the physical ID table, so using more GATag bits for vCPU ID is pointless. > SVM seems to be limited to 256 vcpus in xAPIC mode or 512 vcpus in x2APIC > mode[0]. IIUC You actually won't be able to create guests with more than > 512vcpus as KVM bound checks those max limits very early in the vCPU init (see > avic_init_vcpu()). I guess the alternative would an AVIC inhibit if vCPU count > goes beyond those limits -- probably a must have once avic flips to 1 by default > like Intel. I don't _think_ KVM would have to explicitly inhibit AVIC. I believe the fallout would be that vCPUs >= 512 would simply not be eligible for virtual interrupt delivery, e.g. KVM would get a "Invalid Target in IPI" exit. I haven't dug into the IOMMU side of things though, so it's possible something in that world would necessitate disabling (x2)AVIC. > [0] in APM Volume 2 15.29.4.3 Physical Address Pointer Restrictions, > > * All the addresses point to 4-Kbyte aligned data structures. Bits 11:0 are > reserved (except for offset 0F8h) and should be set to zero. The lower 8 bits of > offset 0F8h are used for the field AVIC_PHYSICAL_MAX_INDEX. VMRUN fails with > #VMEXIT(VMEXIT_INVALID) if AVIC_PHYSICAL_MAX_INDEX is greater than 255 in xAVIC > mode or greater than 511 in x2AVIC mode.
On 2/7/2023 7:21 AM, Sean Christopherson wrote: > From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > Define AVIC_VCPU_ID_MASK based on AVIC_PHYSICAL_MAX_INDEX, i.e. the mask > that effectively controls the largest guest physical APIC ID supported by > x2AVIC, instead of hardcoding the number of bits to 8 (and the number of > VM bits to 24). > > The AVIC GATag is programmed into the AMD IOMMU IRTE to provide a > reference back to KVM in case the IOMMU cannot inject an interrupt into a > non-running vCPU. In such a case, the IOMMU notifies software by creating > a GALog entry with the corresponded GATag, and KVM then uses the GATag to > find the correct VM+vCPU to kick. Dropping bit 8 from the GATag results > in kicking the wrong vCPU when targeting vCPUs with x2APIC ID > 255. > > Fixes: 4d1d7942e36a ("KVM: SVM: Introduce logic to (de)activate x2AVIC mode") > Cc: stable@vger.kernel.org > Reported-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com> > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > Co-developed-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/svm/avic.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index ca684979e90d..326341a22153 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -27,19 +27,29 @@ > #include "irq.h" > #include "svm.h" > > -/* AVIC GATAG is encoded using VM and VCPU IDs */ > -#define AVIC_VCPU_ID_BITS 8 > -#define AVIC_VCPU_ID_MASK ((1 << AVIC_VCPU_ID_BITS) - 1) > +/* > + * Encode the arbitrary VM ID and the vCPU's default APIC ID, i.e the vCPU ID, > + * into the GATag so that KVM can retrieve the correct vCPU from a GALog entry > + * if an interrupt can't be delivered, e.g. because the vCPU isn't running. > + * > + * For the vCPU ID, use however many bits are currently allowed for the max > + * guest physical APIC ID (limited by the size of the physical ID table), and > + * use whatever bits remain to assign arbitrary AVIC IDs to VMs. Note, the > + * size of the GATag is defined by hardware (32 bits), but is an opaque value > + * as far as hardware is concerned. > + */ > +#define AVIC_VCPU_ID_MASK AVIC_PHYSICAL_MAX_INDEX_MASK > > -#define AVIC_VM_ID_BITS 24 > -#define AVIC_VM_ID_NR (1 << AVIC_VM_ID_BITS) > -#define AVIC_VM_ID_MASK ((1 << AVIC_VM_ID_BITS) - 1) > +#define AVIC_VM_ID_SHIFT HWEIGHT32(AVIC_PHYSICAL_MAX_INDEX_MASK) > +#define AVIC_VM_ID_MASK (GENMASK(31, AVIC_VM_ID_SHIFT) >> AVIC_VM_ID_SHIFT) > > -#define AVIC_GATAG(x, y) (((x & AVIC_VM_ID_MASK) << AVIC_VCPU_ID_BITS) | \ > +#define AVIC_GATAG(x, y) (((x & AVIC_VM_ID_MASK) << AVIC_VM_ID_SHIFT) | \ > (y & AVIC_VCPU_ID_MASK)) > -#define AVIC_GATAG_TO_VMID(x) ((x >> AVIC_VCPU_ID_BITS) & AVIC_VM_ID_MASK) > +#define AVIC_GATAG_TO_VMID(x) ((x >> AVIC_VM_ID_SHIFT) & AVIC_VM_ID_MASK) > #define AVIC_GATAG_TO_VCPUID(x) (x & AVIC_VCPU_ID_MASK) > > +static_assert(AVIC_GATAG(AVIC_VM_ID_MASK, AVIC_VCPU_ID_MASK) == -1u); > + > static bool force_avic; > module_param_unsafe(force_avic, bool, 0444); > Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> Thanks, Suravee
On 2/7/2023 11:38 PM, Sean Christopherson wrote: > On Tue, Feb 07, 2023, Joao Martins wrote: >> On 07/02/2023 08:33, Igor Mammedov wrote: >>> On Tue, 7 Feb 2023 00:21:55 +0000 >>> Sean Christopherson <seanjc@google.com> wrote: >>> >>>> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> >>>> >>>> Define AVIC_VCPU_ID_MASK based on AVIC_PHYSICAL_MAX_INDEX, i.e. the mask >>>> that effectively controls the largest guest physical APIC ID supported by >>>> x2AVIC, instead of hardcoding the number of bits to 8 (and the number of >>>> VM bits to 24). >>> >>> Is there any particular reason not to tie it to max supported by KVM >>> KVM_MAX_VCPU_IDS? >>> >>> Another question: >>> will guest fail to start when configured with more than 512 vCPUs >>> or it will start broken? >>> >> >> I think the problem is not so much the GATag (which can really be anything at >> the resolution you want). It's more of an SVM limit AIUI. Provided you can't >> have GATAgs if you don't have guest-mode/AVIC active, then makes sense have the >> same limit on both. Correct. > Yep. The physical ID table, which is needed to achieve full AVIC benefits for a > vCPU, is a single 4KiB page that holds 512 64-bit entries. AIUI, the GATag is > used if and only if the interrupt target is in the physical ID table, so using > more GATag bits for vCPU ID is pointless. Correct. >> SVM seems to be limited to 256 vcpus in xAPIC mode or 512 vcpus in x2APIC >> mode[0]. IIUC You actually won't be able to create guests with more than >> 512vcpus as KVM bound checks those max limits very early in the vCPU init (see >> avic_init_vcpu()). I guess the alternative would an AVIC inhibit if vCPU count >> goes beyond those limits -- probably a must have once avic flips to 1 by default >> like Intel. > > I don't _think_ KVM would have to explicitly inhibit AVIC. I believe the fallout > would be that vCPUs >= 512 would simply not be eligible for virtual interrupt > delivery, e.g. KVM would get a "Invalid Target in IPI" exit. I haven't dug into > the IOMMU side of things though, so it's possible something in that world would > necessitate disabling (x2)AVIC. SVM-AVIC is independent of the IOMMU-AVIC. We can enable SVM-AVIC, and use the legacy IOMMU interrupt remapping mode IRTE[GuestMode]=0. However, I have not explored the case of combining of the two modes. I can look into it and experiment with this case. Thanks, Suravee >> [0] in APM Volume 2 15.29.4.3 Physical Address Pointer Restrictions, >> >> * All the addresses point to 4-Kbyte aligned data structures. Bits 11:0 are >> reserved (except for offset 0F8h) and should be set to zero. The lower 8 bits of >> offset 0F8h are used for the field AVIC_PHYSICAL_MAX_INDEX. VMRUN fails with >> #VMEXIT(VMEXIT_INVALID) if AVIC_PHYSICAL_MAX_INDEX is greater than 255 in xAVIC >> mode or greater than 511 in x2AVIC mode.
diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c index ca684979e90d..326341a22153 100644 --- a/arch/x86/kvm/svm/avic.c +++ b/arch/x86/kvm/svm/avic.c @@ -27,19 +27,29 @@ #include "irq.h" #include "svm.h" -/* AVIC GATAG is encoded using VM and VCPU IDs */ -#define AVIC_VCPU_ID_BITS 8 -#define AVIC_VCPU_ID_MASK ((1 << AVIC_VCPU_ID_BITS) - 1) +/* + * Encode the arbitrary VM ID and the vCPU's default APIC ID, i.e the vCPU ID, + * into the GATag so that KVM can retrieve the correct vCPU from a GALog entry + * if an interrupt can't be delivered, e.g. because the vCPU isn't running. + * + * For the vCPU ID, use however many bits are currently allowed for the max + * guest physical APIC ID (limited by the size of the physical ID table), and + * use whatever bits remain to assign arbitrary AVIC IDs to VMs. Note, the + * size of the GATag is defined by hardware (32 bits), but is an opaque value + * as far as hardware is concerned. + */ +#define AVIC_VCPU_ID_MASK AVIC_PHYSICAL_MAX_INDEX_MASK -#define AVIC_VM_ID_BITS 24 -#define AVIC_VM_ID_NR (1 << AVIC_VM_ID_BITS) -#define AVIC_VM_ID_MASK ((1 << AVIC_VM_ID_BITS) - 1) +#define AVIC_VM_ID_SHIFT HWEIGHT32(AVIC_PHYSICAL_MAX_INDEX_MASK) +#define AVIC_VM_ID_MASK (GENMASK(31, AVIC_VM_ID_SHIFT) >> AVIC_VM_ID_SHIFT) -#define AVIC_GATAG(x, y) (((x & AVIC_VM_ID_MASK) << AVIC_VCPU_ID_BITS) | \ +#define AVIC_GATAG(x, y) (((x & AVIC_VM_ID_MASK) << AVIC_VM_ID_SHIFT) | \ (y & AVIC_VCPU_ID_MASK)) -#define AVIC_GATAG_TO_VMID(x) ((x >> AVIC_VCPU_ID_BITS) & AVIC_VM_ID_MASK) +#define AVIC_GATAG_TO_VMID(x) ((x >> AVIC_VM_ID_SHIFT) & AVIC_VM_ID_MASK) #define AVIC_GATAG_TO_VCPUID(x) (x & AVIC_VCPU_ID_MASK) +static_assert(AVIC_GATAG(AVIC_VM_ID_MASK, AVIC_VCPU_ID_MASK) == -1u); + static bool force_avic; module_param_unsafe(force_avic, bool, 0444);