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

[David Hildenbrand]

> > > > 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