diff mbox

[v5,13/20] hw/acpi/aml-build: Add ToUUID macro

Message ID 1429104309-3844-14-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao April 15, 2015, 1:25 p.m. UTC 
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>
---
 hw/acpi/aml-build.c         | 40 ++++++++++++++++++++++++++++++++++++++++
 include/hw/acpi/aml-build.h |  1 +
 2 files changed, 41 insertions(+)

Comments

Shannon Zhao April 29, 2015, 1:41 p.m. UTC | #1 
On 2015/4/28 17:48, Shannon Zhao wrote:
> On 2015/4/28 17:35, Igor Mammedov wrote:
>> On Tue, 28 Apr 2015 16:52:19 +0800
>> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>>
>>> On 2015/4/28 16:15, Igor Mammedov wrote:
>>>>>> 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); ?
>>>> if you mean hyphens /-/ then they are not encoded,
>>>> but you surely can add checks for them to make sure that
>>>> UUID format is as expected.
>>>>
>>>
>>> I mean uuid[3] points to b not dd. Maybe use following way:
>>>
>>> static uint8_t Hex2Byte(char *src)
>> or even better:
>> Hex2Byte(char *src, byte_idx)
>>    and do pointer arithmetic inside
>>
>> [...]
>>> build_append_byte(var->buf, Hex2Byte(uuid + (3 * 2))); /* dd */
>> build_append_byte(var->buf, Hex2Byte(uuid, 3)); /* dd - at offset 00 */
>> build_append_byte(var->buf, Hex2Byte(uuid, 2)); /* cc - at offset 01 */
>> ...
>>
> Yes, it's better to first four bytes. But there are hyphens /-/, for
> offset 04, 05 and etc it doesn't work. We can't use following expression:
>
> build_append_byte(var->buf, Hex2Byte(uuid, 5)); /* ff - at offset 04 */
> build_append_byte(var->buf, Hex2Byte(uuid, 4)); /* ee - at offset 05 */
> ...
>
>

So about the implementation of this macro, I think I'd like to use 
following approach. This lets Hex2Byte do what it should only do and 
still has a clear look of UUID. What do you think about?

static uint8_t Hex2Byte(char *src)
{
     int hi = Hex2Digit(*src++);
     int lo = Hex2Digit(*src);

     if ((hi < 0) || (lo < 0))
         return -1;

     return (hi << 4) | lo;
}

g_assert((strlen(uuid) == 36) && (uuid[8] == '-') && (uuid[13] == '-')
           && (uuid[18] == '-') && (uuid[23] == '-'));

build_append_byte(var->buf, Hex2Byte(uuid + (3 * 2))); /* dd */
build_append_byte(var->buf, Hex2Byte(uuid + (2 * 2))); /* cc */
build_append_byte(var->buf, Hex2Byte(uuid + (1 * 2))); /* bb */
build_append_byte(var->buf, Hex2Byte(uuid + (0 * 2))); /* aa */

build_append_byte(var->buf, Hex2Byte(uuid + (5 * 2 + 1))); /* ff */
build_append_byte(var->buf, Hex2Byte(uuid + (4 * 2 + 1))); /* ee */

build_append_byte(var->buf, Hex2Byte(uuid + (7 * 2 + 2))); /* hh */
build_append_byte(var->buf, Hex2Byte(uuid + (6 * 2 + 2))); /* gg */

build_append_byte(var->buf, Hex2Byte(uuid + (8 * 2 + 3))); /* ii */
build_append_byte(var->buf, Hex2Byte(uuid + (9 * 2 + 3))); /* jj */

