* [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710
@ 2021-09-03 21:15 Eduardo Habkost
2021-09-03 21:15 ` [PATCH v2 1/3] kvm: x86: Set KVM_MAX_VCPU_ID to 4*KVM_MAX_VCPUS Eduardo Habkost
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Eduardo Habkost @ 2021-09-03 21:15 UTC (permalink / raw)
To: kvm, Paolo Bonzini; +Cc: Sean Christopherson, Juergen Gross, Vitaly Kuznetsov
This:
- Increases KVM_MAX_VCPUS 288 to 1024.
- Increases KVM_MAX_VCPU_ID from 1023 to 4096.
- Increases KVM_SOFT_MAX_VCPUS from 240 to 710.
Note that this conflicts with:
https://lore.kernel.org/lkml/20210903130808.30142-1-jgross@suse.com
Date: Fri, 3 Sep 2021 15:08:01 +0200
From: Juergen Gross <jgross@suse.com>
Subject: [PATCH v2 0/6] x86/kvm: add boot parameters for max vcpu configs
Message-Id: <20210903130808.30142-1-jgross@suse.com>
I don't intend to block Juergen's work. I will be happy to
rebase and resubmit in case Juergen's series is merged first.
However, I do propose that we set the values above as the default
even if Juergen's series is merged.
The additional overhead (on x86_64) of the new defaults will be:
- struct kvm_ioapic will grow from 1628 to 5084 bytes.
- struct kvm will grow from 19328 to 22272 bytes.
- Bitmap stack variables that will grow:
- At kvm_hv_flush_tlb() & kvm_hv_send_ipi(),
vp_bitmap[] and vcpu_bitmap[] will now be 128 bytes long
- vcpu_bitmap at bioapic_write_indirect() will be 128 bytes long
once patch "KVM: x86: Fix stack-out-of-bounds memory access
from ioapic_write_indirect()" is applied
Changes v1 -> v2:
* KVM_MAX_VCPUS is now 1024 (v1 set it to 710)
* KVM_MAX_VCPU_ID is now 4096 (v1 left it unchanged, at 1023)
v1 of this series was:
https://lore.kernel.org/lkml/20210831204535.1594297-1-ehabkost@redhat.com
Date: Tue, 31 Aug 2021 16:45:35 -0400
From: Eduardo Habkost <ehabkost@redhat.com>
Subject: [PATCH] kvm: x86: Increase MAX_VCPUS to 710
Message-Id: <20210831204535.1594297-1-ehabkost@redhat.com>
Eduardo Habkost (3):
kvm: x86: Set KVM_MAX_VCPU_ID to 4*KVM_MAX_VCPUS
kvm: x86: Increase MAX_VCPUS to 1024
kvm: x86: Increase KVM_SOFT_MAX_VCPUS to 710
arch/x86/include/asm/kvm_host.h | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2 1/3] kvm: x86: Set KVM_MAX_VCPU_ID to 4*KVM_MAX_VCPUS 2021-09-03 21:15 [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 Eduardo Habkost @ 2021-09-03 21:15 ` Eduardo Habkost 2021-09-03 21:15 ` [PATCH v2 2/3] kvm: x86: Increase MAX_VCPUS to 1024 Eduardo Habkost ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Eduardo Habkost @ 2021-09-03 21:15 UTC (permalink / raw) To: kvm, Paolo Bonzini; +Cc: Sean Christopherson, Juergen Gross, Vitaly Kuznetsov Instead of requiring KVM_MAX_VCPU_ID to be manually increased every time we increase KVM_MAX_VCPUS, set it to 4*KVM_MAX_VCPUS. This should be enough for CPU topologies where Cores-per-Package and Packages-per-Socket are not powers of 2. In practice, this increases KVM_MAX_VCPU_ID from 1023 to 1152. The only side effect of this change is making some fields in struct kvm_ioapic larger, increasing the struct size from 1628 to 1780 bytes (in x86_64). Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Note: this conflicts with: https://lore.kernel.org/lkml/20210903130808.30142-3-jgross@suse.com Date: Fri, 3 Sep 2021 15:08:03 +0200 From: Juergen Gross <jgross@suse.com> Subject: [PATCH v2 2/6] x86/kvm: add boot parameter for adding vcpu-id bits Message-Id: <20210903130808.30142-3-jgross@suse.com> I would be happy to drop this patch and resubmit the series if Juergen's series gets merged first. This is part of this series only because I'm not sure how the final version of Juergen's changes will look like and how long they will take to be merged. --- arch/x86/include/asm/kvm_host.h | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index af6ce8d4c86a..f4cbc08b8d4d 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -39,7 +39,19 @@ #define KVM_MAX_VCPUS 288 #define KVM_SOFT_MAX_VCPUS 240 -#define KVM_MAX_VCPU_ID 1023 + +/* + * In x86, the VCPU ID corresponds to the APIC ID, and APIC IDs + * might be larger than the actual number of VCPUs because the + * APIC ID encodes CPU topology information. + * + * In the worst case, we'll need less than one extra bit for the + * Core ID, and less than one extra bit for the Package (Die) ID, + * so ratio of 4 should be enough. + */ +#define KVM_VCPU_ID_RATIO 4 +#define KVM_MAX_VCPU_ID (KVM_MAX_VCPUS * KVM_VCPU_ID_RATIO) + /* memory slots that are not exposed to userspace */ #define KVM_PRIVATE_MEM_SLOTS 3 -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] kvm: x86: Increase MAX_VCPUS to 1024 2021-09-03 21:15 [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 Eduardo Habkost 2021-09-03 21:15 ` [PATCH v2 1/3] kvm: x86: Set KVM_MAX_VCPU_ID to 4*KVM_MAX_VCPUS Eduardo Habkost @ 2021-09-03 21:15 ` Eduardo Habkost 2021-09-03 21:16 ` [PATCH v2 3/3] kvm: x86: Increase KVM_SOFT_MAX_VCPUS to 710 Eduardo Habkost 2021-09-06 10:12 ` [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 Paolo Bonzini 3 siblings, 0 replies; 8+ messages in thread From: Eduardo Habkost @ 2021-09-03 21:15 UTC (permalink / raw) To: kvm, Paolo Bonzini; +Cc: Sean Christopherson, Juergen Gross, Vitaly Kuznetsov Increase KVM_MAX_VCPUS to 1024, so we can test larger VMs. I'm not changing KVM_SOFT_MAX_VCPUS yet because I'm afraid it might involve complicated questions around the meaning of "supported" and "recommended" in the upstream tree. KVM_SOFT_MAX_VCPUS will be changed in a separate patch. For reference, visible effects of this change are: - KVM_CAP_MAX_VCPUS will now return 1024 (of course) - Default value for CPUID[HYPERV_CPUID_IMPLEMENT_LIMITS (00x40000005)].EAX will now be 1024 - KVM_MAX_VCPU_ID will change from 1151 to 4096 - Size of struct kvm will increase from 19328 to 22272 bytes (in x86_64) - Size of struct kvm_ioapic will increase from 1780 to 5084 bytes (in x86_64) - Bitmap stack variables that will grow: - At kvm_hv_flush_tlb() kvm_hv_send_ipi(), vp_bitmap[] and vcpu_bitmap[] will now be 128 bytes long - vcpu_bitmap at bioapic_write_indirect() will be 128 bytes long once patch "KVM: x86: Fix stack-out-of-bounds memory access from ioapic_write_indirect()" is applied Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Note: this conflicts with: https://lore.kernel.org/lkml/20210903130808.30142-7-jgross@suse.com Date: Fri, 3 Sep 2021 15:08:07 +0200 From: Juergen Gross <jgross@suse.com> Subject: [PATCH v2 6/6] x86/kvm: add boot parameter for setting max number of vcpus per guest Message-Id: <20210903130808.30142-7-jgross@suse.com> I don't intend this to block Juergen's work. If Juergen's series is merged first, I can redo this patch to change KVM_DEFAULT_MAX_VCPUS instead. --- arch/x86/include/asm/kvm_host.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index f4cbc08b8d4d..e30e40399092 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -37,7 +37,7 @@ #define __KVM_HAVE_ARCH_VCPU_DEBUGFS -#define KVM_MAX_VCPUS 288 +#define KVM_MAX_VCPUS 1024 #define KVM_SOFT_MAX_VCPUS 240 /* -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] kvm: x86: Increase KVM_SOFT_MAX_VCPUS to 710 2021-09-03 21:15 [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 Eduardo Habkost 2021-09-03 21:15 ` [PATCH v2 1/3] kvm: x86: Set KVM_MAX_VCPU_ID to 4*KVM_MAX_VCPUS Eduardo Habkost 2021-09-03 21:15 ` [PATCH v2 2/3] kvm: x86: Increase MAX_VCPUS to 1024 Eduardo Habkost @ 2021-09-03 21:16 ` Eduardo Habkost 2021-09-06 10:12 ` [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 Paolo Bonzini 3 siblings, 0 replies; 8+ messages in thread From: Eduardo Habkost @ 2021-09-03 21:16 UTC (permalink / raw) To: kvm, Paolo Bonzini; +Cc: Sean Christopherson, Juergen Gross, Vitaly Kuznetsov Support for 710 VCPUs was tested by Red Hat since RHEL-8.4, so increase KVM_SOFT_MAX_VCPUS to 710. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Note: I would like to get rid of KVM_SOFT_MAX_VCPUS eventually, but I am simply bumping it to 710 by now, so we can at least declare 710 VCPUs as supported while we discuss alternatives to KVM_SOFT_MAX_VCPUS. --- arch/x86/include/asm/kvm_host.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index e30e40399092..98e8538cecbf 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -38,7 +38,7 @@ #define __KVM_HAVE_ARCH_VCPU_DEBUGFS #define KVM_MAX_VCPUS 1024 -#define KVM_SOFT_MAX_VCPUS 240 +#define KVM_SOFT_MAX_VCPUS 710 /* * In x86, the VCPU ID corresponds to the APIC ID, and APIC IDs -- 2.31.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 2021-09-03 21:15 [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 Eduardo Habkost ` (2 preceding siblings ...) 2021-09-03 21:16 ` [PATCH v2 3/3] kvm: x86: Increase KVM_SOFT_MAX_VCPUS to 710 Eduardo Habkost @ 2021-09-06 10:12 ` Paolo Bonzini 2021-09-11 0:30 ` Sean Christopherson 3 siblings, 1 reply; 8+ messages in thread From: Paolo Bonzini @ 2021-09-06 10:12 UTC (permalink / raw) To: Eduardo Habkost, kvm; +Cc: Sean Christopherson, Juergen Gross, Vitaly Kuznetsov On 03/09/21 23:15, Eduardo Habkost wrote: > This: > - Increases KVM_MAX_VCPUS 288 to 1024. > - Increases KVM_MAX_VCPU_ID from 1023 to 4096. > - Increases KVM_SOFT_MAX_VCPUS from 240 to 710. > > Note that this conflicts with: > https://lore.kernel.org/lkml/20210903130808.30142-1-jgross@suse.com > Date: Fri, 3 Sep 2021 15:08:01 +0200 > From: Juergen Gross <jgross@suse.com> > Subject: [PATCH v2 0/6] x86/kvm: add boot parameters for max vcpu configs > Message-Id: <20210903130808.30142-1-jgross@suse.com> > > I don't intend to block Juergen's work. I will be happy to > rebase and resubmit in case Juergen's series is merged first. > However, I do propose that we set the values above as the default > even if Juergen's series is merged. > > The additional overhead (on x86_64) of the new defaults will be: > - struct kvm_ioapic will grow from 1628 to 5084 bytes. > - struct kvm will grow from 19328 to 22272 bytes. > - Bitmap stack variables that will grow: > - At kvm_hv_flush_tlb() & kvm_hv_send_ipi(), > vp_bitmap[] and vcpu_bitmap[] will now be 128 bytes long > - vcpu_bitmap at bioapic_write_indirect() will be 128 bytes long > once patch "KVM: x86: Fix stack-out-of-bounds memory access > from ioapic_write_indirect()" is applied > > Changes v1 -> v2: > * KVM_MAX_VCPUS is now 1024 (v1 set it to 710) > * KVM_MAX_VCPU_ID is now 4096 (v1 left it unchanged, at 1023) > > v1 of this series was: > > https://lore.kernel.org/lkml/20210831204535.1594297-1-ehabkost@redhat.com > Date: Tue, 31 Aug 2021 16:45:35 -0400 > From: Eduardo Habkost <ehabkost@redhat.com> > Subject: [PATCH] kvm: x86: Increase MAX_VCPUS to 710 > Message-Id: <20210831204535.1594297-1-ehabkost@redhat.com> > > Eduardo Habkost (3): > kvm: x86: Set KVM_MAX_VCPU_ID to 4*KVM_MAX_VCPUS > kvm: x86: Increase MAX_VCPUS to 1024 > kvm: x86: Increase KVM_SOFT_MAX_VCPUS to 710 > > arch/x86/include/asm/kvm_host.h | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > Queued, thanks. Paolo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 2021-09-06 10:12 ` [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 Paolo Bonzini @ 2021-09-11 0:30 ` Sean Christopherson 2021-09-11 15:26 ` Eduardo Habkost 0 siblings, 1 reply; 8+ messages in thread From: Sean Christopherson @ 2021-09-11 0:30 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Eduardo Habkost, kvm, Juergen Gross, Vitaly Kuznetsov On Mon, Sep 06, 2021, Paolo Bonzini wrote: > On 03/09/21 23:15, Eduardo Habkost wrote: > > - Increases KVM_MAX_VCPU_ID from 1023 to 4096. ... > > Eduardo Habkost (3): > > kvm: x86: Set KVM_MAX_VCPU_ID to 4*KVM_MAX_VCPUS ... > > kvm: x86: Increase MAX_VCPUS to 1024 > > kvm: x86: Increase KVM_SOFT_MAX_VCPUS to 710 > > > > arch/x86/include/asm/kvm_host.h | 18 +++++++++++++++--- > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > Queued, thanks. Before we commit to this, can we sort out the off-by-one mess that is KVM_MAX_VCPU_ID? As Eduardo pointed out[*], Juergen's commit 76b4f357d0e7 ("x86/kvm: fix vcpu-id indexed array sizes") _shouldn't_ be necessary because kvm_vm_ioctl_create_vcpu() rejects attempts to create id==KVM_MAX_VCPU_ID if (id >= KVM_MAX_VCPU_ID) return -EINVAL; which aligns with the docs for KVM_CREATE_VCPU: The vcpu id is an integer in the range [0, max_vcpu_id) but the fix is "needed" because rtc_irq_eoi_tracking_reset() does bitmap_zero(ioapic->rtc_status.dest_map.map, KVM_MAX_VCPU_ID + 1); and now we have this DECLARE_BITMAP(map, KVM_MAX_VCPU_ID + 1); u8 vectors[KVM_MAX_VCPU_ID + 1]; which is wrong as well. The "right" fix would have been to change rtc_irq_eoi_tracking_reset(), but that looks all kinds of wrong on the surface. Non-x86 really mucks it up because generic code does: #ifndef KVM_MAX_VCPU_ID #define KVM_MAX_VCPU_ID KVM_MAX_VCPUS #endif which means pretty much everything else can create more vCPUs than vCPU IDs. Maybe fix KVM's internal KVM_MAX_VCPU_ID so that it's incluse, and handle the backwards compability mess in KVM_CAP_MAX_VCPU_ID? Then have the max ID for x86 be (4*KVM_MAX_VCPUS - 1). E.g. something like: diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index e5d5c5ed7dd4..2e5c8081f72b 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4061,7 +4061,8 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) r = KVM_MAX_VCPUS; break; case KVM_CAP_MAX_VCPU_ID: - r = KVM_MAX_VCPU_ID; + /* KVM's ABI is stupid. */ + r = KVM_MAX_VCPU_ID - 1; break; case KVM_CAP_PV_MMU: /* obsolete */ r = 0; diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index b50dbe269f4b..ba46c42a4a6f 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3460,7 +3460,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id) struct kvm_vcpu *vcpu; struct page *page; - if (id >= KVM_MAX_VCPU_ID) + if (id > KVM_MAX_VCPU_ID) return -EINVAL; mutex_lock(&kvm->lock); 17:23:40 ✔ ~/go/src/kernel.org/host $ gdd diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h index caaa0f592d8e..0292d8a5ce5e 100644 --- a/arch/powerpc/include/asm/kvm_book3s.h +++ b/arch/powerpc/include/asm/kvm_book3s.h @@ -434,7 +434,7 @@ extern int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu); #define SPLIT_HACK_OFFS 0xfb000000 /* - * This packs a VCPU ID from the [0..KVM_MAX_VCPU_ID) space down to the + * This packs a VCPU ID from the [0..KVM_MAX_VCPU_ID] space down to the * [0..KVM_MAX_VCPUS) space, using knowledge of the guest's core stride * (but not its actual threading mode, which is not available) to avoid * collisions. diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 9f52f282b1aa..beeebace8d1c 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -33,11 +33,11 @@ #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE #include <asm/kvm_book3s_asm.h> /* for MAX_SMT_THREADS */ -#define KVM_MAX_VCPU_ID (MAX_SMT_THREADS * KVM_MAX_VCORES) +#define KVM_MAX_VCPU_ID (MAX_SMT_THREADS * KVM_MAX_VCORES) - 1 #define KVM_MAX_NESTED_GUESTS KVMPPC_NR_LPIDS #else -#define KVM_MAX_VCPU_ID KVM_MAX_VCPUS +#define KVM_MAX_VCPU_ID KVM_MAX_VCPUS - 1 #endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */ #define __KVM_HAVE_ARCH_INTC_INITIALIZED diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index e5d5c5ed7dd4..5c20c0bd4db6 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4061,7 +4061,8 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) r = KVM_MAX_VCPUS; break; case KVM_CAP_MAX_VCPU_ID: - r = KVM_MAX_VCPU_ID; + /* KVM's ABI is stupid. */ + r = KVM_MAX_VCPU_ID + 1; break; case KVM_CAP_PV_MMU: /* obsolete */ r = 0; diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index ae7735b490b4..37ef972caf4b 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -40,7 +40,7 @@ #include <linux/kvm_dirty_ring.h> #ifndef KVM_MAX_VCPU_ID -#define KVM_MAX_VCPU_ID KVM_MAX_VCPUS +#define KVM_MAX_VCPU_ID KVM_MAX_VCPUS - 1 #endif /* diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index b50dbe269f4b..ba46c42a4a6f 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3460,7 +3460,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id) struct kvm_vcpu *vcpu; struct page *page; - if (id >= KVM_MAX_VCPU_ID) + if (id > KVM_MAX_VCPU_ID) return -EINVAL; mutex_lock(&kvm->lock); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 2021-09-11 0:30 ` Sean Christopherson @ 2021-09-11 15:26 ` Eduardo Habkost 2021-09-13 5:09 ` Juergen Gross 0 siblings, 1 reply; 8+ messages in thread From: Eduardo Habkost @ 2021-09-11 15:26 UTC (permalink / raw) To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, Juergen Gross, Vitaly Kuznetsov On Fri, Sep 10, 2021 at 8:30 PM Sean Christopherson <seanjc@google.com> wrote: > > On Mon, Sep 06, 2021, Paolo Bonzini wrote: > > On 03/09/21 23:15, Eduardo Habkost wrote: > > > - Increases KVM_MAX_VCPU_ID from 1023 to 4096. > > ... > > > > Eduardo Habkost (3): > > > kvm: x86: Set KVM_MAX_VCPU_ID to 4*KVM_MAX_VCPUS > > ... > > > > kvm: x86: Increase MAX_VCPUS to 1024 > > > kvm: x86: Increase KVM_SOFT_MAX_VCPUS to 710 > > > > > > arch/x86/include/asm/kvm_host.h | 18 +++++++++++++++--- > > > 1 file changed, 15 insertions(+), 3 deletions(-) > > > > > > > Queued, thanks. > > Before we commit to this, can we sort out the off-by-one mess that is KVM_MAX_VCPU_ID? > As Eduardo pointed out[*], Juergen's commit 76b4f357d0e7 ("x86/kvm: fix vcpu-id > indexed array sizes") _shouldn't_ be necessary because kvm_vm_ioctl_create_vcpu() > rejects attempts to create id==KVM_MAX_VCPU_ID > > if (id >= KVM_MAX_VCPU_ID) > return -EINVAL; > > which aligns with the docs for KVM_CREATE_VCPU: > > The vcpu id is an integer in the range [0, max_vcpu_id) > > but the fix is "needed" because rtc_irq_eoi_tracking_reset() does > > bitmap_zero(ioapic->rtc_status.dest_map.map, KVM_MAX_VCPU_ID + 1); > > and now we have this > > DECLARE_BITMAP(map, KVM_MAX_VCPU_ID + 1); > u8 vectors[KVM_MAX_VCPU_ID + 1]; > > which is wrong as well. The "right" fix would have been to change > rtc_irq_eoi_tracking_reset(), but that looks all kinds of wrong on the surface. > > Non-x86 really mucks it up because generic code does: > > #ifndef KVM_MAX_VCPU_ID > #define KVM_MAX_VCPU_ID KVM_MAX_VCPUS > #endif > > which means pretty much everything else can create more vCPUs than vCPU IDs. > > Maybe fix KVM's internal KVM_MAX_VCPU_ID so that it's incluse, and handle the > backwards compability mess in KVM_CAP_MAX_VCPU_ID? Then have the max ID for x86 > be (4*KVM_MAX_VCPUS - 1). E.g. something like: Wouldn't it be simpler to just revert 76b4f357d0e7 and rename KVM_MAX_VCPU_ID to KVM_MAX_VCPU_IDS? As far as I can see, every single line of code in KVM (except the 3 lines touched by 76b4f357d0e7) treats both KVM_MAX_VCPU_ID and KVM_CAP_MAX_VCPU_ID as exclusive. Including the API documentation. > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index e5d5c5ed7dd4..2e5c8081f72b 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4061,7 +4061,8 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > r = KVM_MAX_VCPUS; > break; > case KVM_CAP_MAX_VCPU_ID: > - r = KVM_MAX_VCPU_ID; > + /* KVM's ABI is stupid. */ > + r = KVM_MAX_VCPU_ID - 1; > break; > case KVM_CAP_PV_MMU: /* obsolete */ > r = 0; > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index b50dbe269f4b..ba46c42a4a6f 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -3460,7 +3460,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id) > struct kvm_vcpu *vcpu; > struct page *page; > > - if (id >= KVM_MAX_VCPU_ID) > + if (id > KVM_MAX_VCPU_ID) > return -EINVAL; > > mutex_lock(&kvm->lock); > 17:23:40 ✔ ~/go/src/kernel.org/host $ gdd > diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h > index caaa0f592d8e..0292d8a5ce5e 100644 > --- a/arch/powerpc/include/asm/kvm_book3s.h > +++ b/arch/powerpc/include/asm/kvm_book3s.h > @@ -434,7 +434,7 @@ extern int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu); > #define SPLIT_HACK_OFFS 0xfb000000 > > /* > - * This packs a VCPU ID from the [0..KVM_MAX_VCPU_ID) space down to the > + * This packs a VCPU ID from the [0..KVM_MAX_VCPU_ID] space down to the > * [0..KVM_MAX_VCPUS) space, using knowledge of the guest's core stride > * (but not its actual threading mode, which is not available) to avoid > * collisions. > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 9f52f282b1aa..beeebace8d1c 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -33,11 +33,11 @@ > > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > #include <asm/kvm_book3s_asm.h> /* for MAX_SMT_THREADS */ > -#define KVM_MAX_VCPU_ID (MAX_SMT_THREADS * KVM_MAX_VCORES) > +#define KVM_MAX_VCPU_ID (MAX_SMT_THREADS * KVM_MAX_VCORES) - 1 > #define KVM_MAX_NESTED_GUESTS KVMPPC_NR_LPIDS > > #else > -#define KVM_MAX_VCPU_ID KVM_MAX_VCPUS > +#define KVM_MAX_VCPU_ID KVM_MAX_VCPUS - 1 > #endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */ > > #define __KVM_HAVE_ARCH_INTC_INITIALIZED > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index e5d5c5ed7dd4..5c20c0bd4db6 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4061,7 +4061,8 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > r = KVM_MAX_VCPUS; > break; > case KVM_CAP_MAX_VCPU_ID: > - r = KVM_MAX_VCPU_ID; > + /* KVM's ABI is stupid. */ > + r = KVM_MAX_VCPU_ID + 1; > break; > case KVM_CAP_PV_MMU: /* obsolete */ > r = 0; > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index ae7735b490b4..37ef972caf4b 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -40,7 +40,7 @@ > #include <linux/kvm_dirty_ring.h> > > #ifndef KVM_MAX_VCPU_ID > -#define KVM_MAX_VCPU_ID KVM_MAX_VCPUS > +#define KVM_MAX_VCPU_ID KVM_MAX_VCPUS - 1 > #endif > > /* > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index b50dbe269f4b..ba46c42a4a6f 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -3460,7 +3460,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id) > struct kvm_vcpu *vcpu; > struct page *page; > > - if (id >= KVM_MAX_VCPU_ID) > + if (id > KVM_MAX_VCPU_ID) > return -EINVAL; > > mutex_lock(&kvm->lock); > > -- Eduardo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 2021-09-11 15:26 ` Eduardo Habkost @ 2021-09-13 5:09 ` Juergen Gross 0 siblings, 0 replies; 8+ messages in thread From: Juergen Gross @ 2021-09-13 5:09 UTC (permalink / raw) To: Eduardo Habkost, Sean Christopherson; +Cc: Paolo Bonzini, kvm, Vitaly Kuznetsov [-- Attachment #1.1.1: Type: text/plain, Size: 406 bytes --] On 11.09.21 17:26, Eduardo Habkost wrote: > Wouldn't it be simpler to just revert 76b4f357d0e7 and rename > KVM_MAX_VCPU_ID to KVM_MAX_VCPU_IDS? > > As far as I can see, every single line of code in KVM (except the 3 > lines touched by 76b4f357d0e7) treats both KVM_MAX_VCPU_ID and > KVM_CAP_MAX_VCPU_ID as exclusive. Including the API documentation. I agree. Will send patches. Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3135 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-09-13 5:09 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-03 21:15 [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 Eduardo Habkost 2021-09-03 21:15 ` [PATCH v2 1/3] kvm: x86: Set KVM_MAX_VCPU_ID to 4*KVM_MAX_VCPUS Eduardo Habkost 2021-09-03 21:15 ` [PATCH v2 2/3] kvm: x86: Increase MAX_VCPUS to 1024 Eduardo Habkost 2021-09-03 21:16 ` [PATCH v2 3/3] kvm: x86: Increase KVM_SOFT_MAX_VCPUS to 710 Eduardo Habkost 2021-09-06 10:12 ` [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710 Paolo Bonzini 2021-09-11 0:30 ` Sean Christopherson 2021-09-11 15:26 ` Eduardo Habkost 2021-09-13 5:09 ` Juergen Gross
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).