2014-11-27 21:53+0200, Nadav Amit: > Radim Krčmář <rkrcmar@xxxxxxxxxx> wrote: > > - new->cid_mask = (1 << KVM_X2APIC_CID_BITS) - 1; > > - new->lid_mask = 0xffff; > > + new->cid_mask = new->lid_mask = 0xffff; > You set cid_mask to 0xffff, while there are only 16 clusters. I think it is > risky (if you twist my hand would come with a scenario). Let's see :) APIC id is 8 bit, and we compute cluster part of LDR by taking four upper bits, so 16 is enough. It isn't the safest programming practice, but we already fail to check physical_map bounds and any boost to maximal APIC ID is going to require a rewrite, thus I didn't bother to do it ... All uses should be covered with the following hunk, I will add it to v2 after all reviews, diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 6c2b8a5..30e4cc1 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -198,7 +198,7 @@ static void recalculate_apic_map(struct kvm *kvm) cid = apic_cluster_id(new, ldr); lid = apic_logical_id(new, ldr); - if (lid) + if (lid && cid < ARRAY_SIZE(map->logical_map)) new->logical_map[cid][ffs(lid) - 1] = apic; } out: > Yet, why not to set > cid_mask to (ARRAY_SIZE(map->logical_map) - 1) ? We would incorrectly deliver messages intended for high clusters, it has to be 0xffff. -- 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
