[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 7/8] x86/EFI: keep debug info in xen.efi


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 22 Apr 2021 17:53:55 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=XFk+Sehe0HA3BISWSwKE02bFaKvvBPZobt+rvXku52c=; b=go/aYicpsqWF4TflM5/c07TzKFs1SADbUj0ux03lPsJl4SGbF/TXbsbbdI+mNL3qO8VW5M18QTJ2B1wy4ml7bCNq8OsGS1WJQ6f9KFxmAkz9oZll1dKEOyIbb+Qb/oiF8ZTvq5IpZsq29yxKP3eZAGYpZz0iWCXLpp34hSNGb36bqwyZpNMw7hXbNIyiixGquTS9Gko993S7EqeSZGakGcI0o03yj2RC+ILfImQOv/6yEa5g9LNEDyD8L4wFp79I1kXyCUeB5ZhMFSZYTykuqoSPMsxBJmK7vxefGalOtZ+TDqLPCw28RqJyXGpAIaBC4/xohdnNafXSPc5nRavR3Q==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MQt1Q6EgpH7eVmPZEzVbLI16dtqN5iVo8qFqUygwBKhuPkS0imUDq+5KYvgDl3+wv/q3WD1cvDlyvJoI/R4VvMCJjJ0EBN2uLYuR8OOVbDpSz/0tU58RMqSv5pXq/aFnqIYNG2hIUssJPfYAcEw2NzcRvkOBPc3RDveTqZMFdinyKvmZojOj8Zn7WSGOtR7F9hdzDt08Y7LtjO3zuaERzDVbM7ITLZFn9mMNetxW+hVq24wqEvi5/92QCkMXbEuTw5/9S3IVxPWLQ6vMD4jGmxsOuoQ2PsrfCdl2AUZ4UE5Rp28Uv+8uXcknDzWjEvKhp1o8RDtD0Bce2AERQXZkjg==
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Thu, 22 Apr 2021 15:54:18 +0000
  • Ironport-hdrordr: A9a23:Td/qiKy7EGPaSPHYMlCHKrPxp+4kLtp033Aq2lEZdDV8Sebdv9 yynfgdyB//gCsQXnZlotybJKycWxrnm6JdybI6eZOvRhPvtmftFoFt6oP+3ybtcheQysd07o 0lSaR3DbTLYWRSpdrm4QW+DtYryMSG9qftvuvF03JxV2hRC51IxS0RMHf+LmRdQg5aCZ0lUL +V4cRarzStEE5nEPiTLH8DQuTFupn3j5rgexELHFoK7wOJgDOu5tfBYmel9z0ZVC5CxqpnzH jdn2XCl9memtyY6juZ7W/c6JxKhMDso+EjOOWggtUYQw+c8TqAS59mX9S5zVYIicGprG0nid zd5yonVv4Dlk/5WkGQjV/T1xL70DAogkWSu2OwpXf4u8T2SHYbJqN69PpkWyDU4UYho91wuZ gjtwny2us1fHGw6BjV3NTGWwpnkUC5uxMZ4IkupkdSTJcEb/tppZEflXklY6soJj7w64wsDY BVfbjhzctRGGnqCkzxgnNi25iFUHg1A369MzI/k/3Q+T1XkHdl9lAf1cwSk1wRnahNO6Vs1q DqNL9lm6pJSdJTRaVhBP0ZSc/yMWDVRwnQWVjibmjPJeUiATbgupT36LI66KWDf4EJ9oI7nN DkXElDvWA/VkryAaS1rdN22yGIZF/4cSXmy8lY6ZQ8kKb7XqDXPSqKT01ru9e8ot0Ea/erGM qbCdZzObvOPGHuEYFG00nVQJ9JM0QTV8UTp5ISR0+OmMTWMYfn39arMMr7Ff7IK3IJS2n/Cn wMUHzYP8Nb9H2mXXf+nVz/QHXoVkvj/Y9hMaTT8uQJobJ9c7Fkg0wwsxCU98uLITpNvugdZ0 1lOo7qlau9uC2X8A/zniFUEysYKnwQzKTrUntMqwNPGVjza6w/t9KWfn0X+HOGIxR4Xv7HCQ I3nSUxxYuHa7irgQwyAdOuNWyXy1EJomiRcpsakqqfoeDoZ40/FZRjfKBqDw3EG1hUlG9R2S Z+QT5BYnWaOiLliK2jgpBRLvrYbcNAjACiJtMRj2neu0WarcQGXWAaQDaqbM6SjW8VNnhpr2 w015VarKuLmD6pJ2d6qv8/KkdwZGOeB68DMB6If7xOmrfgeBh5SECDgTDysWB0RkPas2Epwk DxJyydfv/GRn5QoGpR3KrR/FRoTWmFZE5rZndmsYpyKHTeth9IoJq2T5v291HURkoJw+kbPj 2AWzcULw907/2c1RKeml+5ZD8b76RrGtaYIKUocrnV1H/oFZaBkrseGeRIuLx/Msr1j+MNWe WDWgOcIT/iEdk10wiNqntNAlgslFAU1dfTnDvr42iz0CRhXb78IFF6S6oaJN/ZxW7+XPqM2I h4i9VwnebYCBSGVve2jYXsKxhEIVfvhETzaccCg5Vdp7gzu7t+BIOza0qC6Fh3mDEFaP7pn0 YfSplh6L/POoVTb9UfEhgpiWYBpZCqFg8XqQT4De81QEE1g1LaN92P5aDUqbBHODzJmCLAfX 2e+TZa5fHLQm+q0qMbEbs5JQ1tGQUBwUUn2OOJbIvLDgq2M8lF4VqhK3e4NJtQUrKMF7lVjh F05biz7qOqXhu9/ADbpj1gJK1St06hXMOpGQqJXddyzObSAyXFvoKapOipjDn2TjOna0MXwa 19HHZgH/hru30Fl4040i+7V6rthFkq+mEuuQ1aqg==
  • Ironport-sdr: dx5rzOVbQp0e8dKhJT14ArpqKO0xFCilmTG9aGxQXJwR89yhYk7bowa6KHM3GVGDgQb1Ux65dn Y6S6EKr1dfEQVWNqztd6ZBZE21qHFFvSAMvfrjFQvR8k8koj14TB7mnDfjilExb4/MYInn8IpW bEmNB59RKmVHbbUExETLoMu8ElAXytfA0XWvOV0AdVoHF2ODxOEnIHInUXJewqwjUIam3R1t57 ajnfw2Nl68dE0FTLtGkj7vR7vHoYIwhdXHlWQTkQtkdoIo49f2Xi+W8+4wbFPsl5jFU0DgI+ST +Wg=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Apr 22, 2021 at 05:46:28PM +0200, Jan Beulich wrote:
> On 22.04.2021 16:56, Roger Pau Monné wrote:
> > On Thu, Apr 22, 2021 at 01:03:13PM +0200, Jan Beulich wrote:
> >> On 22.04.2021 10:14, Roger Pau Monné wrote:
> >>> On Wed, Apr 21, 2021 at 05:38:42PM +0200, Jan Beulich wrote:
> >>>> On 21.04.2021 17:30, Roger Pau Monné wrote:
> >>>>> On Wed, Apr 21, 2021 at 03:06:36PM +0200, Jan Beulich wrote:
> >>>>>> On 21.04.2021 13:15, Roger Pau Monné wrote:
> >>>>>>> On Thu, Apr 01, 2021 at 11:47:03AM +0200, Jan Beulich wrote:
> >>>>>>>> --- a/xen/arch/x86/xen.lds.S
> >>>>>>>> +++ b/xen/arch/x86/xen.lds.S
> >>>>>>>> @@ -312,10 +312,60 @@ SECTIONS
> >>>>>>>>      *(.reloc)
> >>>>>>>>      __base_relocs_end = .;
> >>>>>>>>    }
> >>>>>>>> -  /* Trick the linker into setting the image size to exactly 16Mb. 
> >>>>>>>> */
> >>>>>>>> -  . = ALIGN(__section_alignment__);
> >>>>>>>> -  DECL_SECTION(.pad) {
> >>>>>>>> -    . = ALIGN(MB(16));
> >>>>>>>> +  .debug_abbrev ALIGN(1) (NOLOAD) : {
> >>>>>>>> +     *(.debug_abbrev)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_info ALIGN(1) (NOLOAD) : {
> >>>>>>>> +    *(.debug_info)
> >>>>>>>> +    *(.gnu.linkonce.wi.*)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_types ALIGN(1) (NOLOAD) : {
> >>>>>>>> +    *(.debug_types)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_str ALIGN(1) (NOLOAD) : {
> >>>>>>>> +    *(.debug_str)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_line ALIGN(1) (NOLOAD) : {
> >>>>>>>> +    *(.debug_line)
> >>>>>>>> +    *(.debug_line.*)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_line_str ALIGN(1) (NOLOAD) : {
> >>>>>>>> +    *(.debug_line_str)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_names ALIGN(4) (NOLOAD) : {
> >>>>>>>> +    *(.debug_names)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_frame ALIGN(4) (NOLOAD) : {
> >>>>>>>> +    *(.debug_frame)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_loc ALIGN(1) (NOLOAD) : {
> >>>>>>>> +    *(.debug_loc)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_loclists ALIGN(4) (NOLOAD) : {
> >>>>>>>> +    *(.debug_loclists)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_ranges ALIGN(8) (NOLOAD) : {
> >>>>>>>> +    *(.debug_ranges)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_rnglists ALIGN(4) (NOLOAD) : {
> >>>>>>>> +    *(.debug_rnglists)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_addr ALIGN(8) (NOLOAD) : {
> >>>>>>>> +    *(.debug_addr)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_aranges ALIGN(1) (NOLOAD) : {
> >>>>>>>> +    *(.debug_aranges)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_pubnames ALIGN(1) (NOLOAD) : {
> >>>>>>>> +    *(.debug_pubnames)
> >>>>>>>> +  }
> >>>>>>>> +  .debug_pubtypes ALIGN(1) (NOLOAD) : {
> >>>>>>>> +    *(.debug_pubtypes)
> >>>>>>>> +  }
> >>>>>>>> +  /* Trick the linker into setting the image size to no less than 
> >>>>>>>> 16Mb. */
> >>>>>>>> +  __image_end__ = .;
> >>>>>>>> +  .pad ALIGN(__section_alignment__) : {
> >>>>>>>> +    . = __image_end__ < __image_base__ + MB(16) ? ALIGN(MB(16)) : .;
> >>>>>>>
> >>>>>>> I think this is inside an ifdef EFI region, since this is DWARF info
> >>>>>>> couldn't it also be present when building with EFI disabled?
> >>>>>>
> >>>>>> Of course (and it's not just "could" but "will"), yet the linker will
> >>>>>> do fine (and perhaps even better) without when building ELF. Also
> >>>>>> note that we'll be responsible for keeping the list of sections up-to-
> >>>>>> date. The linker will recognize Dwarf sections by looking for a
> >>>>>> .debug_ prefix. We can't use such here (or at least I'm not aware of
> >>>>>> a suitable mechanism); .debug_* would mean munging together all the
> >>>>>> different kinds of Dwarf sections. Hence by limiting the explicit
> >>>>>> enumeration to PE, I'm trying to avoid anomalies in ELF down the road.
> >>>>>
> >>>>> Right, so we will have to keep this list of debug_ sections updated
> >>>>> manually if/when more of those appear as part of DWARF updates?
> >>>>
> >>>> Yes.
> >>>>
> >>>>> Do we have a way to get some kind of warning or error when a new
> >>>>> section not explicitly handled here appears?
> >>>>
> >>>> ld 2.37 will start warning about such sections, as they'd land at
> >>>> VA 0 and hence below image base.
> >>>
> >>> That seems like a bug in ld?
> >>>
> >>> The '--image-base' option description mentions: "This is the lowest
> >>> memory location that will be used when your program or dll is
> >>> loaded.", so I would expect that if the option is used the default VA
> >>> should be >= image-base, or else the description of the option is not
> >>> consistent, as ld will still place sections at addresses below
> >>> image-base.
> >>
> >> ld's "general" logic is pretty ELF-centric. Hence debugging sections
> >> get placed at VA 0 by default, not matter what the (PE-specific)
> >> --image-base says. Whether that's a bug though I'm not sure: There
> >> are no really good alternatives that could be used by default. Doing
> >> what we do here (and what e.g. Cygwin does) via linker script may not
> >> be appropriate in the common case. In particular it is not generally
> >> correct for debug info to be part of what gets loaded into memory.
> > 
> > So with this change here you placate the warnings from newer ld about
> > having a VA < image base,
> 
> It's not just about silencing the warnings. The resulting image is
> unusable when the sections don't get placed at a suitable VA.

And this wasn't an issue before because the linker won't even attempt
to place DWARF sections into a PE output.

> > but the end result is that now the debug
> > sections will also get loaded when booted from the EFI loader?
> > (because the NOLOAD doesn't have any effect with PE)
> 
> Yes. I currently see no other way to retain debug info in xen.efi.
> But to be clear, the memory debug info occupies isn't lost - we
> still free space from _end (or alike) onwards. .reloc, for example,
> also lives there. And I was wondering whether we shouldn't keep
> .comment this way as well.

Yes, I already realized all this is past _end.

I wonder however if the use of (NOLOAD) makes all this more confusing,
such sections should only be present on the linker script used for the
PE output, and then the (NOLOAD) doesn't make sense there?

If so, I think the (NOLOAD) directive should be dropped, and a comment
noting that the debug sections need to be manually added to the PE
output in order to avoid them being placed at VA 0 would be helpful
IMO, likely also mentioning that they would be loaded but discarded
afterwards by Xen because they are all past _end.

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.