[PATCH 0/3] kvm: API adjustments for dirty page ring patches

[PATCH 0/3] kvm: API adjustments for dirty page ring patches

[Paolo Bonzini]

As posted, the patches for the dirty page ring broke non-x86
architectures, because it moved KVM_COALESCED_MMIO_PAGE_OFFSET
incorrectly to include/uapi/linux/kvm.h.

   In file included from include/linux/kvm_host.h:36:0,
                    from arch/arm64/kernel/hibernate.c:19:
>> arch/arm64/include/asm/kvm_host.h:35:0: warning: "KVM_COALESCED_MMIO_PAGE_OFFSET" redefined
    #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
    
   In file included from include/linux/kvm_host.h:31:0,
                    from arch/arm64/kernel/hibernate.c:19:
   include/uapi/linux/kvm.h:218:0: note: this is the location of the previous definition
    #define KVM_COALESCED_MMIO_PAGE_OFFSET 2

This fixes it by moving it to each architecture's UAPI headers
separately.

Paolo

Paolo Bonzini (2):
  kvm: make KVM_CAP_COALESCED_MMIO architecture agnostic
  kvm: make KVM_COALESCED_MMIO_PAGE_OFFSET public

 arch/arm/include/asm/kvm_host.h     |  1 -
 arch/arm/include/uapi/asm/kvm.h     |  2 ++
 arch/arm/kvm/arm.c                  |  3 ---
 arch/arm64/include/asm/kvm_host.h   |  1 -
 arch/arm64/include/uapi/asm/kvm.h   |  2 ++
 arch/mips/include/asm/kvm_host.h    |  1 -
 arch/mips/include/uapi/asm/kvm.h    |  2 ++
 arch/mips/kvm/mips.c                |  3 ---
 arch/powerpc/include/asm/kvm_host.h |  3 ---
 arch/powerpc/include/uapi/asm/kvm.h |  3 +++
 arch/powerpc/kvm/powerpc.c          |  5 -----
 arch/x86/include/asm/kvm_host.h     |  2 --
 arch/x86/include/uapi/asm/kvm.h     |  3 +++
 arch/x86/kvm/x86.c                  |  3 ---
 include/linux/kvm_host.h            |  2 +-
 virt/kvm/kvm_main.c                 | 13 +++++++++----
 16 files changed, 22 insertions(+), 27 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] kvm: make KVM_CAP_COALESCED_MMIO architecture agnostic

[Paolo Bonzini]

Remove code from architecture files that can be moved to virt/kvm, since there
is already common code for coalesced MMIO.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/arm/kvm/arm.c         | 3 ---
 arch/mips/kvm/mips.c       | 3 ---
 arch/powerpc/kvm/powerpc.c | 5 -----
 arch/x86/kvm/x86.c         | 3 ---
 virt/kvm/kvm_main.c        | 5 +++++
 5 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 96dba7cd8be7..e3c8105ada65 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -209,9 +209,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_IMMEDIATE_EXIT:
 		r = 1;
 		break;
-	case KVM_CAP_COALESCED_MMIO:
-		r = KVM_COALESCED_MMIO_PAGE_OFFSET;
-		break;
 	case KVM_CAP_ARM_SET_DEVICE_ADDR:
 		r = 1;
 		break;
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 15a1b1716c2e..c164fed024d2 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -1030,9 +1030,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_IMMEDIATE_EXIT:
 		r = 1;
 		break;
-	case KVM_CAP_COALESCED_MMIO:
-		r = KVM_COALESCED_MMIO_PAGE_OFFSET;
-		break;
 	case KVM_CAP_NR_VCPUS:
 		r = num_online_cpus();
 		break;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 7cba0de75024..85e1d8b0bf7f 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -523,11 +523,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		/* We support this only for PR */
 		r = !hv_enabled;
 		break;
