Am 9. August 2020 03:48:19 MESZ schrieb Sean Anderson <sean...@gmail.com>: >On 8/8/20 2:56 PM, Heinrich Schuchardt wrote: >> On 8/8/20 7:22 PM, Sean Anderson wrote: >>> On 8/8/20 12:17 PM, Heinrich Schuchardt wrote: >>>> On 8/8/20 5:32 PM, Sean Anderson wrote: >>>>> On 8/8/20 10:59 AM, Heinrich Schuchardt wrote: >>>>>> Hello Anup, >>>>>> >>>>>> I have looking at you OpenSBI code firmware/payloads/test_head.S. >Here >>>>> >>>>> I think the real start is in firmware/fw_base.S. From there, >secondary >>>>> harts loop first in _wait_relocate_copy_done, and then in >>>>> _wait_for_boot_hart, and then they execute the next stage via >>>>> _start_warm and sbi_init. >>>>> >>>>>> like in U-Boot's common/spl/spl_opensbi.c you put all but one >hart in to >>>>>> an enless loop (hang). >>>>> >>>>> As far as I can tell, U-Boot has all harts execute the next stage >when >>>>> SMP is enabled. smp_call_function has all harts execute that >function. >>>> >>>> U-Boot can only run on one hart. Are the other harts trapped in >>>> secondary_hart_loop()? >>> >>> Yes. They also need handle_ipi, and by extension riscv_clear_ipi. >This >>> latter function currently requires that gd_t be valid, and may >require >>> other structures (e.g. a struct udevice) to be valid in the future. >>> >>>> How do we ensure that an UEFI payload does not overwrite this >memory location? >>> >>> The most foolproof is probably to wait for all harts to start >running >>> UEFI code before making any modifications to ram outside the binary. >One >>> easy way to do this is to use amoadd instead of amoswap (e.g. a >semaphor >>> and not a mutex) in the standard boot lottery code. Whichever hart >gets >>> to it first then waits for the value of hart_lottery to reach the >>> expected number of harts. >> >> There is no such requirement in the UEFI specification. > >Hm, well perhaps there should be a shim (or patch to U-Boot) which >implements such a requirement. AFAIK (and please correct me if there is >another option) there is no way to communicate between harts except by >interrupt or shared memory. Interrupts may require substantial code to >handle properly (which could be difficult to trace the requirements >of). >However, shared memory only requires one or two functions to be valid, >plus the memory location itself. I think that solution has been mostly >avoided by U-Boot since the rest of U-Boot is not designed for SMP, so >there is not much infrastructure for atomics, etc. > >> The way to tell which memory should not be overwritten by the UEFI >> payload is an entry in the memory map that the payload can read via >> GetMemoryMap(). So we have to make reservations in the memory map, by >> calling efi_add_memory_map() or by putting the code into the >> __efi_runtime section. >> >> Do I understand it correctly that the secondary harts stay in the >> unrelocated secondary_hart_loop()? In this case __efi_runtime would >not >> be the right section, because that memory section is also relocated >and >> only the relocated code is reserved in the memory map. > >The other harts get relocated in relocate_secondary_harts. > >> >> We would also have to consider the location of secondary_hart_loop() >> when defining the load address of any payload be it UEFI or not. > >Yes. I was looking at your patch to add load addresses [1], and I'm >concerned that 80400000 may be too high for the average kernel. Since >U-Boot relocates to just below 80600000, that leaves only 1.5M or so >for >the kernel to be loaded into.
The patch has: kernel_addr_r=0x80040000 You somehow missed the position of "4". > >--Sean > >[1] >https://patchwork.ozlabs.org/project/uboot/patch/20200729154235.90766-4-xypron.g...@gmx.de/