On Fri, Jul 24, 2015 at 04:55:08PM +0100, Marc Zyngier wrote:
> In order to remove the crude hack where we sneak the masked bit
> into the timer's control register, make use of the phys_irq_map
> API control the active state of the interrupt.
>
> This causes some limited changes to allow for potential error
> propagation.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
> arch/arm/kvm/reset.c | 4 +---
> arch/arm64/kvm/reset.c | 4 +---
> include/kvm/arm_arch_timer.h | 7 +++++--
> virt/kvm/arm/arch_timer.c | 29 ++++++++++++++++++++++-------
> 4 files changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/kvm/reset.c b/arch/arm/kvm/reset.c
> index f558c07..eeb85858 100644
> --- a/arch/arm/kvm/reset.c
> +++ b/arch/arm/kvm/reset.c
> @@ -77,7 +77,5 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> kvm_reset_coprocs(vcpu);
>
> /* Reset arch_timer context */
> - kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
> -
> - return 0;
> + return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
> }
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index b4af618..91cf535 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -121,7 +121,5 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu)
> kvm_reset_sys_regs(vcpu);
>
> /* Reset timer */
> - kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
> -
> - return 0;
> + return kvm_timer_vcpu_reset(vcpu, cpu_vtimer_irq);
> }
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index e596675..e1e4d7c 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -52,13 +52,16 @@ struct arch_timer_cpu {
>
> /* Timer IRQ */
> const struct kvm_irq_level *irq;
> +
> + /* VGIC mapping */
> + struct irq_phys_map *map;
> };
>
> int kvm_timer_hyp_init(void);
> void kvm_timer_enable(struct kvm *kvm);
> void kvm_timer_init(struct kvm *kvm);
> -void kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> - const struct kvm_irq_level *irq);
> +int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> + const struct kvm_irq_level *irq);
> void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu);
> void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu);
> void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu);
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 98c95f2..76e38d2 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -64,10 +64,10 @@ static void kvm_timer_inject_irq(struct kvm_vcpu *vcpu)
> int ret;
> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
>
> - timer->cntv_ctl |= ARCH_TIMER_CTRL_IT_MASK;
> - ret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id,
> - timer->irq->irq,
> - timer->irq->level);
> + kvm_vgic_set_phys_irq_active(timer->map, true);
actually, I'm not sure I understand the point of setting this boolean?
Is it just a check? It seems everywhere we find the map in the VGIC
code, we have a BUG_ON(!map->active), so why doesn't (!!map) imply that
it should be marked as active?
Is this again for non-shared devices?
> + ret = kvm_vgic_inject_mapped_irq(vcpu->kvm, vcpu->vcpu_id,
> + timer->map,
> + timer->irq->level);
> WARN_ON(ret);
> }
>
> @@ -117,7 +117,8 @@ bool kvm_timer_should_fire(struct kvm_vcpu *vcpu)
> cycle_t cval, now;
>
> if ((timer->cntv_ctl & ARCH_TIMER_CTRL_IT_MASK) ||
> - !(timer->cntv_ctl & ARCH_TIMER_CTRL_ENABLE))
> + !(timer->cntv_ctl & ARCH_TIMER_CTRL_ENABLE) ||
> + kvm_vgic_get_phys_irq_active(timer->map))
hmm, maybe this answers my question above. It is the virtual active
state preventing us to keep injecting the virtual side?
I don't see anything functional wrong with this code though.
Thanks,
-Christoffer
> return false;
>
> cval = timer->cntv_cval;
> @@ -184,10 +185,11 @@ void kvm_timer_sync_hwstate(struct kvm_vcpu *vcpu)
> timer_arm(timer, ns);
> }
>
> -void kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> - const struct kvm_irq_level *irq)
> +int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> + const struct kvm_irq_level *irq)
> {
> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
> + struct irq_phys_map *map;
>
> /*
> * The vcpu timer irq number cannot be determined in
> @@ -196,6 +198,17 @@ void kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> * vcpu timer irq number when the vcpu is reset.
> */
> timer->irq = irq;
> +
> + /*
> + * 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);
> + if (WARN_ON(IS_ERR(map)))
> + return PTR_ERR(map);
> +
> + timer->map = map;
> + return 0;
> }
>
> void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu)
> @@ -335,6 +348,8 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu)
> struct arch_timer_cpu *timer = &vcpu->arch.timer_cpu;
>
> timer_disarm(timer);
> + if (timer->map)
> + kvm_vgic_unmap_phys_irq(vcpu, timer->map);
> }
>
> void kvm_timer_enable(struct kvm *kvm)
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
