Re: [Qemu-devel] [PATCH 08/30] hw/i386: use the BYTE-based definitions

[Igor Mammedov]

On Mon, 5 Mar 2018 13:29:15 +0100
Igor Mammedov  wrote:

> On Thu, 15 Feb 2018 01:28:38 -0300
> Philippe Mathieu-Daudé  wrote:
> 
> > It ease code review, unit is explicit.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé 
> > ---  
> [...]
> 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index deb440f286..9ccc6192b5 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -2320,8 +2320,8 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, 
> > GArray *tcpalog)
> >   (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, 
> > NULL);
> >  }
> >  
> > -#define HOLE_640K_START  (640 * 1024)
> > -#define HOLE_640K_END   (1024 * 1024)
> > +#define HOLE_640K_START  (640 * K_BYTE)
> > +#define HOLE_640K_END   (1024 * K_BYTE)  
> nit:
> could be 1 * M_BYTE
> 
> [...]
> 
> Reviewed-by: Igor Mammedov 
taking it back

While building it, I get:

In file included from qemu/hw/acpi/ich9.c:38:0:
qemu/include/hw/i386/ich9.h:25:28: error: ‘K_BYTE’ undeclared here (not in a 
function)
 #define ICH9_CC_SIZE (16 * K_BYTE) /* Chipset configuration registers */
^
qemu/include/hw/i386/ich9.h:56:25: note: in expansion of macro ‘ICH9_CC_SIZE’
 uint8_t chip_config[ICH9_CC_SIZE];
 ^
make: *** [hw/acpi/ich9.o] Error 1

 

Re: [Qemu-devel] [PATCH 08/30] hw/i386: use the BYTE-based definitions

[Igor Mammedov]

On Thu, 15 Feb 2018 01:28:38 -0300
Philippe Mathieu-Daudé  wrote:

> It ease code review, unit is explicit.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
[...]

> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index deb440f286..9ccc6192b5 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2320,8 +2320,8 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, 
> GArray *tcpalog)
>   (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
>  }
>  
> -#define HOLE_640K_START  (640 * 1024)
> -#define HOLE_640K_END   (1024 * 1024)
> +#define HOLE_640K_START  (640 * K_BYTE)
> +#define HOLE_640K_END   (1024 * K_BYTE)
nit:
could be 1 * M_BYTE

[...]

Reviewed-by: Igor Mammedov 

[Qemu-devel] [PATCH 08/30] hw/i386: use the BYTE-based definitions

[Philippe Mathieu-Daudé]

It ease code review, unit is explicit.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/i386/ich9.h |  2 +-
 hw/i386/acpi-build.c   |  4 ++--
 hw/i386/pc.c   | 18 +-
 hw/i386/pc_piix.c  |  2 +-
 hw/i386/pc_q35.c   |  2 +-
 hw/i386/pc_sysfw.c |  8 
 hw/i386/xen/xen-mapcache.c |  2 +-
 hw/intc/apic_common.c  |  2 +-
 hw/pci-host/gpex.c |  2 +-
 hw/pci-host/piix.c |  4 ++--
 hw/pci-host/q35.c  | 16 
 11 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 673d13d28f..87628dd867 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -22,7 +22,7 @@ I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t 
smb_io_base);
 
 void ich9_generate_smi(void);
 
-#define ICH9_CC_SIZE (16 * 1024) /* 16KB. Chipset configuration registers */
+#define ICH9_CC_SIZE (16 * K_BYTE) /* Chipset configuration registers */
 
 #define TYPE_ICH9_LPC_DEVICE "ICH9-LPC"
 #define ICH9_LPC_DEVICE(obj) \
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index deb440f286..9ccc6192b5 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2320,8 +2320,8 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray 
*tcpalog)
  (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
 }
 
-#define HOLE_640K_START  (640 * 1024)
-#define HOLE_640K_END   (1024 * 1024)
+#define HOLE_640K_START  (640 * K_BYTE)
+#define HOLE_640K_END   (1024 * K_BYTE)
 
 static void
 build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 55e69d66fe..94a1f3bc7b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -452,8 +452,8 @@ void pc_cmos_init(PCMachineState *pcms,
 rtc_set_memory(s, 0x15, val);
 rtc_set_memory(s, 0x16, val >> 8);
 /* extended memory (next 64MiB) */
-if (pcms->below_4g_mem_size > 1024 * 1024) {
-val = (pcms->below_4g_mem_size - 1024 * 1024) / 1024;
+if (pcms->below_4g_mem_size > 1 * M_BYTE) {
+val = (pcms->below_4g_mem_size - 1 * M_BYTE) / 1024;
 } else {
 val = 0;
 }
@@ -464,8 +464,8 @@ void pc_cmos_init(PCMachineState *pcms,
 rtc_set_memory(s, 0x30, val);
 rtc_set_memory(s, 0x31, val >> 8);
 /* memory between 16MiB and 4GiB */
-if (pcms->below_4g_mem_size > 16 * 1024 * 1024) {
-val = (pcms->below_4g_mem_size - 16 * 1024 * 1024) / 65536;
+if (pcms->below_4g_mem_size > 16 * M_BYTE) {
+val = (pcms->below_4g_mem_size - 16 * M_BYTE) / 65536;
 } else {
 val = 0;
 }
@@ -1390,11 +1390,11 @@ void pc_memory_init(PCMachineState *pcms,
 }
 
 pcms->hotplug_memory.base =
-ROUND_UP(0x1ULL + pcms->above_4g_mem_size, 1ULL << 30);
+ROUND_UP(0x1ULL + pcms->above_4g_mem_size, G_BYTE);
 
 if (pcmc->enforce_aligned_dimm) {
 /* size hotplug region assuming 1G page max alignment per slot */
-hotplug_mem_size += (1ULL << 30) * machine->ram_slots;
+hotplug_mem_size += machine->ram_slots * G_BYTE;
 }
 
 if ((pcms->hotplug_memory.base + hotplug_mem_size) <
@@ -1436,7 +1436,7 @@ void pc_memory_init(PCMachineState *pcms,
 if (!pcmc->broken_reserved_end) {
 res_mem_end += memory_region_size(>hotplug_memory.mr);
 }
-*val = cpu_to_le64(ROUND_UP(res_mem_end, 0x1ULL << 30));
+*val = cpu_to_le64(ROUND_UP(res_mem_end, G_BYTE));
 fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val));
 }
 
@@ -1472,7 +1472,7 @@ uint64_t pc_pci_hole64_start(void)
 hole64_start = 0x1ULL + pcms->above_4g_mem_size;
 }
 
-return ROUND_UP(hole64_start, 1ULL << 30);
+return ROUND_UP(hole64_start, G_BYTE);
 }
 
 qemu_irq pc_allocate_cpu_irq(void)
@@ -2114,7 +2114,7 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, 
Visitor *v,
 return;
 }
 
-if (value < (1ULL << 20)) {
+if (value < 1 * M_BYTE) {
 warn_report("Only %" PRIu64 " bytes of RAM below the 4GiB boundary,"
 "BIOS may not work with less than 1MiB", value);
 }
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 456dc9e9f0..975dfc848e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -131,7 +131,7 @@ static void pc_init1(MachineState *machine,
 if (lowmem > 0xc000) {
 lowmem = 0xc000;
 }
-if (lowmem & ((1ULL << 30) - 1)) {
+if (lowmem & ((1 * G_BYTE) - 1)) {
 warn_report("Large machine and max_ram_below_4g "
 "(%" PRIu64 ") not a multiple of 1G; "
 "possible bad performance.",
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index aba7541a82..79b84bc559 100644
--- a/hw/i386/pc_q35.c
+++