Received-SPF: pass (google.com: domain of linux-kernel+bounces-72308-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <c09ae4cf-8004-4c64-80f8-47dd1c014158@nvidia.com>
Date: Tue, 20 Feb 2024 09:37:14 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH V3] PCI/ASPM: Update saved buffers with latest ASPM
Content-Language: en-US
To: bhelgaas@google.com, macro@orcam.me.uk, ajayagarwal@google.com,
 ilpo.jarvinen@linux.intel.com, sathyanarayanan.kuppuswamy@linux.intel.com,
 hkallweit1@gmail.com, michael.a.bottini@linux.intel.com,
 johan+linaro@kernel.org
Cc: 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: <20240103103501.2428197-1-vidyas@nvidia.com>
 <20240108124248.1552420-1-vidyas@nvidia.com>
From: Vidya Sagar <vidyas@nvidia.com>
In-Reply-To: <20240108124248.1552420-1-vidyas@nvidia.com>
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?alhBTFIzaXlkVEdWK3RMckJsLzh6bDU5eUM4S3R3c3pkYndNYWxUTi9vZ29o?=
 =?utf-8?B?TnhmT0pUNkdiYy9FYldIcDRXRlJHbXhyS0IyMThJL3YzUmtmb2pvRFMwVDg1?=
 =?utf-8?B?WTlud3NIYVZicVhSRDQvNmdoUmh1VUY1amtxejkxbWxKNXhtYm5pYWZ1S0Vt?=
 =?utf-8?B?RHpVQ3hGcFpDdHltVGxZL0ZRT0hsZnJITmZWTmtRQnJuNzFqQ095aURocUZD?=
 =?utf-8?B?WmY4VnZXOUlzdTR1WWtRUXp4WVN1cGptRWZzWmp5OTYyMHdpcUZSS2FCY2xQ?=
 =?utf-8?B?OFVrOTlSMXNJbldwTXNuVmVxaFlOMGYwbVdhSTVoSHVqT2tmbFdCcHF6WEVt?=
 =?utf-8?B?ekFFcWdTcWFLT1hSZ0xidGRkdUg5Q2M2UUxZazU1SytKRGxPRlBQUkVYdkto?=
 =?utf-8?B?eUFaUVJrQ05yemNXUGlmbjV4aHYwRG1nQmJVMjVqdGNmVmg0L3B4bGJWcDY0?=
 =?utf-8?B?UDhEeXhJV2dXMEJDK3lFZU1CTzVVUEtlcTE1UDJ5UnVPdXE4QlpDS251WUlh?=
 =?utf-8?B?Mm5iNS9saURERUlPQU9yWE1hdUZPOEdvcVdnc0xPVmFrS2JtdVRaSElvRFVy?=
 =?utf-8?B?aHU0cC9pM01JWjRKVWs1aXQ4ZlFRcTVHSFdGR2pkUjFZZlAxdG4rbGNNVXpZ?=
 =?utf-8?B?R2hSa1d5YmhCWHc2UmpkSDVQZFZhaDZqcGV1dG4yMFR5TWY3YXA4T24xN2ZI?=
 =?utf-8?B?QTczcEVlbWNCSUV0d0FNQVVKR0s5N1VMM2RoOEFNekNDU1BiM0hwWFlQcWYy?=
 =?utf-8?B?c2pPRkV2bkRjbENDSktkSlp4ZGcvQ2d6Q0l3V2FXZm1uQUVsNzdiOFo1OVpO?=
 =?utf-8?B?aHFJb1QybjUrQ1lER1NobUkyaGVaMzRYeFAvMnRJL2t0QmpXVkl4RlppQStv?=
 =?utf-8?B?RjFtRm45VEg3N3drVUZIWWZpdzR0NHdkamQxNkZuaEpPeThGd1VUR3AzOHAr?=
 =?utf-8?B?UG1KUXVOY1BVbHY2K2dJcHZaeXB4ZC9KZkJZZzBJV3ROSW5RRWpBbjFlUkln?=
 =?utf-8?B?UGNsTE5aWjEzcUV0WFlQQjNVd3dEaGdBZ3hNNXRPa0tlM1FGRElEMWEwTFZ5?=
 =?utf-8?B?Y1BMSHJjNHkva3RTMDdMYStqVTJPOHc5MTdCVjBocGhOcy9rVTNsUzJyR3Bk?=
 =?utf-8?B?Sk5oTFFqcEhNZXZGSFhUNUdnQ3ladmpwZWMvZUVtajZIenUyWHA5OEsrMXB3?=
 =?utf-8?B?ZUpHTDlkcno5Ync1ekVKUnJwa3FxWHpvbEhRclVkSmJDancwZEE1d08vRm5s?=
 =?utf-8?B?MjkxNFJqbllxRUtOUEtVZEh6cm9aT1o5RzFkL3B6OEtpQzk4b0NCbkx2cXpm?=
 =?utf-8?B?UjY3MFZHeWkwanhpUXZFS0p5STR2ZnJ3bXkxcDAwRHlUWDFLM2czRzFXVGZa?=
 =?utf-8?B?RzdSRERnaG9jY1JYa21LL2dUM2xRL0dxTDgrUFNweGtjTDY3SkNYWFJsUXV4?=
 =?utf-8?B?c1JwY29WSEQrcFQrWVFpR1d3ZUQ2Mk5WRzh3a2xkOXJvSVpmZGFuVkFsNDdK?=
 =?utf-8?B?YVFBU3JYNEd1bVV1Zk1BaGluY000SXpzUzJwNFRjcVk0aWx3TndJV2hNblRl?=
 =?utf-8?B?SDFqNGY0UG0xeGdybFFIWjZJbFM4RVc3QkRMeGdkakUrNlBYZW5UeE94UUp5?=
 =?utf-8?B?RXk4VTNzN09zdWFHR2xpcXhUVk14OTFZRng3NGJWUkxIYWVwYU1WSWVqTmZ0?=
 =?utf-8?B?Q2ZNWG9McFRVM1ZuRUpkS0M3VFNwRHRGd1RmSVkrK3BBTGVXcHdMc3krUEk4?=
 =?utf-8?B?dStjWXhPb2NyOG5SR1RsVjNFbWNLb2JDcFpqem83RVhxcW9pZytlOHByTDl5?=
 =?utf-8?B?dXM1d2Iyc1V3OWdlTXQvWmR1bmhnM0s3UkVqZDYrZVBqbm5Hanl1c0YwN2Ri?=
 =?utf-8?B?TFN1eWVCZDdla3d1OFpOR01FMXQ5M3lWY0t2QW93SlJMOXdSazVyZFBNRVVv?=
 =?utf-8?B?VG5tZ0plaXZkZiszRlNLRk9VSEo0b1RjVW1tRDY3dDdyZ0RiTW0vK0M5ZUdh?=
 =?utf-8?B?T1VZNXhFcWNjUWJTYndqYk0xUno5WVRsUEZpdUVKcUdkWWc0UlhGRkUrQzRh?=
 =?utf-8?B?MlE5YnZES1dCRytYVUpBZUJTRUVhazNBRjdmeTkxcDBCRlcwV2dPT0EzWkRB?=
 =?utf-8?Q?cWccb1WF+vcTMFXa27Y/y9Y3G?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c117b76e-65ab-4dc0-9e33-08dc31c9719e
