Re: [PATCH v6 03/20] xen/riscv: introduce extenstion support check by compiler

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2024-03-18 at 17:58 +0100, Jan Beulich wrote:
> On 15.03.2024 19:05, Oleksii Kurochko wrote:
> > Currently, RISC-V requires two extensions: _zbb and _zihintpause.
> 
> Do we really require Zbb already?
After an introduction of Andrew C. patches [1] it is requited for
__builtin_ffs{l}

[1]
https://lore.kernel.org/xen-devel/20240313172716.2325427-1-andrew.cooper3@xxxxxxxxxx/T/#t
> 
> > This patch introduces a compiler check to check if these extensions
> > are supported.
> > Additionally, it introduces the riscv/booting.txt file, which
> > contains
> > information about the extensions that should be supported by the
> > platform.
> > 
> > In the future, a feature will be introduced to check whether an
> > extension
> > is supported at runtime.
> > However, this feature requires functionality for parsing device
> > tree
> > source (DTS), which is not yet available.
> 
> Can't you query the CPU for its features?
I couldn't find such reg ( or SBI call ) in the spec.

SBI call sbi_probe_extension() exists, but it doesn't check for every
possible extension. As far as I understand it checks only for that one
which are present in SBI spec.

The most closest thing I see how to check that without dts is how it is
done in OpenSBI:
#define csr_read_allowed(csr_num, trap)                         \
        ({                                                      \
        register ulong tinfo asm("a3") = (ulong)trap;           \
        register ulong ttmp asm("a4");                          \
        register ulong mtvec = sbi_hart_expected_trap_addr();   \
        register ulong ret =
0;                                      \
        ((struct sbi_trap_info *)(trap))->cause = 0;            \
        asm volatile(                                           \
                "add %[ttmp], %[tinfo],
zero\n"                 \
                "csrrw %[mtvec], " STR(CSR_MTVEC) ", %[mtvec]\n"\
                "csrr %[ret],
%[csr]\n"                               \
                "csrw " STR(CSR_MTVEC) ", %[mtvec]"             \
            : [mtvec] "+&r"(mtvec), [tinfo] "+&r"(tinfo),       \
              [ttmp] "+&r"(ttmp), [ret] "=&r" (ret)             \
            : [csr] "i" (csr_num)                               \
            : "memory");                                        \
        ret;                                                    \
        })                                                      \
...
        /* Detect if hart supports stimecmp CSR(Sstc extension) */
        if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
                csr_read_allowed(CSR_STIMECMP, (unsigned long)&trap);
                if (!trap.cause)
                        __sbi_hart_update_extension(hfeatures,
                                        SBI_HART_EXT_SSTC, true);
        }

~ Oleksii

> 
> > --- a/xen/arch/riscv/arch.mk
> > +++ b/xen/arch/riscv/arch.mk
> > @@ -3,16 +3,22 @@
> >  
> >  $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
> >  
> > -CFLAGS-$(CONFIG_RISCV_64) += -mabi=lp64
> > +riscv-abi-$(CONFIG_RISCV_32) := -mabi=ilp32
> > +riscv-abi-$(CONFIG_RISCV_64) := -mabi=lp64
> >  
> >  riscv-march-$(CONFIG_RISCV_ISA_RV64G) := rv64g
> >  riscv-march-$(CONFIG_RISCV_ISA_C)       := $(riscv-march-y)c
> >  
> > +extensions := $(call as-insn,$(CC) $(riscv-abi-y) -march=$(riscv-
> > march-y)_zbb,"",_zbb) \
> > +              $(call as-insn,$(CC) $(riscv-abi-y) -march=$(riscv-
> > march-y)_zihintpause,"pause",_zihintpause)
> 
> Imo you want another helper macro here, where all one needs to pass
> in is
> the extension name (i.e. zbb and zihintpause as per above). That'll
> also
> help with line length, I hope.
> 
> Jan