Received-SPF: pass (google.com: domain of linux-kernel+bounces-176850-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
To: Klara Modin <klarasmodin@gmail.com>
CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Lukas Wunner
	<lukas@wunner.de>, "bcfradella@proton.me" <bcfradella@proton.me>, Danil
 Rybakov <danilrybakov249@gmail.com>, Hans de Goede <hdegoede@redhat.com>,
	=?iso-8859-1?Q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>,
	"platform-driver-x86@vger.kernel.org" <platform-driver-x86@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Ben Fradella
	<bfradell@netapp.com>, Ranjan Dutta <ranjan.dutta@intel.com>, Yifan2 Li
	<yifan2.li@intel.com>, Jonathan Yong <jonathan.yong@intel.com>
Subject: Re: [PATCH] p2sb: Don't init until unassigned resources have been
 assigned.
Thread-Topic: [PATCH] p2sb: Don't init until unassigned resources have been
 assigned.
Thread-Index: AQHaojL91N1jMhQERUKPwzt4AofoErGQjnsAgABL7YCAAo3rgA==
Date: Sun, 12 May 2024 10:23:02 +0000
Message-ID: <am2n3qjaxgnciwfxhqs4aeosyug2oy2umuul4dzuagfhxdjlod@7ksrhwbjdxuc>
References: <20240509164905.41016-1-bcfradella@proton.me>
 <Zj0CIPR5djf0-hHb@wunner.de> <Zj40RnWnNqpr4Nom@smile.fi.intel.com>
 <b9ce0494-ab30-4647-9b0b-e01cc19dab23@gmail.com>
In-Reply-To: <b9ce0494-ab30-4647-9b0b-e01cc19dab23@gmail.com>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?iso-8859-1?Q?Wt3fjGcpOyWTa1FiMGVr9rpCB3CDZNTmEPHKuPuo7SVgyvkV4lxzY7scWW?=
 =?iso-8859-1?Q?sZpOqK6Lx0b/1dYY6T7tWEwDUWXjowrAshQ2MF6u3sjlMabey2e6kDeZN4?=
 =?iso-8859-1?Q?AQlkt14EKo415RFIkXeqKNeh6+qSJoZcvvs88JsP2FC0idfsBIphpVC4G0?=
 =?iso-8859-1?Q?hGwYLvqiGuVJ4okMUdTDjywd3XRBqWxq3Urm6ns6zqMEcwXduzq/AZDBs6?=
 =?iso-8859-1?Q?W6JQpluOty3uK/j1u7OM6sEurV+0g/9WOvY56BzqxSMkErmxBjMU1yWPYu?=
 =?iso-8859-1?Q?Lq8R0gz8Pdhe+lYaK/CpVddAVgn8qO560ixNnErBqulzkFvYG9T/ZjUA7D?=
 =?iso-8859-1?Q?f+/r33qECiPgeuy6zObXk9lWgw7SBuyYMfWZR4RqodTJxKG8CU/jVPxftT?=
 =?iso-8859-1?Q?LQEY/EGiwfJqWgFsNRXUXs3hx8MV+lbnxNMOiBEFj7VzJLWsSfmUEXxLzd?=
 =?iso-8859-1?Q?1OhkCOaRpBjNn7FneDWSI86aHjWoaig8pqSIU+LzemFAYgQbwIZhzeVs/B?=
 =?iso-8859-1?Q?XTp+PKGq77V02MIDclYYtZGvjXNTpCHsJDh0Iid7ugaqGsonbPTYfTXGx1?=
 =?iso-8859-1?Q?Dthubwlmh80OMULcNS9iUp2zJ0cJZTc28Hd2CX5C5JNcahNQfaO/4BUJcO?=
 =?iso-8859-1?Q?+/bbWMW4jGZOG+Y1h97jonp4i3PEFByPalNjxE/VvFgqnjmmSOJ+5KDWNO?=
 =?iso-8859-1?Q?UY5BVSGivJ0jxFrdTr5XA+XSzpChNIUUxwqtIZvMT/s+LliIXLOHanow0j?=
 =?iso-8859-1?Q?wGozw92beijzIHcfBLYzJ8BxHWIQu3L1cy+FpOZ69JcGwDL4aaVlGzAUuM?=
 =?iso-8859-1?Q?osCcDOPTMPJcNL6kMfVCW2KLuz32IyN93OmXoaXEFgKm6g++bk+6QLt/iP?=
 =?iso-8859-1?Q?7hWf6Q6jAHq49u0bh1NRm7GHO/BSZfrraI+2Pq2jHdQ/p7Mbsc2ecyDCE0?=
 =?iso-8859-1?Q?RsXUikEbVpX+PjKzvY+jphIeEMwi4D3eJqDyzPcjDVUlBpNRjpdFtaD6h8?=
 =?iso-8859-1?Q?o9PBgHMTng9onJYG4yUPuU3yHhrGKa99Rjg/QyzTfRhAocrsoFgD7XMPW9?=
 =?iso-8859-1?Q?DjGkkvJSzeF2QX1IdMw0HXdr1nwuxar7O/Flwh3HGvSUYy315HaLujdkPp?=
 =?iso-8859-1?Q?TdEYJvam0qYM7cSayCPocspdFTFXrA6SmG6yRFoE1pAZzrV0WOd57nxAeb?=
 =?iso-8859-1?Q?HpleFWsDWCg55CI3U+QS5iyVm5r3w2/zhk+kvRMaydhIT8eJYqfYyUwkGM?=
 =?iso-8859-1?Q?ZqlzEuZuGkFWAWnqDw7CHQguscTOMQXqfQ81bULMjIS0H1XUijOk9RsAPO?=
 =?iso-8859-1?Q?0PCg8sa5/75YMYO6diWxWP/6P9tQgNHsBWuOnmYc5YVsHJw1sn2Rzp+Ze1?=
 =?iso-8859-1?Q?mI57LQuFjoybTnheEjqymfd66INirye6ZtaMElkD20qj2InIH9PdGHuhb1?=
 =?iso-8859-1?Q?b3lwSI6Jss3wg+qnp/JCKVmOYhk4qjf8XqHRiOEErFqAFau3gP3Ji2GBbf?=
 =?iso-8859-1?Q?mI4UhSqzVGSzgOMYklUtAmcbdPHuBpMIIBL71Pq9UwfvoV3XnuOQ8FQ/hz?=
 =?iso-8859-1?Q?xMZ8wij8Mafdxi8PZHG9Jn6Ck2GysvytrqkOZyyDJbKmHC3o5NrRsoaIYN?=
 =?iso-8859-1?Q?DUHtxeetxjSNLTDf8934jOIL914VaggDavHz89GLvNKa3r/BDFHEPiT1eh?=
 =?iso-8859-1?Q?Kp69Bg/F9ETyMlnH3i4=3D?=
Content-Type: text/plain; charset="iso-8859-1"
Content-ID: <5D9FB9FBF043FC4DA951E5AF6FE3AB5A@namprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0:
	5RrQpZMTaBVghft57Bk1BB7pEb+sxQ3ubmOpoLpbMCidUAd6WLl9aFB7cRuCBepRn+pP69CUdUEEMQsYTJMBSzdk0Xou9EzOoawv4VCrW9WN+q8IW2yr1aCrrzVdr5L4aOisDvAEB7ZBQSq9/xfPxZuGEriDeaP9zf+zIEvJrCX4h7+CJVjCyDPnK8fvVV5MVsRWawKjrXBjOWTibWA7XupOlojNICM/uNOaeasYd6Rn++kcudx+D/GyESTpqrRWxnbYxGr3R/FsNCrrB/X4KTSki3TEMIPU1zlPQcmYmQVRtusq33qxp1cwBXxRpfV7uM/16lxgCNfr6zYBylirh0xPlr1F6GWeJwQVxctiKi8oNXJnQ230Lb9f+0ho1uBnmKPf0ZryTBI41JefPP0e9uuI/xfvpmirfKasqrlimgRPqazjo7rKtYRrg14sT+ljuTxmdY31wLRKXqCJI/UOXaE/l9lvpJAHweAsQQYZ39dNLAQpYeSWPLjW2ATlw36CTW/pbPxUPqp+1HbrWpRMG762q0MqcJYfXC2nkbxfKGMPqHkoWtRExUk5IkwU8j+qT/CBhrkVXab/d51nD74TeigYBfAwN6yWIunHjoAlPZjbFCF2vrywT6caxau/dhFd
X-OriginatorOrg: wdc.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM8PR04MB8037.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 43a60500-341e-40f2-f430-08dc726d8144
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 May 2024 10:23:02.5347
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Sk4zxvfaA9U2ytWWLHAmYV8/eYjQun/CeTDdLbBnoCNPjGabkHCE0hI/oVILFFAHXDu63Vve8k5HX6thsYRYnwZDGBGclbUaUY7sA+VyjBI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR04MB6316

On May 10, 2024 / 21:22, Klara Modin wrote:
> On 2024-05-10 16:50, Andy Shevchenko wrote:
> > On Thu, May 09, 2024 at 07:04:32PM +0200, Lukas Wunner wrote:
> > > [cc +=3D Shin'ichiro, Klara, Andy, Danil]
> >=20
> > Thank you!
> >=20
> > > On Thu, May 09, 2024 at 04:49:34PM +0000, bcfradella@proton.me wrote:
> > > > From: Ben Fradella <bfradell@netapp.com>
> > > >=20
> > > > The P2SB could get an invalid BAR from the BIOS, and that won't be =
fixed
> > > > up until pcibios_assign_resources(), which is an fs_initcall().
> > > >=20
> > > > - Move p2sb_fs_init() to an fs_initcall_sync(). This is still early
> > > >    enough to avoid a race with any dependent drivers.
> > > >=20
> > > > - Add a check for IORESOURCE_UNSET in p2sb_valid_resource() to catc=
h
> > > >    unset BARs going forward.
> > > >=20
> > > > - Return error values from p2sb_fs_init() so that the 'initcall_deb=
ug'
> > > >    cmdline arg provides useful data.
> >=20
> > ...
> >=20
> > > >   /*
> > > > - * pci_rescan_remove_lock to avoid access to unhidden P2SB devices=
 can
> > > > - * not be locked in sysfs pci bus rescan path because of deadlock.=
 To
> > > > - * avoid the deadlock, access to P2SB devices with the lock at an =
early
> > > > - * step in kernel initialization and cache required resources. Thi=
s
> > > > - * should happen after subsys_initcall which initializes PCI subsy=
stem
> > > > - * and before device_initcall which requires P2SB resources.
> > > > + * pci_rescan_remove_lock() can not be locked in sysfs pci bus res=
can path
> >=20
> > PCI bus
> >=20
> > > > + * because of deadlock. To avoid the deadlock, access P2SB devices=
 with the lock
> > > > + * at an early step in kernel initialization and cache required re=
sources.
> > > > + *
> > > > + * We want to run as early as possible. If the P2SB was assigned a=
 bad BAR,
> > > > + * we'll need to wait on pcibios_assign_resources() to fix it. So,=
 our list of
> > > > + * initcall dependencies looks something like this:
> > > > + *
> > > > + * ...
> > > > + * subsys_initcall (pci_subsys_init)
> > > > + * fs_initcall     (pcibios_assign_resources)
> > > >    */
> >=20
> > This makes sense to me
> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >=20
> > I assume others will conduct the proper review and testing.
> >=20
>=20
> This patch did not introduce any new issues on my previously problematic
> system.
>=20
> Tested-by: Klara Modin <klarasmodin@gmail.com>

The changes look reasonable and good to me. I also confirmed that the patch
does not affect on my use case using two my test machines.

Reviewed-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>=