[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [PATCH v5 19/46] hw/s390x: Use the IEC binary prefix de
From: |
Cornelia Huck |
Subject: |
Re: [qemu-s390x] [PATCH v5 19/46] hw/s390x: Use the IEC binary prefix definitions |
Date: |
Mon, 25 Jun 2018 15:07:19 +0200 |
On Mon, 25 Jun 2018 09:42:11 -0300
Philippe Mathieu-Daudé <address@hidden> 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é <address@hidden>
> Reviewed-by: Thomas Huth <address@hidden>
> Acked-by: Cornelia Huck <address@hidden>
Hm, I had not looked at the v2+ patches, as this already carried my
ack...
> ---
> hw/s390x/s390-skeys.c | 3 ++-
> hw/s390x/s390-stattrib.c | 3 ++-
...but these two were added later on.
> hw/s390x/sclp.c | 3 ++-
> 3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> index 76241c240e..15f7ab0e53 100644
> --- a/hw/s390x/s390-skeys.c
> +++ b/hw/s390x/s390-skeys.c
> @@ -10,6 +10,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/units.h"
> #include "hw/boards.h"
> #include "hw/s390x/storage-keys.h"
> #include "qapi/error.h"
> @@ -19,7 +20,7 @@
> #include "sysemu/kvm.h"
> #include "migration/register.h"
>
> -#define S390_SKEYS_BUFFER_SIZE 131072 /* Room for 128k storage keys */
> +#define S390_SKEYS_BUFFER_SIZE (128 * KiB) /* Room for 128k storage keys */
This one looks confusing to me. We're not allocating 128 chunks of 1
KiB size, but space enough for 128k byte values. What do others think?
> #define S390_SKEYS_SAVE_FLAG_EOS 0x01
> #define S390_SKEYS_SAVE_FLAG_SKEYS 0x02
> #define S390_SKEYS_SAVE_FLAG_ERROR 0x04
> diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
> index 70b95550a8..5161a1659b 100644
> --- a/hw/s390x/s390-stattrib.c
> +++ b/hw/s390x/s390-stattrib.c
> @@ -10,6 +10,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/units.h"
> #include "hw/boards.h"
> #include "cpu.h"
> #include "migration/qemu-file.h"
> @@ -20,7 +21,7 @@
> #include "qapi/error.h"
> #include "qapi/qmp/qdict.h"
>
> -#define CMMA_BLOCK_SIZE (1 << 10)
> +#define CMMA_BLOCK_SIZE (1 * KiB)
This one looks fine.
>
> #define STATTR_FLAG_EOS 0x01ULL
> #define STATTR_FLAG_MORE 0x02ULL
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 047d577313..bd2a024efd 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -13,6 +13,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/units.h"
> #include "qapi/error.h"
> #include "cpu.h"
> #include "sysemu/sysemu.h"
> @@ -289,7 +290,7 @@ static void sclp_realize(DeviceState *dev, Error **errp)
> ret = s390_set_memory_limit(machine->maxram_size, &hw_limit);
> if (ret == -E2BIG) {
> error_setg(&err, "host supports a maximum of %" PRIu64 " GB",
> - hw_limit >> 30);
> + hw_limit / GiB);
> } else if (ret) {
> error_setg(&err, "setting the guest size failed");
> }