[Skiboot] [PATCH V3]: VAS: Alloc IRQ and port address for each VAS instance

Oliver O'Halloran oohall at gmail.com
Mon Oct 21 19:04:44 AEDT 2019


On Sat, Oct 19, 2019 at 4:34 PM Haren Myneni <haren at linux.vnet.ibm.com> wrote:
>
>
> Setup IRQ and trigger port for each VAS instance. Export these
> values through device-tree with 'interrupts' and 'ibm,vas-port'
> properties in each VAS device node. Kernel setup IRQ and register
> port address for each send window.
>
> Signed-off-by: Sukadev Bhattiprolu <sukadev at linux.vnet.ibm.com>
> Signed-off-by: Haren Myneni <haren at us.ibm.com>
> ---
> Changelog:
> V2: Use standard 'interrupts' property as suggested by Oliver
> V3: Do not return if can not allocate interrupt. Needed only
>     for user space.

Thanks for making those changes.

>  /*
> @@ -423,6 +432,27 @@ static void disable_vas_inst(struct dt_node *np)
>         reset_north_ctl(chip);
>  }
>
> +static void vas_setup_irq(struct proc_chip *chip)
> +{
> +
> +       uint32_t irq;
> +       uint64_t port;

Squash the spurious newline, also swap these so they're in reverse
christmastree style.

> +
> +       irq = xive_alloc_ipi_irqs(chip->id, 1, 64);
> +       if (irq == XIVE_IRQ_ERROR) {
> +               vas_err("Failed to allocate interrupt sources for chipID %d\n",
> +                               chip->id);
> +               return;
> +       }
> +
> +       vas_vdbg("trigger port: 0x%p\n", xive_get_trigger_port(irq));
> +
> +       port = (uint64_t)xive_get_trigger_port(irq);
> +
> +       chip->vas->vas_irq = irq;
> +       chip->vas->vas_port = port;
> +}
> +

>  /*
>   * Initialize one VAS instance and enable it if @enable is true.
>   */
> @@ -452,6 +482,8 @@ static int init_vas_inst(struct dt_node *np, bool enable)
>                                 init_rma(chip))
>                 return -1;
>
> +       vas_setup_irq(chip);

Given that this has been disabled since forever I think it would be a
good idea to make it an opt-in feature for the moment rather than
enabling it by default, so can you gate it behind and nvram option?
e.g.

if (nvram_query_eq_dangerous("vas-irq", "1"))
    vas_setup_irq(chip);

There's also nvram_query_eq_safe(), the only real difference between
thw two is that the _dangerous() variant prints a warning to the
console if it's used to stop people using it in a test environment.

>         create_mm_dt_node(chip);
>
>         prlog(PR_INFO, "VAS: Initialized chip %d\n", chip->id);
>
>


More information about the Skiboot mailing list