All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kvm: x86: hyperv: delete dead code in kvm_hv_hypercall()
@ 2018-03-17 11:48 Dan Carpenter
  2018-03-17 15:11 ` Liran Alon
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Dan Carpenter @ 2018-03-17 11:48 UTC (permalink / raw)
  To: kernel-janitors

"rep_done" is always zero so the "(((u64)rep_done & 0xfff) << 32)"
expression is just zero.  We can remove the "res" temporary variable as
well and just use "ret" directly.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 53bd1913b6fd..80c34c83f882 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1266,8 +1266,8 @@ static u16 kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, bool fast, u64 param)
 
 int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 {
-	u64 param, ingpa, outgpa, ret;
-	uint16_t code, rep_idx, rep_cnt, res = HV_STATUS_SUCCESS, rep_done = 0;
+	u64 param, ingpa, outgpa, ret = HV_STATUS_SUCCESS;
+	uint16_t code, rep_idx, rep_cnt;
 	bool fast, longmode;
 
 	/*
@@ -1306,7 +1306,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 
 	/* Hypercall continuation is not supported yet */
 	if (rep_cnt || rep_idx) {
-		res = HV_STATUS_INVALID_HYPERCALL_CODE;
+		ret = HV_STATUS_INVALID_HYPERCALL_CODE;
 		goto set_result;
 	}
 
@@ -1315,14 +1315,14 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 		kvm_vcpu_on_spin(vcpu, true);
 		break;
 	case HVCALL_SIGNAL_EVENT:
-		res = kvm_hvcall_signal_event(vcpu, fast, ingpa);
-		if (res != HV_STATUS_INVALID_PORT_ID)
+		ret = kvm_hvcall_signal_event(vcpu, fast, ingpa);
+		if (ret != HV_STATUS_INVALID_PORT_ID)
 			break;
 		/* maybe userspace knows this conn_id: fall through */
 	case HVCALL_POST_MESSAGE:
 		/* don't bother userspace if it has no way to handle it */
 		if (!vcpu_to_synic(vcpu)->active) {
-			res = HV_STATUS_INVALID_HYPERCALL_CODE;
+			ret = HV_STATUS_INVALID_HYPERCALL_CODE;
 			break;
 		}
 		vcpu->run->exit_reason = KVM_EXIT_HYPERV;
@@ -1334,12 +1334,11 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 				kvm_hv_hypercall_complete_userspace;
 		return 0;
 	default:
-		res = HV_STATUS_INVALID_HYPERCALL_CODE;
+		ret = HV_STATUS_INVALID_HYPERCALL_CODE;
 		break;
 	}
 
 set_result:
-	ret = res | (((u64)rep_done & 0xfff) << 32);
 	kvm_hv_hypercall_set_result(vcpu, ret);
 	return 1;
 }

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: x86: hyperv: delete dead code in kvm_hv_hypercall()
  2018-03-17 11:48 [PATCH] kvm: x86: hyperv: delete dead code in kvm_hv_hypercall() Dan Carpenter
@ 2018-03-17 15:11 ` Liran Alon
  2018-03-18  3:18 ` Dan Carpenter
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Liran Alon @ 2018-03-17 15:11 UTC (permalink / raw)
  To: kernel-janitors


----- dan.carpenter@oracle.com wrote:

> "rep_done" is always zero so the "(((u64)rep_done & 0xfff) << 32)"
> expression is just zero.  We can remove the "res" temporary variable
> as
> well and just use "ret" directly.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 

I think this commit should better be dropped than applied.
The original code makes sense.
It will make it easier to implement an hyper-v rep hypercall (which require rep_done).
I don't think it makes current code much more complicated than it is without it.

-Liran

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: x86: hyperv: delete dead code in kvm_hv_hypercall()
  2018-03-17 11:48 [PATCH] kvm: x86: hyperv: delete dead code in kvm_hv_hypercall() Dan Carpenter
  2018-03-17 15:11 ` Liran Alon
