| Message ID | 20190909123151.21944-4-fbarrat@linux.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | opencapi: enable card reset and link retraining | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (470ffb5f29d741c3bed600f7bb7bf0cbb270e05a) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On 09/09/2019 14:31, Frederic Barrat wrote: > PHB slots don't have an associated device (slot->pd = NULL). They were > not used by the PCI hotplug framework so far, but with opencapi > virtual PHBs, that's changing. With opencapi, devices are directly > under the PHB (no root complex or intermediate bridge) and the slot > used for hotplug is the PHB slot. > > This patch uses the proper phandle when replying asynchronously to the > OS when using a PHB slot. > > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> > --- Reviewed-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
On 9/9/19 2:31 pm, Frederic Barrat wrote: > PHB slots don't have an associated device (slot->pd = NULL). They were > not used by the PCI hotplug framework so far, but with opencapi > virtual PHBs, that's changing. With opencapi, devices are directly > under the PHB (no root complex or intermediate bridge) and the slot > used for hotplug is the PHB slot. > > This patch uses the proper phandle when replying asynchronously to the > OS when using a PHB slot. > > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> Looks sensible. I've also had a look through opal_pci_set_power_state() and I don't see any way that we can trigger a null dereference by trying to call that API with a virtual PHB in the current code, as an OpenCAPI PHB has neither a pd or the slot ops required for slot power-on/off. Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> > --- > core/pci-opal.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/core/pci-opal.c b/core/pci-opal.c > index 175810d0..69bd65c6 100644 > --- a/core/pci-opal.c > +++ b/core/pci-opal.c > @@ -647,6 +647,17 @@ static int64_t opal_pci_get_power_state(uint64_t id, uint64_t data) > } > opal_call(OPAL_PCI_GET_POWER_STATE, opal_pci_get_power_state, 2); > > +static u32 get_slot_phandle(struct pci_slot *slot) > +{ > + struct phb *phb = slot->phb; > + struct pci_device *pd = slot->pd; > + > + if (pd) > + return pd->dn->phandle; > + else > + return phb->dt_node->phandle; > +} > + > static void rescan_slot_devices(struct pci_slot *slot) > { > struct phb *phb = slot->phb; > @@ -671,8 +682,6 @@ static void set_power_timer(struct timer *t __unused, void *data, > uint64_t now __unused) > { > struct pci_slot *slot = data; > - struct pci_device *pd = slot->pd; > - struct dt_node *dn = pd->dn; > uint8_t link; > struct phb *phb = slot->phb; > > @@ -682,7 +691,7 @@ static void set_power_timer(struct timer *t __unused, void *data, > if (slot->retries-- == 0) { > pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL); > opal_queue_msg(OPAL_MSG_ASYNC_COMP, NULL, NULL, > - slot->async_token, dn->phandle, > + slot->async_token, get_slot_phandle(slot), > slot->power_state, OPAL_BUSY); > } else { > schedule_timer(&slot->timer, msecs_to_tb(10)); > @@ -694,7 +703,7 @@ static void set_power_timer(struct timer *t __unused, void *data, > remove_slot_devices(slot); > pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL); > opal_queue_msg(OPAL_MSG_ASYNC_COMP, NULL, NULL, > - slot->async_token, dn->phandle, > + slot->async_token, get_slot_phandle(slot), > OPAL_PCI_SLOT_POWER_OFF, OPAL_SUCCESS); > break; > } > @@ -706,12 +715,12 @@ static void set_power_timer(struct timer *t __unused, void *data, > rescan_slot_devices(slot); > pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL); > opal_queue_msg(OPAL_MSG_ASYNC_COMP, NULL, NULL, > - slot->async_token, dn->phandle, > + slot->async_token, get_slot_phandle(slot), > OPAL_PCI_SLOT_POWER_ON, OPAL_SUCCESS); > } else if (slot->retries-- == 0) { > pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL); > opal_queue_msg(OPAL_MSG_ASYNC_COMP, NULL, NULL, > - slot->async_token, dn->phandle, > + slot->async_token, get_slot_phandle(slot), > OPAL_PCI_SLOT_POWER_ON, OPAL_BUSY); > } else { > schedule_timer(&slot->timer, msecs_to_tb(10)); >
diff --git a/core/pci-opal.c b/core/pci-opal.c index 175810d0..69bd65c6 100644 --- a/core/pci-opal.c +++ b/core/pci-opal.c @@ -647,6 +647,17 @@ static int64_t opal_pci_get_power_state(uint64_t id, uint64_t data) } opal_call(OPAL_PCI_GET_POWER_STATE, opal_pci_get_power_state, 2); +static u32 get_slot_phandle(struct pci_slot *slot) +{ + struct phb *phb = slot->phb; + struct pci_device *pd = slot->pd; + + if (pd) + return pd->dn->phandle; + else + return phb->dt_node->phandle; +} + static void rescan_slot_devices(struct pci_slot *slot) { struct phb *phb = slot->phb; @@ -671,8 +682,6 @@ static void set_power_timer(struct timer *t __unused, void *data, uint64_t now __unused) { struct pci_slot *slot = data; - struct pci_device *pd = slot->pd; - struct dt_node *dn = pd->dn; uint8_t link; struct phb *phb = slot->phb; @@ -682,7 +691,7 @@ static void set_power_timer(struct timer *t __unused, void *data, if (slot->retries-- == 0) { pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL); opal_queue_msg(OPAL_MSG_ASYNC_COMP, NULL, NULL, - slot->async_token, dn->phandle, + slot->async_token, get_slot_phandle(slot), slot->power_state, OPAL_BUSY); } else { schedule_timer(&slot->timer, msecs_to_tb(10)); @@ -694,7 +703,7 @@ static void set_power_timer(struct timer *t __unused, void *data, remove_slot_devices(slot); pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL); opal_queue_msg(OPAL_MSG_ASYNC_COMP, NULL, NULL, - slot->async_token, dn->phandle, + slot->async_token, get_slot_phandle(slot), OPAL_PCI_SLOT_POWER_OFF, OPAL_SUCCESS); break; } @@ -706,12 +715,12 @@ static void set_power_timer(struct timer *t __unused, void *data, rescan_slot_devices(slot); pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL); opal_queue_msg(OPAL_MSG_ASYNC_COMP, NULL, NULL, - slot->async_token, dn->phandle, + slot->async_token, get_slot_phandle(slot), OPAL_PCI_SLOT_POWER_ON, OPAL_SUCCESS); } else if (slot->retries-- == 0) { pci_slot_set_state(slot, PCI_SLOT_STATE_NORMAL); opal_queue_msg(OPAL_MSG_ASYNC_COMP, NULL, NULL, - slot->async_token, dn->phandle, + slot->async_token, get_slot_phandle(slot), OPAL_PCI_SLOT_POWER_ON, OPAL_BUSY); } else { schedule_timer(&slot->timer, msecs_to_tb(10));
PHB slots don't have an associated device (slot->pd = NULL). They were not used by the PCI hotplug framework so far, but with opencapi virtual PHBs, that's changing. With opencapi, devices are directly under the PHB (no root complex or intermediate bridge) and the slot used for hotplug is the PHB slot. This patch uses the proper phandle when replying asynchronously to the OS when using a PHB slot. Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> --- core/pci-opal.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)