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:   Fri, 16 Sep 2022 11:12:27 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Robert Richter <rrichter@amd.com>,
        Alison Schofield <alison.schofield@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        "Ira Weiny" <ira.weiny@intel.com>,
        Ben Widawsky <bwidawsk@kernel.org>,
        "Dan Williams" <dan.j.williams@intel.com>
CC:     <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Len Brown <lenb@kernel.org>, Robert Richter <rrichter@amd.com>
Subject: RE: [PATCH 15/15] cxl/acpi: Specify module load order dependency for
 the cxl_acpi module
Message-ID: <6324bc8b6c2df_2a6ded2944@dwillia2-xfh.jf.intel.com.notmuch>
References: <20220831081603.3415-1-rrichter@amd.com>
 <20220831081603.3415-16-rrichter@amd.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20220831081603.3415-16-rrichter@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?ZynD0frR+vxbQ8eDXB6V0oPR5JceNZWpNgj1LgPDmzJdz7Ox4srEx06ZP6hH?=
 =?us-ascii?Q?Ty6rN14L5sKIc/RZARcyqT94sAF0ocYrkXBVYupQObabGFvmtGvemMnCdC/x?=
 =?us-ascii?Q?0eoH6qxNE3k3uhpHefEYXQclPeszXm9vpAhKxcuICaXEcD1TussJkPmCwNwk?=
 =?us-ascii?Q?RsHfwqngzfH2GRIqIj3nh1qjwl/kgOXp/dXx4/nteMiUysj+yk2zFEmYJzns?=
 =?us-ascii?Q?3R6KxbPJTemM1uriQ+AvAbxPzRkaWdp+7vg1tEks4FxLeggyYCVclkFVjsdM?=
 =?us-ascii?Q?tQC5E0t+4oP+9dmiAKIDl9NhWswRE745ecc1TSl4ny3uL57Y+SqwGmwyJGay?=
 =?us-ascii?Q?Q8JHcsDkIGioWwIRfQalTE1EPHRsNz/zq4e2ZhiITiXXFb873FNSj3Sqk5kc?=
 =?us-ascii?Q?5P2JHAitJovChxHg5k0f7n1KJXtBDuARoNTRG+ebU5t98tSj6jpoVZMvNUHz?=
 =?us-ascii?Q?S3lmYutUkCMHZ4U72lnTwbR8a4byBdsPCgj7NTtVR4/om1kHpmapM2lie8s6?=
 =?us-ascii?Q?qR2u7XgvCCh7gWsef+UDYbCj1mScfQgIbDxCe8TYTWWDLO6Kbmu4TuyttP0F?=
 =?us-ascii?Q?NS4442Y8Pgl6QJrvj//zb0oxsTWxTKGogQq+E2pA0Rzj06zqw6u8cXZ0r+OU?=
 =?us-ascii?Q?BSAjJPV9PqnhsNj+Q1V7fLnlDr+4R+5pJ3WMkj2IFObcwuYog0a3TQQrvAWi?=
 =?us-ascii?Q?UzQwSZcWtaW/fE/yEnP2m99dzzee1VsXloywhmH/6TyXedrY4nUxoVRfird0?=
 =?us-ascii?Q?uaW7rN+3fae/v10f+bHCrmRFlP2Cx5HqZW4DxDRk3v5a8HX1SrFrOlCDYweI?=
 =?us-ascii?Q?VflEIfCLYPcrp8fC3msQfrIPPSKk6FHSNVekY1YFWoX9nXH8a/67JsshU7KM?=
 =?us-ascii?Q?LArxtvryX98zIdyNvBMGbagjWMlbXKX3mitKPIVclEaTdg6bpiFgvgV6hKx9?=
 =?us-ascii?Q?cPs7PHlVxIw7T9SkIRfg6leHN/ho2oC4vtMws6Ef4ykERjSw8ugrV32XsqqH?=
 =?us-ascii?Q?zDmTL4hHthUeNxidzQfj01Cgr0D7kDEstuMInxruM8T7N2Z7RHquwiw3QsE2?=
 =?us-ascii?Q?7JCABxy5N36Q6GY5S6kT2vq6zkOixTntYPdPcxP38h401xWfHhih+krnlxrS?=
 =?us-ascii?Q?BDFxkLM1kNYli7bQO55tsJZTqxS80BAUGuvc35U/UdRoJQCWy1HWQAgffxrK?=
 =?us-ascii?Q?qbh/N42ZYic69ZtSEY3Eigqz1PqagJDSF1f+Lhsq4VSmRgaPqbRW4yrxVGTY?=
 =?us-ascii?Q?DohIgwbWoGLJ0mrOVLgrXWoDKdKC/TuBCZL/MLtsSh4jNWcpn8Qt7IsZdyKU?=
 =?us-ascii?Q?okNfMGr0hYjyAHklIiee4DqOYT5fshCiv3IijTy67jMraL0mjqrDrmOlRctB?=
 =?us-ascii?Q?8CwNnVxlKEhzY3CjId0Piu6bDL4QNArC9cjBJd/bCg7xlz3r3FR8aERCCEK7?=
 =?us-ascii?Q?OqQnDOLhLyXYf58mBug1VLbhg6MX8JcWwihWgm2djQlXE3nFFEA2XkwqyNU4?=
 =?us-ascii?Q?MPVcydt7aufwuomVfQuVvsBdZ1Fo/MkLSrTjnXuMDDENeal9qMwofKY6CMca?=
 =?us-ascii?Q?B2PmmCMZ8yZr8dfMWoYtBca4a+yBEFc2zQNXKsfLHnkgewXaOpW6BSc/pdpo?=
 =?us-ascii?Q?MA=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e542ff41-7d46-4253-d9dd-08da980f04c0
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Sep 2022 18:12:30.0994
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: GDLCg9YN08XFTiXLbDuXgsL0Z3csVmFwcgP9bVNfhTs+DIa0InF//rwy62xE95//qTOe1umZfVj1SdLG2HfUPi2Xe1vs5XRhsJzJ9YgqnWA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB6945
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Robert Richter wrote:
> In RCD mode the CXL mem dev may be detected on the PCI bus before a
> CXL host is brought up. This may cause a CXL mem initialization
> failure as it expects the CXL host already detected.

