| Message ID | 1377270586-14753-1-git-send-email-yongbok.kim@imgtec.com |
|---|---|
| State | New |
| Headers | show |
On 08/23/2013 08:09 AM, Yongbok Kim wrote: > + /* alias for pre IO hole access */ > + memory_region_init_alias(ram_low_preio, NULL, "mips_malta_low_preio.ram", > + ram_high, 0, MIN(ram_size, (256 << 20))); > + memory_region_add_subregion(system_memory, 0, ram_low_preio); > + > + /* alias for post IO hole access, if there is enough RAM */ > + if (ram_size > (512 << 20)) { > + ram_low_postio = g_new(MemoryRegion, 1); > + memory_region_init_alias(ram_low_postio, NULL, > + "mips_malta_low_postio.ram", > + ram_high, 512 << 20, > + ram_size - (512 << 20)); > + memory_region_add_subregion(system_memory, 512 << 20, ram_low_postio); > + } This sort of thing is what subregion prioirities are for. Ideally you'd have one alias region, with lower priority than the IO hole. Of course, it looks like most of gt64xxx_pci.c needs cleaning up for proper usage of the memory region infrastructure before that can happen... r~
Thanks Richard. To be clear, is that a "this would be a nice future improvement" or a "NAK until this is done"? Paul On Mon 26 Aug 2013 19:01:51 BST, Richard Henderson wrote: > On 08/23/2013 08:09 AM, Yongbok Kim wrote: >> + /* alias for pre IO hole access */ >> + memory_region_init_alias(ram_low_preio, NULL, "mips_malta_low_preio.ram", >> + ram_high, 0, MIN(ram_size, (256 << 20))); >> + memory_region_add_subregion(system_memory, 0, ram_low_preio); >> + >> + /* alias for post IO hole access, if there is enough RAM */ >> + if (ram_size > (512 << 20)) { >> + ram_low_postio = g_new(MemoryRegion, 1); >> + memory_region_init_alias(ram_low_postio, NULL, >> + "mips_malta_low_postio.ram", >> + ram_high, 512 << 20, >> + ram_size - (512 << 20)); >> + memory_region_add_subregion(system_memory, 512 << 20, ram_low_postio); >> + } > > This sort of thing is what subregion prioirities are for. Ideally you'd have > one alias region, with lower priority than the IO hole. > > Of course, it looks like most of gt64xxx_pci.c needs cleaning up for proper > usage of the memory region infrastructure before that can happen... > > > r~
On 08/27/2013 01:45 AM, Paul Burton wrote: > Thanks Richard. To be clear, is that a "this would be a nice future > improvement" or a "NAK until this is done"? Correct. r~
On 08/27/2013 01:45 AM, Paul Burton wrote: > Thanks Richard. To be clear, is that a "this would be a nice future > improvement" or a "NAK until this is done"? Sorry, mis-read your "or" there. I meant the former. r~
Great, thanks for looking it over & for amusing me with your original clear as mud answer :) Paul On Tue 27 Aug 2013 15:55:50 BST, Richard Henderson wrote: > On 08/27/2013 01:45 AM, Paul Burton wrote: >> Thanks Richard. To be clear, is that a "this would be a nice future >> improvement" or a "NAK until this is done"? > > Sorry, mis-read your "or" there. I meant the former. > > > r~
On Fri, Aug 23, 2013 at 04:09:46PM +0100, Yongbok Kim wrote: > From: Paul Burton <paul.burton@imgtec.com> > > A Malta board can support up to 2GiB of RAM. Since the unmapped kseg0/1 > regions are only 512MiB large & the latter 256MiB of those are taken up > by the IO region, access to RAM beyond 256MiB must be done through a > mapped region. In the case of a Linux guest this means we need to use > highmem. > > The mainline Linux kernel does not support highmem for Malta at this > time, however this can be tested using the linux-mti-3.8 kernel branch > available from: > > git://git.linux-mips.org/pub/scm/linux-mti.git > > You should be able to boot a Linux kernel built from the linux-mti-3.8 > branch, with CONFIG_HIGHMEM enabled, using 2GiB RAM by passing "-m 2G" It should be worth noting that for 64-bit kernel, an old kernel from mainline also works (tested on 3.2). > to QEMU and appending the following kernel parameters: > > mem=256m@0x0 mem=256m@0x90000000 mem=1536m@0x20000000 What is the reason for mapping the memory in three different areas? It also works with two areas only: mem=256m@0x0 mem=1792m@0x90000000 > Note that the upper half of the physical address space of a Malta > mirrors the lower half (hence the 2GiB limit) except that the IO region > (0x10000000-0x1fffffff in the lower half) is not mirrored in the upper > half. That is, physical addresses 0x90000000-0x9fffffff access RAM > rather than the IO region. The second mem parameter above accesses the > second 256MiB of RAM through the upper half of the physical address > space, making use of this property in order to avoid the IO region and > use the whole 2GiB RAM. While this is correct, this is a bit hard to follow. Maybe you can add some table like the following: 0x00000000 -> 0x0fffffff RAM 0x10000000 -> 0x1fffffff I/O 0x20000000 -> 0x7fffffff RAM 0x80000000 -> 0x8fffffff RAM (mirror of 0x00000000 -> 0x0fffffff) 0x90000000 -> 0x9fffffff RAM 0xa0000000 -> 0xffffffff RAM (mirror of 0x20000000 -> 0x7fffffff) > The memory setup may be seen as 'backwards' in this commit since the > 'real' memory is mapped in the upper half of the physical address space > and the lower half contains the aliases. On real hardware it would be > typical to see the upper half of the physical address space as the alias > since the bus addresses generated match the lower half of the physical > address space. However since the memory accessible in the upper half of > the physical address space is uninterrupted by the IO region it is > easiest to map the RAM as a whole there, and functionally it makes no > difference to the target code. > > Due to the requirements of accessing the second 256MiB of RAM through > a mapping to the upper half of the physical address space it is usual > for the bootloader to indicate a maximum of 256MiB memory to a kernel. > This allows kernels which do not support such access to boot on systems > with more than 256MiB of RAM. It is also the behaviour assumed by Linux. > QEMUs small generated bootloader is modified to provide this behaviour. > > Signed-off-by: Paul Burton <paul.burton@imgtec.com> > Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> > --- > hw/mips/mips_malta.c | 36 ++++++++++++++++++++++++++++-------- > 1 files changed, 28 insertions(+), 8 deletions(-) > > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index f8d064c..7d112ee 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -827,7 +827,8 @@ static int64_t load_kernel (void) > } > > prom_set(prom_buf, prom_index++, "memsize"); > - prom_set(prom_buf, prom_index++, "%i", loaderparams.ram_size); > + prom_set(prom_buf, prom_index++, "%i", > + MIN(loaderparams.ram_size, 256 << 20)); > prom_set(prom_buf, prom_index++, "modetty0"); > prom_set(prom_buf, prom_index++, "38400n8r"); > prom_set(prom_buf, prom_index++, NULL); > @@ -884,7 +885,9 @@ void mips_malta_init(QEMUMachineInitArgs *args) > char *filename; > pflash_t *fl; > MemoryRegion *system_memory = get_system_memory(); > - MemoryRegion *ram = g_new(MemoryRegion, 1); > + MemoryRegion *ram_high = g_new(MemoryRegion, 1); > + MemoryRegion *ram_low_preio = g_new(MemoryRegion, 1); > + MemoryRegion *ram_low_postio; > MemoryRegion *bios, *bios_copy = g_new(MemoryRegion, 1); > target_long bios_size = FLASH_SIZE; > const size_t smbus_eeprom_size = 8 * 256; > @@ -951,15 +954,32 @@ void mips_malta_init(QEMUMachineInitArgs *args) > env = &cpu->env; > > /* allocate RAM */ > - if (ram_size > (256 << 20)) { > + if (ram_size > (2048u << 20)) { > fprintf(stderr, > - "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", > + "qemu: Too much memory for this machine: %d MB, maximum 2048 MB\n", > ((unsigned int)ram_size / (1 << 20))); > exit(1); > } > - memory_region_init_ram(ram, NULL, "mips_malta.ram", ram_size); > - vmstate_register_ram_global(ram); > - memory_region_add_subregion(system_memory, 0, ram); > + > + /* register RAM at high address where it is undisturbed by IO */ > + memory_region_init_ram(ram_high, NULL, "mips_malta.ram", ram_size); > + vmstate_register_ram_global(ram_high); > + memory_region_add_subregion(system_memory, 0x80000000, ram_high); > + > + /* alias for pre IO hole access */ > + memory_region_init_alias(ram_low_preio, NULL, "mips_malta_low_preio.ram", > + ram_high, 0, MIN(ram_size, (256 << 20))); > + memory_region_add_subregion(system_memory, 0, ram_low_preio); > + > + /* alias for post IO hole access, if there is enough RAM */ > + if (ram_size > (512 << 20)) { > + ram_low_postio = g_new(MemoryRegion, 1); > + memory_region_init_alias(ram_low_postio, NULL, > + "mips_malta_low_postio.ram", > + ram_high, 512 << 20, > + ram_size - (512 << 20)); > + memory_region_add_subregion(system_memory, 512 << 20, ram_low_postio); > + } > > /* generate SPD EEPROM data */ > generate_eeprom_spd(&smbus_eeprom_buf[0 * 256], ram_size); > @@ -992,7 +1012,7 @@ void mips_malta_init(QEMUMachineInitArgs *args) > fl_idx++; > if (kernel_filename) { > /* Write a small bootloader to the flash location. */ > - loaderparams.ram_size = ram_size; > + loaderparams.ram_size = MIN(ram_size, 256 << 20); > loaderparams.kernel_filename = kernel_filename; > loaderparams.kernel_cmdline = kernel_cmdline; > loaderparams.initrd_filename = initrd_filename; The code part looks pretty fine to me, so: Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index f8d064c..7d112ee 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -827,7 +827,8 @@ static int64_t load_kernel (void) } prom_set(prom_buf, prom_index++, "memsize"); - prom_set(prom_buf, prom_index++, "%i", loaderparams.ram_size); + prom_set(prom_buf, prom_index++, "%i", + MIN(loaderparams.ram_size, 256 << 20)); prom_set(prom_buf, prom_index++, "modetty0"); prom_set(prom_buf, prom_index++, "38400n8r"); prom_set(prom_buf, prom_index++, NULL); @@ -884,7 +885,9 @@ void mips_malta_init(QEMUMachineInitArgs *args) char *filename; pflash_t *fl; MemoryRegion *system_memory = get_system_memory(); - MemoryRegion *ram = g_new(MemoryRegion, 1); + MemoryRegion *ram_high = g_new(MemoryRegion, 1); + MemoryRegion *ram_low_preio = g_new(MemoryRegion, 1); + MemoryRegion *ram_low_postio; MemoryRegion *bios, *bios_copy = g_new(MemoryRegion, 1); target_long bios_size = FLASH_SIZE; const size_t smbus_eeprom_size = 8 * 256; @@ -951,15 +954,32 @@ void mips_malta_init(QEMUMachineInitArgs *args) env = &cpu->env; /* allocate RAM */ - if (ram_size > (256 << 20)) { + if (ram_size > (2048u << 20)) { fprintf(stderr, - "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n", + "qemu: Too much memory for this machine: %d MB, maximum 2048 MB\n", ((unsigned int)ram_size / (1 << 20))); exit(1); } - memory_region_init_ram(ram, NULL, "mips_malta.ram", ram_size); - vmstate_register_ram_global(ram); - memory_region_add_subregion(system_memory, 0, ram); + + /* register RAM at high address where it is undisturbed by IO */ + memory_region_init_ram(ram_high, NULL, "mips_malta.ram", ram_size); + vmstate_register_ram_global(ram_high); + memory_region_add_subregion(system_memory, 0x80000000, ram_high); + + /* alias for pre IO hole access */ + memory_region_init_alias(ram_low_preio, NULL, "mips_malta_low_preio.ram", + ram_high, 0, MIN(ram_size, (256 << 20))); + memory_region_add_subregion(system_memory, 0, ram_low_preio); + + /* alias for post IO hole access, if there is enough RAM */ + if (ram_size > (512 << 20)) { + ram_low_postio = g_new(MemoryRegion, 1); + memory_region_init_alias(ram_low_postio, NULL, + "mips_malta_low_postio.ram", + ram_high, 512 << 20, + ram_size - (512 << 20)); + memory_region_add_subregion(system_memory, 512 << 20, ram_low_postio); + } /* generate SPD EEPROM data */ generate_eeprom_spd(&smbus_eeprom_buf[0 * 256], ram_size); @@ -992,7 +1012,7 @@ void mips_malta_init(QEMUMachineInitArgs *args) fl_idx++; if (kernel_filename) { /* Write a small bootloader to the flash location. */ - loaderparams.ram_size = ram_size; + loaderparams.ram_size = MIN(ram_size, 256 << 20); loaderparams.kernel_filename = kernel_filename; loaderparams.kernel_cmdline = kernel_cmdline; loaderparams.initrd_filename = initrd_filename;