On 22.06.2023 22:57, Shawn Anastasio wrote: > On typical Power VMs (e.g. QEMU's -M pseries), a variety of services > including an early serial console are provided by Open Firmware. > Implement the required interfaces to call into Open Firmware and write > to the serial console. > > Since Open Firmware runs in 32-bit Big Endian mode and Xen runs in > 64-bit Little Endian mode, a thunk is required to save/restore > any potentially-clobbered registers as well as to perform the > required endianness switch. Thankfully, linux already has such > a routine, which was imported into ppc64/of-call.S. > > Support for bare metal (PowerNV) will be implemented in a future > patch. > > Signed-off-by: Shawn Anastasio <sanasta...@raptorengineering.com>
Just two minor remarks; I can't really review most of this for lack of knowledge. > --- a/xen/arch/ppc/Makefile > +++ b/xen/arch/ppc/Makefile > @@ -1,4 +1,6 @@ > obj-$(CONFIG_PPC64) += ppc64/ > +obj-y += boot-of.init.o > +obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > obj-y += setup.o While you've followed Julien's earlier comment partly, I think, sorting still isn't alphabetical. If you mean ppc64/ to explicitly be first, you will want to introduce a separating blank line. > --- /dev/null > +++ b/xen/arch/ppc/ppc64/of-call.S > @@ -0,0 +1,83 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Adapted from Linux's arch/powerpc/kernel/entry_64.S, with the > + * following copyright notice: > + * > + * PowerPC version > + * Copyright (C) 1995-1996 Gary Thomas (g...@linuxppc.org) > + * Rewritten by Cort Dougan (c...@cs.nmt.edu) for PReP > + * Copyright (C) 1996 Cort Dougan <c...@cs.nmt.edu> > + * Adapted for Power Macintosh by Paul Mackerras. > + * Low-level exception handlers and MMU support > + * rewritten by Paul Mackerras. > + * Copyright (C) 1996 Paul Mackerras. > + * MPC8xx modifications Copyright (C) 1997 Dan Malek (dma...@jlc.net). > + */ > + > +#include <asm/asm-offsets.h> > +#include <asm/asm-defns.h> > +#include <asm/msr.h> > + > +/* size of minimum stack frame that can hold an entire cpu_user_regs struct > */ > +#define STACK_SWITCH_FRAME_SIZE (UREGS_sizeof + STACK_FRAME_OVERHEAD) > + > + .section .init.text, "ax", @progbits > + > +ENTRY(enter_of) > + mflr %r0 > + std %r0, 16(%r1) > + stdu %r1,-STACK_SWITCH_FRAME_SIZE(%r1) /* Save SP and create stack space > */ > + > + /* > + * Because PROM is running in 32b mode, it clobbers the high order half > + * of all registers that it saves. We therefore save those registers > + * PROM might touch to the stack. (%r0, %r3-%r13 are caller saved) > + */ > + SAVE_GPR(2, %r1) > + SAVE_GPR(13, %r1) > + SAVE_NVGPRS(%r1) > + mfcr %r10 > + mfmsr %r11 > + std %r10, UREGS_cr(%r1) > + std %r11, UREGS_msr(%r1) > + > + /* Put PROM address in SRR0 */ > + mtsrr0 %r4 > + > + /* Setup our trampoline return addr in LR */ > + bcl 20, 31,.+4 > +0: mflr %r4 > + addi %r4, %r4,(1f - 0b) > + mtlr %r4 > + > + /* Prepare a 32-bit mode big endian MSR */ > + LOAD_IMM64(%r12, MSR_SF | MSR_LE) > + andc %r11, %r11, %r12 > + mtsrr1 %r11 > + rfid I think readability would improve if you reserved a minimum number of characters (6?) for the mnemonics, padding with blanks when they're shorter. (This, if you'd be willing to switch, would then also apply to patch 1, iirc.) Jan
