Project

General

Profile

Actions

Bug #11700

closed

DDI hotplug request handler resets connection handle state before performing state change operations

Added by Rob Johnston over 2 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

This is to track upstreaming the following issue from illumos-joyent:

OS-7969 DDI hotplug request handler resets connection handle state before performing state change operations

For details, rationale and testing notes, please refer to the original SmartOS issue:

https://smartos.org/bugview/OS-7969

Actions #1

Updated by Rob Johnston over 2 years ago

There are two primary points of entry to request a state change in the DDI hotplug framework:

ndi_hp_state_change_req, a nexus driver interface that is called by the PCIe hotplug controller in the interrupt path.
ddihp_modctl, the implementation of modctl(2) for the MODHPOPS operation.
The first entry point is relevant for surprise removal of a device, as the PCIe hotplug controller handles hotplug interrupts for hotplug-capable PCIe buses. The second entry point is used by userland programs that do coordinated removal cfgadm(1M and hotplug(1M), via hotplugd.

To perform a state change operation, ddihp_modctl calls into the connection's hotplug operations function with the operation DDI_HPOP_CN_CHANGE_STATE. For both the port and connector operations functions (connections are one of either a port or a connector, each with their own hotplug operations function), the DDI_HPOP_CN_CHANGE_STATE operation will update its knowledge of the state in the DDI framework's connection handle structure, and perform other operations if the requested state and the current state in the connection handle are different.

ndi_hp_state_change_req will also eventually kick off a DDI_HPOP_CN_CHANGE_STATE operation on the connection vector, as ddihp_modctl does. But first, it kicks off a DDI_HPOP_CN_GET_STATE operation to fetch the state again and updates the DDI hotplug framework's state saved on the connection's handle. In the case of a PCIe slot, this action is reading the same registers that caused the interrupt that kicked off ndi_hp_state_change_req in the first place. Later, when the connection's state change operation implementation compares the state the connection knows about with the state that was requested of it, these states will be the same -- the hotplug interrupt that kicked it off already read that state, after all -- so the framework won't do anything in response to the state change.

We should ensure that the entry point for the interrupt path does not reset the DDI framework's notion of the connection state before kicking off the state change operations.

Actions #2

Updated by Electric Monk about 1 year ago

  • Gerrit CR set to 1381
Actions #3

Updated by Electric Monk about 1 year ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit ffb6483089015eb90be1f5e7fc2a96c9929546a6

commit  ffb6483089015eb90be1f5e7fc2a96c9929546a6
Author: Jordan Paige Hendricks <jordan.hendricks@joyent.com>
Date:   2021-04-06T00:17:41.000Z

    11698 Want NVMe Hotplug Support
    11699 x86 pci configurator should not fail device teardown if device is gone
    11700 DDI hotplug request handler resets connection handle state before performing state change operations
    11701 ldi_handle dcmd segfaults occasionally
    Reviewed by: Robert Mustacchi <rm@joyent.com>
    Reviewed by: Rob Johnston <rob.johnston@joyent.com>
    Reviewed by: Paul Winder <paul@winder.uk.net>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF