linux-kernel - Re: [PATCH v4 1/3] PCI: pciehp: Add support for disabling in-band presence

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: <20191127013613.GA233706@google.com>
Date:   Tue, 26 Nov 2019 19:36:13 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Stuart Hayes <stuart.w.hayes@...il.com>
Cc:     Austin Bolen <austin_bolen@...l.com>, keith.busch@...el.com,
        Alexandru Gagniuc <mr.nuke.me@...il.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        "Gustavo A . R . Silva" <gustavo@...eddedor.com>,
        Sinan Kaya <okaya@...nel.org>,
        Oza Pawandeep <poza@...eaurora.org>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, lukas@...ner.de
Subject: Re: [PATCH v4 1/3] PCI: pciehp: Add support for disabling in-band
 presence

On Fri, Oct 25, 2019 at 03:00:45PM -0400, Stuart Hayes wrote:
> From: Alexandru Gagniuc <mr.nuke.me@...il.com>
> 
> The presence detect state (PDS) is normally a logical or of in-band and
> out-of-band presence. As of PCIe 4.0, there is the option to disable
> in-band presence so that the PDS bit always reflects the state of the
> out-of-band presence.
> 
> The recommendation of the PCIe spec is to disable in-band presence
> whenever supported.

I think I'm fine with this patch, but I would like to include the
specific reference for this recommendation.  If you have it handy, I
can just insert it.

> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@...il.com>
> ---
>  drivers/pci/hotplug/pciehp.h     | 1 +
>  drivers/pci/hotplug/pciehp_hpc.c | 9 ++++++++-
>  include/uapi/linux/pci_regs.h    | 2 ++
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
> index 654c972b8ea0..27e4cd6529b0 100644
> --- a/drivers/pci/hotplug/pciehp.h
> +++ b/drivers/pci/hotplug/pciehp.h
> @@ -83,6 +83,7 @@ struct controller {
>  	struct pcie_device *pcie;
>  
>  	u32 slot_cap;				/* capabilities and quirks */
> +	unsigned int inband_presence_disabled:1;
>  
>  	u16 slot_ctrl;				/* control register access */
>  	struct mutex ctrl_lock;
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 1a522c1c4177..dc109d521f30 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -811,7 +811,7 @@ static inline void dbg_ctrl(struct controller *ctrl)
>  struct controller *pcie_init(struct pcie_device *dev)
>  {
>  	struct controller *ctrl;
> -	u32 slot_cap, link_cap;
> +	u32 slot_cap, slot_cap2, link_cap;
>  	u8 poweron;
>  	struct pci_dev *pdev = dev->port;
>  	struct pci_bus *subordinate = pdev->subordinate;
> @@ -869,6 +869,13 @@ struct controller *pcie_init(struct pcie_device *dev)
>  		FLAG(link_cap, PCI_EXP_LNKCAP_DLLLARC),
>  		pdev->broken_cmd_compl ? " (with Cmd Compl erratum)" : "");
>  
> +	pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP2, &slot_cap2);
> +	if (slot_cap2 & PCI_EXP_SLTCAP2_IBPD) {
> +		pcie_write_cmd_nowait(ctrl, PCI_EXP_SLTCTL_IBPD_DISABLE,
> +				      PCI_EXP_SLTCTL_IBPD_DISABLE);
> +		ctrl->inband_presence_disabled = 1;
> +	}
> +
>  	/*
>  	 * If empty slot's power status is on, turn power off.  The IRQ isn't
>  	 * requested yet, so avoid triggering a notification with this command.
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 29d6e93fd15e..ea1cf9546e4d 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -604,6 +604,7 @@
>  #define  PCI_EXP_SLTCTL_PWR_OFF        0x0400 /* Power Off */
>  #define  PCI_EXP_SLTCTL_EIC	0x0800	/* Electromechanical Interlock Control */
>  #define  PCI_EXP_SLTCTL_DLLSCE	0x1000	/* Data Link Layer State Changed Enable */
> +#define  PCI_EXP_SLTCTL_IBPD_DISABLE	0x4000 /* In-band PD disable */
>  #define PCI_EXP_SLTSTA		26	/* Slot Status */
>  #define  PCI_EXP_SLTSTA_ABP	0x0001	/* Attention Button Pressed */
>  #define  PCI_EXP_SLTSTA_PFD	0x0002	/* Power Fault Detected */
> @@ -676,6 +677,7 @@
>  #define PCI_EXP_LNKSTA2		50	/* Link Status 2 */
>  #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2	52	/* v2 endpoints with link end here */
>  #define PCI_EXP_SLTCAP2		52	/* Slot Capabilities 2 */
> +#define  PCI_EXP_SLTCAP2_IBPD	0x0001	/* In-band PD Disable Supported */
>  #define PCI_EXP_SLTCTL2		56	/* Slot Control 2 */
>  #define PCI_EXP_SLTSTA2		58	/* Slot Status 2 */
>  
> -- 
> 2.18.1
>