Re: [Xen-devel] [PATCH 24/57] ARM: timer: Handle level triggered IRQs correctly

[Julien Grall <julien.grall@xxxxxxx>]

On 06/03/18 17:15, Julien Grall wrote:
> On 05/03/18 16:03, Andre Przywara wrote:
> >   /*
> >    * Arch timer interrupt really ought to be level triggered, since the
> >    * design of the timer/comparator mechanism is based around that
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index 7411bff7a7..0713723bb7 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -2024,6 +2024,12 @@ static void enter_hypervisor_head(struct 
> > cpu_user_regs *regs)
> >           if ( current->arch.hcr_el2 & HCR_VA )
> >               current->arch.hcr_el2 = READ_SYSREG(HCR_EL2);
> > +        /*
> > +         * We need to update the state of our emulated devices using 
> > level
> > +         * triggered interrupts before syncing back the VGIC state.
> > +         */
> > +        vtimer_sync(current);
> Am I wondering if it would be worth to #ifdef the code so it is only 
> used for the new vGIC? After all, this will be a nop for it, right?
Also, I would like to see a TODO in the code and the cover letter about 
optimizing this (see RFC patch #17):
"I am a bit worry about re-sampling the virtual interrupt state at every 
traps. It might be worth thinking to do the re-sample when syncing the 
LRs (as you do for HW level interrupt in patch #25). Probably once we 
get the new vGIC merged."
It is not a deal breaker right now. But I don't want see to be forgotten 
once after we merge it.
Cheers,

--
Julien Grall

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