On 2015/4/28 14:54, Igor Mammedov wrote:
> On Wed, 15 Apr 2015 21:25:02 +0800
> Shannon Zhao <zhaoshengl...@huawei.com> wrote:
>
>> From: Shannon Zhao <shannon.z...@linaro.org>
>>
>> Add ToUUID macro, this is useful for generating PCIe ACPI table.
>>
>> Signed-off-by: Shannon Zhao <zhaoshengl...@huawei.com>
>> Signed-off-by: Shannon Zhao <shannon.z...@linaro.org>
>> ---
>> 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 <stdbool.h>
>> #include <string.h>
>> #include "hw/acpi/aml-build.h"
>> +#include "qemu-common.h"
> why do you need this hunk?
>
strtol needs stdlib.h. I should include it directly rather than
including qemu-common.h.
>> #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 */
use build_append_byte(var->buf, HEX2BYTE(uuid + 7); ?
> build_append_byte(var->buf, HEX2BYTE(uuid[2]); /* cc */
use build_append_byte(var->buf, HEX2BYTE(uuid + 5); ?
> ...
>
> 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,
>
>
> .
>
