Re: [Qemu-devel] i386: pc: align gpa<->hpa on 1GB boundary (v5)

[Marcelo Tosatti <mtosatti@redhat.com>]

On Tue, Nov 12, 2013 at 01:45:51PM +0100, Igor Mammedov wrote:
> On Sun, 10 Nov 2013 18:47:53 -0200
> Marcelo Tosatti <mtosatti@redhat.com> wrote:
> 
> [...]
> 
> > @@ -1177,10 +1182,50 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
> >      e820_add_entry(0, below_4g_mem_size, E820_RAM);
> >      if (above_4g_mem_size > 0) {
> >          ram_above_4g = g_malloc(sizeof(*ram_above_4g));
> it is a memory leak when "ram-above-4g" is not created
> 
> > -        memory_region_init_alias(ram_above_4g, NULL, "ram-above-4g", ram,
> > -                                 below_4g_mem_size, above_4g_mem_size);
> > -        memory_region_add_subregion(system_memory, 0x100000000ULL,
> > +        /*
> > +         *
> > +         * If 1GB hugepages are used to back guest RAM, map guest address
> > +         * space in the range [ramsize,ramsize+holesize] to the ram block
> > +         * range [holestart, 4GB]
> > +         *
> > +         *                      0      h     4G     [ramsize,ramsize+holesize]
> > +         *
> > +         * guest-addr-space     [      ]     [      ][xxx]
> > +         *                                /----------/
> > +         * contiguous-ram-block [      ][xxx][     ]
> > +         *
> > +         * So that memory beyond 4GB is aligned on a 1GB boundary,
> > +         * at the host physical address space.
> > +         *
> > +         */
> > +        if (guest_info->gb_align) {
> > +            uint64_t holesize = 0x100000000ULL - below_4g_mem_size;
> > +            uint64_t piecetwosize = holesize - align_offset;
> > +
> > +            assert(piecetwosize <= holesize);
> > +
> > +            if ((above_4g_mem_size - piecetwosize) > 0) {
> here is integer overflow,
> reproducable with:  -mem-path /var/lib/hugetlbfs/global/pagesize-1GB -m 3600

Gah.

> 
> 
> > +                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);
> > +            }
> > +
> > +            ram_above_4g_piecetwo = g_malloc(sizeof(*ram_above_4g_piecetwo));
> > +            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,
> is there a guaranty that "ram-above-4g-piecetwo" will be mapped immediately
> after "ram-above-4g" without any gap?

Yes:

Length of piece one alias: above_4g_mem_size - piecetwosize
Start address of piecetwo: 0x100000000ULL + above_4g_mem_size - piecetwosize

> if there is no then you might need to change how e820_add_entry() for high ram
> is handled and possibly CMOS value as well. 

> 
> > +                                        ram_above_4g_piecetwo);
> > +        } else {
> > +            memory_region_init_alias(ram_above_4g, NULL, "ram-above-4g", ram,
> > +                                    below_4g_mem_size, above_4g_mem_size);
> > +            memory_region_add_subregion(system_memory, 0x100000000ULL,
> >                                      ram_above_4g);
> > +        }
> >          e820_add_entry(0x100000000ULL, above_4g_mem_size, E820_RAM);
> >      }
> [...]
> -- 
> Regards,
>   Igor