Messages in this thread | | | Date | Thu, 24 Feb 2022 13:10:34 +0100 | Subject | Re: [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations | From | Michael Mueller <> |
| |
On 24.02.22 12:36, Claudio Imbrenda wrote: > On Wed, 23 Feb 2022 18:44:20 +0200 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > >> While in this particular case it would not be a (critical) issue, >> the pattern itself is bad and error prone in case somebody blindly >> copies to their code. >> >> Don't cast parameter to unsigned long pointer in the bit operations. >> Instead copy to a local variable on stack of a proper type and use. >> >> Fixes: d77e64141e32 ("KVM: s390: implement GISA IPM related primitives") >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> --- >> arch/s390/include/asm/kvm_host.h | 5 ++++- >> arch/s390/kvm/interrupt.c | 6 +++--- >> 2 files changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h >> index a22c9266ea05..f1c4a1b9b360 100644 >> --- a/arch/s390/include/asm/kvm_host.h >> +++ b/arch/s390/include/asm/kvm_host.h >> @@ -867,7 +867,10 @@ struct kvm_s390_gisa { >> u8 reserved03[11]; >> u32 airq_count; >> } g1; >> - struct { >> + struct { /* as a 256-bit bitmap */ >> + DECLARE_BITMAP(b, 256); >> + } bitmap; >> + struct { /* as a set of 64-bit words */ >> u64 word[4]; >> } u64; >> }; >> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c >> index db933c252dbc..04e055cbd080 100644 >> --- a/arch/s390/kvm/interrupt.c >> +++ b/arch/s390/kvm/interrupt.c >> @@ -304,7 +304,7 @@ static inline int gisa_in_alert_list(struct kvm_s390_gisa *gisa) >> >> static inline void gisa_set_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc) >> { >> - set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa); >> + set_bit_inv(IPM_BIT_OFFSET + gisc, gisa->bitmap.b); > > wouldn't it be enough to pass gisa->u64.word here? > then no cast would be necessary
we do that at several places
arch/s390/kernel/processor.c: for_each_set_bit_inv(bit, (long *)&stfle_fac_list, MAX_FACILITY_BIT) arch/s390/kvm/interrupt.c: set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa); arch/s390/kvm/kvm-s390.c: set_bit_inv(vcpu->vcpu_id, (unsigned long *) sca->mcn); arch/s390/kvm/kvm-s390.c: set_bit_inv(vcpu->vcpu_id, (unsigned long *) &sca->mcn);
> >> } >> >> static inline u8 gisa_get_ipm(struct kvm_s390_gisa *gisa) >> @@ -314,12 +314,12 @@ static inline u8 gisa_get_ipm(struct kvm_s390_gisa *gisa) >> >> static inline void gisa_clear_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc) >> { >> - clear_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa); >> + clear_bit_inv(IPM_BIT_OFFSET + gisc, gisa->bitmap.b); >> } >> >> static inline int gisa_tac_ipm_gisc(struct kvm_s390_gisa *gisa, u32 gisc) >> { >> - return test_and_clear_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long *) gisa); >> + return test_and_clear_bit_inv(IPM_BIT_OFFSET + gisc, gisa->bitmap.b); >> } >> >> static inline unsigned long pending_irqs_no_gisa(struct kvm_vcpu *vcpu) >
| |