linux-kernel - Re: [PATCH v3 2/2] powerpc/rtas: Implement reentrant rtas call

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 May 2020 20:28:15 -0300
From:   Leonardo Bras <leobras.c@...il.com>
To:     Nathan Lynch <nathanl@...ux.ibm.com>
Cc:     linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
        Michael Ellerman <mpe@...erman.id.au>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Allison Randal <allison@...utok.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Nicholas Piggin <npiggin@...il.com>,
        "Gautham R. Shenoy" <ego@...ux.vnet.ibm.com>,
        Nadav Amit <namit@...are.com>
Subject: 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 <leobras.c@...il.com> 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 = &local_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!