On Fri, Feb 27, 2009 at 11:19:09AM +0800, Sheng Yang wrote:
> > - *mask = 0;
>
> I think the caller should not assume anything about the output, so let
> function itself do the clean is better. (however, not in this way for "mask"
> should be KVM_MAX_VCPUS long.)
>
Ok. I'll leave it although all callers of the function zeroes bitmask anyway.
> And, I found some other things are not proper after looking back code, I would
> send a patch to fix it (deliver_bitmask in kvm_get_intr_delivery_bitmask).
>
> > if (dest_mode == 0) { /* Physical mode. */
> > if (dest == 0xFF) { /* Broadcast. */
> > for (i = 0; i < KVM_MAX_VCPUS; ++i)
> > if (kvm->vcpus[i] && kvm->vcpus[i]->arch.apic)
> > - *mask |= 1 << i;
> > + __set_bit(i, *mask);
>
> Should be __set_bit(i, mask)?
>
Oh. Here is updated patch:
Signed-off-by: Gleb Natapov <gleb@xxxxxxxxxx>
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 7c2cb2b..7bd27aa 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -170,12 +170,13 @@ void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
ioapic_debug("dest %d dest_mode %d\n", dest, dest_mode);
- *mask = 0;
+ bitmap_zero(mask, KVM_MAX_VCPUS);
+
if (dest_mode == 0) { /* Physical mode. */
if (dest == 0xFF) { /* Broadcast. */
for (i = 0; i < KVM_MAX_VCPUS; ++i)
if (kvm->vcpus[i] && kvm->vcpus[i]->arch.apic)
- *mask |= 1 << i;
+ __set_bit(i, mask);
return;
}
for (i = 0; i < KVM_MAX_VCPUS; ++i) {
@@ -184,7 +185,7 @@ void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
continue;
if (kvm_apic_match_physical_addr(vcpu->arch.apic, dest)) {
if (vcpu->arch.apic)
- *mask = 1 << i;
+ __set_bit(i, mask);
break;
}
}
@@ -195,7 +196,7 @@ void kvm_ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest,
continue;
if (vcpu->arch.apic &&
kvm_apic_match_logical_addr(vcpu->arch.apic, dest))
- *mask |= 1 << vcpu->vcpu_id;
+ __set_bit(i, mask);
}
ioapic_debug("mask %x\n", *mask);
}
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
