From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>,
kvm@vger.kernel.org, Juergen Gross <jgross@suse.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>
Subject: Re: [PATCH v2 0/3] kvm: x86: Set KVM_MAX_VCPUS=1024, KVM_SOFT_MAX_VCPUS=710
Date: Sat, 11 Sep 2021 00:30:38 +0000 [thread overview]
Message-ID: <YTv4rgPol5vILWay@google.com> (raw)
In-Reply-To: <1111efc8-b32f-bd50-2c0f-4c6f506b544b@redhat.com>
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);
next prev parent reply other threads:[~2021-09-11 0:30 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2021-09-11 15:26 ` Eduardo Habkost
2021-09-13 5:09 ` Juergen Gross
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YTv4rgPol5vILWay@google.com \
--to=seanjc@google.com \
--cc=ehabkost@redhat.com \
--cc=jgross@suse.com \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=vkuznets@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).