Re: git: e13c6a6fca71 - main - Teach the GICv3 driver about a vgic child
Date: Wed, 21 Sep 2022 14:10:13 UTC
On 21 Sept 2022, at 11:00, Andrew Turner <andrew@FreeBSD.org> wrote:
>
> The branch main has been updated by andrew:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=e13c6a6fca7177fb9143ce629c2e33b35b6c4219
>
> commit e13c6a6fca7177fb9143ce629c2e33b35b6c4219
> Author: Andrew Turner <andrew@FreeBSD.org>
> AuthorDate: 2022-09-14 16:29:29 +0000
> Commit: Andrew Turner <andrew@FreeBSD.org>
> CommitDate: 2022-09-21 09:59:13 +0000
>
> Teach the GICv3 driver about a vgic child
>
> This will be used by bhyve to attach a virtual GIC driver.
>
> Sponsored by: Innovate UK
> Sponsored by: The FreeBSD Foundation
> Differential Revision: https://reviews.freebsd.org/D36590
> ---
> sys/arm/arm/gic_common.h | 2 ++
> sys/arm64/arm64/gic_v3.c | 7 +++++++
> sys/arm64/arm64/gic_v3_acpi.c | 36 ++++++++++++++++++++++++++++++++++--
> sys/arm64/arm64/gic_v3_fdt.c | 23 ++++++++++++++++++++++-
> sys/arm64/arm64/gic_v3_var.h | 1 +
> 5 files changed, 66 insertions(+), 3 deletions(-)
>
> diff --git a/sys/arm/arm/gic_common.h b/sys/arm/arm/gic_common.h
> index 9e8fb19300ca..42ec44bb7fab 100644
> --- a/sys/arm/arm/gic_common.h
> +++ b/sys/arm/arm/gic_common.h
> @@ -33,6 +33,7 @@
>
> #define GIC_IVAR_HW_REV 500
> #define GIC_IVAR_BUS 501
> +#define GIC_IVAR_VGIC 502
>
> /* GIC_IVAR_BUS values */
> #define GIC_BUS_UNKNOWN 0
> @@ -42,6 +43,7 @@
>
> __BUS_ACCESSOR(gic, hw_rev, GIC, HW_REV, u_int);
> __BUS_ACCESSOR(gic, bus, GIC, BUS, u_int);
> +__BUS_ACCESSOR(gic, vgic, GIC, VGIC, u_int);
>
> /* Software Generated Interrupts */
> #define GIC_FIRST_SGI 0 /* Irqs 0-15 are SGIs/IPIs. */
> diff --git a/sys/arm64/arm64/gic_v3.c b/sys/arm64/arm64/gic_v3.c
> index 27f41c58fe92..b4678d95fee5 100644
> --- a/sys/arm64/arm64/gic_v3.c
> +++ b/sys/arm64/arm64/gic_v3.c
> @@ -456,6 +456,7 @@ static int
> gic_v3_read_ivar(device_t dev, device_t child, int which, uintptr_t *result)
> {
> struct gic_v3_softc *sc;
> + struct gic_v3_devinfo *di;
>
> sc = device_get_softc(dev);
>
> @@ -481,6 +482,12 @@ gic_v3_read_ivar(device_t dev, device_t child, int which, uintptr_t *result)
> ("gic_v3_read_ivar: Invalid bus type %u", sc->gic_bus));
> *result = sc->gic_bus;
> return (0);
> + case GIC_IVAR_VGIC:
> + di = device_get_ivars(child);
> + if (di == NULL)
> + return (EINVAL);
> + *result = di->is_vgic;
> + return (0);
> }
>
> return (ENOENT);
> diff --git a/sys/arm64/arm64/gic_v3_acpi.c b/sys/arm64/arm64/gic_v3_acpi.c
> index cbe4d5989ec2..cc24b7824ff1 100644
> --- a/sys/arm64/arm64/gic_v3_acpi.c
> +++ b/sys/arm64/arm64/gic_v3_acpi.c
> @@ -48,6 +48,9 @@ __FBSDID("$FreeBSD$");
> #include "gic_v3_reg.h"
> #include "gic_v3_var.h"
>
> +#define GICV3_PRIV_VGIC 0x80000000
> +#define GICV3_PRIV_FLAGS 0x80000000
> +
> struct gic_v3_acpi_devinfo {
> struct gic_v3_devinfo di_gic_dinfo;
> struct resource_list di_rl;
> @@ -86,6 +89,7 @@ struct madt_table_data {
> ACPI_MADT_GENERIC_DISTRIBUTOR *dist;
> int count;
> bool rdist_use_gicc;
> + bool have_vgic;
> };
>
> static void
> @@ -152,6 +156,8 @@ rdist_map(ACPI_SUBTABLE_HEADER *entry, void *arg)
> BUS_SET_RESOURCE(madt_data->parent, madt_data->dev,
> SYS_RES_MEMORY, madt_data->count, intr->GicrBaseAddress,
> count);
> + if (intr->VgicInterrupt == 0)
> + madt_data->have_vgic = false;
>
> default:
> break;
> @@ -164,6 +170,7 @@ gic_v3_acpi_identify(driver_t *driver, device_t parent)
> struct madt_table_data madt_data;
> ACPI_TABLE_MADT *madt;
> vm_paddr_t physaddr;
> + uintptr_t private;
> device_t dev;
>
> physaddr = acpi_find_table(ACPI_SIG_MADT);
> @@ -210,6 +217,7 @@ gic_v3_acpi_identify(driver_t *driver, device_t parent)
>
> madt_data.dev = dev;
> madt_data.rdist_use_gicc = false;
> + madt_data.have_vgic = true;
> acpi_walk_subtables(madt + 1, (char *)madt + madt->Header.Length,
> rdist_map, &madt_data);
> if (madt_data.count == 0) {
> @@ -222,7 +230,12 @@ gic_v3_acpi_identify(driver_t *driver, device_t parent)
> rdist_map, &madt_data);
> }
>
> - acpi_set_private(dev, (void *)(uintptr_t)madt_data.dist->Version);
> + private = madt_data.dist->Version;
> + /* Flag that the VGIC is in use */
> + if (madt_data.have_vgic)
> + private |= GICV3_PRIV_VGIC;
> +
> + acpi_set_private(dev, (void *)private);
>
> out:
> acpi_unmap_table(madt);
> @@ -232,7 +245,7 @@ static int
> gic_v3_acpi_probe(device_t dev)
> {
>
> - switch((uintptr_t)acpi_get_private(dev)) {
> + switch((uintptr_t)acpi_get_private(dev) & ~GICV3_PRIV_FLAGS) {
> case ACPI_MADT_GIC_VERSION_V3:
> case ACPI_MADT_GIC_VERSION_V4:
> break;
> @@ -390,9 +403,14 @@ gic_v3_add_children(ACPI_SUBTABLE_HEADER *entry, void *arg)
> static void
> gic_v3_acpi_bus_attach(device_t dev)
> {
> + struct gic_v3_acpi_devinfo *di;
> + struct gic_v3_softc *sc;
> ACPI_TABLE_MADT *madt;
> + device_t child;
> vm_paddr_t physaddr;
>
> + sc = device_get_softc(dev);
> +
> physaddr = acpi_find_table(ACPI_SIG_MADT);
> if (physaddr == 0)
> return;
> @@ -405,6 +423,20 @@ gic_v3_acpi_bus_attach(device_t dev)
>
> acpi_walk_subtables(madt + 1, (char *)madt + madt->Header.Length,
> gic_v3_add_children, dev);
> + /* Add the vgic child if needed */
> + if (((uintptr_t)acpi_get_private(dev) & GICV3_PRIV_FLAGS) != 0) {
> + child = device_add_child(dev, "vgic", -1);
Do we have anything to attach to this, or is this just creating
children that will never do anything?
> + if (child == NULL) {
> + device_printf(dev, "Could not add vgic child\n");
> + } else {
> + di = malloc(sizeof(*di), M_GIC_V3, M_WAITOK | M_ZERO);
> + resource_list_init(&di->di_rl);
> + di->di_gic_dinfo.gic_domain = -1;
> + di->di_gic_dinfo.is_vgic = 1;
> + sc->gic_nchildren++;
> + device_set_ivars(child, di);
> + }
> + }
>
> acpi_unmap_table(madt);
>
> diff --git a/sys/arm64/arm64/gic_v3_fdt.c b/sys/arm64/arm64/gic_v3_fdt.c
> index 0478bdc2962f..0ca362dc4aea 100644
> --- a/sys/arm64/arm64/gic_v3_fdt.c
> +++ b/sys/arm64/arm64/gic_v3_fdt.c
> @@ -212,9 +212,10 @@ static int
> gic_v3_fdt_print_child(device_t bus, device_t child)
> {
> struct gic_v3_ofw_devinfo *di = device_get_ivars(child);
> - struct resource_list *rl = &di->di_rl;
> + struct resource_list *rl;
> int retval = 0;
>
> + rl = &di->di_rl;
> retval += bus_print_child_header(bus, child);
> retval += resource_list_print_type(rl, "mem", SYS_RES_MEMORY, "%#jx");
> retval += bus_print_child_footer(bus, child);
> @@ -228,6 +229,8 @@ gic_v3_ofw_get_devinfo(device_t bus __unused, device_t child)
> struct gic_v3_ofw_devinfo *di;
>
> di = device_get_ivars(child);
> + if (di->di_gic_dinfo.is_vgic)
> + return (NULL);
> return (&di->di_dinfo);
> }
>
> @@ -352,5 +355,23 @@ gic_v3_ofw_bus_attach(device_t dev)
> }
> }
>
> + /*
> + * If there is a vgic maintanance interupt add a virtual gic
s/interupt/interrupt/
Maybe also capitalise (V)GIC.
> + * child so we can use this in the vmm module for bhyve.
> + */
> + if (OF_hasprop(parent, "interrupts")) {
> + child = device_add_child(dev, "vgic", -1);
> + if (child == NULL) {
> + device_printf(dev, "Could not add vgic child\n");
> + } else {
> + di = malloc(sizeof(*di), M_GIC_V3, M_WAITOK | M_ZERO);
> + resource_list_init(&di->di_rl);
> + di->di_gic_dinfo.gic_domain = -1;
> + di->di_gic_dinfo.is_vgic = 1;
> + device_set_ivars(child, di);
> + sc->gic_nchildren++;
These two lines are unnecessarily swapped compared with the ACPI
version.
> + }
> + }
> +
> return (bus_generic_attach(dev));
> }
> diff --git a/sys/arm64/arm64/gic_v3_var.h b/sys/arm64/arm64/gic_v3_var.h
> index 7722a48cb456..b66fe6c57bb2 100644
> --- a/sys/arm64/arm64/gic_v3_var.h
> +++ b/sys/arm64/arm64/gic_v3_var.h
> @@ -89,6 +89,7 @@ struct gic_v3_softc {
> struct gic_v3_devinfo {
> int gic_domain;
> int msi_xref;
> + int is_vgic;
bool?..
Jess
> };
>
> #define GIC_INTR_ISRC(sc, irq) (&sc->gic_irqs[irq].gi_isrc)