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: <1ade6a9f-9532-c400-9bb0-4e68ed5752ce@linux.intel.com>
Date:   Thu, 7 Nov 2019 11:59:57 -0800
From:   Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
To:     Bjorn Helgaas <helgaas@...nel.org>, Olof Johansson <olof@...om.net>
Cc:     Keith Busch <keith.busch@...el.com>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI/DPC: Add pcie_ports=dpc-native parameter to bring
 back old behavior

Hi,

On 10/25/19 1:20 PM, Bjorn Helgaas wrote:
> On Wed, Oct 23, 2019 at 12:22:05PM -0700, Olof Johansson wrote:
>> In commit eed85ff4c0da7 ("PCI/DPC: Enable DPC only if AER is available"),
>> the behavior was changed such that native (kernel) handling of DPC
>> got tied to whether the kernel also handled AER. While this is what
>> the standard recommends, there are BIOSes out there that lack the DPC
>> handling since it was never required in the past.
> Some systems do not grant OS control of AER via _OSC.  I guess the
> problem is that on those systems, the OS DPC driver used to work, but
> after eed85ff4c0da7, it does not.  Right?

I need some clarification on this issue. Do you mean in these systems,
firmware expects OS to handle DPC and AER, but it does not let OS know
about it via _OSC ?

>
> We should also update negotiate_os_control() to request control of DPC
> via _OSC.  Kuppuswamy's patch [1] does that but hasn't been merged
> yet.  That will conflict with this, but I can resolve that.
>
> I applied this as below (with the nits Keith noticed) to pci/aer for
> v5.5, thanks!
>
> [1] https://lore.kernel.org/r/b638cbd3e122b4c7a58b949d7224230d2c4b34d4.1570145778.git.sathyanarayanan.kuppuswamy@linux.intel.com
>
> commit 35a0b2378c19
> Author: Olof Johansson <olof@...om.net>
> Date:   Wed Oct 23 12:22:05 2019 -0700
>
>      PCI/DPC: Add "pcie_ports=dpc-native" to allow DPC without AER control
>      
>      Prior to eed85ff4c0da7 ("PCI/DPC: Enable DPC only if AER is available"),
>      Linux handled DPC events regardless of whether firmware had granted it
>      ownership of AER or DPC, e.g., via _OSC.
>      
>      PCIe r5.0, sec 6.2.10, recommends that the OS link control of DPC to
>      control of AER, so after eed85ff4c0da7, Linux handles DPC events only if it
>      has control of AER.
>      
>      On platforms that do not grant OS control of AER via _OSC, Linux DPC
>      handling worked before eed85ff4c0da7 but not after.
>      
>      To make Linux DPC handling work on those platforms the same way they did
>      before, add a "pcie_ports=dpc-native" kernel parameter that makes Linux
>      handle DPC events regardless of whether it has control of AER.
>      
>      [bhelgaas: commit log, move pcie_ports_dpc_native to drivers/pci/]
>      Link: https://lore.kernel.org/r/20191023192205.97024-1-olof@lixom.net
>      Signed-off-by: Olof Johansson <olof@...om.net>
>      Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index c7ac2f3ac99f..806c89f79be8 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3540,6 +3540,8 @@
>   			even if the platform doesn't give the OS permission to
>   			use them.  This may cause conflicts if the platform
>   			also tries to use these services.
> +		dpc-native	Use native PCIe service for DPC only.  May
> +				cause conflicts if firmware uses AER or DPC.
>   		compat	Disable native PCIe services (PME, AER, DPC, PCIe
>   			hotplug).
>   
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index a32ec3487a8d..e06f42f58d3d 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -291,7 +291,7 @@ static int dpc_probe(struct pcie_device *dev)
>   	int status;
>   	u16 ctl, cap;
>   
> -	if (pcie_aer_get_firmware_first(pdev))
> +	if (pcie_aer_get_firmware_first(pdev) && !pcie_ports_dpc_native)
>   		return -ENOTSUPP;
>   
>   	dpc = devm_kzalloc(device, sizeof(*dpc), GFP_KERNEL);
> diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
> index 944827a8c7d3..1e673619b101 100644
> --- a/drivers/pci/pcie/portdrv.h
> +++ b/drivers/pci/pcie/portdrv.h
> @@ -25,6 +25,8 @@
>   
>   #define PCIE_PORT_DEVICE_MAXSERVICES   5
>   
> +extern bool pcie_ports_dpc_native;
> +
>   #ifdef CONFIG_PCIEAER
>   int pcie_aer_init(void);
>   #else
> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> index 1b330129089f..5075cb9e850c 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -250,8 +250,13 @@ static int get_port_device_capability(struct pci_dev *dev)
>   		pcie_pme_interrupt_enable(dev, false);
>   	}
>   
> +	/*
> +	 * With dpc-native, allow Linux to use DPC even if it doesn't have
> +	 * permission to use AER.
> +	 */
>   	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> -	    pci_aer_available() && services & PCIE_PORT_SERVICE_AER)
> +	    pci_aer_available() &&
> +	    (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
>   		services |= PCIE_PORT_SERVICE_DPC;
>   
>   	if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM ||
> diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
> index 0a87091a0800..160d67c59310 100644
> --- a/drivers/pci/pcie/portdrv_pci.c
> +++ b/drivers/pci/pcie/portdrv_pci.c
> @@ -29,12 +29,20 @@ bool pcie_ports_disabled;
>    */
>   bool pcie_ports_native;
>   
> +/*
> + * If the user specified "pcie_ports=dpc-native", use the Linux DPC PCIe
> + * service even if the platform hasn't given us permission.
> + */
> +bool pcie_ports_dpc_native;
> +
>   static int __init pcie_port_setup(char *str)
>   {
>   	if (!strncmp(str, "compat", 6))
>   		pcie_ports_disabled = true;
>   	else if (!strncmp(str, "native", 6))
>   		pcie_ports_native = true;
> +	else if (!strncmp(str, "dpc-native", 10))
> +		pcie_ports_dpc_native = true;
>   
>   	return 1;
>   }
>
-- 
Sathyanarayanan Kuppuswamy
Linux kernel developer

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