Re: [PATCH v6 3/4] libelf: Store maximum PHDR p_align

[Jason Andryuk <jason.andryuk@amd.com>]

On 2024-03-28 12:47, Jan Beulich wrote:
> On 27.03.2024 22:51, Jason Andryuk wrote:
>> --- a/xen/common/libelf/libelf-loader.c
>> +++ b/xen/common/libelf/libelf-loader.c
>> @@ -468,6 +468,7 @@ void elf_parse_binary(struct elf_binary *elf)
>>   {
>>       ELF_HANDLE_DECL(elf_phdr) phdr;
>>       uint64_t low = -1, high = 0, paddr, memsz;
>> +    uint64_t max_align = 0, palign;
>>       unsigned i, count;
>>   
>>       count = elf_phdr_count(elf);
>> @@ -481,17 +482,23 @@ void elf_parse_binary(struct elf_binary *elf)
>>               continue;
>>           paddr = elf_uval(elf, phdr, p_paddr);
>>           memsz = elf_uval(elf, phdr, p_memsz);
>> -        elf_msg(elf, "ELF: phdr: paddr=%#" PRIx64 " memsz=%#" PRIx64 "\n",
>> -                paddr, memsz);
>> +        palign = elf_uval(elf, phdr, p_align);
>> +        elf_msg(elf,
>> +                "ELF: phdr: paddr=%#" PRIx64 " memsz=%#" PRIx64 " palign=%#" PRIx64 "\n",
>> +                paddr, memsz, palign);
>>           if ( low > paddr )
>>               low = paddr;
>>           if ( high < paddr + memsz )
>>               high = paddr + memsz;
>> +        if ( max_align < palign )
>> +            max_align = palign;
>>       }
>>       elf->pstart = low;
>>       elf->pend = high;
>> -    elf_msg(elf, "ELF: memory: %#" PRIx64 " -> %#" PRIx64 "\n",
>> -            elf->pstart, elf->pend);
>> +    elf->palign = max_align;
>> +    elf_msg(elf,
>> +            "ELF: memory: %#" PRIx64 " -> %#" PRIx64 " align:%#" PRIx64 "\n",
>> +            elf->pstart, elf->pend, elf->palign);
>>   }
> 
> Hmm, it's just this final logging change which I'm a little concerned by:
> Having looked at Linux'es phdr, I noticed that the addresses there aren't
> necessarily matching the corresponding alignment. Therefore I'm a little
> concerned that the output here might raise questions when people see
> seemingly inconsistent values in the log. Could you/we at least make it
> read like e.g. "align (max): ..."?

max_align?

Looking at my test vmlinux, it looks like PHDR 0 (.text) and PHDR 1 
(.data) are aligned.  It's the PHDR 2 (.init) that isn't aligned.  Is 
that what you see?

This line is already printing the min and max across all the PHDRs, so 
it would only look confusing if the start didn't match the align.

I'm not sure how useful it is to print the alignment, and I considered 
not printing it.  It's only used with PVH Dom0 right now, so it's not 
relevant in most cases.

Regards,
Jason