On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index eee20bb1753c..dd94ee2e736b 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1034,9 +1044,10 @@ void asmlinkage __init noreturn __start_xen(unsigned
> long mbi_p)
> mod = __va(mbi->mods_addr);
> }
>
> + multiboot_to_bootinfo(mbi);
Actually, peeking ahead to the end of the series, we've got this:
void __start_xen(unsigned long mbi_p)
{
...
multiboot_info_t *mbi;
module_t *mod;
...
if ( pvh_boot )
{
ASSERT(mbi_p == 0);
pvh_init(&mbi, &mod);
}
else
{
mbi = __va(mbi_p);
mod = __va(mbi->mods_addr);
}
multiboot_to_bootinfo(mbi, mod);
which are the sum total of the mbi and mod pointers. Worse, pvh_init()
is transforming the PVH into into MB1 info, just to be transformed
immediately to BI.
I expect this is work for the end of the series (I can't think of a nice
way to disentangle it earlier), but could we end up with something more
like:
if ( pvh_boot )
{
ASSERT(mbi_p == 0);
pvh_fill_boot_info();
}
else
{
multiboot_info_t *mbi = __va(mbi_p);
multiboot_fill_boot_info(mbi, __va(mbi->mods_addr));
}
?
Or perhaps even just pass mbi_p in, and have multiboot_fill_boot_info()
do the __va()'s itself.
If so, we probably want to make a naming and possibly prototype
difference in this patch.
~Andrew
