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@xxxxxxxxxx>
> ---
> 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@xxxxxxx>
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
