[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v2 3/6] hw: arm_gic: Keep track of SGI sourc
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [RFC PATCH v2 3/6] hw: arm_gic: Keep track of SGI sources |
Date: |
Mon, 14 Oct 2013 17:33:38 +0100 |
On 14 October 2013 16:36, Peter Maydell <address@hidden> wrote:
> Are you sure the logic in this function is right? (ie that we
> should only clear the sgi_source[][] bit for this source, and
> not completely? If nothing else, I think the interrupt should
> stay pending if some other source cpu still wants it to be
> pending. That is, I think we need to track the pending status
> separately for each (irq,target-cpu,source-cpu) separately for
> SGIs. (I'm not totally sure I have this right though, the spec
> is quite confusing.)
Having spoken to Marc I'm now a bit less confused. For
SGIs:
* there is effectively a pending bit for each
(irq-number, source-cpu, target-cpu) tuple
* for a v2 GIC these are guest-visible in GICD_CPENDSGIRn
and GICD_SPENDSGIRn; for a pre-v2 GIC the state still
exists, it's just not guest-visible
* the overall "pending state" bit in GICD_ISPENDRn and
GICD_ICPENDRn is effectively the logical OR of the
pending bits for each source-cpu; writes to this bit
are ignored
* when the guest reads from GICC_IAR, if the SGI is
pending for multiple source-cpus for this target-cpu
then we clear the pending bit for the (int, src, tgt)
tuple in question, but it will still be set for the
other (int,src,tgt) tuples for the other source-cpus
(and so the GICD_ISPENDRn bit will still be set)
Tangentially, I notice that we don't correctly handle
the PENDING bit for level triggered interrupts, since
we do:
/* Clear pending flags for both level and edge triggered interrupts.
Level triggered IRQs will be reasserted once they become inactive. */
gic_clear_pending(s, new_irq, GIC_TEST_MODEL(new_irq) ? ALL_CPU_MASK : cm,
GIC_SGI_SRC(new_irq, cpu));
in gic_acknowledge_irq(). This is wrong, because section
3.2.4 is clear for a level triggered interrupt that if the
interrupt signal remains asserted (which it usually will be)
then we go from Pending to Active+Pending (whereas our
current implementation goes from Pending to Active and
then resets Pending later in gic_complete_irq()).
-- PMM