Received-SPF: pass (google.com: domain of linux-kernel+bounces-45138-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <7a8f3595-3efc-428a-852a-d9edc8ebb01b@amd.com>
Date: Tue, 30 Jan 2024 12:47:00 -0600
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] PCI: Disable D3cold on Asus B1400 PCI-NVMe bridge
Content-Language: en-US
To: Daniel Drake <drake@endlessos.org>, tglx@linutronix.de, mingo@redhat.com,
 bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org
Cc: hpa@zytor.com, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
 bhelgaas@google.com, david.e.box@linux.intel.com
References: <20240130183124.19985-1-drake@endlessos.org>
From: Mario Limonciello <mario.limonciello@amd.com>
In-Reply-To: <20240130183124.19985-1-drake@endlessos.org>
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?amc4ZFdIVEZ1Y0JjeHZpdjBxTzhOMzRnZ1REcjB4OEtGTm96RGtvSytVaEd3?=
 =?utf-8?B?R3djU3BGc2xPaVRsMG01T1R4ZUFEWWpVMFJvNTFOVVZ6SGxtY25SaDM0enk5?=
 =?utf-8?B?b0lqY25tSEdwT0RrTjg5WFcwRjJCb0pLbTdTYmU1V21kN0dPOHZDTXNiQzc0?=
 =?utf-8?B?OWMvVlNUY1FJcUVpMk5CMnNma3B1bk4yVTA0NllDS2hKMVpZWUlDRFFONDRn?=
 =?utf-8?B?SzN1N2tZMmpBTkY1elpLTngxeGUzQmVONjc4SklwNVh1YWI2R05XWnNJYTk2?=
 =?utf-8?B?bXJCb1F1cTFpN1BnVG5iWTdrc2grVVV5REptNnU3NVFLVGMrVENYdjBOd21I?=
 =?utf-8?B?QXBuRUNVM2RsNWNXd2hrODlqYVdWdEFQYkt1Yk9WRXA5SzVaVi9zMVJiQUFM?=
 =?utf-8?B?eXZqdGZCUGRYcnZHV3NPWkphNlZ6Qy84SFVFcEx0dWJkZFdvdTRLaVR0UnBa?=
 =?utf-8?B?dmJkZG4rdThhZVFvUHVQSFFCZmtWbG1OWnhsUWtiT1BoWkJDY3NvbldaNUpa?=
 =?utf-8?B?U2tZZ1VmeStiNHdGMlFSUk8yVWJ5TytHb3BSeW9aajJDSTVZOXpmQWJISERS?=
 =?utf-8?B?VUdiUDNCL2lGcGNmWkZKaVo4b1ZRY0kyYzBKKzNrbk00emYrTGV0amdPYmht?=
 =?utf-8?B?L0VlRGtXUEF1RDFLM2w5V1lQV3Q0T1g2Tk5tMHJXNW9IMlJDSXlyR2dmU1Ra?=
 =?utf-8?B?WEkvWEhjcm15NDFRRkdmS2RKNjhZWnRtcTNyUlhrSnViSmNHWEVSS3lGODdp?=
 =?utf-8?B?QzdIaUlFN1QyK2RKd09TMVlIRFJtRlNRUWFmWC9RUXBZcXdUTUJzWUtRaDRP?=
 =?utf-8?B?dzB1MFV3aEUvbkRYbkluR1EwSjlHcmowZDlod1R2MFo2ckx4S0p3ak9GVVAz?=
 =?utf-8?B?c1Izb2trV1YvbjZyQllFSXlPdkJQL2tvMTM4RExXcnZ2YWNBMnUxM2w4dU00?=
 =?utf-8?B?K1FMNXNhRnZ2cVk3dEFGeHN6dnBkVDhlOWo5ckpXZkM2NzlIT2tMYlJEb3kz?=
 =?utf-8?B?RWN5RnJoVGVMcU9ncGhOMjBCNzBxMGNZdUo5eDdGTllNQzk5cDA3eWxobzBT?=
 =?utf-8?B?LzlWL2JJaytOTWd6NjVKR0tEQWJFdUIzVGVGa2ZhMm8rRVNsbTdRd0Z1RmV1?=
 =?utf-8?B?SVZvdHhDdGdTSXRwa3ZNbkNsNEhZOGZ4b1J4YnFCUTFzYXB0RkpuampNMVJz?=
 =?utf-8?B?L1hNNkRCY1RkazZxTXZTN2VSRGc5eFJnM3UzaVJyU2lDWGphaWhPRjIyT21F?=
 =?utf-8?B?Zjk1aUhNWEp4eWd6M0JhUDNDWTB6Z1pJaHpxU2t6M21rZHp1QndxTGNuakx2?=
 =?utf-8?B?RncwQllWQkZGV2ZKVUUwNUpVdlgwVlJ2MkUvN3R5SHFYT2FTYm93SW04Nzdu?=
 =?utf-8?B?U1pWN3plbm5HVE5SMXBsS3Rmbk1rdkhpeFd4c1Y4WUNROGVIem9kVmFPVkhS?=
 =?utf-8?B?aTNERHNUa3B5Nll5NmVjaWZCRGU2YjRFY3VGSk9WRWQ3VzZBdXEyREtObTZO?=
 =?utf-8?B?Y0VqWWJrWDdtRnczUG1wbzFIZnhUQ0gwQ3V5U3B1QmFueTc1S01sVStyTnlr?=
 =?utf-8?B?ZkE4U2NtWmFqNWpaS3RQSEhwMkNSSWpkWEg5dXF3ZExBdGFPU0xmd3c5OHZr?=
 =?utf-8?B?VWUzQlU3RlFSME5MY2lrWXh4MUpPVVdOekxZVUd0Y3U5T21rc3hkNWZXMmJF?=
 =?utf-8?B?a3AyVzlzUjd0NkMvR1FTeWtMb05GUzNzc3dCSHduTWorc0cwRy9tdFpQdFB6?=
 =?utf-8?B?dkYxc0dxVGlUcUhpV3dVcEtTMytnVlRwcGs3Z283eVdxZGJHSllVZmNDcmhX?=
 =?utf-8?B?WkczTDVUanh3N1NsMXdjcms3VFBXNEFZTDl1MTZjS2F3RHVXeCt3R2ZvMktL?=
 =?utf-8?B?UElQZVR6d0hxWTg0Zi96bksvaEx1NVpFc1k3MlkzVjFta25qVWJ2dFBLdm1G?=
 =?utf-8?B?RXhSeVN2dnB3b1R2QmczUk9VMzQvbjdveDBqT1hsTHZiMmF1V0hSQVFaL2My?=
 =?utf-8?B?dzlRcHV6aXFwTklEdVBWMzhxRlorK2doWGJDMjdwb29HRG93Y1UyNlVWeWRN?=
 =?utf-8?B?djZ5UCs2KzQ3QXFjM2hPbUFYaStaeWZ5TFdMa2VGL1dqUk9Wcy9MY3M2cEVR?=
 =?utf-8?Q?a9x+YijE9ul65v/qo7xAKMf4F?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 895835bb-4770-46ad-b2bd-08dc21c3d962
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2024 18:47:03.0520
 (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: DMxjsgyt6k+PHcruF+EYBTVLxpxkeA3ar0qC3w/4IAQiPJytVXpIve6f6sWAxy/IVsoyb01gZXb0MeprA9qh3w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR12MB4970

On 1/30/2024 12:31, Daniel Drake wrote:
> The Asus B1400 with production shipped firmware version 304 and VMD
> disabled cannot resume from suspend: the NVMe device becomes unresponsive
> and inaccessible.

Has there already been a check done whether any newer firmware is 
available from ASUS that doesn't suffer the deficiency described below?

> 
> This is because the NVMe device and parent PCI bridge get put into D3cold
> during suspend, and this PCI bridge cannot be recovered from D3cold mode:
> 
>    echo "0000:01:00.0" > /sys/bus/pci/drivers/nvme/unbind
>    echo "0000:00:06.0" > /sys/bus/pci/drivers/pcieport/unbind
>    setpci -s 00:06.0 CAP_PM+4.b=03 # D3hot
>    acpidbg -b "execute \_SB.PC00.PEG0.PXP._OFF"
>    acpidbg -b "execute \_SB.PC00.PEG0.PXP._ON"
>    setpci -s 00:06.0 CAP_PM+4.b=0 # D0
>    echo "0000:00:06.0" > /sys/bus/pci/drivers/pcieport/bind
>    echo "0000:01:00.0" > /sys/bus/pci/drivers/nvme/bind
>    # NVMe probe fails here with -ENODEV
> 
> This appears to be an untested D3cold transition by the vendor; Intel
> socwatch shows that Windows leaves the NVMe device and parent bridge in D0
> during suspend, even though this firmware version has StorageD3Enable=1.
> 
> Experimenting with the DSDT, the _OFF method calls DL23() which sets a L23E
> bit at offset 0xe2 into the PCI configuration space for this root port.
> This is the specific write that the _ON routine is unable to recover from.
> This register is not documented in the public chipset datasheet.
> 
> Disallow D3cold on the PCI bridge to enable successful suspend/resume.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215742
> Signed-off-by: Daniel Drake <drake@endlessos.org>
> ---
>   arch/x86/pci/fixup.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> There's an existing quirk for this product that selects S3 instead of
> s2idle because of this failure. However, after extensive testing,
> we've found that S3 cannot reliably wake up from suspend (firmware issue).
> We need to get s2idle working again; I'll revert the original quirk
> once we have solved this D3cold issue.

Is this the only problem blocking s2idle from working effectively on 
this platform?  If so, I would think you want to just do the revert in 
the same series if it's decided this patch needs to spin again for any 
reason.

> 
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index f347c20247d30..c486d86bb678b 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -907,6 +907,32 @@ static void chromeos_fixup_apl_pci_l1ss_capability(struct pci_dev *dev)
>   DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x5ad6, chromeos_save_apl_pci_l1ss_capability);
>   DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_INTEL, 0x5ad6, chromeos_fixup_apl_pci_l1ss_capability);
>   
> +/*
> + * Disable D3cold on Asus B1400 PCIe bridge at 00:06.0.
> + *
> + * On this platform with VMD off, the NVMe's parent PCI bridge cannot
> + * successfully power back on from D3cold, resulting in unresponsive NVMe on
> + * resume. This appears to be an untested transition by the vendor: Windows
> + * leaves the NVMe and parent bridge in D0 during suspend, even though
> + * StorageD3Enable=1 is set on the production shipped firmware version 304.
> + */
> +static const struct dmi_system_id asus_nvme_broken_d3cold_table[] = {
> +	{
> +		.matches = {
> +				DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +				DMI_MATCH(DMI_PRODUCT_NAME, "ASUS EXPERTBOOK B1400CEAE"),
> +		},
> +	},
> +	{}
> +};
> +
> +static void asus_disable_nvme_d3cold(struct pci_dev *pdev)
> +{
> +	if (dmi_check_system(asus_nvme_broken_d3cold_table) > 0)
> +		pci_d3cold_disable(pdev);
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x9a09, asus_disable_nvme_d3cold);
> +
>   #ifdef CONFIG_SUSPEND
>   /*
>    * Root Ports on some AMD SoCs advertise PME_Support for D3hot and D3cold, but