OpenBSI and U-Boot

[Heinrich Schuchardt]

Am 9. August 2020 03:48:19 MESZ schrieb Sean Anderson <seanga2 at 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.glpk@gmx.de/