X-MS-Exchange-CrossTenant-AuthSource: PH8PR12MB6674.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2024 04:07:24.8132
 (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: aAVVjC96epVt7HmcO+aB/TI8SVWhWu/Ecdwt+O1oZapi4C1V2P0DUxFUXaawoBwFvnZwvBFr7cdn2gZGPsJC7Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR12MB4258

Hi Bjorn,

Any comments for this patch?


Thanks,

Vidya Sagar


On 08-01-2024 18:12, Vidya Sagar wrote:
> Many PCIe device drivers save the configuration state of their respective
> devices during probe and restore the same when their 'slot_reset' hook
> is called through PCIe Error Recovery Handler.
>
> If the system has a change in ASPM policy after the driver's probe is
> called and before error event occurred, 'slot_reset' hook restores the
> PCIe configuration state to what it was at the time of probe but not with
> what it was just before the occurrence of the error event.
> This effectively leads to a mismatch in the ASPM configuration between
> the device and its upstream parent device.
>
> Update the saved configuration state of the device with the latest info
> whenever there is a change w.r.t ASPM policy.
>
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V3:
> * Addressed sathyanarayanan.kuppuswamy's review comments
>
> V2:
> * Rebased on top of the tree code
> * Addressed Bjorn's review comments
>
>   drivers/pci/pcie/aspm.c | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 67b13f26ba7c..1b4f03044ce2 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -138,16 +138,34 @@ static int policy_to_clkpm_state(struct pcie_link_state *link)
>   	return 0;
>   }
>   
> +static void pci_save_aspm_state(struct pci_dev *dev)
> +{
> +	struct pci_cap_saved_state *save_state;
> +	u16 *cap;
> +
> +	if (!pci_is_pcie(dev))
> +		return;
> +
> +	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
> +	if (!save_state)
> +		return;
> +
> +	cap = (u16 *)&save_state->cap.data[0];
> +	pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &cap[1]);
> +}
> +
>   static void pcie_set_clkpm_nocheck(struct pcie_link_state *link, int enable)
>   {
>   	struct pci_dev *child;
>   	struct pci_bus *linkbus = link->pdev->subordinate;
>   	u32 val = enable ? PCI_EXP_LNKCTL_CLKREQ_EN : 0;
>   
> -	list_for_each_entry(child, &linkbus->devices, bus_list)
> +	list_for_each_entry(child, &linkbus->devices, bus_list) {
>   		pcie_capability_clear_and_set_word(child, PCI_EXP_LNKCTL,
>   						   PCI_EXP_LNKCTL_CLKREQ_EN,
>   						   val);
> +		pci_save_aspm_state(child);
> +	}
>   	link->clkpm_enabled = !!enable;
>   }
>   
> @@ -767,6 +785,10 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
>   		pcie_config_aspm_dev(parent, upstream);
>   
>   	link->aspm_enabled = state;
> +
> +	/* Update latest ASPM configuration in saved context */
> +	pci_save_aspm_state(link->downstream);
> +	pci_save_aspm_state(parent);
>   }
>   
>   static void pcie_config_aspm_path(struct pcie_link_state *link)