build_append_byte(var->buf, Hex2Byte(uuid + (10 * 2 + 4))); /* kk */
build_append_byte(var->buf, Hex2Byte(uuid + (11 * 2 + 4))); /* ll */
build_append_byte(var->buf, Hex2Byte(uuid + (12 * 2 + 4))); /* mm */
build_append_byte(var->buf, Hex2Byte(uuid + (13 * 2 + 4))); /* nn */
build_append_byte(var->buf, Hex2Byte(uuid + (14 * 2 + 4))); /* oo */
build_append_byte(var->buf, Hex2Byte(uuid + (15 * 2 + 4))); /* pp */
Shannon Zhao May 4, 2015, 9:30 a.m. UTC | #2 
On 2015/5/4 17:22, Igor Mammedov wrote:
> On Wed, 29 Apr 2015 21:41:39 +0800
> Shannon Zhao <shannon.zhao@linaro.org> wrote:
> 
>>
>>
>> On 2015/4/28 17:48, Shannon Zhao wrote:
>>> On 2015/4/28 17:35, Igor Mammedov wrote:
>>>> On Tue, 28 Apr 2015 16:52:19 +0800
>>>> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>>>>
>>>>> On 2015/4/28 16:15, Igor Mammedov wrote:
>>>>>>>> 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); ?
>>>>>> if you mean hyphens /-/ then they are not encoded,
>>>>>> but you surely can add checks for them to make sure that
>>>>>> UUID format is as expected.
>>>>>>
>>>>>
>>>>> I mean uuid[3] points to b not dd. Maybe use following way:
>>>>>
>>>>> static uint8_t Hex2Byte(char *src)
>>>> or even better:
>>>> Hex2Byte(char *src, byte_idx)
>>>>    and do pointer arithmetic inside
>>>>
>>>> [...]
>>>>> build_append_byte(var->buf, Hex2Byte(uuid + (3 * 2))); /* dd */
>>>> build_append_byte(var->buf, Hex2Byte(uuid, 3)); /* dd - at offset 00 */
>>>> build_append_byte(var->buf, Hex2Byte(uuid, 2)); /* cc - at offset 01 */
>>>> ...
>>>>
>>> Yes, it's better to first four bytes. But there are hyphens /-/, for
>>> offset 04, 05 and etc it doesn't work. We can't use following expression:
>>>
>>> build_append_byte(var->buf, Hex2Byte(uuid, 5)); /* ff - at offset 04 */
>>> build_append_byte(var->buf, Hex2Byte(uuid, 4)); /* ee - at offset 05 */
>>> ...
>>>
>>>
>>
>> So about the implementation of this macro, I think I'd like to use 
>> following approach. This lets Hex2Byte do what it should only do and 
>> still has a clear look of UUID. What do you think about?
>>
>> static uint8_t Hex2Byte(char *src)
>> {
>>      int hi = Hex2Digit(*src++);
>>      int lo = Hex2Digit(*src);
>>
>>      if ((hi < 0) || (lo < 0))
>>          return -1;
> just make Hex2Digit() assert on wrong input.
> 

Ok.

>>
>>      return (hi << 4) | lo;
>> }
>>
>> g_assert((strlen(uuid) == 36) && (uuid[8] == '-') && (uuid[13] == '-')
>>            && (uuid[18] == '-') && (uuid[23] == '-'));
>>
>> build_append_byte(var->buf, Hex2Byte(uuid + (3 * 2))); /* dd */
>                                                ^^^^^
> I'd make it one number, instead of forcing reader to do math
> every time he/she looks at this code.
> 

Ok, will do this. And about other patches in this series could you offer
your comments?

Thanks,
Shannon

> 
>> build_append_byte(var->buf, Hex2Byte(uuid + (2 * 2))); /* cc */
>> build_append_byte(var->buf, Hex2Byte(uuid + (1 * 2))); /* bb */
>> build_append_byte(var->buf, Hex2Byte(uuid + (0 * 2))); /* aa */
>>
>> build_append_byte(var->buf, Hex2Byte(uuid + (5 * 2 + 1))); /* ff */
>> build_append_byte(var->buf, Hex2Byte(uuid + (4 * 2 + 1))); /* ee */
>>
>> build_append_byte(var->buf, Hex2Byte(uuid + (7 * 2 + 2))); /* hh */
>> build_append_byte(var->buf, Hex2Byte(uuid + (6 * 2 + 2))); /* gg */
>>
>> build_append_byte(var->buf, Hex2Byte(uuid + (8 * 2 + 3))); /* ii */
>> build_append_byte(var->buf, Hex2Byte(uuid + (9 * 2 + 3))); /* jj */
>>
>> build_append_byte(var->buf, Hex2Byte(uuid + (10 * 2 + 4))); /* kk */
>> build_append_byte(var->buf, Hex2Byte(uuid + (11 * 2 + 4))); /* ll */
>> build_append_byte(var->buf, Hex2Byte(uuid + (12 * 2 + 4))); /* mm */
>> build_append_byte(var->buf, Hex2Byte(uuid + (13 * 2 + 4))); /* nn */
>> build_append_byte(var->buf, Hex2Byte(uuid + (14 * 2 + 4))); /* oo */
>> build_append_byte(var->buf, Hex2Byte(uuid + (15 * 2 + 4))); /* pp */
>
diff mbox

Patch

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"
 #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;
+    char *end;
+    const char *start = uuid;
+    Aml *UUID = aml_buffer();
+
+    val = strtol(start, &end, 16);
+    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);
+    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);
+    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);
+    }
+
+    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,