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:   Mon, 14 Nov 2022 16:24:06 -0800
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>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        "Davidlohr Bueso" <dave@stgolabs.net>,
        Dave Jiang <dave.jiang@intel.com>,
        Robert Richter <rrichter@amd.com>
Subject: RE: [PATCH v3 4/9] cxl/mem: Skip intermediate port enumeration of
 restricted endpoints (RCDs)
Message-ID: <6372dc26c8f2f_12cdff294e4@dwillia2-xfh.jf.intel.com.notmuch>
References: <20221109104059.766720-1-rrichter@amd.com>
 <20221109104059.766720-5-rrichter@amd.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221109104059.766720-5-rrichter@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?K1DQntRjyGgKkctQwNN4Pvz8m7q2ab+SkpkwfCqXQ8ks4yIrnt1ibz0Tj1Dt?=
 =?us-ascii?Q?Z62rZ57xbdyAtNAWmQQNaoh243en4X9VTW7qmCTs0tIOOfXHm2m6G5rK1JKN?=
 =?us-ascii?Q?hU1pDNXTB3nqkGPpRXXNj6CZi2KxpbOmHtjKoE4C1u0LUYh51om2u2TKkzi2?=
 =?us-ascii?Q?n+o2sQjw69ZW5eGL1B7wNrR3zt3y5mrhVkc9S7xjQdQOHzypvsVyQZwhcZ3K?=
 =?us-ascii?Q?7omQ3PMWTqoFc5KXwdguWYG27G/mx8sN15z43H8WbqsTq8anMqJbaY3i+EQD?=
 =?us-ascii?Q?JkL1WTSiTSsMl4Vs3xstRytLoufEC9I7By3vM7jyGjWEnRyYDWTfcl1d7SJ/?=
 =?us-ascii?Q?s8X0nWnithxWhRedvWfa56fJNSIEJp43kn6WbGlAi476ML1LIqNQnFGWFLLr?=
 =?us-ascii?Q?k2CrcTkvn+WXlscVmbUTz8IzCciULSfz815p766Lhw/3gvWq/UnqSLmDe8wj?=
 =?us-ascii?Q?Jw2PdlRBcmOAvSOA8l9ebHby3qOrZYdvaf0DzTFmQw/20MpJbF08Zqh2sN/1?=
 =?us-ascii?Q?tYWIcckxDzWK9JboVWsdggLwwSE6m0X/A9caKmN8S8IwbgLQr9plI5ADdWCP?=
 =?us-ascii?Q?9aZC4NgitQbaXal+Hhk9K0oJ4GeHKcQpCr5EPEQDhB3I9g8BZgUrTF5o12oO?=
 =?us-ascii?Q?VAoRswK9ULeyQp0Zq/KjEN2ZElIcQukaWiFrIiJYaeXsbK12E43+zPl76M9I?=
 =?us-ascii?Q?K9h9xvKlMc0hLnIqICkCaogf9oHUxd2+9YFsrcsRqQ3XLY2UewiYyU4YvEUQ?=
 =?us-ascii?Q?4UvIod4759c+ZOk1UkrsxwEVHKKsi+Q/YL/qD6e6gKN8FK6aVR39dDUqSwD0?=
 =?us-ascii?Q?Hu8GKcb2g9tCPPRkd3YeyJs+5NSezO1OBMwqzAxlFHAF8TqX/k5h+2EU8CVL?=
 =?us-ascii?Q?Iab86L1fC4rHJOzVupKvqTmAUCikqpAjoRPwGn4yAQYHZp6Zu/cG/chHDlbI?=
 =?us-ascii?Q?OJNb3sz5oRS4Ut5XYQqa7JC2PzLmf2Gr8buiBZnAqjTvwRWnFYJMO9ob4Qsw?=
 =?us-ascii?Q?zOyD2VUAlZnLe5hIbts4bnIqenegzUCMJbZNGkIMSgpi5dzfffGsPti+0EWp?=
 =?us-ascii?Q?BLEKg9IkBhGHppV095VP3tJWM4lszIcaipeFHREyEyF9/LJLtvoXTxkCrxPW?=
 =?us-ascii?Q?2laAk7MsdIeQZpcfc1XF9nsmaqEVo0B2+VPq0tKbGu/RTtrndtYHXAz9UcqI?=
 =?us-ascii?Q?HVMuS/Nz7LxiZc2ksL2o/nrxpkjeisZq8JVuv46sYpuvXp80HqlcsN1coKsR?=
 =?us-ascii?Q?288LdXqEM6U/N/9MXfTSWs6I7O6EK9s/ej+2k+tAyCBwTPy6gfXRhfUSfOdQ?=
 =?us-ascii?Q?SsPBZRljPHwr9yVajm2qJTqG+7AWg9Qfl4lDuzk6x/mi1juqh222cMWAHAaV?=
 =?us-ascii?Q?VBfmCnp7SUYatkv0/vJWxvwfm31LXOZ3qWFxeFEwLdQJ1sCJrmRgAuq7qeOd?=
 =?us-ascii?Q?out7S6W2PnoRVZ0tn3sDhI8UWMOw73nU12hhqXc/JohBz2K0P+jmiCdKuFK8?=
 =?us-ascii?Q?HFSZdPnddldPwqfOrIvRekyBAtvGVSHcoXJrneZF8i39O6QhGNSgchDv5Bir?=
 =?us-ascii?Q?IqAiUj7A5svKRgUFYjfCmCxQkaRhTGCJlw+Q3/S4m4bAxbR8gt+Kb1Hz86vU?=
 =?us-ascii?Q?nw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b588a95d-9cf5-4153-bc2c-08dac69fb6a7
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2126.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Nov 2022 00:24:09.5079
 (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: 7qrVg93XRzw7aH5TXRumQEiloAG0dOJsCfcOTtedwpO3JEpgrK05VXlrOb3C5xhlPrueRcNasXOyHg1RqrMsfIHunioPTz1R8Cl3mrN72pw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL3PR11MB6388
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:
> When an endpoint is found, all ports in beetween the endpoint and the
> CXL host bridge need to be created. In the RCH case there are no ports
> in between a host bridge and the endpoint. Skip the enumeration of
> intermediate ports.
> 
> The port enumeration does not only create all ports, it also
> initializes the endpoint chain by adding the endpoint to every
> downstream port up to the root bridge. This must be done also in RCD
> mode, but is much more simple as the endpoint only needs to be added
> to the host bridge's dport.
> 
> Note: For endpoint removal the cxl_detach_ep() is not needed as it is
> released in cxl_port_release().
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/cxl/core/port.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index d10c3580719b..e21a9c3fe4da 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1366,8 +1366,24 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>  {
>  	struct device *dev = &cxlmd->dev;
>  	struct device *iter;
> +	struct cxl_dport *dport;
> +	struct cxl_port *port;
>  	int rc;
>  
> +	/*
> +	 * Skip intermediate port enumeration in the RCH case, there
> +	 * are no ports in between a host bridge and an endpoint. Only
> +	 * initialize the EP chain.
> +	 */
> +	if (is_cxl_restricted(cxlmd)) {
> +		port = cxl_mem_find_port(cxlmd, &dport);
> +		if (!port)
> +			return -ENXIO;
> +		rc = cxl_add_ep(dport, &cxlmd->dev);

On second look, this seems problematic. While yes it will be deleted
when the root CXL port dies, it will not be deleted if the cxl_pci
driver is reloaded. I will code up a unit test to double check.

I note that cxl_add_ep() for the VH case is skipped for the root CXL
port, so I do not suspect it is needed here either. Did you add it for a
specific reason?