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;
Message-ID: <67068bf5-c96c-6de1-d6cb-9ad4dd524701@intel.com>
Date:   Thu, 22 Jun 2023 17:00:54 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Betterbird/102.11.1
Subject: Re: [PATCH v6 15/27] cxl/port: Store the port's Component Register
 mappings in struct cxl_port
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>,
        <Jonathan.Cameron@huawei.com>, <linux-cxl@vger.kernel.org>
CC:     <rrichter@amd.com>, <linux-kernel@vger.kernel.org>,
        <bhelgaas@google.com>
References: <20230622035126.4130151-1-terry.bowman@amd.com>
 <20230622035126.4130151-16-terry.bowman@amd.com>
Content-Language: en-US
From:   Dave Jiang <dave.jiang@intel.com>
In-Reply-To: <20230622035126.4130151-16-terry.bowman@amd.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?b25JRHZqZGF5Um9DL25CT1VRUzJiTC9wY1hDVWZkV2dzVVVGWTA0U3JLR0pv?=
 =?utf-8?B?b21ZL1BmaUxKd3lucDVpekJ3UWl5T2NVcnZFMWd3blFPZTFQd0RFNWROc0lx?=
 =?utf-8?B?ODdWQ0ZETEw0bDJsMUc2VFkzWnlBSEVlZ1lVbmxiUHNRamlNV2cxQkg1UXJk?=
 =?utf-8?B?K2Vjejd6bkVHNmQ5ZXFWdXNhclF5Rm9VOFhBY2Q4U3owcklXTzZ0Tkh2V1di?=
 =?utf-8?B?NGE1Z2VDWTFYcHFlcmZVTXIrcktFdGlBTU92cE5PMlFMay9uY3pXejg3Q0NW?=
 =?utf-8?B?c0ZBWEN1YmszQW9yOXNxWEd3WVBJanVWc1lXeS9UbEMxZVRpTGxpM0lNNmRD?=
 =?utf-8?B?L1BteWZ2NFdJYXdFdUJLVmdQbEp6NDBrM2c4MS9jdHRPWFdvQ29qa0RWZjFp?=
 =?utf-8?B?VllpOEg1YjFTblhmY25KWVQwcE13bDNBazlDOUl4VVhRNDRYUGlSdkhpZS9I?=
 =?utf-8?B?MG1RVWFRaUVKOHZ5MUJUN3VkSjhGNVRhNkZ1ZnlhcmdBKzNsZkZwQUlOWVRE?=
 =?utf-8?B?bUloby92akhOTE1KeWt1NWdON24wc0g3dkQ2UHpqUUwyUEI5TGFBSzJSYzNC?=
 =?utf-8?B?KzhIRkRqZUFhOGE3dHFZcS9sSXozV3JsQ29Ma0hrTmptVWtwbC9kZlVpeEJR?=
 =?utf-8?B?NmtpOHU0UFdLNnZrNXZqQ1BBZ0d3YWhKSlF2RTcxQnk1bTZEUVdyUVBoOGlx?=
 =?utf-8?B?ZlFPVGJBbkRwOWQxVUtKenRpTGw1NGkvOXhRNlFkekorQklZN1pjRGN3RXVn?=
 =?utf-8?B?WDdsd1pQVysrQVlTNUp4MHRxbVZISTBySGNrVDV0UUFtV2EvSXRIVFAvT1J6?=
 =?utf-8?B?Mnc0MExyT0RCUEZ2a2U5Ym5NTnNCV1hpcGpiUnNkU3J5S0R2U3dwOG9YSk1o?=
 =?utf-8?B?WDh4clM3VE1SdDdrelpreVc3T1RUTFIxVEMxdXBvbDJScjZlb3NUUExrRGJy?=
 =?utf-8?B?Z3Z1N3NBaHJGS04xNTRNTmRvSzVuRzF6YUt3ODJaMTRydEg5dnl5RzI0YUJE?=
 =?utf-8?B?ZEJsWVl4TG1Rd3BlRmJML2dabitRMWRYRDlIRzF5d3l6UVA3NnBIdXdyL2VF?=
 =?utf-8?B?K28vazlsTVdVOVl0azk4K082OE1VbXl1bHlTaGJacmdVNUlxMTBNZEc0QU9k?=
 =?utf-8?B?My9SYmkzQzFtb2FGMm9vRitDOW9ZWHdhczJBR0V0dmZuY1lLZE1HbmJ5aVFk?=
 =?utf-8?B?VzdNNXI5dndjUDZUeENWSVY3SE1RMlhIdW1KK0srRkxmV1pBQVpNRytpQ1hE?=
 =?utf-8?B?cG4xbkhNdWtvMEdRQ0haU3ZoZkU3dUZjNXZVb3pvWjF0MXVZYVBhZE0yYWkz?=
 =?utf-8?B?enFpYXV4c1JIaVB6WVo0b25JeDFPRGE1REVrUU50Q0FQSWo4eHJuMzJkNVI0?=
 =?utf-8?B?YTBzTm9iWXA5MndqUmRTYkNBbmdFallBNWtmbDc4R0huc20yUld5RnlxZXR0?=
 =?utf-8?B?Y1lNdWlqenJnRjgxNmlWWGtidmlpQUZlVFZ5QVJjcUJFSVhvZWZ4VUVwMjJS?=
 =?utf-8?B?NUVPY2tHQ3NoMWljbk1MQkY5eDJCOFlodk5RaFI1UjF3My9lc1dseHdWQ1My?=
 =?utf-8?B?OGJpWEI2MjlQcXpTT2d4SWdndmhPNko1WGdYZkp4cTgyL2E4QjNQNXBOWkt4?=
 =?utf-8?B?S21qbTZESk5KdGw0YWczWVhyNXFOazdjVTFTVGFHWlcrUzd6U3BmQnR4WWE4?=
 =?utf-8?B?aktZWk53RjhCbFRsQS9mRkRzRlZVZXV0MGFwNjI2YWJvdTZpbEMyUWJpYXhv?=
 =?utf-8?B?bTFGaFRobmVkUDh3dTA3T3QwMWhqUXZranV0RjI5VXFzVnQ2YXZGa05nczFk?=
 =?utf-8?B?SFVYZmVuZXJuZDd0TDhzWXdZVG9jQUtQeFVEWUEzSTRYMG9lUG1Sak5ZcnVz?=
 =?utf-8?B?eVFBeC93NEFwclBPYy9zdTdRVWFIbTJ5eVdnZmJhNUVvWitNalJsTW1UNnhG?=
 =?utf-8?B?R21pUFhYK0k1ZDRXU1l3Y2FRUGhKeGRmZmcvOGpzQnFzdnBkaHkrK0hYbkZB?=
 =?utf-8?B?Wk8zK1pSOHdxMktwRHM3ZXg4SUlLYkFtWU9Cbk45d0hwRUdEaWNxdGZrZk9P?=
 =?utf-8?B?YTY3N0MrbGNXVVNqVGF0YlI0Qmc1NGF6UTc2cTBYd2trNHV6K3UwMEE1T0Nq?=
 =?utf-8?B?M1huL014ek8vMmFaakFKVG9HcU5KV1NVZUxFY0lVU0l0dEtsUVREMFkxVlRS?=
 =?utf-8?B?c0E9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a63b581a-af03-4dc1-348f-08db737cecbe
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5984.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jun 2023 00:00:58.9888
 (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: /HKFCv6ohsU4EE95lH3ZNTqJywxShKwS54sGVhSuYeHlMTVagtHJDfqF/ZPSTAecwwngPtebOedLe2o7180ayw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB6654
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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


On 6/21/23 20:51, Terry Bowman wrote:
> From: Robert Richter <rrichter@amd.com>
> 
> CXL capabilities are stored in the Component Registers. To use them,
> the specific I/O ranges of the capabilities must be determined by
> probing the registers. For this, the whole Component Register range
> needs to be mapped temporarily to detect the offset and length of a
> capability range.
> 
> In order to use more than one capability of a component (e.g. RAS and
> HDM) the Component Register are probed and its mappings created
> multiple times. This also causes overlapping I/O ranges as the whole
> Component Register range must be mapped again while a capability's I/O
> range is already mapped.
> 
> Different capabilities cannot be setup at the same time. E.g. the RAS
> capability must be made available as soon as the PCI driver is bound,
> the HDM decoder is setup later during port enumeration. Moreover,
> during early setup it is still unknown if a certain capability is
> needed. A central capability setup is therefore not possible,
> capabilities must be individually enabled once needed during
> initialization.
> 
> To avoid a duplicate register probe and overlapping I/O mappings, only
> probe the Component Registers one time and store the Component
> Register mapping in struct port. The stored mappings can be used later
> to iomap the capability register range when enabling the capability,
> which will be implemented in a follow-on patch.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>   drivers/cxl/core/port.c | 26 ++++++++++++++++++++++++++
>   drivers/cxl/cxl.h       |  2 ++
>   2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 679226023f0c..2e239fd33df9 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -688,6 +688,28 @@ static struct cxl_port *cxl_port_alloc(struct device *uport_dev,
>   	return ERR_PTR(rc);
>   }
>   
> +static int cxl_setup_comp_regs(struct device *dev, struct cxl_register_map *map,
> +			       resource_size_t component_reg_phys)
> +{
> +	if (component_reg_phys == CXL_RESOURCE_NONE)
> +		return 0;
> +
> +	memset(map, 0, sizeof(*map));
> +	map->dev = dev;
> +	map->reg_type = CXL_REGLOC_RBI_COMPONENT;
> +	map->resource = component_reg_phys;
> +	map->max_size = CXL_COMPONENT_REG_BLOCK_SIZE;
> +
> +	return cxl_setup_regs(map);
> +}
> +
> +static inline int cxl_port_setup_regs(struct cxl_port *port,
> +				      resource_size_t component_reg_phys)
> +{
> +	return cxl_setup_comp_regs(&port->dev, &port->comp_map,
> +				   component_reg_phys);
> +}
> +
>   static struct cxl_port *__devm_cxl_add_port(struct device *host,
>   					    struct device *uport_dev,
>   					    resource_size_t component_reg_phys,
> @@ -711,6 +733,10 @@ static struct cxl_port *__devm_cxl_add_port(struct device *host,
>   	if (rc)
>   		goto err;
>   
> +	rc = cxl_port_setup_regs(port, component_reg_phys);
> +	if (rc)
> +		goto err;
> +
>   	rc = device_add(dev);
>   	if (rc)
>   		goto err;
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index fe95f08acb69..37fa5b565362 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -547,6 +547,7 @@ struct cxl_dax_region {
>    * @regions: cxl_region_ref instances, regions mapped by this port
>    * @parent_dport: dport that points to this port in the parent
>    * @decoder_ida: allocator for decoder ids
> + * @comp_map: component register capability mappings
>    * @nr_dports: number of entries in @dports
>    * @hdm_end: track last allocated HDM decoder instance for allocation ordering
>    * @commit_end: cursor to track highest committed decoder for commit ordering
> @@ -566,6 +567,7 @@ struct cxl_port {
>   	struct xarray regions;
>   	struct cxl_dport *parent_dport;
>   	struct ida decoder_ida;
> +	struct cxl_register_map comp_map;
>   	int nr_dports;
>   	int hdm_end;
>   	int commit_end;