Re: [XEN v1] xen: arm: Check if timer is enabled for timer irq

[Julien Grall <julien@xxxxxxx>]

Hi Ayan,

Bertrand already made some comments. I will try to avoid repeating the
same comments.

On 10/08/2022 11:58, Ayan Kumar Halder wrote:
> Refer "Arm Architecture Registers DDI 0595", AArch32 system registers,
You are modifying code that is common between AArch64 and AArch32. So I 
would mention this behavior is common. Also, please specify the version
of the spec. This helps in case the behavior has changed.

Also, NIT: I would prefer if you quote the Arm Arm rather than auxiliary
specifications.

> CNTHP_CTL :- "When the value of the ENABLE bit is 1, ISTATUS indicates
> whether the timer condition is met."
I think the key point here is the field ISTATUS is "unknown" when the 
ENABLE bit is 0.
> Also similar description applies to CNTP_CTL as well.
>
> One should always check that the timer is enabled and status is set, to
> determine if the timer interrupt has been generated.
I understand the theory. In practice, I am not sure this could ever 
happen because the timer interrupt is level and by clearing *_CTL you 
will lower the line and therefore you should not receive the interrupt 
again.
Checking the 'enable' is not going to add too much overhead. So I am 
fine if this is added. That said, would you be able to provide more 
details on how this was spotted?
Cheers,

--
Julien Grall