diff mbox

[v6,6/7] tests: Move reusable ACPI macros into a new header file

Message ID 96c621a0c1588234e473bf0c0d068af8812f3225.1487139038.git.ben@skyportsystems.com
State New
Headers show

Commit Message

ben@skyportsystems.com Feb. 15, 2017, 6:15 a.m. UTC 
From: Ben Warren <ben@skyportsystems.com>

Also usable by upcoming VM Generation ID tests

Signed-off-by: Ben Warren <ben@skyportsystems.com>
---
 tests/acpi-utils.h       | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
 tests/bios-tables-test.c | 72 +---------------------------------------------
 2 files changed, 76 insertions(+), 71 deletions(-)
 create mode 100644 tests/acpi-utils.h

Comments

Igor Mammedov Feb. 15, 2017, 12:54 p.m. UTC | #1 
On Tue, 14 Feb 2017 22:15:48 -0800
ben@skyportsystems.com wrote:

> From: Ben Warren <ben@skyportsystems.com>
> 
> Also usable by upcoming VM Generation ID tests
> 
> Signed-off-by: Ben Warren <ben@skyportsystems.com>
> ---
>  tests/acpi-utils.h       | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/bios-tables-test.c | 72 +---------------------------------------------
>  2 files changed, 76 insertions(+), 71 deletions(-)
>  create mode 100644 tests/acpi-utils.h
> 
> diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
> new file mode 100644
> index 0000000..d5e5eff
> --- /dev/null
> +++ b/tests/acpi-utils.h
> @@ -0,0 +1,75 @@
> +#ifndef TEST_ACPI_UTILS_H
> +#define TEST_ACPI_UTILS_H
here should be includes for types/functions used within this file