This is not unique to RCD mode. It is mitigated by the cxl_bus_rescan()
at the completion of cxl_acpi_probe().


> Address this by
> specifying the module dependencies using MODULE_SOFTDEP().
> 
> The following additional dependencies exist:
> 
>  * cxl_mem depends on cxl_acpi: The CXL hosts must be discovered
>    before the CXL device is initialized.
> 
>  * cxl_acpi depends on cxl_port: The acpi driver adds ports to the cxl
>    bus, the port driver should be loaded before. This might also work
>    if modules are loaded in different order, but a) it aligns with the
>    existing cxl_mem/cxl_port softdep and b) it always guarantees a fix
>    module load order.

Why does a fixed module load order matter?

> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/cxl/acpi.c | 1 +
>  drivers/cxl/mem.c  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 56b2d222afcc..63a1cb295c07 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -834,3 +834,4 @@ module_exit(cxl_host_exit);
>  MODULE_LICENSE("GPL v2");
>  MODULE_IMPORT_NS(CXL);
>  MODULE_IMPORT_NS(ACPI);
> +MODULE_SOFTDEP("pre: cxl_port");

The only reason cxl_acpi would need to preload cxl_port is if it wanted
to do something like:

port = devm_cxl_add_port(...);
if (port->dev.driver)
	/* do something with an enabled port */
else
	/* do something else if the port failed to enable */

> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 64ccf053d32c..ae13ec7d9894 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -128,3 +128,4 @@ MODULE_ALIAS_CXL(CXL_DEVICE_MEMORY_EXPANDER);
>   * endpoint registration.
>   */
>  MODULE_SOFTDEP("pre: cxl_port");
> +MODULE_SOFTDEP("pre: cxl_acpi");

There is no strict requirement that CXL topologies be limited to ACPI
platforms. Per above, the cxl_mem driver will attach when the CXL root
device finally appears, and async out-of-order arrival is ok.