On Thu, 28 Nov 2013 18:08:32 +0100
Stefan Weil <s...@weilnetz.de> wrote:
> Am 28.11.2013 07:29, schrieb Antony Pavlov:
> > Signed-off-by: Antony Pavlov <antonynpav...@gmail.com>
> > Reviewed-by: Richard Henderson <r...@twiddle.net>
> > ---
> > hw/mips/mips_malta.c | 25 +++++++++++++------------
> > include/hw/mips/bios.h | 3 ++-
> > 2 files changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> > index 05c8771..604832f 100644
> > --- a/hw/mips/mips_malta.c
> > +++ b/hw/mips/mips_malta.c
> > @@ -51,6 +51,7 @@
> > #include "sysemu/qtest.h"
> > #include "qemu/error-report.h"
> > #include "hw/empty_slot.h"
> > +#include "qemu/sizes.h"
> >
> > //#define DEBUG_BOARD_INIT
> >
> > @@ -63,7 +64,7 @@
> > #define FPGA_ADDRESS 0x1f000000ULL
> > #define RESET_ADDRESS 0x1fc00000ULL
> >
> > -#define FLASH_SIZE 0x400000
> > +#define FLASH_SIZE SZ_4M
> >
> > #define MAX_IDE_BUS 2
> >
> > @@ -827,8 +828,8 @@ static int64_t load_kernel (void)
> > }
> >
> > prom_set(prom_buf, prom_index++, "memsize");
> > - prom_set(prom_buf, prom_index++, "%i",
> > - MIN(loaderparams.ram_size, 256 << 20));
> > + prom_set(prom_buf, prom_index++, "%li",
> > + MIN(loaderparams.ram_size, SZ_256M));
> > prom_set(prom_buf, prom_index++, "modetty0");
> > prom_set(prom_buf, prom_index++, "38400n8r");
> > prom_set(prom_buf, prom_index++, NULL);
> > @@ -954,10 +955,10 @@ void mips_malta_init(QEMUMachineInitArgs *args)
> > env = &cpu->env;
> >
> > /* allocate RAM */
> > - if (ram_size > (2048u << 20)) {
> > + if (ram_size > SZ_2G) {
> > fprintf(stderr,
> > - "qemu: Too much memory for this machine: %d MB, maximum
> > 2048 MB\n",
> > - ((unsigned int)ram_size / (1 << 20)));
> > + "qemu: Too much memory for this machine: %ld MB, maximum
> > 2048 MB\n",
> > + ((unsigned long)ram_size / SZ_1M));
> > exit(1);
> > }
> >
> > @@ -968,17 +969,17 @@ void mips_malta_init(QEMUMachineInitArgs *args)
> >
> > /* 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)));
> > + ram_high, 0, MIN(ram_size, SZ_256M));
> > 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)) {
> > + if (ram_size > SZ_512M) {
> > 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);
> > + ram_high, SZ_512M,
> > + ram_size - SZ_512M);
> > + memory_region_add_subregion(system_memory, SZ_512M,
> > ram_low_postio);
> > }
> >
> > /* generate SPD EEPROM data */
> > @@ -1012,7 +1013,7 @@ void mips_malta_init(QEMUMachineInitArgs *args)
> > fl_idx++;
> > if (kernel_filename) {
> > /* Write a small bootloader to the flash location. */
> > - loaderparams.ram_size = MIN(ram_size, 256 << 20);
> > + loaderparams.ram_size = MIN(ram_size, SZ_256M);
> > loaderparams.kernel_filename = kernel_filename;
> > loaderparams.kernel_cmdline = kernel_cmdline;
> > loaderparams.initrd_filename = initrd_filename;
> > diff --git a/include/hw/mips/bios.h b/include/hw/mips/bios.h
> > index b4b88ac..3d7da4b 100644
> > --- a/include/hw/mips/bios.h
> > +++ b/include/hw/mips/bios.h
> > @@ -1,6 +1,7 @@
> > #include "cpu.h"
> > +#include "qemu/sizes.h"
> >
> > -#define BIOS_SIZE (4 * 1024 * 1024)
> > +#define BIOS_SIZE SZ_4M
> > #ifdef TARGET_WORDS_BIGENDIAN
> > #define BIOS_FILENAME "mips_bios.bin"
> > #else
>
>
> What about using (256 * MiB) instead of SZ_256M or (256 << 20)? SZ_256
> is better than the last variant, but I prefer the first variant even
> more. It is used in QEMU since a long time (for example in eepro100.c
> and vdi.c).
>
> Of course the definitions for KiB, MiB, GiB, TiB should be moved from
> their current locations to a common header file (I suggest
> qemu-common.h) if we agree on using them everywhere.
>
> The SZ_xxx macros might be useful nevertheless, but do we need a new
> header file, or could they be added to bitops.h?
sizes.h is widely used inside linux kernel, u-boot and barebox so it is a de
facto standard header file. IMHO it's better to keep it as a separate header
file.
--
Best regards,
Antony Pavlov