-#ifdef CONFIG_KVM_MMIO
-	case KVM_CAP_COALESCED_MMIO:
-		r = KVM_COALESCED_MMIO_PAGE_OFFSET;
-		break;
-#endif
 #ifdef CONFIG_KVM_MPIC
 	case KVM_CAP_IRQ_MPIC:
 		r = 1;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0a69c793ed4c..feaaa105d707 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2689,9 +2689,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		 */
 		r = kvm_x86_ops->cpu_has_high_real_mode_segbase();
 		break;
-	case KVM_CAP_COALESCED_MMIO:
-		r = KVM_COALESCED_MMIO_PAGE_OFFSET;
-		break;
 	case KVM_CAP_VAPIC:
 		r = !kvm_x86_ops->cpu_has_accelerated_tpr();
 		break;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 535eef949c42..be03c2994795 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2919,6 +2919,11 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
 	case KVM_CAP_CHECK_EXTENSION_VM:
 	case KVM_CAP_ENABLE_CAP_VM:
 		return 1;
+#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
+	case KVM_CAP_COALESCED_MMIO:
+		return KVM_COALESCED_MMIO_PAGE_OFFSET;
+		break;
+#endif
 #ifdef CONFIG_HAVE_KVM_IRQ_ROUTING
 	case KVM_CAP_IRQ_ROUTING:
 		return KVM_MAX_IRQ_ROUTES;
-- 
1.8.3.1

[PATCH 2/2] kvm: make KVM_COALESCED_MMIO_PAGE_OFFSET public

[Paolo Bonzini]

Its value has never changed; we might as well make it part of the ABI instead
of using the return value of KVM_CHECK_EXTENSION(KVM_CAP_COALESCED_MMIO).

Because PPC does not always make MMIO available, the code has to be made
dependent on CONFIG_KVM_MMIO rather than KVM_COALESCED_MMIO_PAGE_OFFSET.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/arm/include/asm/kvm_host.h     |  1 -
 arch/arm/include/uapi/asm/kvm.h     |  2 ++
 arch/arm64/include/asm/kvm_host.h   |  1 -
 arch/arm64/include/uapi/asm/kvm.h   |  2 ++
 arch/mips/include/asm/kvm_host.h    |  1 -
 arch/mips/include/uapi/asm/kvm.h    |  2 ++
 arch/powerpc/include/asm/kvm_host.h |  3 ---
 arch/powerpc/include/uapi/asm/kvm.h |  3 +++
 arch/x86/include/asm/kvm_host.h     |  2 --
 arch/x86/include/uapi/asm/kvm.h     |  3 +++
 include/linux/kvm_host.h            |  2 +-
 virt/kvm/kvm_main.c                 | 10 +++++-----
 12 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 31ee468ce667..de67ce647501 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -30,7 +30,6 @@
 #define __KVM_HAVE_ARCH_INTC_INITIALIZED
 
 #define KVM_USER_MEM_SLOTS 32
-#define KVM_COALESCED_MMIO_PAGE_OFFSET 1
 #define KVM_HAVE_ONE_REG
 #define KVM_HALT_POLL_NS_DEFAULT 500000
 
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index 6ebd3e6a1fd1..254a38cace2a 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -27,6 +27,8 @@
 #define __KVM_HAVE_IRQ_LINE
 #define __KVM_HAVE_READONLY_MEM
 
+#define KVM_COALESCED_MMIO_PAGE_OFFSET 1
+
 #define KVM_REG_SIZE(id)						\
 	(1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
 
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index e7705e7bb07b..522e4f60976e 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -31,7 +31,6 @@
 #define __KVM_HAVE_ARCH_INTC_INITIALIZED
 
 #define KVM_USER_MEM_SLOTS 512
-#define KVM_COALESCED_MMIO_PAGE_OFFSET 1
 #define KVM_HALT_POLL_NS_DEFAULT 500000
 
 #include <kvm/arm_vgic.h>
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index c2860358ae3e..aa5ab69c1312 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -39,6 +39,8 @@
 #define __KVM_HAVE_IRQ_LINE
 #define __KVM_HAVE_READONLY_MEM
 
+#define KVM_COALESCED_MMIO_PAGE_OFFSET 1
+
 #define KVM_REG_SIZE(id)						\
 	(1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))
 
diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 05e785fc061d..41676132e473 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -70,7 +70,6 @@
 /* memory slots that does not exposed to userspace */
 #define KVM_PRIVATE_MEM_SLOTS	0
 
-#define KVM_COALESCED_MMIO_PAGE_OFFSET 1
 #define KVM_HALT_POLL_NS_DEFAULT 500000
 
 
diff --git a/arch/mips/include/uapi/asm/kvm.h b/arch/mips/include/uapi/asm/kvm.h
index a8a0199bf760..ade6f3025a10 100644
--- a/arch/mips/include/uapi/asm/kvm.h
+++ b/arch/mips/include/uapi/asm/kvm.h
@@ -21,6 +21,8 @@
 
 #define __KVM_HAVE_READONLY_MEM
 
+#define KVM_COALESCED_MMIO_PAGE_OFFSET 1
+
 /*
  * for KVM_GET_REGS and KVM_SET_REGS
  *
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 7bba8f415627..01d05c76f1c7 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -45,9 +45,6 @@
 
 #define __KVM_HAVE_ARCH_INTC_INITIALIZED
 
-#ifdef CONFIG_KVM_MMIO
-#define KVM_COALESCED_MMIO_PAGE_OFFSET 1
-#endif
 #define KVM_HALT_POLL_NS_DEFAULT 10000	/* 10 us */
 
 /* These values are internal and can be increased later */
diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
index 4edbe4bb0e8b..07fbeb927834 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -29,6 +29,9 @@
 #define __KVM_HAVE_IRQ_LINE
 #define __KVM_HAVE_GUEST_DEBUG
 
