On 19.04.2013, at 20:02, Scott Wood wrote:
> On 04/19/2013 09:06:26 AM, Alexander Graf wrote:
>> diff --git a/Documentation/virtual/kvm/devices/mpic.txt
>> b/Documentation/virtual/kvm/devices/mpic.txt
>> index ce98e32..dadc1e0 100644
>> --- a/Documentation/virtual/kvm/devices/mpic.txt
>> +++ b/Documentation/virtual/kvm/devices/mpic.txt
>> @@ -35,3 +35,14 @@ Groups:
>> "attr" is the IRQ number. IRQ numbers for standard sources are the
>> byte offset of the relevant IVPR from EIVPR0, divided by 32.
>> +
>> +IRQ Routing:
>> +
>> + The MPIC emulation supports IRQ routing. Only a single MPIC device can
>> + be instantiated. Once that device has been created, it's available as
>> + irqchip id 0.
>> +
>
>> + This irqchip 0 has 256 interrupt pins. These pins reflect the SRC pins
>> + on the MPIC controller.
>
> This irqchip 0 has 256 interrupt pins, which expose the interrupts in the
> main array of interrupt sources (a.k.a. "SRC" interrupts). The numbering is
> the same as the MPIC device tree binding -- based on the register offset from
> the beginning of the sources array, without regard to any subdivisions in
> chip documentation such as "internal" or "external" interrupts. Default
> routes are established for these pins, with the GSI being equal to the pin
> number.
>
>> + Access to on-SRC registers is not implemented through IRQ routing
>> mechanisms.
>
> s/on-SRC registers/non-SRC interrupts/
>
>> diff --git a/arch/powerpc/kvm/mpic.c b/arch/powerpc/kvm/mpic.c
>> index 10bc08a..d137df8 100644
>> --- a/arch/powerpc/kvm/mpic.c
>> +++ b/arch/powerpc/kvm/mpic.c
>> @@ -1029,6 +1029,7 @@ static int openpic_cpu_write_internal(void *opaque,
>> gpa_t addr,
>> struct irq_source *src;
>> struct irq_dest *dst;
>> int s_IRQ, n_IRQ;
>> + int notify_eoi = -1;
>> pr_debug("%s: cpu %d addr %#llx <= 0x%08x\n", __func__, idx,
>> addr, val);
>> @@ -1087,6 +1088,8 @@ static int openpic_cpu_write_internal(void *opaque,
>> gpa_t addr,
>> }
>> IRQ_resetbit(&dst->servicing, s_IRQ);
>> + /* Notify listeners that the IRQ is over */
>> + notify_eoi = s_IRQ;
>> /* Set up next servicing IRQ */
>> s_IRQ = IRQ_get_next(opp, &dst->servicing);
>> /* Check queued interrupts. */
>> @@ -1104,6 +1107,12 @@ static int openpic_cpu_write_internal(void *opaque,
>> gpa_t addr,
>> break;
>> }
>> + if (notify_eoi != -1) {
>> + spin_unlock_irq(&opp->lock);
>> + kvm_notify_acked_irq(opp->kvm, 0, notify_eoi);
>> + spin_lock_irq(&opp->lock);
>> + }
>
> I'd rather not have the "_irq" here, which could break if we enter this patch
> via an "_irqsave" (I realize there currently is no such path that reaches EOI
> emulation).
>
> Will we ever set notify_eoi when addr != EOI? I'm wondering why it was moved
> out of the switch statement, instead of being put at the end of the case EOI:
> code.
I doubt it, but that's for the compiler to optimize away. I found it cleaner
for some reason to put it down there. I don't think it really matters.
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
