Message ID | 1385620152-4368-3-git-send-email-antonynpavlov@gmail.com |
---|---|
State | New |
Headers | show |
Am 28.11.2013 07:29, schrieb Antony Pavlov: > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> > Reviewed-by: Richard Henderson <rth@twiddle.net> Are the int -> long changes caused by the BIT() macro usage? Otherwise I would've recommended to put that in a follow-up patch. But either way, Reviewed-by: Andreas Färber <afaerber@suse.de> Andreas
Am 28.11.2013 07:29, schrieb Antony Pavlov: > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> > Reviewed-by: Richard Henderson <rth@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? Stefan
On Thu, 28 Nov 2013 15:27:18 +0100 Andreas Färber <afaerber@suse.de> wrote: > Am 28.11.2013 07:29, schrieb Antony Pavlov: > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> > > Reviewed-by: Richard Henderson <rth@twiddle.net> > > Are the int -> long changes caused by the BIT() macro usage? Otherwise I > would've recommended to put that in a follow-up patch. Yes, the BIT() macro give us this conversion: #define BIT(nr) (1UL << (nr)) > But either way, > > Reviewed-by: Andreas Färber <afaerber@suse.de> > > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
On Thu, 28 Nov 2013 18:08:32 +0100 Stefan Weil <sw@weilnetz.de> wrote: > Am 28.11.2013 07:29, schrieb Antony Pavlov: > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> > > Reviewed-by: Richard Henderson <rth@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
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