|
From: | Shannon Zhao |
Subject: | Re: [Qemu-devel] [RESEND PATCH 14/23] hw/acpi/aml-build: Add ToUUID macro |
Date: | Wed, 20 May 2015 21:46:35 +0800 |
User-agent: | Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
On 2015/5/20 21:41, Igor Mammedov wrote:
On Wed, 20 May 2015 19:02:12 +0800 Shannon Zhao <address@hidden> wrote:On 2015/5/20 18:57, Igor Mammedov wrote:On Wed, 20 May 2015 12:19:55 +0800 Shannon Zhao <address@hidden> wrote:From: Shannon Zhao <address@hidden> Add ToUUID macro, this is useful for generating PCIe ACPI table. Signed-off-by: Shannon Zhao <address@hidden> Signed-off-by: Shannon Zhao <address@hidden> Reviewed-by: Igor Mammedov <address@hidden>Please remove my RB in cases when changes in patch are not trivial.Ok, forgot this.--- 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.Michael suggests add a pre-parse function to parse the string to bytes list.As you see result is more complex than original approach, without clear benefits.
Yeah, let's go back then.
On 2015/5/19 16:37, Michael S. Tsirkin wrote:>>> Why doesn't that something else give us a pre-parse structure then?a pre-parse structure?Yes - have caller validate and parse the string, on failure report it to user cleanly, on success give us a byte array avoiding need to call Hex2Byte.+ +/* + * 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,.
-- Shannon
[Prev in Thread] | Current Thread | [Next in Thread] |