Hi Andre,

On 03/05/2018 04:03 PM, Andre Przywara wrote:
+void vgic_v2_fold_lr_state(struct vcpu *vcpu)
+{
+    struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic;
+    unsigned int used_lrs = vcpu->arch.vgic.used_lrs;
+    unsigned long flags;
+    unsigned int lr;
+
+    if ( !used_lrs )    /* No LRs used, so nothing to sync back here. */
+        return;
+
+    gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 0);
+
+    for ( lr = 0; lr < used_lrs; lr++ )
+    {
+        struct gic_lr lr_val;
+        uint32_t intid;
+        struct vgic_irq *irq;
+
+        gic_hw_ops->read_lr(lr, &lr_val);
+
+        /*
+         * TODO: Possible optimization to avoid reading LRs:
+         * Read the ELRSR to find out which of our LRs have been cleared
+         * by the guest. We just need to know the IRQ number for those, which
+         * we could save in an array when populating the LRs.
+         * This trades one MMIO access (ELRSR) for possibly more than one 
(LRs),
+         * but requires some more code to save the IRQ number and to handle
+         * those finished IRQs according to the algorithm below.
+         * We need some numbers to justify this: chances are that we don't
+         * have many LRs in use most of the time, so we might not save much.
+         */
+        gic_hw_ops->clear_lr(lr);
+
+        intid = lr_val.virq;
+        irq = vgic_get_irq(vcpu->domain, vcpu, intid);
+
+        spin_lock_irqsave(&irq->irq_lock, flags);
+
+        /* Always preserve the active bit */
+        irq->active = !!(lr_val.state & GICH_LR_ACTIVE);
+
+        /* Edge is the only case where we preserve the pending bit */
+        if ( irq->config == VGIC_CONFIG_EDGE && (lr_val.state & 
GICH_LR_PENDING) )
+        {
+            irq->pending_latch = true;
+
+            if ( vgic_irq_is_sgi(intid) )
+                irq->source |= (1U << lr_val.source);
+        }


KVM is clearing pending_latch for level IRQ. Why this is not done in Xen?

+
+       /*
+        * Level-triggered mapped IRQs are special because we only
+        * observe rising edges as input to the VGIC.
+        *
+        * If the guest never acked the interrupt we have to sample
+        * the physical line and set the line level, because the
+        * device state could have changed or we simply need to
+        * process the still pending interrupt later.
+        *
+        * If this causes us to lower the level, we have to also clear
+        * the physical active state, since we will otherwise never be
+        * told when the interrupt becomes asserted again.
+        */

The indentation of the comment looks wrong.

+        if ( vgic_irq_is_mapped_level(irq) && (lr_val.state & GICH_LR_PENDING) 
)
+        {
+            struct irq_desc *irqd;
+
+            ASSERT(irq->hwintid >= VGIC_NR_PRIVATE_IRQS);
+
+            irqd = irq_to_desc(irq->hwintid);
+            irq->line_level = gic_read_pending_state(irqd);
+
+            if ( !irq->line_level )
+                gic_set_active_state(irqd, false);
+        }
+
+        spin_unlock_irqrestore(&irq->irq_lock, flags);
+        vgic_put_irq(vcpu->domain, irq);
+    }
+
+    gic_hw_ops->update_hcr_status(GICH_HCR_EN, 0);
+    vgic_cpu->used_lrs = 0;
+}

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Reply via email to