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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.02.2024 18:00, Oleksii wrote:
> 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

Yes, this is kind of what I'd expect.

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

Why the use of "or"? IOW right now I don't see what's "better" here.

Jan