Re: [PATCH 4/4] xen/ppc: Implement early serial console on PowerNV

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.08.2023 02:11, Shawn Anastasio wrote:
> Implement the OPAL firmware calls required to write to the serial
> console on PowerNV systems. Unlike pseries/Open Firmware, the OPAL
> firmware interface can be used past early boot and as such the relevant
> functions are not marked as __init.
> 
> Signed-off-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>

Constraint as before and with two nits
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

> @@ -20,8 +22,20 @@
>   * Load the address of a symbol from the TOC into the specified GPR.
>   */
>  #define LOAD_REG_ADDR(reg,name)                                              
> \
> -    addis reg,%r2,name@toc@ha;                                               
> \
> -    addi  reg,reg,name@toc@l
> +    addis reg, %r2, name@toc@ha;                                             
> \
> +    addi  reg, reg, name@toc@l
> +
> +/*
> + * Declare a global assembly function with a proper TOC setup prologue
> + */
> +#define _GLOBAL_TOC(name)                                                   \
> +    .balign 4;                                                              \
> +    .type name, @function;                                                  \
> +    .globl name;                                                            \
> +name:                                                                       \
> +0:  addis %r2, %r12, (.TOC.-0b)@ha;                                         \
> +    addi  %r2, %r2, (.TOC.-0b)@l;                                           \

Strictly speaking the - want surrounding by blanks, but I wonder whether
to PPC eyes these constructs look more natural without. Please clarify.

> --- /dev/null
> +++ b/xen/arch/ppc/ppc64/opal-calls.S
> @@ -0,0 +1,81 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Adapted from Linux's arch/powerpc/boot/opal-calls.S
> + *
> + * Copyright (c) 2016 IBM Corporation.
> + * Copyright Raptor Engineering, LLC
> + */
> +
> +#include <asm/asm-defns.h>
> +#include <asm/opal-api.h>
> +#include <asm/msr.h>
> +
> +    .text
> +
> +#define OPAL_CALL(name, token)  \
> +    .globl name;                \
> +name:                           \
> +    li      %r0, token;         \
> +    b       opal_call
> +
> + _GLOBAL_TOC(opal_call)

Any reason for the leading blank here?

Where necessary I again think these small items can be taken care of
while committing.

Jan