Received-SPF: pass (google.com: domain of linux-kernel+bounces-27437-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <2223a0c6-e627-4608-88ea-230fd1b5c507@nvidia.com>
Date: Tue, 16 Jan 2024 19:24:01 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH V1] PCI: Clear errors logged in Secondary Status Register
Content-Language: en-US
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: bhelgaas@google.com, linux-pci@vger.kernel.org,
 linux-kernel@vger.kernel.org, treding@nvidia.com, jonathanh@nvidia.com,
 kthota@nvidia.com, mmaddireddy@nvidia.com, sagar.tv@gmail.com
References: <20240112170622.GA2272469@bhelgaas>
From: Vidya Sagar <vidyas@nvidia.com>
In-Reply-To: <20240112170622.GA2272469@bhelgaas>
Content-Type: text/plain; charset=UTF-8; format=flowed
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?c3VBS2p2NWVKVVhYWVd3SnB1K0VIbHRwTmZXZGI1SXRYMm1BZFd2bitMN0hS?=
 =?utf-8?B?RjJta1BMcElDK0d4WnF5RHVwL081a1NGNkVmZWxabFpTRnAwRStnVlQ3V3FH?=
 =?utf-8?B?c0VBU2h0a0QrVVBRSzFObGdwVDQwSEJpYXBtQUdFTVRCVjFMN2RNbjdDV0xT?=
 =?utf-8?B?UWV3Y1YwRy9QU24wQzRSY0oyNzQveDBnYkJSZ0Rwdk1wb294dVBBQ0srZEEv?=
 =?utf-8?B?VkJrSXZPbFkvMFZTZk1aSjVSa3M3WTJ1WFMvbVd2NjhRUndrTjd3UU9iWlVv?=
 =?utf-8?B?V2pQYVpJLzY4Y2JzV1kvWGVneitObEpTME5UNXJmVnoxMi95aDJmMWV6NXdC?=
 =?utf-8?B?S3hXYU9XcWhuYlNseUtSc0Y0TzJoWHVub3dRbVZLY0cxY2lRajZNL3VYam10?=
 =?utf-8?B?N1BaQzV4Y3hreEkxd1pkblE5WE9PakhlOFJiMU13eHdKNFBkbUFIN3Z1dGlM?=
 =?utf-8?B?OVR1NEVvQVRyNS9ReDBjaWE4RUxRbkU4UTFhWWRFRUZIaFlxK1VpYUpaSHRy?=
 =?utf-8?B?Ylp1aGpFTE43YVlYZkJDUzVJWHNvajZSSitBQmw1TGtqTHZMUS9Za2ZyN0Iz?=
 =?utf-8?B?cmlKRytNNWlubFZidjNpSmRQUStZYzhoeE1jZzlKRFJUVkdRdDRtUFBMalFJ?=
 =?utf-8?B?eURKRjJ2eGQyTVl1eDZSUVR5d1pHSmh0ZjZsYXVwUWhqM1BqcGl5cUlnRTBn?=
 =?utf-8?B?djhYOWlackd3MG80bXByZFhPTVZrVVU1MmhvZW9zU1grdDJJN3dCM1dxU202?=
 =?utf-8?B?ekt1azczYm5QUjRyd3VPQmNkdjN4ZzFVbHVWdjdsd2FoeStucnRjWCtmb3Z6?=
 =?utf-8?B?SHVhZFJmTE5KOWNwVURxdkdKckQ1ajZsb1ZUUktiTjFWV1VMOU9Bd0pSSHNt?=
 =?utf-8?B?dU83QllQdjd3bFM4UkdqbGtvRS9vaWhZMk5udkJkaDZ0T2VPRUl0MTBBQ25n?=
 =?utf-8?B?NHptRnRWL21vTnp4bkNpL1lzTVlTS1FkcWVTSjhIT09QYnFQSmluekhWNkxu?=
 =?utf-8?B?QUlEVWlQdll1YjJQVmJrTlROV0YrZnp4KzJGeHB0OGh5c2hLai8xamRXcXY1?=
 =?utf-8?B?a2MvRUtuQmwrNlB4c1NlUXlTWDhRaFZzU3oraEdhem42b0RrdVoxTEErc0xh?=
 =?utf-8?B?WHVWV2dyeFRIRU4vWWlaZ2NSS3lVMVR1SUdISmJtY1d6bE55VVBTZkxZOHRO?=
 =?utf-8?B?MnRRWnBHOHRZU2l1UmVjTENmT2c5MkIwd3doMkNqazFqdEl1S1lQOVNlMnVD?=
 =?utf-8?B?MG1sdWVNME1PMSs4anNFWUQ1SGNvODJFVm9kU2F3ekwrSlh4dDhvWUs1Visx?=
 =?utf-8?B?OHRzYlNDK3pLQlJNWFc2SFFIcUVHRlhTeVRPbC9JNWsxa3VHT3N6eTBuS3Bz?=
 =?utf-8?B?MHV3T251ZWhFWkdlVEdsZ0NNTzNIajM1SzZaUXAwU3hoeDEyd3hTd0YyM3NU?=
 =?utf-8?B?UG9NMU9BcS9qenl1ZmlsejNaWlBzQXovNnk1b3JRUWg5bk1La016WHp5T1Qv?=
 =?utf-8?B?VzdxcFJYSFVCVG1FWGIyVmtIMzlORmFaeXlkZElHZitNcDRPaFplZ0NSS0g1?=
 =?utf-8?B?OFpWUlptMHdQWnl5ZGhNL01COFdsZmlsR3drRitXNzdoQ3kvMDl2bXdWVWdM?=
 =?utf-8?B?NEZEQjArVnl4THp4T3ZHS21URU9zMFJ1Y25VY0tMZTNnT3dqL3FNY3E3NFlN?=
 =?utf-8?B?ZXJMWWJmUndodklSRGMva1FHMXRqTE5GTTFJbjdXRWd6eFV2RjdQNE5UeTJS?=
 =?utf-8?B?TEZHRE1RQnVGN0J0K2lYQkxaREFBVWViMVcrTW1hVi96c1FORmsxa3YrcFVV?=
 =?utf-8?B?SkhJR3pvWXlVa01PclJqZkRCaEppT0ZMZnpYamZYNnI4S21UMGYrVDFTZWN5?=
 =?utf-8?B?SWZmUzk0THh4bklkUGFMdW03MDdLKzl3V3prSUxIc2tBNmJ5NVU3MXBOajF4?=
 =?utf-8?B?dnVXWUVPbWdkU2pmYm0zRWd5L2JqODhJV1hCdUduVXJDY3Bvd2lUQ2M3bUk4?=
 =?utf-8?B?TnRqakZYZCsyazBoOEJtU09jRHJ1aW1DbTVJbTBRTnl6RElEbG1RdlczelRr?=
 =?utf-8?B?OEpXTXk5ZHlRYUxnK3ljREVEZ2xOYkpGZDFIalhTTEZZcG5YaUd4cTlzT1Jm?=
 =?utf-8?Q?/UD/oBJhSmIMGPf8eNsjnG5YU?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ca544864-8138-4364-e1ac-08dc169a9da0
