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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan