Re: [Xen-devel] [PATCH 06/13] xen: detect pre-allocated memory interfering with e820 map

[Juergen Gross <jgross@suse.com>]

On 02/25/2015 05:00 PM, Juergen Gross wrote:
> On 02/25/2015 03:24 PM, David Vrabel wrote:
>> On 24/02/15 06:27, Juergen Gross wrote:
>>> On 02/19/2015 07:07 PM, David Vrabel wrote:
>>>> On 18/02/2015 06:51, Juergen Gross wrote:
>>>>> +{
>>>>> +    unsigned long pfn;
>>>>> +    unsigned long area_start, area_end;
>>>>> +    unsigned i;
>>>>> +
>>>>> +    for (i = 0; i < XEN_N_RESERVED_AREAS; i++) {
>>>>> +
>>>>> +        if (!xen_reserved_area[i].size)
>>>>> +            break;
>>>>> +
>>>>> +        area_start = PFN_DOWN(xen_reserved_area[i].start);
>>>>> +        area_end = PFN_UP(xen_reserved_area[i].start +
>>>>> +                  xen_reserved_area[i].size);
>>>>> +        if (area_start >= end_pfn || area_end <= start_pfn)
>>>>> +            continue;
>>>>> +
>>>>> +        if (area_start > start_pfn)
>>>>> +            xen_set_identity_and_remap_chunk(start_pfn, area_start,
>>>>> +                             released, remapped);
>>>>> +
>>>>> +        if (area_end < end_pfn)
>>>>> +            xen_set_identity_and_remap_chunk(area_end, end_pfn,
>>>>> +                             released, remapped);
>>>>> +
>>>>> +        *remapped += min(area_end, end_pfn) -
>>>>> +                max(area_start, start_pfn);
>>>>> +
>>>>> +        return;
>>>>
>>>> Why not defer the whole chunk that conflicts?  Or for that matter defer
>>>> all this remapping to the last minute.
>>>
>>> There are two problems arising from this:
>>>
>>> - In the initrd case remapping would be deferred too long: the initrd
>>>    data is still in use when device initialization is running. And we
>>>    really want the remap to have happened before PCI space is being
>>> used.
>>
>> I'm not sure I understand what you're saying here.
>
> I thought you wanted to defer the remapping to the point where the
> initrd memory is no longer being used. But the suggestion below is
> more clear.
>
>>
>> I'm suggesting:
>>
>> 1. Reserve all holes.
>>
>> 2. Relocate (if necessary) all modules (initrd, etc.) to regions that
>> are RAM in the e820.
>>
>> 3. Rebuild the p2m in RAM.
>>
>> 4. Relocate frames from E820 holes/reserved to the end, free p2m pages
>> from the holes and replacing them with the read-only 1:1 page (where
>> possible).
>>
>>> - Delaying all remapping to the point where the new p2m list is in place
>>>    would either result in a p2m list with all memory holes covered with
>>>    individual entries as the new list is built with those holes still
>>>    populated, ...
>>>    The first option could easily waste significant amounts of memory (on
>>>    my test machine with 1TB RAM this would have been about 1GB), while
>>>    the second option would be performance critical.
>>
>> I don't understand how this wastes memory.   When you relocate the
>> frames from the holes you can reclaim the p2m pages for the holes (and
>> replace them with the r/o mapped identity p2m page).
>
> Okay, this would work, I guess.
>
> I'll have a try with some new patches...

I tried your approach and hit a problem I can't solve without a major
rework of the kernel's init sequence:

dmi_scan_machine() (and possibly other functions like probe_roms())
need the identity mappings of BIOS, ACPI or PCI memory. Otherwise
SMBIOS, DMI and extension ROMs won't be discovered.

This can be solved only by either a complete rework of the sequence of
called init functions (not desirable, I guess) or by doing the unmap
part of the remapping as early as today.

This means, of course, I was just lucky with my resolution of the p2m
table conflicting with the E820 map by just delaying the remapping of
this memory area: in case it would have collided with an area needed
to be identity mapped early, the machine wouldn't have been able to
boot my kernel. So I really need to relocate the p2m list, even if this
is not as easy as delaying the remapping.


Juergen