* [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations
@ 2022-02-23 16:44 Andy Shevchenko
2022-02-24 11:36 ` Claudio Imbrenda
2022-03-02 15:45 ` Andy Shevchenko
0 siblings, 2 replies; 11+ messages in thread
From: Andy Shevchenko @ 2022-02-23 16:44 UTC (permalink / raw)
To: Christian Borntraeger, kvm, linux-s390, linux-kernel
Cc: Janosch Frank, Claudio Imbrenda, David Hildenbrand,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
Andy Shevchenko
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);
}
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)
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations
2022-02-23 16:44 [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations Andy Shevchenko
@ 2022-02-24 11:36 ` Claudio Imbrenda
2022-02-24 12:10 ` Michael Mueller
2022-02-24 19:51 ` Andy Shevchenko
2022-03-02 15:45 ` Andy Shevchenko
1 sibling, 2 replies; 11+ messages in thread
From: Claudio Imbrenda @ 2022-02-24 11:36 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Christian Borntraeger, kvm, linux-s390, linux-kernel,
Janosch Frank, David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Sven Schnelle
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
> }
>
> 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)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations
2022-02-24 11:36 ` Claudio Imbrenda
@ 2022-02-24 12:10 ` Michael Mueller
2022-03-02 15:44 ` Andy Shevchenko
2022-02-24 19:51 ` Andy Shevchenko
1 sibling, 1 reply; 11+ messages in thread
From: Michael Mueller @ 2022-02-24 12:10 UTC (permalink / raw)
To: Claudio Imbrenda, Andy Shevchenko
Cc: Christian Borntraeger, kvm, linux-s390, linux-kernel,
Janosch Frank, David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Sven Schnelle
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)
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations
2022-02-24 11:36 ` Claudio Imbrenda
2022-02-24 12:10 ` Michael Mueller
@ 2022-02-24 19:51 ` Andy Shevchenko
2022-02-24 23:15 ` David Laight
1 sibling, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2022-02-24 19:51 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: Andy Shevchenko, Christian Borntraeger, open list:VFIO DRIVER,
linux-s390, Linux Kernel Mailing List, Janosch Frank,
David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Sven Schnelle
On Thu, Feb 24, 2022 at 2:51 PM Claudio Imbrenda <imbrenda@linux.ibm.com> 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.
...
> > + struct { /* as a 256-bit bitmap */
> > + DECLARE_BITMAP(b, 256);
> > + } bitmap;
> > + struct { /* as a set of 64-bit words */
> > u64 word[4];
> > } u64;
> > - 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
No, it will have the same hidden bugs. As I stated in the commit
message, the pattern is quite bad even if in particular code it would
work.
Thanks, Michael, for pointing out other places. They all need to be fixed.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations
2022-02-24 19:51 ` Andy Shevchenko
@ 2022-02-24 23:15 ` David Laight
0 siblings, 0 replies; 11+ messages in thread
From: David Laight @ 2022-02-24 23:15 UTC (permalink / raw)
To: 'Andy Shevchenko', Claudio Imbrenda
Cc: Andy Shevchenko, Christian Borntraeger, open list:VFIO DRIVER,
linux-s390, Linux Kernel Mailing List, Janosch Frank,
David Hildenbrand, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Sven Schnelle
From: Andy Shevchenko
> Sent: 24 February 2022 19:51
>
> On Thu, Feb 24, 2022 at 2:51 PM Claudio Imbrenda <imbrenda@linux.ibm.com> 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.
>
> ...
>
> > > + struct { /* as a 256-bit bitmap */
> > > + DECLARE_BITMAP(b, 256);
> > > + } bitmap;
> > > + struct { /* as a set of 64-bit words */
> > > u64 word[4];
> > > } u64;
>
> > > - 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
>
> No, it will have the same hidden bugs. As I stated in the commit
> message, the pattern is quite bad even if in particular code it would
> work.
>
> Thanks, Michael, for pointing out other places. They all need to be fixed.
It may even be worth writing some alternate bitmap functions
that use u64[] and unlocked operations?
Although I think I'd still want to encapsulate the actual array
(somehow) so that what is defined has to be the bitmap type.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations
2022-02-24 12:10 ` Michael Mueller
@ 2022-03-02 15:44 ` Andy Shevchenko
2022-03-02 17:18 ` Yury Norov
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2022-03-02 15:44 UTC (permalink / raw)
To: Michael Mueller, Yury Norov
Cc: Claudio Imbrenda, Christian Borntraeger, kvm, linux-s390,
linux-kernel, Janosch Frank, David Hildenbrand, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Sven Schnelle
On Thu, Feb 24, 2022 at 01:10:34PM +0100, Michael Mueller wrote:
> On 24.02.22 12:36, Claudio Imbrenda wrote:
...
> we do that at several places
Thanks for pointing out.
> arch/s390/kernel/processor.c: for_each_set_bit_inv(bit, (long
> *)&stfle_fac_list, MAX_FACILITY_BIT)
This one requires a separate change, not related to this patch.
> arch/s390/kvm/interrupt.c: set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long
> *) gisa);
This is done in the patch. Not sure how it appears in your list.
> 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);
These two should be fixed in a separate change.
Also this kind of stuff:
bitmap_copy(kvm->arch.cpu_feat, (unsigned long *) data.feat,
KVM_S390_VM_CPU_FEAT_NR_BITS);
might require a new API like
bitmap_from_u64_array()
bitmap_to_u64_array()
Yury?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations
2022-02-23 16:44 [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations Andy Shevchenko
2022-02-24 11:36 ` Claudio Imbrenda
@ 2022-03-02 15:45 ` Andy Shevchenko
1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2022-03-02 15:45 UTC (permalink / raw)
To: Christian Borntraeger, kvm, linux-s390, linux-kernel
Cc: Janosch Frank, Claudio Imbrenda, David Hildenbrand,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle
On Wed, Feb 23, 2022 at 06:44:20PM +0200, Andy Shevchenko 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.
After looking into other similar cases I may conclude they
- need to be fixed
- out of scope of this change
Hence, can this fix be applied?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations
2022-03-02 15:44 ` Andy Shevchenko
@ 2022-03-02 17:18 ` Yury Norov
2022-03-02 17:31 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Yury Norov @ 2022-03-02 17:18 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Michael Mueller, Claudio Imbrenda, Christian Borntraeger, kvm,
linux-s390, linux-kernel, Janosch Frank, David Hildenbrand,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle
On Wed, Mar 02, 2022 at 05:44:03PM +0200, Andy Shevchenko wrote:
> On Thu, Feb 24, 2022 at 01:10:34PM +0100, Michael Mueller wrote:
> > On 24.02.22 12:36, Claudio Imbrenda wrote:
>
> ...
>
> > we do that at several places
>
> Thanks for pointing out.
>
> > arch/s390/kernel/processor.c: for_each_set_bit_inv(bit, (long
> > *)&stfle_fac_list, MAX_FACILITY_BIT)
>
> This one requires a separate change, not related to this patch.
>
> > arch/s390/kvm/interrupt.c: set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long
> > *) gisa);
>
> This is done in the patch. Not sure how it appears in your list.
>
> > 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);
>
> These two should be fixed in a separate change.
>
> Also this kind of stuff:
>
> bitmap_copy(kvm->arch.cpu_feat, (unsigned long *) data.feat,
> KVM_S390_VM_CPU_FEAT_NR_BITS);
>
> might require a new API like
>
> bitmap_from_u64_array()
> bitmap_to_u64_array()
>
> Yury?
If BE32 is still the case then yes.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations
2022-03-02 17:18 ` Yury Norov
@ 2022-03-02 17:31 ` Andy Shevchenko
2022-03-02 18:43 ` Yury Norov
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2022-03-02 17:31 UTC (permalink / raw)
To: Yury Norov
Cc: Michael Mueller, Claudio Imbrenda, Christian Borntraeger, kvm,
linux-s390, linux-kernel, Janosch Frank, David Hildenbrand,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle
On Wed, Mar 02, 2022 at 09:18:35AM -0800, Yury Norov wrote:
> On Wed, Mar 02, 2022 at 05:44:03PM +0200, Andy Shevchenko wrote:
> > On Thu, Feb 24, 2022 at 01:10:34PM +0100, Michael Mueller wrote:
> > > On 24.02.22 12:36, Claudio Imbrenda wrote:
> >
> > ...
> >
> > > we do that at several places
> >
> > Thanks for pointing out.
> >
> > > arch/s390/kernel/processor.c: for_each_set_bit_inv(bit, (long
> > > *)&stfle_fac_list, MAX_FACILITY_BIT)
> >
> > This one requires a separate change, not related to this patch.
> >
> > > arch/s390/kvm/interrupt.c: set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long
> > > *) gisa);
> >
> > This is done in the patch. Not sure how it appears in your list.
> >
> > > 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);
> >
> > These two should be fixed in a separate change.
> >
> > Also this kind of stuff:
> >
> > bitmap_copy(kvm->arch.cpu_feat, (unsigned long *) data.feat,
> > KVM_S390_VM_CPU_FEAT_NR_BITS);
> >
> > might require a new API like
> >
> > bitmap_from_u64_array()
> > bitmap_to_u64_array()
> >
> > Yury?
>
> If BE32 is still the case then yes.
The whole point is to get rid of the bad pattern, while it may still work
in the particular case.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations
2022-03-02 17:31 ` Andy Shevchenko
@ 2022-03-02 18:43 ` Yury Norov
2022-03-03 10:19 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Yury Norov @ 2022-03-02 18:43 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Michael Mueller, Claudio Imbrenda, Christian Borntraeger, kvm,
linux-s390, linux-kernel, Janosch Frank, David Hildenbrand,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle
On Wed, Mar 02, 2022 at 07:31:58PM +0200, Andy Shevchenko wrote:
> On Wed, Mar 02, 2022 at 09:18:35AM -0800, Yury Norov wrote:
> > On Wed, Mar 02, 2022 at 05:44:03PM +0200, Andy Shevchenko wrote:
> > > On Thu, Feb 24, 2022 at 01:10:34PM +0100, Michael Mueller wrote:
> > > > On 24.02.22 12:36, Claudio Imbrenda wrote:
> > >
> > > ...
> > >
> > > > we do that at several places
> > >
> > > Thanks for pointing out.
> > >
> > > > arch/s390/kernel/processor.c: for_each_set_bit_inv(bit, (long
> > > > *)&stfle_fac_list, MAX_FACILITY_BIT)
> > >
> > > This one requires a separate change, not related to this patch.
> > >
> > > > arch/s390/kvm/interrupt.c: set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long
> > > > *) gisa);
> > >
> > > This is done in the patch. Not sure how it appears in your list.
> > >
> > > > 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);
> > >
> > > These two should be fixed in a separate change.
> > >
> > > Also this kind of stuff:
> > >
> > > bitmap_copy(kvm->arch.cpu_feat, (unsigned long *) data.feat,
> > > KVM_S390_VM_CPU_FEAT_NR_BITS);
> > >
> > > might require a new API like
> > >
> > > bitmap_from_u64_array()
> > > bitmap_to_u64_array()
> > >
> > > Yury?
> >
> > If BE32 is still the case then yes.
>
> The whole point is to get rid of the bad pattern, while it may still work
> in the particular case.
Then yes unconditionally. Is it already on table of s390 folks? If no,
I can do it myself.
We have bitmap_from_arr32 and bitmap_to_arr32, so for 64-bit versions,
we'd start from that.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations
2022-03-02 18:43 ` Yury Norov
@ 2022-03-03 10:19 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2022-03-03 10:19 UTC (permalink / raw)
To: Yury Norov
Cc: Michael Mueller, Claudio Imbrenda, Christian Borntraeger, kvm,
linux-s390, linux-kernel, Janosch Frank, David Hildenbrand,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle
On Wed, Mar 02, 2022 at 10:43:54AM -0800, Yury Norov wrote:
> On Wed, Mar 02, 2022 at 07:31:58PM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 02, 2022 at 09:18:35AM -0800, Yury Norov wrote:
> > > On Wed, Mar 02, 2022 at 05:44:03PM +0200, Andy Shevchenko wrote:
> > > > On Thu, Feb 24, 2022 at 01:10:34PM +0100, Michael Mueller wrote:
> > > > > On 24.02.22 12:36, Claudio Imbrenda wrote:
> > > >
> > > > ...
> > > >
> > > > > we do that at several places
> > > >
> > > > Thanks for pointing out.
> > > >
> > > > > arch/s390/kernel/processor.c: for_each_set_bit_inv(bit, (long
> > > > > *)&stfle_fac_list, MAX_FACILITY_BIT)
> > > >
> > > > This one requires a separate change, not related to this patch.
> > > >
> > > > > arch/s390/kvm/interrupt.c: set_bit_inv(IPM_BIT_OFFSET + gisc, (unsigned long
> > > > > *) gisa);
> > > >
> > > > This is done in the patch. Not sure how it appears in your list.
> > > >
> > > > > 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);
> > > >
> > > > These two should be fixed in a separate change.
> > > >
> > > > Also this kind of stuff:
> > > >
> > > > bitmap_copy(kvm->arch.cpu_feat, (unsigned long *) data.feat,
> > > > KVM_S390_VM_CPU_FEAT_NR_BITS);
> > > >
> > > > might require a new API like
> > > >
> > > > bitmap_from_u64_array()
> > > > bitmap_to_u64_array()
> > > >
> > > > Yury?
> > >
> > > If BE32 is still the case then yes.
> >
> > The whole point is to get rid of the bad pattern, while it may still work
> > in the particular case.
>
> Then yes unconditionally. Is it already on table of s390 folks? If no,
> I can do it myself.
>
> We have bitmap_from_arr32 and bitmap_to_arr32, so for 64-bit versions,
> we'd start from that.
Yep, thanks!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-03-03 10:20 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-23 16:44 [PATCH v1 1/1] KVM: s390: Don't cast parameter in bit operations Andy Shevchenko
2022-02-24 11:36 ` Claudio Imbrenda
2022-02-24 12:10 ` Michael Mueller
2022-03-02 15:44 ` Andy Shevchenko
2022-03-02 17:18 ` Yury Norov
2022-03-02 17:31 ` Andy Shevchenko
2022-03-02 18:43 ` Yury Norov
2022-03-03 10:19 ` Andy Shevchenko
2022-02-24 19:51 ` Andy Shevchenko
2022-02-24 23:15 ` David Laight
2022-03-02 15:45 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).