linux-kernel - Re: [PATCH] PCI/DPC: Request DPC only if also requesting AER

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <39ef1387-609c-45ca-9bfa-e01b72cacaaa@linux.intel.com>
Date: Tue, 20 Feb 2024 18:45:32 -0800
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
To: Bjorn Helgaas <helgaas@...nel.org>, linux-pci@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
 stable@...r.kernel.org, Matthew W Carlis <mattc@...estorage.com>,
 Keith Busch <kbusch@...nel.org>, Lukas Wunner <lukas@...ner.de>,
 Mika Westerberg <mika.westerberg@...ux.intel.com>,
 Jesse Brandeburg <jesse.brandeburg@...el.com>
Subject: Re: [PATCH] PCI/DPC: Request DPC only if also requesting AER


On 2/20/24 3:55 PM, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@...gle.com>
>
> When booting with "pci=noaer", we don't request control of AER, but we
> previously *did* request control of DPC, as in the dmesg log attached at
> the bugzilla below:
>
>   Command line: ... pci=noaer
>   acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI EDR HPX-Type3]
>   acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug SHPCHotplug PME PCIeCapability LTR DPC]
>
> That's illegal per PCI Firmware Spec, r3.3, sec 4.5.1, table 4-5, which
> says:
>
>   If the operating system sets this bit [OSC_PCI_EXPRESS_DPC_CONTROL], it
>   must also set bit 7 of the Support field (indicating support for Error
>   Disconnect Recover notifications) and bits 3 and 4 of the Control field
>   (requesting control of PCI Express Advanced Error Reporting and the PCI
>   Express Capability Structure).
>
> Request DPC control only if we have also requested AER control.

Can you also add similar check in calculate_support call?

        if (pci_aer_available() && IS_ENABLED(CONFIG_PCIE_EDR))
                support |= OSC_PCI_EDR_SUPPORT;


>
> Fixes: ac1c8e35a326 ("PCI/DPC: Add Error Disconnect Recover (EDR) support")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218491#c12
> Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: <stable@...r.kernel.org>	# v5.7+
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
> Cc: Matthew W Carlis <mattc@...estorage.com>
> Cc: Keith Busch <kbusch@...nel.org>
> Cc: Lukas Wunner <lukas@...ner.de>
> Cc: Mika Westerberg <mika.westerberg@...ux.intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@...el.com>
> ---
>  drivers/acpi/pci_root.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 58b89b8d950e..1c16965427b3 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -518,17 +518,19 @@ static u32 calculate_control(void)
>  	if (IS_ENABLED(CONFIG_HOTPLUG_PCI_SHPC))
>  		control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
>  
> -	if (pci_aer_available())
> +	if (pci_aer_available()) {
>  		control |= OSC_PCI_EXPRESS_AER_CONTROL;
>  
> -	/*
> -	 * Per the Downstream Port Containment Related Enhancements ECN to
> -	 * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
> -	 * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
> -	 * and EDR.
> -	 */
> -	if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
> -		control |= OSC_PCI_EXPRESS_DPC_CONTROL;
> +		/*
> +		 * Per PCI Firmware Spec, r3.3, sec 4.5.1, table 4-5, the
> +		 * OS can request DPC control only if it has advertised
> +		 * OSC_PCI_EDR_SUPPORT and requested both
> +		 * OSC_PCI_EXPRESS_CAPABILITY_CONTROL and
I think you mean OSC_PCI_EXPRESS_DPC_CONTROL.
> +		 * OSC_PCI_EXPRESS_AER_CONTROL.
> +		 */
> +		if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
> +			control |= OSC_PCI_EXPRESS_DPC_CONTROL;

Since you are cleaning up this part, why not add a patch to remove
CONFIG_PCIE_EDR?


> +	}
>  
>  	return control;
>  }

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer