Re: [PATCH v2] xen: Strip xen.efi by default

[Frediano Ziglio <freddy77@xxxxxxxxx>]

On Thu, Oct 9, 2025 at 12:56 PM Marek Marczykowski-Górecki
<marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Oct 07, 2025 at 04:46:17PM +0200, Jan Beulich wrote:
> > On 07.10.2025 16:23, Marek Marczykowski-Górecki wrote:
> > > On Tue, Oct 07, 2025 at 04:12:13PM +0200, Jan Beulich wrote:
> > >> On 02.10.2025 16:10, Marek Marczykowski-Górecki wrote:
> > >>> On Thu, Oct 02, 2025 at 02:05:56PM +0100, Andrew Cooper wrote:
> > >>>> On 12/06/2025 11:07 am, Frediano Ziglio wrote:
> > >>>>> For xen.gz file we strip all symbols and have an additional
> > >>>>> xen-syms file version with all symbols.
> > >>>>> Make xen.efi more coherent stripping all symbols too.
> > >>>>> xen.efi.elf can be used for debugging.
> > >>>>>
> > >>>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> > >>>
> > >>> Generally,
> > >>> Reviewed-by: Marek Marczykowski-Górecki 
> > >>> <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > >>
> > >> Just to double check: You offer this after having read (and discarded) my
> > >> comments on v1, which v2 left largely unaddressed?
> > >
> > > You mean the one about objcopy result used for debugging? I didn't see
> > > that before, since I wasn't in cc on v1...
> > >
> > > Anyway, are you aware of some specific objcopy issue. Or in other words:
> > > would xen.efi.elf _currently_ be broken (as in - unusable for
> > > debugging/disassembly)?
> >
> > I can't tell. I've seen fair parts of the code in the course of addressing
> > various issues, and I would be very surprised if all of that was working
> > correctly.
> >

Yes, sorry about not replying to this part. At the time I was testing
the various usages we do with that file before replying. Beside
debugging we use it for automatic crash dump analysis and live
patching. Unfortunately live patching was not working for reasons not
bound to this change and it tooks a while to fix it. Once fixed live
patching all our use cases of the ELF-translated file are working
perfectly confirming that the file works correctly.

> > > If not, then I take that relevant part of your
> > > objection is mostly about inconsistent naming (xen.gz -> xen-syms, vs
> > > xen.efi -> xen.efi.elf). Would xen-syms.efi.elf be better?
> >
> > Plus the one asking to strip only debug info, but not the symbol table.
> > (And no, none of the suggested names look really nice to me.)
> >
> > Plus the one indicating that the change better wouldn't be made in the
> > first place. As said, to deal with size issues we already have machinery
> > in place. Not very nice machinery, but it's apparently functioning.
>
> I'm of the opinion that defaults matter. Just having ability to build a
> binary that works on more systems is not sufficient, if you'd need to
> spend a day (or more...) on debugging obscure error message to figure
> out which hidden option to use to get there. And while one could argue
> that CONFIG_DEBUG=y builds are only for people familiar with details to
> deal with such issues, IMO just CONFIG_DEBUG_INFO=y shouldn't need
> arcane knowledge to get it working... And since that's a common option
> to enable in distribution packages, person hitting the issue might not
> even be the one doing the build (and thus controlling the build
> options).
>
> As for the details how to get there, I'm more flexible. Based on earlier
> comments, it seems that (not stripped) xen.efi isn't very useful for
> debugging directly, an ELF version of it is. So IMO it makes sense to
> have the debug binary already converted. But if you say you have use for
> xen.efi with all debug info too, I'm okay with keeping it too, maybe as
> xen-syms.efi. It's a bit of more space (to have both efi and elf version
> with debug info), but since it doesn't apply to the installed version,
> only the one kept in the build directory, not a big issue IMO.
>

Frediano