On 8/9/20 12:23 AM, Heinrich Schuchardt wrote: > 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".
Ah, so I did. That looks fine then. --Sean
