Re: [PATCH] mem/x86: add processor address space check for VM memory

[David Hildenbrand <david@redhat.com>]

>>>> We requested a to hotplug a maximum of "8 GiB", and sized the area slightly larger to allow for some flexibility
>>>> when it comes to placing DIMMs in that "device-memory" area.
>>> Right but here in this example you do not hot plug memory while the VM is running. We can hot plug 8G yes, but the memory may not physically exist yet (and may never exist). How can we use this math to provision device-memory when the memory may not exist physically?
>>
>> We simply reserve a region in GPA space where we can coldplug and hotplug a
>> predefined maximum amount of memory we can hotplug.
>>
>> What do you think is wrong with that?
> 
> The only issue I have is that even though we are accounting for it, the memory actually might not be physically present.

Not sure if "accounting" is the right word; the memory is not present 
and nowhere indicated as present. It's just a reservation of GPA space, 
like the PCI hole is as well.

[...]

>>
>> Yes. In this case ms->ram_size == ms->maxram_size and you cannot cold/hotplug any memory devices.
>>
>> See how pc_memory_init() doesn't call machine_memory_devices_init() in that case.
>>
>> That's what the QEMU user asked for when *not* specifying maxmem (e.g., -m 4g).
>>
>> In order to cold/hotplug any memory devices, you have to tell QEMU ahead of time how much memory
>> you are intending to provide using memory devices (DIMM, NVDIMM, virtio-pmem, virtio-mem).
> 
> So that means that when we are actually hot plugging the memory, there is no need to actually perform additional checks. It can be done statically when -mem and -maxmem etc are provided in the command line.

What memory device code does, is find a free location inside the 
reserved GPA space for memory devices. Then, it maps that device at
that location.

[...]

>> /*
>> * The 64bit pci hole starts after "above 4G RAM" and
>> * potentially the space reserved for memory hotplug.
>> */
>>
>> There is the
>> 	ROUND_UP(hole64_start, 1 * GiB);
>> in there that is not really required for the !hole64 case. It
>> shouldn't matter much in practice I think (besides an aligned value
>> showing up in the error message).
>>
>> We could factor out most of that calculation into a
>> separate function, skipping that alignment to make that
>> clearer.
> 
> Yeah this whole memory segmentation is quite complicated and might benefit from a qemu doc or a refactoring.

Absolutely. Do you have time to work on that (including the updated fix?).

-- 
Cheers,

David / dhildenb