kvm: x86: hyperv: delete dead code in kvm_hv_hypercall()
Subsystems (4)
PATCH 1/1 kvm: x86: hyperv: delete dead code in kvm_hv_hypercall()
"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.
arch/x86/kvm/hyperv.c +7 -8
@@ -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 ; }
Quoted 7 lines · ----- [email protected] 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 <[email protected]>
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
Quoted 8 lines · Liran Alon
Quoted 5 lines · ----- [email protected] 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.
Quoted 5 lines
Quoted 2 lines
Signed-off-by Dan Carpenter <[email protected]>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
Paolo Bonzini
maintainerQuoted 6 lines · Liran Alon
Quoted 6 lines
"
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.
Quoted 6 lines
Quoted 2 lines
Signed-off-by Dan Carpenter <[email protected]>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 requirerep_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
Quoted 53 lines · Dan Carpenter
"
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 <[email protected]>arch/x86/kvm/hyperv.c +6
-6@@ -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 :
arch/x86/kvm/hyperv.c int kvm_hv_hypercall(struct kvm_vcpu *vcpu) · line 1344
Quoted 10 lines
arch/x86/kvm/hyperv.c +1
-2- 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