[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 2/4] x86/vioapic: issue EOI to dpci when switching pin to edge trigger mode
On Thu, Jan 21, 2021 at 05:23:04PM +0100, Jan Beulich wrote: > On 15.01.2021 15:28, Roger Pau Monne wrote: > > --- a/xen/arch/x86/hvm/vioapic.c > > +++ b/xen/arch/x86/hvm/vioapic.c > > @@ -268,6 +268,17 @@ static void vioapic_write_redirent( > > > > spin_unlock(&d->arch.hvm.irq_lock); > > > > + if ( ent.fields.trig_mode == VIOAPIC_EDGE_TRIG && > > + ent.fields.remote_irr && is_iommu_enabled(d) ) > > + /* > > + * Since IRR has been cleared and further interrupts can be > > + * injected also attempt to deassert any virtual line of passed > > + * through devices using this pin. Switching a pin from level > > to > > + * trigger mode can be used as a way to EOI an interrupt at the > > + * IO-APIC level. > > + */ > > + hvm_dpci_eoi(d, gsi); > > + > > if ( is_hardware_domain(d) && unmasked ) > > { > > /* > > I assume in the comment you mean "... from level to edge > mode ...". Yes, that's right, I completely missed it, sorry. > But this isn't reflected in the if() you add - > you do the same also when the mode doesn't change. Or do > you build on the assumption that ent.fields.remote_irr can > only be set if the prior mode was "level" (in which case > an assertion may be warranted, as I don't think this is > overly obvious)? Yes, IRR is only set for level triggered interrupts, so it's indeed build on the assumption that a pin can only have had IRR set when in edge mode when it's being switched from level to edge. I can add an assertion. > Also, looking at this code, is it correct to trigger an IRQ > upon the guest writing the upper half of an unmasked RTE > with remote_irr clear? I'd assume this needs to be strictly > limited to a 1->0 transition of the mask bit. If other code > indeed guarantees this in all cases, perhaps another place > where an assertion would be warranted? Indeed. I don't think it should be possible for a write to the upper half to trigger the injection of an interrupt, as having gsi_assert_count > 0 would imply that either IRR is already set, or that the pin is masked when processing an upper write. I can add that a pre-patch if you agree. In fact we could almost short-circuit the logic after the *pent = ent; line for upper writes if it wasn't for the call to vlapic_adjust_i8259_target, the rest of the code there shouldn't matter for upper writes. And the i8259 target logic that we have is very dodgy I would say. I have plans to fix it at some point, but that requires fixing the virtual periodic timers logic first, which I didn't get around to re-posting. Thanks, Roger.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |