On 9/10/20 3:08 AM, Rick Chen wrote: > Hi Sean > >> On 9/8/20 10:02 PM, Rick Chen wrote: >>> Hi Sean >>> >>>> On the K210, the prior stage bootloader does not clear IPIs. This presents >>>> a problem, because U-Boot up until this point assumes (with one exception) >>>> that IPIs are cleared when it starts. This series attempts to fix this in a >>>> robust manner, and fix several concurrency bugs I noticed while fixing >>>> these other areas. Heinrich previously submitted a patch addressing part of >>>> this problem in [1]. >>>> >>>> [1] >>>> https://patchwork.ozlabs.org/project/uboot/patch/20200811035648.3284-1-xypron.g...@gmx.de/ >>>> >>> >>> It sounds like that the bootloader does not deal with SMP flow well >>> and jump to u-boot-spl, right ? >>> >>> I have a question that why not try to fix the prior stage bootloader >>> to clear IPIs correctly? >> >> Because it is a ROM :) > > Is it a mask ROM or flash ROM ?
I don't know, it's not documented. From what I can tell, it's controlled by the OTP device. However, I haven't thoroughly investigated it. > >> >>> >>> Actually I have encounter a similar SMP issue like you. >>> Our prior stage bootloader will jump to u-boot-spl with the incorrect >>> mstatus and result in the SMP working abnormal in u-boot-spl. >> >> Perhaps we should just clear MIE then? I originally had a patch in this >> series which moved the handle_ipi code into handle_trap, and got rid of >> the manual checks on the interrupt. Something like >> >> secondary_hart_loop: >> wfi >> j secondary_hart_loop >> >> Of course as part of that we would need to explicitly enable and disable >> interrupts. Perhaps not the worst idea, but I didn't include it here >> because I figure the current system work OK, even if it is not what one >> might expect. >> >>> I mean this is an individual case, not a general case. >>> If we try to cover any errors which come from any prior stage bootloaders, >>> the SMP flow will become more and more complicated and hard to debug. >> >> Of course, if a prior bootloader doesn't hold up its end of the >> contract, we can be left with some awful bugs to fix. U-Boot is >> generally not too bad to debug, but I've had an awful time whenever some >> concurrency sneaks into the mix. I think it's much better to confine the >> (necessary) complexity to as few files as possible, so that the rest of >> the code can be ignorant. I think part of that is verifying that we have >> everything in a known state, so that when we see something unexpected, >> we can handle it/panic/whatever instead of silently getting a bug. > > It sounds like an error handling and the errors come from the prior > stage bootloader. > Without U-Boot, does Kernel handle this kind of IPIs not clean > unexpected errors ? Well, software interrupts are disabled on each hart until riscv_intc_init is called (and enables soft irqs). This prevents the handler from being called before ipi_data is initialized. After that, handle_IPI can be called, but if ops == 0 (e.g. the IPI wasn't signaled by Linux), then it just goes to done. --Sean