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: <c03b8d66-65d9-40fc-dd0a-1b8154e24f6e@amd.com>
Date:   Tue, 28 Mar 2023 08:41:38 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v2 4/5] cxl/pci: Forward RCH downstream port-detected
 errors to the CXL.mem dev handler
Content-Language: en-US
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     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, rrichter@amd.com,
        linux-kernel@vger.kernel.org, bhelgaas@google.com
References: <20230324223656.GA2660301@bhelgaas>
From:   Terry Bowman <Terry.Bowman@amd.com>
In-Reply-To: <20230324223656.GA2660301@bhelgaas>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TFlFc2p3ZXhxZ3VUZ0lsd0VPZ01aQUJtb256TkpsT3ZyeFRIcEYrUkZwUU5E?=
 =?utf-8?B?elZ4TEVydFhDUTFXeGZZdTFaRTAwUWVaQTAzd1BBSlNlUU8zR1N5RDJrVjZN?=
 =?utf-8?B?MUhVRmt0M28vVENvdlV2b051MjB4dmF4OGpOaGRlUi9HRlFqQ3RITytyYjVT?=
 =?utf-8?B?bG8yY0lJdFNTZ1RkekwxS2ZnZUtuOVpPZW5UYUVyWjVkcXIwMUZPdVExRGJR?=
 =?utf-8?B?eUVpazhHK3VpWHBYLy9KSUhWZFFmeWpGYTMwUlJheDdlWDlVYkNRa3BnV3cw?=
 =?utf-8?B?bW5EeEsvREdVVXdRbkh2a0RDTi9tVDBFeDJObjNLTU03SEhsUWl4UmNOSW5m?=
 =?utf-8?B?LzlXZHlQUVc1MEZXMitXdXVJSDlFQnVNYWx6MGEzU25ydFRIR0dtYTl6Uno3?=
 =?utf-8?B?cFVXN3FqOEVqNDJDQitJczRQdFc0SlNYUnZ5bkpadC9nK0tzLzF1M2pkSzFI?=
 =?utf-8?B?eGpXbVZrTE1hK3I4VVQ5V0h6THhDaFgzSFBMS2FaUGg4ZUhGaWk0b1ZpMHdh?=
 =?utf-8?B?QW1yUjBuem93U2VyaXoxTjFTOFcxNUc0by9NT29WREpwblVIYVZUOGYwam1o?=
 =?utf-8?B?VWFHTkZTQmZLclA0WW5tdzNYNTJFdTZSUGZmZFhzY3Y0Vk1oMHpmeEZuYUtD?=
 =?utf-8?B?S04yNENzbVc3K1dxOXVPV2w3UVdQdHhKRm5nV3FEQXFKUjhWVUpKekJjTnp5?=
 =?utf-8?B?L1RzUENjNjdzSUh6c21nTDc0Rmx5QXNHSUJxaTJHMm95TnVpVEpmVENkcDFN?=
 =?utf-8?B?MHNqSEtuclJ4WkVqUUJPaERGM1oxWVM2OFRJbnFnL21TcDh3b3dzMWpkdHh5?=
 =?utf-8?B?bUlMNm8veEpNdE5Kckg4N1psNlIvcldTRCtRek5vMFRQVVVzVktxRHg4c2dF?=
 =?utf-8?B?OVdXbXV4a1BkWTZKM0VuRGg2ZmFSWWt5MzMzZXlwYVpBTXNoeWpqOTkra2F4?=
 =?utf-8?B?RndrVW1ub1I3aEMvZlVXaUZIaW5ISmtkdFNMSkhDNENNamkvMUw3Qm4xT1JX?=
 =?utf-8?B?ZDdxOXFiQTd4b21SWElBMkxOQmpnOUdrZ2ZVTk9EZWRNdjY4Zm1oSmsvclN0?=
 =?utf-8?B?Si9WSEZzeU5jbXluajU3UmZNZU9laVFTQW4rcHVNMmt3Z2JwUEVLWEdZZTJo?=
 =?utf-8?B?MVRIR2JucWJ6cUlmMWw5MmdleDN3WDcwdGNOaFJOcWQ5M1NrNElOaFcyMGJt?=
 =?utf-8?B?VCtaTmxqZjdVMlNhTnJmSXBvd3JISUtCZXFlWFBRTjlvOUJaK1M3UVZ0L0RX?=
 =?utf-8?B?SmI4OGRxN25CSlE3Sm9sK0lhTkR4a1NlVlA1V1ZodGpyOXVJdktRcHhuYkFu?=
 =?utf-8?B?ejlYeFdlSksxV3pkbjBlNmw0ZjFXK1g0SHYvS3B5RGtHbHdLUXI2WGFvUzAw?=
 =?utf-8?B?T3E1UC9uS0NwN3BybXRXVTZ6bGNlRjE2NmFia01aTmxXMGtuOFdBUmpGQnVG?=
 =?utf-8?B?RmRsbVAzNzVZT1VGeXZxZTQ2WTc2b3d3Nmh2T0ZLMUFSNmcvZDNhcU1KS1pk?=
 =?utf-8?B?L1JBYloyZEs3QkNqTGFUOGNMaGNwNVVPYzRXSUMzelNieFpKSERSZENLS1Ni?=
 =?utf-8?B?QXJnclpxR0FzRmdQZ0I3Mm1IbFZuMXhhL3VJT05RSlY2YXRSL2NnTkEwWmo4?=
 =?utf-8?B?Q20wWnYwQStoV28zK2VKdWszejlkWFhkZkZPUEZYQ2VFWVNYMjZOeUVpcElv?=
 =?utf-8?B?UUhSbmFwcWZsblZ0UkdDOVBBK0Y2YnBNakk2V2RrdGxvaVAyOFpKOHJRaUtE?=
 =?utf-8?B?K0Z1OXVuQ08yMXAzSzJpTjI1MGh4SnVjbWF3b0VRVlNmMWkwbmZiU0ZmMUdD?=
 =?utf-8?B?N1BkOWozazBJWVgwL283bmlTOVlDLzN4TEVxczNUUGNzbVlCMmdmTSsycU5r?=
 =?utf-8?B?RURSUnlnVFpsNkN0QXRQWkp1SS9URER3OVN5ckVrL3I4KzZSVTEvN1hoQ3BJ?=
 =?utf-8?B?UFZUK0Q1Q29xanRKR0g1WWVPblNnL2k0aWJGYTZ4VmJwazIvZnNRVERpT08y?=
 =?utf-8?B?YytNMC9YMFRIdmE4bk0vUy93V0dDcnNtT29NS0pKYmpRZ1pwNTBROVJsd3Uz?=
 =?utf-8?B?T2RWL0o1QjZYMlRKVHZ2L3ljZ2hiRFc1VVZjc1hnRzlJRXFtQ2ZXRWR5b0JW?=
 =?utf-8?Q?aVmDZlBb2SW3bZ9HQYbMa725r?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a788f7f8-3b95-44a0-f64a-08db2f922950
