[PATCH v5 0/2] let archs decide for vCPU ids

[PATCH v5 0/2] let archs decide for vCPU ids

[Greg Kurz]

Architectures can freely choose vCPU ids as long as they stay below
KVM_MAX_VCPUS, which is the maximum number of vCPUS. This is a problem
for PowerPC where the ids have to be multiples of the number of threads
per core in the host: when the host is POWER8 with 8 threads per core,
we can only have KVM_MAX_VCPUS / 8 in the guest.

This series decouplates the vCPU id limit from the number of vCPUs.

The first patch is a cleanup I kept from v4.

The second patch adds KVM_MAX_VCPU_ID as suggested by Radim.

---

Greg Kurz (2):
      KVM: remove NULL return path for vcpu ids >= KVM_MAX_VCPUS
      kvm: introduce KVM_MAX_VCPU_ID


 Documentation/virtual/kvm/api.txt   |   10 ++++++++--
 arch/powerpc/include/asm/kvm_host.h |    2 ++
 arch/powerpc/kvm/powerpc.c          |    3 +++
 include/linux/kvm_host.h            |   11 ++++++++---
 include/uapi/linux/kvm.h            |    1 +
 virt/kvm/kvm_main.c                 |    2 +-
 6 files changed, 23 insertions(+), 6 deletions(-)

--
Greg

[PATCH v5 1/2] KVM: remove NULL return path for vcpu ids >= KVM_MAX_VCPUS

[Greg Kurz]

Commit c896939f7cff ("KVM: use heuristic for fast VCPU lookup by id") added
a return path that prevents vcpu ids to exceed KVM_MAX_VCPUS. This is a
problem for powerpc where vcpu ids can grow up to 8*KVM_MAX_VCPUS.

This patch simply reverses the logic so that we only try fast path if the
vcpu id can be tried as an index in kvm->vcpus[]. The slow path is not
affected by the change.

Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
v5: - added David's and Conny's R-b tags
---
 include/linux/kvm_host.h |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 5276fe0916fc..23bfe1bd159c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -447,12 +447,13 @@ static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm *kvm, int i)
 
 static inline struct kvm_vcpu *kvm_get_vcpu_by_id(struct kvm *kvm, int id)
 {
-	struct kvm_vcpu *vcpu;
+	struct kvm_vcpu *vcpu = NULL;
 	int i;
 
-	if (id < 0 || id >= KVM_MAX_VCPUS)
+	if (id < 0)
 		return NULL;
-	vcpu = kvm_get_vcpu(kvm, id);
+	if (id < KVM_MAX_VCPUS)
+		vcpu = kvm_get_vcpu(kvm, id);
 	if (vcpu && vcpu->vcpu_id == id)
 		return vcpu;
 	kvm_for_each_vcpu(i, vcpu, kvm)

[PATCH v5 2/2] kvm: introduce KVM_MAX_VCPU_ID

[Greg Kurz]

The KVM_MAX_VCPUS define provides the maximum number of vCPUs per guest, and
also the upper limit for vCPU ids. This is okay for all archs except PowerPC
which can have higher ids, depending on the cpu/core/thread topology. In the
worst case (single threaded guest, host with 8 threads per core), it limits
the maximum number of vCPUS to KVM_MAX_VCPUS / 8.

This patch separates the vCPU numbering from the total number of vCPUs, with
the introduction of KVM_MAX_VCPU_ID, as the maximal valid value for vCPU ids
plus one.

The corresponding KVM_CAP_MAX_VCPU_ID allows userspace to validate vCPU ids
before passing them to KVM_CREATE_VCPU.

Only PowerPC gets unlimited vCPU ids for the moment. This patch doesn't
change anything for other archs.

Suggested-by: Radim Krcmar <rkrcmar@redhat.com>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 Documentation/virtual/kvm/api.txt   |   10 ++++++++--
 arch/powerpc/include/asm/kvm_host.h |    2 ++
 arch/powerpc/kvm/powerpc.c          |    3 +++
 include/linux/kvm_host.h            |    4 ++++
 include/uapi/linux/kvm.h            |    1 +
 virt/kvm/kvm_main.c                 |    2 +-
 6 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 4d0542c5206b..2da127f21ffc 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -199,8 +199,8 @@ Type: vm ioctl
 Parameters: vcpu id (apic id on x86)
 Returns: vcpu fd on success, -1 on error
 
-This API adds a vcpu to a virtual machine.  The vcpu id is a small integer
-in the range [0, max_vcpus).
+This API adds a vcpu to a virtual machine. No more than max_vcpus may be added.
+The vcpu id is an integer in the range [0, max_vcpu_id).
 
 The recommended max_vcpus value can be retrieved using the KVM_CAP_NR_VCPUS of
 the KVM_CHECK_EXTENSION ioctl() at run-time.
@@ -212,6 +212,12 @@ cpus max.
 If the KVM_CAP_MAX_VCPUS does not exist, you should assume that max_vcpus is
 same as the value returned from KVM_CAP_NR_VCPUS.
 
+The maximum possible value for max_vcpu_id can be retrieved using the
+KVM_CAP_MAX_VCPU_ID of the KVM_CHECK_EXTENSION ioctl() at run-time.
+
+If the KVM_CAP_MAX_VCPU_ID does not exist, you should assume that max_vcpu_id
+is the same as the value returned from KVM_CAP_MAX_VCPUS.
+
 On powerpc using book3s_hv mode, the vcpus are mapped onto virtual
 threads in one or more virtual CPU cores.  (This is because the
 hardware requires all the hardware threads in a CPU core to be in the
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index d7b343170453..6b4b78d6131b 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -39,6 +39,8 @@
 #define KVM_MAX_VCPUS		NR_CPUS
 #define KVM_MAX_VCORES		NR_CPUS
 #define KVM_USER_MEM_SLOTS	512
+#define KVM_MAX_VCPU_ID		INT_MAX
+
 
 #ifdef CONFIG_KVM_MMIO
 #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 6a68730774ee..bef0e6e5e8d0 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -580,6 +580,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_MAX_VCPUS:
 		r = KVM_MAX_VCPUS;
 		break;
+	case KVM_CAP_MAX_VCPU_ID:
+		r = KVM_MAX_VCPU_ID;
+		break;
 #ifdef CONFIG_PPC_BOOK3S_64
 	case KVM_CAP_PPC_GET_SMMU_INFO:
 		r = 1;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 23bfe1bd159c..3b4efa1c088c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -35,6 +35,10 @@
 
 #include <asm/kvm_host.h>
 
+#ifndef KVM_MAX_VCPU_ID
+#define KVM_MAX_VCPU_ID KVM_MAX_VCPUS
+#endif
+
 /*
  * The bit 16 ~ bit 31 of kvm_memory_region::flags are internally used
  * in kvm, other bits are visible for userspace which are defined in
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index a7f1f8032ec1..05ebf475104c 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -865,6 +865,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_SPAPR_TCE_64 125
 #define KVM_CAP_ARM_PMU_V3 126
 #define KVM_CAP_VCPU_ATTRIBUTES 127
+#define KVM_CAP_MAX_VCPU_ID 128
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4fd482fb9260..210ab88466fd 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2272,7 +2272,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
 	int r;
 	struct kvm_vcpu *vcpu;
 
-	if (id >= KVM_MAX_VCPUS)
+	if (id >= KVM_MAX_VCPU_ID)
 		return -EINVAL;
 
 	vcpu = kvm_arch_vcpu_create(kvm, id);

Re: [PATCH v5 2/2] kvm: introduce KVM_MAX_VCPU_ID

[Cornelia Huck]

On Tue, 03 May 2016 06:52:02 +0200
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:

> The KVM_MAX_VCPUS define provides the maximum number of vCPUs per guest, and
> also the upper limit for vCPU ids. This is okay for all archs except PowerPC
> which can have higher ids, depending on the cpu/core/thread topology. In the
> worst case (single threaded guest, host with 8 threads per core), it limits
> the maximum number of vCPUS to KVM_MAX_VCPUS / 8.
> 
> This patch separates the vCPU numbering from the total number of vCPUs, with
> the introduction of KVM_MAX_VCPU_ID, as the maximal valid value for vCPU ids
> plus one.
> 
> The corresponding KVM_CAP_MAX_VCPU_ID allows userspace to validate vCPU ids
> before passing them to KVM_CREATE_VCPU.
> 
> Only PowerPC gets unlimited vCPU ids for the moment. This patch doesn't
> change anything for other archs.
> 
> Suggested-by: Radim Krcmar <rkrcmar@redhat.com>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>  Documentation/virtual/kvm/api.txt   |   10 ++++++++--
>  arch/powerpc/include/asm/kvm_host.h |    2 ++
>  arch/powerpc/kvm/powerpc.c          |    3 +++
>  include/linux/kvm_host.h            |    4 ++++
>  include/uapi/linux/kvm.h            |    1 +
>  virt/kvm/kvm_main.c                 |    2 +-
>  6 files changed, 19 insertions(+), 3 deletions(-)
> 

> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 23bfe1bd159c..3b4efa1c088c 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -35,6 +35,10 @@
> 
>  #include <asm/kvm_host.h>
> 
> +#ifndef KVM_MAX_VCPU_ID
> +#define KVM_MAX_VCPU_ID KVM_MAX_VCPUS

As you are defining KVM_MAX_VCPU_ID in any case, would it make sense to
provide the cap in generic code? power will simply override the value.

> +#endif
> +
>  /*
>   * The bit 16 ~ bit 31 of kvm_memory_region::flags are internally used
>   * in kvm, other bits are visible for userspace which are defined in

Re: [PATCH v5 2/2] kvm: introduce KVM_MAX_VCPU_ID

[Greg Kurz]

On Tue, 3 May 2016 08:49:12 +0200
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> On Tue, 03 May 2016 06:52:02 +0200
> Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:
> 
> > The KVM_MAX_VCPUS define provides the maximum number of vCPUs per guest, and
> > also the upper limit for vCPU ids. This is okay for all archs except PowerPC
> > which can have higher ids, depending on the cpu/core/thread topology. In the
> > worst case (single threaded guest, host with 8 threads per core), it limits
> > the maximum number of vCPUS to KVM_MAX_VCPUS / 8.
> > 
> > This patch separates the vCPU numbering from the total number of vCPUs, with
> > the introduction of KVM_MAX_VCPU_ID, as the maximal valid value for vCPU ids
> > plus one.
> > 
> > The corresponding KVM_CAP_MAX_VCPU_ID allows userspace to validate vCPU ids
> > before passing them to KVM_CREATE_VCPU.
> > 
> > Only PowerPC gets unlimited vCPU ids for the moment. This patch doesn't
> > change anything for other archs.
> > 
> > Suggested-by: Radim Krcmar <rkrcmar@redhat.com>
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > ---
> >  Documentation/virtual/kvm/api.txt   |   10 ++++++++--
> >  arch/powerpc/include/asm/kvm_host.h |    2 ++
> >  arch/powerpc/kvm/powerpc.c          |    3 +++
> >  include/linux/kvm_host.h            |    4 ++++
> >  include/uapi/linux/kvm.h            |    1 +
> >  virt/kvm/kvm_main.c                 |    2 +-
> >  6 files changed, 19 insertions(+), 3 deletions(-)
> >   
> 
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index 23bfe1bd159c..3b4efa1c088c 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -35,6 +35,10 @@
> > 
> >  #include <asm/kvm_host.h>
> > 
> > +#ifndef KVM_MAX_VCPU_ID
> > +#define KVM_MAX_VCPU_ID KVM_MAX_VCPUS  
> 
> As you are defining KVM_MAX_VCPU_ID in any case, would it make sense to
> provide the cap in generic code? power will simply override the value.
> 

I'll do that in v6. Thanks !

> > +#endif
> > +
> >  /*
> >   * The bit 16 ~ bit 31 of kvm_memory_region::flags are internally used
> >   * in kvm, other bits are visible for userspace which are defined in  

Re: [PATCH v5 2/2] kvm: introduce KVM_MAX_VCPU_ID

[Radim Krčmář]

2016-05-03 06:52+0200, Greg Kurz:
> The KVM_MAX_VCPUS define provides the maximum number of vCPUs per guest, and
> also the upper limit for vCPU ids. This is okay for all archs except PowerPC
> which can have higher ids, depending on the cpu/core/thread topology. In the
> worst case (single threaded guest, host with 8 threads per core), it limits
> the maximum number of vCPUS to KVM_MAX_VCPUS / 8.
> 
> This patch separates the vCPU numbering from the total number of vCPUs, with
> the introduction of KVM_MAX_VCPU_ID, as the maximal valid value for vCPU ids
> plus one.
> 
> The corresponding KVM_CAP_MAX_VCPU_ID allows userspace to validate vCPU ids
> before passing them to KVM_CREATE_VCPU.
> 
> Only PowerPC gets unlimited vCPU ids for the moment. This patch doesn't
> change anything for other archs.
> 
> Suggested-by: Radim Krcmar <rkrcmar@redhat.com>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> @@ -2272,7 +2272,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
>  	int r;
>  	struct kvm_vcpu *vcpu;
>  
> -	if (id >= KVM_MAX_VCPUS)
> +	if (id >= KVM_MAX_VCPU_ID)
>  		return -EINVAL;

book3s_hv will currently fail with vcpu_id above threads_per_subcore *
KVM_MAX_VCORES, so userspace cannot use KVM_CAP_MAX_VCPU_ID to limit
vcpu_id ... I thought the check for vcpu_id would move to arch-specific
code, like the previous version did, to simplify implementation of a
dynamic limit.

The dynamic limit was too complicated to be worth it?
(This version is ok too.)

Re: [PATCH v5 2/2] kvm: introduce KVM_MAX_VCPU_ID

[Greg Kurz]

On Wed, 4 May 2016 18:45:34 +0200
Radim Krčmář <rkrcmar@redhat.com> wrote:

> 2016-05-03 06:52+0200, Greg Kurz:
> > The KVM_MAX_VCPUS define provides the maximum number of vCPUs per guest, and
> > also the upper limit for vCPU ids. This is okay for all archs except PowerPC
> > which can have higher ids, depending on the cpu/core/thread topology. In the
> > worst case (single threaded guest, host with 8 threads per core), it limits
> > the maximum number of vCPUS to KVM_MAX_VCPUS / 8.
> > 
> > This patch separates the vCPU numbering from the total number of vCPUs, with
> > the introduction of KVM_MAX_VCPU_ID, as the maximal valid value for vCPU ids
> > plus one.
> > 
> > The corresponding KVM_CAP_MAX_VCPU_ID allows userspace to validate vCPU ids
> > before passing them to KVM_CREATE_VCPU.
> > 
> > Only PowerPC gets unlimited vCPU ids for the moment. This patch doesn't
> > change anything for other archs.
> > 
> > Suggested-by: Radim Krcmar <rkrcmar@redhat.com>
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > ---
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > @@ -2272,7 +2272,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
> >  	int r;
> >  	struct kvm_vcpu *vcpu;
> >  
> > -	if (id >= KVM_MAX_VCPUS)
> > +	if (id >= KVM_MAX_VCPU_ID)
> >  		return -EINVAL;  
> 
> book3s_hv will currently fail with vcpu_id above threads_per_subcore *
> KVM_MAX_VCORES, so userspace cannot use KVM_CAP_MAX_VCPU_ID to limit
> vcpu_id ... 

You're right, I guess powerpc should return threads_per_subcore * KVM_MAX_VCORES
instead of INT_MAX then.

> I thought the check for vcpu_id would move to arch-specific
> code, like the previous version did, to simplify implementation of a
> dynamic limit.
> 
> The dynamic limit was too complicated to be worth it?
> (This version is ok too.)
> 

Actually no, it can be as simple as this in arch/powerpc/include/asm/kvm_host.h:

#include <asm/cputhreads.h>
#define KVM_MAX_VCPU_ID		(threads_per_subcore * KVM_MAX_VCORES)

Thanks !