On Wed, Nov 21, 2012 at 1:15 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > Various bits of the vgic code blindly assume that the size of u32 > and unsigned long are the same. This leads to amusing side effects > on 64bit platforms, and makes the code hard to follow. > > Move to proper bitmaps instead of the home grown version, and split > the vcpu specific "pending" bitmap into pending_percpu and pending_shared. > > Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> > --- > arch/arm/include/asm/kvm_vgic.h | 7 +++--- > arch/arm/kvm/vgic.c | 55 +++++++++++++++++++++++++++-------------- > 2 files changed, 41 insertions(+), 21 deletions(-) > > diff --git a/arch/arm/include/asm/kvm_vgic.h b/arch/arm/include/asm/kvm_vgic.h > index e371100..7a5e196 100644 > --- a/arch/arm/include/asm/kvm_vgic.h > +++ b/arch/arm/include/asm/kvm_vgic.h > @@ -51,11 +51,11 @@ > struct vgic_bitmap { > union { > u32 reg[1]; > - unsigned long reg_ul[0]; > + DECLARE_BITMAP(reg_ul, 32); > } percpu[VGIC_MAX_CPUS]; > union { > u32 reg[VGIC_NR_SHARED_IRQS / 32]; > - unsigned long reg_ul[0]; > + DECLARE_BITMAP(reg_ul, VGIC_NR_SHARED_IRQS); > } shared; > }; > > @@ -199,7 +199,8 @@ struct vgic_cpu { > u8 vgic_irq_lr_map[VGIC_NR_IRQS]; > > /* Pending interrupts on this VCPU */ > - DECLARE_BITMAP( pending, VGIC_NR_IRQS); > + DECLARE_BITMAP( pending_percpu, 32); > + DECLARE_BITMAP( pending_shared, VGIC_NR_SHARED_IRQS); > > /* Bitmap of used/free list registers */ > DECLARE_BITMAP( lr_used, 64); > diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c > index e1d25b4..57929db 100644 > --- a/arch/arm/kvm/vgic.c > +++ b/arch/arm/kvm/vgic.c > @@ -100,6 +100,22 @@ static inline int vgic_irq_is_edge(struct vgic_dist *dist, int irq) > return vgic_bitmap_get_irq_val(&dist->irq_cfg, 0, irq); > } > > +static inline void kvm_vgic_vcpu_set_pending_irq(struct kvm_vcpu *vcpu, int irq) > +{ > + if (irq < 32) > + set_bit(irq, vcpu->arch.vgic_cpu.pending_percpu); > + else > + set_bit(irq - 32, vcpu->arch.vgic_cpu.pending_shared); > +} > + > +static inline void kvm_vgic_vcpu_clear_pending_irq(struct kvm_vcpu *vcpu, int irq) > +{ > + if (irq < 32) > + clear_bit(irq, vcpu->arch.vgic_cpu.pending_percpu); > + else > + clear_bit(irq - 32, vcpu->arch.vgic_cpu.pending_shared); > +} > + > /** > * vgic_reg_access - access vgic register > * @mmio: pointer to the data describing the mmio access > @@ -613,24 +629,26 @@ static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg) > static int compute_pending_for_cpu(struct kvm_vcpu *vcpu) > { > struct vgic_dist *dist = &vcpu->kvm->arch.vgic; > - unsigned long *pending, *enabled, *pend; > + unsigned long *pending, *enabled, *pend_percpu, *pend_shared; > int vcpu_id; > > vcpu_id = vcpu->vcpu_id; > - pend = vcpu->arch.vgic_cpu.pending; > + pend_percpu = vcpu->arch.vgic_cpu.pending_percpu; > + pend_shared = vcpu->arch.vgic_cpu.pending_shared; > > pending = vgic_bitmap_get_cpu_map(&dist->irq_state, vcpu_id); > enabled = vgic_bitmap_get_cpu_map(&dist->irq_enabled, vcpu_id); > - bitmap_and(pend, pending, enabled, 32); > + bitmap_and(pend_percpu, pending, enabled, 32); > > pending = vgic_bitmap_get_shared_map(&dist->irq_state); > enabled = vgic_bitmap_get_shared_map(&dist->irq_enabled); > - bitmap_and(pend + 1, pending, enabled, VGIC_NR_SHARED_IRQS); > - bitmap_and(pend + 1, pend + 1, > + bitmap_and(pend_shared, pending, enabled, VGIC_NR_SHARED_IRQS); > + bitmap_and(pend_shared, pend_shared, > vgic_bitmap_get_shared_map(&dist->irq_spi_target[vcpu_id]), > VGIC_NR_SHARED_IRQS); > > - return (find_first_bit(pend, VGIC_NR_IRQS) < VGIC_NR_IRQS); > + return (find_first_bit(pend_percpu, 32) < 32 || > + find_first_bit(pend_shared, VGIC_NR_SHARED_IRQS) < VGIC_NR_SHARED_IRQS); > } > > /* > @@ -767,7 +785,7 @@ static void __kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu) > > /* SGIs */ > pending = vgic_bitmap_get_cpu_map(&dist->irq_state, vcpu_id); > - for_each_set_bit(i, vgic_cpu->pending, 16) { > + for_each_set_bit(i, vgic_cpu->pending_percpu, 16) { > unsigned long sources; > > sources = dist->irq_sgi_sources[vcpu_id][i]; > @@ -787,7 +805,7 @@ static void __kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu) > } > > /* PPIs */ > - for_each_set_bit_from(i, vgic_cpu->pending, 32) { > + for_each_set_bit_from(i, vgic_cpu->pending_percpu, 32) { > if (!vgic_queue_irq(vcpu, 0, i)) { > overflow = 1; > continue; > @@ -799,21 +817,22 @@ static void __kvm_vgic_sync_to_cpu(struct kvm_vcpu *vcpu) > > /* SPIs */ > pending = vgic_bitmap_get_shared_map(&dist->irq_state); > - for_each_set_bit_from(i, vgic_cpu->pending, VGIC_NR_IRQS) { > - if (vgic_bitmap_get_irq_val(&dist->irq_active, 0, i)) > + for_each_set_bit(i, vgic_cpu->pending_shared, VGIC_NR_SHARED_IRQS) { > + int irq = i + 32; > + if (vgic_bitmap_get_irq_val(&dist->irq_active, 0, irq)) > continue; /* level interrupt, already queued */ > > - if (!vgic_queue_irq(vcpu, 0, i)) { > + if (!vgic_queue_irq(vcpu, 0, irq)) { > overflow = 1; > continue; > } > > /* Immediate clear on edge, set active on level */ > - if (vgic_irq_is_edge(dist, i)) { > - clear_bit(i - 32, pending); > - clear_bit(i, vgic_cpu->pending); > + if (vgic_irq_is_edge(dist, irq)) { > + clear_bit(i, pending); > + kvm_vgic_vcpu_clear_pending_irq(vcpu, irq); > } else { > - vgic_bitmap_set_irq_val(&dist->irq_active, 0, i, 1); > + vgic_bitmap_set_irq_val(&dist->irq_active, 0, irq, 1); > } > } > > @@ -963,7 +982,7 @@ static bool vgic_update_irq_state(struct kvm *kvm, int cpuid, > vcpu = kvm_get_vcpu(kvm, cpuid); > > if (level) { > - set_bit(irq_num, vcpu->arch.vgic_cpu.pending); > + kvm_vgic_vcpu_set_pending_irq(vcpu, irq_num); > set_bit(cpuid, &dist->irq_pending_on_cpu); > } > > @@ -1031,11 +1050,11 @@ static irqreturn_t vgic_maintenance_handler(int irq, void *data) > /* Any additionnal pending interrupt? */ > if (vgic_bitmap_get_irq_val(&dist->irq_state, > vcpu->vcpu_id, irq)) { > - set_bit(irq, vcpu->arch.vgic_cpu.pending); > + kvm_vgic_vcpu_set_pending_irq(vcpu, irq); > set_bit(vcpu->vcpu_id, > &dist->irq_pending_on_cpu); > } else { > - clear_bit(irq, vgic_cpu->pending); > + kvm_vgic_vcpu_clear_pending_irq(vcpu, irq); > } > } > } > -- > 1.7.12 > > man, that was surprisingly hard to review (supports the case for your following pending patch). thanks, applied. _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm