[PATCH 2/9] KVM: arm/arm64: vgic: Avoid flushing vgic state when there's no pending IRQ

[Marc Zyngier]

On 20/03/17 10:58, Christoffer Dall wrote:
> From: Shih-Wei Li <shihwei at cs.columbia.edu>
> 
> We do not need to flush vgic states in each world switch unless
> there is pending IRQ queued to the vgic's ap list. We can thus reduce
> the overhead by not grabbing the spinlock and not making the extra
> function call to vgic_flush_lr_state.
> 
> Signed-off-by: Shih-Wei Li <shihwei at cs.columbia.edu>
> Signed-off-by: Christoffer Dall <cdall at linaro.org>
> ---
>  virt/kvm/arm/vgic/vgic.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> index 2ac0def..1436c2e 100644
> --- a/virt/kvm/arm/vgic/vgic.c
> +++ b/virt/kvm/arm/vgic/vgic.c
> @@ -637,12 +637,17 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
>  /* Sync back the hardware VGIC state into our emulation after a guest's run. */
>  void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
>  {
> +	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> +
>  	if (unlikely(!vgic_initialized(vcpu->kvm)))
>  		return;
>  
>  	vgic_process_maintenance_interrupt(vcpu);
>  	vgic_fold_lr_state(vcpu);
>  	vgic_prune_ap_list(vcpu);
> +
> +	/* Make sure we can fast-path in flush_hwstate */
> +	vgic_cpu->used_lrs = 0;
>  }
>  
>  /* Flush our emulation state into the GIC hardware before entering the guest. */
> @@ -651,6 +656,9 @@ void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
>  	if (unlikely(!vgic_initialized(vcpu->kvm)))
>  		return;
>  
> +	if (list_empty(&vcpu->arch.vgic_cpu.ap_list_head))
> +		return;
> +

While I can see the READ_ONCE in list_empty(), it is not completely
obvious that the race with another CPU injecting an interrupt is safe
(if I get it correctly, it will have the same effect as if it was added
right after the critical section below).

Can we have a nice comment explaining this?

>  	spin_lock(&vcpu->arch.vgic_cpu.ap_list_lock);
>  	vgic_flush_lr_state(vcpu);
>  	spin_unlock(&vcpu->arch.vgic_cpu.ap_list_lock);
> 

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...