| Message ID | 20220420222530.910125-4-yury.norov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bitmap: fix conversion from/to fix-sized arrays | expand |
On 21.04.22 00:25, Yury Norov wrote: > Copying bitmaps from/to 64-bit arrays with bitmap_copy is not safe > in general case. Use designated functions instead. > Just so I understand correctly: there is no BUG, it's just cleaner to do it that way, correct? IIUC, bitmap_to_arr64() translates to bitmap_copy_clear_tail() on s390x. As the passed length is always 1024 (KVM_S390_VM_CPU_FEAT_NR_BITS), we essentially end up with bitmap_copy() again. Looks cleaner to me Reviewed-by: David Hildenbrand <david@redhat.com>
On Thu, Apr 21, 2022 at 09:24:20AM +0200, David Hildenbrand wrote: > On 21.04.22 00:25, Yury Norov wrote: > > Copying bitmaps from/to 64-bit arrays with bitmap_copy is not safe > > in general case. Use designated functions instead. > > > > Just so I understand correctly: there is no BUG, it's just cleaner to do > it that way, correct? Yes. there's no bug, but the pattern is considered bad. https://lore.kernel.org/all/YiCWNdWd+AsLbDkp@smile.fi.intel.com/T/#m9080cbb8a8235d7d4b7e38292cee8e4903f9afe4q > IIUC, bitmap_to_arr64() translates to bitmap_copy_clear_tail() on s390x. Yes. > As the passed length is always 1024 (KVM_S390_VM_CPU_FEAT_NR_BITS), we > essentially end up with bitmap_copy() again. > > > Looks cleaner to me Thanks. > Reviewed-by: David Hildenbrand <david@redhat.com> > > > -- > Thanks, > > David / dhildenb
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 156d1c25a3c1..a353bb43ee48 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -1332,8 +1332,7 @@ static int kvm_s390_set_processor_feat(struct kvm *kvm, mutex_unlock(&kvm->lock); return -EBUSY; } - bitmap_copy(kvm->arch.cpu_feat, (unsigned long *) data.feat, - KVM_S390_VM_CPU_FEAT_NR_BITS); + bitmap_from_arr64(kvm->arch.cpu_feat, data.feat, KVM_S390_VM_CPU_FEAT_NR_BITS); mutex_unlock(&kvm->lock); VM_EVENT(kvm, 3, "SET: guest feat: 0x%16.16llx.0x%16.16llx.0x%16.16llx", data.feat[0], @@ -1504,8 +1503,7 @@ static int kvm_s390_get_processor_feat(struct kvm *kvm, { struct kvm_s390_vm_cpu_feat data; - bitmap_copy((unsigned long *) data.feat, kvm->arch.cpu_feat, - KVM_S390_VM_CPU_FEAT_NR_BITS); + bitmap_to_arr64(data.feat, kvm->arch.cpu_feat, KVM_S390_VM_CPU_FEAT_NR_BITS); if (copy_to_user((void __user *)attr->addr, &data, sizeof(data))) return -EFAULT; VM_EVENT(kvm, 3, "GET: guest feat: 0x%16.16llx.0x%16.16llx.0x%16.16llx", @@ -1520,9 +1518,7 @@ static int kvm_s390_get_machine_feat(struct kvm *kvm, { struct kvm_s390_vm_cpu_feat data; - bitmap_copy((unsigned long *) data.feat, - kvm_s390_available_cpu_feat, - KVM_S390_VM_CPU_FEAT_NR_BITS); + bitmap_to_arr64(data.feat, kvm_s390_available_cpu_feat, KVM_S390_VM_CPU_FEAT_NR_BITS); if (copy_to_user((void __user *)attr->addr, &data, sizeof(data))) return -EFAULT; VM_EVENT(kvm, 3, "GET: host feat: 0x%16.16llx.0x%16.16llx.0x%16.16llx",
Copying bitmaps from/to 64-bit arrays with bitmap_copy is not safe in general case. Use designated functions instead. Signed-off-by: Yury Norov <yury.norov@gmail.com> --- arch/s390/kvm/kvm-s390.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)