Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Nathan Lynch <nathanl@linux.ibm.com>
To:     Leonardo Bras <leobras.c@gmail.com>
Cc:     linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
        Michael Ellerman <mpe@ellerman.id.au>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Paul Mackerras <paulus@samba.org>,
        Allison Randal <allison@lohutok.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Nicholas Piggin <npiggin@gmail.com>,
        Leonardo Bras <leobras.c@gmail.com>,
        "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>,
        Nadav Amit <namit@vmware.com>
Subject: Re: [PATCH v3 2/2] powerpc/rtas: Implement reentrant rtas call
In-Reply-To: <20200514011245.127174-3-leobras.c@gmail.com>
References: <20200514011245.127174-1-leobras.c@gmail.com> <20200514011245.127174-3-leobras.c@gmail.com>
Date:   Thu, 14 May 2020 13:58:18 -0500
Message-ID: <878shu2vjp.fsf@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

Leonardo Bras <leobras.c@gmail.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?