Hi Andrew,
wt., 28 wrz 2021 o 14:43 Andrew Turner <and...@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 <and...@freebsd.org>
> AuthorDate: 2021-09-28 11:36:42 +0000
> Commit: Andrew Turner <and...@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, antran...@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?
Best regards,
Marcin
_______________________________________________
dev-commits-src-main@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main
To unsubscribe, send any mail to "dev-commits-src-main-unsubscr...@freebsd.org"
