| Message ID | 20200907190027.669086-2-hudson@trmm.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi: Unified Xen hypervisor/kernel/initrd images | expand |
On 07.09.2020 21:00, Trammell Hudson wrote: > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -156,6 +156,7 @@ SECTIONS > __note_gnu_build_id_end = .; > } :note :text > #elif defined(BUILD_ID_EFI) > + . = ALIGN(32); /* workaround binutils section overlap bug */ > DECL_SECTION(.buildid) { > __note_gnu_build_id_start = .; > *(.buildid) It being "just" 32 bytes may make it look as if we could take this without much thinking, but I'm then struggling where we would draw the boundary. The binutils bug having got fixed (or at least worked around), I don't really like this getting applied uniformly, the more that nothing would normally have the requirement you have (to be able to objcopy the whole thing). Personally I think this kind of a workaround patch is something distros ought to be fine to carry, if they care about the functionality and only until they get around to upgrade their binutils. But I'll be happy to hear differing opinions. I also don't see any mention anywhere of why it's 32 bytes, and not 16 or 64 or yet something else. Finally, please Cc maintainers on patch submissions. Jan
On Tuesday, September 8, 2020 11:04 AM, Jan Beulich <jbeulich@suse.com> wrote: > [...] > Personally I think this kind of a workaround patch is something > distros ought to be fine to carry, if they care about the > functionality and only until they get around to upgrade their > binutils. But I'll be happy to hear differing opinions. Y'all just merged something to support building with make 3.81, released in *2006*, so why require a bleeding edge binutils to work with the executable image? > I also don't see any mention anywhere of why it's 32 bytes, and not > 16 or 64 or yet something else. It is 32 because you said 32 was probably fine. -- Trammell
On 08.09.2020 11:30, Trammell Hudson wrote: > On Tuesday, September 8, 2020 11:04 AM, Jan Beulich <jbeulich@suse.com> wrote: >> [...] >> Personally I think this kind of a workaround patch is something >> distros ought to be fine to carry, if they care about the >> functionality and only until they get around to upgrade their >> binutils. But I'll be happy to hear differing opinions. > > Y'all just merged something to support building with make 3.81, > released in *2006*, so why require a bleeding edge binutils to > work with the executable image? Building Xen has to work on the tool chain versions we document it works on (and we're in the process of discussing to raise the base line). Playing with the output of our build system is an entirely different thing. As with, I think, the majority of new features, distros would pick up your new functionality mainly for use in new versions, and hence would likely run with new binutils anyway by that time. >> I also don't see any mention anywhere of why it's 32 bytes, and not >> 16 or 64 or yet something else. > > It is 32 because you said 32 was probably fine. Well, that's then setting us up for running into the same issue again in case this "probably" turns out wrong. Referring to the size of the structure created by binutils to insert the build ID, otoh, could maybe give a proper reason. However, there's also the question whether this (not) functioning also depends on the particular size and/or alignment of the preceding section. Iirc this was the reason why you had thought there was a connection to live patching enabled / disabled in the build. Jan
On Tuesday, September 8, 2020 8:29 AM, Jan Beulich <jbeulich@suse.com> wrote: > [...] As with, I think, the majority of new > features, distros would pick up your new functionality mainly for > use in new versions, and hence would likely run with new binutils > anyway by that time. It also occurs to me that the binutils used to process the xen.efi image does not need to be the same as the one used to generate it, so there are no build-time dependencies on having this fix in place. Dropping this patch from the series doesn't affect the ability of a distribution with a new binutils from being able to build unified images, so I'm fine with abandoning it. Are there any further thoughts on the other parts of the series? -- Trammell
On 14.09.2020 11:14, Trammell Hudson wrote: > On Tuesday, September 8, 2020 8:29 AM, Jan Beulich <jbeulich@suse.com> wrote: >> [...] As with, I think, the majority of new >> features, distros would pick up your new functionality mainly for >> use in new versions, and hence would likely run with new binutils >> anyway by that time. > > It also occurs to me that the binutils used to process the xen.efi > image does not need to be the same as the one used to generate it, > so there are no build-time dependencies on having this fix in place. > > Dropping this patch from the series doesn't affect the ability of a > distribution with a new binutils from being able to build unified > images, so I'm fine with abandoning it. > > Are there any further thoughts on the other parts of the series? It's on my list of things to look at, yes. I'm sorry it's taking some time to get there. Jan
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index 0273f79152..ba691b1890 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -156,6 +156,7 @@ SECTIONS __note_gnu_build_id_end = .; } :note :text #elif defined(BUILD_ID_EFI) + . = ALIGN(32); /* workaround binutils section overlap bug */ DECL_SECTION(.buildid) { __note_gnu_build_id_start = .; *(.buildid)