Re: [PATCH 04/14] powerpc/vas: Setup IRQ mapping and register port for each window

[Haren Myneni <haren@linux.ibm.com>]

On Wed, 2019-12-18 at 18:18 +1100, Oliver O'Halloran wrote:
> On Wed, Nov 27, 2019 at 12:07 PM Haren Myneni <haren@linux.vnet.ibm.com> wrote:
> >
> > *snip*
> >
> > @@ -36,7 +62,18 @@ static int init_vas_instance(struct platform_device *pdev)
> >                 return -ENODEV;
> >         }
> >
> > -       if (pdev->num_resources != 4) {
> > +       rc = of_property_read_u64(dn, "ibm,vas-port", &port);
> > +       if (rc) {
> > +               pr_err("No ibm,vas-port property for %s?\n", pdev->name);
> > +               /* No interrupts property */
> > +               nresources = 4;
> > +       }
> > +
> > +       /*
> > +        * interrupts property is available with 'ibm,vas-port' property.
> > +        * 4 Resources and 1 IRQ if interrupts property is available.
> > +        */
> > +       if (pdev->num_resources != nresources) {
> >                 pr_err("Unexpected DT configuration for [%s, %d]\n",
> >                                 pdev->name, vasid);
> >                 return -ENODEV;
> 
> Right, so adding the IRQ in firmware will break the VAS driver in
> existing kernels since it changes the resource count. This is IMO a
> bug in the VAS driver that you should fix, but it does mean we need to
> think twice about having firmware assign an interrupt at boot.

Correct, Hence added vas-user-space nvram switch in skiboot.  

> 
> I had a closer look at this series and I'm not convinced that any
> firmware changes are actually required either. We already have OPAL
> calls for allocating an hwirq for the kernel to use and for getting
> the IRQ's XIVE trigger port (see pnv_ocxl_alloc_xive_irq() for an
> example). Why not use those here too? Doing so would allow us to
> assign interrupts to individual windows too which might be useful for
> the windows used by the kernel.

Thanks for the pointer. like using pnv_ocxl_alloc_xive_irq(), we can
disregard FW change. BTW, VAS fault handling is needed only for user
space VAS windows. 

 int vas_alloc_xive_irq(u32 chipid, u32 *irq, u64 *trigger_addr)
{
        __be64 flags, trigger_page;
        u32 hwirq;
        s64 rc;

        hwirq = opal_xive_allocate_irq_raw(chipid);
        if (hwirq < 0)
                return -ENOENT;

        rc = opal_xive_get_irq_info(hwirq, &flags, NULL, &trigger_page,
NULL,
                                NULL);
        if (rc || !trigger_page) {
                xive_native_free_irq(hwirq);
                return -ENOENT;
        }

        *irq = hwirq;
        *trigger_addr = be64_to_cpu(trigger_page);
        return 0;
}

We can have common function for VAS and cxl except per chip IRQ
allocation is needed for each VAS instance. I will post patch-set with
this change.

Thanks
Haren