| Message ID | 20220824153011.4004573-1-scgl@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: Pass initialized arg even if unused | expand |
On Wed, Aug 24, 2022 at 05:30:11PM +0200, Janis Schoetterl-Glausch wrote: > This silences smatch warnings reported by kbuild bot: > arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot'. > arch/s390/kvm/gaccess.c:1064 access_guest_with_key() error: uninitialized symbol 'prot'. > > This is because it cannot tell that the value is not used in this case. > The trans_exc* only examine prot if code is PGM_PROTECTION. > Pass a dummy value for other codes. > > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> > --- > arch/s390/kvm/gaccess.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) ... > @@ -503,6 +505,7 @@ static int trans_exc_ending(struct kvm_vcpu *vcpu, int code, unsigned long gva, > > switch (code) { > case PGM_PROTECTION: > + WARN(unlikely(prot == PROT_NONE), "Invalid prot argument"); The WARN macro comes with unlikely, please get rid of unlikely here. Also: > + case PROT_NONE: > + /* We should never get here, acts like termination */ Why not put it here? And make it WARN_ON_ONCE() in addition? > gpa = kvm_s390_real_to_abs(vcpu, ga); > - if (kvm_is_error_gpa(vcpu->kvm, gpa)) > + if (kvm_is_error_gpa(vcpu->kvm, gpa)) { > rc = PGM_ADDRESSING; > + prot = PROT_NONE; > + } ... > + if (rc == PGM_PROTECTION) > + prot = PROT_TYPE_KEYC; > + else > + prot = PROT_NONE; For both cases I would suggest to preinitialize prot with PROT_NONE in order to keep the code smaller - but not my call.
On Thu, 2022-08-25 at 10:30 +0200, Heiko Carstens wrote: > On Wed, Aug 24, 2022 at 05:30:11PM +0200, Janis Schoetterl-Glausch wrote: > > This silences smatch warnings reported by kbuild bot: > > arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot'. > > arch/s390/kvm/gaccess.c:1064 access_guest_with_key() error: uninitialized symbol 'prot'. > > > > This is because it cannot tell that the value is not used in this case. > > The trans_exc* only examine prot if code is PGM_PROTECTION. > > Pass a dummy value for other codes. > > > > Reported-by: kernel test robot <lkp@intel.com> > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> > > --- > > arch/s390/kvm/gaccess.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) [...] > > gpa = kvm_s390_real_to_abs(vcpu, ga); > > - if (kvm_is_error_gpa(vcpu->kvm, gpa)) > > + if (kvm_is_error_gpa(vcpu->kvm, gpa)) { > > rc = PGM_ADDRESSING; > > + prot = PROT_NONE; > > + } > ... > > + if (rc == PGM_PROTECTION) > > + prot = PROT_TYPE_KEYC; > > + else > > + prot = PROT_NONE; > > For both cases I would suggest to preinitialize prot with PROT_NONE in > order to keep the code smaller - but not my call. I chose this to make it really obvious that PROT_NONE is used only in cases where code != PGM_PROTECTION.
diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c index 082ec5f2c3a5..89186701116a 100644 --- a/arch/s390/kvm/gaccess.c +++ b/arch/s390/kvm/gaccess.c @@ -489,6 +489,8 @@ enum prot_type { PROT_TYPE_ALC = 2, PROT_TYPE_DAT = 3, PROT_TYPE_IEP = 4, + /* Dummy value for passing an initialized value when code != PGM_PROTECTION */ + PROT_NONE, }; static int trans_exc_ending(struct kvm_vcpu *vcpu, int code, unsigned long gva, u8 ar, @@ -503,6 +505,7 @@ static int trans_exc_ending(struct kvm_vcpu *vcpu, int code, unsigned long gva, switch (code) { case PGM_PROTECTION: + WARN(unlikely(prot == PROT_NONE), "Invalid prot argument"); switch (prot) { case PROT_TYPE_IEP: tec->b61 = 1; @@ -519,6 +522,8 @@ static int trans_exc_ending(struct kvm_vcpu *vcpu, int code, unsigned long gva, case PROT_TYPE_DAT: tec->b61 = 1; break; + case PROT_NONE: + /* We should never get here, acts like termination */ } if (terminate) { tec->b56 = 0; @@ -968,8 +973,10 @@ static int guest_range_to_gpas(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar, return rc; } else { gpa = kvm_s390_real_to_abs(vcpu, ga); - if (kvm_is_error_gpa(vcpu->kvm, gpa)) + if (kvm_is_error_gpa(vcpu->kvm, gpa)) { rc = PGM_ADDRESSING; + prot = PROT_NONE; + } } if (rc) return trans_exc(vcpu, rc, ga, ar, mode, prot); @@ -1112,8 +1119,6 @@ int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar, if (rc == PGM_PROTECTION && try_storage_prot_override) rc = access_guest_page_with_key(vcpu->kvm, mode, gpas[idx], data, fragment_len, PAGE_SPO_ACC); - if (rc == PGM_PROTECTION) - prot = PROT_TYPE_KEYC; if (rc) break; len -= fragment_len; @@ -1123,6 +1128,10 @@ int access_guest_with_key(struct kvm_vcpu *vcpu, unsigned long ga, u8 ar, if (rc > 0) { bool terminate = (mode == GACC_STORE) && (idx > 0); + if (rc == PGM_PROTECTION) + prot = PROT_TYPE_KEYC; + else + prot = PROT_NONE; rc = trans_exc_ending(vcpu, rc, ga, ar, mode, prot, terminate); } out_unlock:
This silences smatch warnings reported by kbuild bot: arch/s390/kvm/gaccess.c:859 guest_range_to_gpas() error: uninitialized symbol 'prot'. arch/s390/kvm/gaccess.c:1064 access_guest_with_key() error: uninitialized symbol 'prot'. This is because it cannot tell that the value is not used in this case. The trans_exc* only examine prot if code is PGM_PROTECTION. Pass a dummy value for other codes. Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> --- arch/s390/kvm/gaccess.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)