| Message ID | 20250319171346.19478-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/boot: Simplify the expression for extra allocation space | expand |
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.cooper3@citrix.com> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> One thought... > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@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. Regards, Jason > + > + last_addr = find_memory(d, &elf, extra_space); > if ( last_addr == INVALID_PADDR ) > { > printk("Unable to find a memory region to load initrd and metadata\n"); > > base-commit: aa78a7d10c6c2ef877b34d119a6db934c201ddfd
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.cooper3@citrix.com> > > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> Thanks. > > One thought... > >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Roger Pau Monné <roger.pau@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-3b82c2b77c9d@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
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); + + last_addr = find_memory(d, &elf, extra_space); if ( last_addr == INVALID_PADDR ) { printk("Unable to find a memory region to load initrd and metadata\n");
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.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/hvm/dom0_build.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) base-commit: aa78a7d10c6c2ef877b34d119a6db934c201ddfd