Re: [Qemu-devel] [PATCH v3 29/41] hw/mips: Use the BYTE-based definitions

[Alistair Francis]

On Sun, Apr 15, 2018 at 4:42 PM, Philippe Mathieu-Daudé  wrote:
> It eases code review, unit is explicit.
>
> Patch generated using:
>
>   $ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ include/hw/
>
> and modified manually.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  include/hw/intc/mips_gic.h |  3 ++-
>  include/hw/mips/bios.h |  3 ++-
>  hw/mips/boston.c   |  4 ++--
>  hw/mips/mips_fulong2e.c|  7 ---
>  hw/mips/mips_malta.c   | 23 +--
>  hw/mips/mips_r4k.c | 11 ++-
>  hw/misc/mips_itu.c |  3 ++-
>  hw/pci-host/xilinx-pcie.c  |  5 +++--
>  8 files changed, 34 insertions(+), 25 deletions(-)
>
> diff --git a/include/hw/intc/mips_gic.h b/include/hw/intc/mips_gic.h
> index b98d50094a..c0da44bdb3 100644
> --- a/include/hw/intc/mips_gic.h
> +++ b/include/hw/intc/mips_gic.h
> @@ -11,6 +11,7 @@
>  #ifndef MIPS_GIC_H
>  #define MIPS_GIC_H
>
> +#include "qemu/units.h"
>  #include "hw/timer/mips_gictimer.h"
>  #include "cpu.h"
>  /*
> @@ -19,7 +20,7 @@
>
>  /* The MIPS default location */
>  #define GIC_BASE_ADDR   0x1bdcULL
> -#define GIC_ADDRSPACE_SZ(128 * 1024)
> +#define GIC_ADDRSPACE_SZ(128 * K_BYTE)
>
>  /* Constants */
>  #define GIC_POL_POS 1
> diff --git a/include/hw/mips/bios.h b/include/hw/mips/bios.h
> index b4b88ac43d..b4c97ce87c 100644
> --- a/include/hw/mips/bios.h
> +++ b/include/hw/mips/bios.h
> @@ -1,6 +1,7 @@
> +#include "qemu/units.h"
>  #include "cpu.h"
>
> -#define BIOS_SIZE (4 * 1024 * 1024)
> +#define BIOS_SIZE (4 * M_BYTE)
>  #ifdef TARGET_WORDS_BIGENDIAN
>  #define BIOS_FILENAME "mips_bios.bin"
>  #else
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index fb23161b33..edc39e91f7 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -32,7 +32,7 @@
>  #include "hw/mips/cpudevs.h"
>  #include "hw/pci-host/xilinx-pcie.h"
>  #include "qapi/error.h"
> -#include "qemu/cutils.h"
> +#include "qemu/units.h"
>  #include "qemu/error-report.h"
>  #include "qemu/log.h"
>  #include "chardev/char.h"
> @@ -437,7 +437,7 @@ static void boston_mach_init(MachineState *machine)
>  bool is_64b;
>
>  if ((machine->ram_size % G_BYTE) ||
> -(machine->ram_size > (2 * G_BYTE))) {
> +(machine->ram_size > 2 * G_BYTE)) {
>  error_report("Memory size must be 1GB or 2GB");
>  exit(1);
>  }
> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
> index 02fb2fdcc4..779883db7c 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -19,6 +19,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "qapi/error.h"
>  #include "hw/hw.h"
>  #include "hw/i386/pc.h"
> @@ -159,7 +160,7 @@ static int64_t load_kernel (CPUMIPSState *env)
>  /* Setup minimum environment variables */
>  prom_set(prom_buf, index++, "busclock=3300");
>  prom_set(prom_buf, index++, "cpuclock=1");
> -prom_set(prom_buf, index++, "memsize=%i", 
> loaderparams.ram_size/1024/1024);
> +prom_set(prom_buf, index++, "memsize=%llu", loaderparams.ram_size / 
> M_BYTE);
>  prom_set(prom_buf, index++, "modetty0=38400n8r");
>  prom_set(prom_buf, index++, NULL);
>
> @@ -303,10 +304,10 @@ static void mips_fulong2e_init(MachineState *machine)
>  qemu_register_reset(main_cpu_reset, cpu);
>
>  /* fulong 2e has 256M ram. */
> -ram_size = 256 * 1024 * 1024;
> +ram_size = 256 * M_BYTE;
>
>  /* fulong 2e has a 1M flash.Winbond W39L040AP70Z */
> -bios_size = 1024 * 1024;
> +bios_size = 1 * M_BYTE;
>
>  /* allocate RAM */
>  memory_region_allocate_system_memory(ram, NULL, "fulong2e.ram", 
> ram_size);
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index f6513a4fd5..240dd762be 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -23,6 +23,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "qemu-common.h"
>  #include "cpu.h"
>  #include "hw/hw.h"
> @@ -844,7 +845,8 @@ static int64_t load_kernel (void)
>  /* The kernel allocates the bootmap memory in the low memory 
> after
> the initrd.  It takes at most 128kiB for 2GB RAM and 4kiB
> pages.  */
> -initrd_offset = (loaderparams.ram_low_size - initrd_size - 131072
> +initrd_offset = (loaderparams.ram_low_size - initrd_size
> + - (128 * K_BYTE)
>   - ~INITRD_PAGE_MASK) & INITRD_PAGE_MASK;
>  if (kernel_high >= initrd_offset) {
>  error_report("memory too small for initial ram disk '%s'",
> @@ -1022,9 +1024,9 @@ void mips_malta_init(MachineState *machine)
>  mips_create_cpu(s, machine->cpu_type, _irq, _irq);
>
>  /* allocate RAM */
> -if (ram_size > (2048u << 20)) {
> -error_report("Too much memory for 

[Qemu-devel] [PATCH v3 29/41] hw/mips: Use the BYTE-based definitions

[Philippe Mathieu-Daudé]

It eases code review, unit is explicit.

Patch generated using:

  $ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ include/hw/

and modified manually.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/intc/mips_gic.h |  3 ++-
 include/hw/mips/bios.h |  3 ++-
 hw/mips/boston.c   |  4 ++--
 hw/mips/mips_fulong2e.c|  7 ---
 hw/mips/mips_malta.c   | 23 +--
 hw/mips/mips_r4k.c | 11 ++-
 hw/misc/mips_itu.c |  3 ++-
 hw/pci-host/xilinx-pcie.c  |  5 +++--
 8 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/include/hw/intc/mips_gic.h b/include/hw/intc/mips_gic.h
index b98d50094a..c0da44bdb3 100644
--- a/include/hw/intc/mips_gic.h
+++ b/include/hw/intc/mips_gic.h
@@ -11,6 +11,7 @@
 #ifndef MIPS_GIC_H
 #define MIPS_GIC_H
 
+#include "qemu/units.h"
 #include "hw/timer/mips_gictimer.h"
 #include "cpu.h"
 /*
@@ -19,7 +20,7 @@
 
 /* The MIPS default location */
 #define GIC_BASE_ADDR   0x1bdcULL
-#define GIC_ADDRSPACE_SZ(128 * 1024)
+#define GIC_ADDRSPACE_SZ(128 * K_BYTE)
 
 /* Constants */
 #define GIC_POL_POS 1
diff --git a/include/hw/mips/bios.h b/include/hw/mips/bios.h
index b4b88ac43d..b4c97ce87c 100644
--- a/include/hw/mips/bios.h
+++ b/include/hw/mips/bios.h
@@ -1,6 +1,7 @@
+#include "qemu/units.h"
 #include "cpu.h"
 
-#define BIOS_SIZE (4 * 1024 * 1024)
+#define BIOS_SIZE (4 * M_BYTE)
 #ifdef TARGET_WORDS_BIGENDIAN
 #define BIOS_FILENAME "mips_bios.bin"
 #else
diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index fb23161b33..edc39e91f7 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -32,7 +32,7 @@
 #include "hw/mips/cpudevs.h"
 #include "hw/pci-host/xilinx-pcie.h"
 #include "qapi/error.h"
-#include "qemu/cutils.h"
+#include "qemu/units.h"
 #include "qemu/error-report.h"
 #include "qemu/log.h"
 #include "chardev/char.h"
@@ -437,7 +437,7 @@ static void boston_mach_init(MachineState *machine)
 bool is_64b;
 
 if ((machine->ram_size % G_BYTE) ||
-(machine->ram_size > (2 * G_BYTE))) {
+(machine->ram_size > 2 * G_BYTE)) {
 error_report("Memory size must be 1GB or 2GB");
 exit(1);
 }
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 02fb2fdcc4..779883db7c 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -19,6 +19,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "qapi/error.h"
 #include "hw/hw.h"
 #include "hw/i386/pc.h"
@@ -159,7 +160,7 @@ static int64_t load_kernel (CPUMIPSState *env)
 /* Setup minimum environment variables */
 prom_set(prom_buf, index++, "busclock=3300");
 prom_set(prom_buf, index++, "cpuclock=1");
-prom_set(prom_buf, index++, "memsize=%i", loaderparams.ram_size/1024/1024);
+prom_set(prom_buf, index++, "memsize=%llu", loaderparams.ram_size / 
M_BYTE);
 prom_set(prom_buf, index++, "modetty0=38400n8r");
 prom_set(prom_buf, index++, NULL);
 
@@ -303,10 +304,10 @@ static void mips_fulong2e_init(MachineState *machine)
 qemu_register_reset(main_cpu_reset, cpu);
 
 /* fulong 2e has 256M ram. */
-ram_size = 256 * 1024 * 1024;
+ram_size = 256 * M_BYTE;
 
 /* fulong 2e has a 1M flash.Winbond W39L040AP70Z */
-bios_size = 1024 * 1024;
+bios_size = 1 * M_BYTE;
 
 /* allocate RAM */
 memory_region_allocate_system_memory(ram, NULL, "fulong2e.ram", ram_size);
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index f6513a4fd5..240dd762be 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -23,6 +23,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "qemu-common.h"
 #include "cpu.h"
 #include "hw/hw.h"
@@ -844,7 +845,8 @@ static int64_t load_kernel (void)
 /* The kernel allocates the bootmap memory in the low memory after
the initrd.  It takes at most 128kiB for 2GB RAM and 4kiB
pages.  */
-initrd_offset = (loaderparams.ram_low_size - initrd_size - 131072
+initrd_offset = (loaderparams.ram_low_size - initrd_size
+ - (128 * K_BYTE)
  - ~INITRD_PAGE_MASK) & INITRD_PAGE_MASK;
 if (kernel_high >= initrd_offset) {
 error_report("memory too small for initial ram disk '%s'",
@@ -1022,9 +1024,9 @@ void mips_malta_init(MachineState *machine)
 mips_create_cpu(s, machine->cpu_type, _irq, _irq);
 
 /* allocate RAM */
-if (ram_size > (2048u << 20)) {
-error_report("Too much memory for this machine: %dMB, maximum 2048MB",
- ((unsigned int)ram_size / (1 << 20)));
+if (ram_size > 2 * G_BYTE) {
+error_report("Too much memory for this machine: %lluMB, maximum 
2048MB",
+ ram_size / M_BYTE);
 exit(1);
 }
 
@@ -1035,17 +1037,18 @@ void mips_malta_init(MachineState *machine)
 
 /* alias for pre IO hole access */