diff mbox series

[5/5] xen/ppc: Implement early serial console on PowerNV

Message ID 3023ad320b42fa3787bb71a9cf83b34965668fe9.1690579561.git.sanastasio@raptorengineering.com (mailing list archive)
State Superseded
Headers show
Series xen/ppc: Add PowerNV bare metal support | expand

Commit Message

Shawn Anastasio July 28, 2023, 9:35 p.m. UTC
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@raptorengineering.com>
---
 xen/arch/ppc/include/asm/asm-defns.h | 12 ++++
 xen/arch/ppc/opal.c                  | 27 ++++++++-
 xen/arch/ppc/ppc64/Makefile          |  1 +
 xen/arch/ppc/ppc64/asm-offsets.c     |  4 ++
 xen/arch/ppc/ppc64/opal-calls.S      | 82 ++++++++++++++++++++++++++++
 5 files changed, 125 insertions(+), 1 deletion(-)
 create mode 100644 xen/arch/ppc/ppc64/opal-calls.S

Comments

Jan Beulich Aug. 1, 2023, 11:19 a.m. UTC | #1
On 28.07.2023 23:35, Shawn Anastasio wrote:
> --- a/xen/arch/ppc/include/asm/asm-defns.h
> +++ b/xen/arch/ppc/include/asm/asm-defns.h
> @@ -23,6 +23,18 @@
>      addis reg,%r2,name@toc@ha;                                               \
>      addi  reg,reg,name@toc@l

Noticing only now, because of the issue ...

> +/*
> + * 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;                                              \
> +    .localentry name,.-name

... being widened - could we gain blanks after the commas? Down here
(to match the code in context) another padding blank after "addi"
would also be nice.

> --- a/xen/arch/ppc/opal.c
> +++ b/xen/arch/ppc/opal.c
> @@ -8,9 +8,28 @@
>  #include <xen/init.h>
>  #include <xen/lib.h>
>  
> -/* Global OPAL struct containing entrypoint and base */
> +/* Global OPAL struct containing entrypoint and base used by opal-calls.S */
>  struct opal opal;
>  
> +int64_t opal_console_write(int64_t term_number, uint64_t *length,
> +                           uint8_t *buffer);

Would this perhaps better be void *, eliminating the need for casting
in calls of this function?

> +int64_t opal_console_flush(int64_t term_number);
> +int64_t opal_reinit_cpus(uint64_t flags);
> +
> +static void opal_putchar(char c)

Can't this be __init?