> +
> +/* DSDT and SSDTs format */
> +typedef struct {
> +    AcpiTableHeader header;
> +    gchar *aml;            /* aml bytecode from guest */
> +    gsize aml_len;
> +    gchar *aml_file;
> +    gchar *asl;            /* asl code generated from aml */
> +    gsize asl_len;
> +    gchar *asl_file;
> +    bool tmp_files_retain;   /* do not delete the temp asl/aml */
> +} QEMU_PACKED AcpiSdtTable;
> +
> +#define ACPI_READ_FIELD(field, addr)           \
> +    do {                                       \
> +        switch (sizeof(field)) {               \
> +        case 1:                                \
> +            field = readb(addr);               \
> +            break;                             \
> +        case 2:                                \
> +            field = readw(addr);               \
> +            break;                             \
> +        case 4:                                \
> +            field = readl(addr);               \
> +            break;                             \
> +        case 8:                                \
> +            field = readq(addr);               \
> +            break;                             \
> +        default:                               \
> +            g_assert(false);                   \
> +        }                                      \
> +        addr += sizeof(field);                  \
> +    } while (0);
> +
> +#define ACPI_READ_ARRAY_PTR(arr, length, addr)  \
> +    do {                                        \
> +        int idx;                                \
> +        for (idx = 0; idx < length; ++idx) {    \
> +            ACPI_READ_FIELD(arr[idx], addr);    \
> +        }                                       \
> +    } while (0);
> +
> +#define ACPI_READ_ARRAY(arr, addr)                               \
> +    ACPI_READ_ARRAY_PTR(arr, sizeof(arr) / sizeof(arr[0]), addr)
> +
> +#define ACPI_READ_TABLE_HEADER(table, addr)                      \
> +    do {                                                         \
> +        ACPI_READ_FIELD((table)->signature, addr);               \
> +        ACPI_READ_FIELD((table)->length, addr);                  \
> +        ACPI_READ_FIELD((table)->revision, addr);                \
> +        ACPI_READ_FIELD((table)->checksum, addr);                \
> +        ACPI_READ_ARRAY((table)->oem_id, addr);                  \
> +        ACPI_READ_ARRAY((table)->oem_table_id, addr);            \
> +        ACPI_READ_FIELD((table)->oem_revision, addr);            \
> +        ACPI_READ_ARRAY((table)->asl_compiler_id, addr);         \
> +        ACPI_READ_FIELD((table)->asl_compiler_revision, addr);   \
> +    } while (0);
> +
> +#define ACPI_ASSERT_CMP(actual, expected) do { \
> +    uint32_t ACPI_ASSERT_CMP_le = cpu_to_le32(actual); \
> +    char ACPI_ASSERT_CMP_str[5] = {}; \
> +    memcpy(ACPI_ASSERT_CMP_str, &ACPI_ASSERT_CMP_le, 4); \
> +    g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
> +} while (0)
> +
> +#define ACPI_ASSERT_CMP64(actual, expected) do { \
> +    uint64_t ACPI_ASSERT_CMP_le = cpu_to_le64(actual); \
> +    char ACPI_ASSERT_CMP_str[9] = {}; \
> +    memcpy(ACPI_ASSERT_CMP_str, &ACPI_ASSERT_CMP_le, 8); \
> +    g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
> +} while (0)
> +
> +#endif  /* TEST_ACPI_UTILS_H */
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 5404805..c642f7f 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -17,6 +17,7 @@
>  #include "hw/acpi/acpi-defs.h"
>  #include "hw/smbios/smbios.h"
>  #include "qemu/bitmap.h"
> +#include "acpi-utils.h"
>  #include "boot-sector.h"
>  
>  #define MACHINE_PC "pc"
> @@ -24,18 +25,6 @@
>  
>  #define ACPI_REBUILD_EXPECTED_AML "TEST_ACPI_REBUILD_AML"
>  
> -/* DSDT and SSDTs format */
> -typedef struct {
> -    AcpiTableHeader header;
> -    gchar *aml;            /* aml bytecode from guest */
> -    gsize aml_len;
> -    gchar *aml_file;
> -    gchar *asl;            /* asl code generated from aml */
> -    gsize asl_len;
> -    gchar *asl_file;
> -    bool tmp_files_retain;   /* do not delete the temp asl/aml */
> -} QEMU_PACKED AcpiSdtTable;
> -
>  typedef struct {
>      const char *machine;
>      const char *variant;
> @@ -53,65 +42,6 @@ typedef struct {
>      int required_struct_types_len;
>  } test_data;
>  
> -#define ACPI_READ_FIELD(field, addr)           \
> -    do {                                       \
> -        switch (sizeof(field)) {               \
> -        case 1:                                \
> -            field = readb(addr);               \
> -            break;                             \
> -        case 2:                                \
> -            field = readw(addr);               \
> -            break;                             \
> -        case 4:                                \
> -            field = readl(addr);               \
> -            break;                             \
> -        case 8:                                \
> -            field = readq(addr);               \
> -            break;                             \
> -        default:                               \
> -            g_assert(false);                   \
> -        }                                      \
> -        addr += sizeof(field);                  \
> -    } while (0);
> -
> -#define ACPI_READ_ARRAY_PTR(arr, length, addr)  \
> -    do {                                        \
> -        int idx;                                \
> -        for (idx = 0; idx < length; ++idx) {    \
> -            ACPI_READ_FIELD(arr[idx], addr);    \
> -        }                                       \
> -    } while (0);
> -
> -#define ACPI_READ_ARRAY(arr, addr)                               \
> -    ACPI_READ_ARRAY_PTR(arr, sizeof(arr)/sizeof(arr[0]), addr)
> -
> -#define ACPI_READ_TABLE_HEADER(table, addr)                      \
> -    do {                                                         \
> -        ACPI_READ_FIELD((table)->signature, addr);               \
> -        ACPI_READ_FIELD((table)->length, addr);                  \
> -        ACPI_READ_FIELD((table)->revision, addr);                \
> -        ACPI_READ_FIELD((table)->checksum, addr);                \
> -        ACPI_READ_ARRAY((table)->oem_id, addr);                  \
> -        ACPI_READ_ARRAY((table)->oem_table_id, addr);            \
> -        ACPI_READ_FIELD((table)->oem_revision, addr);            \
> -        ACPI_READ_ARRAY((table)->asl_compiler_id, addr);         \
> -        ACPI_READ_FIELD((table)->asl_compiler_revision, addr);   \
> -    } while (0);
> -
> -#define ACPI_ASSERT_CMP(actual, expected) do { \
> -    uint32_t ACPI_ASSERT_CMP_le = cpu_to_le32(actual); \
> -    char ACPI_ASSERT_CMP_str[5] = {}; \
> -    memcpy(ACPI_ASSERT_CMP_str, &ACPI_ASSERT_CMP_le, 4); \
> -    g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
> -} while (0)
> -
> -#define ACPI_ASSERT_CMP64(actual, expected) do { \
> -    uint64_t ACPI_ASSERT_CMP_le = cpu_to_le64(actual); \
> -    char ACPI_ASSERT_CMP_str[9] = {}; \
> -    memcpy(ACPI_ASSERT_CMP_str, &ACPI_ASSERT_CMP_le, 8); \
> -    g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
> -} while (0)
> -
>  static char disk[] = "tests/acpi-test-disk-XXXXXX";
>  static const char *data_dir = "tests/acpi-test-data";
>  #ifdef CONFIG_IASL
Eric Blake Feb. 15, 2017, 9:35 p.m. UTC | #2 
On 02/15/2017 12:15 AM, ben@skyportsystems.com wrote:
> From: Ben Warren <ben@skyportsystems.com>
> 
> Also usable by upcoming VM Generation ID tests
> 
> Signed-off-by: Ben Warren <ben@skyportsystems.com>
> ---
>  tests/acpi-utils.h       | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/bios-tables-test.c | 72 +---------------------------------------------
>  2 files changed, 76 insertions(+), 71 deletions(-)
>  create mode 100644 tests/acpi-utils.h
> 
> diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
> new file mode 100644
> index 0000000..d5e5eff
> --- /dev/null
> +++ b/tests/acpi-utils.h
> @@ -0,0 +1,75 @@
> +#ifndef TEST_ACPI_UTILS_H
> +#define TEST_ACPI_UTILS_H

