On Wed, Nov 13, 2013 at 05:58:32PM -0200, Marcelo Tosatti wrote: > On Wed, Nov 13, 2013 at 06:13:15PM +0100, Paolo Bonzini wrote: > > > assert(piecetwosize <= holesize); > > > > > > piecetwosize = MIN(above_4g_mem_size, piecetwosize); > > > if ((above_4g_mem_size - piecetwosize) > 0) { > > > memory_region_init_alias(ram_above_4g, NULL, > > > "ram-above-4g", > > > ram, 0x100000000ULL, > > > above_4g_mem_size - > > > piecetwosize); > > > memory_region_add_subregion(system_memory, 0x100000000ULL, > > > ram_above_4g); > > > } else { > > > g_free(ram_above_4g); > > > } > > > memory_region_init_alias(ram_above_4g_piecetwo, NULL, > > > "ram-above-4g-piecetwo", ram, > > > 0x100000000ULL - holesize, > > > piecetwosize); > > > memory_region_add_subregion(system_memory, > > > 0x100000000ULL + > > > above_4g_mem_size - piecetwosize, > > > ram_above_4g_piecetwo); > > > > There is still a small problem in that the 2MB rounding must not be > > done for old machine types. > > > > I did a really careful review of the code and everything else looks okay > > to me. However, it grew by accretion from v1 and now it took me really a > > long time to figure it out... I adjusted it a bit and the result seems > > easier to understand to me. > > > > Here's the hw/i386/pc.c part of the patch (the patch from v6 is unreadable): > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 12c436e..f2fd138 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -1156,8 +1156,10 @@ FWCfgState *pc_memory_init(MemoryRegion > > *system_memory, > > { > > int linux_boot, i; > > MemoryRegion *ram, *option_rom_mr; > > - MemoryRegion *ram_below_4g, *ram_above_4g; > > + MemoryRegion *ram_below_4g, *ram_above_4g_pieceone, > > *ram_above_4g_piecetwo; > > FWCfgState *fw_cfg; > > + uint64_t holesize, pieceonesize, piecetwosize; > > + uint64_t memsize, align_offset; > > > > linux_boot = (kernel_filename != NULL); > > > > @@ -1165,26 +1167,74 @@ FWCfgState *pc_memory_init(MemoryRegion > > *system_memory, > > * aliases to address portions of it, mostly for backwards > > compatibility > > * with older qemus that used qemu_ram_alloc(). > > */ > > + memsize = below_4g_mem_size + above_4g_mem_size; > > + holesize = 0x100000000ULL - below_4g_mem_size; > > + > > + /* If 1GB hugepages are used to back guest RAM, we want the > > + * physical address 4GB to map to 4GB in the RAM, so that > > + * memory beyond 4GB is aligned on a 1GB boundary, at the > > + * host physical address space. Thus, the ram block range > > + * [holestart, 4GB] is mapped to the last holesize bytes of RAM: > > + * > > + * 0 h 4G memsize-holesize > > + * > > + * contiguous-ram-block [xxxxxx][yyy][zzzzz] > > + * '-----------. > > + * guest-addr-space [xxxxxx] [zzzzz][yyy] > > + * > > + * This is only done in new-enough machine types, and of course > > + * it is only necessary if the [zzzzz] block exists at all. > > + */ > > + if (guest_info->gb_align && above_4g_mem_size > holesize) { > > + /* Round the allocation up to 2 MB to use more hugepages.
To align to 2MB boundary, the number of hugepages is the same. > > + * Remove the slack from the [yyy] piece so that pieceonesize > > + * (and thus the start of piecetwo) remains aligned. > > + */ > > + align_offset = ROUND_UP(memsize, 1UL << 21) - memsize; > > + piecetwosize = holesize - align_offset; > > + } else { > > + /* There's no "piece one", all memory above 4G starts Piece two. > > + * at below_4g_mem_size in the RAM block. Also no need > > + * to align anything. > > + */ > > + align_offset = 0; > > + piecetwosize = above_4g_mem_size; > > + } > > + > > ram = g_malloc(sizeof(*ram)); > > - memory_region_init_ram(ram, NULL, "pc.ram", > > - below_4g_mem_size + above_4g_mem_size); > > + memory_region_init_ram(ram, NULL, "pc.ram", memsize + align_offset); > > vmstate_register_ram_global(ram); > > *ram_memory = ram; > > + > > ram_below_4g = g_malloc(sizeof(*ram_below_4g)); > > memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram, > > 0, below_4g_mem_size); > > memory_region_add_subregion(system_memory, 0, ram_below_4g); > > + > > + pieceonesize = above_4g_mem_size - piecetwosize; > > + if (pieceonesize) { > > + ram_above_4g_pieceone = g_malloc(sizeof(*ram_above_4g_pieceone)); > > + memory_region_init_alias(ram_above_4g_pieceone, NULL, > > + "ram-above-4g-pieceone", ram, > > + 0x100000000ULL, pieceonesize); > > + memory_region_add_subregion(system_memory, 0x100000000ULL, > > + ram_above_4g_pieceone); > > + } > > Can you change the name of aliases and subregions without breaking > migration? > > Its much simpler, i'm fine with it. Test with Q35?