On Thu, Dec 11, 2025 at 3:33 AM Julien Grall <[email protected]> wrote:
>
> Hi Saman,
>
> A bit of process first. Usually, when sending a v2, a new thread is
> started (IOW, this is not sent in reply to v1).
Hi Julien,
Thanks :)
>
> On 11/12/2025 02:39, Saman Dehghan wrote:
> > This patch enables building Xen on arm64 architecture using the Clang
> > compiler.
> > Changes include:
> > - Add explicit -march=armv8 flag for arm64 builds.
> > - Add `__attribute__((target("fp-armv8")))` to `vfp_save_state` and
> > `vfp_restore_state` functions when building with Clang to allow
> > FP instructions despite `-mgeneral-regs-only`.
> >
> > Signed-off-by: Saman Dehghan <[email protected]>
> > ---
> > README | 2 ++
> > xen/arch/arm/arch.mk | 1 +
> > xen/arch/arm/arm64/vfp.c | 6 ++++++
> > 3 files changed, 9 insertions(+)
> >
> > diff --git a/README b/README
> > index 889a4ea906..67c1aa7fe6 100644
> > --- a/README
> > +++ b/README
> > @@ -45,6 +45,8 @@ provided by your OS distributor:
> > - For ARM:
> > - GCC 5.1 or later
> > - GNU Binutils 2.25 or later
> > + or
> > + - Clang/LLVM 11 or later
> > - For RISC-V 64-bit:
> > - GCC 12.2 or later
> > - GNU Binutils 2.39 or later
> > diff --git a/xen/arch/arm/arch.mk b/xen/arch/arm/arch.mk
> > index 9c4bedfb3b..bcf548069b 100644
> > --- a/xen/arch/arm/arch.mk
> > +++ b/xen/arch/arm/arch.mk
> > @@ -13,6 +13,7 @@ ifeq ($(CONFIG_MPU),y)
> > CFLAGS-$(CONFIG_ARM_64) += -march=armv8-r
> > else
> > CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
> > +CFLAGS-$(CONFIG_ARM_64) += -march=armv8
> > endif
> > CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
> > $(call cc-option-add,CFLAGS-$(CONFIG_ARM_64),CC,-mno-outline-atomics)
> > diff --git a/xen/arch/arm/arm64/vfp.c b/xen/arch/arm/arm64/vfp.c
> > index c4f89c7b0e..51fd2ddc54 100644
> > --- a/xen/arch/arm/arm64/vfp.c
> > +++ b/xen/arch/arm/arm64/vfp.c
> > @@ -46,6 +46,9 @@ static inline void restore_state(const uint64_t *fpregs)
> > : : "Q" (*fpregs), "r" (fpregs));
> > }
> >
> > +#if defined(CONFIG_CC_IS_CLANG)
> > +__attribute__((target("fp-armv8")))
> > +#endif
>
> Based on Jan's comment, I am a bit puzzled why adding #ifdef is
> sufficient. In fact, I do agree with Jan, my understanding of
> target(...) is this will impact the ABI.
>
> I haven't experienced any issue with the C side yet. But I know in the
> Rust world (they also have an LLVM backend), they decided to prevent
> enabling fp/neon [1] at the function level.
>
> Did you find any documentation that would suggest this is safe?
>
> Now regarding the issue you mentioned in v1:
>
> > On top of those, `READ_SYSREG(FPSR)`, `READ_SYSREG(FPCR)`,
> > `WRITE_SYSREG(v->arch.vfp.fpsr, FPSR)`and
> > `WRITE_SYSREG(v->arch.vfp.fpcr, FPCR)` using FP.
> > I think I cannot apply __attribute__ on statements.
>
> Do you mean the compiler will complain that you are trying to access
> FPCR/FPSR if you don't add the __atribute__ at the function level?
>
> If so, what you could possibly do is either rewriting the functions in
> assembly or open-code the "{WRITE, READ}_SYSREG()" and add a line
> ".arch_extension fp".
I couldn't find any documentation to suggest that it is safe. I will
do what you and Jan suggested and use ".arch_extension fp".
~Saman
>
> Cheers,
>
> [1] https://github.com/llvm/llvm-project/issues/110632
>
> --
> Julien Grall
>
