Re: [Xen-devel] [PATCH for-4.5 v11 2/7] xen/arm: Add vgic callback to read irq priority

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 15 Sep 2014, Vijay Kilari wrote:
> On Fri, Sep 12, 2014 at 11:38 PM, Stefano Stabellini
> <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> > On Fri, 12 Sep 2014, vijay.kilari@xxxxxxxxx wrote:
> >> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
> >>
> >> Use callback in vgic driver to read priority for
> >> a given irq
> >>
> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxxxxxxxxxx>
> >> ---
> >> v11: use vgic_irq_rank
> >> ---
> >>  xen/arch/arm/vgic-v2.c     |   12 ++++++++++++
> >>  xen/arch/arm/vgic.c        |    2 +-
> >>  xen/include/asm-arm/vgic.h |    2 ++
> >>  3 files changed, 15 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> >> index 54751b6..e674192 100644
> >> --- a/xen/arch/arm/vgic-v2.c
> >> +++ b/xen/arch/arm/vgic-v2.c
> >> @@ -521,6 +521,17 @@ static struct vcpu *vgic_v2_get_target_vcpu(struct 
> >> vcpu *v, unsigned int irq)
> >>      return v_target;
> >>  }
> >>
> >> +static int vgic_v2_get_irq_priority(struct vcpu *v, unsigned int irq)
> >> +{
> >> +    int priority;
> >> +    struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
> >
> > this is good
> >
> >
> >> +    ASSERT(spin_is_locked(&rank->lock));
> >> +    priority = vgic_byte_read(rank->ipriority[(irq%32)/4], 0, irq % 4);
> >
> > this has been changed without mention in the change log for the patch, it 
> > was
> >
> > priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8, irq,
> >             DABT_WORD)], 0, irq & 0x3);
> >
> > It is best to use the previous version, as it uses the common conversion
> > functions.
> 
>  I changed it because we don't need do clumsy things
> to get ipriority index with irq number in hand. If you don't
> have any further comments, I will send final version with this change
> for your ack.

TBH I think that the open coded version is more readable than the macro,
but we need to be consistent (so that we can easily change the macro and
fix all the call sites in one change for example).

In any case I don't think that you need to resend just for this small
change. We can fix in a follow up patch.


> >
> >
> >
> >
> >> +    return priority;
> >> +}
> >> +
> >>  static int vgic_v2_vcpu_init(struct vcpu *v)
> >>  {
> >>      int i;
> >> @@ -555,6 +566,7 @@ static int vgic_v2_domain_init(struct domain *d)
> >>  static const struct vgic_ops vgic_v2_ops = {
> >>      .vcpu_init   = vgic_v2_vcpu_init,
> >>      .domain_init = vgic_v2_domain_init,
> >> +    .get_irq_priority = vgic_v2_get_irq_priority,
> >>      .get_target_vcpu = vgic_v2_get_target_vcpu,
> >>  };
> >>
> >> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> >> index c6e9479..d24990f 100644
> >> --- a/xen/arch/arm/vgic.c
> >> +++ b/xen/arch/arm/vgic.c
> >> @@ -368,7 +368,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
> >> irq)
> >>      bool_t running;
> >>
> >>      vgic_lock_rank(v, rank, flags);
> >> -    priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8, irq, 
> >> DABT_WORD)], 0, irq & 0x3);
> >> +    priority = v->domain->arch.vgic.handler->get_irq_priority(v, irq);
> >>      vgic_unlock_rank(v, rank, flags);
> >>
> >>      spin_lock_irqsave(&v->arch.vgic.lock, flags);
> >> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> >> index 338ba03..a9f1943 100644
> >> --- a/xen/include/asm-arm/vgic.h
> >> +++ b/xen/include/asm-arm/vgic.h
> >> @@ -96,6 +96,8 @@ struct vgic_ops {
> >>      int (*vcpu_init)(struct vcpu *v);
> >>      /* Domain specific initialization of vGIC */
> >>      int (*domain_init)(struct domain *d);
> >> +    /* Get priority for a given irq stored in vgic structure */
> >> +    int (*get_irq_priority)(struct vcpu *v, unsigned int irq);
> >>      /* Get the target vcpu for a given virq. The rank lock is already 
> >> taken
> >>       * when calling this. */
> >>      struct vcpu *(*get_target_vcpu)(struct vcpu *v, unsigned int irq);
> >> --
> >> 1.7.9.5
> >>
> 

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