[GIT PULL 0/5] KVM: s390: Let user space control the cpu states

[GIT PULL 0/5] KVM: s390: Let user space control the cpu states

[Christian Borntraeger]

Paolo,

The following changes since commit 9f6226a762c7ae02f6a23a3d4fc552dafa57ea23:

  arch: x86: kvm: x86.c: Cleaning up variable is set more than once (2014-06-30 16:52:04 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git  tags/kvm-s390-20140715

for you to fetch changes up to 6352e4d2dd9a349024a41356148eced553e1dce4:

  KVM: s390: implement KVM_(S|G)ET_MP_STATE for user space state control (2014-07-10 14:11:17 +0200)

----------------------------------------------------------------
This series enables the "KVM_(S|G)ET_MP_STATE" ioctls on s390 to make
the cpu state settable by user space.

This is necessary to avoid races in s390 SIGP/reset handling which
happen because some SIGPs are handled in QEMU, while others are
handled in the kernel. Together with the busy conditions as return
value of SIGP races happen especially in areas like starting and
stopping of CPUs. (For example, there is a program 'cpuplugd', that
runs on several s390 distros which does automatic onlining and
offlining on cpus.)

As soon as the MPSTATE interface is used, user space takes complete
control of the cpu states. Otherwise the kernel will use the old way.

Therefore, the new kernel continues to work fine with old QEMUs.

----------------------------------------------------------------
David Hildenbrand (5):
      KVM: s390: allow only one SIGP STOP (AND STORE STATUS) at a time
      KVM: s390: move finalization of SIGP STOP orders to kvm_s390_vcpu_stop
      KVM: s390: remove __cpu_is_stopped and expose is_vcpu_stopped
      KVM: prepare for KVM_(S|G)ET_MP_STATE on other architectures
      KVM: s390: implement KVM_(S|G)ET_MP_STATE for user space state control

 Documentation/virtual/kvm/api.txt | 31 ++++++++++++++---------
 arch/s390/include/asm/kvm_host.h  |  1 +
 arch/s390/kvm/diag.c              |  3 ++-
 arch/s390/kvm/intercept.c         | 32 ++++++++++--------------
 arch/s390/kvm/kvm-s390.c          | 52 +++++++++++++++++++++++++++++++--------
 arch/s390/kvm/kvm-s390.h          | 10 ++++++--
 arch/s390/kvm/sigp.c              |  7 +++++-
 include/uapi/linux/kvm.h          |  7 +++++-
 8 files changed, 98 insertions(+), 45 deletions(-)

[GIT PULL 1/5] KVM: s390: allow only one SIGP STOP (AND STORE STATUS) at a time

[Christian Borntraeger]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

A SIGP STOP (AND STORE STATUS) order is complete as soon as the VCPU has been
stopped. This patch makes sure that only one SIGP STOP (AND STORE STATUS) may
be pending at a time (as defined by the architecture). If the action_bits are
still set, a SIGP STOP has been issued but not completed yet. The VCPU is busy
for further SIGP STOP orders.

Also set the CPUSTAT_STOP_INT after the action_bits variable has been modified
(the same order that is used when injecting a KVM_S390_SIGP_STOP from
userspace).

Both changes are needed in preparation for a user space driven VCPU state change
(to avoid race conditions).

Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/sigp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
index 43079a4..fd7fb5c 100644
--- a/arch/s390/kvm/sigp.c
+++ b/arch/s390/kvm/sigp.c
@@ -136,6 +136,11 @@ static int __inject_sigp_stop(struct kvm_s390_local_interrupt *li, int action)
 	inti->type = KVM_S390_SIGP_STOP;
 
 	spin_lock_bh(&li->lock);
+	if (li->action_bits & ACTION_STOP_ON_STOP) {
+		/* another SIGP STOP is pending */
+		rc = SIGP_CC_BUSY;
+		goto out;
+	}
 	if ((atomic_read(li->cpuflags) & CPUSTAT_STOPPED)) {
 		kfree(inti);
 		if ((action & ACTION_STORE_ON_STOP) != 0)
@@ -144,8 +149,8 @@ static int __inject_sigp_stop(struct kvm_s390_local_interrupt *li, int action)
 	}
 	list_add_tail(&inti->list, &li->list);
 	atomic_set(&li->active, 1);
-	atomic_set_mask(CPUSTAT_STOP_INT, li->cpuflags);
 	li->action_bits |= action;
+	atomic_set_mask(CPUSTAT_STOP_INT, li->cpuflags);
 	if (waitqueue_active(li->wq))
 		wake_up_interruptible(li->wq);
 out:
-- 
1.8.4.2

[GIT PULL 2/5] KVM: s390: move finalization of SIGP STOP orders to kvm_s390_vcpu_stop

[Christian Borntraeger]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

Let's move the finalization of SIGP STOP and SIGP STOP AND STORE STATUS orders to
the point where the VCPU is actually stopped.

This change is needed to prepare for a user space driven VCPU state change. The
action_bits may only be cleared when setting the cpu state to STOPPED while
holding the local irq lock.

Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/intercept.c | 31 ++++++++++++-------------------
 arch/s390/kvm/kvm-s390.c  |  8 ++++++++
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
index a0b586c..ac6b325 100644
--- a/arch/s390/kvm/intercept.c
+++ b/arch/s390/kvm/intercept.c
@@ -56,32 +56,25 @@ static int handle_noop(struct kvm_vcpu *vcpu)
 static int handle_stop(struct kvm_vcpu *vcpu)
 {
 	int rc = 0;
+	unsigned int action_bits;
 
 	vcpu->stat.exit_stop_request++;
-	spin_lock_bh(&vcpu->arch.local_int.lock);
-
 	trace_kvm_s390_stop_request(vcpu->arch.local_int.action_bits);
 
-	if (vcpu->arch.local_int.action_bits & ACTION_STOP_ON_STOP) {
-		kvm_s390_vcpu_stop(vcpu);
-		vcpu->arch.local_int.action_bits &= ~ACTION_STOP_ON_STOP;
-		VCPU_EVENT(vcpu, 3, "%s", "cpu stopped");
-		rc = -EOPNOTSUPP;
-	}
+	action_bits = vcpu->arch.local_int.action_bits;
 
-	if (vcpu->arch.local_int.action_bits & ACTION_STORE_ON_STOP) {
-		vcpu->arch.local_int.action_bits &= ~ACTION_STORE_ON_STOP;
-		/* store status must be called unlocked. Since local_int.lock
-		 * only protects local_int.* and not guest memory we can give
-		 * up the lock here */
-		spin_unlock_bh(&vcpu->arch.local_int.lock);
+	if (!(action_bits & ACTION_STOP_ON_STOP))
+		return 0;
+
+	if (action_bits & ACTION_STORE_ON_STOP) {
 		rc = kvm_s390_vcpu_store_status(vcpu,
 						KVM_S390_STORE_STATUS_NOADDR);
-		if (rc >= 0)
-			rc = -EOPNOTSUPP;
-	} else
-		spin_unlock_bh(&vcpu->arch.local_int.lock);
-	return rc;
+		if (rc)
+			return rc;
+	}
+
+	kvm_s390_vcpu_stop(vcpu);
+	return -EOPNOTSUPP;
 }
 
 static int handle_validity(struct kvm_vcpu *vcpu)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 2f3e14f..c507789 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1494,7 +1494,15 @@ void kvm_s390_vcpu_stop(struct kvm_vcpu *vcpu)
 	spin_lock_bh(&vcpu->kvm->arch.start_stop_lock);
 	online_vcpus = atomic_read(&vcpu->kvm->online_vcpus);
 
+	/* Need to lock access to action_bits to avoid a SIGP race condition */
+	spin_lock_bh(&vcpu->arch.local_int.lock);
 	atomic_set_mask(CPUSTAT_STOPPED, &vcpu->arch.sie_block->cpuflags);
+
+	/* SIGP STOP and SIGP STOP AND STORE STATUS has been fully processed */
+	vcpu->arch.local_int.action_bits &=
+				 ~(ACTION_STOP_ON_STOP | ACTION_STORE_ON_STOP);
+	spin_unlock_bh(&vcpu->arch.local_int.lock);
+
 	__disable_ibs_on_vcpu(vcpu);
 
 	for (i = 0; i < online_vcpus; i++) {
-- 
1.8.4.2

[GIT PULL 3/5] KVM: s390: remove __cpu_is_stopped and expose is_vcpu_stopped

[Christian Borntraeger]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

The function "__cpu_is_stopped" is not used any more. Let's remove it and
expose the function "is_vcpu_stopped" instead, which is actually what we want.

This patch also converts an open coded check for CPUSTAT_STOPPED to
is_vcpu_stopped().

Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/kvm-s390.c | 7 +------
 arch/s390/kvm/kvm-s390.h | 4 ++--
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index c507789..3428953 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -926,7 +926,7 @@ static int kvm_arch_vcpu_ioctl_set_initial_psw(struct kvm_vcpu *vcpu, psw_t psw)
 {
 	int rc = 0;
 
-	if (!(atomic_read(&vcpu->arch.sie_block->cpuflags) & CPUSTAT_STOPPED))
+	if (!is_vcpu_stopped(vcpu))
 		rc = -EBUSY;
 	else {
 		vcpu->run->psw_mask = psw.mask;
@@ -1413,11 +1413,6 @@ int kvm_s390_vcpu_store_status(struct kvm_vcpu *vcpu, unsigned long addr)
 	return kvm_s390_store_status_unloaded(vcpu, addr);
 }
 
-static inline int is_vcpu_stopped(struct kvm_vcpu *vcpu)
-{
-	return atomic_read(&(vcpu)->arch.sie_block->cpuflags) & CPUSTAT_STOPPED;
-}
-
 static void __disable_ibs_on_vcpu(struct kvm_vcpu *vcpu)
 {
 	kvm_check_request(KVM_REQ_ENABLE_IBS, vcpu);
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index a8655ed..77ed846 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -45,9 +45,9 @@ do { \
 	  d_args); \
 } while (0)
 
-static inline int __cpu_is_stopped(struct kvm_vcpu *vcpu)
+static inline int is_vcpu_stopped(struct kvm_vcpu *vcpu)
 {
-	return atomic_read(&vcpu->arch.sie_block->cpuflags) & CPUSTAT_STOP_INT;
+	return atomic_read(&vcpu->arch.sie_block->cpuflags) & CPUSTAT_STOPPED;
 }
 
 static inline int kvm_is_ucontrol(struct kvm *kvm)
-- 
1.8.4.2

[GIT PULL 4/5] KVM: prepare for KVM_(S|G)ET_MP_STATE on other architectures

[Christian Borntraeger]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

Highlight the aspects of the ioctls that are actually specific to x86
and ia64. As defined restrictions (irqchip) and mp states may not apply
to other architectures, these parts are flagged to belong to x86 and ia64.

In preparation for the use of KVM_(S|G)ET_MP_STATE by s390.
Fix a spelling error (KVM_SET_MP_STATE vs. KVM_SET_MPSTATE) on the way.

Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 Documentation/virtual/kvm/api.txt | 21 ++++++++++++---------
 include/uapi/linux/kvm.h          |  3 ++-
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 0fe3649..904c61c 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -988,18 +988,20 @@ uniprocessor guests).
 
 Possible values are:
 
- - KVM_MP_STATE_RUNNABLE:        the vcpu is currently running
+ - KVM_MP_STATE_RUNNABLE:        the vcpu is currently running [x86, ia64]
  - KVM_MP_STATE_UNINITIALIZED:   the vcpu is an application processor (AP)
-                                 which has not yet received an INIT signal
+                                 which has not yet received an INIT signal [x86,
+                                 ia64]
  - KVM_MP_STATE_INIT_RECEIVED:   the vcpu has received an INIT signal, and is
-                                 now ready for a SIPI
+                                 now ready for a SIPI [x86, ia64]
  - KVM_MP_STATE_HALTED:          the vcpu has executed a HLT instruction and
-                                 is waiting for an interrupt
+                                 is waiting for an interrupt [x86, ia64]
  - KVM_MP_STATE_SIPI_RECEIVED:   the vcpu has just received a SIPI (vector
-                                 accessible via KVM_GET_VCPU_EVENTS)
+                                 accessible via KVM_GET_VCPU_EVENTS) [x86, ia64]
 
-This ioctl is only useful after KVM_CREATE_IRQCHIP.  Without an in-kernel
-irqchip, the multiprocessing state must be maintained by userspace.
+On x86 and ia64, this ioctl is only useful after KVM_CREATE_IRQCHIP. Without an
+in-kernel irqchip, the multiprocessing state must be maintained by userspace on
+these architectures.
 
 
 4.39 KVM_SET_MP_STATE
@@ -1013,8 +1015,9 @@ Returns: 0 on success; -1 on error
 Sets the vcpu's current "multiprocessing state"; see KVM_GET_MP_STATE for
 arguments.
 
-This ioctl is only useful after KVM_CREATE_IRQCHIP.  Without an in-kernel
-irqchip, the multiprocessing state must be maintained by userspace.
+On x86 and ia64, this ioctl is only useful after KVM_CREATE_IRQCHIP. Without an
+in-kernel irqchip, the multiprocessing state must be maintained by userspace on
+these architectures.
 
 
 4.40 KVM_SET_IDENTITY_MAP_ADDR
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index e11d8f1..37d4ec6 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -399,8 +399,9 @@ struct kvm_vapic_addr {
 	__u64 vapic_addr;
 };
 
-/* for KVM_SET_MPSTATE */
+/* for KVM_SET_MP_STATE */
 
+/* not all states are valid on all architectures */
 #define KVM_MP_STATE_RUNNABLE          0
 #define KVM_MP_STATE_UNINITIALIZED     1
 #define KVM_MP_STATE_INIT_RECEIVED     2
-- 
1.8.4.2

[GIT PULL 5/5] KVM: s390: implement KVM_(S|G)ET_MP_STATE for user space state control

[Christian Borntraeger]

From: David Hildenbrand <dahi@linux.vnet.ibm.com>

This patch
- adds s390 specific MP states to linux headers and documents them
- implements the KVM_{SET,GET}_MP_STATE ioctls
- enables KVM_CAP_MP_STATE
- allows user space to control the VCPU state on s390.

If user space sets the VCPU state using the ioctl KVM_SET_MP_STATE, we can disable
manual changing of the VCPU state and trust user space to do the right thing.

Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 Documentation/virtual/kvm/api.txt | 10 ++++++++--
 arch/s390/include/asm/kvm_host.h  |  1 +
 arch/s390/kvm/diag.c              |  3 ++-
 arch/s390/kvm/intercept.c         |  3 ++-
 arch/s390/kvm/kvm-s390.c          | 37 +++++++++++++++++++++++++++++++++----
 arch/s390/kvm/kvm-s390.h          |  6 ++++++
 include/uapi/linux/kvm.h          |  4 ++++
 7 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 904c61c..a41465b 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -974,7 +974,7 @@ for vm-wide capabilities.
 4.38 KVM_GET_MP_STATE
 
 Capability: KVM_CAP_MP_STATE
-Architectures: x86, ia64
+Architectures: x86, ia64, s390
 Type: vcpu ioctl
 Parameters: struct kvm_mp_state (out)
 Returns: 0 on success; -1 on error
@@ -998,6 +998,12 @@ Possible values are:
                                  is waiting for an interrupt [x86, ia64]
  - KVM_MP_STATE_SIPI_RECEIVED:   the vcpu has just received a SIPI (vector
                                  accessible via KVM_GET_VCPU_EVENTS) [x86, ia64]
+ - KVM_MP_STATE_STOPPED:         the vcpu is stopped [s390]
+ - KVM_MP_STATE_CHECK_STOP:      the vcpu is in a special error state [s390]
+ - KVM_MP_STATE_OPERATING:       the vcpu is operating (running or halted)
+                                 [s390]
+ - KVM_MP_STATE_LOAD:            the vcpu is in a special load/startup state
+                                 [s390]
 
 On x86 and ia64, this ioctl is only useful after KVM_CREATE_IRQCHIP. Without an
 in-kernel irqchip, the multiprocessing state must be maintained by userspace on
@@ -1007,7 +1013,7 @@ these architectures.
 4.39 KVM_SET_MP_STATE
 
 Capability: KVM_CAP_MP_STATE
-Architectures: x86, ia64
+Architectures: x86, ia64, s390
 Type: vcpu ioctl
 Parameters: struct kvm_mp_state (in)
 Returns: 0 on success; -1 on error
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 4181d7b..c2ba020 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -418,6 +418,7 @@ struct kvm_arch{
 	int css_support;
 	int use_irqchip;
 	int use_cmma;
+	int user_cpu_state_ctrl;
 	struct s390_io_adapter *adapters[MAX_S390_IO_ADAPTERS];
 	wait_queue_head_t ipte_wq;
 	spinlock_t start_stop_lock;
diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
index 0161675..59bd8f9 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -176,7 +176,8 @@ static int __diag_ipl_functions(struct kvm_vcpu *vcpu)
 		return -EOPNOTSUPP;
 	}
 
-	kvm_s390_vcpu_stop(vcpu);
+	if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm))
+		kvm_s390_vcpu_stop(vcpu);
 	vcpu->run->s390_reset_flags |= KVM_S390_RESET_SUBSYSTEM;
 	vcpu->run->s390_reset_flags |= KVM_S390_RESET_IPL;
 	vcpu->run->s390_reset_flags |= KVM_S390_RESET_CPU_INIT;
diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
index ac6b325..eaf4629 100644
--- a/arch/s390/kvm/intercept.c
+++ b/arch/s390/kvm/intercept.c
@@ -73,7 +73,8 @@ static int handle_stop(struct kvm_vcpu *vcpu)
 			return rc;
 	}
 
-	kvm_s390_vcpu_stop(vcpu);
+	if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm))
+		kvm_s390_vcpu_stop(vcpu);
 	return -EOPNOTSUPP;
 }
 
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 3428953..fdf88f7 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -167,6 +167,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_DEVICE_CTRL:
 	case KVM_CAP_ENABLE_CAP_VM:
 	case KVM_CAP_VM_ATTRIBUTES:
+	case KVM_CAP_MP_STATE:
 		r = 1;
 		break;
 	case KVM_CAP_NR_VCPUS:
@@ -595,7 +596,8 @@ static void kvm_s390_vcpu_initial_reset(struct kvm_vcpu *vcpu)
 	vcpu->arch.sie_block->pp = 0;
 	vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
 	kvm_clear_async_pf_completion_queue(vcpu);
-	kvm_s390_vcpu_stop(vcpu);
+	if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm))
+		kvm_s390_vcpu_stop(vcpu);
 	kvm_s390_clear_local_irqs(vcpu);
 }
 