+/* Not always available, but if it is, this is the correct offset.  */
+#define KVM_COALESCED_MMIO_PAGE_OFFSET 1
+
 struct kvm_regs {
 	__u64 pc;
 	__u64 cr;
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 7dbb8d622683..d962fa998a6f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -43,8 +43,6 @@
 #define KVM_PRIVATE_MEM_SLOTS 3
 #define KVM_MEM_SLOTS_NUM (KVM_USER_MEM_SLOTS + KVM_PRIVATE_MEM_SLOTS)
 
-#define KVM_PIO_PAGE_OFFSET 1
-#define KVM_COALESCED_MMIO_PAGE_OFFSET 2
 #define KVM_HALT_POLL_NS_DEFAULT 400000
 
 #define KVM_IRQCHIP_NUM_PINS  KVM_IOAPIC_NUM_PINS
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 739c0c594022..c2824d02ba37 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -9,6 +9,9 @@
 #include <linux/types.h>
 #include <linux/ioctl.h>
 
+#define KVM_PIO_PAGE_OFFSET 1
+#define KVM_COALESCED_MMIO_PAGE_OFFSET 2
+
 #define DE_VECTOR 0
 #define DB_VECTOR 1
 #define BP_VECTOR 3
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index e2b5856ff1e9..8a6ab7d16e77 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -403,7 +403,7 @@ struct kvm {
 	struct kvm_vm_stat stat;
 	struct kvm_arch arch;
 	refcount_t users_count;
-#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
+#ifdef CONFIG_KVM_MMIO
 	struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
 	spinlock_t ring_lock;
 	struct list_head coalesced_zones;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index be03c2994795..933f066fb781 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2349,7 +2349,7 @@ static int kvm_vcpu_fault(struct vm_fault *vmf)
 	else if (vmf->pgoff == KVM_PIO_PAGE_OFFSET)
 		page = virt_to_page(vcpu->arch.pio_data);
 #endif
-#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
+#ifdef CONFIG_KVM_MMIO
 	else if (vmf->pgoff == KVM_COALESCED_MMIO_PAGE_OFFSET)
 		page = virt_to_page(vcpu->kvm->coalesced_mmio_ring);
 #endif
@@ -2919,7 +2919,7 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
 	case KVM_CAP_CHECK_EXTENSION_VM:
 	case KVM_CAP_ENABLE_CAP_VM:
 		return 1;
-#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
+#ifdef CONFIG_KVM_MMIO
 	case KVM_CAP_COALESCED_MMIO:
 		return KVM_COALESCED_MMIO_PAGE_OFFSET;
 		break;
@@ -2988,7 +2988,7 @@ static long kvm_vm_ioctl(struct file *filp,
 		r = kvm_vm_ioctl_get_dirty_log(kvm, &log);
 		break;
 	}
-#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
+#ifdef CONFIG_KVM_MMIO
 	case KVM_REGISTER_COALESCED_MMIO: {
 		struct kvm_coalesced_mmio_zone zone;
 
@@ -3180,7 +3180,7 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 	kvm = kvm_create_vm(type);
 	if (IS_ERR(kvm))
 		return PTR_ERR(kvm);
-#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
+#ifdef CONFIG_KVM_MMIO
 	r = kvm_coalesced_mmio_init(kvm);
 	if (r < 0) {
 		kvm_put_kvm(kvm);
@@ -3233,7 +3233,7 @@ static long kvm_dev_ioctl(struct file *filp,
 #ifdef CONFIG_X86
 		r += PAGE_SIZE;    /* pio data page */
 #endif
-#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
+#ifdef CONFIG_KVM_MMIO
 		r += PAGE_SIZE;    /* coalesced mmio ring page */
 #endif
 		break;
-- 
1.8.3.1

Re: [PATCH 1/2] kvm: make KVM_CAP_COALESCED_MMIO architecture agnostic

[Radim Krčmář]

2017-03-31 13:53+0200, Paolo Bonzini:
> Remove code from architecture files that can be moved to virt/kvm, since there
> is already common code for coalesced MMIO.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

Re: [PATCH 2/2] kvm: make KVM_COALESCED_MMIO_PAGE_OFFSET public

[Radim Krčmář]

2017-03-31 13:53+0200, Paolo Bonzini:
> Its value has never changed; we might as well make it part of the ABI instead
> of using the return value of KVM_CHECK_EXTENSION(KVM_CAP_COALESCED_MMIO).
> 
> Because PPC does not always make MMIO available, the code has to be made
> dependent on CONFIG_KVM_MMIO rather than KVM_COALESCED_MMIO_PAGE_OFFSET.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

I like the change to CONFIG_KVM_MMIO and the interface isn't suited for
dynamic offsets based on enabled features anyway ...

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

Re: [PATCH 1/2] kvm: make KVM_CAP_COALESCED_MMIO architecture agnostic

[Radim Krčmář]

2017-03-31 13:53+0200, Paolo Bonzini:
> Remove code from architecture files that can be moved to virt/kvm, since there
> is already common code for coalesced MMIO.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> @@ -2919,6 +2919,11 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
> +#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
> +	case KVM_CAP_COALESCED_MMIO:
> +		return KVM_COALESCED_MMIO_PAGE_OFFSET;
> +		break;

This 'break;' is pointless and would be best removed when applying,
thanks.

> +#endif

Re: [PATCH 1/2] kvm: make KVM_CAP_COALESCED_MMIO architecture agnostic

[David Hildenbrand]

On 31.03.2017 15:41, Radim Krčmář wrote:
> 2017-03-31 13:53+0200, Paolo Bonzini:
>> Remove code from architecture files that can be moved to virt/kvm, since there
>> is already common code for coalesced MMIO.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> @@ -2919,6 +2919,11 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
>> +#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET
>> +	case KVM_CAP_COALESCED_MMIO:
>> +		return KVM_COALESCED_MMIO_PAGE_OFFSET;
>> +		break;
> 
> This 'break;' is pointless and would be best removed when applying,
> thanks.

With that,

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David