Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RISC-V ACLINT Support #53

Merged
merged 3 commits into from
Nov 21, 2023
Merged

RISC-V ACLINT Support #53

merged 3 commits into from
Nov 21, 2023

Conversation

ninolomata
Copy link
Member

@ninolomata ninolomata commented Feb 17, 2023

Features:

  • IPIs sent via ACLINT instead of SBI ECALLs

Notes:
Linux -> https://github.com/avpatel/linux/tree/riscv_aclint_v5
QEMU -> 7.2 with flag -M aclint=on

Copy link
Member

@josecm josecm left a comment

Choose a reason for hiding this comment

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

I agree we should support the ACLINT to take advantage of faster IPIs in the platforms thave have one. However, I don't think virtualizing the ACLINT is useful because the cost of trap-and-emulate is larger than the direct hypercall. I'd say it would be better we don't support a virtual ACLINT, until maybe in the future the ACLINT adds support for virtualization or we find some guest that only supports IPIs through the ACLINT.

@josecm josecm self-assigned this Jun 16, 2023
src/arch/riscv/inc/arch/aclint.h Outdated Show resolved Hide resolved
src/arch/riscv/interrupts.c Outdated Show resolved Hide resolved
src/arch/riscv/interrupts.c Outdated Show resolved Hide resolved
src/arch/riscv/inc/arch/aclint.h Outdated Show resolved Hide resolved
src/arch/riscv/inc/arch/vm.h Outdated Show resolved Hide resolved
src/arch/riscv/inc/arch/platform.h Outdated Show resolved Hide resolved
src/arch/riscv/interrupts.c Outdated Show resolved Hide resolved
@josecm
Copy link
Member

josecm commented Oct 18, 2023

Please also, squash the two commits in a single one.

@ninolomata ninolomata force-pushed the feat/aclint branch 2 times, most recently from db37707 to e0be390 Compare October 19, 2023 21:56
@ninolomata
Copy link
Member Author

@josecm I addressed all your comments in this recent update.

@josecm
Copy link
Member

josecm commented Oct 20, 2023

Actually, since the information about the clint being present or not is already present in the platform description I was thinking about adding to the platform define scripts the automatic generation of the macro. In a scripts/arch/riscv/platform_defs_gen.c

...
    if (platform.arch.aclint_sswi.base != 0) {
        printf("#define ACLINT_PRESENT");
    }
...

I'll be happy to do this myself if you don't have the time as it will involve a little more work for supporting arch-specific macro definition scripts.

@ninolomata
Copy link
Member Author

@josecm I did as you ask and created a platform specific script. If you want, I can create a separate commit just for that.

@josecm
Copy link
Member

josecm commented Oct 21, 2023

@josecm I did as you ask and created a platform specific script. If you want, I can create a separate commit just for that.

Thanks! Yes, a separate commit for that would be great!

src/arch/riscv/arch.mk Outdated Show resolved Hide resolved
src/arch/riscv/objects.mk Outdated Show resolved Hide resolved
scripts/arch/riscv/platform_defs_gen.c Outdated Show resolved Hide resolved
src/arch/riscv/arch.mk Outdated Show resolved Hide resolved
src/arch/riscv/inc/arch/interrupts.h Outdated Show resolved Hide resolved
src/arch/riscv/interrupts.c Outdated Show resolved Hide resolved
josecm
josecm previously approved these changes Nov 14, 2023
@josecm
Copy link
Member

josecm commented Nov 14, 2023

@ninolomata Can you resolve the conflicts in src/platform/qemu-riscv64-virt/virt_desc.c ?

Features:
- IPIs sent via aclint instead of sbi ecalls

Signed-off-by: Bruno Sa <bruno.vilaca.sa@gmail.com>
Add the IPIC option on QEMU platform to select
ACLINT or SBI as the interrupt controller for IPIs

Signed-off-by: Bruno Sa <bruno.vilaca.sa@gmail.com>
@ninolomata
Copy link
Member Author

@josecm all good now.

@josecm
Copy link
Member

josecm commented Nov 17, 2023

Now the code seems fine, but I think we need to add also a build check for compiling with IPIC == IPIC_ACLINT. I'll add this later today.

Signed-off-by: Jose Martins <josemartins90@gmail.com>
@josecm
Copy link
Member

josecm commented Nov 21, 2023

Copy link
Member

@sandro2pinto sandro2pinto left a comment

Choose a reason for hiding this comment

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

All good to me.

@josecm josecm merged commit bff11a9 into bao-project:main Nov 21, 2023
15 checks passed
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.

5 participants