@@ -980,13 +982,34 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu,
 				    struct kvm_mp_state *mp_state)
 {
-	return -EINVAL; /* not implemented yet */
+	/* CHECK_STOP and LOAD are not supported yet */
+	return is_vcpu_stopped(vcpu) ? KVM_MP_STATE_STOPPED :
+				       KVM_MP_STATE_OPERATING;
 }
 
 int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 				    struct kvm_mp_state *mp_state)
 {
-	return -EINVAL; /* not implemented yet */
+	int rc = 0;
+
+	/* user space knows about this interface - let it control the state */
+	vcpu->kvm->arch.user_cpu_state_ctrl = 1;
+
+	switch (mp_state->mp_state) {
+	case KVM_MP_STATE_STOPPED:
+		kvm_s390_vcpu_stop(vcpu);
+		break;
+	case KVM_MP_STATE_OPERATING:
+		kvm_s390_vcpu_start(vcpu);
+		break;
+	case KVM_MP_STATE_LOAD:
+	case KVM_MP_STATE_CHECK_STOP:
+		/* fall through - CHECK_STOP and LOAD are not supported yet */
+	default:
+		rc = -ENXIO;
+	}
+
+	return rc;
 }
 
 bool kvm_s390_cmma_enabled(struct kvm *kvm)
