From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57832) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YmzPd-0006SO-Lp for qemu-devel@nongnu.org; Tue, 28 Apr 2015 02:54:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YmzPZ-0002lk-7W for qemu-devel@nongnu.org; Tue, 28 Apr 2015 02:54:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42618) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YmzPZ-0002l5-0c for qemu-devel@nongnu.org; Tue, 28 Apr 2015 02:54:29 -0400 Date: Tue, 28 Apr 2015 08:54:12 +0200 From: Igor Mammedov Message-ID: <20150428085412.5d215d01@nial.brq.redhat.com> In-Reply-To: <1429104309-3844-14-git-send-email-zhaoshenglong@huawei.com> References: <1429104309-3844-1-git-send-email-zhaoshenglong@huawei.com> <1429104309-3844-14-git-send-email-zhaoshenglong@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 13/20] hw/acpi/aml-build: Add ToUUID macro List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Shannon Zhao Cc: peter.maydell@linaro.org, hangaohuai@huawei.com, mst@redhat.com, a.spyridakis@virtualopensystems.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, peter.huangpeng@huawei.com, alex.bennee@linaro.org, hanjun.guo@linaro.org, pbonzini@redhat.com, lersek@redhat.com, christoffer.dall@linaro.org, shannon.zhao@linaro.org On Wed, 15 Apr 2015 21:25:02 +0800 Shannon Zhao wrote: > From: Shannon Zhao > > Add ToUUID macro, this is useful for generating PCIe ACPI table. > > Signed-off-by: Shannon Zhao > Signed-off-by: Shannon Zhao > --- > hw/acpi/aml-build.c | 40 ++++++++++++++++++++++++++++++++++++++++ > include/hw/acpi/aml-build.h | 1 + > 2 files changed, 41 insertions(+) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index b99bb13..316d5a5 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -25,6 +25,7 @@ > #include > #include > #include "hw/acpi/aml-build.h" > +#include "qemu-common.h" why do you need this hunk? > #include "qemu/bswap.h" > #include "qemu/bitops.h" > #include "hw/acpi/bios-linker-loader.h" > @@ -948,6 +949,45 @@ Aml *aml_qword_memory(AmlDecode dec, AmlMinFixed min_fixed, > addr_trans, len, flags); > } > > +/* > + * ACPI 3.0: 17.5.124 ToUUID (Convert String to UUID Macro) > + * e.g. UUID: E5C937D0-3553-4d7a-9117-EA4D19C3434D > + * call aml_touuid("E5C937D0-3553-4d7a-9117-EA4D19C3434D"); > + */ > +Aml *aml_touuid(const char *uuid) > +{ > + int i; > + long int val; unsigned long long int ??? > + char *end; > + const char *start = uuid; > + Aml *UUID = aml_buffer(); s/UUID/var/ > + > + val = strtol(start, &end, 16); may be use strtoull() > + g_assert((end - start) == 8); > + build_append_int_noprefix(UUID->buf, val, 4); > + start = end + 1; > + val = strtol(start, &end, 16); > + g_assert((end - start) == 4); > + build_append_int_noprefix(UUID->buf, val, 2); > + start = end + 1; > + val = strtol(start, &end, 16); > + g_assert((end - start) == 4); > + build_append_int_noprefix(UUID->buf, val, 2); this corresponds to -gghh- part of UUID according to spec it would be better if you add pattern mentioned in spec in this function and then put comments marking places which handle specific part of it. > + start = end + 1; > + val = strtol(start, &end, 16); > + g_assert((end - start) == 4); > + build_append_int_noprefix(UUID->buf, (val >> 8) & 0xFF, 1); > + build_append_int_noprefix(UUID->buf, val & 0xFF, 1); add comment to it that according to spec bytes here are flipped around that's why special treatment. > + start = end + 1; > + val = strtol(start, &end, 16); > + g_assert((end - start) == 12); > + for (i = 40; i >= 0; i -= 8) { > + build_append_int_noprefix(UUID->buf, (val >> i) & 0xFF, 1); > + } > + btw: whole thing might be simpler if an intermediate conversion is avoided, just pack buffer as in spec byte by byte: /* format: aabbccdd-eeff-gghh-iijj-kkllmmnnoopp */ assert(strlen(uuid) == ...); build_append_byte(var->buf, HEX2BYTE(uuid[3]); /* dd */ build_append_byte(var->buf, HEX2BYTE(uuid[2]); /* cc */ ... easy to validate just by looking at "UUID Buffer Format" table in spec > + return UUID; > +} > + > void > build_header(GArray *linker, GArray *table_data, > AcpiTableHeader *h, const char *sig, int len, uint8_t rev) > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > index d1b9fe7..b41fd0c 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -259,6 +259,7 @@ Aml *aml_buffer(void); > Aml *aml_resource_template(void); > Aml *aml_field(const char *name, AmlFieldFlags flags); > Aml *aml_varpackage(uint32_t num_elements); > +Aml *aml_touuid(const char *uuid); > > void > build_header(GArray *linker, GArray *table_data,