| Message ID | 20241006214956.24339-35-dpsmith@apertussolutions.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Boot modules for Hyperlaunch | expand |
On 2024-10-06 17:49, Daniel P. Smith wrote: > Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> > --- > xen/arch/x86/setup.c | 13 ++++--------- > 1 file changed, 4 insertions(+), 9 deletions(-) > > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index 30a139074833..b3b6e6f38622 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -276,8 +276,6 @@ custom_param("acpi", parse_acpi_param); > > static const char *cmdline_cook(const char *p, const char *loader_name); > > -static const struct boot_module *__initdata initial_images; > - > struct boot_info __initdata xen_boot_info; > > static struct boot_info __init *multiboot_fill_boot_info(unsigned long mbi_p) > @@ -336,9 +334,9 @@ unsigned long __init initial_images_nrpages(nodeid_t node) > > for ( nr = i = 0; i < bi->nr_modules; ++i ) > { > - unsigned long start = initial_images[i].mod->mod_start; > + unsigned long start = bi->mods[i].mod->mod_start; > unsigned long end = start + > - PFN_UP(initial_images[i].mod->mod_end); > + PFN_UP(bi->mods[i].mod->mod_end); Fits on a single line. > > if ( end > node_start && node_end > start ) > nr += min(node_end, end) - max(node_start, start); > @@ -355,15 +353,14 @@ void __init discard_initial_images(void) > for ( i = 0; i < bi->nr_modules; ++i ) > { > uint64_t start = > - (uint64_t)initial_images[i].mod->mod_start << PAGE_SHIFT; > + (uint64_t)bi->mods[i].mod->mod_start << PAGE_SHIFT; Fits on one line. Can also be pfn_to_paddr(), which applies to earlier patches. Having said that, maybe it's okay to skip pfn_to_paddr as at the end of the series mods[i].start is used without a shift. i.e. fewer transformations in these "mechanical" changes make review easier. Unless someone else wants pfn_to_addr(), I am okay without that conversion. > > init_domheap_pages(start, > start + > - PAGE_ALIGN(initial_images[i].mod->mod_end)); > + PAGE_ALIGN(bi->mods[i].mod->mod_end)); One line. > } > > bi->nr_modules = 0; > - initial_images = NULL; > } > > static void __init init_idle_domain(void) With the line fixups: Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
On 10/8/24 15:04, Jason Andryuk wrote: > On 2024-10-06 17:49, Daniel P. Smith wrote: >> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> >> --- >> xen/arch/x86/setup.c | 13 ++++--------- >> 1 file changed, 4 insertions(+), 9 deletions(-) >> >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c >> index 30a139074833..b3b6e6f38622 100644 >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -276,8 +276,6 @@ custom_param("acpi", parse_acpi_param); >> static const char *cmdline_cook(const char *p, const char >> *loader_name); >> -static const struct boot_module *__initdata initial_images; >> - >> struct boot_info __initdata xen_boot_info; >> static struct boot_info __init *multiboot_fill_boot_info(unsigned >> long mbi_p) >> @@ -336,9 +334,9 @@ unsigned long __init >> initial_images_nrpages(nodeid_t node) >> for ( nr = i = 0; i < bi->nr_modules; ++i ) >> { >> - unsigned long start = initial_images[i].mod->mod_start; >> + unsigned long start = bi->mods[i].mod->mod_start; >> unsigned long end = start + >> - PFN_UP(initial_images[i].mod->mod_end); >> + PFN_UP(bi->mods[i].mod->mod_end); > > Fits on a single line. Ack. >> if ( end > node_start && node_end > start ) >> nr += min(node_end, end) - max(node_start, start); >> @@ -355,15 +353,14 @@ void __init discard_initial_images(void) >> for ( i = 0; i < bi->nr_modules; ++i ) >> { >> uint64_t start = >> - (uint64_t)initial_images[i].mod->mod_start << PAGE_SHIFT; >> + (uint64_t)bi->mods[i].mod->mod_start << PAGE_SHIFT; > > Fits on one line. Can also be pfn_to_paddr(), which applies to earlier > patches. Having said that, maybe it's okay to skip pfn_to_paddr as at > the end of the series mods[i].start is used without a shift. i.e. fewer > transformations in these "mechanical" changes make review easier. Unless > someone else wants pfn_to_addr(), I am okay without that conversion. After I did a few based on a comment from Jan on v4, I realized I am making changes that just go away or shift to a different conversion macro. I would prefer to just leave them to the end and make sure that when arriving at the final form, none of them are using any open coded forms. I will move it up to a single line. >> init_domheap_pages(start, >> start + >> - PAGE_ALIGN(initial_images[i].mod->mod_end)); >> + PAGE_ALIGN(bi->mods[i].mod->mod_end)); > > One line. Ack. >> } >> bi->nr_modules = 0; >> - initial_images = NULL; >> } >> static void __init init_idle_domain(void) > > With the line fixups: > > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> Thanks! v/r, dps
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 30a139074833..b3b6e6f38622 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -276,8 +276,6 @@ custom_param("acpi", parse_acpi_param); static const char *cmdline_cook(const char *p, const char *loader_name); -static const struct boot_module *__initdata initial_images; - struct boot_info __initdata xen_boot_info; static struct boot_info __init *multiboot_fill_boot_info(unsigned long mbi_p) @@ -336,9 +334,9 @@ unsigned long __init initial_images_nrpages(nodeid_t node) for ( nr = i = 0; i < bi->nr_modules; ++i ) { - unsigned long start = initial_images[i].mod->mod_start; + unsigned long start = bi->mods[i].mod->mod_start; unsigned long end = start + - PFN_UP(initial_images[i].mod->mod_end); + PFN_UP(bi->mods[i].mod->mod_end); if ( end > node_start && node_end > start ) nr += min(node_end, end) - max(node_start, start); @@ -355,15 +353,14 @@ void __init discard_initial_images(void) for ( i = 0; i < bi->nr_modules; ++i ) { uint64_t start = - (uint64_t)initial_images[i].mod->mod_start << PAGE_SHIFT; + (uint64_t)bi->mods[i].mod->mod_start << PAGE_SHIFT; init_domheap_pages(start, start + - PAGE_ALIGN(initial_images[i].mod->mod_end)); + PAGE_ALIGN(bi->mods[i].mod->mod_end)); } bi->nr_modules = 0; - initial_images = NULL; } static void __init init_idle_domain(void) @@ -1383,8 +1380,6 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT); kexec_reserve_area(); - initial_images = bi->mods; - for ( i = 0; !efi_enabled(EFI_LOADER) && i < bi->nr_modules; i++ ) { if ( bi->mods[i].mod->mod_start & (PAGE_SIZE - 1) )
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> --- xen/arch/x86/setup.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)