From owner-dev-commits-src-all@freebsd.org Wed Sep 29 15:43:03 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 5021767659B; Wed, 29 Sep 2021 15:43:03 +0000 (UTC) (envelope-from andrew@fubar.geek.nz) Received: from fry.fubar.geek.nz (fry.fubar.geek.nz [139.59.165.16]) by mx1.freebsd.org (Postfix) with ESMTP id 4HKLHT6FXsz3wK9; Wed, 29 Sep 2021 15:43:01 +0000 (UTC) (envelope-from andrew@fubar.geek.nz) Received: from smtpclient.apple (cpc91232-cmbg18-2-0-cust554.5-4.cable.virginm.net [82.2.126.43]) by fry.fubar.geek.nz (Postfix) with ESMTPSA id 90B6F4E630; Wed, 29 Sep 2021 15:42:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fubar.geek.nz; s=mail; t=1632930144; bh=qL4oHJUVnt3YWcsblqA/C1sJy2AKwgVCIBCq1t9RIlQ=; h=Subject:From:In-Reply-To:Date:Cc:References:To; b=XGCkZLAD5PMOsdCVJKzdRudOOzu0PeV2cHWecqUV0lb+eX9/vTGgPoDq6Vcs6P4a6 iwTo/Zn/9I7gU19sjaXgiQP+mfemwl3+kJnY0Qwsbu7WOf04IiS1x+3qs/rQIvqWi3 CIPLAel1T2IXAwcxUPpxJPhoQrfMg1D9+sTVtJhn/E8ilKORGUFz1CT+2FjbdsWUDG 4MK9s/nWJXBRYMf4MSug95HmynNWbVvBxsLKP3fWqjo/42pKmRmboHnZ/GxiWEhzle EgAcaL95Nvn2EYHzGUNW0yq44XeCrNPvnmjGl7C0S1GfFNZ0DlWvokqV4PPCCeD+jx UhwHmDV/fzG0luFM/SOvF7WFUDNBocpOjOQAvPDNOibv3FUBZtk9iREp4WGiYpyYC6 g7VhFPMzwi9RVLAmDaG/errQoUutmblL+zCJGppvg3Vih2+DpIppue5oFZ40GIGvrn 1HEfV7ikkegPAzknQeKBqO9e+lwBNPFsCf1oRHfT01QHABJ50C4+Bw/lkCILhMTlyw 6yN98m3bZR+uo9L5fZ0JsYWudBVJf2bGnbIKScx98icEv/1tpf1EHV8MUWuZSzac2D NRHwm+65G90R3M4Jd9KZwfrhxwfZORBrbwgZWrSh/V6M5iii8/9rQkS13dPZu4a1Vc n3eprBgLFYsitBvjJ2l0Nkl4= Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.120.0.1.13\)) Subject: Re: git: f3aa0098a82e - main - Use mtx_lock_spin in the gic driver From: Andrew Turner In-Reply-To: Date: Wed, 29 Sep 2021 16:42:24 +0100 Cc: Andrew Turner , src-committers , dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: <66731C91-BAE9-4D5D-B7DE-F1D7AA94395D@fubar.geek.nz> References: <202109281243.18SChldh001988@gitrepo.freebsd.org> To: Marcin Wojtas X-Mailer: Apple Mail (2.3654.120.0.1.13) X-Rspamd-Queue-Id: 4HKLHT6FXsz3wK9 X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=fubar.geek.nz header.s=mail header.b=XGCkZLAD; dmarc=pass (policy=none) header.from=fubar.geek.nz; spf=pass (mx1.freebsd.org: domain of andrew@fubar.geek.nz designates 139.59.165.16 as permitted sender) smtp.mailfrom=andrew@fubar.geek.nz X-Spamd-Result: default: False [-3.40 / 15.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; R_DKIM_ALLOW(-0.20)[fubar.geek.nz:s=mail]; FREEFALL_USER(0.00)[andrew]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; MV_CASE(0.50)[]; MID_RHS_MATCH_FROM(0.00)[]; MIME_GOOD(-0.10)[text/plain]; R_SPF_ALLOW(-0.20)[+mx]; NEURAL_HAM_LONG(-1.00)[-1.000]; RCPT_COUNT_FIVE(0.00)[5]; TO_MATCH_ENVRCPT_SOME(0.00)[]; DKIM_TRACE(0.00)[fubar.geek.nz:+]; DMARC_POLICY_ALLOW(-0.50)[fubar.geek.nz,none]; NEURAL_HAM_SHORT(-1.00)[-1.000]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; RCVD_NO_TLS_LAST(0.10)[]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:14061, ipnet:139.59.160.0/20, country:US]; RCVD_COUNT_TWO(0.00)[2]; MAILMAN_DEST(0.00)[dev-commits-src-all,dev-commits-src-main] X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Sep 2021 15:43:03 -0000 > On 28 Sep 2021, at 16:36, Marcin Wojtas wrote: >=20 > Hi Andrew, >=20 > wt., 28 wrz 2021 o 14:43 Andrew Turner = napisa=C5=82(a): >>=20 >> The branch main has been updated by andrew: >>=20 >> URL: = https://cgit.FreeBSD.org/src/commit/?id=3Df3aa0098a82ebf7712aa13716d794aa7= e4ac59cd >>=20 >> commit f3aa0098a82ebf7712aa13716d794aa7e4ac59cd >> Author: Andrew Turner >> AuthorDate: 2021-09-28 11:36:42 +0000 >> Commit: Andrew Turner >> CommitDate: 2021-09-28 11:42:06 +0000 >>=20 >> Use mtx_lock_spin in the gic driver >>=20 >> 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. >>=20 >> Submitted by: jrtc27 ("Change all the mtx_(un)lock(&sc->mutex) = to be the _spin versions.") >> Reported by: mw, antranigv@freebsd.am >> Sponsored by: The FreeBSD Foundation >> --- >> sys/arm/arm/gic.c | 20 ++++++++++---------- >> 1 file changed, 10 insertions(+), 10 deletions(-) >>=20 >> 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, >>=20 >> sc =3D device_get_softc(dev); >>=20 >> - mtx_lock(&sc->mutex); >> + mtx_lock_spin(&sc->mutex); >>=20 >> found =3D false; >> for (irq =3D 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, >>=20 >> /* Not enough interrupts were found */ >> if (!found || irq =3D=3D sc->sc_spi_end) { >> - mtx_unlock(&sc->mutex); >> + mtx_unlock_spin(&sc->mutex); >> return (ENXIO); >> } >>=20 >> @@ -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 |=3D GI_FLAG_MSI_USED; >> } >> - mtx_unlock(&sc->mutex); >> + mtx_unlock_spin(&sc->mutex); >>=20 >> for (i =3D 0; i < count; i++) >> srcs[i] =3D (struct intr_irqsrc *)&sc->gic_irqs[irq + = i]; >> @@ -1118,7 +1118,7 @@ arm_gic_release_msi(device_t dev, device_t = child, int count, >>=20 >> sc =3D device_get_softc(dev); >>=20 >> - mtx_lock(&sc->mutex); >> + mtx_lock_spin(&sc->mutex); >> for (i =3D 0; i < count; i++) { >> gi =3D (struct gic_irqsrc *)isrc[i]; >>=20 >> @@ -1128,7 +1128,7 @@ arm_gic_release_msi(device_t dev, device_t = child, int count, >>=20 >> gi->gi_flags &=3D ~GI_FLAG_MSI_USED; >> } >> - mtx_unlock(&sc->mutex); >> + mtx_unlock_spin(&sc->mutex); >>=20 >> return (0); >> } >> @@ -1142,7 +1142,7 @@ arm_gic_alloc_msix(device_t dev, device_t = child, device_t *pic, >>=20 >> sc =3D device_get_softc(dev); >>=20 >> - mtx_lock(&sc->mutex); >> + mtx_lock_spin(&sc->mutex); >> /* Find an unused interrupt */ >> for (irq =3D sc->sc_spi_start; irq < sc->sc_spi_end; irq++) { >> KASSERT((sc->gic_irqs[irq].gi_flags & GI_FLAG_MSI) !=3D = 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 =3D=3D sc->sc_spi_end) { >> - mtx_unlock(&sc->mutex); >> + mtx_unlock_spin(&sc->mutex); >> return (ENXIO); >> } >>=20 >> /* Mark the interrupt as used */ >> sc->gic_irqs[irq].gi_flags |=3D GI_FLAG_MSI_USED; >> - mtx_unlock(&sc->mutex); >> + mtx_unlock_spin(&sc->mutex); >>=20 >> *isrcp =3D (struct intr_irqsrc *)&sc->gic_irqs[irq]; >> *pic =3D 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) =3D=3D = GI_FLAG_MSI_USED, >> ("%s: Trying to release an unused MSI-X interrupt", = __func__)); >>=20 >> - mtx_lock(&sc->mutex); >> + mtx_lock_spin(&sc->mutex); >> gi->gi_flags &=3D ~GI_FLAG_MSI_USED; >> - mtx_unlock(&sc->mutex); >> + mtx_unlock_spin(&sc->mutex); >>=20 >> return (0); >> } >=20 > 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): >=20 > 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 =3D=3D 0); > - MPASS(sc->sc_spi_count =3D=3D 0); > MPASS(value >=3D GIC_FIRST_SPI); > MPASS(value < sc->nirqs); >=20 > @@ -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 !=3D 0); > - MPASS(sc->sc_spi_count =3D=3D 0); >=20 > sc->sc_spi_count =3D value; > sc->sc_spi_end =3D sc->sc_spi_start + sc->sc_spi_count; >=20 > 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. Andrew