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, 12 Jun 2023 13:39:47 -0700
From:   Dan Williams <dan.j.williams@intel.com>
To:     Terry Bowman <terry.bowman@amd.com>, <alison.schofield@intel.com>,
        <vishal.l.verma@intel.com>, <ira.weiny@intel.com>,
        <bwidawsk@kernel.org>, <dan.j.williams@intel.com>,
        <dave.jiang@intel.com>, <Jonathan.Cameron@huawei.com>,
        <linux-cxl@vger.kernel.org>
CC:     <terry.bowman@amd.com>, <rrichter@amd.com>,
        <linux-kernel@vger.kernel.org>, <bhelgaas@google.com>
Subject: RE: [PATCH v5 11/26] cxl/pci: Early setup RCH dport component
 registers from RCRB
Message-ID: <64878293e1b01_1433ac29446@dwillia2-xfh.jf.intel.com.notmuch>
References: <20230607221651.2454764-1-terry.bowman@amd.com>
 <20230607221651.2454764-12-terry.bowman@amd.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230607221651.2454764-12-terry.bowman@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?k8jKBRyeRQ4eSfHMjnlzuWex1lzB3bv68JnlYrL8B085t3L/JqJEqDy/0wlG?=
 =?us-ascii?Q?omDuAU2Y2p78lOTIRwnR39aE/W0z2Iw4Q5dDpF38tpeb2DSHqqr1uAu1uhhU?=
 =?us-ascii?Q?xmH44GARwdPXZBMP8DpZZ6FujaXsRUNH98/DOeLeT+jMIUIg067XvgGvhhrv?=
 =?us-ascii?Q?VuLTVeCAaibB7LAzoxSk4KkzPfpzfCNHwu/2LlKLXZB0KPU4xVbybz1BjRnK?=
 =?us-ascii?Q?TdjLTRgDWVnXwURlW8elljkHa3EyPo2UEWqME8tvaJVGod+NgafuTg7ch4GM?=
 =?us-ascii?Q?s5IE9gU0Xa13b/1JfXmlA2XfxTR+HmVN0US36m8T+Bm9GykcPLnlL+yzZSqV?=
 =?us-ascii?Q?Y7lLNiaTQEwNxMZRsohVKET89sJ9u76ycSi4uv5lEmSAjvy609gjhACJlqwW?=
 =?us-ascii?Q?1sC9uFZcpbDUVbPHFOBFGdE3KGb6BA8Oj3ydTfpPT5Rgqgf6NjRxEixgm6jv?=
 =?us-ascii?Q?JwQmWkC3x9weEr/wp9X5RLbvyekJs7uHYomCIEtMX/RczGyhGCOiDKon0aIV?=
 =?us-ascii?Q?v203x1htiXOwaAnSnaN2Cbqlbm+H4GMImLj+Am8pTnAdL5EVo8KNZRVcGIh6?=
 =?us-ascii?Q?bWiGepG2uYc13asrtf8bvt0DwRhhwIaY2DGFkIeZJ025mY2Gd3t89I14jTbv?=
 =?us-ascii?Q?Q2jUdMqHpxamz7BhYcVArVMUgsMKekkkN1djmFOf/QcjeiU/PvSAKU0hoeHp?=
 =?us-ascii?Q?6K4euO7QstPUWnJn1JMABr1xXhbzZEc4ZRNkla4SSyNBx19zY37MxjPpO+lw?=
 =?us-ascii?Q?3UFbl5YDqWmTPb7AJFTlNaWG3Zsu2G9jLmhr6gB/V+hGPslemYjXm7btk6tf?=
 =?us-ascii?Q?/9Dz/Iyjv/Pd6i6RvWei8Fl1fibK4u0ojLdcTy8wxCNn+USMPY9gra70AB7n?=
 =?us-ascii?Q?Kd++6qVFISL+YTiTEy/uQzOTR8GzajfLPHnOOzrqalw4RSVsb2CltpFLUmoJ?=
 =?us-ascii?Q?hPihxFzhgjS2Fsc3o4nYaknnPLn4NZKHMsMRB3g3APm9mJelPFhNtOUo6kKr?=
 =?us-ascii?Q?Cniq6Ljcw9HGCaTokUAOZdpJwSD9EBv2KAi4I2TS5BrIgFcjcSv1FNteyZha?=
 =?us-ascii?Q?MnpsNIbOXY115w0skEd4lmEcPhTX3qzIcxq4BKmTE3rKZkgG4oopLBSir7UT?=
 =?us-ascii?Q?fM9r4KE+NScheDJQbdAMfTAB50gnPwPuypFWjsTkZB1ycRX1OGh8IL5J6DPk?=
 =?us-ascii?Q?RtbFRLS0CdbgUMOKem4k2EvQkj7KLqyyUH//7BvLIV+FqPR+8m76f+mZ3L1z?=
 =?us-ascii?Q?n5+W51/lJDtgffS+TNh4y5B3l3GiYJbgZ9vcjJrXpNFWpkQ1eGEJ1sdLUu7j?=
 =?us-ascii?Q?f8N8P2RM/Tt6iOOQlfDV6Ujtdk27QhXuR2eEZVnAyj+a++YSSGcAMkM4gZUO?=
 =?us-ascii?Q?QdWh+c2jLv2WCMEjJX/52xgJqMkDIZi51CUCE3OC3nUAAx1I0lzmyJTQWVvL?=
 =?us-ascii?Q?EZ4y/nXTCFZ8WcovAUmXHurGOqnsk54HR8tIeM4b0AAtE4gCG758TysczMa5?=
 =?us-ascii?Q?nBn8sc3lcWLa7wBOFmjZYqNV4QAtVtjCQAAHNj5FOXRxDypwjGdx+QE0jfF0?=
 =?us-ascii?Q?NXcVd4Tx7Dv9OpdEbF/qAyHAtY8n18UOBIuJIEAI9bSxPMaIBhR8vw5og2PK?=
 =?us-ascii?Q?tw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 36d8a09b-0b1b-492b-9fb4-08db6b852b1d
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jun 2023 20:39:50.3126
 (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: 3Nt74v87esK/oiybu3e1lsQ+JnjJsr2gVc7CneAXKXMavYj3b4WHAwD9TIJusD/Q15Ld2D9p2EGyWKw9642jqg+xNwvfRTw4SVWB8tJFSyI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB8325
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE 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

Terry Bowman wrote:
> From: Robert Richter <rrichter@amd.com>
> 
> CXL RAS capabilities must be enabled and accessible as soon as the CXL
> endpoint is detected in the PCI hierarchy and bound to the cxl_pci
> driver. This needs to be independent of other modules such as cxl_port
> or cxl_mem.
> 
> CXL RAS capabilities reside in the Component Registers. For an RCH
> this is determined by probing RCRB which is implemented very late once
> the CXL Memory Device is created.
> 
> Change this by moving the RCRB probe to the cxl_pci driver. Do this by
> using a new introduced function cxl_pci_find_port() similar to
> cxl_mem_find_port() to determine the involved dport by the endpoint's
> PCI handle. Plug this into the existing cxl_pci_setup_regs() function
> to setup Component Registers. Probe the RCRB in case the Component
> Registers cannot be located through the CXL Register Locator
> capability.
> 
> This unifies code and early sets up the Component Registers at the
> same time for both, VH and RCH mode. Only the cxl_pci driver is
> involved for this. This allows an early mapping of the CXL RAS
> capability registers.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
[..]
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 945ca0304d68..2975b232fcd1 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -274,13 +274,48 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
>  	return 0;
>  }
>  
> +/* Extract RCRB, use same function interface as cxl_find_regblock(). */
> +static int cxl_rcrb_get_comp_regs(struct pci_dev *pdev,
> +				  enum cxl_regloc_type type,
> +				  struct cxl_register_map *map)
> +{
> +	struct cxl_dport *dport;
> +	resource_size_t component_reg_phys;
> +
> +	memset(map, 0, sizeof(*map));
> +	map->dev = &pdev->dev;
> +	map->resource = CXL_RESOURCE_NONE;
> +
> +	if (type != CXL_REGLOC_RBI_COMPONENT)
> +		return -ENODEV;
> +
> +	if (!cxl_pci_find_port(pdev, &dport) || !dport->rch)
> +		return -ENXIO;

I noticed while reviewing a later patch that this pattern leaks the port
reference from cxl_pci_find_port().

I.e. this needs to do:

	port = cxl_pci_find_port(...);
	if (!port)
		return -ENXIO;
	if (!dport->rch)
		goto out;
	...
out:
	put_device(&port->dev);