From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48298) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YuNjD-0000Lp-DI for qemu-devel@nongnu.org; Mon, 18 May 2015 12:17:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YuNj9-00060l-3P for qemu-devel@nongnu.org; Mon, 18 May 2015 12:17:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43367) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YuNj8-0005zF-S1 for qemu-devel@nongnu.org; Mon, 18 May 2015 12:17:15 -0400 Date: Mon, 18 May 2015 18:17:02 +0200 From: "Michael S. Tsirkin" Message-ID: <20150518180850-mutt-send-email-mst@redhat.com> References: <1431595182-7552-1-git-send-email-zhaoshenglong@huawei.com> <1431595182-7552-15-git-send-email-zhaoshenglong@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1431595182-7552-15-git-send-email-zhaoshenglong@huawei.com> Subject: Re: [Qemu-devel] [PATCH v7 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, a.spyridakis@virtualopensystems.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, peter.huangpeng@huawei.com, alex.bennee@linaro.org, hanjun.guo@linaro.org, imammedo@redhat.com, pbonzini@redhat.com, lersek@redhat.com, christoffer.dall@linaro.org, shannon.zhao@linaro.org On Thu, May 14, 2015 at 05:19:33PM +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 | 46 +++++++++++++++++++++++++++++++++++++++++++++ > include/hw/acpi/aml-build.h | 1 + > 2 files changed, 47 insertions(+) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index 6197896..0e4d35f 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -956,6 +956,52 @@ Aml *aml_qword_memory(AmlDecode dec, AmlMinFixed min_fixed, > addr_trans, len, flags); > } > > +static uint8_t Hex2Byte(const char *src) > +{ > + int hi = Hex2Digit(*src++); > + int lo = Hex2Digit(*src); Just src[0] src[1] would be clearer. > + > + g_assert(((hi >= 0) && (hi <= 15)) && ((lo >= 0) && (lo <= 15))); Pls use simple assert everywhere, it does not matter and rest of acpi uses plain assert. > + return (hi << 4) | lo; > +} Really should be lower-case. We need to fix Hex2Digit too, but since Hex2Digit is broken, I'll let this one in and we can fix both with a patch on top. > + > +/* > + * 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); > + > + /* format: aabbccdd-eeff-gghh-iijj-kkllmmnnoopp */ This documents the argument, so really belongs near the function's head. > + g_assert((strlen(uuid) == 36) && (uuid[8] == '-') && (uuid[13] == '-') > + && (uuid[18] == '-') && (uuid[23] == '-')); Does something else validate the parameter? It's not friendly to handle errors by assert. Why doesn't that something else give us a pre-parse structure then? > + > + build_append_byte(var->buf, Hex2Byte(uuid + 6)); /* dd - at offset 00 */ > + build_append_byte(var->buf, Hex2Byte(uuid + 4)); /* cc - at offset 01 */ > + build_append_byte(var->buf, Hex2Byte(uuid + 2)); /* bb - at offset 02 */ > + build_append_byte(var->buf, Hex2Byte(uuid + 0)); /* aa - at offset 03 */ > + > + build_append_byte(var->buf, Hex2Byte(uuid + 11)); /* ff - at offset 04 */ > + build_append_byte(var->buf, Hex2Byte(uuid + 9)); /* ee - at offset 05 */ > + > + build_append_byte(var->buf, Hex2Byte(uuid + 16)); /* hh - at offset 06 */ > + build_append_byte(var->buf, Hex2Byte(uuid + 14)); /* gg - at offset 07 */ > + > + build_append_byte(var->buf, Hex2Byte(uuid + 19)); /* ii - at offset 08 */ > + build_append_byte(var->buf, Hex2Byte(uuid + 21)); /* jj - at offset 09 */ > + > + build_append_byte(var->buf, Hex2Byte(uuid + 24)); /* kk - at offset 10 */ > + build_append_byte(var->buf, Hex2Byte(uuid + 26)); /* ll - at offset 11 */ > + build_append_byte(var->buf, Hex2Byte(uuid + 28)); /* mm - at offset 12 */ > + build_append_byte(var->buf, Hex2Byte(uuid + 30)); /* nn - at offset 13 */ > + build_append_byte(var->buf, Hex2Byte(uuid + 32)); /* oo - at offset 14 */ > + build_append_byte(var->buf, Hex2Byte(uuid + 34)); /* pp - at offset 15 */ Can we use strtoull? > + > + 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 615f3c1..f5e764c 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -265,6 +265,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, > -- > 2.1.0 >