Hi Drew, On 2020/2/11 21:37, Andrew Jones wrote:
Let's bail out of the wait loop if we see the expected state to save over six seconds of run time. Make sure we wait a bit before reading the registers and double check again after, though, to somewhat mitigate the chance of seeing the expected state by accident. We also take this opportunity to push more IRQ state code to the library. Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
[...]
+ +enum gic_irq_state gic_irq_state(int irq)
This is a *generic* name while this function only deals with PPI. Maybe we can use something like gic_ppi_state() instead? Or you will have to take all interrupt types into account in a single function, which is not a easy job I think.
+{ + enum gic_irq_state state; + bool pending = false, active = false; + void *base; + + assert(gic_common_ops); + + switch (gic_version()) { + case 2: + base = gicv2_dist_base(); + pending = readl(base + GICD_ISPENDR) & (1 << PPI(irq)); + active = readl(base + GICD_ISACTIVER) & (1 << PPI(irq)); + break; + case 3: + base = gicv3_sgi_base(); + pending = readl(base + GICR_ISPENDR0) & (1 << PPI(irq)); + active = readl(base + GICR_ISACTIVER0) & (1 << PPI(irq));
And you may also want to ensure that the 'irq' is valid for PPI(). Or personally, I'd just use a real PPI number (PPI(info->irq)) as the input parameter of this function.
+ break; + } + + if (!active && !pending) + state = GIC_IRQ_STATE_INACTIVE; + if (pending) + state = GIC_IRQ_STATE_PENDING; + if (active) + state = GIC_IRQ_STATE_ACTIVE; + if (active && pending) + state = GIC_IRQ_STATE_ACTIVE_PENDING; + + return state; +}
Otherwise, Reviewed-by: Zenghui Yu <yuzenghui@xxxxxxxxxx> Tested-by: Zenghui Yu <yuzenghui@xxxxxxxxxx> Thanks _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm