From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35592) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fXRLz-0002yn-Ca for qemu-devel@nongnu.org; Mon, 25 Jun 2018 09:16:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fXRLv-0003XW-CX for qemu-devel@nongnu.org; Mon, 25 Jun 2018 09:16:23 -0400 References: <20180625124238.25339-1-f4bug@amsat.org> <20180625124238.25339-20-f4bug@amsat.org> <20180625150719.0392da6c.cohuck@redhat.com> From: David Hildenbrand Message-ID: Date: Mon, 25 Jun 2018 15:16:15 +0200 MIME-Version: 1.0 In-Reply-To: <20180625150719.0392da6c.cohuck@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v5 19/46] hw/s390x: Use the IEC binary prefix definitions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= Cc: Thomas Huth , Stefan Weil , qemu-devel@nongnu.org, qemu-trivial@nongnu.org, Richard Henderson , Alexander Graf , Christian Borntraeger , "open list:S390" On 25.06.2018 15:07, Cornelia Huck wrote: > On Mon, 25 Jun 2018 09:42:11 -0300 > Philippe Mathieu-Daud=C3=A9 wrote: >=20 >> It eases code review, unit is explicit. >> >> Patch generated using: >> >> $ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ includ= e/hw/ >> >> and modified manually. >> >> Signed-off-by: Philippe Mathieu-Daud=C3=A9 >> Reviewed-by: Thomas Huth >> Acked-by: Cornelia Huck >=20 > Hm, I had not looked at the v2+ patches, as this already carried my > ack... >=20 >> --- >> hw/s390x/s390-skeys.c | 3 ++- >> hw/s390x/s390-stattrib.c | 3 ++- >=20 > ...but these two were added later on. >=20 >> 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 @@ >> */ >> =20 >> #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" >> =20 >> -#define S390_SKEYS_BUFFER_SIZE 131072 /* Room for 128k storage keys = */ >> +#define S390_SKEYS_BUFFER_SIZE (128 * KiB) /* Room for 128k storage = keys */ >=20 > 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? Hm, as this define is called "_SIZE" it should be the right thing to do. I would agree if it would be "_SKEY.*_COUNT" --=20 Thanks, David / dhildenb