@ 2018-03-18  3:18 ` Dan Carpenter
  2018-03-19 16:01 ` Paolo Bonzini
  2018-03-23 19:12 ` Paolo Bonzini
  3 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2018-03-18  3:18 UTC (permalink / raw)
  To: kernel-janitors

On Sat, Mar 17, 2018 at 08:11:42AM -0700, Liran Alon wrote:
> 
> ----- dan.carpenter@oracle.com wrote:
> 
> > "rep_done" is always zero so the "(((u64)rep_done & 0xfff) << 32)"
> > expression is just zero.  We can remove the "res" temporary variable
> > as
> > well and just use "ret" directly.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> 
> I think this commit should better be dropped than applied.
> The original code makes sense.

It causes static checker warnings and kernel style says there shouldn't
be code without a user.  Plus we're not really deleting it, it's still
there in the git log.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: x86: hyperv: delete dead code in kvm_hv_hypercall()
  2018-03-17 11:48 [PATCH] kvm: x86: hyperv: delete dead code in kvm_hv_hypercall() Dan Carpenter
  2018-03-17 15:11 ` Liran Alon
  2018-03-18  3:18 ` Dan Carpenter
@ 2018-03-19 16:01 ` Paolo Bonzini
  2018-03-23 19:12 ` Paolo Bonzini
  3 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2018-03-19 16:01 UTC (permalink / raw)
  To: kernel-janitors

On 17/03/2018 16:11, Liran Alon wrote:
> 
>> "rep_done" is always zero so the "(((u64)rep_done & 0xfff) << 32)"
>> expression is just zero.  We can remove the "res" temporary variable
>> as
>> well and just use "ret" directly.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
> I think this commit should better be dropped than applied.
> The original code makes sense.
> It will make it easier to implement an hyper-v rep hypercall (which require rep_done).
> I don't think it makes current code much more complicated than it is without it.

If we really want to be ready for rep hypercalls, it would be better to
have a macro

#define HVCALL_MAKE_RESULT(retcode, rep_done)

and call it with a constant 0 as the second argument.  A variable that
is never written except in the initializer is only adding noise.

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: x86: hyperv: delete dead code in kvm_hv_hypercall()
  2018-03-17 11:48 [PATCH] kvm: x86: hyperv: delete dead code in kvm_hv_hypercall() Dan Carpenter
                   ` (2 preceding siblings ...)
  2018-03-19 16:01 ` Paolo Bonzini
@ 2018-03-23 19:12 ` Paolo Bonzini
  3 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2018-03-23 19:12 UTC (permalink / raw)
  To: kernel-janitors

On 17/03/2018 12:48, Dan Carpenter wrote:
> "rep_done" is always zero so the "(((u64)rep_done & 0xfff) << 32)"
> expression is just zero.  We can remove the "res" temporary variable as
> well and just use "ret" directly.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 53bd1913b6fd..80c34c83f882 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1266,8 +1266,8 @@ static u16 kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, bool fast, u64 param)
>  
>  int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  {
> -	u64 param, ingpa, outgpa, ret;
> -	uint16_t code, rep_idx, rep_cnt, res = HV_STATUS_SUCCESS, rep_done = 0;
> +	u64 param, ingpa, outgpa, ret = HV_STATUS_SUCCESS;
> +	uint16_t code, rep_idx, rep_cnt;
>  	bool fast, longmode;
>  
>  	/*
> @@ -1306,7 +1306,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  
>  	/* Hypercall continuation is not supported yet */
>  	if (rep_cnt || rep_idx) {
> -		res = HV_STATUS_INVALID_HYPERCALL_CODE;
> +		ret = HV_STATUS_INVALID_HYPERCALL_CODE;
>  		goto set_result;
>  	}
>  
> @@ -1315,14 +1315,14 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  		kvm_vcpu_on_spin(vcpu, true);
>  		break;
>  	case HVCALL_SIGNAL_EVENT:
> -		res = kvm_hvcall_signal_event(vcpu, fast, ingpa);
> -		if (res != HV_STATUS_INVALID_PORT_ID)
> +		ret = kvm_hvcall_signal_event(vcpu, fast, ingpa);
> +		if (ret != HV_STATUS_INVALID_PORT_ID)
>  			break;
>  		/* maybe userspace knows this conn_id: fall through */
>  	case HVCALL_POST_MESSAGE:
>  		/* don't bother userspace if it has no way to handle it */
>  		if (!vcpu_to_synic(vcpu)->active) {
> -			res = HV_STATUS_INVALID_HYPERCALL_CODE;
> +			ret = HV_STATUS_INVALID_HYPERCALL_CODE;
>  			break;
>  		}
>  		vcpu->run->exit_reason = KVM_EXIT_HYPERV;
> @@ -1334,12 +1334,11 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  				kvm_hv_hypercall_complete_userspace;
>  		return 0;
>  	default:
> -		res = HV_STATUS_INVALID_HYPERCALL_CODE;
> +		ret = HV_STATUS_INVALID_HYPERCALL_CODE;
>  		break;
>  	}
>  
>  set_result:
> -	ret = res | (((u64)rep_done & 0xfff) << 32);
>  	kvm_hv_hypercall_set_result(vcpu, ret);
>  	return 1;
>  }
> 

Queued, thanks.

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-03-23 19:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-17 11:48 [PATCH] kvm: x86: hyperv: delete dead code in kvm_hv_hypercall() Dan Carpenter
2018-03-17 15:11 ` Liran Alon
2018-03-18  3:18 ` Dan Carpenter
2018-03-19 16:01 ` Paolo Bonzini
2018-03-23 19:12 ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.