git: f3aa0098a82e - main - Use mtx_lock_spin in the gic driver
Marcin Wojtas
mw at semihalf.com
Wed Sep 29 23:26:30 UTC 2021
śr., 29 wrz 2021 o 17:42 Andrew Turner <andrew at fubar.geek.nz> napisał(a):
>
>
>
> > On 28 Sep 2021, at 16:36, Marcin Wojtas <mw at semihalf.com> wrote:
> >
> > Hi Andrew,
> >
> > wt., 28 wrz 2021 o 14:43 Andrew Turner <andrew at freebsd.org> napisał(a):
> >>
> >> The branch main has been updated by andrew:
> >>
> >> URL: https://cgit.FreeBSD.org/src/commit/?id=f3aa0098a82ebf7712aa13716d794aa7e4ac59cd
> >>
> >> commit f3aa0098a82ebf7712aa13716d794aa7e4ac59cd
> >> Author: Andrew Turner <andrew at FreeBSD.org>
> >> AuthorDate: 2021-09-28 11:36:42 +0000
> >> Commit: Andrew Turner <andrew at FreeBSD.org>
> >> CommitDate: 2021-09-28 11:42:06 +0000
> >>
> >> Use mtx_lock_spin in the gic driver
> >>
> >> The mutex was changed to a spin lock when the MSI/MSI-X handling was
> >> moved from the gicv2m to the gic driver. Update the calls to lock
> >> and unlock the mutex to the spin variant.
> >>
> >> Submitted by: jrtc27 ("Change all the mtx_(un)lock(&sc->mutex) to be the _spin versions.")
> >> Reported by: mw, antranigv at freebsd.am
> >> Sponsored by: The FreeBSD Foundation
> >> ---
> >> sys/arm/arm/gic.c | 20 ++++++++++----------
> >> 1 file changed, 10 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/sys/arm/arm/gic.c b/sys/arm/arm/gic.c
> >> index d7edd7885404..89db4e324600 100644
> >> --- a/sys/arm/arm/gic.c
> >> +++ b/sys/arm/arm/gic.c
> >> @@ -1056,7 +1056,7 @@ arm_gic_alloc_msi(device_t dev, device_t child, int count, int maxcount,
> >>
> >> sc = device_get_softc(dev);
> >>
> >> - mtx_lock(&sc->mutex);
> >> + mtx_lock_spin(&sc->mutex);
> >>
> >> found = false;
> >> for (irq = sc->sc_spi_start; irq < sc->sc_spi_end; irq++) {
> >> @@ -1091,7 +1091,7 @@ arm_gic_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->mutex);
> >> + mtx_unlock_spin(&sc->mutex);
> >> return (ENXIO);
> >> }
> >>
> >> @@ -1099,7 +1099,7 @@ arm_gic_alloc_msi(device_t dev, device_t child, int count, int maxcount,
> >> /* Mark the interrupt as used */
> >> sc->gic_irqs[irq + i].gi_flags |= GI_FLAG_MSI_USED;
> >> }
> >> - mtx_unlock(&sc->mutex);
> >> + mtx_unlock_spin(&sc->mutex);
> >>
> >> for (i = 0; i < count; i++)
> >> srcs[i] = (struct intr_irqsrc *)&sc->gic_irqs[irq + i];
> >> @@ -1118,7 +1118,7 @@ arm_gic_release_msi(device_t dev, device_t child, int count,
> >>
> >> sc = device_get_softc(dev);
> >>
> >> - mtx_lock(&sc->mutex);
> >> + mtx_lock_spin(&sc->mutex);
> >> for (i = 0; i < count; i++) {
> >> gi = (struct gic_irqsrc *)isrc[i];
> >>
> >> @@ -1128,7 +1128,7 @@ arm_gic_release_msi(device_t dev, device_t child, int count,
> >>
> >> gi->gi_flags &= ~GI_FLAG_MSI_USED;
> >> }
> >> - mtx_unlock(&sc->mutex);
> >> + mtx_unlock_spin(&sc->mutex);
> >>
> >> return (0);
> >> }
> >> @@ -1142,7 +1142,7 @@ arm_gic_alloc_msix(device_t dev, device_t child, device_t *pic,
> >>
> >> sc = device_get_softc(dev);
> >>
> >> - mtx_lock(&sc->mutex);
> >> + mtx_lock_spin(&sc->mutex);
> >> /* Find an unused interrupt */
> >> for (irq = sc->sc_spi_start; irq < sc->sc_spi_end; irq++) {
> >> KASSERT((sc->gic_irqs[irq].gi_flags & GI_FLAG_MSI) != 0,
> >> @@ -1152,13 +1152,13 @@ arm_gic_alloc_msix(device_t dev, device_t child, device_t *pic,
> >> }
> >> /* No free interrupt was found */
> >> if (irq == sc->sc_spi_end) {
> >> - mtx_unlock(&sc->mutex);
> >> + mtx_unlock_spin(&sc->mutex);
> >> return (ENXIO);
> >> }
> >>
> >> /* Mark the interrupt as used */
> >> sc->gic_irqs[irq].gi_flags |= GI_FLAG_MSI_USED;
> >> - mtx_unlock(&sc->mutex);
> >> + mtx_unlock_spin(&sc->mutex);
> >>
> >> *isrcp = (struct intr_irqsrc *)&sc->gic_irqs[irq];
> >> *pic = dev;
> >> @@ -1178,9 +1178,9 @@ arm_gic_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->mutex);
> >> + mtx_lock_spin(&sc->mutex);
> >> gi->gi_flags &= ~GI_FLAG_MSI_USED;
> >> - mtx_unlock(&sc->mutex);
> >> + mtx_unlock_spin(&sc->mutex);
> >>
> >> return (0);
> >> }
> >
> > Thank you for the patch, it fixes the MSI-X in my setup, but in order
> > to operate properly on Marvell SoCs (equipped with a standard GICv2m),
> > I have to remove the asserts, which were added in c6f3076d4405 (Move
> > the GICv2m msi handling to the parent):
> >
> > diff --git a/sys/arm/arm/gic.c b/sys/arm/arm/gic.c
> > index 89db4e324600..b5d72a2a6c49 100644
> > --- a/sys/arm/arm/gic.c
> > +++ b/sys/arm/arm/gic.c
> > @@ -528,8 +528,6 @@ arm_gic_write_ivar(device_t dev, device_t child,
> > int which, uintptr_t value)
> > * 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);
> > MPASS(value >= GIC_FIRST_SPI);
> > MPASS(value < sc->nirqs);
> >
> > @@ -537,7 +535,6 @@ arm_gic_write_ivar(device_t dev, device_t child,
> > int which, uintptr_t value)
> > return (0);
> > case GIC_IVAR_MBI_COUNT:
> > MPASS(sc->sc_spi_start != 0);
> > - MPASS(sc->sc_spi_count == 0);
> >
> > sc->sc_spi_count = value;
> > sc->sc_spi_end = sc->sc_spi_start + sc->sc_spi_count;
> >
> > Any thoughts?
>
> Can you try the patch in https://reviews.freebsd.org/D32224. The above change is to check there is only a single mbi range, however it appears your hardware has multiple gicv2m devices so will try to reserve multiple mbi ranges.
>
After applying this patch the MSI-X work fine and I can boot the OS
without issue.
Thanks,
Marcin
More information about the dev-commits-src-all
mailing list