linux-kernel - Re: [PATCH v2 9/9] PCI: hotplug: Prefix ctrl

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: <20190509140326.GA88424@google.com>
Date:   Thu, 9 May 2019 09:03:26 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Frederick Lawler <fred@...dlawl.com>
Cc:     linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        mika.westerberg@...ux.intel.com, lukas@...ner.de,
        andriy.shevchenko@...ux.intel.com, keith.busch@...el.com,
        mr.nuke.me@...il.com, liudongdong3@...wei.com, thesven73@...il.com
Subject: Re: [PATCH v2 9/9] PCI: hotplug: Prefix ctrl_*() dmesg logs with
 pciehp slot name

On Thu, May 02, 2019 at 10:59:46PM -0500, Frederick Lawler wrote:
> Remove current uses of "Slot(%s)" and then prefix ctrl_*() dmesg
> with pciehp slot name to include the slot name for all uses of ctrl_*()
> wrappers.
> 
> Signed-off-by: Frederick Lawler <fred@...dlawl.com>
> ---
>  drivers/pci/hotplug/pciehp.h      | 12 ++++---
>  drivers/pci/hotplug/pciehp_core.c |  9 +++--
>  drivers/pci/hotplug/pciehp_ctrl.c | 58 ++++++++++++-------------------
>  drivers/pci/hotplug/pciehp_hpc.c  |  5 ++-
>  4 files changed, 38 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
> index 06ff9d31405e..e1cdc3565c62 100644
> --- a/drivers/pci/hotplug/pciehp.h
> +++ b/drivers/pci/hotplug/pciehp.h
> @@ -32,13 +32,17 @@ extern int pciehp_poll_time;
>  extern bool pciehp_debug;
>  
>  #define ctrl_dbg(ctrl, format, arg...)					\
> -	pci_dbg(ctrl->pcie->port, format, ## arg)
> +	pci_dbg(ctrl->pcie->port, "Slot(%s): " format,			\
> +		slot_name(ctrl), ## arg)

This would be nice to do, but given the current code organization, I don't
think it's actually feasible to use slot_name() in these wrappers because
the slot name is initialized in init_slot(), but there are lots of places
we can emit messages before that, especially if debug is enabled:

  pciehp_probe
    pcie_init
      dbg_ctrl
        ctrl_info                          # no slot yet
      ctrl_info("Slot #%d AttnBtn%c ...")  # no slot yet
      if (POWER_CTRL)
        pciehp_get_power_status
	  ctrl_dbg("SLOTCTRL")             # no slot yet
	if (...)
	  pcie_disable_notification
	    pcie_write_cmd
	      pcie_do_write_cmd
	        ctrl_info("no response")   # no slot yet
	    ctrl_dbg("SLOTCTRL")           # no slot yet
	  pciehp_power_off_slot
	    ctrl_dbg("SLOTCTRL")           # no slot yet
    init_slot                              # slot valid after this returns