Re: [PATCH v3 05/12] xen/riscv: add kernel loading support

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

On 4/22/26 3:54 PM, Jan Beulich wrote:
> On 22.04.2026 15:47, Oleksii Kurochko wrote:
>> On 4/22/26 1:57 PM, Jan Beulich wrote:
>>> On 22.04.2026 13:45, Oleksii Kurochko wrote:
>>>> On 4/21/26 10:57 AM, Jan Beulich wrote:
>>>>> On 10.04.2026 17:54, Oleksii Kurochko wrote:
>>>>>> +static paddr_t __init kernel_image_place(struct kernel_info *info)
>>>>>> +{
>>>>>> +    paddr_t load_addr = INVALID_PADDR;
>>>>>> +    uint64_t image_size = info->image.image_size ?: info->image.len;
>>>>>> +    const struct membanks *banks = kernel_info_get_mem_const(info);
>>>>>> +    unsigned int nr_banks = banks->nr_banks;
>>>>>> +    unsigned int bi;
>>>>>> +
>>>>>> +    dprintk(XENLOG_DEBUG, "nr_banks(%u)\n", nr_banks);
>>>>>> +
>>>>>> +    /*
>>>>>> +     * At the moment, RISC-V's Linux kernel should be always position
>>>>>> +     * independent based on "Per-MMU execution" of boot.rst:
>>>>>> +     *   https://docs.kernel.org/arch/riscv/boot.html#pre-mmu-execution
>>>>>> +     *
>>>>>> +     * But just for the case when RISC-V's Linux kernel isn't position
>>>>>> +     * independent it is needed to take load address from
>>>>>> +     * info->image.start.
>>>>>> +     *
>>>>>> +     * If `start` is zero, the Image is position independent.
>>>>>> +     */
>>>>>> +    if ( likely(!info->image.start) )
>>>>>> +    {
>>>>>> +        for ( bi = 0; bi != nr_banks; bi++ )
>>>>>> +        {
>>>>>> +            const struct membank *bank = &banks->bank[bi];
>>>>>> +            paddr_t bank_start = bank->start;
>>>>>> +            /*
>>>>>> +             * According to boot.rst kernel load address should be properly
>>>>>> +             * aligned:
>>>>>> +             *   https://docs.kernel.org/arch/riscv/boot.html#kernel-location
>>>>>> +             *
>>>>>> +             * As Image in this case is PIC we can ignore
>>>>>> +             * info->image.text_offset.
>>>>>> +             */
>>>>>> +            paddr_t aligned_start = ROUNDUP(bank_start, KERNEL_LOAD_ADDR_ALIGNMENT);
>>>>>> +            paddr_t bank_end = bank_start + bank->size;
>>>>>> +            paddr_t bank_size;
>>>>>> +
>>>>>> +            if ( aligned_start > bank_end )
>>>>>> +                continue;
>>>>>> +
>>>>>> +            bank_size = bank_end - aligned_start;
>>>>>> +
>>>>>> +            dprintk(XENLOG_DEBUG, "bank[%u].start=%"PRIpaddr"\n", bi, bank->start);
>>>>>> +
>>>>>> +            if ( image_size <= bank_size )
>>>>>> +            {
>>>>>> +                load_addr = aligned_start;
>>>>>> +                break;
>>>>>> +            }
>>>>>> +        }
>>>>>> +    }
>>>>>> +    else
>>>>>> +    {
>>>>>> +        load_addr = info->image.start + info->image.text_offset;
>>>>>
>>>>> Why does stuff ahead of text_offset not need loading?
>>>>
>>>> Here we just calculating only a place where kernel will be loaded. The
>>>> full kernel image will be loaded in kernel_image_load().
>>>
>>> Okay, but if you calculate an address where the full image won't fit,
>>> how are things going to work?
>>
>> If the full image won't fit than the necessary bank won't be found in
>> for() loop below and so this kernel will be rejected.
>>
>> I expect that in the case when info->image.start is not 0 (so isn't
>> Image isn't PIC) Image want to be specifically load to info->image.start
>> + info->image.text_offset. Is it wrong statement?
> 
> I don't know, but the adding in of .text_offset looks questionable to me.
> I simply don't know why such offsetting would be wanted / needed.

In the <linux>/Documentation/arch/riscv/boot-image-header.rst:

This header format is compliant with PE/COFF header and largely inspired 
from ARM64 header. Thus, both ARM64 & RISC-V header can be combined into 
one common header in future.

Than if to look at the comment near text_offset it is mentioned that:
/* Image load offset, little endian */

So it is basically Image load offset and that is why I expect it should 
be added to calculation of load_addr.

Considering that boot image header is based on Arm64 I assume that an 
explanation regarding text_offset for Arm64 
(<linux>/Documentation/arch/arm64/booting.rst:) should be true for RISC-V:
```
The Image must be placed text_offset bytes from a 2MB aligned base
address anywhere in usable system RAM and called there. The region
between the 2 MB aligned base address and the start of the image has no
special significance to the kernel, and may be used for other purposes.
At least image_size bytes from the start of the image must be free for
use by the kernel.
```

~ Oleksii