Re: [Minios-devel] [UNIKRAFT PATCHv3 5/7] plat/common: Find and register IRQ for arch_timer

[Julien Grall <julien.grall@xxxxxxx>]

On 9/16/19 8:52 AM, Justin He (Arm Technology China) wrote:
> Hi  Santiago
Hi all,

@Santiago, it is quite difficult to follow the thread when you start 
your answer with "COMMENT". May I ask you to configure your e-mail 
client to quote properly (i.e >)?
Furthermore, disclaimer footer should be avoided on the mailing list. 
You are basically saying this is confidential but you send to everyone 
(mailing list are archived)...
[...]

> > COMMENT: There is nothing that we would like to do here? Not even
> > disable the IRQ? As the timer is not stopped, when the counter overflows
> > we would get a new interrupt otherwise (although the overflow could
> > happen in a very very long time, right?)
> In previous version, we added a generic_timer_mask_irq() in
> generic_timer_irq_handler. But as per the suggestion [1] from Julien, we
> removed it. Besides, we referred to the minios logic at [2], it only called
> unmask and mask in block_domain (which is equivalent to unikraft's
> generic_timer_cpu_block)
Looking at my comments again, I am not sure where I suggested to remove 
generic_timer_mask_irq()... Can you expand it?
FWIW, the two main comments on the previous versions were:
   1) isb() should be added after updating the system register to 
ensure that the system system is synchronized
   2) This is common code between arm32 and arm64. But the system 
register name are arm64... Accesses should be stub in arch-specific 
header so the code can work for both arm32 and arm64.
From a quick look 1) has been addressed, 2) is still pending.

Cheers,

--
Julien Grall

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