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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.05.19 at 11:23, <andrew.cooper3@xxxxxxxxxx> wrote:
> 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.

Without having gone back to the old thread, I think part of
the motivation to avoid such direct data accesses was that
an initial version of the patch actually broke things by
introducing such accesses.

Apart from that, despite not being a heavy C++ user, I
very much appreciate the language's fundamental desire to
avoid direct accesses to data elements, providing functions
(methods) to do so instead.

Jan



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