Re: [Xen-devel] [PATCH 7/7] xen/arm: phys_timer fixes

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

> @@ -33,8 +34,8 @@ static void phys_timer_expired(void *data)
>  {
>      struct vtimer *t = data;
>      t->ctl |= CNTx_CTL_PENDING;
> -    t->ctl &= ~CNTx_CTL_MASK;
> -    vgic_vcpu_inject_irq(t->v, 30, 1);
> +    if ( !(t->ctl & CNTx_CTL_MASK) )
> +        vgic_vcpu_inject_irq(t->v, 30, 1);

Thinking about the previous discussion about exposing the change of the
mask bit to the guest, it just occurred to me that t->ctl.MASK need not
actually represent the state of the underlying physical mask bit, since
we do emulate accesses after all.

>  }
>  
>  static void virt_timer_expired(void *data)
> @@ -44,6 +45,17 @@ static void virt_timer_expired(void *data)
>      vgic_vcpu_inject_irq(t->v, 27, 1);
>  }
>   
> +static void phys_timer_gic_callback(struct vcpu *v, int irq)
> +{
> +    v->arch.phys_timer.ctl &= ~CNTx_CTL_PENDING;
> +}
> +
> +int __init init_ptimer(void)
> +{
> +    register_gic_callback(30, phys_timer_gic_callback);
> +    return 0;
> +}
> +
>  int vcpu_vtimer_init(struct vcpu *v)
>  {
>      struct vtimer *t = &v->arch.phys_timer;
> @@ -119,13 +131,15 @@ static int vtimer_emulate_32(struct cpu_user_regs 
> *regs, union hsr hsr)
>          {
>              v->arch.phys_timer.ctl = *r;
>  
> -            if ( v->arch.phys_timer.ctl & CNTx_CTL_ENABLE )
> +            if ( ((v->arch.phys_timer.ctl & CNTx_CTL_MASK) &&
> +                  (v->arch.phys_timer.ctl & CNTx_CTL_PENDING)) ||

Why does PENDING matter here and below?

> +                 !(v->arch.phys_timer.ctl & CNTx_CTL_ENABLE) )
> +                stop_timer(&v->arch.phys_timer.timer);
> +            else
>              {
>                  set_timer(&v->arch.phys_timer.timer,
>                            v->arch.phys_timer.cval + 
> v->arch.phys_timer.offset);
>              }
> -            else
> -                stop_timer(&v->arch.phys_timer.timer);
>          }
>  
>          return 1;
> @@ -139,7 +153,11 @@ static int vtimer_emulate_32(struct cpu_user_regs *regs, 
> union hsr hsr)
>          else
>          {
>              v->arch.phys_timer.cval = now + ticks_to_ns(*r);
> -            if ( v->arch.phys_timer.ctl & CNTx_CTL_ENABLE )
> +            if ( ((v->arch.phys_timer.ctl & CNTx_CTL_MASK) &&
> +                  (v->arch.phys_timer.ctl & CNTx_CTL_PENDING)) ||
> +                 !(v->arch.phys_timer.ctl & CNTx_CTL_ENABLE) )
> +                stop_timer(&v->arch.phys_timer.timer);
> +            else
>              {
>                  set_timer(&v->arch.phys_timer.timer,
>                            v->arch.phys_timer.cval + 
> v->arch.phys_timer.offset);



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