Skip to content

Conversation

@asdfugil
Copy link
Contributor

@asdfugil asdfugil commented Aug 4, 2025

Compile C code as base armv8-a, and compile assembly codes as armv8.2-a to allow m1n1 to be properly compiled for older SoCs.

@asdfugil
Copy link
Contributor Author

asdfugil commented Aug 4, 2025

The build failure appears to be caused by https://src.fedoraproject.org is down/not working properly.

@asdfugil
Copy link
Contributor Author

asdfugil commented Aug 8, 2025

updated to specify armv8.2-a directly whenever needed

@asdfugil
Copy link
Contributor Author

asdfugil commented Aug 8, 2025

the build failures appears to be because of the build environment not supplying some runtime builtins that are required when LSE atomics are unavailable. however the m1n1.spec file is outside of this repo so nothing can be done here.

Copy link
Member

@jannau jannau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I'm happy with this change. I would prefer if the HV continues to use LSE atomics if available.
Could you live with enabling this only for old SoCs via a config variable?

src/utils.h Outdated
({ \
u64 val; \
__asm__ volatile("mrs\t%0, " #reg : "=r"(val)); \
__asm__ volatile(".arch armv8.2-a\nmrs\t%0, " #reg : "=r"(val)); \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please break the asm statement into 2 lines like

        __asm__ volatile(".arch armv8.2-a\n"                                                       \
                         "mrs\t%0, " #reg : "=r"(val));                                            \

-ffreestanding -fpic -ffunction-sections -fdata-sections \
-nostdinc -isystem $(shell $(CC) -print-file-name=include) -isystem sysinc \
-fno-stack-protector -mstrict-align -march=armv8.2-a \
-fno-stack-protector -mstrict-align -Wa,-march=armv8.2-a \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is enough for armv8-a under all circumstances. I think you have to explicitly set -march=armv8-a and add -mno-outline-atomics to make sure we end up with base armv8-a

@asdfugil
Copy link
Contributor Author

asdfugil commented Aug 8, 2025

I'm not sure if I'm happy with this change. I would prefer if the HV continues to use LSE atomics if available. Could you live with enabling this only for old SoCs via a config variable?

Actually the HV is already not using LSE atomics in some cases when it could have:
HV calls into spin_lock(), and the asm block there was changed to not use LSE atomics

How about compiling HV C files with -march=armv8.2-a, and then define a hv_spin_lock() that restores the old spin_lock() behavior (and use it only inside HV C files)?

@jannau
Copy link
Member

jannau commented Aug 8, 2025

spinlock (and hv) still use built-in atomics and with -march=armv8.2-a those translate to LSE atomics

@asdfugil
Copy link
Contributor Author

asdfugil commented Aug 8, 2025

spinlock (and hv) still use built-in atomics and with -march=armv8.2-a those translate to LSE atomics

yeah but the asm block used to use LSE atomics as well but this was changed so m1n1 could work on armv8.0 869d2ae

Compile non-HV C code as base armv8-a, and compile assembly and HV codes as
armv8.2-a to allow m1n1 to be properly compiled for older SoCs.

Signed-off-by: Nick Chan <towinchenmi@gmail.com>
@asdfugil
Copy link
Contributor Author

asdfugil commented Aug 10, 2025

updates:

split the mrs/msr marco inline asm statement into two lines
use -mno-outline-atomics and -march=armv8-a in general
HV codes are compiled with -march=armv8.2-a still, and there's hv versions of spin_lock() and spin_unlock() (spin_lock() explicitly uses casa in its inline asm block which is the behavior before a7-a11 support is added)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants