Re: [RFT PATCH v3 16/27] irqchip/apple-aic: Add support for the Apple Interrupt Controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/03/2021 22.31, Marc Zyngier wrote:
+	if ((read_sysreg_s(SYS_ICH_HCR_EL2) & ICH_HCR_EN) &&
+		read_sysreg_s(SYS_ICH_MISR_EL2) != 0) {
+		pr_err("vGIC IRQ fired, disabling.\n");

Please add a _ratelimited here. Whilst debugging KVM on this machine,
I ended up with this firing at such a rate that it was impossible to
do anything useful. Ratelimiting it allowed me to pinpoint the
problem.

Ouch. Done for v4.

+static void aic_fiq_eoi(struct irq_data *d)
+{
+	/* We mask to ack (where we can), so we need to unmask at EOI. */
+	if (!irqd_irq_disabled(d) && !irqd_irq_masked(d))

Ah, be careful here: irqd_irq_masked() doesn't do what you think it
does for per-CPU interrupts. It's been on my list to fix for the rVIC
implementation, but I never got around to doing it, and all decent ICs
hide this from SW by having a HW-managed mask, similar to what is on
the IRQ side.

I can see two possibilities:

- you can track the masked state directly and use that instead of
   these predicates

- you can just drop the masking altogether as this is only useful to a
   hosted hypervisor (KVM), which will have to do its own masking
   behind the scenes anyway


Since you're using the masking in KVM after all, I'm tracking the mask state in a percpu variable now. Also folded in your two minor bugfixes from the KVM series. Cheers!


--
Hector Martin (marcan@xxxxxxxxx)
Public Key: https://mrcn.st/pub



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux