Re: [PATCH v4 25/30] xen/riscv: add minimal stuff to processor.h to build full Xen

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2024-02-19 at 09:06 +0100, Jan Beulich wrote:
> On 16.02.2024 12:16, Oleksii wrote:
> > On Thu, 2024-02-15 at 17:43 +0100, Jan Beulich wrote:
> > > On 15.02.2024 17:38, Oleksii wrote:
> > > > On Tue, 2024-02-13 at 14:33 +0100, Jan Beulich wrote:
> > > > > On 05.02.2024 16:32, Oleksii Kurochko wrote:
> > > > > > +   depends on LLD_VERSION >= 150000 || LD_VERSION >=
> > > > > > 23600
> > > > > 
> > > > > What's the linker dependency here? Depending on the answer I
> > > > > might
> > > > > further
> > > > > ask why "TOOLCHAIN" when elsewhere we use CC_HAS_ or HAS_CC_
> > > > > or
> > > > > HAS_AS_.
> > > > I missed to introduce {L}LLD_VERSION config. It should output
> > > > from
> > > > the
> > > > command:
> > > >   riscv64-linux-gnu-ld --version
> > > 
> > > Doesn't answer my question though where the linker version
> > > matters
> > > here.
> > Then I misinterpreted your initial question.
> > Could you please provide further clarification or rephrase it for
> > better understanding?
> > 
> > Probably, your question was about why linker dependency is needed
> > here,
> > then
> > it is not sufficient to check if a toolchain supports a particular 
> > extension without checking if the linker supports that extension   
> > too.
> > For example, Clang 15 supports Zihintpause but GNU bintutils
> > 2.35.2 does not, leading build errors like so:
> >     
> >    riscv64-linux-gnu-ld: -march=rv64i_zihintpause2p0: Invalid or
> >    unknown z ISA extension: 'zihintpause'
> 
> Hmm, that's certainly "interesting" behavior of the RISC-V linker.
> Yet
> isn't the linker capability expected to be tied to that of gas? I
> would
> find it far more natural if a gas dependency existed here. If such a
> connection cannot be taken for granted, I'm pretty sure you'd need to
> probe both then anyway.

Wouldn't it be enough in this case instead of introducing of new
configs and etc, just to do the following:
   +ifeq ($(CONFIG_RISCV_64),y)
   +has_zihintpause = $(call as-insn,$(CC) -mabi=lp64 -
   march=rv64i_zihintpause, "pause",_zihintpause,)
   +else
   +has_zihintpause = $(call as-insn,$(CC) -mabi=ilp32 -
   march=rv32i_zihintpause, "pause",_zihintpause,)
   +endif
   +
    riscv-march-$(CONFIG_RISCV_ISA_RV64G) := rv64g
    riscv-march-$(CONFIG_RISCV_ISA_C)       := $(riscv-march-y)c
   -riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-
   y)_zihintpause
    
    # Note that -mcmodel=medany is used so that Xen can be mapped
    # into the upper half _or_ the lower half of the address space.
    # -mcmodel=medlow would force Xen into the lower half.
    
   -CFLAGS += -march=$(riscv-march-y) -mstrict-align -mcmodel=medany
   +CFLAGS += -march=$(riscv-march-y)$(has_zihintpause) -mstrict-align
   -
   mcmodel=medany

Probably, it would be better:
   ...
   +CFLAGS += -march=$(riscv-march-y)$(call or,$(has_zihintpause)) -
   mstrict-align -
   mcmodel=medany


~ Oleksii