[Skiboot] [PATCH skiboot] hw/phb3/naples: Disable D-states

[Oliver]

On Tue, Jan 29, 2019 at 1:53 PM Alexey Kardashevskiy <aik at ozlabs.ru> wrote:
>
>
>
> On 22/01/2019 15:27, Alexey Kardashevskiy wrote:
> >
> >
> > On 11/01/2019 16:11, Benjamin Herrenschmidt wrote:
> >> On Fri, 2019-01-11 at 15:56 +1100, David Gibson wrote:
> >>> On Fri, Jan 11, 2019 at 03:06:52PM +1100, Alexey Kardashevskiy wrote:
> >>>> Putting "Mellanox Technologies MT27700 Family [ConnectX-4] [15b3:1013]"
> >>>> (more precisely, the second of 2 its PCI functions, no matter in what
> >>>> order) into the D3 state causes EEH with the "PCT timeout" error.
> >>>> This has been noticed on garrison machines only and firestones do not
> >>>> seem to have this issue.
> >>>>
> >>>> This disables D-states changing for devices on root buses on Naples by
> >>>> installing a config space access filter (copied from PHB4).
> >>>>
> >>>> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
> >>>
> >>> Um.. what case is this trying to affect?  hw/phb3.c is only used with
> >>> pnv machines which are unlikely to be using passed through devices.
> >>
> >> This is the firmware. All config accesses go through it whether they
> >> come from host or guest. This effectively will block all attempts at
> >> changing D states on a directly attached device on Naples chip.
> >
> >
> > Care to ack? Thanks,
>
>
> Apparently not :(

*checks*

Yep, that's definitely a patch. I acknowledge that this is in fact a patch.

> >
> >>
> >> Cheers,
> >> Ben.
> >>
> >>>> ---
> >>>>  hw/phb3.c | 29 +++++++++++++++++++++++++++++
> >>>>  1 file changed, 29 insertions(+)
> >>>>
> >>>> diff --git a/hw/phb3.c b/hw/phb3.c
> >>>> index 771ccfc..04c8b78 100644
> >>>> --- a/hw/phb3.c
> >>>> +++ b/hw/phb3.c
> >>>> @@ -573,6 +573,22 @@ static void phb3_endpoint_init(struct phb *phb,
> >>>>    pci_cfg_write32(phb, bdfn, aercap + PCIECAP_AER_CAPCTL, val32);
> >>>>  }
> >>>>
> >>>> +static int64_t phb3_pcicfg_no_dstate(void *dev __unused,
> >>>> +                               struct pci_cfg_reg_filter *pcrf,
> >>>> +                               uint32_t offset, uint32_t len __unused,
> >>>> +                               uint32_t *data __unused,  bool write)
> >>>> +{
> >>>> +  uint32_t loff = offset - pcrf->start;
> >>>> +
> >>>> +  /* Disable D-state change on children of the PHB. For now we
> >>>> +   * simply block all writes to the PM control/status
> >>>> +   */
> >>>> +  if (write && loff >= 4 && loff < 6)
> >>>> +          return OPAL_SUCCESS;
> >>>> +
> >>>> +  return OPAL_PARTIAL;
> >>>> +}
> >>>> +
> >>>>  static void phb3_check_device_quirks(struct phb *phb, struct pci_device *dev)
> >>>>  {
> >>>>    struct phb3 *p = phb_to_phb3(phb);
> >>>> @@ -601,6 +617,19 @@ static void phb3_check_device_quirks(struct phb *phb, struct pci_device *dev)
> >>>>            else
> >>>>                    modectl &= ~PPC_BIT(14);
> >>>>            xscom_write(p->chip_id, p->pe_xscom + 0x0b, modectl);
> >>>> +
> >>>> +          /*
> >>>> +           * Naples has a problem with D-states at least on Mellanox CX4,
> >>>> +           * disable changing D-state on Naples like we do it for PHB4.
> >>>> +           */
> >>>> +          if (PHB3_IS_NAPLES(p) &&
> >>>> +              pci_has_cap(dev, PCI_CFG_CAP_ID_PM, false)) {
> >>>> +                  pci_add_cfg_reg_filter(dev,
> >>>> +                                  pci_cap(dev, PCI_CFG_CAP_ID_PM, false),
> >>>> +                                  8,
> >>>> +                                  PCI_REG_FLAG_WRITE,
> >>>> +                                  phb3_pcicfg_no_dstate);
> >>>> +          }
> >>>>    } else if (dev->primary_bus == 0) {
> >>>>            /*
> >>>>             * Emulate the prefetchable window of the root port
> >>
> >
>
> --
> Alexey