[Skiboot] [PATCH] npu2-opencapi: Don't send commands to NPU when link is down

[Frederic Barrat]

Le 17/07/2018 à 04:18, Oliver a écrit :
> On Mon, Jul 16, 2018 at 7:13 PM, Frederic Barrat <fbarrat at linux.ibm.com> wrote:
>> Even if the link is down, the PCI scan framework always try to access
>> the root device ("scan upstream only"). For opencapi, there's no root
>> device, so we may try to issue a config operation when the link is
>> down. The operation fails, but it raises a FIR bit and can trigger an
>> HMI.
> 
> The PCI core will only attempt to scan a device on a bus if the
> corresponding bit is set in the scan_map of the phb structure.
> For a real PHB we set the scan_map to 0x1 because there's
> always the RC that needs to be scanned.
> 
>   The real problem here is that npu2_opencapi_setup_device()
> sets the scan_map to 1 for the virtual PHB so I'd say the real fix
> here is to initialise it to zero and set the scan_map appropriately
> once the link is trained.


Agreed, that would be nicer.

   Fred



>> To fix it, we now keep track of the link state and avoid sending any
>> configuration space operations to the NPU if we know the link is not
>> ready.
>>
>>
>> Signed-off-by: Frederic Barrat <fbarrat at linux.ibm.com>
>> ---
>>   hw/npu2-opencapi.c | 10 +++++++++-
>>   include/npu2.h     |  1 +
>>   2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/npu2-opencapi.c b/hw/npu2-opencapi.c
>> index f82e6562..474ac0fb 100644
>> --- a/hw/npu2-opencapi.c
>> +++ b/hw/npu2-opencapi.c
>> @@ -1107,6 +1107,7 @@ static int64_t npu2_opencapi_poll_link(struct pci_slot *slot)
>>                  return pci_slot_set_sm_timeout(slot, msecs_to_tb(1));
>>
>>          case OCAPI_SLOT_LINK_TRAINED:
>> +               dev->link_down = false;
>>                  otl_enabletx(chip_id, dev->npu->xscom_base, dev);
>>                  pci_slot_set_state(slot, OCAPI_SLOT_NORMAL);
>>                  return OPAL_SUCCESS;
>> @@ -1137,7 +1138,7 @@ static int64_t npu2_opencapi_freset(struct pci_slot *slot)
>>          case OCAPI_SLOT_NORMAL:
>>          case OCAPI_SLOT_FRESET_START:
>>                  OCAPIDBG(dev, "FRESET starts\n");
>> -
>> +               dev->link_down = true;
>>                  if (slot->ops.get_presence_state)
>>                          slot->ops.get_presence_state(slot, &presence);
>>                  if (!presence) {
>> @@ -1261,6 +1262,9 @@ static int64_t npu2_opencapi_pcicfg_read(struct phb *phb, uint32_t bdfn,
>>          if (rc)
>>                  return rc;
>>
>> +       if (dev->link_down)
>> +               return OPAL_HARDWARE;
>> +
>>          genid_base = dev->bars[1].npu2_bar.base +
>>                  (index_to_block(dev->index) == NPU2_BLOCK_OTL1 ? 256 : 0);
>>
>> @@ -1319,6 +1323,9 @@ static int64_t npu2_opencapi_pcicfg_write(struct phb *phb, uint32_t bdfn,
>>          if (rc)
>>                  return rc;
>>
>> +       if (dev->link_down)
>> +               return OPAL_HARDWARE;
>> +
>>          genid_base = dev->bars[1].npu2_bar.base +
>>                  (index_to_block(dev->index) == NPU2_BLOCK_OTL1 ? 256 : 0);
>>
>> @@ -1637,6 +1644,7 @@ static void npu2_opencapi_setup_device(struct dt_node *dn_link, struct npu2 *n,
>>          dev->bdfn = 0;
>>          dev->train_need_fence = false;
>>          dev->train_fenced = false;
>> +       dev->link_down = true;
>>          n->total_devices++;
>>
>>          /* Find I2C port for handling device reset */
>> diff --git a/include/npu2.h b/include/npu2.h
>> index 4c2e20e0..c151f0e3 100644
>> --- a/include/npu2.h
>> +++ b/include/npu2.h
>> @@ -138,6 +138,7 @@ struct npu2_dev {
>>          uint64_t                i2c_port_id_ocapi;
>>          bool                    train_need_fence;
>>          bool                    train_fenced;
>> +       bool                    link_down;
>>   };
>>
>>   struct npu2 {
>> --
>> 2.17.1
>>
>> _______________________________________________
>> Skiboot mailing list
>> Skiboot at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/skiboot
>