Re: [RFC PATCH 17/49] ARM: timer: Handle level triggered IRQs correctly

[Julien Grall <julien.grall@arm.com>]

On 12/02/18 18:23, Andre Przywara wrote:
> Hi,

Hi Andre,

> On 12/02/18 15:19, Julien Grall wrote:
>> Hi Andre,
>>
>> On 09/02/18 14:39, Andre Przywara wrote:
>>> The ARM Generic Timer uses a level-sensitive interrupt semantic. We
>>> easily catch when the line goes high, as this triggers the hardware IRQ.
>>> However we have to sync the state of the interrupt condition at certain
>>> points to catch when the line goes low and we can remove the vtimer vIRQ
>>> from the vGIC (and the LR).
>>> The VGIC in Xen so far only implemented edge triggered vIRQs, really, so
>>> we need to add new functionality to re-sample the interrupt state.
>>
>> You might want to make a summary of the discussion we had with Marc Z.
>> today here. This would help the other to understand why sample the
>> interrupt state is necessary :).
> 
> Yes, I just saw that I somehow missed copying the elaborate comment from
> Christoffer. Fixed now, indeed without this background it's next to
> impossible to understand this ;-)
> 
>> Also do we need to do that for the emulated physical timer?
> 
> Mmh, good question. I believe this whole timer story needs a good think
> again.
> 
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>> ---
>>>    xen/arch/arm/time.c     | 34 ++++++++++++++++++++++++++++++++++
>>>    xen/arch/arm/traps.c    |  1 +
>>>    xen/include/xen/timer.h |  2 ++
>>>    3 files changed, 37 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
>>> index c11fcfeadd..98ebb4305d 100644
>>> --- a/xen/arch/arm/time.c
>>> +++ b/xen/arch/arm/time.c
>>> @@ -263,6 +263,40 @@ static void vtimer_interrupt(int irq, void
>>> *dev_id, struct cpu_user_regs *regs)
>>>        vgic_inject_irq(current->domain, current,
>>> current->arch.virt_timer.irq, true);
>>>    }
>>>    +/**
>>
>> One * is enough.
> 
> That's kernel-doc comment style:
> https://www.kernel.org/doc/html/v4.9/kernel-documentation.html#writing-kernel-doc-comments
> 
> That allows tools to scan the tree and extract function documentation in
> an automated way. A bit like markdown: still perfectly readable by
> humans, but parse-able by scripts as well.
> 
> I was hoping that it wouldn't hurt to have this in Xen as well, as I
> copied this already in other parts of this code.

I was blindly following the CODING_STYLE requirements :). But let's keep 
/** if it helps script.

Cheers,

-- 
Julien Grall

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