Re: [Xen-devel] virq injection probelem in Xen ARM 4.5

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 22 Apr 2015, ììì wrote:
> HI
>

Please send emails in plain text, it is very hard to read html on my
MUA.

>
> I might find the VIRQ injection in Xen ARM 4.5Âand fix with simple way.
>
> Check please.
>
> Â
>
> Problem
>
> - In Xen ARM 4.5, SPI's pending_irq structure could be accessedÂ
> simultaneously from each pCPU.
>
> Â
>
> Reason
>
> - When inject SPI VIRQ to domain0, Xen ARM 4.5 set bit HW(31bit) bit in list 
> register.Âso domain0's EOI commandÂmakes deactivate
> IRQ without Xen ARM interference. ButÂSPI's pending_irqÂis not updatedÂ
> immediately . because Xen ARM 4.5 dosen't use maintenance
> interrupt. After a while domain0's vcpu that receive SPI VIRQ is go to hyp 
> mode and updateÂSPI's pending_irqÂin
> enter_hypervisor_head() function.ÂAfter EOI in domain0's, same SPI IRQ can 
> occur immediately andÂSPI's pending_irq is updated in
> vgic_vcpu_inject_irq() function.
>
> If enter_hypervisor_head() is excecuted in pcpu1 andÂSPI's pending_irq is 
> updated in pcpu0, SPI's pending_irq is could be
> accesedÂsimultaneously. To accessÂSPI's pending_irq Xen have to acquireÂ
> v->arch.vgic.lock. But this lock don't guarantee exclusive
> access ofÂSPI's pending_irq.
>
> Example
>
> * Machine has 2 pcpu
>
> * Domain0 has 2 vcpu. vcpu0 is running on pcpu0 and vcpu1 is running on pcpu1.
>
> * All SPI is go to pcpu0 that run Domain0's vcpu0.
>
> * Assume that second IRQ 100 occur immediately after deactivate first IRQ 100.
>
> * Assume that first IRQ 100 injectedÂDomain0's vcpu1 and second IRQ 100 
> injected Domain0's vcpu0. First IRQ 100 is managed in
> pcpu1 and second IRQ 100 is managed in pcpu0.

This can only happen if Dom0 writes to the virtual GICD_ITARGETSR
register to change the delivery of IRQ 100.  If the write is done on
vcpu1, such a write would cause an immediate trap in the hypervisor and
clearing of the LRs, before the interrupt routing (virtual and physical)
is changed. If the write is done on vcpu0, the physical routing of the
irq won't be changed yet, see arch_move_irqs. The next interrupt will
still be delivererd to pcpu1, that is going to clear the LRs and only
then change the physical interrupt routing. Therefore only the third IRQ
100 interrupt will reach pcpu0 directly.


> - After deactivate first IRQ 100 in pcpu1, IRQ 100's pending_irq struct is 
> not updated immediately. But second IRQ 100 occur
> andÂIRQ 100's pending_irq pcpu0.Âenter_hypervisor_head() andÂ
> vgic_vcpu_inject_irq() could be executedÂsimultaneously andÂIRQ 100's
> pending_irq struct could be changedÂsimultaneously.
>
> Â
>
> Fix
>
> - To prevent access SPI's pending_irq struct, deactivate irq must be executed 
> after updatedÂSPI's pending_irq struct. So Xen ARM
> 4.5 cannot useÂHW(31bit) bit in list register to prevent domain0 deactivated 
> SPI.ÂXen ARM 4.5Âhave to manually deactivate SPI
> after updatedÂSPI's pending_irq struct.
>
> Â
>
> Code Changed
>
> - I Modify below functions. and this functions works well in my SPI Routing 
> Code in Arndale Board.
>
> ---------------------------------------------------------------------------------------------
>
> *Â
>
> static void gic_update_one_lr(struct vcpu *v, int i)
>
> {
>
> Â Â struct pending_irq *p;
>
> Â Â int irq;
>
> Â Â struct gic_lr lr_val;
>
> Â
>
> Â Â ASSERT(spin_is_locked(&v->arch.vgic.lock));
>
> Â Â ASSERT(!local_irq_is_enabled());
>
> Â
>
> Â Â gic_hw_ops->read_lr(i, &lr_val);
>
> Â Â irq = lr_val.virq;
>
> Â Â p = irq_to_pending(v, irq);
>
> Â Â if ( lr_val.state & GICH_LR_ACTIVE )
>
> Â Â {
>
> Â Â Â Â set_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
>
> Â Â Â Â if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) &&
>
> Â Â Â Â Â Â Âtest_and_clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status) )
>
> Â Â Â Â {
>
> Â Â Â Â Â Â if ( p->desc == NULL )
>
> Â Â Â Â Â Â {
>
> Â Â Â Â Â Â Â Â Âlr_val.state |= GICH_LR_PENDING;
>
> Â Â Â Â Â Â Â Â Âgic_hw_ops->write_lr(i, &lr_val);
>
> Â Â Â Â Â Â }
>
> Â Â Â Â Â Â else
>
> Â Â Â Â Â Â Â Â gdprintk(XENLOG_WARNING, "unable to inject hw irq=%d into 
> d%dv%d: already active in LR%d\n",
>
> Â Â Â Â Â Â Â Â Â Â Â Â Âirq, v->domain->domain_id, v->vcpu_id, i);
>
> Â Â Â Â }
>
> Â Â }
>
> Â Â else if ( lr_val.state & GICH_LR_PENDING )
>
> Â Â {
>
> Â Â Â Â int q __attribute__ ((unused)) = 
> test_and_clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status);
>
> #ifdef GIC_DEBUG
>
> Â Â Â Â if ( q )
>
> Â Â Â Â Â Â gdprintk(XENLOG_DEBUG, "trying to inject irq=%d into d%dv%d, when 
> it is already pending in LR%d\n",
>
> Â Â Â Â Â Â Â Â Â Â irq, v->domain->domain_id, v->vcpu_id, i);
>
> #endif
>
> Â Â }
>
> Â Â else
>
> Â Â {
>
> Â Â Â Â gic_hw_ops->clear_lr(i);
>
> Â Â Â Â clear_bit(i, &this_cpu(lr_mask));
>
> Â
>
> Â Â Â Â clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
>
> Â Â Â Â clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
>
> Â Â Â Â p->lr = GIC_INVALID_LR;
>
> Â Â Â Â if ( test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) &&
>
> Â Â Â Â Â Â Âtest_bit(GIC_IRQ_GUEST_QUEUED, &p->status) &&
>
> Â Â Â Â Â Â Â!test_bit(GIC_IRQ_GUEST_MIGRATING, &p->status) )
>
> Â Â Â Â Â Â gic_raise_guest_irq(v, irq, p->priority);
>
> Â Â Â Â else {
>
> Â Â Â Â Â Â list_del_init(&p->inflight);
>
> Â Â Â Â Â Â if ( test_and_clear_bit(GIC_IRQ_GUEST_MIGRATING, &p->status) )
>
> Â Â Â Â Â Â {
>
> Â Â Â Â Â Â Â Â struct vcpu *v_target = vgic_get_target_vcpu(v, irq);
>
> Â Â Â Â Â Â Â Â irq_set_affinity(p->desc, cpumask_of(v_target->processor));
>
> Â Â Â Â Â Â }
>
> Â Â Â Â }
>
> //* supsupi
>
> Â Â Â Â if ( p->desc != NULL )
>
> gic_hw_ops->deactivate_irq(p->desc);
>
> Â Â }
>
> Â
>
> }
>
> ---------------------------------------------------------------------------------------------
>
> static void gicv2_update_lr(int lr, const struct pending_irq *p,
>
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â unsigned int state)
>
> {
>
> Â Â uint32_t lr_reg;
>
> Â
>
> Â Â BUG_ON(lr >= gicv2_info.nr_lrs);
>
> Â Â BUG_ON(lr < 0);
>
> Â
>
> Â Â lr_reg = (((state & GICH_V2_LR_STATE_MASK) << GICH_V2_LR_STATE_SHIFT) Â|
>
> Â Â Â Â Â Â Â ((GIC_PRI_TO_GUEST(p->priority) & GICH_V2_LR_PRIORITY_MASK)
>
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â<< GICH_V2_LR_PRIORITY_SHIFT) |
>
> Â Â Â Â Â Â Â ((p->irq & GICH_V2_LR_VIRTUAL_MASK) << 
> GICH_V2_LR_VIRTUAL_SHIFT));
>
> Â
>
> //* supsupi
>
> Â Â if ( p->desc != NULL )
>
> lr_reg |= GICH_V2_LR_MAINTENANCE_IRQ;
>
> Â
>
> Â Â writel_gich(lr_reg, GICH_LR + lr * 4);
>
> Â
>
> }
>
> ---------------------------------------------------------------------------------------------
>
> Thank you
>
> Â
>
> [?img=m9Rm%2BBFm%2BBFohAnZFAM9pxMXaAUlMxFoKxUwpoi0Fqk0MrUXFoEqM4ioFrpvtzFXp6UwFSl5WLl51zlqDBFdp6d5MreRhoRq%2Bzk4M6lT7NFdM6i0WzwGW
> 40gpBE5Mr0db40%2F74FTWt%3D%3D.gif]
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel