On 11/7/24 15:47, Jason Andryuk wrote:
On 2024-11-02 13:25, Daniel P. Smith wrote:
Introduce the start and size fields to struct boot_module and
assigns their value during boot_info construction. All uses of
module_t to get
the address and size of a module are replaced with start and size.
The EFI entry point is a special case, as the EFI file loading boot
service may
load a file beyond the 4G barrier. As a result, to make the address
fit in the
32bit integer used by the MB1 module_t structure, the frame number is
stored in
mod_start and size in mod_end. Until the EFI entry point is
enlightened to work
with boot_info and boot_module, multiboot_fill_boot_info will handle the
alternate values in mod_start and mod_end when EFI is detected.
A result of the switch to start/size removes all uses of the mod field in
struct boot_modules, along with the uses of bootstra_map() and
release_module()
functions. With all usage gone, they all are dropped here.
Signed-off-by: Daniel P. Smith <[email protected]>
--
Changes since v7:
- add the start/size change to bootstrap_map_bm()
- convert all BM start/size when introduced, consolidates:
x86/boot: populate boot module for xen entry
x86/boot: transition relocation calculations to struct boot_module
- consolidates all the removal commits
Changes since v6:
- put the efi conversion for mod_start and mod_end back along with check
- dropped unnecessary cast
- updated the population of start and size fields to take into account
efi
Changes since v5:
- switched EFI population of mod_start/mod_end to addresses
a# edit 336ac1fc0019 x86/boot: introduce boot domain
---
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index d9785acf89b6..18b93d6a272a 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -313,13 +313,29 @@ static struct boot_info *__init
multiboot_fill_boot_info(
*/
for ( i = 0; i < MAX_NR_BOOTMODS && i < bi->nr_modules; i++ )
{
- bi->mods[i].mod = &mods[i];
-
bi->mods[i].cmdline_pa = mods[i].string;
+
+ if ( !efi_enabled(EFI_LOADER) )
+ {
+ /*
+ * The EFI loader gives us modules which are already
frame/size.
+ * Switch back to address/size.
+ */
This comment...
+ bi->mods[i].start = mods[i].mod_start;
+ bi->mods[i].size = mods[i].mod_end - mods[i].mod_start;
+ }
+ else
+ {
+ /*
+ * PVH and BIOS loaders give us modules which are start/end.
+ * Switch to address/size.
+ */
And this comment are reversed? But I would just use positive logic:
if ( efi_enabled(EFI_LOADER) )
/* EFI case */
else
/* non-EFI */
Ack.
+ bi->mods[i].start = pfn_to_paddr(mods[i].mod_start);
+ bi->mods[i].size = mods[i].mod_end;
+ }
}
/* Variable 'i' should be one entry past the last module. */
- bi->mods[i].mod = &mods[bi->nr_modules];
bi->mods[i].type = BOOTMOD_XEN;
return bi;
@@ -335,8 +351,8 @@ unsigned long __init
initial_images_nrpages(nodeid_t node)
for ( nr = i = 0; i < bi->nr_modules; ++i )
{
- unsigned long start = bi->mods[i].mod->mod_start;
- unsigned long end = start + PFN_UP(bi->mods[i].mod->mod_end);
+ unsigned long start = bi->mods[i].start;
This should be paddr_to_pfn(bi->mods[i].start)?
Yes, and honestly probably a bad names for the vars as start/end tends
to refer to addresses while pages usually have mfn/pfn in their name.
The question is the churn from variable name not desired.
+ unsigned long end = start + PFN_UP(bi->mods[i].size);
if ( end > node_start && node_end > start )
nr += min(node_end, end) - max(node_start, start);
@@ -1745,13 +1733,11 @@ void asmlinkage __init noreturn __start_xen(void)
for ( i = 0; i < bi->nr_modules; ++i )
{
- const struct boot_module *bm = &bi->mods[i];
+ unsigned long s = bi->mods[i].start, l = bi->mods[i].size;
- set_pdx_range(bm->mod->mod_start,
- bm->mod->mod_start + PFN_UP(bm->mod->mod_end));
- map_pages_to_xen((unsigned long)mfn_to_virt(bm->mod->mod_start),
- _mfn(bm->mod->mod_start),
- PFN_UP(bm->mod->mod_end), PAGE_HYPERVISOR);
+ set_pdx_range(paddr_to_pfn(s), paddr_to_pfn(s) + PFN_UP(l));
This is fine today since s (.start) is checked for page alignment. The
other option would be `paddr_to_pfn(s + l) + 1`, but I'm not sure that
is an improvement.
Out of curiosity, why are you thinking that module start would never be
paged aligned?
You don't have to change anything. Just noting something I noticed.
Ack.
v/r,
dps
