Re: [Xen-devel] [PATCH v3 03/39] ARM: GIC: Allow tweaking the active and pending state of an IRQ

[Andre Przywara <andre.przywara@xxxxxxxxxx>]

Hi,

On 22/03/18 01:51, Julien Grall wrote:
> Hi Andre,
> 
> On 03/21/2018 04:31 PM, Andre Przywara wrote:
>> When playing around with hardware mapped, level triggered virtual IRQs,
>> there is the need to explicitly set the active or pending state of an
>> interrupt at some point.
>> To prepare the GIC for that, we introduce a set_active_state() and a
>> set_pending_state() function to let the VGIC manipulate the state of
>> an associated hardware IRQ.
>> This takes care of properly setting the _IRQ_INPROGRESS bit.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
>> ---
>> Changelog v2 ... v3:
>> - rework setting _IRQ_INPROGRESS bit:
>>    - no change when changing active state
>>    - unconditional set/clear on changing pending state
>> - drop introduction of gicv[23]_peek_irq() (only needed in the next
>> patch now)
>>
>> Changelog v1 ... v2:
>> - properly set _IRQ_INPROGRESS bit
>> - add gicv[23]_peek_irq() (pulled in from later patch)
>> - move wrappers functions into gic.h
>>
>>   xen/arch/arm/gic-v2.c     | 36 ++++++++++++++++++++++++++++++++++++
>>   xen/arch/arm/gic-v3.c     | 32 ++++++++++++++++++++++++++++++++
>>   xen/include/asm-arm/gic.h | 24 ++++++++++++++++++++++++
>>   3 files changed, 92 insertions(+)
>>
>> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
>> index aa0fc6c1a1..d1f1578c05 100644
>> --- a/xen/arch/arm/gic-v2.c
>> +++ b/xen/arch/arm/gic-v2.c
>> @@ -243,6 +243,40 @@ static void gicv2_poke_irq(struct irq_desc *irqd,
>> uint32_t offset)
>>       writel_gicd(1U << (irqd->irq % 32), offset + (irqd->irq / 32) * 4);
>>   }
>>   +static void gicv2_set_active_state(struct irq_desc *irqd, bool active)
>> +{
>> +    ASSERT(spin_is_locked(&irqd->lock));
>> +
>> +    if ( active )
>> +    {
>> +        if ( test_bit(_IRQ_GUEST, &irqd->status) )
> 
> I don't understand why you only set/clear INPROGRESS bit for interrupt
> routed to guest. This will matter when releasing interrupt used by Xen
> (see release_irq).

D'oh, indeed! Seems like I am too focused on the _V_GIC these days ;-)

Fixed.

Cheers,
Andre.

> Note that I don't expect this helper to be call on Xen IRQ, but I think
> we should make
> 
> Other than same remark on GICv3 code, the pending implementation looks
> good to me now.
> 
> Cheers,
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel