[Qemu-devel] [RFC 0/2] use sizes.h macros for power-of-two sizes

[Qemu-devel] [RFC 0/2] use sizes.h macros for power-of-two sizes

[Antony Pavlov]

[RFC 1/2] include/: import sizes.h from barebox
[RFC 2/2] hw/mips: use sizes.h macros

The sizes.h macros is a easy-to-read method of
power-of-two memory sizes representation. The sizes.h
macros are actively used in linux kernel and other
projects, so let's use them in QEMU too.

[Qemu-devel] [RFC 1/2] include/: import sizes.h from barebox

[Antony Pavlov]

The header file sizes.h is used in linux kernel,
barebox bootloader and u-boot bootloader. It provides
the short and easy-to-read names for power-of-two
numbers. The numbers like this are othen used
for memory range sizes.

This commits imports the sizes.h from barebox bootloader
source code (see
http://git.pengutronix.de/?p=barebox.git;a=blob;f=include/sizes.h)
but not his twin from linux as the barebox header file
is more neutral and has neither barebox mention
nor linux mention.

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 include/sizes.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 include/sizes.h

diff --git a/include/sizes.h b/include/sizes.h
new file mode 100644
index 0000000..aa0fbdc
--- /dev/null
+++ b/include/sizes.h
@@ -0,0 +1,48 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307	 USA
+ */
+/*  Size defintions
+ *  Copyright (C) ARM Limited 1998. All rights reserved.
+ */
+
+#ifndef __sizes_h
+#define __sizes_h			1
+
+/* handy sizes */
+#define SZ_1K				0x00000400
+#define SZ_4K				0x00001000
+#define SZ_8K				0x00002000
+#define SZ_16K				0x00004000
+#define SZ_32K				0x00008000
+#define SZ_64K				0x00010000
+#define SZ_128K				0x00020000
+#define SZ_256K				0x00040000
+#define SZ_512K				0x00080000
+
+#define SZ_1M				0x00100000
+#define SZ_2M				0x00200000
+#define SZ_4M				0x00400000
+#define SZ_8M				0x00800000
+#define SZ_16M				0x01000000
+#define SZ_32M				0x02000000
+#define SZ_64M				0x04000000
+#define SZ_128M				0x08000000
+#define SZ_256M				0x10000000
+#define SZ_512M				0x20000000
+
+#define SZ_1G				0x40000000
+#define SZ_2G				0x80000000
+
+#endif	/* __sizes_h */
-- 
1.8.4.rc3

[Qemu-devel] [RFC 2/2] hw/mips: use sizes.h macros

[Antony Pavlov]

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 hw/mips/mips_malta.c   | 3 ++-
 include/hw/mips/bios.h | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index ae0921c..7e8978b 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 "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
 
diff --git a/include/hw/mips/bios.h b/include/hw/mips/bios.h
index b4b88ac..e744425 100644
--- a/include/hw/mips/bios.h
+++ b/include/hw/mips/bios.h
@@ -1,6 +1,7 @@
 #include "cpu.h"
+#include "sizes.h"
 
-#define BIOS_SIZE (4 * 1024 * 1024)
+#define BIOS_SIZE SZ_4M
 #ifdef TARGET_WORDS_BIGENDIAN
 #define BIOS_FILENAME "mips_bios.bin"
 #else
-- 
1.8.4.rc3

Re: [Qemu-devel] [RFC 1/2] include/: import sizes.h from barebox

[Aurelien Jarno]

On Wed, Sep 04, 2013 at 11:35:27AM +0400, Antony Pavlov wrote:
> The header file sizes.h is used in linux kernel,
> barebox bootloader and u-boot bootloader. It provides
> the short and easy-to-read names for power-of-two
> numbers. The numbers like this are othen used
> for memory range sizes.
> 
> This commits imports the sizes.h from barebox bootloader
> source code (see
> http://git.pengutronix.de/?p=barebox.git;a=blob;f=include/sizes.h)
> but not his twin from linux as the barebox header file
> is more neutral and has neither barebox mention
> nor linux mention.
> 
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  include/sizes.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 include/sizes.h
> 
> diff --git a/include/sizes.h b/include/sizes.h
> new file mode 100644
> index 0000000..aa0fbdc
> --- /dev/null
> +++ b/include/sizes.h

I am not sure it is the correct location. Paolo, can you please look at that?

> @@ -0,0 +1,48 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307	 USA
> + */
> +/*  Size defintions
> + *  Copyright (C) ARM Limited 1998. All rights reserved.
> + */
> +
> +#ifndef __sizes_h
> +#define __sizes_h			1

I think this should be in upper case, prefixed with QEMU_ or _QEMU.

> +
> +/* handy sizes */
> +#define SZ_1K				0x00000400
> +#define SZ_4K				0x00001000
> +#define SZ_8K				0x00002000
> +#define SZ_16K				0x00004000
> +#define SZ_32K				0x00008000
> +#define SZ_64K				0x00010000
> +#define SZ_128K				0x00020000
> +#define SZ_256K				0x00040000
> +#define SZ_512K				0x00080000
> +
> +#define SZ_1M				0x00100000
> +#define SZ_2M				0x00200000
> +#define SZ_4M				0x00400000
> +#define SZ_8M				0x00800000
> +#define SZ_16M				0x01000000
> +#define SZ_32M				0x02000000
> +#define SZ_64M				0x04000000
> +#define SZ_128M				0x08000000
> +#define SZ_256M				0x10000000
> +#define SZ_512M				0x20000000
> +
> +#define SZ_1G				0x40000000
> +#define SZ_2G				0x80000000
> +
> +#endif	/* __sizes_h */



-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [RFC 2/2] hw/mips: use sizes.h macros

[Aurelien Jarno]

On Wed, Sep 04, 2013 at 11:35:28AM +0400, Antony Pavlov wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  hw/mips/mips_malta.c   | 3 ++-
>  include/hw/mips/bios.h | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index ae0921c..7e8978b 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 "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
>  
> diff --git a/include/hw/mips/bios.h b/include/hw/mips/bios.h
> index b4b88ac..e744425 100644
> --- a/include/hw/mips/bios.h
> +++ b/include/hw/mips/bios.h
> @@ -1,6 +1,7 @@
>  #include "cpu.h"
> +#include "sizes.h"
>  
> -#define BIOS_SIZE (4 * 1024 * 1024)
> +#define BIOS_SIZE SZ_4M
>  #ifdef TARGET_WORDS_BIGENDIAN
>  #define BIOS_FILENAME "mips_bios.bin"
>  #else

Besides the comments from the previous patch:

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [RFC 1/2] include/: import sizes.h from barebox

[Paolo Bonzini]

Il 09/09/2013 19:32, Aurelien Jarno ha scritto:
> On Wed, Sep 04, 2013 at 11:35:27AM +0400, Antony Pavlov wrote:
>> The header file sizes.h is used in linux kernel,
>> barebox bootloader and u-boot bootloader. It provides
>> the short and easy-to-read names for power-of-two
>> numbers. The numbers like this are othen used
>> for memory range sizes.
>>
>> This commits imports the sizes.h from barebox bootloader
>> source code (see
>> http://git.pengutronix.de/?p=barebox.git;a=blob;f=include/sizes.h)
>> but not his twin from linux as the barebox header file
>> is more neutral and has neither barebox mention
>> nor linux mention.
>>
>> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
>> ---
>>  include/sizes.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>  create mode 100644 include/sizes.h
>>
>> diff --git a/include/sizes.h b/include/sizes.h
>> new file mode 100644
>> index 0000000..aa0fbdc
>> --- /dev/null
>> +++ b/include/sizes.h
> 
> I am not sure it is the correct location. Paolo, can you please look at that?

Yeah, probably should be include/qemu.  (For a good reason---to not
conflict with /usr/include/sizes.h that anyone might have installed).

Paolo

>> @@ -0,0 +1,48 @@
>> +/*
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307	 USA
>> + */
>> +/*  Size defintions
>> + *  Copyright (C) ARM Limited 1998. All rights reserved.
>> + */
>> +
>> +#ifndef __sizes_h
>> +#define __sizes_h			1
> 
> I think this should be in upper case, prefixed with QEMU_ or _QEMU.
> 
>> +
>> +/* handy sizes */
>> +#define SZ_1K				0x00000400
>> +#define SZ_4K				0x00001000
>> +#define SZ_8K				0x00002000
>> +#define SZ_16K				0x00004000
>> +#define SZ_32K				0x00008000
>> +#define SZ_64K				0x00010000
>> +#define SZ_128K				0x00020000
>> +#define SZ_256K				0x00040000
>> +#define SZ_512K				0x00080000
>> +
>> +#define SZ_1M				0x00100000
>> +#define SZ_2M				0x00200000
>> +#define SZ_4M				0x00400000
>> +#define SZ_8M				0x00800000
>> +#define SZ_16M				0x01000000
>> +#define SZ_32M				0x02000000
>> +#define SZ_64M				0x04000000
>> +#define SZ_128M				0x08000000
>> +#define SZ_256M				0x10000000
>> +#define SZ_512M				0x20000000
>> +
>> +#define SZ_1G				0x40000000
>> +#define SZ_2G				0x80000000
>> +
>> +#endif	/* __sizes_h */
> 
> 
> 

Re: [Qemu-devel] [RFC 1/2] include/: import sizes.h from barebox

[Stefan Weil]

Am 09.09.2013 19:32, schrieb Aurelien Jarno:
> On Wed, Sep 04, 2013 at 11:35:27AM +0400, Antony Pavlov wrote:
>> @@ -0,0 +1,48 @@
>> +/*
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307	 USA
>> + */
>> +/*  Size defintions

Is this the original comment? Then someone should send them a fix :-)

>> + *  Copyright (C) ARM Limited 1998. All rights reserved.
>> + */
>> +
>> +#ifndef __sizes_h
>> +#define __sizes_h			1
> I think this should be in upper case, prefixed with QEMU_ or _QEMU.
>
>> +
>> +/* handy sizes */
>> +#define SZ_1K				0x00000400
>> +#define SZ_4K				0x00001000
>> +#define SZ_8K				0x00002000

What about replacing all tabs to spaces before importing code to QEMU?
This improves QEMU compatibility without requiring too much additional
efforts during initial import and later updates.

I'd really appreciate that.

Stefan

Re: [Qemu-devel] [RFC 1/2] include/: import sizes.h from barebox

[Richard Henderson]

On 09/09/2013 10:32 AM, Aurelien Jarno wrote:
>> +#define SZ_2G				0x80000000

At minimum this must have a U or LL suffix.

And if this one has the suffix, then for consistency the rest ought as well.


r~