No copyright blurb? Also, does MAINTAINERS need an update to cover the
new file?
ben@skyportsystems.com Feb. 15, 2017, 9:58 p.m. UTC | #3 
> On Feb 15, 2017, at 1:35 PM, Eric Blake <eblake@redhat.com> wrote:
> 
> On 02/15/2017 12:15 AM, ben@skyportsystems.com <mailto:ben@skyportsystems.com> wrote:
>> From: Ben Warren <ben@skyportsystems.com>
>> 
>> Also usable by upcoming VM Generation ID tests
>> 
>> Signed-off-by: Ben Warren <ben@skyportsystems.com>
>> ---
>> tests/acpi-utils.h       | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
>> tests/bios-tables-test.c | 72 +---------------------------------------------
>> 2 files changed, 76 insertions(+), 71 deletions(-)
>> create mode 100644 tests/acpi-utils.h
>> 
>> diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
>> new file mode 100644
>> index 0000000..d5e5eff
>> --- /dev/null
>> +++ b/tests/acpi-utils.h
>> @@ -0,0 +1,75 @@
>> +#ifndef TEST_ACPI_UTILS_H
>> +#define TEST_ACPI_UTILS_H
> 
> No copyright blurb? Also, does MAINTAINERS need an update to cover the
> new file?
> 
Sure, I didn’t realize the header files all have copyright headers.  As for MAINTAINERS, do you mean I should add a device entry for vmgenid?

thanks,
Ben
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org <http://libvirt.org/>
Eric Blake Feb. 15, 2017, 10:56 p.m. UTC | #4 
On 02/15/2017 03:58 PM, Ben Warren wrote:

>> 
>> ---
>> tests/acpi-utils.h       | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
>> tests/bios-tables-test.c | 72 +---------------------------------------------
>> 2 files changed, 76 insertions(+), 71 deletions(-)


>> No copyright blurb? Also, does MAINTAINERS need an update to cover the
>> new file?
>>
> Sure, I didn’t realize the header files all have copyright headers.  As for MAINTAINERS, do you mean I should add a device entry for vmgenid?

In this patch, you're just refactoring to a new tests/acpi-utils.h, so
I'd normally suggest adding it to the blurb that owns
tests/bios-tables-test.c - but as a pre-existing problem, that also has
no listed maintainer.  we're trying to ensure that all new added files
have something listed in MAINTAINERS, even if it is in a misc section
that only emails the list, although it's harder to say what maintainer
to use for existing files that you are merely touching, and failure to
list a maintainer is not (yet) a hard failure (although there have been
patches proposed to scripts/checkpatch.pl to tighten the rules).

A new section for vmgenid might not be a bad idea, especially it if
covers more files than just the one addition I noticed in this patch.
ben@skyportsystems.com Feb. 15, 2017, 11:05 p.m. UTC | #5 
> On Feb 15, 2017, at 2:56 PM, Eric Blake <eblake@redhat.com> wrote:
> 
> On 02/15/2017 03:58 PM, Ben Warren wrote:
> 
>>> 
>>> ---
>>> tests/acpi-utils.h       | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> tests/bios-tables-test.c | 72 +---------------------------------------------
>>> 2 files changed, 76 insertions(+), 71 deletions(-)
> 
> 
>>> No copyright blurb? Also, does MAINTAINERS need an update to cover the
>>> new file?
>>> 
>> Sure, I didn’t realize the header files all have copyright headers.  As for MAINTAINERS, do you mean I should add a device entry for vmgenid?
> 
> In this patch, you're just refactoring to a new tests/acpi-utils.h, so
> I'd normally suggest adding it to the blurb that owns
> tests/bios-tables-test.c - but as a pre-existing problem, that also has
> no listed maintainer.  we're trying to ensure that all new added files
> have something listed in MAINTAINERS, even if it is in a misc section
> that only emails the list, although it's harder to say what maintainer
> to use for existing files that you are merely touching, and failure to
> list a maintainer is not (yet) a hard failure (although there have been
> patches proposed to scripts/checkpatch.pl to tighten the rules).
> 
> A new section for vmgenid might not be a bad idea, especially it if
> covers more files than just the one addition I noticed in this patch.
> 
Thank you for clarifying.  I’ll take care of it.
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 
—Ben
diff mbox

Patch

diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
new file mode 100644
index 0000000..d5e5eff
--- /dev/null
+++ b/tests/acpi-utils.h
@@ -0,0 +1,75 @@ 
+#ifndef TEST_ACPI_UTILS_H
+#define TEST_ACPI_UTILS_H
+
+/* DSDT and SSDTs format */
+typedef struct {
+    AcpiTableHeader header;
+    gchar *aml;            /* aml bytecode from guest */
+    gsize aml_len;
+    gchar *aml_file;
+    gchar *asl;            /* asl code generated from aml */
+    gsize asl_len;
+    gchar *asl_file;
+    bool tmp_files_retain;   /* do not delete the temp asl/aml */
+} QEMU_PACKED AcpiSdtTable;
+
+#define ACPI_READ_FIELD(field, addr)           \
+    do {                                       \
+        switch (sizeof(field)) {               \
+        case 1:                                \
+            field = readb(addr);               \
+            break;                             \
+        case 2:                                \
+            field = readw(addr);               \
+            break;                             \
+        case 4:                                \
+            field = readl(addr);               \
+            break;                             \
+        case 8:                                \
+            field = readq(addr);               \
+            break;                             \
+        default:                               \
+            g_assert(false);                   \
+        }                                      \
+        addr += sizeof(field);                  \
+    } while (0);
+
+#define ACPI_READ_ARRAY_PTR(arr, length, addr)  \
+    do {                                        \
+        int idx;                                \
+        for (idx = 0; idx < length; ++idx) {    \
+            ACPI_READ_FIELD(arr[idx], addr);    \
+        }                                       \
+    } while (0);
+
+#define ACPI_READ_ARRAY(arr, addr)                               \
+    ACPI_READ_ARRAY_PTR(arr, sizeof(arr) / sizeof(arr[0]), addr)
+
+#define ACPI_READ_TABLE_HEADER(table, addr)                      \
+    do {                                                         \
+        ACPI_READ_FIELD((table)->signature, addr);               \
+        ACPI_READ_FIELD((table)->length, addr);                  \
+        ACPI_READ_FIELD((table)->revision, addr);                \
+        ACPI_READ_FIELD((table)->checksum, addr);                \
+        ACPI_READ_ARRAY((table)->oem_id, addr);                  \
+        ACPI_READ_ARRAY((table)->oem_table_id, addr);            \
+        ACPI_READ_FIELD((table)->oem_revision, addr);            \
+        ACPI_READ_ARRAY((table)->asl_compiler_id, addr);         \
+        ACPI_READ_FIELD((table)->asl_compiler_revision, addr);   \
+    } while (0);
+
+#define ACPI_ASSERT_CMP(actual, expected) do { \
+    uint32_t ACPI_ASSERT_CMP_le = cpu_to_le32(actual); \
+    char ACPI_ASSERT_CMP_str[5] = {}; \
+    memcpy(ACPI_ASSERT_CMP_str, &ACPI_ASSERT_CMP_le, 4); \
+    g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
+} while (0)
+
+#define ACPI_ASSERT_CMP64(actual, expected) do { \
+    uint64_t ACPI_ASSERT_CMP_le = cpu_to_le64(actual); \
+    char ACPI_ASSERT_CMP_str[9] = {}; \
+    memcpy(ACPI_ASSERT_CMP_str, &ACPI_ASSERT_CMP_le, 8); \
+    g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
+} while (0)
+
+#endif  /* TEST_ACPI_UTILS_H */
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 5404805..c642f7f 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -17,6 +17,7 @@ 
 #include "hw/acpi/acpi-defs.h"
 #include "hw/smbios/smbios.h"
 #include "qemu/bitmap.h"
+#include "acpi-utils.h"
 #include "boot-sector.h"
 
 #define MACHINE_PC "pc"
@@ -24,18 +25,6 @@ 
 
 #define ACPI_REBUILD_EXPECTED_AML "TEST_ACPI_REBUILD_AML"
 
-/* DSDT and SSDTs format */
-typedef struct {
-    AcpiTableHeader header;
-    gchar *aml;            /* aml bytecode from guest */
-    gsize aml_len;
-    gchar *aml_file;
-    gchar *asl;            /* asl code generated from aml */
-    gsize asl_len;
-    gchar *asl_file;
-    bool tmp_files_retain;   /* do not delete the temp asl/aml */
-} QEMU_PACKED AcpiSdtTable;
-
 typedef struct {
     const char *machine;
     const char *variant;
@@ -53,65 +42,6 @@  typedef struct {
     int required_struct_types_len;
 } test_data;
 
-#define ACPI_READ_FIELD(field, addr)           \
-    do {                                       \
-        switch (sizeof(field)) {               \
-        case 1:                                \
-            field = readb(addr);               \
-            break;                             \
-        case 2:                                \
-            field = readw(addr);               \
-            break;                             \
-        case 4:                                \
-            field = readl(addr);               \
-            break;                             \
-        case 8:                                \
-            field = readq(addr);               \
-            break;                             \
-        default:                               \
-            g_assert(false);                   \
-        }                                      \
-        addr += sizeof(field);                  \
-    } while (0);
-
-#define ACPI_READ_ARRAY_PTR(arr, length, addr)  \
-    do {                                        \
-        int idx;                                \
-        for (idx = 0; idx < length; ++idx) {    \
-            ACPI_READ_FIELD(arr[idx], addr);    \
-        }                                       \
-    } while (0);
-
-#define ACPI_READ_ARRAY(arr, addr)                               \
-    ACPI_READ_ARRAY_PTR(arr, sizeof(arr)/sizeof(arr[0]), addr)
-
-#define ACPI_READ_TABLE_HEADER(table, addr)                      \
-    do {                                                         \
-        ACPI_READ_FIELD((table)->signature, addr);               \
-        ACPI_READ_FIELD((table)->length, addr);                  \
-        ACPI_READ_FIELD((table)->revision, addr);                \
-        ACPI_READ_FIELD((table)->checksum, addr);                \
-        ACPI_READ_ARRAY((table)->oem_id, addr);                  \
-        ACPI_READ_ARRAY((table)->oem_table_id, addr);            \
-        ACPI_READ_FIELD((table)->oem_revision, addr);            \
-        ACPI_READ_ARRAY((table)->asl_compiler_id, addr);         \
-        ACPI_READ_FIELD((table)->asl_compiler_revision, addr);   \
-    } while (0);
-
-#define ACPI_ASSERT_CMP(actual, expected) do { \
-    uint32_t ACPI_ASSERT_CMP_le = cpu_to_le32(actual); \
-    char ACPI_ASSERT_CMP_str[5] = {}; \
-    memcpy(ACPI_ASSERT_CMP_str, &ACPI_ASSERT_CMP_le, 4); \
-    g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
-} while (0)
-
-#define ACPI_ASSERT_CMP64(actual, expected) do { \
-    uint64_t ACPI_ASSERT_CMP_le = cpu_to_le64(actual); \
-    char ACPI_ASSERT_CMP_str[9] = {}; \
-    memcpy(ACPI_ASSERT_CMP_str, &ACPI_ASSERT_CMP_le, 8); \
-    g_assert_cmpstr(ACPI_ASSERT_CMP_str, ==, expected); \
-} while (0)
-
 static char disk[] = "tests/acpi-test-disk-XXXXXX";
 static const char *data_dir = "tests/acpi-test-data";
 #ifdef CONFIG_IASL