feat: add efiapi support for loongarch64 by ZR233 · Pull Request #149879 · rust-lang/rust (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation15 Commits1 Checks11 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
To fix: #149877
Support extern "efiapi" for target loongarch64
Copilot AI review requested due to automatic review settings
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
r? @madsmtm
rustbot has assigned @madsmtm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
This comment was marked as spam.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I'm unfamiliar with LoongArch, so I'll ping the target maintainers:
@rustbot ping loongarch
I'm also a bit unsure of the process, but I think this'll need a lang FCP. But let's get to that once the implementation is a bit more fleshed out.
View changes since this review
| LoongArch64, |
|---|
| /// Architectures which don't need other considerations for ABI lowering |
| Other, |
| } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should support be added in InlineAsmClobberAbi::parse as well?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, that was intentionally not done in #111499, so I'm unsure here?
| LoongArch64, |
|---|
| /// Architectures which don't need other considerations for ABI lowering |
| Other, |
| } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| (ExternAbi::EfiApi, ArchKind::Aarch64 | ArchKind::Riscv | ArchKind::X86) => CanonAbi::C, | |
|---|---|---|
| ( | ||
| ExternAbi::EfiApi, | ||
| ArchKind::Aarch64 | ArchKind::Riscv | ArchKind::X86 | ArchKind::LoongArch64, |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why only LoongArch64? Isn't LoongArch32 the same?
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Reminder, once the PR becomes ready for a review, use @rustbot ready.
Error: This team (loongarch) cannot be pinged via this command; it may need to be added to triagebot.toml on the default branch.
Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip.
Thanks for your patch. Since LoongArch currently lacks basic UEFI support, adding efiapi does not change anything at the moment. We are actively working on bringing UEFI support to LoongArch, which involves the PE format specification, LLVM, and related components. More here:
loongson-community/discussions#108
Thanks for your patch. Since LoongArch currently lacks basic UEFI support, adding
efiapidoes not change anything at the moment. We are actively working on bringing UEFI support to LoongArch, which involves the PE format specification, LLVM, and related components. More here:
Thank you for the reply. I see you've already implemented similar code, so I’ll go ahead and close this duplicate PR.
Could we prioritize handling the efiapi-to-C mapping first? When doing kernel development with the loongarch64-unknown-none target—specifically when implementing a custom PE header (similar to Linux’s approach) and using uefi-rs—the aforementioned related components aren’t actually needed.
Status: This is awaiting some action (such as code changes or more information) from the author.
label
Could we prioritize handling the efiapi-to-C mapping first? When doing kernel development with the loongarch64-unknown-none target—specifically when implementing a custom PE header (similar to Linux’s approach) and using uefi-rs—the aforementioned related components aren’t actually needed.
I agree. We can generate ELF objects first, convert relocations manually, and then link and convert to PE for UEFI. I think landing the ABI mapping for efiapi separately will help unblock LoongArch UEFI app development, so I’ll do that first.
EDIT: #150012
Labels
Relevant to the compiler team, which will review and decide on the PR/issue.