Hi Andre,

Overall this patch looks good. Few comments below.

On 03/05/2018 04:03 PM, Andre Przywara wrote:
+/*
+ * Only valid injection if changing level for level-triggered IRQs or for a
+ * rising edge.
+ */
+static bool vgic_validate_injection(struct vgic_irq *irq, bool level)
+{
+    if ( irq->config == VGIC_CONFIG_LEVEL )
+        return irq->line_level != level;
+
+    return level;

TBH, I would have preferred to keep the switch here. It was much clearer the second case is for edge. I would be ok with the if providing comment explaining "return level" is for edge.

+}
+
+/**
+ * vgic_queue_irq_unlock() - Queue an IRQ to a VCPU, to be injected to a guest.
+ * @d:        The domain the virtual IRQ belongs to.
+ * @irq:      A pointer to the vgic_irq of the virtual IRQ, with the lock held.
+ * @flags:    The flags used when having grabbed the IRQ lock.
+ *
+ * Check whether an IRQ needs to (and can) be queued to a VCPU's ap list.
+ * Do the queuing if necessary, taking the right locks in the right order.
+ *
+ * Needs to be entered with the IRQ lock already held, but will return
+ * with all locks dropped.
+ *
+ * Returns: True when the IRQ was queued, false otherwise.

The function is now returning void. It sounds like a left-over from the previous version?

+ */
+void vgic_queue_irq_unlock(struct domain *d, struct vgic_irq *irq,
+                           unsigned long flags)
+{

[...]

diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h
index a3befd386b..3430955d9f 100644
--- a/xen/arch/arm/vgic/vgic.h
+++ b/xen/arch/arm/vgic/vgic.h
@@ -17,9 +17,19 @@
  #ifndef __XEN_ARM_VGIC_VGIC_H__
  #define __XEN_ARM_VGIC_VGIC_H__
+static inline bool irq_is_pending(struct vgic_irq *irq)
+{
+    if ( irq->config == VGIC_CONFIG_EDGE )
+        return irq->pending_latch;
+    else
+        return irq->pending_latch || irq->line_level;
+}
+
  struct vgic_irq *vgic_get_irq(struct domain *d, struct vcpu *vcpu,
                                u32 intid);
  void vgic_put_irq(struct domain *d, struct vgic_irq *irq);
+void vgic_queue_irq_unlock(struct domain *d, struct vgic_irq *irq,
+               unsigned long flags);

The indentation looks wrong here.

static inline void vgic_get_irq_kref(struct vgic_irq *irq)
  {


Cheers,

--
Julien Grall

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

Reply via email to