[PATCH 0/3] KVM: Make KVM_CHECK_EXTENSION a VM ioctl

[PATCH 0/3] KVM: Make KVM_CHECK_EXTENSION a VM ioctl

[Alexander Graf]

On PowerPC we have a small problem :). We can run both HV and PR style VMs
on the same kvm fd. While this is great, it means that anything that's
different between the two needs to have a token in form of a VM fd to find
out which one we're asking for.

The one thing where this bites us are CAPs. We ask for them on the kvm fd,
not the vm fd. So we can only take a random guess whether the user is asking
for HV or PR capabilities.

So far we got away with this reasonably well - most people will only load one
of the two modules and the only thing that *really* breaks is hypercall exposure
to user space, so a PR guest will not be able to do KVM hypercalls when HV KVM
is loaded on the host, making the magic page unavailable to it.

But this still isn't a great situation to be in. Instead, we really should just
make the CHECK_EXTENSION ioctl available at VM level. Then we know for sure
what user space is asking for.


Alex 

Alexander Graf (3):
  KVM: Rename and add argument to check_extension
  KVM: Allow KVM_CHECK_EXTENSION on the vm fd
  KVM: PPC: Book3S: Provide different CAPs based on HV or PR mode

 Documentation/virtual/kvm/api.txt |  5 +++-
 arch/arm/kvm/arm.c                |  2 +-
 arch/ia64/kvm/kvm-ia64.c          |  2 +-
 arch/mips/kvm/mips.c              |  2 +-
 arch/powerpc/kvm/powerpc.c        | 14 +++++++---
 arch/s390/kvm/kvm-s390.c          |  2 +-
 arch/x86/kvm/x86.c                |  2 +-
 include/linux/kvm_host.h          |  2 +-
 virt/kvm/kvm_main.c               | 59 ++++++++++++++++++++-------------------
 9 files changed, 51 insertions(+), 39 deletions(-)

-- 
1.8.1.4

[PATCH 1/3] KVM: Rename and add argument to check_extension

[Alexander Graf]

In preparation to make the check_extension function available to VM scope
we add a struct kvm * argument to the function header and rename the function
accordingly. It will still be called from the /dev/kvm fd, but with a NULL
argument for struct kvm *.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/arm/kvm/arm.c         | 2 +-
 arch/ia64/kvm/kvm-ia64.c   | 2 +-
 arch/mips/kvm/mips.c       | 2 +-
 arch/powerpc/kvm/powerpc.c | 2 +-
 arch/s390/kvm/kvm-s390.c   | 2 +-
 arch/x86/kvm/x86.c         | 2 +-
 include/linux/kvm_host.h   | 2 +-
 virt/kvm/kvm_main.c        | 6 +++---
 8 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 3c82b37..cb77f999 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -184,7 +184,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 	}
 }
 
-int kvm_dev_ioctl_check_extension(long ext)
+int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 {
 	int r;
 	switch (ext) {
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 6a4309b..0729ba6 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -190,7 +190,7 @@ void kvm_arch_check_processor_compat(void *rtn)
 	*(int *)rtn = 0;
 }
 
-int kvm_dev_ioctl_check_extension(long ext)
+int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 {
 
 	int r;
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 4fda672..cd71141 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -886,7 +886,7 @@ int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf)
 	return VM_FAULT_SIGBUS;
 }
 
