On Fri, 2022-11-18 at 09:07 -0600, Nathan Lynch wrote:
> The code in rtas_get_error_log_max() doesn't cause problems in
> practice, but there are no measures to ensure that the lazy
> initialization of the static rtas_error_log_max variable is atomic,
> and it's not worth adding them.
>
> Initialize the static rtas_error_log_max variable at boot when we're
> single-threaded instead of lazily on first use. Use the more
> appropriate of_property_read_u32() API instead of rtas_token() to
> consult the "rtas-error-log-max" property, which is not the name of
> an
> RTAS function. Convert use of printk() to pr_warn() and distinguish
> the possible error cases.
>
> Signed-off-by: Nathan Lynch <nath...@linux.ibm.com>
Reviewed-by: Andrew Donnellan <a...@linux.ibm.com>
> ---
> arch/powerpc/kernel/rtas.c | 37 ++++++++++++++++++++++++++----------
> -
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 51f0508593a7..3fa84c247415 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -353,6 +353,9 @@ int rtas_service_present(const char *service)
> EXPORT_SYMBOL(rtas_service_present);
>
> #ifdef CONFIG_RTAS_ERROR_LOGGING
> +
> +static u32 rtas_error_log_max __ro_after_init = RTAS_ERROR_LOG_MAX;
> +
> /*
> * Return the firmware-specified size of the error log buffer
> * for all rtas calls that require an error buffer argument.
> @@ -360,21 +363,30 @@ EXPORT_SYMBOL(rtas_service_present);
> */
> int rtas_get_error_log_max(void)
> {
> - static int rtas_error_log_max;
> - if (rtas_error_log_max)
> - return rtas_error_log_max;
> -
> - rtas_error_log_max = rtas_token ("rtas-error-log-max");
> - if ((rtas_error_log_max == RTAS_UNKNOWN_SERVICE) ||
> - (rtas_error_log_max > RTAS_ERROR_LOG_MAX)) {
> - printk (KERN_WARNING "RTAS: bad log buffer size
> %d\n",
> - rtas_error_log_max);
> - rtas_error_log_max = RTAS_ERROR_LOG_MAX;
> - }
> return rtas_error_log_max;
> }
> EXPORT_SYMBOL(rtas_get_error_log_max);
>
> +static void __init init_error_log_max(void)
> +{
> + static const char propname[] __initconst = "rtas-error-log-
> max";
> + u32 max;
> +
> + if (of_property_read_u32(rtas.dev, propname, &max)) {
> + pr_warn("%s not found, using default of %u\n",
> + propname, RTAS_ERROR_LOG_MAX);
> + max = RTAS_ERROR_LOG_MAX;
> + }
> +
> + if (max > RTAS_ERROR_LOG_MAX) {
> + pr_warn("%s = %u, clamping max error log size to
> %u\n",
> + propname, max, RTAS_ERROR_LOG_MAX);
> + max = RTAS_ERROR_LOG_MAX;
> + }
> +
> + rtas_error_log_max = max;
> +}
> +
>
> static char rtas_err_buf[RTAS_ERROR_LOG_MAX];
> static int rtas_last_error_token;
> @@ -432,6 +444,7 @@ static char *__fetch_rtas_last_error(char
> *altbuf)
> #else /* CONFIG_RTAS_ERROR_LOGGING */
> #define __fetch_rtas_last_error(x) NULL
> #define get_errorlog_buffer() NULL
> +static void __init init_error_log_max(void) {}
> #endif
>
>
> @@ -1341,6 +1354,8 @@ void __init rtas_initialize(void)
> no_entry = of_property_read_u32(rtas.dev, "linux,rtas-entry",
> &entry);
> rtas.entry = no_entry ? rtas.base : entry;
>
> + init_error_log_max();
> +
> /*
> * Discover these now to avoid device tree lookups in the
> * panic path.
--
Andrew Donnellan OzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
