Hi Igor, On 2/22/19 1:57 PM, Igor Mammedov wrote: > On Wed, 20 Feb 2019 23:39:53 +0100 > Eric Auger <eric.au...@redhat.com> wrote: > >> Up to now the memory map has been static and the high IO region >> base has always been 256GiB. >> >> This patch modifies the virt_set_memmap() function, which freezes >> the memory map, so that the high IO range base becomes floating, >> located after the initial RAM and the device memory. >> >> The function computes >> - the base of the device memory, >> - the size of the device memory and >> - the highest GPA used in the memory map. >> >> The two former will be used when defining the device memory region >> while the latter will be used at VM creation to choose the requested >> IPA size. >> >> Setting all the existing highmem IO regions beyond the RAM >> allows to have a single contiguous RAM region (initial RAM and >> possible hotpluggable device memory). That way we do not need >> to do invasive changes in the EDK2 FW to support a dynamic >> RAM base. >> >> Still the user cannot request an initial RAM size greater than 255GB. >> Also we handle the case where maxmem or slots options are passed, >> although no device memory is usable at the moment. In this case, we >> just ignore those settings. >> >> Signed-off-by: Eric Auger <eric.au...@redhat.com> >> --- >> hw/arm/virt.c | 47 ++++++++++++++++++++++++++++++++++--------- >> include/hw/arm/virt.h | 3 +++ >> 2 files changed, 41 insertions(+), 9 deletions(-) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 12039a0367..9db602457b 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -107,8 +107,9 @@ >> * of a terabyte of RAM will be doing it on a host with more than a >> * terabyte of physical address space.) >> */ >> -#define RAMLIMIT_GB 255 >> -#define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024) >> +#define RAMBASE GiB >> +#define LEGACY_RAMLIMIT_GB 255 >> +#define LEGACY_RAMLIMIT_BYTES (LEGACY_RAMLIMIT_GB * GiB) >> >> /* Addresses and sizes of our components. >> * 0..128MB is space for a flash device so we can run bootrom code such as >> UEFI. >> @@ -149,7 +150,7 @@ static const MemMapEntry base_memmap[] = { >> [VIRT_PCIE_MMIO] = { 0x10000000, 0x2eff0000 }, >> [VIRT_PCIE_PIO] = { 0x3eff0000, 0x00010000 }, >> [VIRT_PCIE_ECAM] = { 0x3f000000, 0x01000000 }, >> - [VIRT_MEM] = { 0x40000000, RAMLIMIT_BYTES }, >> + [VIRT_MEM] = { RAMBASE, LEGACY_RAMLIMIT_BYTES }, >> }; >> >> /* >> @@ -1367,16 +1368,48 @@ static uint64_t >> virt_cpu_mp_affinity(VirtMachineState *vms, int idx) >> >> static void virt_set_memmap(VirtMachineState *vms) >> { >> + MachineState *ms = MACHINE(vms); >> hwaddr base; >> int i; >> >> + if (ms->maxram_size > ms->ram_size || ms->ram_slots > 0) { >> + error_report("mach-virt: does not support device memory: " >> + "ignore maxmem and slots options"); >> + ms->maxram_size = ms->ram_size; >> + ms->ram_slots = 0; >> + } >> + if (ms->ram_size > (ram_addr_t)LEGACY_RAMLIMIT_BYTES) { >> + error_report("mach-virt: cannot model more than %dGB RAM", >> + LEGACY_RAMLIMIT_GB); >> + exit(1); >> + } > I'd drop these checks and amend below code so that it would init device_memory > logic when ram_slots > 0. It should simplify follow up patches by dropping > all machine version specific parts. I don't have sufficient knowledge of virtio-mem/virtio-pmem. Do they also use slots?
> It shouldn't break old machines as layout stays the same but would allow to > start old machine with pc-dimms which is fine from migration pov as target > also should be able to start QEMU with the same options for migration to > start. > > >> + >> vms->memmap = extended_memmap; >> >> for (i = 0; i < ARRAY_SIZE(base_memmap); i++) { >> vms->memmap[i] = base_memmap[i]; >> } >> >> - vms->high_io_base = 256 * GiB; /* Top of the legacy initial RAM region >> */ >> + /* >> + * We compute the base of the high IO region depending on the >> + * amount of initial and device memory. The device memory start/size >> + * is aligned on 1GiB. We never put the high IO region below 256GiB >> + * so that if maxram_size is < 255GiB we keep the legacy memory map. >> + * The device region size assumes 1GiB page max alignment per slot. >> + */ >> + vms->device_memory_base = ROUND_UP(RAMBASE + ms->ram_size, GiB); >> + vms->device_memory_size = ms->maxram_size - ms->ram_size + >> + ms->ram_slots * GiB; So does everyone agree on this device memory size computation? I would like to make sure this is future proof. Do I need to add a machine option like on x86 to enforce slot alignment or is it OK? Thanks Eric >> + >> + vms->high_io_base = vms->device_memory_base + >> + ROUND_UP(vms->device_memory_size, GiB); >> + if (vms->high_io_base < vms->device_memory_base) { >> + error_report("maxmem/slots too huge"); >> + exit(EXIT_FAILURE); >> + } >> + if (vms->high_io_base < 256 * GiB) { >> + vms->high_io_base = 256 * GiB; >> + } >> base = vms->high_io_base; >> >> for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) { >> @@ -1387,6 +1420,7 @@ static void virt_set_memmap(VirtMachineState *vms) >> vms->memmap[i].size = size; >> base += size; >> } >> + vms->highest_gpa = base - 1; >> } >> >> static void machvirt_init(MachineState *machine) >> @@ -1470,11 +1504,6 @@ static void machvirt_init(MachineState *machine) >> >> vms->smp_cpus = smp_cpus; >> >> - if (machine->ram_size > vms->memmap[VIRT_MEM].size) { >> - error_report("mach-virt: cannot model more than %dGB RAM", >> RAMLIMIT_GB); >> - exit(1); >> - } >> - >> if (vms->virt && kvm_enabled()) { >> error_report("mach-virt: KVM does not support providing " >> "Virtualization extensions to the guest CPU"); >> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h >> index 3dc7a6c5d5..acad0400d8 100644 >> --- a/include/hw/arm/virt.h >> +++ b/include/hw/arm/virt.h >> @@ -132,6 +132,9 @@ typedef struct { >> uint32_t iommu_phandle; >> int psci_conduit; >> hwaddr high_io_base; >> + hwaddr highest_gpa; >> + hwaddr device_memory_base; >> + hwaddr device_memory_size; >> } VirtMachineState; >> >> #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM) >