X-MS-Exchange-CrossTenant-AuthSource: PH8PR12MB6674.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jan 2024 13:54:10.8620
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: X0j+KzBnAYUQqxa+vOzYeQKsrdQ/3Kdf6dt28zNuyZPlLJz1IDHvCbj1J4AfNVW5OiEaHjnaGkwy/nbhgYHw/g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB7211


On 1/12/2024 10:36 PM, Bjorn Helgaas wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Thu, Jan 04, 2024 at 07:02:29AM +0530, Vidya Sagar wrote:
>> If a downstream port has a PCIe switch connected to it, the enumeration
>> process leaves the 'Received Master Abort' bit set in the Secondary
>> Status Register of the downstream port because of the Unsupported
>> Requests (URs) take place in the downstream hierarchy. Since the
>> ownership of Secondary Status Register always lies with the OS including
>> systems with Firmware-First approach for error handling[1], clear the
>> error status bits in the Secondary Status Register post enumeration.
> 
> I would expect these URs to happen when enumerating below *all* PCIe
> Root Ports (not just when switches are present), and Master Aborts
> should happen in conventional PCI.
Agree.
There was a misunderstanding from my side because of which I had said
that the 'Received Master Abort' bit gets set only if there is a PCIe
switch connected downstream. I'll correct it in my next patch.

> 
> Similarly, I don't think Firmware-First is relevant here.  Only the
> fact that the OS owns PCI_SEC_STATUS because there's no mechanism to
> negotiate for platform ownership of it.
I mentioned about Firmware-First as a continuation to the discussion we
had in [1]. But, agree that, this being a standalone patch, there is no
need to mentioned about Firmware-First flow.

> 
> We're in the merge window right now, so we'll start merging v6.9
> material after v6.8-rc1 is tagged.
> 
>> [1] https://lore.kernel.org/all/1fb9d746-0695-4d19-af98-f442f31cd464@nvidia.com/T/
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>>   drivers/pci/probe.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 43159965e09e..edf8202465d8 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1470,6 +1470,9 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>>        }
>>
>>   out:
>> +     /* Clear errors in the Secondary Status Register */
>> +     pci_write_config_word(dev, PCI_SEC_STATUS, 0xffff);
>> +
>>        pci_write_config_word(dev, PCI_BRIDGE_CONTROL, bctl);
>>
>>        pm_runtime_put(&dev->dev);
>> --
>> 2.25.1
>>