Re: [Xen-devel] [RFC PATCH 2/7] x86/boot: Remove gratuitous call back into low-memory code

["David Woodhouse" <dwmw2@xxxxxxxxxxxxx>]

> On 02/05/2019 09:14, Jan Beulich wrote:
>>>>> On 01.05.19 at 13:17, <dwmw2@xxxxxxxxxxxxx> wrote:
>>> We appear to have implemented a memcpy() in the low-memory trampoline
>>> which we then call into from __start_xen(), for no adequately defined
>>> reason.
>> May I suggest that in cases like this you look at the commit
>> introducing the function? It supplies a reason:
>>
>> "Add a new raw e820 table for common purpose and copy the BIOS buffer
>>  to it. Doing the copying in assembly avoids the need to export the
>>  symbols for the BIOS E820 buffer and number of entries."
>
> I completely agree with David here.  That description is completely
> insufficient for justifying why the logic was done that way in the end,
> and I would not have accepted the patch in that form at all.
>
> To be clear.  I understand (and agree) why having our main copy of the
> e820 table in the trampoline is a bad thing, and moving the main copy
> out of the trampoline is fine.
>
> What isn't fine is why, in the adjusted world, we call back into what
> appears to be the trampoline, but isn't, to get access to data which the
> calling code can already access.  In particular...
>
>> I have to admit that I see value in this, as it avoids introduction
>> of wrong accesses to these variables.
>
> ...this reasoning is bogus.  We're either accessing the data itself, or
> the memcpy function, but there is no possible way to programatically
> avoid "wrong" access into the trampoline, because we're still accessing
> it.
>
> Therefore, I don't understand what possible benefit not exporting the
> data has in the first place, and why complicating it with a call to a
> function (which isn't actually executing where it appears to live), is
> considered a benefit.

Note that after bringing these two gratuitously differently handled
symbols in line with everything else in the boot trampoline, a later patch
in the series puts all this stuff into its own data section which is
copied back up to the Xen image at the end of the real mode phase of boot,
and all the pointer gymnastics for all of them go away completely

--
dwmw2


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel