diff mbox series

Fix unpack

Message ID 20191113140306.2952-1-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series Fix unpack | expand

Commit Message

Janosch Frank Nov. 13, 2019, 2:03 p.m. UTC 
That should be easier to read :)

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 arch/s390/kvm/pv.c | 60 +++++++++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 25 deletions(-)

Comments

Thomas Huth Nov. 13, 2019, 2:19 p.m. UTC | #1 
On 13/11/2019 15.03, Janosch Frank wrote:
> That should be easier to read :)
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  arch/s390/kvm/pv.c | 60 +++++++++++++++++++++++++++-------------------
>  1 file changed, 35 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
> index 94cf16f40f25..fd73afb33b20 100644
> --- a/arch/s390/kvm/pv.c
> +++ b/arch/s390/kvm/pv.c
> @@ -195,43 +195,53 @@ int kvm_s390_pv_set_sec_parms(struct kvm *kvm,
>  	return 0;
>  }
>  
> -int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
> -		       unsigned long tweak)
> +static int unpack_one(struct kvm *kvm, unsigned long addr, u64 tweak[2])
>  {
> -	int i, rc = 0;
> +	int rc;
>  	struct uv_cb_unp uvcb = {
>  		.header.cmd = UVC_CMD_UNPACK_IMG,
>  		.header.len = sizeof(uvcb),
>  		.guest_handle = kvm_s390_pv_handle(kvm),
> -		.tweak[0] = tweak
> +		.gaddr = addr,
> +		.tweak[0] = tweak[0],
> +		.tweak[1] = tweak[1],
>  	};
>  
> -	if (addr & ~PAGE_MASK || size & ~PAGE_MASK)
> -		return -EINVAL;
> +	rc = uv_call(0, (u64)&uvcb);
> +	if (!rc)
> +		return rc;
> +	if (uvcb.header.rc == 0x10a) {
> +		/* If not yet mapped fault and retry */
> +		rc = gmap_fault(kvm->arch.gmap, uvcb.gaddr,
> +				FAULT_FLAG_WRITE);
> +		if (!rc)
> +			return -EAGAIN;
> +	}
> +	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: failed addr %llx rc %x rrc %x",
> +		 uvcb.gaddr, uvcb.header.rc, uvcb.header.rrc);
> +	return rc;
> +}
>  
> +int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
> +		       unsigned long tweak)
> +{
> +	int rc = 0;
> +	u64 tw[2] = {tweak, 0};
> +
> +	if (addr & ~PAGE_MASK || !size || size & ~PAGE_MASK)
> +		return -EINVAL;
>  
>  	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: start addr %lx size %lx",
>  		 addr, size);
> -	for (i = 0; i < size / PAGE_SIZE; i++) {
> -		uvcb.gaddr = addr + i * PAGE_SIZE;
> -		uvcb.tweak[1] = i * PAGE_SIZE;
> -retry:
> -		rc = uv_call(0, (u64)&uvcb);
> -		if (!rc)
> +	while (tw[1] < size) {
> +		rc = unpack_one(kvm, addr, tw);
> +		if (rc == -EAGAIN)
>  			continue;
> -		/* If not yet mapped fault and retry */
> -		if (uvcb.header.rc == 0x10a) {
> -			rc = gmap_fault(kvm->arch.gmap, uvcb.gaddr,
> -					FAULT_FLAG_WRITE);
> -			if (rc)
> -				return rc;
> -			goto retry;
> -		}
> -		VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: failed addr %llx rc %x rrc %x",
> -			 uvcb.gaddr, uvcb.header.rc, uvcb.header.rrc);
> -		break;
> +		if (rc)
> +			break;
> +		addr += PAGE_SIZE;
> +		tw[1] += PAGE_SIZE;
>  	}
> -	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: finished with rc %x rrc %x",
> -		 uvcb.header.rc, uvcb.header.rrc);
> +	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: finished rc %x", rc);
>  	return rc;
>  }
> 

Yes, I think that will be better, thanks!

 Thomas
Cornelia Huck Nov. 13, 2019, 2:36 p.m. UTC | #2 
On Wed, 13 Nov 2019 09:03:06 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:

You seem to have dropped some cc:s :(

> That should be easier to read :)
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  arch/s390/kvm/pv.c | 60 +++++++++++++++++++++++++++-------------------
>  1 file changed, 35 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
> index 94cf16f40f25..fd73afb33b20 100644
> --- a/arch/s390/kvm/pv.c
> +++ b/arch/s390/kvm/pv.c
> @@ -195,43 +195,53 @@ int kvm_s390_pv_set_sec_parms(struct kvm *kvm,
>  	return 0;
>  }
>  
> -int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
> -		       unsigned long tweak)
> +static int unpack_one(struct kvm *kvm, unsigned long addr, u64 tweak[2])
>  {
> -	int i, rc = 0;
> +	int rc;
>  	struct uv_cb_unp uvcb = {
>  		.header.cmd = UVC_CMD_UNPACK_IMG,
>  		.header.len = sizeof(uvcb),
>  		.guest_handle = kvm_s390_pv_handle(kvm),
> -		.tweak[0] = tweak
> +		.gaddr = addr,
> +		.tweak[0] = tweak[0],
> +		.tweak[1] = tweak[1],
>  	};
>  
> -	if (addr & ~PAGE_MASK || size & ~PAGE_MASK)
> -		return -EINVAL;
> +	rc = uv_call(0, (u64)&uvcb);
> +	if (!rc)
> +		return rc;
> +	if (uvcb.header.rc == 0x10a) {
> +		/* If not yet mapped fault and retry */
> +		rc = gmap_fault(kvm->arch.gmap, uvcb.gaddr,
> +				FAULT_FLAG_WRITE);
> +		if (!rc)
> +			return -EAGAIN;
> +	}
> +	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: failed addr %llx rc %x rrc %x",
> +		 uvcb.gaddr, uvcb.header.rc, uvcb.header.rrc);
> +	return rc;
> +}
>  
> +int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
> +		       unsigned long tweak)
> +{
> +	int rc = 0;
> +	u64 tw[2] = {tweak, 0};
> +
> +	if (addr & ~PAGE_MASK || !size || size & ~PAGE_MASK)
> +		return -EINVAL;
>  
>  	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: start addr %lx size %lx",
>  		 addr, size);
> -	for (i = 0; i < size / PAGE_SIZE; i++) {
> -		uvcb.gaddr = addr + i * PAGE_SIZE;
> -		uvcb.tweak[1] = i * PAGE_SIZE;
> -retry:
> -		rc = uv_call(0, (u64)&uvcb);
> -		if (!rc)
> +	while (tw[1] < size) {
> +		rc = unpack_one(kvm, addr, tw);
> +		if (rc == -EAGAIN)
>  			continue;
> -		/* If not yet mapped fault and retry */
> -		if (uvcb.header.rc == 0x10a) {
> -			rc = gmap_fault(kvm->arch.gmap, uvcb.gaddr,
> -					FAULT_FLAG_WRITE);
> -			if (rc)
> -				return rc;
> -			goto retry;
> -		}
> -		VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: failed addr %llx rc %x rrc %x",
> -			 uvcb.gaddr, uvcb.header.rc, uvcb.header.rrc);
> -		break;
> +		if (rc)
> +			break;
> +		addr += PAGE_SIZE;
> +		tw[1] += PAGE_SIZE;
>  	}
> -	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: finished with rc %x rrc %x",
> -		 uvcb.header.rc, uvcb.header.rrc);
> +	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: finished rc %x", rc);
>  	return rc;
>  }

Yes, this looks more readable.
diff mbox series

Patch

diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
index 94cf16f40f25..fd73afb33b20 100644
--- a/arch/s390/kvm/pv.c
+++ b/arch/s390/kvm/pv.c
@@ -195,43 +195,53 @@  int kvm_s390_pv_set_sec_parms(struct kvm *kvm,
 	return 0;
 }
 
-int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
-		       unsigned long tweak)
+static int unpack_one(struct kvm *kvm, unsigned long addr, u64 tweak[2])
 {
-	int i, rc = 0;
+	int rc;
 	struct uv_cb_unp uvcb = {
 		.header.cmd = UVC_CMD_UNPACK_IMG,
 		.header.len = sizeof(uvcb),
 		.guest_handle = kvm_s390_pv_handle(kvm),
-		.tweak[0] = tweak
+		.gaddr = addr,
+		.tweak[0] = tweak[0],
+		.tweak[1] = tweak[1],
 	};
 
-	if (addr & ~PAGE_MASK || size & ~PAGE_MASK)
-		return -EINVAL;
+	rc = uv_call(0, (u64)&uvcb);
+	if (!rc)
+		return rc;
+	if (uvcb.header.rc == 0x10a) {
+		/* If not yet mapped fault and retry */
+		rc = gmap_fault(kvm->arch.gmap, uvcb.gaddr,
+				FAULT_FLAG_WRITE);
+		if (!rc)
+			return -EAGAIN;
+	}
+	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: failed addr %llx rc %x rrc %x",
+		 uvcb.gaddr, uvcb.header.rc, uvcb.header.rrc);
+	return rc;
+}
 
+int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
+		       unsigned long tweak)
+{
+	int rc = 0;
+	u64 tw[2] = {tweak, 0};
+
+	if (addr & ~PAGE_MASK || !size || size & ~PAGE_MASK)
+		return -EINVAL;
 
 	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: start addr %lx size %lx",
 		 addr, size);
-	for (i = 0; i < size / PAGE_SIZE; i++) {
-		uvcb.gaddr = addr + i * PAGE_SIZE;
-		uvcb.tweak[1] = i * PAGE_SIZE;
-retry:
-		rc = uv_call(0, (u64)&uvcb);
-		if (!rc)
+	while (tw[1] < size) {
+		rc = unpack_one(kvm, addr, tw);
+		if (rc == -EAGAIN)
 			continue;
-		/* If not yet mapped fault and retry */
-		if (uvcb.header.rc == 0x10a) {
-			rc = gmap_fault(kvm->arch.gmap, uvcb.gaddr,
-					FAULT_FLAG_WRITE);
-			if (rc)
-				return rc;
-			goto retry;
-		}
-		VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: failed addr %llx rc %x rrc %x",
-			 uvcb.gaddr, uvcb.header.rc, uvcb.header.rrc);
-		break;
+		if (rc)
+			break;
+		addr += PAGE_SIZE;
+		tw[1] += PAGE_SIZE;
 	}
-	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: finished with rc %x rrc %x",
-		 uvcb.header.rc, uvcb.header.rrc);
+	VM_EVENT(kvm, 3, "PROTVIRT VM UNPACK: finished rc %x", rc);
 	return rc;
 }