1. 08 Oct, 2018 4 commits
  2. 04 Oct, 2018 1 commit
  3. 02 Oct, 2018 19 commits
  4. 26 Sep, 2018 2 commits
    • Keith Busch's avatar
      PCI/ERR: Run error recovery callbacks for all affected devices · bfcb79fc
      Keith Busch authored
      If an Endpoint reported an error with ERR_FATAL, we previously ran driver
      error recovery callbacks only for the Endpoint's driver.  But if we reset a
      Link to recover from the error, all downstream components are affected,
      including the Endpoint, any multi-function peers, and children of those
      peers.
      
      Initiate the Link reset from the deepest Downstream Port that is
      reliable, and call the error recovery callbacks for all its children.
      
      If a Downstream Port (including a Root Port) reports an error, we assume
      the Port itself is reliable and we need to reset its downstream Link.  In
      all other cases (Switch Upstream Ports, Endpoints, Bridges, etc), we assume
      the Link leading to the component needs to be reset, so we initiate the
      reset at the parent Downstream Port.
      
      This allows two other clean-ups.  First, we currently only use a Link
      reset, which can only be initiated using a Downstream Port, so we can
      remove checks for Endpoints.  Second, the Downstream Port where we initiate
      the Link reset is reliable (unlike components downstream from it), so the
      special cases for error detect and resume are no longer necessary.
      Signed-off-by: default avatarKeith Busch <keith.busch@intel.com>
      [bhelgaas: changelog]
      Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
      Reviewed-by: default avatarSinan Kaya <okaya@kernel.org>
      bfcb79fc
    • Keith Busch's avatar
      PCI/ERR: Handle fatal error recovery · bdb5ac85
      Keith Busch authored
      We don't need to be paranoid about the topology changing while handling an
      error.  If the device has changed in a hotplug capable slot, we can rely on
      the presence detection handling to react to a changing topology.
      
      Restore the fatal error handling behavior that existed before merging DPC
      with AER with 7e9084b3 ("PCI/AER: Handle ERR_FATAL with removal and
      re-enumeration of devices").
      Signed-off-by: default avatarKeith Busch <keith.busch@intel.com>
      Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
      Reviewed-by: default avatarSinan Kaya <okaya@kernel.org>
      bdb5ac85
  5. 21 Sep, 2018 3 commits
  6. 20 Sep, 2018 3 commits
  7. 18 Sep, 2018 8 commits
    • Lukas Wunner's avatar
      PCI: hotplug: Document TODOs · a0d58937
      Lukas Wunner authored
      While refactoring the PCI hotplug core's API, I noticed a significant
      amount of technical debt in some of the hotplug drivers.  Document the
      issues that caught my eye for starters.
      
      I do not have hardware at my disposal that utilizes the listed drivers
      and I think that's a prerequisite to work on them to ensure that no
      regressions sneak in.  But some of this hardware is so old that it may be
      hard to come by.  Obviously, it is fine to support old hardware, but the
      drivers need to be maintained.
      
      If noone steps up, perhaps we should consider sunsetting a few drivers
      by moving them to staging.  Based on my findings, ibmphp would be the
      first candidate.  I've found it fairly difficult to apply my API
      refactorings to it and have listed some obvious bugs in the driver.
      cpqphp is also in need of a modernization and would be a second
      candidate for relegation to staging.
      
      shpchp was introduced in the same commit as pciehp but hasn't benefited
      from the same amount of refactoring due to the decline of conventional
      PCI's relevance.  Yet hardware supporting it may be more prevalent than
      for the proprietary hotplug methods.
      
      Per Documentation/process/2.Process.rst, "a TODO file should be present"
      for drivers in staging.  The file introduced by the present commit may
      serve as a basis for this.
      Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
      Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
      Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
      Cc: Scott Murray <scott@spiteful.org>
      Cc: Dan Zink <dan.zink@hpe.com>
      Cc: Prarit Bhargava <prarit@redhat.com>
      a0d58937
    • Lukas Wunner's avatar
      PCI: hotplug: Embed hotplug_slot · 125450f8
      Lukas Wunner authored
      When the PCI hotplug core and its first user, cpqphp, were introduced in
      February 2002 with historic commit a8a2069f, cpqphp allocated a slot
      struct for its internal use plus a hotplug_slot struct to be registered
      with the hotplug core and linked the two with pointers:
      https://git.kernel.org/tglx/history/c/a8a2069f432c
      
      Nowadays, the predominant pattern in the tree is to embed ("subclass")
      such structures in one another and cast to the containing struct with
      container_of().  But it wasn't until July 2002 that container_of() was
      introduced with historic commit ec4f2142:
      https://git.kernel.org/tglx/history/c/ec4f214232cf
      
      pnv_php, introduced in 2016, did the right thing and embedded struct
      hotplug_slot in its internal struct pnv_php_slot, but all other drivers
      cargo-culted cpqphp's design and linked separate structs with pointers.
      
      Embedding structs is preferrable to linking them with pointers because
      it requires fewer allocations, thereby reducing overhead and simplifying
      error paths.  Casting an embedded struct to the containing struct
      becomes a cheap subtraction rather than a dereference.  And having fewer
      pointers reduces the risk of them pointing nowhere either accidentally
      or due to an attack.
      
      Convert all drivers to embed struct hotplug_slot in their internal slot
      struct.  The "private" pointer in struct hotplug_slot thereby becomes
      unused, so drop it.
      Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
      Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
      Reviewed-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      Acked-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>  # drivers/pci/hotplug/rpa*
      Acked-by: Sebastian Ott <sebott@linux.ibm.com>        # drivers/pci/hotplug/s390*
      Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> # drivers/platform/x86
      Cc: Len Brown <lenb@kernel.org>
      Cc: Scott Murray <scott@spiteful.org>
      Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
      Cc: Paul Mackerras <paulus@samba.org>
      Cc: Michael Ellerman <mpe@ellerman.id.au>
      Cc: Oliver OHalloran <oliveroh@au1.ibm.com>
      Cc: Gavin Shan <gwshan@linux.vnet.ibm.com>
      Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
      Cc: Corentin Chary <corentin.chary@gmail.com>
      Cc: Darren Hart <dvhart@infradead.org>
      125450f8
    • Lukas Wunner's avatar
      PCI: hotplug: Drop hotplug_slot_info · a7da2161
      Lukas Wunner authored
      Ever since the PCI hotplug core was introduced in 2002, drivers had to
      allocate and register a struct hotplug_slot_info for every slot:
      https://git.kernel.org/tglx/history/c/a8a2069f432c
      
      Apparently the idea was that drivers furnish the hotplug core with an
      up-to-date card presence status, power status, latch status and
      attention indicator status as well as notify the hotplug core of changes
      thereof.  However only 4 out of 12 hotplug drivers bother to notify the
      hotplug core with pci_hp_change_slot_info() and the hotplug core never
      made any use of the information:  There is just a single macro in
      pci_hotplug_core.c, GET_STATUS(), which uses the hotplug_slot_info if
      the driver lacks the corresponding callback in hotplug_slot_ops.  The
      macro is called when the user reads the attribute via sysfs.
      
      Now, if the callback isn't defined, the attribute isn't exposed in sysfs
      in the first place (see e.g. has_power_file()).  There are only two
      situations when the hotplug_slot_info would actually be accessed:
      
      * If the driver defines ->enable_slot or ->disable_slot but not
        ->get_power_status.
      
      * If the driver defines ->set_attention_status but not
        ->get_attention_status.
      
      There is no driver doing the former and just a single driver doing the
      latter, namely pnv_php.c.  Amend it with a ->get_attention_status
      callback.  With that, the hotplug_slot_info becomes completely unused by
      the PCI hotplug core.  But a few drivers use it internally as a cache:
      
      cpcihp uses it to cache the latch_status and adapter_status.
      cpqhp uses it to cache the adapter_status.
      pnv_php and rpaphp use it to cache the attention_status.
      shpchp uses it to cache all four values.
      
      Amend these drivers to cache the information in their private slot
      struct.  shpchp's slot struct already contains members to cache the
      power_status and adapter_status, so additional members are only needed
      for the other two values.  In the case of cpqphp, the cached value is
      only accessed in a single place, so instead of caching it, read the
      current value from the hardware.
      
      Caution:  acpiphp, cpci, cpqhp, shpchp, asus-wmi and eeepc-laptop
      populate the hotplug_slot_info with initial values on probe.  That code
      is herewith removed.  There is a theoretical chance that the code has
      side effects without which the driver fails to function, e.g. if the
      ACPI method to read the adapter status needs to be executed at least
      once on probe.  That seems unlikely to me, still maintainers should
      review the changes carefully for this possibility.
      
      Rafael adds: "I'm not aware of any case in which it will break anything,
      [...] but if that happens, it may be necessary to add the execution of
      the control methods in question directly to the initialization part."
      Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
      Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
      Reviewed-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      Acked-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>  # drivers/pci/hotplug/rpa*
      Acked-by: Sebastian Ott <sebott@linux.ibm.com>        # drivers/pci/hotplug/s390*
      Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> # drivers/platform/x86
      Cc: Len Brown <lenb@kernel.org>
      Cc: Scott Murray <scott@spiteful.org>
      Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
      Cc: Paul Mackerras <paulus@samba.org>
      Cc: Michael Ellerman <mpe@ellerman.id.au>
      Cc: Oliver OHalloran <oliveroh@au1.ibm.com>
      Cc: Gavin Shan <gwshan@linux.vnet.ibm.com>
      Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
      Cc: Corentin Chary <corentin.chary@gmail.com>
      Cc: Darren Hart <dvhart@infradead.org>
      a7da2161
    • Lukas Wunner's avatar
      PCI: hotplug: Constify hotplug_slot_ops · 81c4b5bf
      Lukas Wunner authored
      Hotplug drivers cannot declare their hotplug_slot_ops const, making them
      attractive targets for attackers, because upon registration of a hotplug
      slot, __pci_hp_initialize() writes to the "owner" and "mod_name" members
      in that struct.
      
      Fix by moving these members to struct hotplug_slot and constify every
      driver's hotplug_slot_ops except for pciehp.
      
      pciehp constructs its hotplug_slot_ops at runtime based on the PCIe
      port's capabilities, hence cannot declare them const.  It can be
      converted to __write_rarely once that's mainlined:
      http://www.openwall.com/lists/kernel-hardening/2016/11/16/3Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
      Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
      Reviewed-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
      Acked-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>  # drivers/pci/hotplug/rpa*
      Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> # drivers/platform/x86
      Cc: Len Brown <lenb@kernel.org>
      Cc: Scott Murray <scott@spiteful.org>
      Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
      Cc: Paul Mackerras <paulus@samba.org>
      Cc: Michael Ellerman <mpe@ellerman.id.au>
      Cc: Oliver OHalloran <oliveroh@au1.ibm.com>
      Cc: Gavin Shan <gwshan@linux.vnet.ibm.com>
      Cc: Sebastian Ott <sebott@linux.vnet.ibm.com>
      Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
      Cc: Corentin Chary <corentin.chary@gmail.com>
      Cc: Darren Hart <dvhart@infradead.org>
      81c4b5bf
    • Lukas Wunner's avatar
      PCI: pciehp: Reshuffle controller struct for clarity · d7587142
      Lukas Wunner authored
      The members in pciehp's controller struct are arranged in a seemingly
      arbitrary order and have grown to an amount that I no longer consider
      easily graspable by contributors.
      
      Sort the members into 5 rubrics:
      * Slot Capabilities register and quirks
      * Slot Control register access
      * Slot Status register event handling
      * state machine
      * hotplug core interface
      
      Obviously, this is just my personal bikeshed color and if anyone has a
      better idea, please come forward.  Any ordering will do as long as the
      information is presented in a manageable manner.
      
      No functional change intended.
      Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
      Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
      d7587142
    • Lukas Wunner's avatar
      PCI: pciehp: Rename controller struct members for clarity · 4ff3126e
      Lukas Wunner authored
      Of the members which were just moved from pciehp's slot struct to the
      controller struct, rename "lock" to "state_lock" and rename "work" to
      "button_work" for clarity.  Perform the rename separately to the
      unification of the two structs per Sinan's request.
      
      No functional change intended.
      Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
      Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
      Cc: Sinan Kaya <okaya@kernel.org>
      4ff3126e
    • Lukas Wunner's avatar
      PCI: pciehp: Unify controller and slot structs · 5790a9c7
      Lukas Wunner authored
      pciehp was originally introduced together with shpchp in a single
      commit, c16b4b14 ("PCI Hotplug: Add SHPC and PCI Express hot-plug
      drivers"):
      https://git.kernel.org/tglx/history/c/c16b4b14d980
      
      shpchp supports up to 31 slots per controller, hence uses separate slot
      and controller structs.  pciehp has a 1:1 relationship between slot and
      controller and therefore never required this separation.  Nevertheless,
      because much of the code had been copy-pasted between the two drivers,
      pciehp likewise uses separate structs to this very day.
      
      The artificial separation of data structures adds unnecessary complexity
      and bloat to pciehp and requires constantly chasing pointers at runtime.
      
      Simplify the driver by merging struct slot into struct controller.
      Merge the slot constructor pcie_init_slot() and the destructor
      pcie_cleanup_slot() into the controller counterparts.
      
      No functional change intended.
      Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
      Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
      5790a9c7
    • Lukas Wunner's avatar
      PCI: pciehp: Tolerate Presence Detect hardwired to zero · 80696f99
      Lukas Wunner authored
      The WiGig Bus Extension (WBE) specification allows tunneling PCIe over
      IEEE 802.11.  A product implementing this spec is the wil6210 from
      Wilocity (now part of Qualcomm Atheros).  It integrates a PCIe switch
      with a wireless network adapter:
      
        00.0-+              [1ae9:0101]  Upstream Port
             +-00.0-+       [1ae9:0200]  Downstream Port
             |      +-00.0  [168c:0034]  Atheros AR9462 Wireless Network Adapter
             +-02.0         [1ae9:0201]  Downstream Port
             +-03.0         [1ae9:0201]  Downstream Port
      
      Wirelessly attached devices presumably appear below the hotplug ports
      with device ID [1ae9:0201].  Oddly, the Downstream Port [1ae9:0200]
      leading to the wireless network adapter is likewise Hotplug Capable,
      but has its Presence Detect State bit hardwired to zero.  Even if the
      Link Active bit is set, Presence Detect is zero, so this cannot be
      caused by in-band presence detection but only by broken hardware.
      
      pciehp assumes an empty slot if Presence Detect State is zero,
      regardless of Link Active being one.  Consequently, up until v4.18 it
      removes the wireless network adapter in pciehp_resume().  From v4.19 it
      already does so in pciehp_probe().
      
      Be lenient towards broken hardware and assume the slot is occupied if
      Link Active is set:  Introduce pciehp_card_present_or_link_active()
      and use it in lieu of pciehp_get_adapter_status() everywhere, except
      in pciehp_handle_presence_or_link_change() whose log messages depend
      on which of Presence Detect State or Link Active is set.
      
      Remove the Presence Detect State check from __pciehp_enable_slot()
      because it is only called if either of Presence Detect State or Link
      Active is set.
      
      Caution: There is a possibility that broken hardware exists which has
      working Presence Detect but hardwires Link Active to one.  On such
      hardware the slot will now incorrectly be considered always occupied.
      If such hardware is discovered, this commit can be rolled back and a
      quirk can be added which sets is_hotplug_bridge = 0 for [1ae9:0200].
      
      Link: https://bugzilla.kernel.org/show_bug.cgi?id=200839Reported-and-tested-by: default avatarDavid Yang <mmyangfl@gmail.com>
      Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
      Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
      Cc: Rajat Jain <rajatja@google.com>
      Cc: Ashok Raj <ashok.raj@intel.com>
      80696f99