| Message ID | 20241019182054.3842879-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/boot: Fix PVH boot following the start of the MBI->BI conversion | expand |
On 10/19/24 14:20, Andrew Cooper wrote: > pvh_init() sets up the mbi pointer, but leaves mbi_p at 0. This isn't > compatbile with multiboot_fill_boot_info() starting from the physical address, > in order to remove the use of the mbi pointer. > > Fixes: 038826b61e85 ("x86/boot: move x86 boot module counting into a new boot_info struct") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Daniel P. Smith <dpsmith@apertussolutions.com> > > This is a testiment to how tangled the boot code really is. > --- > xen/arch/x86/setup.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index 6746ed8cced6..bfede5064e8c 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -1048,6 +1048,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) > { > ASSERT(mbi_p == 0); > pvh_init(&mbi, &mod); > + mbi_p = __pa(mbi); > } > else > { > > base-commit: e9f227685e7204cb2293576ee5b745b828cb3cd7 Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
On 19/10/2024 7:23 pm, Daniel P. Smith wrote: > On 10/19/24 14:20, Andrew Cooper wrote: >> pvh_init() sets up the mbi pointer, but leaves mbi_p at 0. This isn't >> compatbile with multiboot_fill_boot_info() starting from the physical >> address, >> in order to remove the use of the mbi pointer. >> >> Fixes: 038826b61e85 ("x86/boot: move x86 boot module counting into a >> new boot_info struct") >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> CC: Daniel P. Smith <dpsmith@apertussolutions.com> >> >> This is a testiment to how tangled the boot code really is. >> --- >> xen/arch/x86/setup.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c >> index 6746ed8cced6..bfede5064e8c 100644 >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -1048,6 +1048,7 @@ void asmlinkage __init noreturn >> __start_xen(unsigned long mbi_p) >> { >> ASSERT(mbi_p == 0); >> pvh_init(&mbi, &mod); >> + mbi_p = __pa(mbi); >> } >> else >> { >> >> base-commit: e9f227685e7204cb2293576ee5b745b828cb3cd7 > > Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com> Thanks. I'll get this committed right away so OSSTest can start working on untangling itself. ~Andrew
On Sat, Oct 19, 2024 at 07:20:54PM +0100, Andrew Cooper wrote: > pvh_init() sets up the mbi pointer, but leaves mbi_p at 0. This isn't > compatbile with multiboot_fill_boot_info() starting from the physical address, > in order to remove the use of the mbi pointer. > > Fixes: 038826b61e85 ("x86/boot: move x86 boot module counting into a new boot_info struct") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Daniel P. Smith <dpsmith@apertussolutions.com> > > This is a testiment to how tangled the boot code really is. Did it causes crash in some boot configuration? If so, did some test tripped on this (from what I see, not a gitlab one)? > --- > xen/arch/x86/setup.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index 6746ed8cced6..bfede5064e8c 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -1048,6 +1048,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) > { > ASSERT(mbi_p == 0); > pvh_init(&mbi, &mod); > + mbi_p = __pa(mbi); > } > else > { > > base-commit: e9f227685e7204cb2293576ee5b745b828cb3cd7 > -- > 2.39.5 > >
On 21/10/2024 2:54 pm, Marek Marczykowski-Górecki wrote: > On Sat, Oct 19, 2024 at 07:20:54PM +0100, Andrew Cooper wrote: >> pvh_init() sets up the mbi pointer, but leaves mbi_p at 0. This isn't >> compatbile with multiboot_fill_boot_info() starting from the physical address, >> in order to remove the use of the mbi pointer. >> >> Fixes: 038826b61e85 ("x86/boot: move x86 boot module counting into a new boot_info struct") >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> CC: Daniel P. Smith <dpsmith@apertussolutions.com> >> >> This is a testiment to how tangled the boot code really is. > Did it causes crash in some boot configuration? If so, did some test > tripped on this (from what I see, not a gitlab one)? OSSTest found it. E1t2D0w-0005rF-45@osstest.test-lab.xenproject.org I'm writing a PVSHIM test for Gitlab CI right now, seeing as OSSTest is going away imminently. https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1505384944 ~Andrew
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 6746ed8cced6..bfede5064e8c 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1048,6 +1048,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) { ASSERT(mbi_p == 0); pvh_init(&mbi, &mod); + mbi_p = __pa(mbi); } else {
pvh_init() sets up the mbi pointer, but leaves mbi_p at 0. This isn't compatbile with multiboot_fill_boot_info() starting from the physical address, in order to remove the use of the mbi pointer. Fixes: 038826b61e85 ("x86/boot: move x86 boot module counting into a new boot_info struct") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Daniel P. Smith <dpsmith@apertussolutions.com> This is a testiment to how tangled the boot code really is. --- xen/arch/x86/setup.c | 1 + 1 file changed, 1 insertion(+) base-commit: e9f227685e7204cb2293576ee5b745b828cb3cd7