On Thu, Apr 21, 2016 at 07:41:01PM +0200, Eric Auger wrote:
> Hi Andre,
> On 04/15/2016 04:04 PM, Andre Przywara wrote:
> > From: Christoffer Dall <christoffer.d...@linaro.org>
> > 
> > The communication of a Linux IRQ number from outside the VGIC to the
> > vgic was a leftover from the day when the vgic code cared about how a
> > particular device injects virtual interrupts mapped to a physical
> > interrupt.
> > 
> > We can safely remove this notion, leaving all physical IRQ handling to
> > be done in the device driver (the arch timer in this case), which makes
> > room for a saner API for the new VGIC.
> > 
> > Signed-off-by: Christoffer Dall <christoffer.d...@linaro.org>
> > Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
> > ---
> >  include/kvm/arm_vgic.h    |  3 +--
> >  virt/kvm/arm/arch_timer.c | 22 ++++++++++++++++++++--
> >  virt/kvm/arm/vgic.c       | 20 ++------------------
> >  3 files changed, 23 insertions(+), 22 deletions(-)
> > 
> > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> > index 43eeb18..49c559e 100644
> > --- a/include/kvm/arm_vgic.h
> > +++ b/include/kvm/arm_vgic.h
> > @@ -157,7 +157,6 @@ struct vgic_io_device {
> >  struct irq_phys_map {
> >     u32                     virt_irq;
> >     u32                     phys_irq;
> > -   u32                     irq;
> >  };
> >  
> >  struct irq_phys_map_entry {
> > @@ -345,7 +344,7 @@ int kvm_vgic_inject_mapped_irq(struct kvm *kvm, int 
> > cpuid,
> >  void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
> >  int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
> >  struct irq_phys_map *kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu,
> > -                                      int virt_irq, int irq);
> > +                                      int virt_irq, int phys_irq);
> >  int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int virt_irq);
> >  bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int virt_irq);
> >  
> > diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> > index b4d96b1..cfdf88f 100644
> > --- a/virt/kvm/arm/arch_timer.c
> > +++ b/virt/kvm/arm/arch_timer.c
> > @@ -274,7 +274,7 @@ void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
> >     if (timer->active_cleared_last && !phys_active)
> >             return;
> >  
> > -   ret = irq_set_irqchip_state(timer->map->irq,
> > +   ret = irq_set_irqchip_state(host_vtimer_irq,
> >                                 IRQCHIP_STATE_ACTIVE,
> >                                 phys_active);
> >     WARN_ON(ret);
> > @@ -307,6 +307,9 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> >  {
> >     struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> >     struct irq_phys_map *map;
> > +   struct irq_desc *desc;
> > +   struct irq_data *data;
> > +   int phys_irq;
> >  
> >     /*
> >      * The vcpu timer irq number cannot be determined in
> > @@ -326,10 +329,25 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> >     kvm_timer_update_state(vcpu);
> >  
> >     /*
> > +    * Find the physical IRQ number corresponding to the host_vtimer_irq
> > +    */
> > +   desc = irq_to_desc(host_vtimer_irq);
> > +   if (!desc) {
> can this really happen?

this is just moving the logic.  We had this check before, so I assume
so...

> > +           kvm_err("%s: no interrupt descriptor\n", __func__);
> > +           return -EINVAL;
> > +   }
> > +
> > +   data = irq_desc_get_irq_data(desc);
> > +   while (data->parent_data)
> > +           data = data->parent_data;
> > +
> > +   phys_irq = data->hwirq;
> > +
> > +   /*
> >      * Tell the VGIC that the virtual interrupt is tied to a
> >      * physical interrupt. We do that once per VCPU.
> >      */
> > -   map = kvm_vgic_map_phys_irq(vcpu, irq->irq, host_vtimer_irq);
> > +   map = kvm_vgic_map_phys_irq(vcpu, irq->irq, phys_irq);
> >     if (WARN_ON(IS_ERR(map)))
> >             return PTR_ERR(map);
> >  
> > diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> > index 2d7ae35..ac5838b 100644
> > --- a/virt/kvm/arm/vgic.c
> > +++ b/virt/kvm/arm/vgic.c
> > @@ -1723,27 +1723,13 @@ static struct list_head 
> > *vgic_get_irq_phys_map_list(struct kvm_vcpu *vcpu,
> >   * Returns a valid pointer on success, and an error pointer otherwise
> >   */
> the doc comment must be updated
>  * @irq: The Linux IRQ number
> 
> Besides
> 
> Reviewed-by: Eric Auger <eric.au...@linaro.org>
> 
Thanks!

Andre, let me know if you need me to provide an updated patch or if you
can just tweak that comment.

-Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to