@@ -1284,7 +1307,13 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	if (vcpu->sigset_active)
 		sigprocmask(SIG_SETMASK, &vcpu->sigset, &sigsaved);
 
-	kvm_s390_vcpu_start(vcpu);
+	if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm)) {
+		kvm_s390_vcpu_start(vcpu);
+	} else if (is_vcpu_stopped(vcpu)) {
+		pr_err_ratelimited("kvm-s390: can't run stopped vcpu %d\n",
+				   vcpu->vcpu_id);
+		return -EINVAL;
+	}
 
 	switch (kvm_run->exit_reason) {
 	case KVM_EXIT_S390_SIEIC:
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index 77ed846..33a0e4b 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -129,6 +129,12 @@ static inline void kvm_s390_set_psw_cc(struct kvm_vcpu *vcpu, unsigned long cc)
 	vcpu->arch.sie_block->gpsw.mask |= cc << 44;
 }
 
+/* are cpu states controlled by user space */
+static inline int kvm_s390_user_cpu_state_ctrl(struct kvm *kvm)
+{
+	return kvm->arch.user_cpu_state_ctrl != 0;
+}
+
 int kvm_s390_handle_wait(struct kvm_vcpu *vcpu);
 enum hrtimer_restart kvm_s390_idle_wakeup(struct hrtimer *timer);
 void kvm_s390_tasklet(unsigned long parm);
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 37d4ec6..9b744af 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -407,6 +407,10 @@ struct kvm_vapic_addr {
 #define KVM_MP_STATE_INIT_RECEIVED     2
 #define KVM_MP_STATE_HALTED            3
 #define KVM_MP_STATE_SIPI_RECEIVED     4
+#define KVM_MP_STATE_STOPPED           5
+#define KVM_MP_STATE_CHECK_STOP        6
+#define KVM_MP_STATE_OPERATING         7
+#define KVM_MP_STATE_LOAD              8
 
 struct kvm_mp_state {
 	__u32 mp_state;
-- 
1.8.4.2

Re: [GIT PULL 0/5] KVM: s390: Let user space control the cpu states

[Paolo Bonzini]

Il 15/07/2014 15:27, Christian Borntraeger ha scritto:
> Paolo,
> 
> The following changes since commit 9f6226a762c7ae02f6a23a3d4fc552dafa57ea23:
> 
>   arch: x86: kvm: x86.c: Cleaning up variable is set more than once (2014-06-30 16:52:04 +0200)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git  tags/kvm-s390-20140715
> 
> for you to fetch changes up to 6352e4d2dd9a349024a41356148eced553e1dce4:
> 
>   KVM: s390: implement KVM_(S|G)ET_MP_STATE for user space state control (2014-07-10 14:11:17 +0200)
> 
> ----------------------------------------------------------------
> This series enables the "KVM_(S|G)ET_MP_STATE" ioctls on s390 to make
> the cpu state settable by user space.
> 
> This is necessary to avoid races in s390 SIGP/reset handling which
> happen because some SIGPs are handled in QEMU, while others are
> handled in the kernel. Together with the busy conditions as return
> value of SIGP races happen especially in areas like starting and
> stopping of CPUs. (For example, there is a program 'cpuplugd', that
> runs on several s390 distros which does automatic onlining and
> offlining on cpus.)
> 
> As soon as the MPSTATE interface is used, user space takes complete
> control of the cpu states. Otherwise the kernel will use the old way.
> 
> Therefore, the new kernel continues to work fine with old QEMUs.
> 
> ----------------------------------------------------------------
> David Hildenbrand (5):
>       KVM: s390: allow only one SIGP STOP (AND STORE STATUS) at a time
>       KVM: s390: move finalization of SIGP STOP orders to kvm_s390_vcpu_stop
>       KVM: s390: remove __cpu_is_stopped and expose is_vcpu_stopped
>       KVM: prepare for KVM_(S|G)ET_MP_STATE on other architectures
>       KVM: s390: implement KVM_(S|G)ET_MP_STATE for user space state control
> 
>  Documentation/virtual/kvm/api.txt | 31 ++++++++++++++---------
>  arch/s390/include/asm/kvm_host.h  |  1 +
>  arch/s390/kvm/diag.c              |  3 ++-
>  arch/s390/kvm/intercept.c         | 32 ++++++++++--------------
>  arch/s390/kvm/kvm-s390.c          | 52 +++++++++++++++++++++++++++++++--------
>  arch/s390/kvm/kvm-s390.h          | 10 ++++++--
>  arch/s390/kvm/sigp.c              |  7 +++++-
>  include/uapi/linux/kvm.h          |  7 +++++-
>  8 files changed, 98 insertions(+), 45 deletions(-)
> 

Alex, wdyt about this patch series?  Does it make sense to use
KVM_GET/SET_MP_STATE or should the one-reg interface be a better match?

It's a bit weird that running and halted map to the same mp_state on
s390.  I would be more confident that KVM_GET/SET_MP_STATE is the right
choice if it had at least the KVM_MP_STATE_RUNNABLE and
KVM_MP_STATE_HALTED.  Christian, where is the halted state stored, is it
in the PSW?

Paolo

Re: [GIT PULL 0/5] KVM: s390: Let user space control the cpu states

[Christian Borntraeger]

On 18/07/14 16:49, Paolo Bonzini wrote:
> Il 15/07/2014 15:27, Christian Borntraeger ha scritto:
>> Paolo,
>>
>> The following changes since commit 9f6226a762c7ae02f6a23a3d4fc552dafa57ea23:
>>
>>   arch: x86: kvm: x86.c: Cleaning up variable is set more than once (2014-06-30 16:52:04 +0200)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git  tags/kvm-s390-20140715
>>
>> for you to fetch changes up to 6352e4d2dd9a349024a41356148eced553e1dce4:
>>
>>   KVM: s390: implement KVM_(S|G)ET_MP_STATE for user space state control (2014-07-10 14:11:17 +0200)
>>
>> ----------------------------------------------------------------
>> This series enables the "KVM_(S|G)ET_MP_STATE" ioctls on s390 to make
>> the cpu state settable by user space.
>>
>> This is necessary to avoid races in s390 SIGP/reset handling which
>> happen because some SIGPs are handled in QEMU, while others are
>> handled in the kernel. Together with the busy conditions as return
>> value of SIGP races happen especially in areas like starting and
>> stopping of CPUs. (For example, there is a program 'cpuplugd', that
>> runs on several s390 distros which does automatic onlining and
>> offlining on cpus.)
>>
>> As soon as the MPSTATE interface is used, user space takes complete
>> control of the cpu states. Otherwise the kernel will use the old way.
>>
>> Therefore, the new kernel continues to work fine with old QEMUs.
>>
>> ----------------------------------------------------------------
>> David Hildenbrand (5):
>>       KVM: s390: allow only one SIGP STOP (AND STORE STATUS) at a time
>>       KVM: s390: move finalization of SIGP STOP orders to kvm_s390_vcpu_stop
>>       KVM: s390: remove __cpu_is_stopped and expose is_vcpu_stopped
>>       KVM: prepare for KVM_(S|G)ET_MP_STATE on other architectures
>>       KVM: s390: implement KVM_(S|G)ET_MP_STATE for user space state control
>>
>>  Documentation/virtual/kvm/api.txt | 31 ++++++++++++++---------
>>  arch/s390/include/asm/kvm_host.h  |  1 +
>>  arch/s390/kvm/diag.c              |  3 ++-
>>  arch/s390/kvm/intercept.c         | 32 ++++++++++--------------
>>  arch/s390/kvm/kvm-s390.c          | 52 +++++++++++++++++++++++++++++++--------
>>  arch/s390/kvm/kvm-s390.h          | 10 ++++++--
>>  arch/s390/kvm/sigp.c              |  7 +++++-
>>  include/uapi/linux/kvm.h          |  7 +++++-
>>  8 files changed, 98 insertions(+), 45 deletions(-)
>>
> 
> Alex, wdyt about this patch series?  Does it make sense to use
> KVM_GET/SET_MP_STATE or should the one-reg interface be a better match?
> 
> It's a bit weird that running and halted map to the same mp_state on
> s390.  I would be more confident that KVM_GET/SET_MP_STATE is the right
> choice if it had at least the KVM_MP_STATE_RUNNABLE and
> KVM_MP_STATE_HALTED.  Christian, where is the halted state stored, is it
> in the PSW?

Yes, there is a bit in the PSW called wait. It is pretty much similar to 
the HLT instruction: The CPU does not continue the execution, but it will
accept all interrupts that are not fenced via control registers or PSW. 
Its mostly used for cpu_idle. KVM on s390 is always doing the wait in the
kernel (IOW, we always have something like halt_in_kernel), except for the
disabled wait which boils down to no execution and all interrupts off. This is
used for error states of the OS and a special case (we set the guest in the
panic state).

So having such a state wont buy us much. It would be even wrong, because
we want our MP_STATE defines to be a 1:1 match of the states that are defined
in the architecture as proper CPU states. Some of the SIGP calls will return the
state of the target CPU and that depends on the CPU state as defined in the
architecture. The wait bit does not have an influence on the return value.


So instead of modelling as x86, we actually want to model the mp_states as 
defined for the architecture. What I can see from the x86 defines, its somewhat
similar: it matches the x86 architecture and not the QEMU model.


ONEREG would work as well (you can make it work with almost every interface),
but mp_state looks like a better fit to me, because its is an interface to define
CPU states that are not directly tied to runtime registers.
Furthermore, the bits in PSW and registers are only considered by the HW if
the CPU is in the operating state. By using ONEREG, we would have a "register"
that does not follow that rule.

Christian

PS: See SA22-7832 chapter 4-1 (http://publibfi.boulder.ibm.com/epubs/pdf/dz9zr009.pdf !! its big)
---snip---
The stopped, operating, load, and check-stop states
are four mutually exclusive states of the CPU. When
the CPU is in the stopped state, instructions and
interruptions, other than the restart interruption, are
not executed. In the operating state, the CPU exe-
cutes instructions and takes interruptions, subject to
the control of the program-status word (PSW) and
control registers, and in the manner specified by the
setting of the operator-facility rate control. The CPU
is in the load state during the initial-program-loading
operation of ESA/390. The CPU enters the check-
stop state only as the result of machine malfunctions.
---snip---

Re: [GIT PULL 0/5] KVM: s390: Let user space control the cpu states

[Paolo Bonzini]

Il 21/07/2014 09:47, Christian Borntraeger ha scritto:
> So having such a state wont buy us much. It would be even wrong, because
> we want our MP_STATE defines to be a 1:1 match of the states that are defined
> in the architecture as proper CPU states. Some of the SIGP calls will return the
> state of the target CPU and that depends on the CPU state as defined in the
> architecture. The wait bit does not have an influence on the return value.

Thanks for the explanation.

Paolo