> +{
> +    uint64_t len;
> +    if (c == '\n')

Nit: Blank line please between declaration(s) and statement(s). (At
least one more instance below.)

Also please add the missing blanks in the if(), seeing that otherwise
the file is aiming at being Xen style.

> +    {
> +        char buf = '\r';
> +        len = cpu_to_be64(1);
> +        opal_console_write(0, &len, (uint8_t *) &buf);
> +    }
> +    len = cpu_to_be64(1);
> +    opal_console_write(0, &len, (uint8_t *) &c);
> +    opal_console_flush(0);
> +}
> +
>  void __init boot_opal_init(const void *fdt)
>  {
>      int opal_node;
> @@ -45,4 +64,10 @@ void __init boot_opal_init(const void *fdt)
>  
>      opal.base = be64_to_cpu(*opal_base);
>      opal.entry = be64_to_cpu(*opal_entry);
> +
> +    early_printk_init(opal_putchar);
> +
> +    /* Ask OPAL to set HID0 for Little Endian interrupts + Radix TLB support */
> +    opal_reinit_cpus(OPAL_REINIT_CPUS_HILE_LE | OPAL_REINIT_CPUS_MMU_RADIX
> +                     | OPAL_REINIT_CPUS_MMU_HASH);

Nit: operators on continued lines go at the end of the earlier line.

> --- /dev/null
> +++ b/xen/arch/ppc/ppc64/opal-calls.S
> @@ -0,0 +1,82 @@
> +/* 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/asm-offsets.h>

Would it make sense to have asm-defns.h include asm-offsets.h, like
x86 and Arm do?

> +#include <asm/opal-api.h>
> +#include <asm/msr.h>
> +
> +    .text

Is any of this code still needed post-init?

> +#define OPAL_CALL(name, token)  \
> +    .globl name;                \
> +name:                           \
> +    li      %r0, token;         \
> +    b       opal_call;

I think the trailing semicolon wants omitting.

Jan
Shawn Anastasio Aug. 1, 2023, 10:54 p.m. UTC | #2
On 8/1/23 6:19 AM, Jan Beulich wrote:
> On 28.07.2023 23:35, Shawn Anastasio wrote:
>> --- a/xen/arch/ppc/include/asm/asm-defns.h
>> +++ b/xen/arch/ppc/include/asm/asm-defns.h
>> @@ -23,6 +23,18 @@
>>      addis reg,%r2,name@toc@ha;                                               \
>>      addi  reg,reg,name@toc@l
> 
> Noticing only now, because of the issue ...
> 
>> +/*
>> + * 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;                                              \
>> +    .localentry name,.-name
> 
> ... being widened - could we gain blanks after the commas? Down here
> (to match the code in context) another padding blank after "addi"
> would also be nice.

Sure, will do in v2.

>> --- a/xen/arch/ppc/opal.c
>> +++ b/xen/arch/ppc/opal.c
>> @@ -8,9 +8,28 @@
>>  #include <xen/init.h>
>>  #include <xen/lib.h>
>>  
>> -/* Global OPAL struct containing entrypoint and base */
>> +/* Global OPAL struct containing entrypoint and base used by opal-calls.S */
>>  struct opal opal;
>>  
>> +int64_t opal_console_write(int64_t term_number, uint64_t *length,
>> +                           uint8_t *buffer);
> 
> Would this perhaps better be void *, eliminating the need for casting
> in calls of this function?

I made it uint8_t to match the official OPAL API documentation (though I
now see I missed a `const`):
https://open-power.github.io/skiboot/doc/opal-api/opal-console-read-write-1-2.html#opal-console-write

In this case though, the type information of this parameter might not be
that important and changing it to void* to avoid the cast is fine with
me.

>> +int64_t opal_console_flush(int64_t term_number);
>> +int64_t opal_reinit_cpus(uint64_t flags);
>> +
>> +static void opal_putchar(char c)
> 
> Can't this be __init?

Unlike OpenFirmware, OPAL calls are expected to be used by the OS during
its entire lifecycle, not just during early boot, so the full
(non-early) serial console driver would likely want to use these
functions as well.

> 
>> +{
>> +    uint64_t len;
>> +    if (c == '\n')
> 
> Nit: Blank line please between declaration(s) and statement(s). (At
> least one more instance below.)

Will fix.

> Also please add the missing blanks in the if(), seeing that otherwise
> the file is aiming at being Xen style.

Ditto.

>> +    {
>> +        char buf = '\r';
>> +        len = cpu_to_be64(1);
>> +        opal_console_write(0, &len, (uint8_t *) &buf);
>> +    }
>> +    len = cpu_to_be64(1);
>> +    opal_console_write(0, &len, (uint8_t *) &c);
>> +    opal_console_flush(0);
>> +}
>> +
>>  void __init boot_opal_init(const void *fdt)
>>  {
>>      int opal_node;
>> @@ -45,4 +64,10 @@ void __init boot_opal_init(const void *fdt)
>>  
>>      opal.base = be64_to_cpu(*opal_base);
>>      opal.entry = be64_to_cpu(*opal_entry);
>> +
>> +    early_printk_init(opal_putchar);
>> +
>> +    /* Ask OPAL to set HID0 for Little Endian interrupts + Radix TLB support */
>> +    opal_reinit_cpus(OPAL_REINIT_CPUS_HILE_LE | OPAL_REINIT_CPUS_MMU_RADIX
>> +                     | OPAL_REINIT_CPUS_MMU_HASH);
> 
> Nit: operators on continued lines go at the end of the earlier line.

Will fix.

>> --- /dev/null
>> +++ b/xen/arch/ppc/ppc64/opal-calls.S
>> @@ -0,0 +1,82 @@
>> +/* 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/asm-offsets.h>
> 
> Would it make sense to have asm-defns.h include asm-offsets.h, like
> x86 and Arm do?

Sure, I'll make this change along with the formatting updates in
asm-defns.h

>> +#include <asm/opal-api.h>
>> +#include <asm/msr.h>
>> +
>> +    .text
> 
> Is any of this code still needed post-init?

Yes, see above.

>> +#define OPAL_CALL(name, token)  \
>> +    .globl name;                \
>> +name:                           \
>> +    li      %r0, token;         \
>> +    b       opal_call;
> 
> I think the trailing semicolon wants omitting.

Will fix.

> Jan

Thanks,
Shawn
Jan Beulich Aug. 2, 2023, 7:10 a.m. UTC | #3
On 02.08.2023 00:54, Shawn Anastasio wrote:
> On 8/1/23 6:19 AM, Jan Beulich wrote:
>> On 28.07.2023 23:35, Shawn Anastasio wrote:
>>> +static void opal_putchar(char c)
>>
>> Can't this be __init?
> 
> Unlike OpenFirmware, OPAL calls are expected to be used by the OS during
> its entire lifecycle, not just during early boot, so the full
> (non-early) serial console driver would likely want to use these
> functions as well.

Well, in the present usage it's unneeded post-init afaict. Hence if you
don't want to add __init until another use appears, please add a
respective remark in the description.

Jan
diff mbox series

Patch

diff --git a/xen/arch/ppc/include/asm/asm-defns.h b/xen/arch/ppc/include/asm/asm-defns.h
index 5821f9024d..54d578ea3b 100644
--- a/xen/arch/ppc/include/asm/asm-defns.h
+++ b/xen/arch/ppc/include/asm/asm-defns.h
@@ -23,6 +23,18 @@ 
     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;                                              \
+    .localentry name,.-name
+
 /*
  * Depending on how we were booted, the CPU could be running in either
  * Little Endian or Big Endian mode. The following trampoline from Linux
diff --git a/xen/arch/ppc/opal.c b/xen/arch/ppc/opal.c
index 251de8ac23..cc75ccfcbe 100644
--- a/xen/arch/ppc/opal.c
+++ b/xen/arch/ppc/opal.c
@@ -8,9 +8,28 @@ 
 #include <xen/init.h>
 #include <xen/lib.h>
 
-/* Global OPAL struct containing entrypoint and base */
+/* Global OPAL struct containing entrypoint and base used by opal-calls.S */
 struct opal opal;
 
+int64_t opal_console_write(int64_t term_number, uint64_t *length,
+                           uint8_t *buffer);
+int64_t opal_console_flush(int64_t term_number);
+int64_t opal_reinit_cpus(uint64_t flags);
+
+static void opal_putchar(char c)
+{
+    uint64_t len;
+    if (c == '\n')
+    {
+        char buf = '\r';
+        len = cpu_to_be64(1);
+        opal_console_write(0, &len, (uint8_t *) &buf);
+    }
+    len = cpu_to_be64(1);
+    opal_console_write(0, &len, (uint8_t *) &c);
+    opal_console_flush(0);
+}
+
 void __init boot_opal_init(const void *fdt)
 {
     int opal_node;
@@ -45,4 +64,10 @@  void __init boot_opal_init(const void *fdt)
 
     opal.base = be64_to_cpu(*opal_base);
     opal.entry = be64_to_cpu(*opal_entry);
+
+    early_printk_init(opal_putchar);
+
+    /* Ask OPAL to set HID0 for Little Endian interrupts + Radix TLB support */
+    opal_reinit_cpus(OPAL_REINIT_CPUS_HILE_LE | OPAL_REINIT_CPUS_MMU_RADIX
+                     | OPAL_REINIT_CPUS_MMU_HASH);
 }
diff --git a/xen/arch/ppc/ppc64/Makefile b/xen/arch/ppc/ppc64/Makefile
index f4956daaa9..b9a91dc15f 100644
--- a/xen/arch/ppc/ppc64/Makefile
+++ b/xen/arch/ppc/ppc64/Makefile
@@ -1,2 +1,3 @@ 
 obj-y += head.o
 obj-y += of-call.o
+obj-y += opal-calls.o
diff --git a/xen/arch/ppc/ppc64/asm-offsets.c b/xen/arch/ppc/ppc64/asm-offsets.c
index e1129cb0f4..c15c1bf136 100644
--- a/xen/arch/ppc/ppc64/asm-offsets.c
+++ b/xen/arch/ppc/ppc64/asm-offsets.c
@@ -6,6 +6,7 @@ 
 
 #include <xen/macros.h>
 #include <asm/processor.h>
+#include <asm/boot.h>
 
 #define DEFINE(_sym, _val)                                                  \
     asm volatile ( "\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \
@@ -46,6 +47,9 @@  void __dummy__(void)
     OFFSET(UREGS_cr, struct cpu_user_regs, cr);
     OFFSET(UREGS_fpscr, struct cpu_user_regs, fpscr);
     DEFINE(UREGS_sizeof, sizeof(struct cpu_user_regs));
+
+    OFFSET(OPAL_base, struct opal, base);
+    OFFSET(OPAL_entry, struct opal, entry);
 }
 
 /*
diff --git a/xen/arch/ppc/ppc64/opal-calls.S b/xen/arch/ppc/ppc64/opal-calls.S
new file mode 100644
index 0000000000..f754308725
--- /dev/null
+++ b/xen/arch/ppc/ppc64/opal-calls.S
@@ -0,0 +1,82 @@ 
+/* 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/asm-offsets.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)
+    /* Back up LR, CR, r2 in caller's stack frame */
+    mflr    %r11
+    mfcr    %r12
+    std     %r2, 24(%r1)
+    std     %r11, 16(%r1)
+    stw     %r12, 8(%r1)
+
+    /* Use r14 (non-volatile) to store the virtual address of opal_return_mmu */
+    std     %r14, -8(%r1)
+    stdu    %r1, -48(%r1)
+    LOAD_REG_ADDR(%r14, opal_return_mmu)
+
+    /*
+     * Setup new MSR without LE or MMU. Original MSR will be preserved across
+     * opal call in r13
+     */
+    mfmsr   %r13
+    li      %r11, MSR_LE | MSR_IR | MSR_DR
+    andc    %r12, %r13, %r11
+    mthsrr1 %r12
+
+    LOAD_REG_ADDR(%r11, opal_return_real)
+    mtlr     %r11
+
+    /* Load the opal call entry point and base */
+    LOAD_REG_ADDR(%r11, opal)
+    ld      %r12, OPAL_entry(%r11)
+    ld      %r2, OPAL_base(%r11)
+    mthsrr0 %r12
+    hrfid
+
+opal_return_real:
+    /*
+     * OPAL will always return to us in Big Endian mode. Since we are going
+     * to restore the old MSR with the correct endianness and MMU status set, we
+     * can avoid an unnecessary FIXUP_ENDIAN trampoline by just encoding the
+     * required Big Endian instructions to restore the old MSR direclty.
+     */
+    .long 0xa64bbb7d /* mthsrr1 %r13 (Old MSR) */
+    .long 0xa64bda7d /* mthsrr0 %r14 (Virtual address of opal_return_mmu) */
+    .long 0x2402004c /* hrfid */
+
+opal_return_mmu:
+    /*
+     * We're back in the correct endianness and MMU mode, restore registers
+     * and return
+     */
+    addi    %r1, %r1, 48
+    ld      %r14, -8(%r1)
+    lwz     %r11, 8(%r1)
+    ld      %r12, 16(%r1)
+    ld      %r2, 24(%r1)
+    mtcr    %r11
+    mtlr    %r12
+
+    blr
+
+OPAL_CALL(opal_console_write, OPAL_CONSOLE_WRITE)
+OPAL_CALL(opal_console_flush, OPAL_CONSOLE_FLUSH)
+OPAL_CALL(opal_reinit_cpus, OPAL_REINIT_CPUS)