X-MS-Exchange-CrossTenant-AuthSource: DS0PR12MB6390.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Mar 2023 13:41:41.1899
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: epRbA1GRyCxzpfQpQZa7+mFW6ByeliSGHGIJp7nSguFhLo4dVRpZbho2nVpdp+ClyNOBmbz0whxCkyXw165SKw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR12MB5139
X-Spam-Status: No, score=-0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE
        autolearn=unavailable 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

Hi Bjorn,

On 3/24/23 17:36, Bjorn Helgaas wrote:
> I'd call this a "PCI/AER: ..." patch since that's where all the
> changes are.
> 
> On Thu, Mar 23, 2023 at 04:38:07PM -0500, Terry Bowman wrote:
>> From: Robert Richter <rrichter@amd.com>
>>
>> In RCD mode a CXL device (RCD) is exposed as an RCiEP, but CXL
>> downstream and upstream ports are not enumerated and not visible in
>> the PCIe hierarchy. Protocol and link errors are sent to an RCEC.
> 
> "RCD" isn't a common term in drivers/pci; can you expand it once here?
> 
>> Now, RCH downstream port-detected errors are signaled as internal AER
>> errors (UIE/CIE) with the RCEC's source ID. A CXL handler must then
> 
> Similarly, "UIE" and "CIE" are new to drivers/pci; can you expand them
> before using?  I assume Uncorrectable Internal Error (UIE) and
> Corrected Internal Error (CIE)?  (Annoying that the PCIe spec uses
> "Correctable" in general, but "Corrected" for Internal Errors.)
> 
>> inspect the error status in various CXL registers residing in the
>> dport's component register space (CXL RAS cap) or the dport's RCRB
>> (AER ext cap). [1]
>>
>> This patch connects errors showing up in the RCEC's error handler with
> 
> "Connect errors ..." (we already know this text applies to *this
> patch*).
> 
>> the CXL subsystem. Implement this by forwarding the error to all CXL
>> devices below the RCEC. Since the entire CXL device is controlled only
>> using PCIe Configuration Space of device 0, Function 0, only pass it
>> there [2]. These devices have the Memory Device class code set
>> (PCI_CLASS_MEMORY_CXL, 502h) and the existing cxl_pci driver can
>> implement the handler.
> 
>> The CXL device driver is then responsible to
>> enable error reporting in the RCEC's AER cap
> 
> I don't know exactly what you mean by "error reporting in the RCEC's
> AER cap", but IIUC, for non-Root Port devices, generation of ERR_COR/
> ERR_NONFATAL/ERR_FATAL messages is controlled by the Device Control
> register and should already be enabled by pci_aer_init().
> 
> Maybe you mean setting AER mask/severity specifically for Internal
> Errors?  I'm hoping to get as much of AER management as we can in the
> PCI core and out of drivers, so maybe we need a new PCI interface to
> do that.
> 
> In any event, I assume this sort of configuration would be an
> enumeration-time thing, while *this* patch is a run-time thing, so
> maybe this information belongs with a different patch?
> 
>> (esp. CIE and UIE) and to
>> inspect the dport's CXL registers in addition (CXL RAS cap and AER ext
>> cap).
>>
>> The reason for choosing this implementation is that a CXL RCEC device
>> is bound to the AER port driver, but the driver does not allow it to
>> register a custom specific handler to support CXL. Connecting the RCEC
>> hard-wired with a CXL handler does not work, as the CXL subsystem
>> might not be present all the time. The alternative to add an
>> implementation to the portdrv to allow the registration of a custom
>> RCEC error handler isn't worth doing it as CXL would be its only user.
>> Instead, just check for an CXL RCEC and pass it down to the connected
>> CXL device's error handler.
>>
>> [1] CXL 3.0 spec, 12.2.1.1 RCH Downstream Port-detected Errors
>> [2] CXL 3.0 spec, 8.1.3 PCIe DVSEC for CXL Devices
>>
>> Co-developed-by: Terry Bowman <terry.bowman@amd.com>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> Signed-off-by: Robert Richter <rrichter@amd.com>
> 
> Since you're sending this patch (Terry) your Signed-off-by should be
> last.
> 

I'll move my Signed-off-by to the last.

Regards,
Terry