Message ID | 1432095595-12968-1-git-send-email-zhaoshenglong@huawei.com |
---|---|
State | New |
Headers | show |
On Wed, 20 May 2015 12:19:55 +0800 Shannon Zhao <zhaoshenglong@huawei.com> wrote: > From: Shannon Zhao <shannon.zhao@linaro.org> > > Add ToUUID macro, this is useful for generating PCIe ACPI table. > > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> > Reviewed-by: Igor Mammedov <imammedo@redhat.com> 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,
On 2015/5/20 18:57, Igor Mammedov wrote: > On Wed, 20 May 2015 12:19:55 +0800 > Shannon Zhao <zhaoshenglong@huawei.com> wrote: > >> From: Shannon Zhao <shannon.zhao@linaro.org> >> >> Add ToUUID macro, this is useful for generating PCIe ACPI table. >> >> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> >> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> >> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > 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. 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, > > > . >
On Wed, 20 May 2015 19:02:12 +0800 Shannon Zhao <zhaoshenglong@huawei.com> wrote: > > > On 2015/5/20 18:57, Igor Mammedov wrote: > > On Wed, 20 May 2015 12:19:55 +0800 > > Shannon Zhao <zhaoshenglong@huawei.com> wrote: > > > >> From: Shannon Zhao <shannon.zhao@linaro.org> > >> > >> Add ToUUID macro, this is useful for generating PCIe ACPI table. > >> > >> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> > >> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> > >> Reviewed-by: Igor Mammedov <imammedo@redhat.com> > > 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. > > 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, > > > > > > . > > >
On 2015/5/20 21:41, Igor Mammedov wrote: > On Wed, 20 May 2015 19:02:12 +0800 > Shannon Zhao <zhaoshenglong@huawei.com> wrote: > >> >> >> On 2015/5/20 18:57, Igor Mammedov wrote: >>> On Wed, 20 May 2015 12:19:55 +0800 >>> Shannon Zhao <zhaoshenglong@huawei.com> wrote: >>> >>>> From: Shannon Zhao <shannon.zhao@linaro.org> >>>> >>>> Add ToUUID macro, this is useful for generating PCIe ACPI table. >>>> >>>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com> >>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> >>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com> >>> 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, >>> >>> >>> . >>> >> >
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); +} + +/* + * 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,