| Message ID | 96c621a0c1588234e473bf0c0d068af8812f3225.1487139038.git.ben@skyportsystems.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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?
> 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/>
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.
> 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 --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