On Thursday 26 February 2009 21:03:39 Gleb Natapov wrote:
> And don't zero bitmap twice. Assume that caller zeros it.
>
> Signed-off-by: Gleb Natapov <gleb@xxxxxxxxxx>
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index 7c2cb2b..21627b9 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -170,12 +170,11 @@ void kvm_ioapic_get_delivery_bitmask(struct
> kvm_ioapic *ioapic, u8 dest,
>
> ioapic_debug("dest %d dest_mode %d\n", dest, dest_mode);
>
Hi Gleb
> - *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.)
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)?
--
regards
Yang, Sheng
> return;
> }
> for (i = 0; i < KVM_MAX_VCPUS; ++i) {
> @@ -184,7 +183,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 +194,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
