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: <Y3vH4GzKYoXqC2Af@intel.com>
Date:   Mon, 21 Nov 2022 13:48:00 -0500
From:   Rodrigo Vivi <rodrigo.vivi@...el.com>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>
CC:     Bjorn Helgaas <helgaas@...nel.org>, Lukas Wunner <lukas@...ner.de>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux ACPI <linux-acpi@...r.kernel.org>,
        Linux PCI <linux-pci@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>
Subject: Re: [PATCH v1 2/2] ACPI: PCI: hotplug: Avoid setting
 is_hotplug_bridge for PCIe Upstream Ports

On Mon, Nov 21, 2022 at 07:16:57PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> 
> It is reported that on some systems pciehp binds to an Upstream Port and
> attempts to operate it which causes devices below the Port to disappear
> from the bus.
> 
> This happens because acpiphp sets is_hotplug_bridge for that Port (after
> receiving a Device Check notification on it from the platform firmware
> via ACPI) during the enumeration of PCI devices and so when
> get_port_device_capability() runs, it sees that is_hotplug_bridge is
> set and adds PCIE_PORT_SERVICE_HP to Port services (which allows pciehp
> to bind to the Port in question).
> 
> Even though this particular problem can be addressed by making the
> portdrv_core checks more robust, it also causes power management to
> work differently on the affected systems which generally is not
> desirable (PCIe Ports with is_hotplug_bridge set have to pass
> additional tests to be allowed to go into the D3hot/cold power
> states which affects runtime PM of devices below these Ports).
> 
> For this reason, amend check_hotplug_bridge() with a PCIe type check
> to prevent it from setting is_hotplug_bridge for Upstream Ports.
> 
> Reported-by: Rodrigo Vivi <rodrigo.vivi@...el.com>

for the series:

Tested-by: Rodrigo Vivi <rodrigo.vivi@...el.com>

Based on all the explanations you gave and docs you showed to me
recently this makes total sense and the double protection seems
good to me.

Let's see if Lukas agree, but feel free to also use if needed:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@...el.com>

> Suggested-by: Lukas Wunner <lukas@...ner.de>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
>  drivers/pci/hotplug/acpiphp_glue.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
> +++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
> @@ -411,6 +411,14 @@ static void check_hotplug_bridge(struct
>  	if (dev->is_hotplug_bridge)
>  		return;
>  
> +	/*
> +	 * In the PCIe case, only Root Ports and Downstream Ports are capable of
> +	 * accommodating hotplug devices, so avoid marking Upstream Ports as
> +	 * "hotplug bridges".
> +	 */
> +	if (pci_is_pcie(dev) && pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM)
> +		return;
> +
>  	list_for_each_entry(func, &slot->funcs, sibling) {
>  		if (PCI_FUNC(dev->devfn) == func->function) {
>  			dev->is_hotplug_bridge = 1;
> 
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