| Message ID | 20200305181308.944595-14-seanga2@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: Add Sipeed Maix support | expand |
Hi Sean > The IPI code could have race conditions in several places. > * Several harts could race on the value of gd->arch->clint/plic > * Non-boot harts could race with the main hart on the DM subsystem In > addition, if an IPI was pending when U-Boot started, it would cause the > IPI handler to jump to address 0. > > To address these problems, a new function riscv_init_ipi is introduced. It > is called once during arch_cpu_init_dm. Before this point, no riscv_*_ipi > functions may be called. Access is synchronized by gd->arch->ipi_ready. > > Signed-off-by: Sean Anderson <seanga2 at gmail.com> > --- > > Can you try to clear mip/sip in startup flow before secondary_hart_loop: > Maybe it can help to overcome the problem of calling riscv_clear_ipi() > before riscv_init_ipi() that you added. How is the verified result about trying to clear mip/sip in startup flow ? I have asked you twice in v5, but you still have no response about it. [PATCH v5 27/33] riscv: Fix race conditions when initializing IPI https://patchwork.ozlabs.org/patch/1246864/#2377119 Thanks Rick > Changes in v6: > - Fix some formatting > - Clear IPIs before enabling interrupts instead of using a ipi_ready flag > - Only print messages on error in smp code > > Changes in v5: > - New > > arch/riscv/cpu/cpu.c | 6 ++++ > arch/riscv/cpu/start.S | 2 ++ > arch/riscv/include/asm/smp.h | 43 +++++++++++++++++++++++++++ > arch/riscv/lib/andes_plic.c | 34 ++++++++------------- > arch/riscv/lib/sbi_ipi.c | 5 ++++ > arch/riscv/lib/sifive_clint.c | 33 +++++++-------------- > arch/riscv/lib/smp.c | 56 ++++++++--------------------------- > 7 files changed, 92 insertions(+), 87 deletions(-) > > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c > index e457f6acbf..f851374255 100644 > --- a/arch/riscv/cpu/cpu.c > +++ b/arch/riscv/cpu/cpu.c > @@ -96,6 +96,12 @@ int arch_cpu_init_dm(void) > csr_write(CSR_SATP, 0); > } > > +#ifdef CONFIG_SMP > + ret = riscv_init_ipi(); > + if (ret) > + return ret; > +#endif > + > return 0; > } > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > index 6b3ff99c38..e8740c8568 100644 > --- a/arch/riscv/cpu/start.S > +++ b/arch/riscv/cpu/start.S > @@ -67,6 +67,8 @@ _start: > #else > li t0, SIE_SSIE > #endif > + /* Clear any pending IPIs */ > + csrc MODE_PREFIX(ip), t0 > csrs MODE_PREFIX(ie), t0 > #endif > > diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h > index 74de92ed13..1b428856b2 100644 > --- a/arch/riscv/include/asm/smp.h > +++ b/arch/riscv/include/asm/smp.h > @@ -51,4 +51,47 @@ void handle_ipi(ulong hart); > */ > int smp_call_function(ulong addr, ulong arg0, ulong arg1, int wait); > > +/** > + * riscv_init_ipi() - Initialize inter-process interrupt (IPI) driver > + * > + * Platform code must provide this function. This function is called once after > + * the cpu driver is initialized. No other riscv_*_ipi() calls will be made > + * before this function is called. > + * > + * @return 0 if OK, -ve on error > + */ > +int riscv_init_ipi(void); > + > +/** > + * riscv_send_ipi() - Send inter-processor interrupt (IPI) > + * > + * Platform code must provide this function. > + * > + * @hart: Hart ID of receiving hart > + * @return 0 if OK, -ve on error > + */ > +int riscv_send_ipi(int hart); > + > +/** > + * riscv_clear_ipi() - Clear inter-processor interrupt (IPI) > + * > + * Platform code must provide this function. > + * > + * @hart: Hart ID of hart to be cleared > + * @return 0 if OK, -ve on error > + */ > +int riscv_clear_ipi(int hart); > + > +/** > + * riscv_get_ipi() - Get status of inter-processor interrupt (IPI) > + * > + * Platform code must provide this function. > + * > + * @hart: Hart ID of hart to be checked > + * @pending: Pointer to variable with result of the check, > + * 1 if IPI is pending, 0 otherwise > + * @return 0 if OK, -ve on error > + */ > +int riscv_get_ipi(int hart, int *pending); > + > #endif > diff --git a/arch/riscv/lib/andes_plic.c b/arch/riscv/lib/andes_plic.c > index 20529ab3eb..8484f76386 100644 > --- a/arch/riscv/lib/andes_plic.c > +++ b/arch/riscv/lib/andes_plic.c > @@ -30,20 +30,6 @@ > #define SEND_IPI_TO_HART(hart) (0x80 >> (hart)) > > DECLARE_GLOBAL_DATA_PTR; > -static int init_plic(void); > - > -#define PLIC_BASE_GET(void) \ > - do { \ > - long *ret; \ > - \ > - if (!gd->arch.plic) { \ > - ret = syscon_get_first_range(RISCV_SYSCON_PLIC); \ > - if (IS_ERR(ret)) \ > - return PTR_ERR(ret); \ > - gd->arch.plic = ret; \ > - init_plic(); \ > - } \ > - } while (0) > > static int enable_ipi(int hart) > { > @@ -93,13 +79,21 @@ static int init_plic(void) > return -ENODEV; > } > > +int riscv_init_ipi(void) > +{ > + int ret = syscon_get_first_range(RISCV_SYSCON_PLIC); > + > + if (IS_ERR(ret)) > + return PTR_ERR(ret); > + gd->arch.plic = ret; > + > + return init_plic(); > +} > + > int riscv_send_ipi(int hart) > { > - unsigned int ipi; > + unsigned int ipi = (SEND_IPI_TO_HART(hart) << (8 * gd->arch.boot_hart)); > > - PLIC_BASE_GET(); > - > - ipi = (SEND_IPI_TO_HART(hart) << (8 * gd->arch.boot_hart)); > writel(ipi, (void __iomem *)PENDING_REG(gd->arch.plic, > gd->arch.boot_hart)); > > @@ -110,8 +104,6 @@ int riscv_clear_ipi(int hart) > { > u32 source_id; > > - PLIC_BASE_GET(); > - > source_id = readl((void __iomem *)CLAIM_REG(gd->arch.plic, hart)); > writel(source_id, (void __iomem *)CLAIM_REG(gd->arch.plic, hart)); > > @@ -120,8 +112,6 @@ int riscv_clear_ipi(int hart) > > int riscv_get_ipi(int hart, int *pending) > { > - PLIC_BASE_GET(); > - > *pending = readl((void __iomem *)PENDING_REG(gd->arch.plic, > gd->arch.boot_hart)); > *pending = !!(*pending & SEND_IPI_TO_HART(hart)); > diff --git a/arch/riscv/lib/sbi_ipi.c b/arch/riscv/lib/sbi_ipi.c > index 9a698ce74e..310d1bd2a4 100644 > --- a/arch/riscv/lib/sbi_ipi.c > +++ b/arch/riscv/lib/sbi_ipi.c > @@ -7,6 +7,11 @@ > #include <common.h> > #include <asm/sbi.h> > > +int riscv_init_ipi(void) > +{ > + return 0; > +} > + > int riscv_send_ipi(int hart) > { > ulong mask; > diff --git a/arch/riscv/lib/sifive_clint.c b/arch/riscv/lib/sifive_clint.c > index 5e0d25720b..78fc6c868d 100644 > --- a/arch/riscv/lib/sifive_clint.c > +++ b/arch/riscv/lib/sifive_clint.c > @@ -24,22 +24,8 @@ > > DECLARE_GLOBAL_DATA_PTR; > > -#define CLINT_BASE_GET(void) \ > - do { \ > - long *ret; \ > - \ > - if (!gd->arch.clint) { \ > - ret = syscon_get_first_range(RISCV_SYSCON_CLINT); \ > - if (IS_ERR(ret)) \ > - return PTR_ERR(ret); \ > - gd->arch.clint = ret; \ > - } \ > - } while (0) > - > int riscv_get_time(u64 *time) > { > - CLINT_BASE_GET(); > - > *time = readq((void __iomem *)MTIME_REG(gd->arch.clint)); > > return 0; > @@ -47,17 +33,24 @@ int riscv_get_time(u64 *time) > > int riscv_set_timecmp(int hart, u64 cmp) > { > - CLINT_BASE_GET(); > - > writeq(cmp, (void __iomem *)MTIMECMP_REG(gd->arch.clint, hart)); > > return 0; > } > > +int riscv_init_ipi(void) > +{ > + long *ret = syscon_get_first_range(RISCV_SYSCON_CLINT); > + > + if (IS_ERR(ret)) > + return PTR_ERR(ret); > + gd->arch.clint = ret; > + > + return 0; > +} > + > int riscv_send_ipi(int hart) > { > - CLINT_BASE_GET(); > - > writel(1, (void __iomem *)MSIP_REG(gd->arch.clint, hart)); > > return 0; > @@ -65,8 +58,6 @@ int riscv_send_ipi(int hart) > > int riscv_clear_ipi(int hart) > { > - CLINT_BASE_GET(); > - > writel(0, (void __iomem *)MSIP_REG(gd->arch.clint, hart)); > > return 0; > @@ -74,8 +65,6 @@ int riscv_clear_ipi(int hart) > > int riscv_get_ipi(int hart, int *pending) > { > - CLINT_BASE_GET(); > - > *pending = readl((void __iomem *)MSIP_REG(gd->arch.clint, hart)); > > return 0; > diff --git a/arch/riscv/lib/smp.c b/arch/riscv/lib/smp.c > index 17adb35730..fe992eb00f 100644 > --- a/arch/riscv/lib/smp.c > +++ b/arch/riscv/lib/smp.c > @@ -12,38 +12,6 @@ > > DECLARE_GLOBAL_DATA_PTR; > > -/** > - * riscv_send_ipi() - Send inter-processor interrupt (IPI) > - * > - * Platform code must provide this function. > - * > - * @hart: Hart ID of receiving hart > - * @return 0 if OK, -ve on error > - */ > -extern int riscv_send_ipi(int hart); > - > -/** > - * riscv_clear_ipi() - Clear inter-processor interrupt (IPI) > - * > - * Platform code must provide this function. > - * > - * @hart: Hart ID of hart to be cleared > - * @return 0 if OK, -ve on error > - */ > -extern int riscv_clear_ipi(int hart); > - > -/** > - * riscv_get_ipi() - Get status of inter-processor interrupt (IPI) > - * > - * Platform code must provide this function. > - * > - * @hart: Hart ID of hart to be checked > - * @pending: Pointer to variable with result of the check, > - * 1 if IPI is pending, 0 otherwise > - * @return 0 if OK, -ve on error > - */ > -extern int riscv_get_ipi(int hart, int *pending); > - > static int send_ipi_many(struct ipi_data *ipi, int wait) > { > ofnode node, cpus; > @@ -114,7 +82,6 @@ void handle_ipi(ulong hart) > return; > > __smp_mb(); > - > smp_function = (void (*)(ulong, ulong, ulong))gd->arch.ipi[hart].addr; > invalidate_icache_all(); > > @@ -124,7 +91,13 @@ void handle_ipi(ulong hart) > */ > ret = riscv_clear_ipi(hart); > if (ret) { > - pr_err("Cannot clear IPI of hart %ld\n", hart); > + pr_err("Cannot clear IPI of hart %ld (error %d)\n", hart, ret); > + return; > + } > + > + /* Sanity check */ > + if (!smp_function) { > + pr_err("smp_function is NULL on hart %ld\n", hart); > return; > } > > @@ -133,14 +106,11 @@ void handle_ipi(ulong hart) > > int smp_call_function(ulong addr, ulong arg0, ulong arg1, int wait) > { > - int ret = 0; > - struct ipi_data ipi; > + struct ipi_data ipi = { > + .addr = addr, > + .arg0 = arg0, > + .arg1 = arg1, > + }; > > - ipi.addr = addr; > - ipi.arg0 = arg0; > - ipi.arg1 = arg1; > - > - ret = send_ipi_many(&ipi, wait); > - > - return ret; > + return send_ipi_many(&ipi, wait); > } > -- > 2.25.0 >
On 3/10/20 4:20 AM, Rick Chen wrote: > Hi Sean > >> The IPI code could have race conditions in several places. >> * Several harts could race on the value of gd->arch->clint/plic >> * Non-boot harts could race with the main hart on the DM subsystem In >> addition, if an IPI was pending when U-Boot started, it would cause the >> IPI handler to jump to address 0. >> >> To address these problems, a new function riscv_init_ipi is introduced. It >> is called once during arch_cpu_init_dm. Before this point, no riscv_*_ipi >> functions may be called. Access is synchronized by gd->arch->ipi_ready. >> >> Signed-off-by: Sean Anderson <seanga2 at gmail.com> >> --- >> > >> Can you try to clear mip/sip in startup flow before secondary_hart_loop: >> Maybe it can help to overcome the problem of calling riscv_clear_ipi() >> before riscv_init_ipi() that you added. > > How is the verified result about trying to clear mip/sip in startup flow ? > I have asked you twice in v5, but you still have no response about it. > > [PATCH v5 27/33] riscv: Fix race conditions when initializing IPI > https://patchwork.ozlabs.org/patch/1246864/#2377119 > > Thanks > Rick I managed to get it working, and this patch incorporates that change. However, I forgot to update the commit message. The original issue I had was related to an accidental change to my config, and not to the clearing of MIP. --Sean
Hi Sean > On 3/10/20 4:20 AM, Rick Chen wrote: > > Hi Sean > > > >> The IPI code could have race conditions in several places. > >> * Several harts could race on the value of gd->arch->clint/plic > >> * Non-boot harts could race with the main hart on the DM subsystem In > >> addition, if an IPI was pending when U-Boot started, it would cause the > >> IPI handler to jump to address 0. > >> > >> To address these problems, a new function riscv_init_ipi is introduced. It > >> is called once during arch_cpu_init_dm. Before this point, no riscv_*_ipi > >> functions may be called. Access is synchronized by gd->arch->ipi_ready. > >> > >> Signed-off-by: Sean Anderson <seanga2 at gmail.com> > >> --- > >> > > > >> Can you try to clear mip/sip in startup flow before secondary_hart_loop: > >> Maybe it can help to overcome the problem of calling riscv_clear_ipi() > >> before riscv_init_ipi() that you added. > > > > How is the verified result about trying to clear mip/sip in startup flow ? > > I have asked you twice in v5, but you still have no response about it. > > > > [PATCH v5 27/33] riscv: Fix race conditions when initializing IPI > > https://patchwork.ozlabs.org/patch/1246864/#2377119 > > > > Thanks > > Rick > > I managed to get it working, and this patch incorporates that change. > However, I forgot to update the commit message. The original issue I had > was related to an accidental change to my config, and not to the > clearing of MIP. So it is not a race condition issue, right ? Maybe you shall split this into two patchs as below patch 1 riscv: Clear pending ipi in start code Describe that it can how and what it help to fix the problem you encounter more detail e.g. (Perhaps the prior stage sends an IPI but does not clear it) ... patch 2 riscv: refine ipi initialize code flow Describe that your motivation and intention more detail e.g. I think the macro approach is a bit confusing, since it's unclear at first glance what function will be initializing the clint/plic. Given U-Boot's otherwise completely SMP-unsafe design, I think it's better to be explicit and conservative in these areas. It can help us to roll back and debug in the future. Thanks, Rick > > --Sean
On 3/10/20 9:33 PM, Rick Chen wrote: > Hi Sean > >> On 3/10/20 4:20 AM, Rick Chen wrote: >>> Hi Sean >>> >>>> The IPI code could have race conditions in several places. >>>> * Several harts could race on the value of gd->arch->clint/plic >>>> * Non-boot harts could race with the main hart on the DM subsystem In >>>> addition, if an IPI was pending when U-Boot started, it would cause the >>>> IPI handler to jump to address 0. >>>> >>>> To address these problems, a new function riscv_init_ipi is introduced. It >>>> is called once during arch_cpu_init_dm. Before this point, no riscv_*_ipi >>>> functions may be called. Access is synchronized by gd->arch->ipi_ready. >>>> >>>> Signed-off-by: Sean Anderson <seanga2 at gmail.com> >>>> --- >>>> >>> >>>> Can you try to clear mip/sip in startup flow before secondary_hart_loop: >>>> Maybe it can help to overcome the problem of calling riscv_clear_ipi() >>>> before riscv_init_ipi() that you added. >>> >>> How is the verified result about trying to clear mip/sip in startup flow ? >>> I have asked you twice in v5, but you still have no response about it. >>> >>> [PATCH v5 27/33] riscv: Fix race conditions when initializing IPI >>> https://patchwork.ozlabs.org/patch/1246864/#2377119 >>> >>> Thanks >>> Rick >> >> I managed to get it working, and this patch incorporates that change. >> However, I forgot to update the commit message. The original issue I had >> was related to an accidental change to my config, and not to the >> clearing of MIP. > > So it is not a race condition issue, right ? It is sort of a race condition? If the IP CSR is not cleared, then we can have a race condition. > > Maybe you shall split this into two patchs as below > > patch 1 > riscv: Clear pending ipi in start code > Describe that it can how and what it help to fix the problem you > encounter more detail > e.g. > (Perhaps the prior stage sends an IPI but does not clear it) ... > > patch 2 > riscv: refine ipi initialize code flow > Describe that your motivation and intention more detail > e.g. > I think the macro approach is a bit confusing, > since it's unclear at first glance what function > will be initializing the clint/plic. > Given U-Boot's otherwise completely SMP-unsafe design, > I think it's better to be explicit and conservative in these areas. > > It can help us to roll back and debug in the future. > > Thanks, > Rick This will be split in the next revision. >> >> --Sean
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index e457f6acbf..f851374255 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -96,6 +96,12 @@ int arch_cpu_init_dm(void) csr_write(CSR_SATP, 0); } +#ifdef CONFIG_SMP + ret = riscv_init_ipi(); + if (ret) + return ret; +#endif + return 0; } diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 6b3ff99c38..e8740c8568 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -67,6 +67,8 @@ _start: #else li t0, SIE_SSIE #endif + /* Clear any pending IPIs */ + csrc MODE_PREFIX(ip), t0 csrs MODE_PREFIX(ie), t0 #endif diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h index 74de92ed13..1b428856b2 100644 --- a/arch/riscv/include/asm/smp.h +++ b/arch/riscv/include/asm/smp.h @@ -51,4 +51,47 @@ void handle_ipi(ulong hart); */ int smp_call_function(ulong addr, ulong arg0, ulong arg1, int wait); +/** + * riscv_init_ipi() - Initialize inter-process interrupt (IPI) driver + * + * Platform code must provide this function. This function is called once after + * the cpu driver is initialized. No other riscv_*_ipi() calls will be made + * before this function is called. + * + * @return 0 if OK, -ve on error + */ +int riscv_init_ipi(void); + +/** + * riscv_send_ipi() - Send inter-processor interrupt (IPI) + * + * Platform code must provide this function. + * + * @hart: Hart ID of receiving hart + * @return 0 if OK, -ve on error + */ +int riscv_send_ipi(int hart); + +/** + * riscv_clear_ipi() - Clear inter-processor interrupt (IPI) + * + * Platform code must provide this function. + * + * @hart: Hart ID of hart to be cleared + * @return 0 if OK, -ve on error + */ +int riscv_clear_ipi(int hart); + +/** + * riscv_get_ipi() - Get status of inter-processor interrupt (IPI) + * + * Platform code must provide this function. + * + * @hart: Hart ID of hart to be checked + * @pending: Pointer to variable with result of the check, + * 1 if IPI is pending, 0 otherwise + * @return 0 if OK, -ve on error + */ +int riscv_get_ipi(int hart, int *pending); + #endif diff --git a/arch/riscv/lib/andes_plic.c b/arch/riscv/lib/andes_plic.c index 20529ab3eb..8484f76386 100644 --- a/arch/riscv/lib/andes_plic.c +++ b/arch/riscv/lib/andes_plic.c @@ -30,20 +30,6 @@ #define SEND_IPI_TO_HART(hart) (0x80 >> (hart)) DECLARE_GLOBAL_DATA_PTR; -static int init_plic(void); - -#define PLIC_BASE_GET(void) \ - do { \ - long *ret; \ - \ - if (!gd->arch.plic) { \ - ret = syscon_get_first_range(RISCV_SYSCON_PLIC); \ - if (IS_ERR(ret)) \ - return PTR_ERR(ret); \ - gd->arch.plic = ret; \ - init_plic(); \ - } \ - } while (0) static int enable_ipi(int hart) { @@ -93,13 +79,21 @@ static int init_plic(void) return -ENODEV; } +int riscv_init_ipi(void) +{ + int ret = syscon_get_first_range(RISCV_SYSCON_PLIC); + + if (IS_ERR(ret)) + return PTR_ERR(ret); + gd->arch.plic = ret; + + return init_plic(); +} + int riscv_send_ipi(int hart) { - unsigned int ipi; + unsigned int ipi = (SEND_IPI_TO_HART(hart) << (8 * gd->arch.boot_hart)); - PLIC_BASE_GET(); - - ipi = (SEND_IPI_TO_HART(hart) << (8 * gd->arch.boot_hart)); writel(ipi, (void __iomem *)PENDING_REG(gd->arch.plic, gd->arch.boot_hart)); @@ -110,8 +104,6 @@ int riscv_clear_ipi(int hart) { u32 source_id; - PLIC_BASE_GET(); - source_id = readl((void __iomem *)CLAIM_REG(gd->arch.plic, hart)); writel(source_id, (void __iomem *)CLAIM_REG(gd->arch.plic, hart)); @@ -120,8 +112,6 @@ int riscv_clear_ipi(int hart) int riscv_get_ipi(int hart, int *pending) { - PLIC_BASE_GET(); - *pending = readl((void __iomem *)PENDING_REG(gd->arch.plic, gd->arch.boot_hart)); *pending = !!(*pending & SEND_IPI_TO_HART(hart)); diff --git a/arch/riscv/lib/sbi_ipi.c b/arch/riscv/lib/sbi_ipi.c index 9a698ce74e..310d1bd2a4 100644 --- a/arch/riscv/lib/sbi_ipi.c +++ b/arch/riscv/lib/sbi_ipi.c @@ -7,6 +7,11 @@ #include <common.h> #include <asm/sbi.h> +int riscv_init_ipi(void) +{ + return 0; +} + int riscv_send_ipi(int hart) { ulong mask; diff --git a/arch/riscv/lib/sifive_clint.c b/arch/riscv/lib/sifive_clint.c index 5e0d25720b..78fc6c868d 100644 --- a/arch/riscv/lib/sifive_clint.c +++ b/arch/riscv/lib/sifive_clint.c @@ -24,22 +24,8 @@ DECLARE_GLOBAL_DATA_PTR; -#define CLINT_BASE_GET(void) \ - do { \ - long *ret; \ - \ - if (!gd->arch.clint) { \ - ret = syscon_get_first_range(RISCV_SYSCON_CLINT); \ - if (IS_ERR(ret)) \ - return PTR_ERR(ret); \ - gd->arch.clint = ret; \ - } \ - } while (0) - int riscv_get_time(u64 *time) { - CLINT_BASE_GET(); - *time = readq((void __iomem *)MTIME_REG(gd->arch.clint)); return 0; @@ -47,17 +33,24 @@ int riscv_get_time(u64 *time) int riscv_set_timecmp(int hart, u64 cmp) { - CLINT_BASE_GET(); - writeq(cmp, (void __iomem *)MTIMECMP_REG(gd->arch.clint, hart)); return 0; } +int riscv_init_ipi(void) +{ + long *ret = syscon_get_first_range(RISCV_SYSCON_CLINT); + + if (IS_ERR(ret)) + return PTR_ERR(ret); + gd->arch.clint = ret; + + return 0; +} + int riscv_send_ipi(int hart) { - CLINT_BASE_GET(); - writel(1, (void __iomem *)MSIP_REG(gd->arch.clint, hart)); return 0; @@ -65,8 +58,6 @@ int riscv_send_ipi(int hart) int riscv_clear_ipi(int hart) { - CLINT_BASE_GET(); - writel(0, (void __iomem *)MSIP_REG(gd->arch.clint, hart)); return 0; @@ -74,8 +65,6 @@ int riscv_clear_ipi(int hart) int riscv_get_ipi(int hart, int *pending) { - CLINT_BASE_GET(); - *pending = readl((void __iomem *)MSIP_REG(gd->arch.clint, hart)); return 0; diff --git a/arch/riscv/lib/smp.c b/arch/riscv/lib/smp.c index 17adb35730..fe992eb00f 100644 --- a/arch/riscv/lib/smp.c +++ b/arch/riscv/lib/smp.c @@ -12,38 +12,6 @@ DECLARE_GLOBAL_DATA_PTR; -/** - * riscv_send_ipi() - Send inter-processor interrupt (IPI) - * - * Platform code must provide this function. - * - * @hart: Hart ID of receiving hart - * @return 0 if OK, -ve on error - */ -extern int riscv_send_ipi(int hart); - -/** - * riscv_clear_ipi() - Clear inter-processor interrupt (IPI) - * - * Platform code must provide this function. - * - * @hart: Hart ID of hart to be cleared - * @return 0 if OK, -ve on error - */ -extern int riscv_clear_ipi(int hart); - -/** - * riscv_get_ipi() - Get status of inter-processor interrupt (IPI) - * - * Platform code must provide this function. - * - * @hart: Hart ID of hart to be checked - * @pending: Pointer to variable with result of the check, - * 1 if IPI is pending, 0 otherwise - * @return 0 if OK, -ve on error - */ -extern int riscv_get_ipi(int hart, int *pending); - static int send_ipi_many(struct ipi_data *ipi, int wait) { ofnode node, cpus; @@ -114,7 +82,6 @@ void handle_ipi(ulong hart) return; __smp_mb(); - smp_function = (void (*)(ulong, ulong, ulong))gd->arch.ipi[hart].addr; invalidate_icache_all(); @@ -124,7 +91,13 @@ void handle_ipi(ulong hart) */ ret = riscv_clear_ipi(hart); if (ret) { - pr_err("Cannot clear IPI of hart %ld\n", hart); + pr_err("Cannot clear IPI of hart %ld (error %d)\n", hart, ret); + return; + } + + /* Sanity check */ + if (!smp_function) { + pr_err("smp_function is NULL on hart %ld\n", hart); return; } @@ -133,14 +106,11 @@ void handle_ipi(ulong hart) int smp_call_function(ulong addr, ulong arg0, ulong arg1, int wait) { - int ret = 0; - struct ipi_data ipi; + struct ipi_data ipi = { + .addr = addr, + .arg0 = arg0, + .arg1 = arg1, + }; - ipi.addr = addr; - ipi.arg0 = arg0; - ipi.arg1 = arg1; - - ret = send_ipi_many(&ipi, wait); - - return ret; + return send_ipi_many(&ipi, wait); }
The IPI code could have race conditions in several places. * Several harts could race on the value of gd->arch->clint/plic * Non-boot harts could race with the main hart on the DM subsystem In addition, if an IPI was pending when U-Boot started, it would cause the IPI handler to jump to address 0. To address these problems, a new function riscv_init_ipi is introduced. It is called once during arch_cpu_init_dm. Before this point, no riscv_*_ipi functions may be called. Access is synchronized by gd->arch->ipi_ready. Signed-off-by: Sean Anderson <seanga2 at gmail.com> --- Changes in v6: - Fix some formatting - Clear IPIs before enabling interrupts instead of using a ipi_ready flag - Only print messages on error in smp code Changes in v5: - New arch/riscv/cpu/cpu.c | 6 ++++ arch/riscv/cpu/start.S | 2 ++ arch/riscv/include/asm/smp.h | 43 +++++++++++++++++++++++++++ arch/riscv/lib/andes_plic.c | 34 ++++++++------------- arch/riscv/lib/sbi_ipi.c | 5 ++++ arch/riscv/lib/sifive_clint.c | 33 +++++++-------------- arch/riscv/lib/smp.c | 56 ++++++++--------------------------- 7 files changed, 92 insertions(+), 87 deletions(-)