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@xxxxxxxxxxxxxxxxxx> 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
