[PATCH 4/6] KVM: s390: fix sigp set prefix status stored cases

2012-06-26 Thread Cornelia Huck
From: Heiko Carstens heiko.carst...@de.ibm.com

If an invalid parameter is passed or the addressed cpu is in an
incorrect state sigp set prefix will store a status.
This status must only have bits set as defined by the architecture.
The current kvm implementation missed to clear bits and also did
not set the intended status bit (and instead of or operation).

Signed-off-by: Heiko Carstens heiko.carst...@de.ibm.com
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
---
 arch/s390/kvm/sigp.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
index caccc0e..ca544d5 100644
--- a/arch/s390/kvm/sigp.c
+++ b/arch/s390/kvm/sigp.c
@@ -207,6 +207,7 @@ static int __sigp_set_prefix(struct kvm_vcpu *vcpu, u16 
cpu_addr, u32 address,
address = address  0x7fffe000u;
if (copy_from_guest_absolute(vcpu, tmp, address, 1) ||
   copy_from_guest_absolute(vcpu, tmp, address + PAGE_SIZE, 1)) {
+   *reg = 0xUL;
*reg |= SIGP_STATUS_INVALID_PARAMETER;
return 1; /* invalid parameter */
}
@@ -220,8 +221,9 @@ static int __sigp_set_prefix(struct kvm_vcpu *vcpu, u16 
cpu_addr, u32 address,
li = fi-local_int[cpu_addr];
 
if (li == NULL) {
+   *reg = 0xUL;
+   *reg |= SIGP_STATUS_INCORRECT_STATE;
rc = 1; /* incorrect state */
-   *reg = SIGP_STATUS_INCORRECT_STATE;
kfree(inti);
goto out_fi;
}
@@ -229,8 +231,9 @@ static int __sigp_set_prefix(struct kvm_vcpu *vcpu, u16 
cpu_addr, u32 address,
spin_lock_bh(li-lock);
/* cpu must be in stopped state */
if (!(atomic_read(li-cpuflags)  CPUSTAT_STOPPED)) {
+   *reg = 0xUL;
+   *reg |= SIGP_STATUS_INCORRECT_STATE;
rc = 1; /* incorrect state */
-   *reg = SIGP_STATUS_INCORRECT_STATE;
kfree(inti);
goto out_li;
}
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/6] KVM: s390: fix sigp set prefix status stored cases

2012-06-26 Thread Alexander Graf

On 26.06.2012, at 16:06, Cornelia Huck wrote:

 From: Heiko Carstens heiko.carst...@de.ibm.com
 
 If an invalid parameter is passed or the addressed cpu is in an
 incorrect state sigp set prefix will store a status.
 This status must only have bits set as defined by the architecture.
 The current kvm implementation missed to clear bits and also did
 not set the intended status bit (and instead of or operation).
 
 Signed-off-by: Heiko Carstens heiko.carst...@de.ibm.com
 Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com

What was the net effect of this for a guest? Any problems rising from it?


Alex

 ---
 arch/s390/kvm/sigp.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
 index caccc0e..ca544d5 100644
 --- a/arch/s390/kvm/sigp.c
 +++ b/arch/s390/kvm/sigp.c
 @@ -207,6 +207,7 @@ static int __sigp_set_prefix(struct kvm_vcpu *vcpu, u16 
 cpu_addr, u32 address,
   address = address  0x7fffe000u;
   if (copy_from_guest_absolute(vcpu, tmp, address, 1) ||
  copy_from_guest_absolute(vcpu, tmp, address + PAGE_SIZE, 1)) {
 + *reg = 0xUL;
   *reg |= SIGP_STATUS_INVALID_PARAMETER;
   return 1; /* invalid parameter */
   }
 @@ -220,8 +221,9 @@ static int __sigp_set_prefix(struct kvm_vcpu *vcpu, u16 
 cpu_addr, u32 address,
   li = fi-local_int[cpu_addr];
 
   if (li == NULL) {
 + *reg = 0xUL;
 + *reg |= SIGP_STATUS_INCORRECT_STATE;
   rc = 1; /* incorrect state */
 - *reg = SIGP_STATUS_INCORRECT_STATE;
   kfree(inti);
   goto out_fi;
   }
 @@ -229,8 +231,9 @@ static int __sigp_set_prefix(struct kvm_vcpu *vcpu, u16 
 cpu_addr, u32 address,
   spin_lock_bh(li-lock);
   /* cpu must be in stopped state */
   if (!(atomic_read(li-cpuflags)  CPUSTAT_STOPPED)) {
 + *reg = 0xUL;
 + *reg |= SIGP_STATUS_INCORRECT_STATE;
   rc = 1; /* incorrect state */
 - *reg = SIGP_STATUS_INCORRECT_STATE;
   kfree(inti);
   goto out_li;
   }
 -- 
 1.7.10.4
 

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/6] KVM: s390: fix sigp set prefix status stored cases

2012-06-26 Thread Cornelia Huck
On Tue, 26 Jun 2012 16:56:08 +0200
Alexander Graf ag...@suse.de wrote:

 
 On 26.06.2012, at 16:06, Cornelia Huck wrote:
 
  From: Heiko Carstens heiko.carst...@de.ibm.com
  
  If an invalid parameter is passed or the addressed cpu is in an
  incorrect state sigp set prefix will store a status.
  This status must only have bits set as defined by the architecture.
  The current kvm implementation missed to clear bits and also did
  not set the intended status bit (and instead of or operation).
  
  Signed-off-by: Heiko Carstens heiko.carst...@de.ibm.com
  Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
 
 What was the net effect of this for a guest? Any problems rising from it?

The guest might see some unexpected status bits if it did call sigp set
prefix in an incorrect way. I'm not aware that anybody has actually
seen that.

Cornelia

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/6] KVM: s390: fix sigp set prefix status stored cases

2012-06-26 Thread Alexander Graf


Am 26.06.2012 um 17:39 schrieb Cornelia Huck cornelia.h...@de.ibm.com:

 On Tue, 26 Jun 2012 16:56:08 +0200
 Alexander Graf ag...@suse.de wrote:
 
 
 On 26.06.2012, at 16:06, Cornelia Huck wrote:
 
 From: Heiko Carstens heiko.carst...@de.ibm.com
 
 If an invalid parameter is passed or the addressed cpu is in an
 incorrect state sigp set prefix will store a status.
 This status must only have bits set as defined by the architecture.
 The current kvm implementation missed to clear bits and also did
 not set the intended status bit (and instead of or operation).
 
 Signed-off-by: Heiko Carstens heiko.carst...@de.ibm.com
 Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
 
 What was the net effect of this for a guest? Any problems rising from it?
 
 The guest might see some unexpected status bits if it did call sigp set
 prefix in an incorrect way. I'm not aware that anybody has actually
 seen that.

Yeah, we only need the set prefix on vm init and here it should always succeed, 
so this one is not all that urgent :).

Alex

 
 Cornelia
 
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html