Re: [XEN PATCH v2 1/2] arch/riscv: initial RISC-V support to build/run minimal Xen

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2023-01-04 at 20:36 +0000, Andrew Cooper wrote:
> On 30/12/2022 1:01 pm, Oleksii Kurochko wrote:
> > The patch provides a minimal amount of changes to start
> > build and run minimal Xen binary at GitLab CI&CD that will
> > allow continuous checking of the build status of RISC-V Xen.
> > 
> > Except introduction of new files the following changes were done:
> > * Redefinition of ALIGN define from '.algin 2' to '.align 4' as
> 
> "align"
> 
> >   RISC-V implementations (except for with C extension) enforce 32-
> > bit
> 
> While the C extension might mean things to RISC-V experts, it is
> better
> to say the "compression" extension here so it's clear to non-RISC-V
> experts reading the commit message too.
> 
> But, do we actually care about C?
> 
> ENTRY() needs to be 4 byte aligned because one of the few things it
> is
> going to be used for is {m,s}tvec which requires 4-byte alignment
> even
> with an IALIGN of 2.
> 
We don't care about C (at least now).
> 
> I'd drop all talk about C and just say that 2 was an incorrect choice
> previously.
> 
> >   instruction address alignment.  With C extension, 16-bit and 32-
> > bit
> >   are both allowed.
> > * ALL_OBJ-y and ALL_LIBS-y were temporary overwritted to produce
> >   a minimal hypervisor image otherwise it will be required to push
> >   huge amount of headers and stubs for common, drivers, libs etc
> > which
> >   aren't necessary for now.
> > * Section changed from .text to .text.header for start function
> >   to make it the first one executed.
> > * Rework riscv64/Makefile logic to rebase over changes since the
> > first
> >   RISC-V commit.
> > 
> > RISC-V Xen can be built by the following instructions:
> >   $ CONTAINER=riscv64 ./automation/scripts/containerize \
> >        make XEN_TARGET_ARCH=riscv64 tiny64_defconfig
> 
> This needs a `-C xen` in this rune.
> 
> > diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
> > index 942e4ffbc1..74386beb85 100644
> > --- a/xen/arch/riscv/Makefile
> > +++ b/xen/arch/riscv/Makefile
> > @@ -1,2 +1,18 @@
> > +obj-$(CONFIG_RISCV_64) += riscv64/
> > +
> > +$(TARGET): $(TARGET)-syms
> > +       $(OBJCOPY) -O binary -S $< $@
> > +
> > +$(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
> > +
> > +$(obj)/xen.lds: $(src)/xen.lds.S FORCE
> > +       $(call if_changed_dep,cpp_lds_S)
> > +
> > +clean-files := $(objtree)/.xen-syms.[0-9]*
> 
> We don't need clean-files now that the main link has been simplified.
> 
> > diff --git a/xen/arch/riscv/include/asm/config.h
> > b/xen/arch/riscv/include/asm/config.h
> > index e2ae21de61..e10e13ba53 100644
> > --- a/xen/arch/riscv/include/asm/config.h
> > +++ b/xen/arch/riscv/include/asm/config.h
> > @@ -36,6 +39,10 @@
> >    name:
> >  #endif
> >  
> > +#define XEN_VIRT_START  _AT(UL,0x00200000)
> 
> Space after the comma.
> 
> Otherwise, LGTM.
> 
Thanks for the comments.
They were fixed in patch series v4:
[PATCH v4 0/2] Add minimal RISC-V Xen build and build testing

~ Oleksii
> ~Andrew