Re: [PATCH v2 2/4] KVM: ARM: vgic: register kvm_device_ops dynamically

[Paolo Bonzini <pbonzini@redhat.com>]

Il 01/07/2014 16:45, Will Deacon ha scritto:
> Now that we have a dynamic means to register kvm_device_ops, use that
> for the ARM VGIC, instead of relying on the static table.
>
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  include/linux/kvm_host.h |   1 -
>  virt/kvm/arm/vgic.c      | 156 +++++++++++++++++++++++------------------------
>  virt/kvm/kvm_main.c      |   4 --
>  3 files changed, 78 insertions(+), 83 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index b75faaf0d76d..4317fdd10696 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1088,7 +1088,6 @@ int kvm_register_device_ops(struct kvm_device_ops *ops, u32 type);
>  extern struct kvm_device_ops kvm_mpic_ops;
>  extern struct kvm_device_ops kvm_xics_ops;
>  extern struct kvm_device_ops kvm_vfio_ops;
> -extern struct kvm_device_ops kvm_arm_vgic_v2_ops;
>  extern struct kvm_device_ops kvm_flic_ops;
>
>  #ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 795ab482333d..d9d0bcebad2b 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -1502,83 +1502,6 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
>  	return 0;
>  }
>
> -static void vgic_init_maintenance_interrupt(void *info)
> -{
> -	enable_percpu_irq(vgic->maint_irq, 0);
> -}
> -
> -static int vgic_cpu_notify(struct notifier_block *self,
> -			   unsigned long action, void *cpu)
> -{
> -	switch (action) {
> -	case CPU_STARTING:
> -	case CPU_STARTING_FROZEN:
> -		vgic_init_maintenance_interrupt(NULL);
> -		break;
> -	case CPU_DYING:
> -	case CPU_DYING_FROZEN:
> -		disable_percpu_irq(vgic->maint_irq);
> -		break;
> -	}
> -
> -	return NOTIFY_OK;
> -}
> -
> -static struct notifier_block vgic_cpu_nb = {
> -	.notifier_call = vgic_cpu_notify,
> -};
> -
> -static const struct of_device_id vgic_ids[] = {
> -	{ .compatible = "arm,cortex-a15-gic", .data = vgic_v2_probe, },
> -	{ .compatible = "arm,gic-v3", .data = vgic_v3_probe, },
> -	{},
> -};
> -
> -int kvm_vgic_hyp_init(void)
> -{
> -	const struct of_device_id *matched_id;
> -	int (*vgic_probe)(struct device_node *,const struct vgic_ops **,
> -			  const struct vgic_params **);
> -	struct device_node *vgic_node;
> -	int ret;
> -
> -	vgic_node = of_find_matching_node_and_match(NULL,
> -						    vgic_ids, &matched_id);
> -	if (!vgic_node) {
> -		kvm_err("error: no compatible GIC node found\n");
> -		return -ENODEV;
> -	}
> -
> -	vgic_probe = matched_id->data;
> -	ret = vgic_probe(vgic_node, &vgic_ops, &vgic);
> -	if (ret)
> -		return ret;
> -
> -	ret = request_percpu_irq(vgic->maint_irq, vgic_maintenance_handler,
> -				 "vgic", kvm_get_running_vcpus());
> -	if (ret) {
> -		kvm_err("Cannot register interrupt %d\n", vgic->maint_irq);
> -		return ret;
> -	}
> -
> -	ret = __register_cpu_notifier(&vgic_cpu_nb);
> -	if (ret) {
> -		kvm_err("Cannot register vgic CPU notifier\n");
> -		goto out_free_irq;
> -	}
> -
> -	on_each_cpu(vgic_init_maintenance_interrupt, NULL, 1);
> -
> -	/* Callback into for arch code for setup */
> -	vgic_arch_setup(vgic);
> -
> -	return 0;
> -
> -out_free_irq:
> -	free_percpu_irq(vgic->maint_irq, kvm_get_running_vcpus());
> -	return ret;
> -}
> -
>  /**
>   * kvm_vgic_init - Initialize global VGIC state before running any VCPUs
>   * @kvm: pointer to the kvm struct
> @@ -2042,7 +1965,7 @@ static int vgic_create(struct kvm_device *dev, u32 type)
>  	return kvm_vgic_create(dev->kvm);
>  }
>
> -struct kvm_device_ops kvm_arm_vgic_v2_ops = {
> +static struct kvm_device_ops kvm_arm_vgic_v2_ops = {
>  	.name = "kvm-arm-vgic",
>  	.create = vgic_create,
>  	.destroy = vgic_destroy,
> @@ -2050,3 +1973,80 @@ struct kvm_device_ops kvm_arm_vgic_v2_ops = {
>  	.get_attr = vgic_get_attr,
>  	.has_attr = vgic_has_attr,
>  };
> +
> +static void vgic_init_maintenance_interrupt(void *info)
> +{
> +	enable_percpu_irq(vgic->maint_irq, 0);
> +}
> +
> +static int vgic_cpu_notify(struct notifier_block *self,
> +			   unsigned long action, void *cpu)
> +{
> +	switch (action) {
> +	case CPU_STARTING:
> +	case CPU_STARTING_FROZEN:
> +		vgic_init_maintenance_interrupt(NULL);
> +		break;
> +	case CPU_DYING:
> +	case CPU_DYING_FROZEN:
> +		disable_percpu_irq(vgic->maint_irq);
> +		break;
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block vgic_cpu_nb = {
> +	.notifier_call = vgic_cpu_notify,
> +};
> +
> +static const struct of_device_id vgic_ids[] = {
> +	{ .compatible = "arm,cortex-a15-gic", .data = vgic_v2_probe, },
> +	{ .compatible = "arm,gic-v3", .data = vgic_v3_probe, },
> +	{},
> +};
> +
> +int kvm_vgic_hyp_init(void)
> +{
> +	const struct of_device_id *matched_id;
> +	int (*vgic_probe)(struct device_node *,const struct vgic_ops **,
> +			  const struct vgic_params **);
> +	struct device_node *vgic_node;
> +	int ret;
> +
> +	vgic_node = of_find_matching_node_and_match(NULL,
> +						    vgic_ids, &matched_id);
> +	if (!vgic_node) {
> +		kvm_err("error: no compatible GIC node found\n");
> +		return -ENODEV;
> +	}
> +
> +	vgic_probe = matched_id->data;
> +	ret = vgic_probe(vgic_node, &vgic_ops, &vgic);
> +	if (ret)
> +		return ret;
> +
> +	ret = request_percpu_irq(vgic->maint_irq, vgic_maintenance_handler,
> +				 "vgic", kvm_get_running_vcpus());
> +	if (ret) {
> +		kvm_err("Cannot register interrupt %d\n", vgic->maint_irq);
> +		return ret;
> +	}
> +
> +	ret = __register_cpu_notifier(&vgic_cpu_nb);
> +	if (ret) {
> +		kvm_err("Cannot register vgic CPU notifier\n");
> +		goto out_free_irq;
> +	}
> +
> +	on_each_cpu(vgic_init_maintenance_interrupt, NULL, 1);
> +
> +	/* Callback into for arch code for setup */
> +	vgic_arch_setup(vgic);
> +	return kvm_register_device_ops(&kvm_arm_vgic_v2_ops,
> +				       KVM_DEV_TYPE_ARM_VGIC_V2);
> +
> +out_free_irq:
> +	free_percpu_irq(vgic->maint_irq, kvm_get_running_vcpus());
> +	return ret;
> +}
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index a8539ae10247..7eab47222dcf 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2271,10 +2271,6 @@ static struct kvm_device_ops *kvm_device_ops_table[KVM_DEV_TYPE_MAX] = {
>  	[KVM_DEV_TYPE_VFIO]		= &kvm_vfio_ops,
>  #endif
>
> -#ifdef CONFIG_KVM_ARM_VGIC
> -	[KVM_DEV_TYPE_ARM_VGIC_V2]	= &kvm_arm_vgic_v2_ops,
> -#endif
> -
>  #ifdef CONFIG_S390
>  	[KVM_DEV_TYPE_FLIC]		= &kvm_flic_ops,
>  #endif
>

Christopher, Marc, can you review/ack this?

Paolo