arch: riscv: Add support for custom SOC IPI, per-core init #65824

luchnikov · 2023-11-27T17:22:55Z

Allow RISCV SOC implementation to define its own custom:

IPI implementation (e.g. Andes usage of PLIC for IPI)
per-core init (e.g. per-core PMA implementation)

Signed-off-by: Maxim Adelman [email protected]

cfriedt

LGTM - this is similar to what was done for sys_io in #60771

ycsin

Normally *_SOC_HAS_CUSTOM_* would turn these into default implementation and allows devs to implement their own z_soc_* APIs to overwrite, are we breaking that convention here?

arch/riscv/Kconfig

arch/riscv/core/smp.c

luchnikov · 2023-11-29T00:38:12Z

Normally *_SOC_HAS_CUSTOM_* would turn these into default implementation and allows devs to implement their own z_soc_* APIs to overwrite, are we breaking that convention here?

This one encompasses a number of SOC-specific overrides which are not easily encompassed with conditional z_soc_*. I added more details to the Kconfig

nashif · 2023-11-29T02:32:26Z

Normally *_SOC_HAS_CUSTOM_* would turn these into default implementation and allows devs to implement their own z_soc_* APIs to overwrite, are we breaking that convention here?

This one encompasses a number of SOC-specific overrides which are not easily encompassed with conditional z_soc_*. I added more details to the Kconfig

hmm, this is basically removing most of the SMP implementation out, even the SYS_INIT()! It is not really clear what needs to be implemented because this is just disabling a bunch of calls and interfaces, that does not look right.

ycsin · 2024-09-17T06:03:09Z

Toggled to trigger CI

ycsin · 2024-09-17T07:38:10Z

Does this still need to go through an architecture review?

npitre

Looks fine to me. My preference would be made of an overridable weak empty
stub to cut down on all the kconfig noise but current trend is to go the
noisy way.

ycsin · 2024-09-17T15:06:02Z

arch/riscv/core/smp.c

+/* IPI */
+#if defined(CONFIG_SMP) && !defined(CONFIG_RISCV_SOC_HAS_CUSTOM_SMP_IPI)


nit [1/2]: I think the following makes it clearer that this portion of code is for SMP, and within it, there's a portion of code that shouldn't be compiled when CONFIG_RISCV_SOC_HAS_CUSTOM_SMP_IPI=y

Suggested change

/* IPI */

#if defined(CONFIG_SMP) && !defined(CONFIG_RISCV_SOC_HAS_CUSTOM_SMP_IPI)

#ifdef CONFIG_SMP

#ifndef CONFIG_RISCV_SOC_HAS_CUSTOM_SMP_IPI

arch/riscv/core/smp.c

ycsin · 2024-09-17T15:11:44Z

arch/riscv/core/smp.c

@@ -74,11 +75,15 @@ void arch_secondary_cpu_init(int hartid)
 #endif
 #ifdef CONFIG_SMP
 	irq_enable(RISCV_IRQ_MSOFT);
+#endif
+#ifdef CONFIG_RISCV_SOC_HAS_CUSTOM_PER_CORE_INIT
+	z_soc_per_core_init();


I think there should be an entry for this in arch_kernel_init() similar to the z_riscv_pmp_init(), so that the function is ran by the main core as well - https://github.com/zephyrproject-rtos/zephyr/blob/main/arch/riscv/include/kernel_arch_func.h#L54

This probably should be in a separate commit as well

ycsin · 2024-09-17T15:23:09Z

For context, this PR essentially allows us to do what's being done with esp32 in esp32-mp.c. The current RISC-V IPI implementation was based on CLINT, but not all RISC-V SoC can implement their IPI through CLINT.

For example, the Andes AE350 SoC's MSIP is connected to the plic-sw, and IPI has to be routed via the plic-sw instead.

zephyr/dts/riscv/andes/andes_v5_ae350.dtsi

Lines 182 to 188 in 6194159

    
           mbox: mbox-controller@e6400000 { 
        
           	compatible = "andestech,plic-sw"; 
        
           	reg = <0xe6400000 0x00400000>; 
        
           	#mbox-cells = <1>; 
        
           	channel-max = <30>; 
        
           	status = "okay"; 
        
           };

zephyr/drivers/mbox/mbox_andes_plic_sw.c

