[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 2/4] xen: Add files needed for minimal ppc64le build
On Wed Jun 14, 2023 at 10:51 AM CDT, Jan Beulich wrote: > On 13.06.2023 16:50, Shawn Anastasio wrote: > > --- /dev/null > > +++ b/xen/arch/ppc/Makefile > > @@ -0,0 +1,16 @@ > > +obj-$(CONFIG_PPC64) += ppc64/ > > + > > +$(TARGET): $(TARGET)-syms > > + cp -f $< $@ > > + > > +$(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds > > + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) -o $@ > > + $(NM) -pa --format=sysv $(@D)/$(@F) \ > > + | $(objtree)/tools/symbols --all-symbols --xensyms --sysv > > --sort \ > > + >$(@D)/$(@F).map > > Elsewhere we recently switched these uses of $(@D)/$(@F) to just $@. > Please can you do so here as well? Sure, will fix in v4. > > --- /dev/null > > +++ b/xen/arch/ppc/arch.mk > > @@ -0,0 +1,11 @@ > > +######################################## > > +# Power-specific definitions > > + > > +ppc-march-$(CONFIG_POWER_ISA_2_07B) := power8 > > +ppc-march-$(CONFIG_POWER_ISA_3_00) := power9 > > + > > +CFLAGS += -mcpu=$(ppc-march-y) -mstrict-align -mcmodel=large -mabi=elfv2 > > -mno-altivec -mno-vsx > > Wouldn't it make sense to also pass -mlittle here, such that a tool > chain defaulting to big-endian can still be used? Good call. On this topic, I suppose I'll also add -m64 to allow 32-bit toolchains to be used as well. > > --- /dev/null > > +++ b/xen/arch/ppc/ppc64/head.S > > @@ -0,0 +1,27 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > + > > +.section .text.header, "ax", %progbits > > + > > +ENTRY(start) > > + /* > > + * Depending on how we were booted, the CPU could be running in either > > + * Little Endian or Big Endian mode. The following trampoline from > > Linux > > + * cleverly uses an instruction that encodes to a NOP if the CPU's > > + * endianness matches the assumption of the assembler (LE, in our case) > > + * or a branch to code that performs the endian switch in the other > > case. > > + */ > > + tdi 0, 0, 0x48 /* Reverse endian of b . + 8 */ > > + b $ + 44 /* Skip trampoline if endian is good */ > > If I get this right, $ and . are interchangable on Power? If not, > then all is fine and there likely is a reason to use . in the > comment but $ in the code. But if so, it would be nice if both > could match, and I guess with other architectures in mind . would > be preferable. As hinted by the comment, this code was directly inherited from Linux and I'm not sure why the original author chose '$' instead of '.'. That said, as far as I can tell you are correct about the two being interchangeable, and changing the $ to . results in the exact same machine code. I can go ahead and make the change for consistency in v4. > > + DECL_SECTION(.bss) { /* BSS */ > > + __bss_start = .; > > + *(.bss.stack_aligned) > > + . = ALIGN(PAGE_SIZE); > > + *(.bss.page_aligned) > > ... the one between the two .bss parts looks unmotivated. Within > a section definition ALIGN() typically only makes sense when followed > by a label definition, like ... Correct me if I'm wrong, but wouldn't the ALIGN here serve to ensure that the subsequent '.bss.page_aligned' section has the correct alignment that its name implies? > Jan Thanks, Shawn
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |