On 2024-04-17 09:24, Jan Beulich wrote:
On 08.04.2024 18:56, Jason Andryuk wrote:
On 2024-04-08 03:00, Jan Beulich wrote:
On 04.04.2024 23:25, Jason Andryuk wrote:
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -537,6 +537,111 @@ static paddr_t __init find_memory(
return INVALID_PADDR;
}
+static bool __init check_load_address(
+ const struct domain *d, const struct elf_binary *elf)
+{
+ paddr_t kernel_start = (uintptr_t)elf->dest_base;
+ paddr_t kernel_end = kernel_start + elf->dest_size;
+ unsigned int i;
While properly typed here, ...
+static paddr_t __init find_kernel_memory(
+ const struct domain *d, struct elf_binary *elf,
+ const struct elf_dom_parms *parms)
+{
+ paddr_t kernel_size = elf->dest_size;
+ unsigned int align;
+ int i;
... I must have missed when this was changed to plain int. It should have
been unsigned int here, too, ...
+ if ( parms->phys_align != UNSET_ADDR32 )
+ align = parms->phys_align;
+ else if ( elf->palign >= PAGE_SIZE )
+ align = elf->palign;
+ else
+ align = MB(2);
+
+ /* Search backwards to find the highest address. */
+ for ( i = d->arch.nr_e820 - 1; i >= 0 ; i-- )
... with this suitably adjusted. However, I'm not going to change this while
committing, to avoid screwing up.
I intentionally changed this. Looping downwards, a signed int allows
writing the check naturally with i >= 0. I think it's clearer when
written this way.
Just to clarify: Is
for ( i = d->arch.nr_e820; i--; )
any less clear?
It's not something I normally write, so I had to think about it more. If
you are already familiar with such a construct, then that isn't an issue
for you.
Your way is more subtle in my opinion because it relies on the post
decrement to ensure correct bounds within the loop body. I prefer i >=
0 because it clearly states the valid index values.
Is your main concern that you only want unsigned values as array indices?
(While replying I also notice there's a stray blank
in the for() you have, ahead of the 2nd semicolon.)
Sorry about that.
Regards,
Jason