Lines 200 to 210 in 6194159

    
           static int mbox_andes_init(const struct device *dev) 
        
           { 
        
           	/* Setup IRQ handler for PLIC SW driver */ 
        
           	IRQ_CONNECT(RISCV_IRQ_MSOFT, 1, 
        
           		    andes_plic_sw_irq_handler, DEVICE_DT_INST_GET(0), 0); 
        
           #ifndef CONFIG_SMP 
        
           	irq_enable(RISCV_IRQ_MSOFT); 
        
           #endif 
        
           	return 0; 
        
           }

I guess the architecture-review is probably not required since this is not something alien, removing the tag for now

ycsin

#65824 (comment)

ycsin · 2024-09-17T15:48:40Z

arch/riscv/Kconfig

@@ -119,6 +119,22 @@ config RISCV_SOC_HAS_CUSTOM_SYS_IO
 	  the RISC-V SoC needs to do something different and more than reading and
 	  writing the registers.

+config RISCV_SOC_HAS_CUSTOM_SMP_IPI


The current implementation is using CLINT, I wonder if we should name this RISCV_SOC_SMP_IPI_CLINT instead to pave the way for the intro of RISCV_SOC_SMP_IPI_PLIC in the future, so that it is a choice between CLINT & PLIC, instead of default vs custom?

It would make sense if we upstream our implementation as the default PLIC implementation, otherwise it would be unclear as to what the behavior would be if RISCV_SOC_SMP_IPI_CLINT was not set

I think it should be fine as this is an arch-level Kconfig, would like to hear from @fkokosinski @npitre and others as well

npitre · 2024-09-17T18:29:19Z

Or even `RISCV_SMP_IPI_CLINT`.

Allow RISCV implementation to define its own custom IPI implementation (e.g. Andes usage of PLIC for IPI) Signed-off-by: Maxim Adelman <[email protected]> Update arch/riscv/core/smp.c Co-authored-by: Yong Cong Sin <[email protected]>

ycsin · 2024-09-19T03:50:25Z

arch/riscv/core/smp.c

@@ -74,12 +76,17 @@ void arch_secondary_cpu_init(int hartid)
 #endif
 #ifdef CONFIG_SMP
 	irq_enable(RISCV_IRQ_MSOFT);
+#endif
+#ifdef CONFIG_SOC_SMP_PER_CORE_INIT_HOOK
+	soc_smp_per_core_init_hook();


We should run this for the main core, probably by adding an invocation of this in arch_kernel_init() similar to the z_riscv_pmp_init(), so that the function is ran by the main core as well, otherwise this would be a per_secondary_core_init_hook

I also think that it would be nice to generalize this new hook as soc_per_core_init_hook() so that it is not SMP-specific. Any soc that would like some additional core initialization code can use this hook

github-actions · 2024-11-19T00:35:29Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

ycsin · 2024-11-19T02:43:32Z

This is now done in #78496 + #81256

luchnikov requested review from ycsin and edersondisouza November 27, 2023 17:22

luchnikov self-assigned this Nov 27, 2023

zephyrbot added the area: RISCV RISCV Architecture (32-bit & 64-bit) label Nov 27, 2023

zephyrbot requested review from carlocaione, fkokosinski, katsuster, kgugala, mgielda, npitre and tgorochowik November 27, 2023 17:23

cfriedt previously approved these changes Nov 27, 2023

View reviewed changes

luchnikov force-pushed the riscv_custom_ipi branch from d82ffcb to a468172 Compare November 27, 2023 17:39

npitre previously approved these changes Nov 27, 2023

View reviewed changes

cfriedt assigned fkokosinski and unassigned luchnikov Nov 27, 2023

fkokosinski previously approved these changes Nov 28, 2023

View reviewed changes

ycsin requested changes Nov 28, 2023

View reviewed changes

arch/riscv/Kconfig Outdated Show resolved Hide resolved

arch/riscv/core/smp.c Outdated Show resolved Hide resolved

luchnikov dismissed stale reviews from fkokosinski, npitre, and cfriedt via 4594cb5 November 29, 2023 00:30

luchnikov force-pushed the riscv_custom_ipi branch from a468172 to 4594cb5 Compare November 29, 2023 00:30

luchnikov requested review from npitre, fkokosinski, cfriedt and ycsin November 29, 2023 00:35

luchnikov force-pushed the riscv_custom_ipi branch from 4594cb5 to 486192a Compare November 29, 2023 02:26

luchnikov changed the title ~~arch: riscv: Add support for custom IPI implementation~~ arch: riscv: Add support for custom SOC IPI, per-core init Nov 29, 2023

ycsin reopened this Sep 17, 2024

ycsin mentioned this pull request Sep 17, 2024

arch: riscv: smp: refactor IPI functions and guard with CONFIG_RISCV_SMP_IPI_CLINT #78493

Closed

1 task

ycsin mentioned this pull request Sep 17, 2024

init: per-core init hook #78496

Merged

npitre previously approved these changes Sep 17, 2024

View reviewed changes

ycsin reviewed Sep 17, 2024

View reviewed changes

ycsin requested changes Sep 17, 2024

View reviewed changes

ycsin removed the Architecture Review Discussion in the Architecture WG required label Sep 17, 2024

ycsin reviewed Sep 17, 2024

View reviewed changes

luchnikov dismissed stale reviews from npitre, cfriedt, and fkokosinski via 7134bbd September 18, 2024 07:43

luchnikov force-pushed the riscv_custom_ipi branch from 7134bbd to abce116 Compare September 18, 2024 10:37

zephyrbot added the area: Kernel label Sep 18, 2024

zephyrbot requested review from andyross, ceolin and peter-mitsis September 18, 2024 10:37

luchnikov force-pushed the riscv_custom_ipi branch from abce116 to ec45018 Compare September 18, 2024 10:43

luchnikov force-pushed the riscv_custom_ipi branch from ec45018 to ce32a59 Compare September 18, 2024 10:45

ycsin reviewed Sep 19, 2024

View reviewed changes

ycsin mentioned this pull request Sep 24, 2024

riscv: smp: support IPI via PLIC #78917

Open

ycsin mentioned this pull request Nov 12, 2024

arch: riscv: smp: allow other IPI implementation #81256

Merged

github-actions bot added the Stale label Nov 19, 2024

luchnikov closed this Nov 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

arch: riscv: Add support for custom SOC IPI, per-core init #65824

arch: riscv: Add support for custom SOC IPI, per-core init #65824

luchnikov commented Nov 27, 2023 •

edited

Loading

cfriedt left a comment

ycsin left a comment

luchnikov commented Nov 29, 2023

nashif commented Nov 29, 2023

ycsin commented Sep 17, 2024

ycsin commented Sep 17, 2024

npitre left a comment

ycsin Sep 17, 2024

ycsin Sep 17, 2024

ycsin Sep 17, 2024

ycsin commented Sep 17, 2024 •

edited

Loading

ycsin left a comment

ycsin Sep 17, 2024

luchnikov Sep 18, 2024

ycsin Sep 19, 2024

npitre commented Sep 17, 2024 via email

ycsin Sep 19, 2024 •

edited

Loading

github-actions bot commented Nov 19, 2024

ycsin commented Nov 19, 2024

		/* IPI */
		#if defined(CONFIG_SMP) && !defined(CONFIG_RISCV_SOC_HAS_CUSTOM_SMP_IPI)

arch: riscv: Add support for custom SOC IPI, per-core init #65824

arch: riscv: Add support for custom SOC IPI, per-core init #65824

Conversation

luchnikov commented Nov 27, 2023 • edited Loading

cfriedt left a comment

Choose a reason for hiding this comment

ycsin left a comment

Choose a reason for hiding this comment

luchnikov commented Nov 29, 2023

nashif commented Nov 29, 2023

ycsin commented Sep 17, 2024

ycsin commented Sep 17, 2024

npitre left a comment

Choose a reason for hiding this comment

ycsin Sep 17, 2024

Choose a reason for hiding this comment

ycsin Sep 17, 2024

Choose a reason for hiding this comment

ycsin Sep 17, 2024

Choose a reason for hiding this comment

ycsin commented Sep 17, 2024 • edited Loading

ycsin left a comment

Choose a reason for hiding this comment

ycsin Sep 17, 2024

Choose a reason for hiding this comment

luchnikov Sep 18, 2024

Choose a reason for hiding this comment

ycsin Sep 19, 2024

Choose a reason for hiding this comment

npitre commented Sep 17, 2024 via email

ycsin Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Nov 19, 2024

ycsin commented Nov 19, 2024

luchnikov commented Nov 27, 2023 •

edited

Loading

ycsin commented Sep 17, 2024 •

edited

Loading

ycsin Sep 19, 2024 •

edited

Loading