Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 9 Apr 2022 10:13:15 +0200
From:   Pavel Machek <pavel@denx.de>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     linux-kernel@vger.kernel.org, stable@vger.kernel.org,
        Liguang Zhang <zhangliguang@linux.alibaba.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Lukas Wunner <lukas@wunner.de>
Subject: Re: [PATCH 5.10 126/599] PCI: pciehp: Clear cmd_busy bit in polling
 mode
Message-ID: <20220409081314.GA19452@amd>
References: <20220405070258.802373272@linuxfoundation.org>
 <20220405070302.589741179@linuxfoundation.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
        protocol="application/pgp-signature"; boundary="bg08WKrSYDhXBjb5"
Content-Disposition: inline
In-Reply-To: <20220405070302.589741179@linuxfoundation.org>
User-Agent: Mutt/1.5.23 (2014-03-12)
Precedence: bulk


--bg08WKrSYDhXBjb5
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi!

> From: Liguang Zhang <zhangliguang@linux.alibaba.com>
>=20
> commit 92912b175178c7e895f5e5e9f1e30ac30319162b upstream.
>=20
> Writes to a Downstream Port's Slot Control register are PCIe hotplug
> "commands."  If the Port supports Command Completed events, software must
> wait for a command to complete before writing to Slot Control again.
>=20
> pcie_do_write_cmd() sets ctrl->cmd_busy when it writes to Slot Control.  =
If
> software notification is enabled, i.e., PCI_EXP_SLTCTL_HPIE and
> PCI_EXP_SLTCTL_CCIE are set, ctrl->cmd_busy is cleared by pciehp_isr().
>=20
> But when software notification is disabled, as it is when pcie_init()
> powers off an empty slot, pcie_wait_cmd() uses pcie_poll_cmd() to poll for
> command completion, and it neglects to clear ctrl->cmd_busy, which leads =
to
> spurious timeouts:

I'm pretty sure this fixes the problem, but... it is still not fully
correct.

> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -98,6 +98,8 @@ static int pcie_poll_cmd(struct controll
>  		if (slot_status & PCI_EXP_SLTSTA_CC) {
>  			pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
>  						   PCI_EXP_SLTSTA_CC);
> +			ctrl->cmd_busy =3D 0;
> +			smp_mb();
>  			return 1;
>  		}

Is the memory barrier neccessary? I don't see corresponding memory
barrier for reading.

If it is neccessary, should we have WRITE_ONCE at the very least, or
probably normal atomic operations?

Best regards,
								Pavel
--=20
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

--bg08WKrSYDhXBjb5
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iEYEARECAAYFAmJRQBoACgkQMOfwapXb+vJmSACgki8gGnt/pbitsMqI7fSOXeB6
KMcAn3tpJEX3NYiHw6SwAKOoI6HrVXiR
=5uvu
-----END PGP SIGNATURE-----

--bg08WKrSYDhXBjb5--