On 19/03/2025 9:41 pm, Jason Andryuk wrote: > On 2025-03-19 13:13, Andrew Cooper wrote: >> The expression for one parameter of find_memory() is already >> complicated and >> about to become moreso. Break it out into a new variable, and >> express it in >> an easier-to-follow way. >> >> No functional change. >> >> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> > > Reviewed-by: Jason Andryuk <jason.andr...@amd.com>
Thanks. > > One thought... > >> --- >> CC: Jan Beulich <jbeul...@suse.com> >> CC: Roger Pau Monné <roger....@citrix.com> >> --- >> xen/arch/x86/hvm/dom0_build.c | 18 +++++++++++------- >> 1 file changed, 11 insertions(+), 7 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/dom0_build.c >> b/xen/arch/x86/hvm/dom0_build.c >> index 6a4453103a9a..6591949984b8 100644 >> --- a/xen/arch/x86/hvm/dom0_build.c >> +++ b/xen/arch/x86/hvm/dom0_build.c >> @@ -654,6 +654,7 @@ static int __init pvh_load_kernel( >> const char *cmdline = image->cmdline_pa ? >> __va(image->cmdline_pa) : NULL; >> struct elf_binary elf; >> struct elf_dom_parms parms; >> + size_t extra_space; >> paddr_t last_addr; >> struct hvm_start_info start_info = { 0 }; >> struct hvm_modlist_entry mod = { 0 }; >> @@ -711,13 +712,16 @@ static int __init pvh_load_kernel( >> * split into smaller allocations, done as a single region in >> order to >> * simplify it. >> */ >> - last_addr = find_memory(d, &elf, sizeof(start_info) + >> - (initrd ? ROUNDUP(initrd_len, PAGE_SIZE) + >> - sizeof(mod) >> - : 0) + >> - (cmdline ? ROUNDUP(strlen(cmdline) + 1, >> - elf_64bit(&elf) ? 8 : 4) >> - : 0)); >> + extra_space = sizeof(start_info); >> + >> + if ( initrd ) >> + extra_space += sizeof(mod) + ROUNDUP(initrd_len, PAGE_SIZE); >> + >> + if ( cmdline ) >> + extra_space += ROUNDUP(strlen(cmdline) + 1, >> + elf_64bit(&elf) ? 8 : 4); > > These component values are re-calculated below. With additional > variables, they could be calculated once and used for find_memory() > and later last_addr adjustments. This is a prerequisite for Jan's https://lore.kernel.org/xen-devel/730d8143-8cda-49da-a48a-3b82c2b77...@suse.com/T/#u which adjusts several aspects. Given Jan is intending to do some cleanup in this area, I'll not complicated my change further. If there are dregs left afterwards, the can be cleaned up too. ~Andrew