From owner-dev-commits-src-main@freebsd.org Tue Sep 28 15:36:39 2021 Return-Path: Delivered-To: dev-commits-src-main@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 3E48467EB4F for ; Tue, 28 Sep 2021 15:36:39 +0000 (UTC) (envelope-from mw@semihalf.com) Received: from mail-yb1-xb2f.google.com (mail-yb1-xb2f.google.com [IPv6:2607:f8b0:4864:20::b2f]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HJkBb0v4lz3jlS for ; Tue, 28 Sep 2021 15:36:39 +0000 (UTC) (envelope-from mw@semihalf.com) Received: by mail-yb1-xb2f.google.com with SMTP id w19so29911246ybs.3 for ; Tue, 28 Sep 2021 08:36:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=uh5xAj6E7QII6QFkS+2H8Me9eGbnj23770z6A0qXmxY=; b=U8mghCTPfRYmuxEpsguLvc2p1RID8Tvo+J5YDx3ZGWUVV3Rd0J75Ysn0wW5lTZBucu EIPU2LfMEw1hhhSMzufeMZvzFO9x7vhtbfjqB1MWbIGdzeLsq2IgnT+TyrkAbh+P1awc r23tcnzB9BCBMQmX+JIxsAWLtdmE0vJpwWHv4F8jUVkAWvZjL74jBV7LCNFEJta815J7 +U1ZQNiFvO24CTXhtaqocsYoJ7Xnu/lzNNIGqbug1F+6CqSBdCmcDK8B3YGoFRGOTvy5 XE9r9HJGs3UWHQPiKBFGp/Jb8/R5IQP2OUxghQx4ZI59N9oadI30u9iVAhklG8GNeaZx /4ug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=uh5xAj6E7QII6QFkS+2H8Me9eGbnj23770z6A0qXmxY=; b=7D/UAsZjANY8iXfSsaUto2SkUEdZWY5ZMBLeuDHIZf/XOOiBNdRdt1QFp2nPbpwP70 8JBVrPg5At6XPaFaxf6rTwqRWJ3Bx8h740cH5+WYxZ39C5304ueF5O7eaPy435sFzXfP DO9wbUlOcGQJ80jtNtC1GgoxB3U74p45kftAtTz5asN6S6oPeliWmzEO9blvzIwMvtOL CUr/e6n27NYhyNVmpxrSG4DAr/gwVPfQ5XvvpKkf0T48pN8++U8EZlW0LyL3YawmfbCj D4TTIUZGzxJz9VBArnZ642FAo2bXsYnytJoDrNp9tWsObr/ZnX+XQ1FLv3f1UYg2cgY4 SzUQ== X-Gm-Message-State: AOAM533zEGzgtLOgKUdZ0+UHEB1d6mXSptp+wGTlMOkcYLwZmz0aU0gM wfChiTa9HJhaS6d3J6DR4/IA10aRBrh3nImaj0H7+w== X-Google-Smtp-Source: ABdhPJxVeXkgJncIyHKTn9WNNZYopjgTExBTz77SS7+akOfUNu8qn02zijIssqle9meTNYOR/HSH4fh6KvsDwTuBaqM= X-Received: by 2002:a5b:783:: with SMTP id b3mr7122459ybq.328.1632843398313; Tue, 28 Sep 2021 08:36:38 -0700 (PDT) MIME-Version: 1.0 References: <202109281243.18SChldh001988@gitrepo.freebsd.org> In-Reply-To: <202109281243.18SChldh001988@gitrepo.freebsd.org> From: Marcin Wojtas Date: Tue, 28 Sep 2021 17:36:27 +0200 Message-ID: Subject: Re: git: f3aa0098a82e - main - Use mtx_lock_spin in the gic driver To: Andrew Turner Cc: src-committers , dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4HJkBb0v4lz3jlS X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Sep 2021 15:36:39 -0000 Hi Andrew, wt., 28 wrz 2021 o 14:43 Andrew Turner napisa=C5=82(a)= : > > The branch main has been updated by andrew: > > URL: https://cgit.FreeBSD.org/src/commit/?id=3Df3aa0098a82ebf7712aa13716d= 794aa7e4ac59cd > > commit f3aa0098a82ebf7712aa13716d794aa7e4ac59cd > Author: Andrew Turner > AuthorDate: 2021-09-28 11:36:42 +0000 > Commit: Andrew Turner > 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 b= e 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(-) > > 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 =3D device_get_softc(dev); > > - mtx_lock(&sc->mutex); > + mtx_lock_spin(&sc->mutex); > > 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, > > /* 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); > } > > @@ -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); > > 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, i= nt count, > > sc =3D device_get_softc(dev); > > - mtx_lock(&sc->mutex); > + mtx_lock_spin(&sc->mutex); > for (i =3D 0; i < count; i++) { > gi =3D (struct gic_irqsrc *)isrc[i]; > > @@ -1128,7 +1128,7 @@ arm_gic_release_msi(device_t dev, device_t child, i= nt count, > > gi->gi_flags &=3D ~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, de= vice_t *pic, > > sc =3D device_get_softc(dev); > > - 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); > } > > /* 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); > > *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__)= ); > > - 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); > > 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 allo= ws * us to reserve the registers when GIC_IVAR_MBI_COUNT is s= et. */ - 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); @@ -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); sc->sc_spi_count =3D value; sc->sc_spi_end =3D sc->sc_spi_start + sc->sc_spi_count; Any thoughts? Best regards, Marcin