> On 14-Sep-2023, at 5:19 PM, David Hildenbrand <da...@redhat.com> wrote:
>
>>>>> 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.
Yes I see the function memory_device_get_free_addr() that does all the range
checks and validations and this gets called from the pre_plug() handler.
>
> [...]
>
>>> /*
>>> * 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?).
Other than the fix you proposed I am not sure if we need to fix anything else
atm. Seems physical address space bound checks are already in place.
Re: doc, maybe. I will add it to my TODO list.
>
> --
> Cheers,
>
> David / dhildenb
