From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56873) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yv1gY-00011J-Ew for qemu-devel@nongnu.org; Wed, 20 May 2015 06:57:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yv1gV-0005Ep-1x for qemu-devel@nongnu.org; Wed, 20 May 2015 06:57:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33675) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yv1gU-0005DM-Ps for qemu-devel@nongnu.org; Wed, 20 May 2015 06:57:10 -0400 Date: Wed, 20 May 2015 12:57:04 +0200 From: Igor Mammedov Message-ID: <20150520125704.6d4167cf@nial.brq.redhat.com> In-Reply-To: <1432095595-12968-1-git-send-email-zhaoshenglong@huawei.com> References: <1431595182-7552-15-git-send-email-zhaoshenglong@huawei.com> <1432095595-12968-1-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] [RESEND PATCH 14/23] 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, 20 May 2015 12:19:55 +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 > Reviewed-by: Igor Mammedov Please remove my RB in cases when changes in patch are not trivial. > --- > hw/acpi/aml-build.c | 112 ++++++++++++++++++++++++++++++++++++++++++++ > include/hw/acpi/aml-build.h | 1 + > 2 files changed, 113 insertions(+) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index 7fcc44e..bdcbad2 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -28,6 +28,8 @@ > #include "qemu/bswap.h" > #include "qemu/bitops.h" > #include "hw/acpi/bios-linker-loader.h" > +#include "qapi/error.h" > +#include "qemu-common.h" > > static GArray *build_alloc_array(void) > { > @@ -955,6 +957,116 @@ Aml *aml_qword_memory(AmlDecode dec, AmlMinFixed min_fixed, > addr_trans, len, flags); > } > > +static void String2Byte(const char *str, uint8_t *byte_list) > +{ > + unsigned long long val; > + char *end; > + const char *start = str; > + int i, j = 0; > + Error *err = NULL; > + > + if (strlen(str) != 36) { > + error_setg(&err, "Length of UUID string is not 36"); > + goto error; > + } > + > + val = strtoull(start, &end, 16); > + if ((end - start) != 8) { > + error_setg(&err, "Length of first part of UUID string is not 8"); > + goto error; > + } > + for (i = 3; i >= 0; i--) { > + byte_list[j] = extract32(val, (8 * i), 8); > + j++; > + } > + start = end + 1; > + > + val = strtoull(start, &end, 16); > + if ((end - start) != 4) { > + error_setg(&err, "Length of second part of UUID string is not 4"); > + goto error; > + } > + for (i = 1; i >= 0; i--) { > + byte_list[j] = extract32(val, (8 * i), 8); > + j++; > + } > + start = end + 1; > + > + val = strtoull(start, &end, 16); > + if ((end - start) != 4) { > + error_setg(&err, "Length of third part of UUID string is not 4"); > + goto error; > + } > + for (i = 1; i >= 0; i--) { > + byte_list[j] = extract32(val, (8 * i), 8); > + j++; > + } > + start = end + 1; > + > + val = strtoull(start, &end, 16); > + if ((end - start) != 4) { > + error_setg(&err, "Length of fourth part of UUID string is not 4"); > + goto error; > + } > + for (i = 1; i >= 0; i--) { > + byte_list[j] = extract32(val, (8 * i), 8); > + j++; > + } > + start = end + 1; > + > + val = strtoull(start, &end, 16); > + if ((end - start) != 12) { > + error_setg(&err, "Length of fifth part of UUID string is not 12"); > + goto error; > + } > + for (i = 5; i >= 0; i--) { > + byte_list[j] = extract64(val, (8 * i), 8); > + j++; > + } > + > + return; > + > +error: > + error_report_err(err); > + exit(1); > +} To me this looks just crazy, I find the previous patch much easier to read/understand maybe previous patch +separated asserts as was suggested an earlier. > + > +/* > + * ACPI 3.0: 17.5.124 ToUUID (Convert String to UUID Macro) > + * e.g. UUID: aabbccdd-eeff-gghh-iijj-kkllmmnnoopp > + * call aml_touuid("aabbccdd-eeff-gghh-iijj-kkllmmnnoopp"); > + */ > +Aml *aml_touuid(const char *uuid) > +{ > + Aml *var = aml_bundle(0x11 /* BufferOp */, AML_BUFFER); > + uint8_t byte_list[16]; > + > + String2Byte(uuid, byte_list); > + > + build_append_byte(var->buf, byte_list[3]); /* dd - at offset 00 */ > + build_append_byte(var->buf, byte_list[2]); /* cc - at offset 01 */ > + build_append_byte(var->buf, byte_list[1]); /* bb - at offset 02 */ > + build_append_byte(var->buf, byte_list[0]); /* aa - at offset 03 */ > + > + build_append_byte(var->buf, byte_list[5]); /* ff - at offset 04 */ > + build_append_byte(var->buf, byte_list[4]); /* ee - at offset 05 */ > + > + build_append_byte(var->buf, byte_list[7]); /* hh - at offset 06 */ > + build_append_byte(var->buf, byte_list[6]); /* gg - at offset 07 */ > + > + build_append_byte(var->buf, byte_list[8]); /* ii - at offset 08 */ > + build_append_byte(var->buf, byte_list[9]); /* jj - at offset 09 */ > + > + build_append_byte(var->buf, byte_list[10]); /* kk - at offset 10 */ > + build_append_byte(var->buf, byte_list[11]); /* ll - at offset 11 */ > + build_append_byte(var->buf, byte_list[12]); /* mm - at offset 12 */ > + build_append_byte(var->buf, byte_list[13]); /* nn - at offset 13 */ > + build_append_byte(var->buf, byte_list[14]); /* oo - at offset 14 */ > + build_append_byte(var->buf, byte_list[15]); /* pp - at offset 15 */ > + > + return var; > +} > + > 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 fac70ea..a873b46 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -257,6 +257,7 @@ Aml *aml_buffer(int buffer_size, uint8_t *byte_list); > Aml *aml_resource_template(void); > Aml *aml_field(const char *name, AmlAccessType type, AmlUpdateRule rule); > Aml *aml_varpackage(uint32_t num_elements); > +Aml *aml_touuid(const char *uuid); > > void > build_header(GArray *linker, GArray *table_data,