git: c6f3076d4405 - main - Move the GICv2m msi handling to the parent
Marcin Wojtas
mw at semihalf.com
Tue Sep 28 00:32:19 UTC 2021
Hi Andrew,
wt., 14 wrz 2021 o 12:42 Andrew Turner <andrew at freebsd.org> napisał(a):
>
> The branch main has been updated by andrew:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=c6f3076d44055f7b02467ce074210f73d0ce0ef6
>
> commit c6f3076d44055f7b02467ce074210f73d0ce0ef6
> Author: Andrew Turner <andrew at FreeBSD.org>
> AuthorDate: 2021-09-01 09:39:01 +0000
> Commit: Andrew Turner <andrew at FreeBSD.org>
> CommitDate: 2021-09-14 07:24:52 +0000
>
> Move the GICv2m msi handling to the parent
>
> This is in preperation for adding support for the GICv2m driver as a
> child of the GICv3 driver.
>
> PR: 258136
> Reported by: trasz
> Sponsored by: The FreeBSD Foundation
> Differential Revision: https://reviews.freebsd.org/D31767
> ---
> sys/arm/arm/gic.c | 296 ++++++++++++++++++++++++++++++-----------------
> sys/arm/arm/gic.h | 8 +-
> sys/arm/arm/gic_common.h | 4 +
> 3 files changed, 195 insertions(+), 113 deletions(-)
>
> diff --git a/sys/arm/arm/gic.c b/sys/arm/arm/gic.c
> index 1851e69644ed..bd34e92b9e28 100644
> --- a/sys/arm/arm/gic.c
> +++ b/sys/arm/arm/gic.c
> @@ -501,6 +501,56 @@ arm_gic_read_ivar(device_t dev, device_t child, int which, uintptr_t *result)
> ("arm_gic_read_ivar: Invalid bus type %u", sc->gic_bus));
> *result = sc->gic_bus;
> return (0);
> + case GIC_IVAR_MBI_START:
> + *result = sc->sc_spi_start;
> + return (0);
> + case GIC_IVAR_MBI_COUNT:
> + *result = sc->sc_spi_count;
> + return (0);
> + }
> +
> + return (ENOENT);
> +}
> +
> +static int
> +arm_gic_write_ivar(device_t dev, device_t child, int which, uintptr_t value)
> +{
> + struct arm_gic_softc *sc;
> +
> + sc = device_get_softc(dev);
> +
> + switch(which) {
> + case GIC_IVAR_HW_REV:
> + case GIC_IVAR_BUS:
> + return (EINVAL);
> + case GIC_IVAR_MBI_START:
> + /*
> + * GIC_IVAR_MBI_START must be set once and first. This allows
> + * us to reserve the registers when GIC_IVAR_MBI_COUNT is set.
> + */
> + MPASS(sc->sc_spi_start == 0);
> + MPASS(sc->sc_spi_count == 0);
This patch breaks GICv2m on all Marvell Armada 7k8k and CN913x. After
reverting it works as expected. I tried removing the above 2 asserts +
the one in line 540 - instead of init it fails later, during PCIE
endpoint msix configuration:
em0: Using 1024 TX descriptors and 1024 RX descriptors
em0: Using 2 RX queues 2 TX queues
panic: mtx_lock() of spin mutex GIC lock @
/home/mw/current/sys/arm/arm/gic.c:1145
cpuid = 0
time = 1
KDB: stack backtrace:
db_trace_self() at db_trace_self
db_trace_self_wrapper() at db_trace_self_wrapper+0x30
vpanic() at vpanic+0x184
panic() at panic+0x44
__mtx_lock_flags() at __mtx_lock_flags+0x1a8
arm_gic_alloc_msix() at arm_gic_alloc_msix+0x40
intr_alloc_msix() at intr_alloc_msix+0x190
generic_pcie_acpi_alloc_msix() at generic_pcie_acpi_alloc_msix+0x78
pci_alloc_msix_method() at pci_alloc_msix_method+0x1a8
iflib_device_register() at iflib_device_register+0xae4
iflib_device_attach() at iflib_device_attach+0xd0
device_attach() at device_attach+0x410
device_probe_and_attach() at device_probe_and_attach+0x7c
bus_generic_attach() at bus_generic_attach+0x18
pci_attach() at pci_attach+0xe8
device_attach() at device_attach+0x410
device_probe_and_attach() at device_probe_and_attach+0x7c
bus_generic_attach() at bus_generic_attach+0x18
device_attach() at device_attach+0x410
device_probe_and_attach() at device_probe_and_attach+0x7c
bus_generic_new_pass() at bus_generic_new_pass+0xec
bus_generic_new_pass() at bus_generic_new_pass+0xd0
bus_generic_new_pass() at bus_generic_new_pass+0xd0
bus_set_pass() at bus_set_pass+0x8c
mi_startup() at mi_startup+0x12c
virtdone() at virtdone+0x6c
KDB: enter: panic
[ thread pid 0 tid 100000 ]
Stopped at kdb_enter+0x44: undefined f900c11f
Any ideas?
Best regards,
Marcin
> + MPASS(value >= GIC_FIRST_SPI);
> + MPASS(value < sc->nirqs);
> +
> + sc->sc_spi_start = value;
> + return (0);
> + case GIC_IVAR_MBI_COUNT:
> + MPASS(sc->sc_spi_start != 0);
> + MPASS(sc->sc_spi_count == 0);
> + MPASS(value >= sc->sc_spi_start);
> + MPASS(value >= GIC_FIRST_SPI);
> +
> + sc->sc_spi_count = value;
> + sc->sc_spi_end = sc->sc_spi_start + sc->sc_spi_count;
> +
> + MPASS(sc->sc_spi_end <= sc->nirqs);
> +
> + /* Reserve these interrupts for MSI/MSI-X use */
> + arm_gic_reserve_msi_range(dev, sc->sc_spi_start,
> + sc->sc_spi_count);
> +
> + return (0);
> }
>
> return (ENOENT);
> @@ -995,99 +1045,20 @@ arm_gic_ipi_setup(device_t dev, u_int ipi, struct intr_irqsrc **isrcp)
> }
> #endif
>
> -static device_method_t arm_gic_methods[] = {
> - /* Bus interface */
> - DEVMETHOD(bus_print_child, arm_gic_print_child),
> - DEVMETHOD(bus_add_child, bus_generic_add_child),
> - DEVMETHOD(bus_alloc_resource, arm_gic_alloc_resource),
> - DEVMETHOD(bus_release_resource, bus_generic_release_resource),
> - DEVMETHOD(bus_activate_resource,bus_generic_activate_resource),
> - DEVMETHOD(bus_read_ivar, arm_gic_read_ivar),
> -
> - /* Interrupt controller interface */
> - DEVMETHOD(pic_disable_intr, arm_gic_disable_intr),
> - DEVMETHOD(pic_enable_intr, arm_gic_enable_intr),
> - DEVMETHOD(pic_map_intr, arm_gic_map_intr),
> - DEVMETHOD(pic_setup_intr, arm_gic_setup_intr),
> - DEVMETHOD(pic_teardown_intr, arm_gic_teardown_intr),
> - DEVMETHOD(pic_post_filter, arm_gic_post_filter),
> - DEVMETHOD(pic_post_ithread, arm_gic_post_ithread),
> - DEVMETHOD(pic_pre_ithread, arm_gic_pre_ithread),
> -#ifdef SMP
> - DEVMETHOD(pic_bind_intr, arm_gic_bind_intr),
> - DEVMETHOD(pic_init_secondary, arm_gic_init_secondary),
> - DEVMETHOD(pic_ipi_send, arm_gic_ipi_send),
> - DEVMETHOD(pic_ipi_setup, arm_gic_ipi_setup),
> -#endif
> - { 0, 0 }
> -};
> -
> -DEFINE_CLASS_0(gic, arm_gic_driver, arm_gic_methods,
> - sizeof(struct arm_gic_softc));
> -
> -/*
> - * GICv2m support -- the GICv2 MSI/MSI-X controller.
> - */
> -
> -#define GICV2M_MSI_TYPER 0x008
> -#define MSI_TYPER_SPI_BASE(x) (((x) >> 16) & 0x3ff)
> -#define MSI_TYPER_SPI_COUNT(x) (((x) >> 0) & 0x3ff)
> -#define GICv2M_MSI_SETSPI_NS 0x040
> -#define GICV2M_MSI_IIDR 0xFCC
> -
> -int
> -arm_gicv2m_attach(device_t dev)
> -{
> - struct arm_gicv2m_softc *sc;
> - uint32_t typer;
> - int rid;
> -
> - sc = device_get_softc(dev);
> -
> - rid = 0;
> - sc->sc_mem = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid,
> - RF_ACTIVE);
> - if (sc->sc_mem == NULL) {
> - device_printf(dev, "Unable to allocate resources\n");
> - return (ENXIO);
> - }
> -
> - typer = bus_read_4(sc->sc_mem, GICV2M_MSI_TYPER);
> - sc->sc_spi_start = MSI_TYPER_SPI_BASE(typer);
> - sc->sc_spi_count = MSI_TYPER_SPI_COUNT(typer);
> - sc->sc_spi_end = sc->sc_spi_start + sc->sc_spi_count;
> -
> - /* Reserve these interrupts for MSI/MSI-X use */
> - arm_gic_reserve_msi_range(device_get_parent(dev), sc->sc_spi_start,
> - sc->sc_spi_count);
> -
> - mtx_init(&sc->sc_mutex, "GICv2m lock", NULL, MTX_DEF);
> -
> - intr_msi_register(dev, sc->sc_xref);
> -
> - if (bootverbose)
> - device_printf(dev, "using spi %u to %u\n", sc->sc_spi_start,
> - sc->sc_spi_start + sc->sc_spi_count - 1);
> -
> - return (0);
> -}
> -
> static int
> -arm_gicv2m_alloc_msi(device_t dev, device_t child, int count, int maxcount,
> +arm_gic_alloc_msi(device_t dev, device_t child, int count, int maxcount,
> device_t *pic, struct intr_irqsrc **srcs)
> {
> - struct arm_gic_softc *psc;
> - struct arm_gicv2m_softc *sc;
> + struct arm_gic_softc *sc;
> int i, irq, end_irq;
> bool found;
>
> KASSERT(powerof2(count), ("%s: bad count", __func__));
> KASSERT(powerof2(maxcount), ("%s: bad maxcount", __func__));
>
> - psc = device_get_softc(device_get_parent(dev));
> sc = device_get_softc(dev);
>
> - mtx_lock(&sc->sc_mutex);
> + mtx_lock(&sc->mutex);
>
> found = false;
> for (irq = sc->sc_spi_start; irq < sc->sc_spi_end; irq++) {
> @@ -1106,11 +1077,11 @@ arm_gicv2m_alloc_msi(device_t dev, device_t child, int count, int maxcount,
> break;
> }
>
> - KASSERT((psc->gic_irqs[end_irq].gi_flags & GI_FLAG_MSI)!= 0,
> + KASSERT((sc->gic_irqs[end_irq].gi_flags & GI_FLAG_MSI)!= 0,
> ("%s: Non-MSI interrupt found", __func__));
>
> /* This is already used */
> - if ((psc->gic_irqs[end_irq].gi_flags & GI_FLAG_MSI_USED) ==
> + if ((sc->gic_irqs[end_irq].gi_flags & GI_FLAG_MSI_USED) ==
> GI_FLAG_MSI_USED) {
> found = false;
> break;
> @@ -1122,34 +1093,34 @@ arm_gicv2m_alloc_msi(device_t dev, device_t child, int count, int maxcount,
>
> /* Not enough interrupts were found */
> if (!found || irq == sc->sc_spi_end) {
> - mtx_unlock(&sc->sc_mutex);
> + mtx_unlock(&sc->mutex);
> return (ENXIO);
> }
>
> for (i = 0; i < count; i++) {
> /* Mark the interrupt as used */
> - psc->gic_irqs[irq + i].gi_flags |= GI_FLAG_MSI_USED;
> + sc->gic_irqs[irq + i].gi_flags |= GI_FLAG_MSI_USED;
> }
> - mtx_unlock(&sc->sc_mutex);
> + mtx_unlock(&sc->mutex);
>
> for (i = 0; i < count; i++)
> - srcs[i] = (struct intr_irqsrc *)&psc->gic_irqs[irq + i];
> - *pic = device_get_parent(dev);
> + srcs[i] = (struct intr_irqsrc *)&sc->gic_irqs[irq + i];
> + *pic = dev;
>
> return (0);
> }
>
> static int
> -arm_gicv2m_release_msi(device_t dev, device_t child, int count,
> +arm_gic_release_msi(device_t dev, device_t child, int count,
> struct intr_irqsrc **isrc)
> {
> - struct arm_gicv2m_softc *sc;
> + struct arm_gic_softc *sc;
> struct gic_irqsrc *gi;
> int i;
>
> sc = device_get_softc(dev);
>
> - mtx_lock(&sc->sc_mutex);
> + mtx_lock(&sc->mutex);
> for (i = 0; i < count; i++) {
> gi = (struct gic_irqsrc *)isrc[i];
>
> @@ -1159,50 +1130,48 @@ arm_gicv2m_release_msi(device_t dev, device_t child, int count,
>
> gi->gi_flags &= ~GI_FLAG_MSI_USED;
> }
> - mtx_unlock(&sc->sc_mutex);
> + mtx_unlock(&sc->mutex);
>
> return (0);
> }
>
> static int
> -arm_gicv2m_alloc_msix(device_t dev, device_t child, device_t *pic,
> +arm_gic_alloc_msix(device_t dev, device_t child, device_t *pic,
> struct intr_irqsrc **isrcp)
> {
> - struct arm_gicv2m_softc *sc;
> - struct arm_gic_softc *psc;
> + struct arm_gic_softc *sc;
> int irq;
>
> - psc = device_get_softc(device_get_parent(dev));
> sc = device_get_softc(dev);
>
> - mtx_lock(&sc->sc_mutex);
> + mtx_lock(&sc->mutex);
> /* Find an unused interrupt */
> for (irq = sc->sc_spi_start; irq < sc->sc_spi_end; irq++) {
> - KASSERT((psc->gic_irqs[irq].gi_flags & GI_FLAG_MSI) != 0,
> + KASSERT((sc->gic_irqs[irq].gi_flags & GI_FLAG_MSI) != 0,
> ("%s: Non-MSI interrupt found", __func__));
> - if ((psc->gic_irqs[irq].gi_flags & GI_FLAG_MSI_USED) == 0)
> + if ((sc->gic_irqs[irq].gi_flags & GI_FLAG_MSI_USED) == 0)
> break;
> }
> /* No free interrupt was found */
> if (irq == sc->sc_spi_end) {
> - mtx_unlock(&sc->sc_mutex);
> + mtx_unlock(&sc->mutex);
> return (ENXIO);
> }
>
> /* Mark the interrupt as used */
> - psc->gic_irqs[irq].gi_flags |= GI_FLAG_MSI_USED;
> - mtx_unlock(&sc->sc_mutex);
> + sc->gic_irqs[irq].gi_flags |= GI_FLAG_MSI_USED;
> + mtx_unlock(&sc->mutex);
>
> - *isrcp = (struct intr_irqsrc *)&psc->gic_irqs[irq];
> - *pic = device_get_parent(dev);
> + *isrcp = (struct intr_irqsrc *)&sc->gic_irqs[irq];
> + *pic = dev;
>
> return (0);
> }
>
> static int
> -arm_gicv2m_release_msix(device_t dev, device_t child, struct intr_irqsrc *isrc)
> +arm_gic_release_msix(device_t dev, device_t child, struct intr_irqsrc *isrc)
> {
> - struct arm_gicv2m_softc *sc;
> + struct arm_gic_softc *sc;
> struct gic_irqsrc *gi;
>
> sc = device_get_softc(dev);
> @@ -1211,13 +1180,122 @@ arm_gicv2m_release_msix(device_t dev, device_t child, struct intr_irqsrc *isrc)
> KASSERT((gi->gi_flags & GI_FLAG_MSI_USED) == GI_FLAG_MSI_USED,
> ("%s: Trying to release an unused MSI-X interrupt", __func__));
>
> - mtx_lock(&sc->sc_mutex);
> + mtx_lock(&sc->mutex);
> gi->gi_flags &= ~GI_FLAG_MSI_USED;
> - mtx_unlock(&sc->sc_mutex);
> + mtx_unlock(&sc->mutex);
>
> return (0);
> }
>
> +static device_method_t arm_gic_methods[] = {
> + /* Bus interface */
> + DEVMETHOD(bus_print_child, arm_gic_print_child),
> + DEVMETHOD(bus_add_child, bus_generic_add_child),
> + DEVMETHOD(bus_alloc_resource, arm_gic_alloc_resource),
> + DEVMETHOD(bus_release_resource, bus_generic_release_resource),
> + DEVMETHOD(bus_activate_resource,bus_generic_activate_resource),
> + DEVMETHOD(bus_read_ivar, arm_gic_read_ivar),
> + DEVMETHOD(bus_write_ivar, arm_gic_write_ivar),
> +
> + /* Interrupt controller interface */
> + DEVMETHOD(pic_disable_intr, arm_gic_disable_intr),
> + DEVMETHOD(pic_enable_intr, arm_gic_enable_intr),
> + DEVMETHOD(pic_map_intr, arm_gic_map_intr),
> + DEVMETHOD(pic_setup_intr, arm_gic_setup_intr),
> + DEVMETHOD(pic_teardown_intr, arm_gic_teardown_intr),
> + DEVMETHOD(pic_post_filter, arm_gic_post_filter),
> + DEVMETHOD(pic_post_ithread, arm_gic_post_ithread),
> + DEVMETHOD(pic_pre_ithread, arm_gic_pre_ithread),
> +#ifdef SMP
> + DEVMETHOD(pic_bind_intr, arm_gic_bind_intr),
> + DEVMETHOD(pic_init_secondary, arm_gic_init_secondary),
> + DEVMETHOD(pic_ipi_send, arm_gic_ipi_send),
> + DEVMETHOD(pic_ipi_setup, arm_gic_ipi_setup),
> +#endif
> +
> + /* MSI/MSI-X */
> + DEVMETHOD(msi_alloc_msi, arm_gic_alloc_msi),
> + DEVMETHOD(msi_release_msi, arm_gic_release_msi),
> + DEVMETHOD(msi_alloc_msix, arm_gic_alloc_msix),
> + DEVMETHOD(msi_release_msix, arm_gic_release_msix),
> +
> + { 0, 0 }
> +};
> +
> +DEFINE_CLASS_0(gic, arm_gic_driver, arm_gic_methods,
> + sizeof(struct arm_gic_softc));
> +
> +/*
> + * GICv2m support -- the GICv2 MSI/MSI-X controller.
> + */
> +
> +#define GICV2M_MSI_TYPER 0x008
> +#define MSI_TYPER_SPI_BASE(x) (((x) >> 16) & 0x3ff)
> +#define MSI_TYPER_SPI_COUNT(x) (((x) >> 0) & 0x3ff)
> +#define GICv2M_MSI_SETSPI_NS 0x040
> +#define GICV2M_MSI_IIDR 0xFCC
> +
> +int
> +arm_gicv2m_attach(device_t dev)
> +{
> + struct arm_gicv2m_softc *sc;
> + uint32_t typer;
> + u_int spi_start, spi_count;
> + int rid;
> +
> + sc = device_get_softc(dev);
> +
> + rid = 0;
> + sc->sc_mem = bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid,
> + RF_ACTIVE);
> + if (sc->sc_mem == NULL) {
> + device_printf(dev, "Unable to allocate resources\n");
> + return (ENXIO);
> + }
> +
> + typer = bus_read_4(sc->sc_mem, GICV2M_MSI_TYPER);
> + spi_start = MSI_TYPER_SPI_BASE(typer);
> + spi_count = MSI_TYPER_SPI_COUNT(typer);
> + gic_set_mbi_start(dev, spi_start);
> + gic_set_mbi_count(dev, spi_count);
> +
> + intr_msi_register(dev, sc->sc_xref);
> +
> + if (bootverbose)
> + device_printf(dev, "using spi %u to %u\n", spi_start,
> + spi_start + spi_count - 1);
> +
> + return (0);
> +}
> +
> +static int
> +arm_gicv2m_alloc_msi(device_t dev, device_t child, int count, int maxcount,
> + device_t *pic, struct intr_irqsrc **srcs)
> +{
> + return (MSI_ALLOC_MSI(device_get_parent(dev), child, count, maxcount,
> + pic, srcs));
> +}
> +
> +static int
> +arm_gicv2m_release_msi(device_t dev, device_t child, int count,
> + struct intr_irqsrc **isrc)
> +{
> + return (MSI_RELEASE_MSI(device_get_parent(dev), child, count, isrc));
> +}
> +
> +static int
> +arm_gicv2m_alloc_msix(device_t dev, device_t child, device_t *pic,
> + struct intr_irqsrc **isrcp)
> +{
> + return (MSI_ALLOC_MSIX(device_get_parent(dev), child, pic, isrcp));
> +}
> +
> +static int
> +arm_gicv2m_release_msix(device_t dev, device_t child, struct intr_irqsrc *isrc)
> +{
> + return (MSI_RELEASE_MSIX(device_get_parent(dev), child, isrc));
> +}
> +
> static int
> arm_gicv2m_map_msi(device_t dev, device_t child, struct intr_irqsrc *isrc,
> uint64_t *addr, uint32_t *data)
> diff --git a/sys/arm/arm/gic.h b/sys/arm/arm/gic.h
> index 74cfbbee9d5a..55f7f0cc5e44 100644
> --- a/sys/arm/arm/gic.h
> +++ b/sys/arm/arm/gic.h
> @@ -63,17 +63,17 @@ struct arm_gic_softc {
>
> int nranges;
> struct arm_gic_range * ranges;
> +
> + u_int sc_spi_start;
> + u_int sc_spi_end;
> + u_int sc_spi_count;
> };
>
> DECLARE_CLASS(arm_gic_driver);
>
> struct arm_gicv2m_softc {
> struct resource *sc_mem;
> - struct mtx sc_mutex;
> uintptr_t sc_xref;
> - u_int sc_spi_start;
> - u_int sc_spi_end;
> - u_int sc_spi_count;
> };
>
> DECLARE_CLASS(arm_gicv2m_driver);
> diff --git a/sys/arm/arm/gic_common.h b/sys/arm/arm/gic_common.h
> index 9e8fb19300ca..52827d03e600 100644
> --- a/sys/arm/arm/gic_common.h
> +++ b/sys/arm/arm/gic_common.h
> @@ -33,6 +33,8 @@
>
> #define GIC_IVAR_HW_REV 500
> #define GIC_IVAR_BUS 501
> +#define GIC_IVAR_MBI_START 510
> +#define GIC_IVAR_MBI_COUNT 511
>
> /* GIC_IVAR_BUS values */
> #define GIC_BUS_UNKNOWN 0
> @@ -42,6 +44,8 @@
>
> __BUS_ACCESSOR(gic, hw_rev, GIC, HW_REV, u_int);
> __BUS_ACCESSOR(gic, bus, GIC, BUS, u_int);
> +__BUS_ACCESSOR(gic, mbi_start, GIC, MBI_START, u_int);
> +__BUS_ACCESSOR(gic, mbi_count, GIC, MBI_COUNT, u_int);
>
> /* Software Generated Interrupts */
> #define GIC_FIRST_SGI 0 /* Irqs 0-15 are SGIs/IPIs. */
More information about the dev-commits-src-all
mailing list