[PATCH 7/9] KVM: arm/arm64: vgic: Implement early VGIC init functionality

[Marc Zyngier]

On 20/03/17 10:58, Christoffer Dall wrote:
> Implement early initialization for both the distributor and the CPU
> interfaces.  The basic idea is that even though the VGIC is not
> functional or not requested from user space, the critical path of the
> run loop can still call VGIC functions that just won't do anything,
> without them having to check additional initialization flags to ensure
> they don't look at uninitialized data structures.
> 
> Signed-off-by: Christoffer Dall <cdall at linaro.org>
> ---
>  virt/kvm/arm/vgic/vgic-init.c | 96 +++++++++++++++++++++++++------------------
>  1 file changed, 56 insertions(+), 40 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
> index 3762fd1..25fd1b9 100644
> --- a/virt/kvm/arm/vgic/vgic-init.c
> +++ b/virt/kvm/arm/vgic/vgic-init.c
> @@ -24,7 +24,12 @@
>  
>  /*
>   * Initialization rules: there are multiple stages to the vgic
> - * initialization, both for the distributor and the CPU interfaces.
> + * initialization, both for the distributor and the CPU interfaces.  The basic
> + * idea is that even though the VGIC is not functional or not requested from
> + * user space, the critical path of the run loop can still call VGIC functions
> + * that just won't do anything, without them having to check additional
> + * initialization flags to ensure they don't look at uninitialized data
> + * structures.
>   *
>   * Distributor:
>   *
> @@ -39,23 +44,67 @@
>   *
>   * CPU Interface:
>   *
> - * - kvm_vgic_cpu_early_init(): initialization of static data that
> + * - kvm_vgic_vcpu_early_init(): initialization of static data that
>   *   doesn't depend on any sizing information or emulation type. No
>   *   allocation is allowed there.
>   */
>  
>  /* EARLY INIT */
>  
> -/*
> - * Those 2 functions should not be needed anymore but they
> - * still are called from arm.c
> +/**
> + * kvm_vgic_early_init() - Initialize static VGIC VCPU data structures
> + * @kvm: The VM whose VGIC districutor should be initialized
> + *
> + * Only do initialization of static structures that don't require any
> + * allocation or sizing information from userspace.  vgic_init() called
> + * kvm_vgic_dist_init() which takes care of the rest.
>   */
>  void kvm_vgic_early_init(struct kvm *kvm)
>  {
> +	struct vgic_dist *dist = &kvm->arch.vgic;
> +
> +	INIT_LIST_HEAD(&dist->lpi_list_head);
> +	spin_lock_init(&dist->lpi_list_lock);
>  }
>  
> +/**
> + * kvm_vgic_vcpu_early_init() - Initialize static VGIC VCPU data structures
> + * @vcpu: The VCPU whose VGIC data structures whould be initialized
> + *
> + * Only do initialization, but do not actually enable the VGIC CPU interface
> + * yet.
> + */
>  void kvm_vgic_vcpu_early_init(struct kvm_vcpu *vcpu)
>  {
> +	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> +	int i;
> +
> +	INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
> +	spin_lock_init(&vgic_cpu->ap_list_lock);
> +
> +	/*
> +	 * Enable and configure all SGIs to be edge-triggered and
> +	 * configure all PPIs as level-triggered.
> +	 */
> +	for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
> +		struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
> +
> +		INIT_LIST_HEAD(&irq->ap_list);
> +		spin_lock_init(&irq->irq_lock);
> +		irq->intid = i;
> +		irq->vcpu = NULL;
> +		irq->target_vcpu = vcpu;
> +		irq->targets = 1U << vcpu->vcpu_id;
> +		kref_init(&irq->refcount);
> +		if (vgic_irq_is_sgi(i)) {
> +			/* SGIs */
> +			irq->enabled = 1;
> +			irq->config = VGIC_CONFIG_EDGE;
> +		} else {
> +			/* PPIs */
> +			irq->config = VGIC_CONFIG_LEVEL;
> +		}
> +	}
>  }
>  
>  /* CREATION */
> @@ -148,9 +197,6 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
>  	struct kvm_vcpu *vcpu0 = kvm_get_vcpu(kvm, 0);
>  	int i;
>  
> -	INIT_LIST_HEAD(&dist->lpi_list_head);
> -	spin_lock_init(&dist->lpi_list_lock);
> -
>  	dist->spis = kcalloc(nr_spis, sizeof(struct vgic_irq), GFP_KERNEL);
>  	if (!dist->spis)
>  		return  -ENOMEM;
> @@ -181,41 +227,11 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
>  }
>  
>  /**
> - * kvm_vgic_vcpu_init: initialize the vcpu data structures and
> - * enable the VCPU interface
> - * @vcpu: the VCPU which's VGIC should be initialized
> + * kvm_vgic_vcpu_init() - Enable the VCPU interface
> + * @vcpu: the VCPU which's VGIC should be enabled
>   */
>  static void kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
>  {
> -	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
> -	int i;
> -
> -	INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
> -	spin_lock_init(&vgic_cpu->ap_list_lock);
> -
> -	/*
> -	 * Enable and configure all SGIs to be edge-triggered and
> -	 * configure all PPIs as level-triggered.
> -	 */
> -	for (i = 0; i < VGIC_NR_PRIVATE_IRQS; i++) {
> -		struct vgic_irq *irq = &vgic_cpu->private_irqs[i];
> -
> -		INIT_LIST_HEAD(&irq->ap_list);
> -		spin_lock_init(&irq->irq_lock);
> -		irq->intid = i;
> -		irq->vcpu = NULL;
> -		irq->target_vcpu = vcpu;
> -		irq->targets = 1U << vcpu->vcpu_id;
> -		kref_init(&irq->refcount);
> -		if (vgic_irq_is_sgi(i)) {
> -			/* SGIs */
> -			irq->enabled = 1;
> -			irq->config = VGIC_CONFIG_EDGE;
> -		} else {
> -			/* PPIs */
> -			irq->config = VGIC_CONFIG_LEVEL;
> -		}
> -	}
>  	if (kvm_vgic_global_state.type == VGIC_V2)
>  		vgic_v2_enable(vcpu);
>  	else
> 

Since this function is now about enabling the vgic on a given vcpu,
shouldn't the name reflect this? Other than that:

Acked-by: Marc Zyngier <marc.zyngier at arm.com>

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