-int kvm_dev_ioctl_check_extension(long ext)
+int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 {
 	int r;
 
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index fe0257a..2a0e497 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -385,7 +385,7 @@ void kvm_arch_sync_events(struct kvm *kvm)
 {
 }
 
-int kvm_dev_ioctl_check_extension(long ext)
+int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 {
 	int r;
 	/* FIXME!!
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 2f3e14f..00268ca 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -146,7 +146,7 @@ long kvm_arch_dev_ioctl(struct file *filp,
 	return -EINVAL;
 }
 
-int kvm_dev_ioctl_check_extension(long ext)
+int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 {
 	int r;
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f65c22c..e9f0a92 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2616,7 +2616,7 @@ out:
 	return r;
 }
 
-int kvm_dev_ioctl_check_extension(long ext)
+int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 {
 	int r;
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index ec4e3bd..5065b95 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -602,7 +602,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg);
 int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf);
 
-int kvm_dev_ioctl_check_extension(long ext);
+int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext);
 
 int kvm_get_dirty_log(struct kvm *kvm,
 			struct kvm_dirty_log *log, int *is_dirty);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4b6c01b..e28f3ca 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2571,7 +2571,7 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 	return r;
 }
 
-static long kvm_dev_ioctl_check_extension_generic(long arg)
+static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
 {
 	switch (arg) {
 	case KVM_CAP_USER_MEMORY:
@@ -2595,7 +2595,7 @@ static long kvm_dev_ioctl_check_extension_generic(long arg)
 	default:
 		break;
 	}
-	return kvm_dev_ioctl_check_extension(arg);
+	return kvm_vm_ioctl_check_extension(kvm, arg);
 }
 
 static long kvm_dev_ioctl(struct file *filp,
@@ -2614,7 +2614,7 @@ static long kvm_dev_ioctl(struct file *filp,
 		r = kvm_dev_ioctl_create_vm(arg);
 		break;
 	case KVM_CHECK_EXTENSION:
-		r = kvm_dev_ioctl_check_extension_generic(arg);
+		r = kvm_vm_ioctl_check_extension_generic(NULL, arg);
 		break;
 	case KVM_GET_VCPU_MMAP_SIZE:
 		r = -EINVAL;
-- 
1.8.1.4

[PATCH 2/3] KVM: Allow KVM_CHECK_EXTENSION on the vm fd

[Alexander Graf]

The KVM_CHECK_EXTENSION is only available on the kvm fd today. Unfortunately
on PPC some of the capabilities change depending on the way a VM was created.

So instead we need a way to expose capabilities as VM ioctl, so that we can
see which VM type we're using (HV or PR). To enable this, add the
KVM_CHECK_EXTENSION ioctl to our vm ioctl portfolio.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 Documentation/virtual/kvm/api.txt |  5 +++-
 virt/kvm/kvm_main.c               | 57 ++++++++++++++++++++-------------------
 2 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 6955318..f0ac03d 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -150,7 +150,7 @@ of banks, as set via the KVM_X86_SETUP_MCE ioctl.
 
 Capability: basic
 Architectures: all
-Type: system ioctl
+Type: system ioctl, vm ioctl
 Parameters: extension identifier (KVM_CAP_*)
 Returns: 0 if unsupported; 1 (or some other positive integer) if supported
 
@@ -160,6 +160,9 @@ receives an integer that describes the extension availability.
 Generally 0 means no and 1 means yes, but some extensions may report
 additional information in the integer return value.
 
+Based on their initialization different VMs may have different capabilities.
+It is thus encouraged to use the vm ioctl to query for capabilities (available
+as of 3.17).
 
 4.5 KVM_GET_VCPU_MMAP_SIZE
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e28f3ca..6f1c1cc 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2324,6 +2324,33 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
 	return 0;
 }
 
+static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
+{
+	switch (arg) {
+	case KVM_CAP_USER_MEMORY:
+	case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
+	case KVM_CAP_JOIN_MEMORY_REGIONS_WORKS:
+#ifdef CONFIG_KVM_APIC_ARCHITECTURE
+	case KVM_CAP_SET_BOOT_CPU_ID:
+#endif
+	case KVM_CAP_INTERNAL_ERROR_DATA:
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_CAP_SIGNAL_MSI:
+#endif
+#ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
+	case KVM_CAP_IRQFD_RESAMPLE:
+#endif
+		return 1;
+#ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
+	case KVM_CAP_IRQ_ROUTING:
+		return KVM_MAX_IRQ_ROUTES;
+#endif
+	default:
+		break;
+	}
+	return kvm_vm_ioctl_check_extension(kvm, arg);
+}
+
 static long kvm_vm_ioctl(struct file *filp,
 			   unsigned int ioctl, unsigned long arg)
 {
@@ -2487,6 +2514,9 @@ static long kvm_vm_ioctl(struct file *filp,
 		r = 0;
 		break;
 	}
+	case KVM_CHECK_EXTENSION:
+		r = kvm_vm_ioctl_check_extension_generic(kvm, arg);
+		break;
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)
@@ -2571,33 +2601,6 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 	return r;
 }
 
-static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
-{
-	switch (arg) {
-	case KVM_CAP_USER_MEMORY:
-	case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
-	case KVM_CAP_JOIN_MEMORY_REGIONS_WORKS:
-#ifdef CONFIG_KVM_APIC_ARCHITECTURE
-	case KVM_CAP_SET_BOOT_CPU_ID:
-#endif
-	case KVM_CAP_INTERNAL_ERROR_DATA:
-#ifdef CONFIG_HAVE_KVM_MSI
-	case KVM_CAP_SIGNAL_MSI:
-#endif
-#ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
-	case KVM_CAP_IRQFD_RESAMPLE:
-#endif
-		return 1;
-#ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
-	case KVM_CAP_IRQ_ROUTING:
-		return KVM_MAX_IRQ_ROUTES;
-#endif
-	default:
-		break;
-	}
-	return kvm_vm_ioctl_check_extension(kvm, arg);
-}
-
 static long kvm_dev_ioctl(struct file *filp,
 			  unsigned int ioctl, unsigned long arg)
 {
-- 
1.8.1.4

[PATCH 3/3] KVM: PPC: Book3S: Provide different CAPs based on HV or PR mode

[Alexander Graf]

With Book3S KVM we can create both PR and HV VMs in parallel on the same
machine. That gives us new challenges on the CAPs we return - both have
different capabilities.

When we get asked about CAPs on the kvm fd, there's nothing we can do. We
can try to be smart and assume we're running HV if HV is available, PR
otherwise. However with the newly added VM CHECK_EXTENSION we can now ask
for capabilities directly on a VM which knows whether it's PR or HV.

With this patch I can successfully expose KVM PVINFO data to user space
in the PR case, fixing magic page mapping for PAPR guests.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/powerpc/kvm/powerpc.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 2a0e497..6e13fcf 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -388,11 +388,17 @@ void kvm_arch_sync_events(struct kvm *kvm)
 int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 {
 	int r;
-	/* FIXME!!
-	 * Should some of this be vm ioctl ? is it possible now ?
-	 */
+	/* Assume we're using HV mode when the HV module is loaded */
 	int hv_enabled = kvmppc_hv_ops ? 1 : 0;
 
+	if (kvm) {
+		/*
+		 * Hooray - we know which VM type we're running on. Depend on
+		 * that rather than the guess above.
+		 */
+		hv_enabled = is_kvmppc_hv_enabled(kvm);
+	}
+
 	switch (ext) {
 #ifdef CONFIG_BOOKE
 	case KVM_CAP_PPC_BOOKE_SREGS:
-- 
1.8.1.4

Re: [PATCH 2/3] KVM: Allow KVM_CHECK_EXTENSION on the vm fd

[Alexander Graf]

On 14.07.14 19:03, Alexander Graf wrote:
> The KVM_CHECK_EXTENSION is only available on the kvm fd today. Unfortunately
> on PPC some of the capabilities change depending on the way a VM was created.
>
> So instead we need a way to expose capabilities as VM ioctl, so that we can
> see which VM type we're using (HV or PR). To enable this, add the
> KVM_CHECK_EXTENSION ioctl to our vm ioctl portfolio.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>

Hrm, I guess this itself should also get a CAP :).


Alex

[PATCH v2 2/3] KVM: Allow KVM_CHECK_EXTENSION on the vm fd

[Alexander Graf]

The KVM_CHECK_EXTENSION is only available on the kvm fd today. Unfortunately
on PPC some of the capabilities change depending on the way a VM was created.

So instead we need a way to expose capabilities as VM ioctl, so that we can
see which VM type we're using (HV or PR). To enable this, add the
KVM_CHECK_EXTENSION ioctl to our vm ioctl portfolio.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - add CAP to expose the vm based CHECK_EXTENSION ioctl availability
---
 Documentation/virtual/kvm/api.txt |  7 +++--
 include/uapi/linux/kvm.h          |  1 +
 virt/kvm/kvm_main.c               | 58 +++++++++++++++++++++------------------
 3 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 6955318..235630c 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -148,9 +148,9 @@ of banks, as set via the KVM_X86_SETUP_MCE ioctl.
 
 4.4 KVM_CHECK_EXTENSION
 
-Capability: basic
+Capability: basic, KVM_CAP_CHECK_EXTENSION_VM for vm ioctl
 Architectures: all
-Type: system ioctl
+Type: system ioctl, vm ioctl
 Parameters: extension identifier (KVM_CAP_*)
 Returns: 0 if unsupported; 1 (or some other positive integer) if supported
 
@@ -160,6 +160,9 @@ receives an integer that describes the extension availability.
 Generally 0 means no and 1 means yes, but some extensions may report
 additional information in the integer return value.
 
+Based on their initialization different VMs may have different capabilities.
+It is thus encouraged to use the vm ioctl to query for capabilities (available
+with KVM_CAP_CHECK_EXTENSION_VM on the vm fd)
 
 4.5 KVM_GET_VCPU_MMAP_SIZE
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 0418b74..51776ca 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -759,6 +759,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_ARM_PSCI_0_2 102
 #define KVM_CAP_PPC_FIXUP_HCALL 103
 #define KVM_CAP_PPC_ENABLE_HCALL 104
+#define KVM_CAP_CHECK_EXTENSION_VM 105
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e28f3ca..1b95cc9 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2324,6 +2324,34 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
 	return 0;
 }
 
+static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
+{
+	switch (arg) {
+	case KVM_CAP_USER_MEMORY:
+	case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
+	case KVM_CAP_JOIN_MEMORY_REGIONS_WORKS:
+#ifdef CONFIG_KVM_APIC_ARCHITECTURE
+	case KVM_CAP_SET_BOOT_CPU_ID:
+#endif
+	case KVM_CAP_INTERNAL_ERROR_DATA:
+#ifdef CONFIG_HAVE_KVM_MSI
+	case KVM_CAP_SIGNAL_MSI:
+#endif
+#ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
+	case KVM_CAP_IRQFD_RESAMPLE:
+#endif
+	case KVM_CAP_CHECK_EXTENSION_VM:
+		return 1;
+#ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
+	case KVM_CAP_IRQ_ROUTING:
+		return KVM_MAX_IRQ_ROUTES;
+#endif
+	default:
+		break;
+	}
+	return kvm_vm_ioctl_check_extension(kvm, arg);
+}
+
 static long kvm_vm_ioctl(struct file *filp,
 			   unsigned int ioctl, unsigned long arg)
 {
@@ -2487,6 +2515,9 @@ static long kvm_vm_ioctl(struct file *filp,
 		r = 0;
 		break;
 	}
+	case KVM_CHECK_EXTENSION:
+		r = kvm_vm_ioctl_check_extension_generic(kvm, arg);
+		break;
 	default:
 		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 		if (r == -ENOTTY)
@@ -2571,33 +2602,6 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 	return r;
 }
 
-static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
-{
-	switch (arg) {
-	case KVM_CAP_USER_MEMORY:
-	case KVM_CAP_DESTROY_MEMORY_REGION_WORKS:
-	case KVM_CAP_JOIN_MEMORY_REGIONS_WORKS:
-#ifdef CONFIG_KVM_APIC_ARCHITECTURE
-	case KVM_CAP_SET_BOOT_CPU_ID:
-#endif
-	case KVM_CAP_INTERNAL_ERROR_DATA:
-#ifdef CONFIG_HAVE_KVM_MSI
-	case KVM_CAP_SIGNAL_MSI:
-#endif
-#ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
-	case KVM_CAP_IRQFD_RESAMPLE:
-#endif
-		return 1;
-#ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
-	case KVM_CAP_IRQ_ROUTING:
-		return KVM_MAX_IRQ_ROUTES;
-#endif
-	default:
-		break;
-	}
-	return kvm_vm_ioctl_check_extension(kvm, arg);
-}
-
 static long kvm_dev_ioctl(struct file *filp,
 			  unsigned int ioctl, unsigned long arg)
 {
-- 
1.8.1.4

Re: [PATCH 0/3] KVM: Make KVM_CHECK_EXTENSION a VM ioctl

[Cornelia Huck]

On Mon, 14 Jul 2014 19:03:35 +0200
Alexander Graf <agraf@suse.de> wrote:

> On PowerPC we have a small problem :). We can run both HV and PR style VMs
> on the same kvm fd. While this is great, it means that anything that's
> different between the two needs to have a token in form of a VM fd to find
> out which one we're asking for.
> 
> The one thing where this bites us are CAPs. We ask for them on the kvm fd,
> not the vm fd. So we can only take a random guess whether the user is asking
> for HV or PR capabilities.
> 
> So far we got away with this reasonably well - most people will only load one
> of the two modules and the only thing that *really* breaks is hypercall exposure
> to user space, so a PR guest will not be able to do KVM hypercalls when HV KVM
> is loaded on the host, making the magic page unavailable to it.
> 
> But this still isn't a great situation to be in. Instead, we really should just
> make the CHECK_EXTENSION ioctl available at VM level. Then we know for sure
> what user space is asking for.

I think this makes sense.

It adds one situation we didn't have before, though: We now have the
checking ioctl acting on a target where we also have the enabling
ioctl. Previously, it made sense to advertise capabilities that can be
enabled on the vcpus on the kvm fd; but what should happen for a
capability that can be enabled on a vm if the vm fd is queried? Should
it always be advertised, or only if it has been enabled?

(I just noticed that we don't advertise KVM_CAP_S390_IRQCHIP, which is a
capability that can be enabled per-vm...)

Re: [PATCH 0/3] KVM: Make KVM_CHECK_EXTENSION a VM ioctl

[Alexander Graf]

> Am 15.07.2014 um 08:50 schrieb Cornelia Huck <cornelia.huck@de.ibm.com>:
> 
> On Mon, 14 Jul 2014 19:03:35 +0200
> Alexander Graf <agraf@suse.de> wrote:
> 
>> On PowerPC we have a small problem :). We can run both HV and PR style VMs
>> on the same kvm fd. While this is great, it means that anything that's
>> different between the two needs to have a token in form of a VM fd to find
>> out which one we're asking for.
>> 
>> The one thing where this bites us are CAPs. We ask for them on the kvm fd,
>> not the vm fd. So we can only take a random guess whether the user is asking
>> for HV or PR capabilities.
>> 
>> So far we got away with this reasonably well - most people will only load one
>> of the two modules and the only thing that *really* breaks is hypercall exposure
>> to user space, so a PR guest will not be able to do KVM hypercalls when HV KVM
>> is loaded on the host, making the magic page unavailable to it.
>> 
>> But this still isn't a great situation to be in. Instead, we really should just
>> make the CHECK_EXTENSION ioctl available at VM level. Then we know for sure
>> what user space is asking for.
> 
> I think this makes sense.
> 
> It adds one situation we didn't have before, though: We now have the
> checking ioctl acting on a target where we also have the enabling
> ioctl. Previously, it made sense to advertise capabilities that can be
> enabled on the vcpus on the kvm fd; but what should happen for a
> capability that can be enabled on a vm if the vm fd is queried? Should
> it always be advertised, or only if it has been enabled?

