[Skiboot] [PATCH v2 5/8] npu2: Refactor NPU OPAL calls

[Oliver O'Halloran]

On Mon, Jul 15, 2019 at 3:53 PM Alexey Kardashevskiy <aik at ozlabs.ru> wrote:
>
> On 09/07/2019 07:07, Reza Arbab wrote:
>  > *snip*
> > +#include <skiboot.h>
> > +#include <pci.h>
> > +#include <phb4.h>
> > +#include <npu2.h>
> > +
> > +static int64_t opal_npu_init_context(uint64_t phb_id, int pid __unused,
> > +                                  uint64_t msr, uint64_t bdf)
> > +{
> > +     struct phb *phb = pci_get_phb(phb_id);
> > +
> > +     if (!phb || phb->phb_type != phb_type_npu_v2)
>
>
> This and the same chunk but with phb_type_npu_v3 in 7/8 suggest these
> opal_npu_init_context/etc should go to phb_ops or even create an
> phb_npu_ops and add callback there for every case where you do "phb_type
> != phb_type_npu_vX" no?

I don't mind it tbh. An ops callback structure is preferable when all
the cases get too annoying to enumerate or when you need to keep
everything in separate compilation units. We have neither problem here
and the direct function calls added in 7/8 make the code easier to
browse compared to indirect calls.

> > +             return OPAL_PARAMETER;
> > +
> > +     return npu2_init_context(phb, msr, bdf);
> > +}
> > +opal_call(OPAL_NPU_INIT_CONTEXT, opal_npu_init_context, 4);
> > +
> > +static int64_t opal_npu_destroy_context(uint64_t phb_id, uint64_t pid __unused,
> > +                                     uint64_t bdf)
> > +{
> > +     struct phb *phb = pci_get_phb(phb_id);
> > +
> > +     if (!phb || phb->phb_type != phb_type_npu_v2)
> > +             return OPAL_PARAMETER;
> > +
> > +     return npu2_destroy_context(phb, bdf);
> > +}
> > +opal_call(OPAL_NPU_DESTROY_CONTEXT, opal_npu_destroy_context, 3);
> > +
> > +static int64_t opal_npu_map_lpar(uint64_t phb_id, uint64_t bdf, uint64_t lparid,
> > +                              uint64_t lpcr)
> > +{
> > +     struct phb *phb = pci_get_phb(phb_id);
> > +
> > +     if (!phb || phb->phb_type != phb_type_npu_v2)
> > +             return OPAL_PARAMETER;
> > +
> > +     return npu2_map_lpar(phb, bdf, lparid, lpcr);
> > +}
> > +opal_call(OPAL_NPU_MAP_LPAR, opal_npu_map_lpar, 4);
> > +
> > +static int npu_check_relaxed_ordering(struct phb *phb, struct pci_device *pd,
> > +                                   void *enable)
> > +{
> > +     /*
> > +      * IBM PCIe bridge devices (ie. the root ports) can always allow relaxed
> > +      * ordering
> > +      */
> > +     if (pd->vdid == 0x04c11014)
>
>
> Nit (I do realize this moved unchanged from elsewhere): I am a bit
> surprised not to see vendor ids defined in include/pci-cfg.h. Do we know
> for sure the device id won't change?

If you want to do this then make a pci-ids.h and dump them in there.
We've got a few VDIDs around that could go in there.

Oliver