Re: [PATCH for-4.22] xen/x86: Always strip xen.efi

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1: Type: text/plain, Size: 2723 bytes --]

On Tue, Jun 09, 2026 at 05:56:10PM +0100, Andrew Cooper wrote:
> On 08/06/2026 9:01 pm, Marek Marczykowski-Górecki wrote:
> > On Mon, Jun 08, 2026 at 06:31:08PM +0100, Andrew Cooper wrote:
> >> From: Frediano Ziglio <frediano.ziglio@citrix.com>
> >>
> >> xen.efi with debugging symbols is ~45MB, down to ~9.3MB when stripped.
> >> Multiple firmwares (as seen by QubesOS, Trenchboot, and XenServer) are unable
> >> to boot xen.efi when debugging symbols are included.
> >>
> >> Either way, having debug symbols by default is abnormal and contrary to how
> >> the non-EFI path works.
> >>
> >> Produce xen-syms.efi unconditionally, just like xen-syms.  If
> >> CONFIG_DEBUG_INFO is enabled, these will contain debug symbols, and if not,
> >> then not.  When xen-syms is processed by mkelf32, the debug symbols are simply
> >> discarded.  For xen-syms.efi, call $(STRIP) to produce xen.efi.
> >>
> >> Some old versions of binutils ld managed to produce efi files which the
> >> matching version of strip couldn't process.  This includes Binutils 2.26
> >> included in Ubuntu 16.04.  Delete the workaround for this bug, and require a
> >> less broken toolchain.
> > While I see Ubuntu 16.04 dropped, how is the "require a less broken
> > toolchain" addressed? By implicitly disabling xen.efi build on broken
> > toolchain? Maybe README should have a note about needing newer Binutils
> > for xen.efi? Currently it says just Binutils 2.25. There is a section
> > about optional build deps, maybe add there something like "GNU Binutils
> > X.Y (required for building xen.efi)", if the version is known, or at
> > least "GNU Binutils capable of producing non-broken PE files (required
> > for building xen.efi)" if the version is not known.
> 
> xen.efi has never had any relation to the README minimum toolchain version.
> 
> It has always probed the toolchain, and silently turned itself off it
> doesn't like the result.  In this case, we drop one of the "lets work
> around this bug different" checks which ends up excluding the problem
> revision.

Ok, in that case

Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> If you prefer, I could re-split the patch, and state on the first patch
> that it's a prerequisite to be able to use $(STRIP) in the second patch ?
> 
> binutils' PE+ support is horribly buggy and Xen is the only user in this
> area.  At some point, 2.46 (practically bleeding edge) is going to be
> required, seeing as it's the first version of bintuils where we don't
> need to hexedit the PE+ header in order to satisfy the signing process.
> 
> ~Andrew

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]