Re: [PATCH v2 05/11] xen/riscv: add kernel loading support

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.03.2026 16:30, Oleksii Kurochko wrote:
> On 3/30/26 4:47 PM, Jan Beulich wrote:
>> On 23.03.2026 17:29, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/include/asm/config.h
>>> +++ b/xen/arch/riscv/include/asm/config.h
>>> @@ -151,6 +151,19 @@
>>>   extern unsigned long phys_offset; /* = load_start - XEN_VIRT_START */
>>>   #endif
>>>   
>>> +/*
>>> + * KERNEL_LOAD_ADDR_ALIGNMENT is defined based on paragraph of
>>> + * "Kernel location" of boot.rst:
>>> + * https://docs.kernel.org/arch/riscv/boot.html#kernel-location
>>> + */
>>> +#if defined(CONFIG_RISCV_32)
>>> +#define KERNEL_LOAD_ADDR_ALIGNMENT MB(4)
>>> +#elif defined(CONFIG_RISCV_64)
>>> +#define KERNEL_LOAD_ADDR_ALIGNMENT MB(2)
>>> +#else
>>> +#error "Define KERNEL_LOAD_ADDR_ALIGNMENT"
>>> +#endif
>>
>> But that's Linux-specific. You want to be able to loader other OS kernels,
>> I suppose? The needed alignment should be a property of the kernel image,
>> suitably conveyed to the loader.
> 
> Then likely some updates will be needed...
> 
>>
>> Is Arm similarly capable of loading only Linux images? What about in
>> particular XTF?
> 
> ... they are pretend as they are Linux kernel zImage:
> 
> https://gitlab.com/xen-project/fusa/xtf/-/commit/dec72d83291d6782b3f41df66987c8a25eac422f#line_9f6eadcd7_A42
> 
> and in the case of XTF:
>      /* Magic number used to identify this is an ARM Linux zImage */
>      .word   ZIMAGE_MAGIC_NUMBER
>      /* The address the zImage starts at (0 = relocatable) */
>      .word   0
>      /* The address the zImage ends at */
>      .word   (_end - _start)
> 
> zImage.start is set to 0 so KERNEL_LOAD_ADDR_ALIGNMENT won't be applied 
> and load address from DTS's kernel node will be taken.
> 
> Other example in mind I have it is Zephyr OS, and the also use Image 
> protocol by enabling CONFIG_AARCH64_IMAGE_HEADER. So Xen can boot it too.

ISTR Andrew saying that he'd really like to be able to use plain ELF.
Anyway, if Linux Image is clearly stated as a only thing presently
supported, that's perhaps okay for the time being.

>>> +static void __init kernel_image_load(struct kernel_info *info)
>>> +{
>>> +    int rc;
>>> +    paddr_t load_addr = kernel_image_place(info);
>>> +    paddr_t paddr = info->image.kernel_addr;
>>> +    paddr_t len = info->image.len;
>>> +    void *kernel;
>>> +
>>> +    info->entry = load_addr;
>>
>> What if this is outside of memory bank 0 (as is possible when
>> info->image.start is non-zero).
> 
> It will be an issue and panic() will occur in place_modules().

Will it? I just looked again, and I can't help thinking that it won't.

>>> +    /* Currently there is no length in the header, so just use the size */
>>> +    start = 0;
>>> +    end = size;
>>
>> What's image_size then?
> 
> The comment is incorrect, the length is present in the header, but it is 
> effective length which isn't equal to the size of binary and is actually 
> bigger then binary size.
> 
> So here we want to use 'size' as it is a size of binary itself.

What is "effective length"? That sounds a little like e.g. .bss extending
past the (file) image, yet such would nee taking into account for allocation
(but not for reading in / copying over).

Jan