Re: [Xen-devel] [PATCH v2 4/6] x86/boot: Rename trampoline_{start, end} to boot_trampoline_{start, end}
> On 19.08.2019 17:24, David Woodhouse wrote: >> On Mon, 2019-08-12 at 11:55 +0200, Jan Beulich wrote: >>> On 09.08.2019 17:02, David Woodhouse wrote: From: David Woodhouse In preparation for splitting the boot and permanent trampolines from each other. Some of these will change back, but most are boot so do the plain search/replace that way first, then a subsequent patch will extract the permanent trampoline code. >>> >>> To be honest I don't view it as helpful to do things in this order. >>> If you first re-arranged the ordering of items within the trampoline, >>> we'd then not end up with an intermediate state where the labels are >>> misleading. Is there a reason things can't sensibly be done the other >>> way around? >> >> Obviously I did all this in a working tree first, swore at it a lot and >> finally got it working, then attempted to split it up into separate >> meaningful commits which individually made sense. There is plenty of >> room for subjectivity in the choices I made in that last step. >> >> I'm not sure I quite see why you say the labels are misleading. My >> intent was to apply labels based on what each object is *used* for, >> despite the fact that to start with they're all actually in the same >> place. And then to actually move each different type of symbol into its >> separate section/location to clean things up. >> >> Is it just the code comments at the start of trampoline.S that you find >> misleading in the interim stage? Because those *don't* purely talk >> about what bootsym/bootdatasym/trampsym/tramp32sym are used for; they >> do say how they are (eventually) relocated. I suppose I could rip that >> code comment out of patch #3 completely and add it again in a later >> commit... or just just add it again. I write code comments in an >> attempt to be helpful to those who come after me (especially when >> that's actually myself) but if they're going to cause problems, then >> maybe they're more hassle than they're worth? > > No, it's actually the label names: The "boot" that this patch prefixes > to them isn't correct until all post-boot (i.e. AP bringup) parts > have been moved out of the framed block of code. Hm, OK. AFK at this moment but I'll take another look. Basically there wasn't a perfect way to label and move things; either way there were glitches during the transition and my recollection was that I preferred this one because it was purely cosmetic and only lasted for a commit or two. Will see if I can come up with something nicer within the amount of time it is reasonable to spend on such a transitional issue. -- dwmw2 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 4/6] x86/boot: Rename trampoline_{start, end} to boot_trampoline_{start, end}
On 19.08.2019 17:24, David Woodhouse wrote: On Mon, 2019-08-12 at 11:55 +0200, Jan Beulich wrote: On 09.08.2019 17:02, David Woodhouse wrote: From: David Woodhouse In preparation for splitting the boot and permanent trampolines from each other. Some of these will change back, but most are boot so do the plain search/replace that way first, then a subsequent patch will extract the permanent trampoline code. To be honest I don't view it as helpful to do things in this order. If you first re-arranged the ordering of items within the trampoline, we'd then not end up with an intermediate state where the labels are misleading. Is there a reason things can't sensibly be done the other way around? Obviously I did all this in a working tree first, swore at it a lot and finally got it working, then attempted to split it up into separate meaningful commits which individually made sense. There is plenty of room for subjectivity in the choices I made in that last step. I'm not sure I quite see why you say the labels are misleading. My intent was to apply labels based on what each object is *used* for, despite the fact that to start with they're all actually in the same place. And then to actually move each different type of symbol into its separate section/location to clean things up. Is it just the code comments at the start of trampoline.S that you find misleading in the interim stage? Because those *don't* purely talk about what bootsym/bootdatasym/trampsym/tramp32sym are used for; they do say how they are (eventually) relocated. I suppose I could rip that code comment out of patch #3 completely and add it again in a later commit... or just just add it again. I write code comments in an attempt to be helpful to those who come after me (especially when that's actually myself) but if they're going to cause problems, then maybe they're more hassle than they're worth? No, it's actually the label names: The "boot" that this patch prefixes to them isn't correct until all post-boot (i.e. AP bringup) parts have been moved out of the framed block of code. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 4/6] x86/boot: Rename trampoline_{start, end} to boot_trampoline_{start, end}
On Mon, 2019-08-12 at 11:55 +0200, Jan Beulich wrote: > On 09.08.2019 17:02, David Woodhouse wrote: > > From: David Woodhouse > > > > In preparation for splitting the boot and permanent trampolines from > > each other. Some of these will change back, but most are boot so do the > > plain search/replace that way first, then a subsequent patch will extract > > the permanent trampoline code. > > To be honest I don't view it as helpful to do things in this order. > If you first re-arranged the ordering of items within the trampoline, > we'd then not end up with an intermediate state where the labels are > misleading. Is there a reason things can't sensibly be done the other > way around? Obviously I did all this in a working tree first, swore at it a lot and finally got it working, then attempted to split it up into separate meaningful commits which individually made sense. There is plenty of room for subjectivity in the choices I made in that last step. I'm not sure I quite see why you say the labels are misleading. My intent was to apply labels based on what each object is *used* for, despite the fact that to start with they're all actually in the same place. And then to actually move each different type of symbol into its separate section/location to clean things up. Is it just the code comments at the start of trampoline.S that you find misleading in the interim stage? Because those *don't* purely talk about what bootsym/bootdatasym/trampsym/tramp32sym are used for; they do say how they are (eventually) relocated. I suppose I could rip that code comment out of patch #3 completely and add it again in a later commit... or just just add it again. I write code comments in an attempt to be helpful to those who come after me (especially when that's actually myself) but if they're going to cause problems, then maybe they're more hassle than they're worth? smime.p7s Description: S/MIME cryptographic signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 4/6] x86/boot: Rename trampoline_{start, end} to boot_trampoline_{start, end}
On 09.08.2019 17:02, David Woodhouse wrote: From: David Woodhouse In preparation for splitting the boot and permanent trampolines from each other. Some of these will change back, but most are boot so do the plain search/replace that way first, then a subsequent patch will extract the permanent trampoline code. To be honest I don't view it as helpful to do things in this order. If you first re-arranged the ordering of items within the trampoline, we'd then not end up with an intermediate state where the labels are misleading. Is there a reason things can't sensibly be done the other way around? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
