kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexandru Elisei <alexandru.elisei@arm.com>
To: Marc Zyngier <maz@kernel.org>,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	kvmarm@lists.cs.columbia.edu
Cc: James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Eric Auger <eric.auger@redhat.com>,
	Hector Martin <marcan@marcan.st>,
	Mark Rutland <mark.rutland@arm.com>,
	kernel-team@android.com
Subject: Re: [PATCH v3 1/9] irqchip/gic: Split vGIC probing information from the GIC code
Date: Tue, 18 May 2021 17:51:11 +0100	[thread overview]
Message-ID: <cca6328e-1710-a7ac-e89e-a7dabe16f81f@arm.com> (raw)
In-Reply-To: <20210510134824.1910399-2-maz@kernel.org>

Hi Marc,

On 5/10/21 2:48 PM, Marc Zyngier wrote:
> The vGIC advertising code is unsurprisingly very much tied to
> the GIC implementations. However, we are about to extend the
> support to lesser implementations.
>
> Let's dissociate the vgic registration from the GIC code and
> move it into KVM, where it makes a bit more sense. This also
> allows us to mark the gic_kvm_info structures as __initdata.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/vgic/vgic-init.c        | 18 +++++++++--
>  drivers/irqchip/irq-gic-common.c       | 13 --------
>  drivers/irqchip/irq-gic-common.h       |  2 --
>  drivers/irqchip/irq-gic-v3.c           |  6 ++--
>  drivers/irqchip/irq-gic.c              |  6 ++--
>  include/linux/irqchip/arm-gic-common.h | 25 +---------------
>  include/linux/irqchip/arm-vgic-info.h  | 41 ++++++++++++++++++++++++++
>  7 files changed, 63 insertions(+), 48 deletions(-)
>  create mode 100644 include/linux/irqchip/arm-vgic-info.h
>
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index 58cbda00e56d..2fdb65529594 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -482,6 +482,16 @@ static irqreturn_t vgic_maintenance_handler(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> +static struct gic_kvm_info *gic_kvm_info;
> +
> +void __init vgic_set_kvm_info(const struct gic_kvm_info *info)
> +{
> +	BUG_ON(gic_kvm_info != NULL);
> +	gic_kvm_info = kmalloc(sizeof(*info), GFP_KERNEL);
> +	if (gic_kvm_info)
> +		*gic_kvm_info = *info;
> +}
> +
>  /**
>   * kvm_vgic_init_cpu_hardware - initialize the GIC VE hardware
>   *
> @@ -509,10 +519,8 @@ void kvm_vgic_init_cpu_hardware(void)
>   */
>  int kvm_vgic_hyp_init(void)
>  {
> -	const struct gic_kvm_info *gic_kvm_info;
>  	int ret;
>  
> -	gic_kvm_info = gic_get_kvm_info();
>  	if (!gic_kvm_info)
>  		return -ENODEV;
>  
> @@ -536,10 +544,14 @@ int kvm_vgic_hyp_init(void)
>  		ret = -ENODEV;
>  	}
>  
> +	kvm_vgic_global_state.maint_irq = gic_kvm_info->maint_irq;
> +
> +	kfree(gic_kvm_info);
> +	gic_kvm_info = NULL;

I double checked and gic_kvm_info is not used after this point (vgic_{v2,v3}_probe
make copies of the various fields). And after returning an error (below) this
function cannot be called again.

> +
>  	if (ret)
>  		return ret;
>  
> -	kvm_vgic_global_state.maint_irq = gic_kvm_info->maint_irq;
>  	ret = request_percpu_irq(kvm_vgic_global_state.maint_irq,
>  				 vgic_maintenance_handler,
>  				 "vgic", kvm_get_running_vcpus());
> diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
> index f47b41dfd023..a610821c8ff2 100644
> --- a/drivers/irqchip/irq-gic-common.c
> +++ b/drivers/irqchip/irq-gic-common.c
> @@ -12,19 +12,6 @@
>  
>  static DEFINE_RAW_SPINLOCK(irq_controller_lock);
>  
> -static const struct gic_kvm_info *gic_kvm_info;
> -
> -const struct gic_kvm_info *gic_get_kvm_info(void)
> -{
> -	return gic_kvm_info;
> -}
> -
> -void gic_set_kvm_info(const struct gic_kvm_info *info)
> -{
> -	BUG_ON(gic_kvm_info != NULL);
> -	gic_kvm_info = info;
> -}
> -
>  void gic_enable_of_quirks(const struct device_node *np,
>  			  const struct gic_quirk *quirks, void *data)
>  {
> diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
> index ccba8b0fe0f5..27e3d4ed4f32 100644
> --- a/drivers/irqchip/irq-gic-common.h
> +++ b/drivers/irqchip/irq-gic-common.h
> @@ -28,6 +28,4 @@ void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
>  void gic_enable_of_quirks(const struct device_node *np,
>  			  const struct gic_quirk *quirks, void *data);
>  
> -void gic_set_kvm_info(const struct gic_kvm_info *info);
> -
>  #endif /* _IRQ_GIC_COMMON_H */
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 37a23aa6de37..453fc425eede 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -103,7 +103,7 @@ EXPORT_SYMBOL(gic_nonsecure_priorities);
>  /* ppi_nmi_refs[n] == number of cpus having ppi[n + 16] set as NMI */
>  static refcount_t *ppi_nmi_refs;
>  
> -static struct gic_kvm_info gic_v3_kvm_info;
> +static struct gic_kvm_info gic_v3_kvm_info __initdata;
>  static DEFINE_PER_CPU(bool, has_rss);
>  
>  #define MPIDR_RS(mpidr)			(((mpidr) & 0xF0UL) >> 4)
> @@ -1852,7 +1852,7 @@ static void __init gic_of_setup_kvm_info(struct device_node *node)
>  
>  	gic_v3_kvm_info.has_v4 = gic_data.rdists.has_vlpis;
>  	gic_v3_kvm_info.has_v4_1 = gic_data.rdists.has_rvpeid;
> -	gic_set_kvm_info(&gic_v3_kvm_info);
> +	vgic_set_kvm_info(&gic_v3_kvm_info);
>  }
>  
>  static int __init gic_of_init(struct device_node *node, struct device_node *parent)
> @@ -2168,7 +2168,7 @@ static void __init gic_acpi_setup_kvm_info(void)
>  
>  	gic_v3_kvm_info.has_v4 = gic_data.rdists.has_vlpis;
>  	gic_v3_kvm_info.has_v4_1 = gic_data.rdists.has_rvpeid;
> -	gic_set_kvm_info(&gic_v3_kvm_info);
> +	vgic_set_kvm_info(&gic_v3_kvm_info);
>  }
>  
>  static int __init
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index b1d9c22caf2e..2de9ec8ece0c 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -119,7 +119,7 @@ static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key);
>  
>  static struct gic_chip_data gic_data[CONFIG_ARM_GIC_MAX_NR] __read_mostly;
>  
> -static struct gic_kvm_info gic_v2_kvm_info;
> +static struct gic_kvm_info gic_v2_kvm_info __initdata;
>  
>  static DEFINE_PER_CPU(u32, sgi_intid);
>  
> @@ -1451,7 +1451,7 @@ static void __init gic_of_setup_kvm_info(struct device_node *node)
>  		return;
>  
>  	if (static_branch_likely(&supports_deactivate_key))
> -		gic_set_kvm_info(&gic_v2_kvm_info);
> +		vgic_set_kvm_info(&gic_v2_kvm_info);
>  }
>  
>  int __init
> @@ -1618,7 +1618,7 @@ static void __init gic_acpi_setup_kvm_info(void)
>  
>  	gic_v2_kvm_info.maint_irq = irq;
>  
> -	gic_set_kvm_info(&gic_v2_kvm_info);
> +	vgic_set_kvm_info(&gic_v2_kvm_info);
>  }
>  
>  static int __init gic_v2_acpi_init(union acpi_subtable_headers *header,
> diff --git a/include/linux/irqchip/arm-gic-common.h b/include/linux/irqchip/arm-gic-common.h
> index fa8c0455c352..1177f3a1aed5 100644
> --- a/include/linux/irqchip/arm-gic-common.h
> +++ b/include/linux/irqchip/arm-gic-common.h
> @@ -7,8 +7,7 @@
>  #ifndef __LINUX_IRQCHIP_ARM_GIC_COMMON_H
>  #define __LINUX_IRQCHIP_ARM_GIC_COMMON_H
>  
> -#include <linux/types.h>
> -#include <linux/ioport.h>
> +#include <linux/irqchip/arm-vgic-info.h>
>  
>  #define GICD_INT_DEF_PRI		0xa0
>  #define GICD_INT_DEF_PRI_X4		((GICD_INT_DEF_PRI << 24) |\
> @@ -16,28 +15,6 @@
>  					(GICD_INT_DEF_PRI << 8) |\
>  					GICD_INT_DEF_PRI)
>  
> -enum gic_type {
> -	GIC_V2,
> -	GIC_V3,
> -};
> -
> -struct gic_kvm_info {
> -	/* GIC type */
> -	enum gic_type	type;
> -	/* Virtual CPU interface */
> -	struct resource vcpu;
> -	/* Interrupt number */
> -	unsigned int	maint_irq;
> -	/* Virtual control interface */
> -	struct resource vctrl;
> -	/* vlpi support */
> -	bool		has_v4;
> -	/* rvpeid support */
> -	bool		has_v4_1;
> -};
> -
> -const struct gic_kvm_info *gic_get_kvm_info(void);
> -
>  struct irq_domain;
>  struct fwnode_handle;
>  int gicv2m_init(struct fwnode_handle *parent_handle,
> diff --git a/include/linux/irqchip/arm-vgic-info.h b/include/linux/irqchip/arm-vgic-info.h
> new file mode 100644
> index 000000000000..0319636be928
> --- /dev/null
> +++ b/include/linux/irqchip/arm-vgic-info.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * include/linux/irqchip/arm-vgic-info.h
> + *
> + * Copyright (C) 2016 ARM Limited, All Rights Reserved.
> + */
> +#ifndef __ARM_VGIC_INFO_H
> +#define __ARM_VGIC_INFO_H

Totally irrelevant nitpick, but the header guards from the other files in this
directory are like __LINUX_IRQCHIP_ARM_VGIC_INFO_H. Regardless, the patch looks
correct to me, the functions are called at the exact moment in the boot flow, only
where the VGIC info is saved is different:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

> +
> +#include <linux/types.h>
> +#include <linux/ioport.h>
> +
> +enum gic_type {
> +	/* Full GICv2 */
> +	GIC_V2,
> +	/* Full GICv3, optionally with v2 compat */
> +	GIC_V3,
> +};
> +
> +struct gic_kvm_info {
> +	/* GIC type */
> +	enum gic_type	type;
> +	/* Virtual CPU interface */
> +	struct resource vcpu;
> +	/* Interrupt number */
> +	unsigned int	maint_irq;
> +	/* Virtual control interface */
> +	struct resource vctrl;
> +	/* vlpi support */
> +	bool		has_v4;
> +	/* rvpeid support */
> +	bool		has_v4_1;
> +};
> +
> +#ifdef CONFIG_KVM
> +void vgic_set_kvm_info(const struct gic_kvm_info *info);
> +#else
> +static inline void vgic_set_kvm_info(const struct gic_kvm_info *info) {}
> +#endif
> +
> +#endif

  reply	other threads:[~2021-05-18 16:50 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-10 13:48 [PATCH v3 0/9] KVM: arm64: Initial host support for the Apple M1 Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 1/9] irqchip/gic: Split vGIC probing information from the GIC code Marc Zyngier
2021-05-18 16:51   ` Alexandru Elisei [this message]
2021-05-10 13:48 ` [PATCH v3 2/9] KVM: arm64: Handle physical FIQ as an IRQ while running a guest Marc Zyngier
2021-05-20 17:46   ` Alexandru Elisei
2021-05-10 13:48 ` [PATCH v3 3/9] KVM: arm64: vgic: Be tolerant to the lack of maintenance interrupt Marc Zyngier
2021-05-10 16:19   ` Mark Rutland
2021-05-10 17:44     ` Marc Zyngier
2021-05-11 11:13       ` Mark Rutland
2021-05-10 13:48 ` [PATCH v3 4/9] KVM: arm64: vgic: Let an interrupt controller advertise lack of HW deactivation Marc Zyngier
2021-05-21 17:01   ` Alexandru Elisei
2021-05-24 17:17     ` Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 5/9] KVM: arm64: vgic: move irq->get_input_level into an ops structure Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 6/9] KVM: arm64: vgic: Implement SW-driven deactivation Marc Zyngier
2021-05-24 16:53   ` Alexandru Elisei
2021-05-24 17:43     ` Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 7/9] KVM: arm64: timer: Refactor IRQ configuration Marc Zyngier
2021-05-14 12:46   ` Zenghui Yu
2021-05-24 17:48     ` Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 8/9] KVM: arm64: timer: Add support for SW-based deactivation Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 9/9] irqchip/apple-aic: Advertise some level of vGICv3 compatibility Marc Zyngier
2021-05-12 16:22 ` [PATCH v3 0/9] KVM: arm64: Initial host support for the Apple M1 Alexandru Elisei
2021-05-12 16:33   ` Marc Zyngier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cca6328e-1710-a7ac-e89e-a7dabe16f81f@arm.com \
    --to=alexandru.elisei@arm.com \
    --cc=eric.auger@redhat.com \
    --cc=james.morse@arm.com \
    --cc=kernel-team@android.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=marcan@marcan.st \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --subject='Re: [PATCH v3 1/9] irqchip/gic: Split vGIC probing information from the GIC code' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).