Re: [Xen-devel] Fixes for low memory allocation machinery in early boot code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.09.16 at 14:11, <daniel.kiper@xxxxxxxxxx> wrote:
> On Fri, Sep 16, 2016 at 06:15:10AM -0600, Jan Beulich wrote:
>> >>> On 14.09.16 at 10:23, <daniel.kiper@xxxxxxxxxx> wrote:
>> > Additionally, my investigation has shown that there are no bound checks in
>> > low memory allocation machinery for trampoline (by the way, in BIOS path we
>> > allocate 64 KiB for trampoline but in EFI code we properly calculate its 
>> > size;
>> > so, I think we should do the same calculation in BIOS path), stack and 
>> > boot data
>> > taken from multiboot protocol. Hence, relevant fixes should be added here 
>> > too.
>>
>> Would be nice, yes, but we need to weigh the need to presumably do
>> at least some of this in assembly code (for now at least) against the
>> potential gain.
>>
>> > Moreover I think that at least allocation machinery with additional checks
>> > described in last paragraph can be used on EFI platforms when Xen is booted
>> > via multiboot2 protocol. However, then high limit should be defined as 1 
>> > MiB.
>> > Though I think that low limit, 256 KiB, should stay as is.
>>
>> Why 1Mb? The 640k limit derives from hardware aspects, but doesn't
>> depend on the software environment.
> 
> I do not expect that anything which is not memory will reside between 640 KiB
> and 1 MiB on EFI platforms. So, if firmware says that it is usable why we 
> cannot
> use it? And I have a feeling that this idea lead to currently existing checks
> around trampoline allocation code for EFI. Though, as I saw EFI platforms
> usually does not expose region 640 KiB and 1 MiB as usable. However, this
> does not mean that they cannot in the future.

Hmm, when the region (or part of it) is reported as available, then I
guess we could use it. But as to there being RAM - I doubt it, since
for the next little while, EFI or not, we're talking about PC compatible
systems, which don't normally have RAM in that range.

>> > So, I think that we should prepare following patches:
>> >   - allocate properly calculated amount of memory for trampoline,
>> >   - define high/low limit as a constants and use them,
>> >   - add bounds checks for chosen low memory region, and bounds
>> >     checks in allocation machinery for trampoline and stack,
>> >   - add bounds checks to allocator in reloc.c.
>> >
>> > I have a feeling that this fixes are not very critical, however, nice to 
>> > have.
>>
>> Indeed. I'd just like to avoid that new code (read: your mb2 series)
>> gets introduced with similar issues. Just like the original EFI code at
>> least tries to properly allocate the trampoline space.
> 
> OK, I have a feeling that you wish to postpone most of proposed changes for 
> later.
> I can understand that. However, if we wish check that there is sufficient 
> amount
> of memory available for trampoline we should decide which value to use. Should
> we use 64 KiB from BIOS Xen assembly code or trampoline real size like in Xen
> EFI code? Trampoline real size is much more sensible for me but this requires
> some changes in assembly code allocating trampoline.

Why? Current EFI code uses the actual size too. Hence I think you
could do so as well in your new additions, leaving the legacy code
alone until you or someone else means to overhaul it.

> We can allocate trampoline
> real size on both EFI and BIOS platforms. Or leave BIOS case as is and use
> trampoline real size just only on EFI platforms.

Exactly.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel