Re: [Xen-devel] [PATCH 1/2] xen/arm: support the ARM generic virtual timer

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-02-14 at 11:46 +0000, Stefano Stabellini wrote:
> On Thu, 24 Jan 2013, Tim Deegan wrote:
> > At 16:17 +0000 on 09 Jan (1357748277), Stefano Stabellini wrote:
> > > +static void vtimer_interrupt(int irq, void *dev_id, struct cpu_user_regs 
> > > *regs)
> > > +{
> > > +    current->arch.virt_timer.ctl = READ_CP32(CNTV_CTL);
> > > +    WRITE_CP32(current->arch.virt_timer.ctl | CNTx_CTL_MASK, CNTV_CTL);
> > 
> > This is masking the vtimer interrupt in a way that's visible to the
> > currently running guest.  Is that going to confuse the guest?
> >
> > When we talked about this before I had imagined that the masking would
> > happen in the GIC, where the guest doesn't see it. 
> 
> I know it is not ideal but it is safe, it is not creating any problems to
> Linux and it is even the same thing that KVM does).
> More importantly I just couldn't get the GIC strategy to work correctly
> (actually it work decently well on the physical Versatile Express but it
> hangs the emulator, this tells me that it is not the way it was intended
> to be used by the ARM people).

I think we should ask ARM about this, it could just be a model bug. I'm
sure that they would be interested in cases where the virtualness leaks
out of the hardware like this.

Ian.

> 
> 
> > > +    vgic_vcpu_inject_irq(current, irq, 1);
> > > +}
> > > +
> > >  /* Set up the timer interrupt on this CPU */
> > >  void __cpuinit init_timer_interrupt(void)
> > >  {
> > > @@ -172,6 +183,7 @@ void __cpuinit init_timer_interrupt(void)
> > >  
> > >      /* XXX Need to find this IRQ number from devicetree? */
> > >      request_irq(26, timer_interrupt, 0, "hyptimer", NULL);
> > > +    request_irq(27, vtimer_interrupt, 0, "virtimer", NULL);
> > >      request_irq(30, timer_interrupt, 0, "phytimer", NULL);
> > >  }
> > >  
> > > +static void virt_timer_expired(void *data)
> > > +{
> > > +    struct vtimer *t = data;
> > > +    vcpu_wake(t->v);
> > 
> > Shouldn't this also inject the irq?  Otherwise when an unscheduled
> > guest's timer goes off, we take two interrupts - one for the hyp timer
> > to call this function and then another immediately after
> > virt_timer_restore() which causes us to inject the vtimer irq. 
> > If we injected the irq here (and arranged to mask the vtimer irq) we'd
> > avoid a second interrupt.
> 
> This might be a good idea, I'll give it a try.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel