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

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

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

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@cloud.com>
> >>>
> >>> Generally,
> >>> Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> >>
> >> 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.
> 
> > 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.

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

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