[PATCH 0/3] Introduce per-vm kvm device attributes

[PATCH 0/3] Introduce per-vm kvm device attributes

[Christian Borntraeger]

Marcelo, Gleb, Alex,

this is a followup to the recent discussion (re config device)

To avoid a bunch of new ioctl, lets reuse the device attributes on
the "VM device". All of these attributes are architecture specific.
Patch1 introduces the interface,
Patch2 makes CMMA configurable
Patch3 uses the interface introduced in patch1

Marcelo, Gleb, Alex, are you ok with the approach of patch 1? Then we
will continue using that for VM specific controls instead of adding
new ioctls. (Otherwise we will continue with new ioctl).

Dominik Dingel (3):
  kvm: s390: Per-vm kvm device controls
  kvm: s390: make cmma usage conditionally
  kvm: s390: Exploiting generic userspace interface for cmma

 Documentation/virtual/kvm/api.txt        |   8 +-
 Documentation/virtual/kvm/devices/vm.txt |  24 +++++
 arch/s390/include/asm/kvm_host.h         |   1 +
 arch/s390/include/uapi/asm/kvm.h         |   7 ++
 arch/s390/kvm/kvm-s390.c                 | 155 ++++++++++++++++++++++++++-----
 arch/s390/kvm/kvm-s390.h                 |   7 +-
 arch/s390/kvm/priv.c                     |   2 +-
 include/uapi/linux/kvm.h                 |   1 +
 8 files changed, 176 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/virtual/kvm/devices/vm.txt

-- 
1.8.4.2

[PATCH 1/3] kvm: s390: Per-vm kvm device controls

[Christian Borntraeger]

From: Dominik Dingel <dingel@linux.vnet.ibm.com>

We sometimes need to get/set attributes specific to a virtual machine
and so need something else than ONE_REG.

Let's copy the KVM_DEVICE approach, and define the respective ioctls
for the vm file descriptor.

Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 Documentation/virtual/kvm/api.txt        |  8 ++---
 Documentation/virtual/kvm/devices/vm.txt |  6 ++++
 arch/s390/kvm/kvm-s390.c                 | 54 ++++++++++++++++++++++++++++++++
 include/uapi/linux/kvm.h                 |  1 +
 4 files changed, 65 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/virtual/kvm/devices/vm.txt

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index c24211d..f69731a 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2314,8 +2314,8 @@ struct kvm_create_device {
 
 4.80 KVM_SET_DEVICE_ATTR/KVM_GET_DEVICE_ATTR
 
-Capability: KVM_CAP_DEVICE_CTRL
-Type: device ioctl
+Capability: KVM_CAP_DEVICE_CTRL, KVM_CAP_VM_ATTRIBUTES for vm device
+Type: device ioctl, vm ioctl
 Parameters: struct kvm_device_attr
 Returns: 0 on success, -1 on error
 Errors:
@@ -2340,8 +2340,8 @@ struct kvm_device_attr {
 
 4.81 KVM_HAS_DEVICE_ATTR
 
-Capability: KVM_CAP_DEVICE_CTRL
-Type: device ioctl
+Capability: KVM_CAP_DEVICE_CTRL, KVM_CAP_VM_ATTRIBUTES for vm device
+Type: device ioctl, vm ioctl
 Parameters: struct kvm_device_attr
 Returns: 0 on success, -1 on error
 Errors:
diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
new file mode 100644
index 0000000..4fe1532
--- /dev/null
+++ b/Documentation/virtual/kvm/devices/vm.txt
@@ -0,0 +1,6 @@
+Generic vm interface
+====================================
+
+The interface handles per-vm attributes, such as CMMA status on s390 and is
+similar in way to ONE_REG, but targeting the whole vm instead of one vcpu
+alone. It is available with KVM_CAP_VM_ATTRIBUTES.
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 346a347..c335a2e 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -162,6 +162,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_IOEVENTFD:
 	case KVM_CAP_DEVICE_CTRL:
 	case KVM_CAP_ENABLE_CAP_VM:
+	case KVM_CAP_VM_ATTRIBUTES:
 		r = 1;
 		break;
 	case KVM_CAP_NR_VCPUS:
@@ -257,11 +258,43 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
 	return r;
 }
 
+static int kvm_s390_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	int ret;
+
+	switch (attr->group) {
+	default:
+		ret = -ENXIO;
+		break;
+	}
+
+	return ret;
+}
+
+static int kvm_s390_vm_get_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	return -ENXIO;
+}
+
+static int kvm_s390_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	int ret;
+
+	switch (attr->group) {
+	default:
+		ret = -ENXIO;
+		break;
+	}
+
+	return ret;
+}
+
 long kvm_arch_vm_ioctl(struct file *filp,
 		       unsigned int ioctl, unsigned long arg)
 {
 	struct kvm *kvm = filp->private_data;
 	void __user *argp = (void __user *)arg;
+	struct kvm_device_attr attr;
 	int r;
 
 	switch (ioctl) {
@@ -294,6 +327,27 @@ long kvm_arch_vm_ioctl(struct file *filp,
 		}
 		break;
 	}
+	case KVM_SET_DEVICE_ATTR: {
+		r = -EFAULT;
+		if (copy_from_user(&attr, (void __user *)arg, sizeof(attr)))
+			break;
+		r = kvm_s390_vm_set_attr(kvm, &attr);
+		break;
+	}
+	case KVM_GET_DEVICE_ATTR: {
+		r = -EFAULT;
+		if (copy_from_user(&attr, (void __user *)arg, sizeof(attr)))
+			break;
+		r = kvm_s390_vm_get_attr(kvm, &attr);
+		break;
+	}
+	case KVM_HAS_DEVICE_ATTR: {
+		r = -EFAULT;
+		if (copy_from_user(&attr, (void __user *)arg, sizeof(attr)))
+			break;
+		r = kvm_s390_vm_has_attr(kvm, &attr);
+		break;
+	}
 	default:
 		r = -ENOTTY;
 	}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index a8f4ee5..90acfe4 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -743,6 +743,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_IOAPIC_POLARITY_IGNORED 97
 #define KVM_CAP_ENABLE_CAP_VM 98
 #define KVM_CAP_S390_IRQCHIP 99
+#define KVM_CAP_VM_ATTRIBUTES 100
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
1.8.4.2

[PATCH 2/3] kvm: s390: make cmma usage conditionally

[Christian Borntraeger]

From: Dominik Dingel <dingel@linux.vnet.ibm.com>

When userspace reset the guest without notifying kvm, the CMMA state
of the pages might be unused, resulting in guest data corruption.
To avoid this, CMMA must be enabled only if userspace understands
the implications.

CMMA must be enabled before vCPU creation. It can't be switched off
once enabled.  All subsequently created vCPUs will be enabled for
CMMA according to the CMMA state of the VM.

Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |  1 +
 arch/s390/kvm/kvm-s390.c         | 59 +++++++++++++++++++++++++---------------
 arch/s390/kvm/kvm-s390.h         |  7 +++--
 arch/s390/kvm/priv.c             |  2 +-
 4 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index a993b6f..b61ac41 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -288,6 +288,7 @@ struct kvm_arch{
 	struct gmap *gmap;
 	int css_support;
 	int use_irqchip;
+	int use_cmma;
 	struct s390_io_adapter *adapters[MAX_S390_IO_ADAPTERS];
 };
 
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index c335a2e..fc2fe49 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -437,9 +437,8 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 	if (kvm_is_ucontrol(vcpu->kvm))
 		gmap_free(vcpu->arch.gmap);
 
-	if (vcpu->arch.sie_block->cbrlo)
-		__free_page(__pfn_to_page(
-				vcpu->arch.sie_block->cbrlo >> PAGE_SHIFT));
+	if (kvm_s390_cmma_enabled(vcpu->kvm))
+		kvm_s390_vcpu_unsetup_cmma(vcpu);
 	free_page((unsigned long)(vcpu->arch.sie_block));
 
 	kvm_vcpu_uninit(vcpu);
