Date: Tue, 28 Sep 2021 02:32:07 +0200 From: Marcin Wojtas <mw@semihalf.com> To: Andrew Turner <andrew@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: c6f3076d4405 - main - Move the GICv2m msi handling to the parent Message-ID: <CAPv3WKfUCpsxNySo13B0m%2BEtwmXYUEpoSaUiFORtwfySdhsh9g@mail.gmail.com> In-Reply-To: <202109141042.18EAgYCI041614@gitrepo.freebsd.org> References: <202109141042.18EAgYCI041614@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Andrew, wt., 14 wrz 2021 o 12:42 Andrew Turner <andrew@freebsd.org> napisa=C5=82(a)= : > > The branch main has been updated by andrew: > > URL: https://cgit.FreeBSD.org/src/commit/?id=3Dc6f3076d44055f7b02467ce074= 210f73d0ce0ef6 > > commit c6f3076d44055f7b02467ce074210f73d0ce0ef6 > Author: Andrew Turner <andrew@FreeBSD.org> > AuthorDate: 2021-09-01 09:39:01 +0000 > Commit: Andrew Turner <andrew@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_bu= s)); > *result =3D sc->gic_bus; > return (0); > + case GIC_IVAR_MBI_START: > + *result =3D sc->sc_spi_start; > + return (0); > + case GIC_IVAR_MBI_COUNT: > + *result =3D sc->sc_spi_count; > + return (0); > + } > + > + return (ENOENT); > +} > + > +static int > +arm_gic_write_ivar(device_t dev, device_t child, int which, uintptr_t va= lue) > +{ > + struct arm_gic_softc *sc; > + > + sc =3D 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 al= lows > + * us to reserve the registers when GIC_IVAR_MBI_COUNT is= set. > + */ > + MPASS(sc->sc_spi_start =3D=3D 0); > + MPASS(sc->sc_spi_count =3D=3D 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 =3D 0 time =3D 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 >=3D GIC_FIRST_SPI); > + MPASS(value < sc->nirqs); > + > + sc->sc_spi_start =3D value; > + return (0); > + case GIC_IVAR_MBI_COUNT: > + MPASS(sc->sc_spi_start !=3D 0); > + MPASS(sc->sc_spi_count =3D=3D 0); > + MPASS(value >=3D sc->sc_spi_start); > + MPASS(value >=3D GIC_FIRST_SPI); > + > + sc->sc_spi_count =3D value; > + sc->sc_spi_end =3D sc->sc_spi_start + sc->sc_spi_count; > + > + MPASS(sc->sc_spi_end <=3D 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[] =3D { > - /* 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 =3D device_get_softc(dev); > - > - rid =3D 0; > - sc->sc_mem =3D bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid, > - RF_ACTIVE); > - if (sc->sc_mem =3D=3D NULL) { > - device_printf(dev, "Unable to allocate resources\n"); > - return (ENXIO); > - } > - > - typer =3D bus_read_4(sc->sc_mem, GICV2M_MSI_TYPER); > - sc->sc_spi_start =3D MSI_TYPER_SPI_BASE(typer); > - sc->sc_spi_count =3D MSI_TYPER_SPI_COUNT(typer); > - sc->sc_spi_end =3D 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_star= t, > - 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_sta= rt, > - 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 maxcou= nt, > +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 =3D device_get_softc(device_get_parent(dev)); > sc =3D device_get_softc(dev); > > - mtx_lock(&sc->sc_mutex); > + mtx_lock(&sc->mutex); > > found =3D false; > for (irq =3D 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_FLA= G_MSI)!=3D 0, > + KASSERT((sc->gic_irqs[end_irq].gi_flags & GI_FLAG= _MSI)!=3D 0, > ("%s: Non-MSI interrupt found", __func__)); > > /* This is already used */ > - if ((psc->gic_irqs[end_irq].gi_flags & GI_FLAG_MS= I_USED) =3D=3D > + if ((sc->gic_irqs[end_irq].gi_flags & GI_FLAG_MSI= _USED) =3D=3D > GI_FLAG_MSI_USED) { > found =3D 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 =3D=3D sc->sc_spi_end) { > - mtx_unlock(&sc->sc_mutex); > + mtx_unlock(&sc->mutex); > return (ENXIO); > } > > for (i =3D 0; i < count; i++) { > /* Mark the interrupt as used */ > - psc->gic_irqs[irq + i].gi_flags |=3D GI_FLAG_MSI_USED; > + sc->gic_irqs[irq + i].gi_flags |=3D GI_FLAG_MSI_USED; > } > - mtx_unlock(&sc->sc_mutex); > + mtx_unlock(&sc->mutex); > > for (i =3D 0; i < count; i++) > - srcs[i] =3D (struct intr_irqsrc *)&psc->gic_irqs[irq + i]= ; > - *pic =3D device_get_parent(dev); > + srcs[i] =3D (struct intr_irqsrc *)&sc->gic_irqs[irq + i]; > + *pic =3D 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 =3D device_get_softc(dev); > > - mtx_lock(&sc->sc_mutex); > + mtx_lock(&sc->mutex); > for (i =3D 0; i < count; i++) { > gi =3D (struct gic_irqsrc *)isrc[i]; > > @@ -1159,50 +1130,48 @@ arm_gicv2m_release_msi(device_t dev, device_t chi= ld, int count, > > gi->gi_flags &=3D ~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 =3D device_get_softc(device_get_parent(dev)); > sc =3D device_get_softc(dev); > > - mtx_lock(&sc->sc_mutex); > + mtx_lock(&sc->mutex); > /* Find an unused interrupt */ > for (irq =3D sc->sc_spi_start; irq < sc->sc_spi_end; irq++) { > - KASSERT((psc->gic_irqs[irq].gi_flags & GI_FLAG_MSI) !=3D = 0, > + KASSERT((sc->gic_irqs[irq].gi_flags & GI_FLAG_MSI) !=3D 0= , > ("%s: Non-MSI interrupt found", __func__)); > - if ((psc->gic_irqs[irq].gi_flags & GI_FLAG_MSI_USED) =3D= =3D 0) > + if ((sc->gic_irqs[irq].gi_flags & GI_FLAG_MSI_USED) =3D= =3D 0) > break; > } > /* No free interrupt was found */ > if (irq =3D=3D 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 |=3D GI_FLAG_MSI_USED; > - mtx_unlock(&sc->sc_mutex); > + sc->gic_irqs[irq].gi_flags |=3D GI_FLAG_MSI_USED; > + mtx_unlock(&sc->mutex); > > - *isrcp =3D (struct intr_irqsrc *)&psc->gic_irqs[irq]; > - *pic =3D device_get_parent(dev); > + *isrcp =3D (struct intr_irqsrc *)&sc->gic_irqs[irq]; > + *pic =3D 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 *i= src) > { > - struct arm_gicv2m_softc *sc; > + struct arm_gic_softc *sc; > struct gic_irqsrc *gi; > > sc =3D device_get_softc(dev); > @@ -1211,13 +1180,122 @@ arm_gicv2m_release_msix(device_t dev, device_t c= hild, struct intr_irqsrc *isrc) > KASSERT((gi->gi_flags & GI_FLAG_MSI_USED) =3D=3D 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 &=3D ~GI_FLAG_MSI_USED; > - mtx_unlock(&sc->sc_mutex); > + mtx_unlock(&sc->mutex); > > return (0); > } > > +static device_method_t arm_gic_methods[] =3D { > + /* 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 =3D device_get_softc(dev); > + > + rid =3D 0; > + sc->sc_mem =3D bus_alloc_resource_any(dev, SYS_RES_MEMORY, &rid, > + RF_ACTIVE); > + if (sc->sc_mem =3D=3D NULL) { > + device_printf(dev, "Unable to allocate resources\n"); > + return (ENXIO); > + } > + > + typer =3D bus_read_4(sc->sc_mem, GICV2M_MSI_TYPER); > + spi_start =3D MSI_TYPER_SPI_BASE(typer); > + spi_count =3D 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 maxcou= nt, > + device_t *pic, struct intr_irqsrc **srcs) > +{ > + return (MSI_ALLOC_MSI(device_get_parent(dev), child, count, maxco= unt, > + 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, isr= c)); > +} > + > +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 *isr= c, > 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/IPI= s. */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPv3WKfUCpsxNySo13B0m%2BEtwmXYUEpoSaUiFORtwfySdhsh9g>