Check_extension tells us what we can potentially enable, not what is enabled. So it should always be advertised :).


Alex

> 
> (I just noticed that we don't advertise KVM_CAP_S390_IRQCHIP, which is a
> capability that can be enabled per-vm...)
> 

Re: [PATCH 0/3] KVM: Make KVM_CHECK_EXTENSION a VM ioctl

[Paolo Bonzini]

Il 14/07/2014 19:03, Alexander Graf ha scritto:
> On PowerPC we have a small problem :). We can run both HV and PR style VMs
> on the same kvm fd. While this is great, it means that anything that's
> different between the two needs to have a token in form of a VM fd to find
> out which one we're asking for.
>
> The one thing where this bites us are CAPs. We ask for them on the kvm fd,
> not the vm fd. So we can only take a random guess whether the user is asking
> for HV or PR capabilities.
>
> So far we got away with this reasonably well - most people will only load one
> of the two modules and the only thing that *really* breaks is hypercall exposure
> to user space, so a PR guest will not be able to do KVM hypercalls when HV KVM
> is loaded on the host, making the magic page unavailable to it.
>
> But this still isn't a great situation to be in. Instead, we really should just
> make the CHECK_EXTENSION ioctl available at VM level. Then we know for sure
> what user space is asking for.
>
>
> Alex
>
> Alexander Graf (3):
>   KVM: Rename and add argument to check_extension
>   KVM: Allow KVM_CHECK_EXTENSION on the vm fd
>   KVM: PPC: Book3S: Provide different CAPs based on HV or PR mode
>
>  Documentation/virtual/kvm/api.txt |  5 +++-
>  arch/arm/kvm/arm.c                |  2 +-
>  arch/ia64/kvm/kvm-ia64.c          |  2 +-
>  arch/mips/kvm/mips.c              |  2 +-
>  arch/powerpc/kvm/powerpc.c        | 14 +++++++---
>  arch/s390/kvm/kvm-s390.c          |  2 +-
>  arch/x86/kvm/x86.c                |  2 +-
>  include/linux/kvm_host.h          |  2 +-
>  virt/kvm/kvm_main.c               | 59 ++++++++++++++++++++-------------------
>  9 files changed, 51 insertions(+), 39 deletions(-)
>

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Feel free to include it in your pull request.

Paolo