Received-SPF: pass (google.com: domain of linux-kernel+bounces-104063-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <60689e0c-4e53-49c9-82a6-7c431008c177@intel.com>
Date: Fri, 15 Mar 2024 13:08:58 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH 4/6] PCI/AER: Extend RCH RAS error handling to support
 VH topology case
To: Dan Williams <dan.j.williams@intel.com>, <rrichter@amd.com>,
	<terry.bowman@amd.com>
CC: <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20240313083602.239201-1-ming4.li@intel.com>
 <20240313083602.239201-5-ming4.li@intel.com>
 <65f3b2b29cfe9_aa22294e8@dwillia2-mobl3.amr.corp.intel.com.notmuch>
 <f2dd9f36-5e69-4c6e-9640-7140a694086c@intel.com>
 <65f3c8fb458d0_aa2229435@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Content-Language: en-US
From: "Li, Ming" <ming4.li@intel.com>
Organization: Intel
In-Reply-To: <65f3c8fb458d0_aa2229435@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VWZLUGsxVzY3R1pFelhKREl6SWQ4Rnp4K0poZ21PY3ZubVdWM2VGMXlwc0JO?=
 =?utf-8?B?UktyYWFLTXhudVlLVy9nUWRHMXhPaW5rOXBZR3FzMFM1S25YTTFVTnNpbXFU?=
 =?utf-8?B?Vkt5YSsvR3Qzb3c5Q0JoWktOSUVRdFpPZ3E3dkRXbzFxNll4L09OOGJhZmVX?=
 =?utf-8?B?UEwxZE1vNVAwYUYrZGYza1lKMXphL1pPczdvbmUzNXVVNHhPN3I0aGhMYTRm?=
 =?utf-8?B?dWh3c2ZwQjV2bFFxemhtYjNmYlRXOGtXUU1WNFhxa1ZyUWFMRTF3cFY1dEtS?=
 =?utf-8?B?TkZtckRKOVhRV0JRRnpMTERaYm5WTEE2RUY5UElBZ0Q1NWhIK2hpMjRtTzFE?=
 =?utf-8?B?bnlYV2R4YmRkOUJ6VHZHdldLY2JIaEhDMGMweXErWjBtOEROL0xmcVNDUHJG?=
 =?utf-8?B?dURrOWY5WjFZR3NDQktwa3lwSXJOaW5SNElNZnZtOXRvYk4vTFJhQjBlTzg1?=
 =?utf-8?B?SUQ1MDdxVUx0QXdqMUZ2UFlVVXJEM1VZY1l6ZzN2bWpFSXZJZXJlZGhEM2U5?=
 =?utf-8?B?SzdBa05mSnhQeWI4MUpjSU1XNG16U28xLzRZY1BadExOZmRYdHZSMzhldmww?=
 =?utf-8?B?UHo4REtyR2FEYm9Gc1o4ZVBqbkFZQkVPWmhsUElGTFcxZjRiTDRTZXV3VEw3?=
 =?utf-8?B?UENZR1ZSbHdzV0RsV0FCYTM4UVNLZmlBNG9EQlozRUVuQXN5WWRWT0ZVaVlG?=
 =?utf-8?B?L0R2ZFhsbzNTQVpyWUc3K1ltQnBrdTVuaUlvVUFjR01qS3g1NTE5V2MxRXVE?=
 =?utf-8?B?c3BGSFpYbzNNaklsVyt6V0lBSzdORFQvZmxJMjI4cmpJOFErNjY4YTZkc3Rz?=
 =?utf-8?B?YTlDWGo4UXcyaWFlUDVra1A4V1pldHNpcXBXNGxaQitjQmdYQlEvTDlHMy8y?=
 =?utf-8?B?Ukk4NWozTTN2TG5EaW5kNHJIczQvZkxHWXUzQzVnTEIwaG1NK1VvcHc5eFpr?=
 =?utf-8?B?ZnlJOHlybStqMkM3cXkyQlVVdkZVVzdnQ1ZjSDdsZWM0Z1l5RnhhdjFsSmd3?=
 =?utf-8?B?VGtmSTRYMEs2TzV6M05GcGl0ZEx1ZGtPTU12S2puSU0zNkNQc3FZUEwrWFdZ?=
 =?utf-8?B?cUV5bU41VjBzR1BPS0hzanVObWREZWNnMGVEU2psTjdhOHBZYmlPWXB5M3lx?=
 =?utf-8?B?cVBJd1MrT3MxTzFpRnpaRzErUTRGeHBpSitlYjFyRFRreEFFd25Cc0Z5OWZM?=
 =?utf-8?B?Z1U5anB6TXN6NTRyMkNYVWZXbUlSVzNxU1dCalhDSXdyeGZCUjk1Q25zUXYz?=
 =?utf-8?B?SHFKQTNHaVdQV2IxTWIzZXdnZXdGS05paFNYZzNmUVRkWFE1VTA2NW5ZQnV6?=
 =?utf-8?B?T1pvMWlnQ2ZOSjZydjU2cWdENDRtMDJmVHBidFg5WFBRRm5DdmZKNTN5dDcz?=
 =?utf-8?B?TVA0TzRldTVTVmhnd1RWL0svZGg3SUNEN2ZiYkhPTVI1SVRhOTRlV1BReGRz?=
 =?utf-8?B?TU83eUl3TTRBVnMrNk1hU1FieTNrOHdiQ0p6ZWIwd0gyR1VhdUpVWGdXeHBS?=
 =?utf-8?B?aTBqRExTbGxRakpOU3dBblhPTnFYa2owVlFTN0FhazZ3WDg4SWo3WjdvalpO?=
 =?utf-8?B?VzYrQnl4djNuS0oxVFNxVUlkbmFRbTRIa1RtZjNjSHozYXl1R0ZIV2JsblQ1?=
 =?utf-8?B?SVlwdHZZUlF3Mm9NQTVSelRWZE5OemxnbFQzNW1naDhKQWdzRWtuMHcvY3V2?=
 =?utf-8?B?MWRYNmdwVkhyQmpjQjZrTVhRWDhWcWFVVTJFbS9hOEdUMlhiOWRYWmFCaHMv?=
 =?utf-8?B?c1NFRGFQelhGaFVORkhXcTdqMEJTSUU3c3Qyejg1Wk5QZzhZTTFZaDhxc0xL?=
 =?utf-8?B?NkwxQnJpN1N2R0p1SVptbkg2WGtMdTZVZ0NPMHpobG91WXMvZjllNXp2Q05I?=
 =?utf-8?B?R01MQktSVjFxb084Q0VMRTUrRHlVUUpYSTdxSTRSUDdYUmpBVFJvWHJlK0pI?=
 =?utf-8?B?Y0hvSkVpNkR0K2ZjVFRMVEN5S2ZtNkgzd0xNeGk5ZWxzdE9tdjNRbDNCVjFu?=
 =?utf-8?B?aVVRREJVdGVpUmNjVXdzYS9SVXZvdEZUNU56Qit0Q0Z6bFZGVzdEVURETjZU?=
 =?utf-8?B?YWMwc3IyOEN3bVhFMFN5S3dyK3FPem9YSjh5TE5ONk54YitvWU5UTEZRVVNC?=
 =?utf-8?Q?tOHF47XpcvNeZit63itMq5wvx?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 33b1a15a-9cf3-48a5-9ed4-08dc44ae098a
X-MS-Exchange-CrossTenant-AuthSource: IA1PR11MB7200.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Mar 2024 05:09:05.8096
 (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: B5DYe6/T5JE/RbuEvt6KFBkH6PCYyRFGOo8p1t457fsFQph3U9iEuC1tp8eMf2fqzrc9totWiUL1kUPrWUB8ow==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB6373
X-OriginatorOrg: intel.com

On 3/15/2024 12:05 PM, Dan Williams wrote:
> Li, Ming wrote:
>> On 3/15/2024 10:30 AM, Dan Williams wrote:
>>> Li Ming wrote:
>>>> When RCEC captures CXL.cachemem protocol errors detected by CXL root
>>>> port, the recommendation from CXL r3.1 9.18.1.5 is :
>>>>
>>>> 	"Probe all CXL Downstream Ports and determine whether they have logged an
>>>> 	error in the CXL.io or CXL.cachemem status registers."
>>>>
>>>> The flow is similar with RCH RAS error handling, so reuse it to support
>>>> above case.
>>>>
>>>> Signed-off-by: Li Ming <ming4.li@intel.com>
>>>> ---
>>>>  drivers/pci/pcie/aer.c | 20 ++++++++++++--------
>>>>  1 file changed, 12 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
>>>> index 364c74e47273..79bfa5fb78f4 100644
>>>> --- a/drivers/pci/pcie/aer.c
>>>> +++ b/drivers/pci/pcie/aer.c
>>>> @@ -996,11 +996,15 @@ static bool is_internal_error(struct aer_err_info *info)
>>>>  	return info->status & PCI_ERR_UNC_INTN;
>>>>  }
>>>>  
>>>> -static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
>>>> +static int cxl_handle_error_iter(struct pci_dev *dev, void *data)
>>>>  {
>>>>  	struct aer_err_info *info = (struct aer_err_info *)data;
>>>>  	const struct pci_error_handlers *err_handler;
>>>>  
>>>> +	/* Skip the RCiEP devices not associating with RCEC */
>>>> +	if ((pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END) &&
>>>> +	    !dev->rcec)
>>>> +		return 0;
>>>>  	if (!is_cxl_mem_dev(dev) || !cxl_error_is_native(dev))
>>>>  		return 0;
>>>
>>> is_cxl_mem_dev(dev) will always be false in the VH case, so how does
>>> this change help the VH case?
>>
>> Hi Dan,
>>
>> I think it won't be false if the CXL memory device is an endpoint.
>> pcie_walk_rcec_all() will walk all pci_dev in RCEC assocaited bus ranges. So these two checkings can help us to filter:
>> 1. CXL memory device is an RCiEP associated with RCEC in the RCH case
>> 2. CXL memory device is not an RCiEP, so it should be an endpoint in the VH case.
> 
> It will be an endpoint, but I though cxl_handle_error_iter() is only
> called for RCIEPs and RPs that are share a bus range with the RCEC. The
> endpoint in the VH case is downstream of the RP.
> 
> I had been assuming that pci_walk_bus() limits itself to buses within
> the Root Complex however it descends the entire bus hierarchy so this
> implementation will walk the entire topology on all root ports
> associated with the RCEC looking for any CXL device. That feels wrong.
> 
> I would expect that this limits it self to only finding root ports and
> then only proceeding if that root port has a directly attached CXL
> device.
> 
Got it, will change it in v2, thank you.

> Note, when you send a v2 of this RFC be sure to copy linux-pci for these
> core changes to PCI error handling.
Sure, I made a mistake here.