@@ -553,9 +552,26 @@ int kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
+void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu)
+{
+	free_page(vcpu->arch.sie_block->cbrlo);
+	vcpu->arch.sie_block->cbrlo = 0;
+}
+
+int kvm_s390_vcpu_setup_cmma(struct kvm_vcpu *vcpu)
+{
+	vcpu->arch.sie_block->cbrlo = get_zeroed_page(GFP_KERNEL);
+	if (!vcpu->arch.sie_block->cbrlo)
+		return -ENOMEM;
+
+	vcpu->arch.sie_block->ecb2 |= 0x80;
+	vcpu->arch.sie_block->ecb2 &= ~0x08;
+	return 0;
+}
+
 int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 {
-	struct page *cbrl;
+	int rc = 0;
 
 	atomic_set(&vcpu->arch.sie_block->cpuflags, CPUSTAT_ZARCH |
 						    CPUSTAT_SM |
@@ -569,13 +585,10 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 	vcpu->arch.sie_block->eca   = 0xC1002001U;
 	vcpu->arch.sie_block->fac   = (int) (long) vfacilities;
 	vcpu->arch.sie_block->ictl |= ICTL_ISKE | ICTL_SSKE | ICTL_RRBE;
-	if (kvm_enabled_cmma()) {
-		cbrl = alloc_page(GFP_KERNEL | __GFP_ZERO);
-		if (cbrl) {
-			vcpu->arch.sie_block->ecb2 |= 0x80;
-			vcpu->arch.sie_block->ecb2 &= ~0x08;
-			vcpu->arch.sie_block->cbrlo = page_to_phys(cbrl);
-		}
+	if (kvm_s390_cmma_enabled(vcpu->kvm)) {
+		rc = kvm_s390_vcpu_setup_cmma(vcpu);
+		if (rc)
+			return rc;
 	}
 	hrtimer_init(&vcpu->arch.ckc_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
 	tasklet_init(&vcpu->arch.tasklet, kvm_s390_tasklet,
@@ -583,7 +596,7 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 	vcpu->arch.ckc_timer.function = kvm_s390_idle_wakeup;
 	get_cpu_id(&vcpu->arch.cpu_id);
 	vcpu->arch.cpu_id.version = 0xff;
-	return 0;
+	return rc;
 }
 
 struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
@@ -890,6 +903,18 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 	return -EINVAL; /* not implemented yet */
 }
 
+bool kvm_s390_cmma_enabled(struct kvm *kvm)
+{
+	if (!MACHINE_IS_LPAR)
+		return false;
+	/* only enable for z10 and later */
+	if (!MACHINE_HAS_EDAT1)
+		return false;
+	if (!kvm->arch.use_cmma)
+		return false;
+	return true;
+}
+
 static int kvm_s390_handle_requests(struct kvm_vcpu *vcpu)
 {
 	/*
@@ -1072,16 +1097,6 @@ static int vcpu_post_run(struct kvm_vcpu *vcpu, int exit_reason)
 	return rc;
 }
 
-bool kvm_enabled_cmma(void)
-{
-	if (!MACHINE_IS_LPAR)
-		return false;
-	/* only enable for z10 and later */
-	if (!MACHINE_HAS_EDAT1)
-		return false;
-	return true;
-}
-
 static int __vcpu_run(struct kvm_vcpu *vcpu)
 {
 	int rc, exit_reason;
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index 3c1e227..460ccd8 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -158,8 +158,11 @@ void s390_vcpu_block(struct kvm_vcpu *vcpu);
 void s390_vcpu_unblock(struct kvm_vcpu *vcpu);
 void exit_sie(struct kvm_vcpu *vcpu);
 void exit_sie_sync(struct kvm_vcpu *vcpu);
-/* are we going to support cmma? */
-bool kvm_enabled_cmma(void);
+int kvm_s390_vcpu_setup_cmma(struct kvm_vcpu *vcpu);
+void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu);
+/* is cmma enabled */
+bool kvm_s390_cmma_enabled(struct kvm *kvm);
+
 /* implemented in diag.c */
 int kvm_s390_handle_diag(struct kvm_vcpu *vcpu);
 
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index 8a63e99..9a04d74 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -656,7 +656,7 @@ static int handle_essa(struct kvm_vcpu *vcpu)
 	VCPU_EVENT(vcpu, 5, "cmma release %d pages", entries);
 	gmap = vcpu->arch.gmap;
 	vcpu->stat.instruction_essa++;
-	if (!kvm_enabled_cmma() || !vcpu->arch.sie_block->cbrlo)
+	if (!kvm_s390_cmma_enabled(vcpu->kvm))
 		return kvm_s390_inject_program_int(vcpu, PGM_OPERATION);
 
 	if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_PSTATE)
-- 
1.8.4.2

[PATCH 3/3] kvm: s390: Exploiting generic userspace interface for cmma

[Christian Borntraeger]

From: Dominik Dingel <dingel@linux.vnet.ibm.com>

To enable CMMA and to reset its state we use the vm kvm_device ioctls,
encapsulating attributes within the KVM_S390_VM_MEM_CTRL group.

Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 Documentation/virtual/kvm/devices/vm.txt | 18 ++++++++++++++
 arch/s390/include/uapi/asm/kvm.h         |  7 ++++++
 arch/s390/kvm/kvm-s390.c                 | 42 ++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
index 4fe1532..ff495f5 100644
--- a/Documentation/virtual/kvm/devices/vm.txt
+++ b/Documentation/virtual/kvm/devices/vm.txt
@@ -4,3 +4,21 @@ Generic vm interface
 The interface handles per-vm attributes, such as CMMA status on s390 and is
 similar in way to ONE_REG, but targeting the whole vm instead of one vcpu
 alone. It is available with KVM_CAP_VM_ATTRIBUTES.
+
+Following groups and attributes are supported by this interface:
+
+1. GROUP: KVM_S390_VM_MEM_CTRL
+Architectures: s390
+
+1.1. ATTRIBUTE: KVM_S390_VM_MEM_CTRL
+Parameters: none
+Returns: -EBUSY if already a vcpus is defined, otherwise 0
+
+Enables CMMA for the virtual machine
+
+1.2. ATTRIBUTE: KVM_S390_VM_CLR_CMMA
+Parameteres: none
+Returns: 0
+
+Clear the CMMA status for all guest pages, so any pages the guest marked
+as unused are again used any may not be reclaimed by the host.
diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
index c003c6a..e35c798 100644
--- a/arch/s390/include/uapi/asm/kvm.h
+++ b/arch/s390/include/uapi/asm/kvm.h
@@ -54,6 +54,13 @@ struct kvm_s390_io_adapter_req {
 	__u64 addr;
 };
 
+/* kvm attr_group  on vm fd */
+#define KVM_S390_VM_MEM_CTRL		0
+
+/* kvm attributes for mem_ctrl */
+#define KVM_S390_VM_MEM_ENABLE_CMMA	0
+#define KVM_S390_VM_MEM_CLR_CMMA	1
+
 /* for KVM_GET_REGS and KVM_SET_REGS */
 struct kvm_regs {
 	/* general purpose regs for s390 */
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index fc2fe49..2486a11 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -258,11 +258,43 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
 	return r;
 }
 
+static int kvm_s390_mem_control(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	int ret;
+	unsigned int idx;
+	switch (attr->attr) {
+	case KVM_S390_VM_MEM_ENABLE_CMMA:
+		ret = -EBUSY;
+		mutex_lock(&kvm->lock);
+		if (atomic_read(&kvm->online_vcpus) == 0) {
+			kvm->arch.use_cmma = 1;
+			ret = 0;
+		}
+		mutex_unlock(&kvm->lock);
+		break;
+	case KVM_S390_VM_MEM_CLR_CMMA:
+		mutex_lock(&kvm->lock);
+		idx = srcu_read_lock(&kvm->srcu);
+		page_table_reset_pgste(kvm->arch.gmap->mm, 0, TASK_SIZE, false);
+		srcu_read_unlock(&kvm->srcu, idx);
+		mutex_unlock(&kvm->lock);
+		ret = 0;
+		break;
+	default:
+		ret = -ENXIO;
+		break;
+	}
+	return ret;
+}
+
 static int kvm_s390_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
 {
 	int ret;
 
 	switch (attr->group) {
+	case KVM_S390_VM_MEM_CTRL:
+		ret = kvm_s390_mem_control(kvm, attr);
+		break;
 	default:
 		ret = -ENXIO;
 		break;
@@ -281,6 +313,16 @@ static int kvm_s390_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
 	int ret;
 
 	switch (attr->group) {
+	case KVM_S390_VM_MEM_CTRL:
+		switch (attr->attr) {
+		case KVM_S390_VM_MEM_ENABLE_CMMA:
+		case KVM_S390_VM_MEM_CLR_CMMA:
+			ret = 0;
+			break;
+		default:
+			ret = -ENXIO;
+			break;
+		}
 	default:
 		ret = -ENXIO;
 		break;
-- 
1.8.4.2

Re: [PATCH 1/3] kvm: s390: Per-vm kvm device controls

[Alexander Graf]

On 10.04.14 13:16, Christian Borntraeger wrote:
> From: Dominik Dingel <dingel@linux.vnet.ibm.com>
>
> We sometimes need to get/set attributes specific to a virtual machine
> and so need something else than ONE_REG.
>
> Let's copy the KVM_DEVICE approach, and define the respective ioctls
> for the vm file descriptor.
>
> Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>   Documentation/virtual/kvm/api.txt        |  8 ++---
>   Documentation/virtual/kvm/devices/vm.txt |  6 ++++
>   arch/s390/kvm/kvm-s390.c                 | 54 ++++++++++++++++++++++++++++++++
>   include/uapi/linux/kvm.h                 |  1 +
>   4 files changed, 65 insertions(+), 4 deletions(-)
>   create mode 100644 Documentation/virtual/kvm/devices/vm.txt
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index c24211d..f69731a 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2314,8 +2314,8 @@ struct kvm_create_device {
>   
>   4.80 KVM_SET_DEVICE_ATTR/KVM_GET_DEVICE_ATTR
>   
> -Capability: KVM_CAP_DEVICE_CTRL
> -Type: device ioctl
> +Capability: KVM_CAP_DEVICE_CTRL, KVM_CAP_VM_ATTRIBUTES for vm device
> +Type: device ioctl, vm ioctl
>   Parameters: struct kvm_device_attr
>   Returns: 0 on success, -1 on error
>   Errors:
> @@ -2340,8 +2340,8 @@ struct kvm_device_attr {
>   
>   4.81 KVM_HAS_DEVICE_ATTR
>   
> -Capability: KVM_CAP_DEVICE_CTRL
> -Type: device ioctl
> +Capability: KVM_CAP_DEVICE_CTRL, KVM_CAP_VM_ATTRIBUTES for vm device
> +Type: device ioctl, vm ioctl
>   Parameters: struct kvm_device_attr
>   Returns: 0 on success, -1 on error
>   Errors:
> diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
> new file mode 100644
> index 0000000..4fe1532
> --- /dev/null
> +++ b/Documentation/virtual/kvm/devices/vm.txt
> @@ -0,0 +1,6 @@
> +Generic vm interface
> +====================================
> +
> +The interface handles per-vm attributes, such as CMMA status on s390 and is
> +similar in way to ONE_REG, but targeting the whole vm instead of one vcpu
> +alone. It is available with KVM_CAP_VM_ATTRIBUTES.

This document doesn't sound impressively useful :).


Alex

Re: [PATCH 1/3] kvm: s390: Per-vm kvm device controls

[Christian Borntraeger]

On 10/04/14 13:29, Alexander Graf wrote:
> 
> On 10.04.14 13:16, Christian Borntraeger wrote:
>> From: Dominik Dingel <dingel@linux.vnet.ibm.com>
>>
>> We sometimes need to get/set attributes specific to a virtual machine
>> and so need something else than ONE_REG.
>>
>> Let's copy the KVM_DEVICE approach, and define the respective ioctls
>> for the vm file descriptor.
>>
>> Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>   Documentation/virtual/kvm/api.txt        |  8 ++---
>>   Documentation/virtual/kvm/devices/vm.txt |  6 ++++
>>   arch/s390/kvm/kvm-s390.c                 | 54 ++++++++++++++++++++++++++++++++
>>   include/uapi/linux/kvm.h                 |  1 +
>>   4 files changed, 65 insertions(+), 4 deletions(-)
>>   create mode 100644 Documentation/virtual/kvm/devices/vm.txt
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index c24211d..f69731a 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -2314,8 +2314,8 @@ struct kvm_create_device {
>>     4.80 KVM_SET_DEVICE_ATTR/KVM_GET_DEVICE_ATTR
>>   -Capability: KVM_CAP_DEVICE_CTRL
>> -Type: device ioctl
>> +Capability: KVM_CAP_DEVICE_CTRL, KVM_CAP_VM_ATTRIBUTES for vm device
>> +Type: device ioctl, vm ioctl
>>   Parameters: struct kvm_device_attr
>>   Returns: 0 on success, -1 on error
>>   Errors:
>> @@ -2340,8 +2340,8 @@ struct kvm_device_attr {
>>     4.81 KVM_HAS_DEVICE_ATTR
>>   -Capability: KVM_CAP_DEVICE_CTRL
>> -Type: device ioctl
>> +Capability: KVM_CAP_DEVICE_CTRL, KVM_CAP_VM_ATTRIBUTES for vm device
>> +Type: device ioctl, vm ioctl
>>   Parameters: struct kvm_device_attr
>>   Returns: 0 on success, -1 on error
>>   Errors:
>> diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
>> new file mode 100644
>> index 0000000..4fe1532
>> --- /dev/null
>> +++ b/Documentation/virtual/kvm/devices/vm.txt
>> @@ -0,0 +1,6 @@
>> +Generic vm interface
>> +====================================
>> +
>> +The interface handles per-vm attributes, such as CMMA status on s390 and is
>> +similar in way to ONE_REG, but targeting the whole vm instead of one vcpu
>> +alone. It is available with KVM_CAP_VM_ATTRIBUTES.
> 
> This document doesn't sound impressively useful :).

Will be filled with patch3. I asked Dominik to split this out into this patch,
since we introduce the capability here.

Re: [PATCH 1/3] kvm: s390: Per-vm kvm device controls

[Alexander Graf]

On 10.04.14 13:33, Christian Borntraeger wrote:
> On 10/04/14 13:29, Alexander Graf wrote:
>> On 10.04.14 13:16, Christian Borntraeger wrote:
>>> From: Dominik Dingel <dingel@linux.vnet.ibm.com>
>>>
>>> We sometimes need to get/set attributes specific to a virtual machine
>>> and so need something else than ONE_REG.
>>>
>>> Let's copy the KVM_DEVICE approach, and define the respective ioctls
>>> for the vm file descriptor.
>>>
>>> Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>>    Documentation/virtual/kvm/api.txt        |  8 ++---
>>>    Documentation/virtual/kvm/devices/vm.txt |  6 ++++
>>>    arch/s390/kvm/kvm-s390.c                 | 54 ++++++++++++++++++++++++++++++++
>>>    include/uapi/linux/kvm.h                 |  1 +
>>>    4 files changed, 65 insertions(+), 4 deletions(-)
>>>    create mode 100644 Documentation/virtual/kvm/devices/vm.txt
>>>
>>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>>> index c24211d..f69731a 100644
>>> --- a/Documentation/virtual/kvm/api.txt
>>> +++ b/Documentation/virtual/kvm/api.txt
>>> @@ -2314,8 +2314,8 @@ struct kvm_create_device {
>>>      4.80 KVM_SET_DEVICE_ATTR/KVM_GET_DEVICE_ATTR
>>>    -Capability: KVM_CAP_DEVICE_CTRL
>>> -Type: device ioctl
>>> +Capability: KVM_CAP_DEVICE_CTRL, KVM_CAP_VM_ATTRIBUTES for vm device
>>> +Type: device ioctl, vm ioctl
>>>    Parameters: struct kvm_device_attr
>>>    Returns: 0 on success, -1 on error
>>>    Errors:
>>> @@ -2340,8 +2340,8 @@ struct kvm_device_attr {
>>>      4.81 KVM_HAS_DEVICE_ATTR
>>>    -Capability: KVM_CAP_DEVICE_CTRL
>>> -Type: device ioctl
>>> +Capability: KVM_CAP_DEVICE_CTRL, KVM_CAP_VM_ATTRIBUTES for vm device
>>> +Type: device ioctl, vm ioctl
>>>    Parameters: struct kvm_device_attr
>>>    Returns: 0 on success, -1 on error
>>>    Errors:
>>> diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
>>> new file mode 100644
>>> index 0000000..4fe1532
>>> --- /dev/null
>>> +++ b/Documentation/virtual/kvm/devices/vm.txt
>>> @@ -0,0 +1,6 @@
>>> +Generic vm interface
>>> +====================================
>>> +
>>> +The interface handles per-vm attributes, such as CMMA status on s390 and is
>>> +similar in way to ONE_REG, but targeting the whole vm instead of one vcpu
>>> +alone. It is available with KVM_CAP_VM_ATTRIBUTES.
>> This document doesn't sound impressively useful :).
> Will be filled with patch3. I asked Dominik to split this out into this patch,
> since we introduce the capability here.

Ah, it's in .../devices/. Fair enough.


Alex

Re: [PATCH 0/3] Introduce per-vm kvm device attributes

[Alexander Graf]

On 10.04.14 13:16, Christian Borntraeger wrote:
> Marcelo, Gleb, Alex,
>
> this is a followup to the recent discussion (re config device)
>
> To avoid a bunch of new ioctl, lets reuse the device attributes on
> the "VM device". All of these attributes are architecture specific.
> Patch1 introduces the interface,
> Patch2 makes CMMA configurable
> Patch3 uses the interface introduced in patch1
>
> Marcelo, Gleb, Alex, are you ok with the approach of patch 1? Then we
> will continue using that for VM specific controls instead of adding
> new ioctls. (Otherwise we will continue with new ioctl).

Looks reasonable to me, though I wouldn't be opposed to ENABLE_CAP + 
specific ioctl for reset in this case either. Whatever works better for 
you guys.

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


Alex

Re: [PATCH 3/3] kvm: s390: Exploiting generic userspace interface for cmma\

[Marcelo Tosatti]

On Thu, Apr 10, 2014 at 01:16:44PM +0200, Christian Borntraeger wrote:
> From: Dominik Dingel <dingel@linux.vnet.ibm.com>
> 
> To enable CMMA and to reset its state we use the vm kvm_device ioctls,
> encapsulating attributes within the KVM_S390_VM_MEM_CTRL group.
> 
> Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  Documentation/virtual/kvm/devices/vm.txt | 18 ++++++++++++++
>  arch/s390/include/uapi/asm/kvm.h         |  7 ++++++
>  arch/s390/kvm/kvm-s390.c                 | 42 ++++++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+)

Sounds awkward to use these three ioctls for something not 
returned by KVM_CREATE_DEVICE.

/* ioctls for fds returned by KVM_CREATE_DEVICE */
#define KVM_SET_DEVICE_ATTR       _IOW(KVMIO,  0xe1, struct kvm_device_attr)
#define KVM_GET_DEVICE_ATTR       _IOW(KVMIO,  0xe2, struct kvm_device_attr)
#define KVM_HAS_DEVICE_ATTR       _IOW(KVMIO,  0xe3, struct kvm_device_attr)

Is that much of a problem to introduce

struct kvm_vm_attr and

/* ioctls for fds returned by KVM_CREATE_VM */
#define KVM_SET_VM_ATTR       _IOW(KVMIO,  0xa, struct kvm_vm_attr)
#define KVM_GET_VM_ATTR       _IOW(KVMIO,  0xb, struct kvm_vm_attr)
#define KVM_HAS_VM_ATTR       _IOW(KVMIO,  0xc, struct kvm_vm_attr)

?

Other than that (which would be mostly organizational issue) per-vm
attributes seem fine to me.

Re: [PATCH 3/3] kvm: s390: Exploiting generic userspace interface for cmma\

[Alexander Graf]

On 16.04.14 20:49, Marcelo Tosatti wrote:
> On Thu, Apr 10, 2014 at 01:16:44PM +0200, Christian Borntraeger wrote:
>> From: Dominik Dingel <dingel@linux.vnet.ibm.com>
>>
>> To enable CMMA and to reset its state we use the vm kvm_device ioctls,
>> encapsulating attributes within the KVM_S390_VM_MEM_CTRL group.
>>
>> Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>   Documentation/virtual/kvm/devices/vm.txt | 18 ++++++++++++++
>>   arch/s390/include/uapi/asm/kvm.h         |  7 ++++++
>>   arch/s390/kvm/kvm-s390.c                 | 42 ++++++++++++++++++++++++++++++++
>>   3 files changed, 67 insertions(+)
> Sounds awkward to use these three ioctls for something not
> returned by KVM_CREATE_DEVICE.
>
> /* ioctls for fds returned by KVM_CREATE_DEVICE */
> #define KVM_SET_DEVICE_ATTR       _IOW(KVMIO,  0xe1, struct kvm_device_attr)
> #define KVM_GET_DEVICE_ATTR       _IOW(KVMIO,  0xe2, struct kvm_device_attr)
> #define KVM_HAS_DEVICE_ATTR       _IOW(KVMIO,  0xe3, struct kvm_device_attr)
>
> Is that much of a problem to introduce
>
> struct kvm_vm_attr and
>
> /* ioctls for fds returned by KVM_CREATE_VM */
> #define KVM_SET_VM_ATTR       _IOW(KVMIO,  0xa, struct kvm_vm_attr)
> #define KVM_GET_VM_ATTR       _IOW(KVMIO,  0xb, struct kvm_vm_attr)
> #define KVM_HAS_VM_ATTR       _IOW(KVMIO,  0xc, struct kvm_vm_attr)
>
> ?

We could just alias them, no?

#define KVM_SET_VM_ATTR KVM_SET_DEVICE_ATTR

But I don't feel strongly either way.


Alex

> Other than that (which would be mostly organizational issue) per-vm
> attributes seem fine to me.
>

Re: [PATCH 3/3] kvm: s390: Exploiting generic userspace interface for cmma\

[Marcelo Tosatti]

On Wed, Apr 16, 2014 at 09:47:52PM +0200, Alexander Graf wrote:
> 
> On 16.04.14 20:49, Marcelo Tosatti wrote:
> >On Thu, Apr 10, 2014 at 01:16:44PM +0200, Christian Borntraeger wrote:
> >>From: Dominik Dingel <dingel@linux.vnet.ibm.com>
> >>
> >>To enable CMMA and to reset its state we use the vm kvm_device ioctls,
> >>encapsulating attributes within the KVM_S390_VM_MEM_CTRL group.
> >>
> >>Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
> >>Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >>---
> >>  Documentation/virtual/kvm/devices/vm.txt | 18 ++++++++++++++
> >>  arch/s390/include/uapi/asm/kvm.h         |  7 ++++++
> >>  arch/s390/kvm/kvm-s390.c                 | 42 ++++++++++++++++++++++++++++++++
> >>  3 files changed, 67 insertions(+)
> >Sounds awkward to use these three ioctls for something not
> >returned by KVM_CREATE_DEVICE.
> >
> >/* ioctls for fds returned by KVM_CREATE_DEVICE */
> >#define KVM_SET_DEVICE_ATTR       _IOW(KVMIO,  0xe1, struct kvm_device_attr)
> >#define KVM_GET_DEVICE_ATTR       _IOW(KVMIO,  0xe2, struct kvm_device_attr)
> >#define KVM_HAS_DEVICE_ATTR       _IOW(KVMIO,  0xe3, struct kvm_device_attr)
> >
> >Is that much of a problem to introduce
> >
> >struct kvm_vm_attr and
> >
> >/* ioctls for fds returned by KVM_CREATE_VM */
> >#define KVM_SET_VM_ATTR       _IOW(KVMIO,  0xa, struct kvm_vm_attr)
> >#define KVM_GET_VM_ATTR       _IOW(KVMIO,  0xb, struct kvm_vm_attr)
> >#define KVM_HAS_VM_ATTR       _IOW(KVMIO,  0xc, struct kvm_vm_attr)
> >
> >?
> 
> We could just alias them, no?
> 
> #define KVM_SET_VM_ATTR KVM_SET_DEVICE_ATTR
> 
> But I don't feel strongly either way.

Ok not a huge deal.

Assuming the series will be resubmitted with 
improved virtual/kvm/devices/vm.txt.