Re: [PATCH v3 2/2] powerpc/rtas: Implement reentrant rtas call
On Thu, 2020-05-14 at 20:28 -0300, Leonardo Bras wrote:
> Yes, you are right.
> I will also add preempt_{dis,en}able, which in most kernels will
> compile out, but it will be kind of 'ready' if we ever decide to
> support PREEMPT.
>
> Thanks for the feedback!
v4:
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20200514235138.150722-3-leobra...@gmail.com/
Re: [PATCH v3 2/2] powerpc/rtas: Implement reentrant rtas call
Hello Nathan,
On Thu, 2020-05-14 at 13:58 -0500, Nathan Lynch wrote:
> Hi,
>
> Leonardo Bras writes:
> > +/**
> > + * rtas_call_reentrant() - Used for reentrant rtas calls
> > + * @token: Token for desired reentrant RTAS call
> > + * @nargs: Number of Input Parameters
> > + * @nret: Number of Output Parameters
> > + * @outputs: Array of outputs
> > + * @...: Inputs for desired RTAS call
> > + *
> > + * According to LoPAR documentation, only "ibm,int-on", "ibm,int-off",
> > + * "ibm,get-xive" and "ibm,set-xive" are currently reentrant.
> > + * Reentrant calls need their own rtas_args buffer, so not using
> > rtas.args, but
> > + * PACA one instead.
> > + *
> > + * Return: -1 on error,
> > + * First output value of RTAS call if (nret > 0),
> > + * 0 otherwise,
> > + */
> > +
> > +int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...)
> > +{
> > + va_list list;
> > + struct rtas_args *args;
> > + int i;
> > +
> > + if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
> > + return -1;
> > +
> > + /* We use the per-cpu (PACA) rtas args buffer */
> > + args = _paca->reentrant_args;
> > +
> > + va_start(list, outputs);
> > + va_rtas_call_unlocked(args, token, nargs, nret, list);
> > + va_end(list);
> > +
> > + if (nret > 1 && outputs)
> > + for (i = 0; i < nret - 1; ++i)
> > + outputs[i] = be32_to_cpu(args->rets[i + 1]);
>
> Doesn't this need to be more careful about preemption and exceptions?
> I.e. the args structure in the paca needs to be protected from
> concurrent use somehow, like any per-cpu data structure.
>
> local_irq_save/restore while accessing local_paca->reentrant_args here
> would be sufficient I think?
Yes, you are right.
I will also add preempt_{dis,en}able, which in most kernels will
compile out, but it will be kind of 'ready' if we ever decide to
support PREEMPT.
Thanks for the feedback!
Re: [PATCH v3 2/2] powerpc/rtas: Implement reentrant rtas call
Hi,
Leonardo Bras writes:
> +/**
> + * rtas_call_reentrant() - Used for reentrant rtas calls
> + * @token: Token for desired reentrant RTAS call
> + * @nargs: Number of Input Parameters
> + * @nret:Number of Output Parameters
> + * @outputs: Array of outputs
> + * @...: Inputs for desired RTAS call
> + *
> + * According to LoPAR documentation, only "ibm,int-on", "ibm,int-off",
> + * "ibm,get-xive" and "ibm,set-xive" are currently reentrant.
> + * Reentrant calls need their own rtas_args buffer, so not using rtas.args,
> but
> + * PACA one instead.
> + *
> + * Return: -1 on error,
> + * First output value of RTAS call if (nret > 0),
> + * 0 otherwise,
> + */
> +
> +int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...)
> +{
> + va_list list;
> + struct rtas_args *args;
> + int i;
> +
> + if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
> + return -1;
> +
> + /* We use the per-cpu (PACA) rtas args buffer */
> + args = _paca->reentrant_args;
> +
> + va_start(list, outputs);
> + va_rtas_call_unlocked(args, token, nargs, nret, list);
> + va_end(list);
> +
> + if (nret > 1 && outputs)
> + for (i = 0; i < nret - 1; ++i)
> + outputs[i] = be32_to_cpu(args->rets[i + 1]);
Doesn't this need to be more careful about preemption and exceptions?
I.e. the args structure in the paca needs to be protected from
concurrent use somehow, like any per-cpu data structure.
local_irq_save/restore while accessing local_paca->reentrant_args here
would be sufficient I think?
