Re: [PATCH v7 0/2] x86/boot: Reduce assembly code

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Oct 02, 2024 at 04:27:19PM +0100, Frediano Ziglio wrote:
> On Wed, Oct 2, 2024 at 3:04 PM Marek Marczykowski-Górecki
> <marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Tue, Oct 01, 2024 at 11:22:37AM +0100, Frediano Ziglio wrote:
> > > This series came from part of the work of removing duplications between
> > > boot code and rewriting part of code from assembly to C.
> > > Rewrites EFI code in pure C.
> >
> > The MB2+EFI tests on Adler Lake fail with this series:
> > https://gitlab.com/xen-project/people/marmarek/xen/-/pipelines/1478766782
> > Looking at the VGA output (unfortunately not collected by the test
> > itself) it hangs just after bootloader, before printing anything on the
> > screen (or even clearing it after bootloader). The serial is silent too.
> >
> 
> I tried multiple times to take a look at the logs, but I keep getting error 
> 500.

500? That's weird. Anyway, serial log is empty, so you haven't lost
much.
But also, I've ran it a couple more times and it is some regression.
Staging reliably passes while staging+this series fails.

Unfortunately I don't have any more info besides it hangs before
printing anything on serial or VGA. Maybe it hanging only on Intel but
not AMD is some hint? Or maybe it's some memory layout or firmware
differences that matter here (bootloader is exactly the same)?
FWIW some links:
successful staging run on ADL: 
https://gitlab.com/xen-project/people/marmarek/xen/-/jobs/7980146338
failed staging+this run on ADL: 
https://gitlab.com/xen-project/people/marmarek/xen/-/jobs/7980330394
successful staging run on Zen3+: 
https://gitlab.com/xen-project/people/marmarek/xen/-/jobs/7980146359
successful staging+this run on Zen3+: 
https://gitlab.com/xen-project/people/marmarek/xen/-/jobs/7980146359

> > It does pass on Zen 3+ runners.
> >
> > Since there were some issues with the ADL runner today on plain staging,
> > I'm not 100% sure if it isn't some infrastructure issue yet. But the
> > symptoms look different than usual infra issues (and different than
> > todays failures on staging), so I think it's more likely an issue with
> > the patches here.
> >
> > > Changes since v1, more details in specific commits:
> > > - style updates;
> > > - comments and descriptions improvements;
> > > - other improvements.
> > >
> > > Changes since v2:
> > > - rebased on master, resolved conflicts;
> > > - add comment on trampoline section.
> > >
> > > Changes since v3:
> > > - changed new function name;
> > > - declare efi_multiboot2 in a separate header;
> > > - distinguish entry point from using magic number;
> > > - other minor changes (see commens in commits).
> > >
> > > Changes since v4:
> > > - rebase on staging;
> > > - set %fs and %gs as other segment registers;
> > > - style and other changes.
> > >
> > > Changes since v5:
> > > - fixed a typo.
> > >
> > > Changes since v6:
> > > - remove merged patch;
> > > - comment and style;
> > > - change some pointer checks to avoid overflows;
> > > - rename parse-mbi2.c to mbi2.c.
> > >
> > > Frediano Ziglio (2):
> > >   x86/boot: Rewrite EFI/MBI2 code partly in C
> > >   x86/boot: Improve MBI2 structure check
> > >
> > >  xen/arch/x86/boot/head.S       | 146 +++++++--------------------------
> > >  xen/arch/x86/efi/Makefile      |   1 +
> > >  xen/arch/x86/efi/efi-boot.h    |   7 +-
> > >  xen/arch/x86/efi/mbi2.c        |  66 +++++++++++++++
> > >  xen/arch/x86/efi/stub.c        |  10 +--
> > >  xen/arch/x86/include/asm/efi.h |  18 ++++
> > >  6 files changed, 123 insertions(+), 125 deletions(-)
> > >  create mode 100644 xen/arch/x86/efi/mbi2.c
> > >  create mode 100644 xen/arch/x86/include/asm/efi.h
> > >
> 
> Frediano

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature