* [PATCH 0/5] hw/smbios: add core_count2 to smbios table type 4
@ 2022-05-27 16:56 Julia Suvorova
2022-05-27 16:56 ` [PATCH 1/5] " Julia Suvorova
` (4 more replies)
0 siblings, 5 replies; 28+ messages in thread
From: Julia Suvorova @ 2022-05-27 16:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Igor Mammedov, Ani Sinha, Julia Suvorova
The SMBIOS 3.0 specification provides the ability to reflect over
255 cores. The 64-bit entry point has been used for a while, but
structure type 4 has not been updated before, so the dmidecode output
looked like this (-smp 280):
Handle 0x0400, DMI type 4, 42 bytes
Processor Information
...
Core Count: 24
Core Enabled: 24
Thread Count: 1
...
Big update in the bios-tables-test as it couldn't work with SMBIOS 3.0.
Julia Suvorova (5):
hw/smbios: add core_count2 to smbios table type 4
bios-tables-test: teach test to use smbios 3.0 tables
tests/acpi: allow changes for core_count2 test
bios-tables-test: add test for number of cores > 255
tests/acpi: update tables for new core count test
include/hw/firmware/smbios.h | 3 +
hw/smbios/smbios.c | 11 ++-
tests/qtest/bios-tables-test.c | 136 +++++++++++++++++++++------
tests/data/acpi/q35/APIC.core-count2 | Bin 0 -> 2478 bytes
tests/data/acpi/q35/DSDT.core-count2 | Bin 0 -> 32429 bytes
tests/data/acpi/q35/FACP.core-count2 | Bin 0 -> 244 bytes
6 files changed, 121 insertions(+), 29 deletions(-)
create mode 100644 tests/data/acpi/q35/APIC.core-count2
create mode 100644 tests/data/acpi/q35/DSDT.core-count2
create mode 100644 tests/data/acpi/q35/FACP.core-count2
--
2.35.1
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/5] hw/smbios: add core_count2 to smbios table type 4
2022-05-27 16:56 [PATCH 0/5] hw/smbios: add core_count2 to smbios table type 4 Julia Suvorova
@ 2022-05-27 16:56 ` Julia Suvorova
2022-05-28 4:34 ` Ani Sinha
2022-05-27 16:56 ` [PATCH 2/5] bios-tables-test: teach test to use smbios 3.0 tables Julia Suvorova
` (3 subsequent siblings)
4 siblings, 1 reply; 28+ messages in thread
From: Julia Suvorova @ 2022-05-27 16:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Igor Mammedov, Ani Sinha, Julia Suvorova
In order to use the increased number of cpus, we need to bring smbios
tables in line with the SMBIOS 3.0 specification. This allows us to
introduce core_count2 which acts as a duplicate of core_count if we have
fewer cores than 256, and contains the actual core number per socket if
we have more.
core_enabled2 and thread_count2 fields work the same way.
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
include/hw/firmware/smbios.h | 3 +++
hw/smbios/smbios.c | 11 +++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
index 4b7ad77a44..c427ae5558 100644
--- a/include/hw/firmware/smbios.h
+++ b/include/hw/firmware/smbios.h
@@ -187,6 +187,9 @@ struct smbios_type_4 {
uint8_t thread_count;
uint16_t processor_characteristics;
uint16_t processor_family2;
+ uint16_t core_count2;
+ uint16_t core_enabled2;
+ uint16_t thread_count2;
} QEMU_PACKED;
/* SMBIOS type 11 - OEM strings */
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 60349ee402..45d7be6b30 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -709,8 +709,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
- t->core_count = t->core_enabled = ms->smp.cores;
- t->thread_count = ms->smp.threads;
+
+ t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
+ t->core_enabled = t->core_count;
+
+ t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
+
+ t->thread_count = (ms->smp.threads > 255) ? 0xFF : ms->smp.threads;
+ t->thread_count2 = cpu_to_le16(ms->smp.threads);
+
t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
t->processor_family2 = cpu_to_le16(0x01); /* Other */
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/5] bios-tables-test: teach test to use smbios 3.0 tables
2022-05-27 16:56 [PATCH 0/5] hw/smbios: add core_count2 to smbios table type 4 Julia Suvorova
2022-05-27 16:56 ` [PATCH 1/5] " Julia Suvorova
@ 2022-05-27 16:56 ` Julia Suvorova
2022-05-30 6:10 ` Ani Sinha
2022-06-02 15:04 ` Igor Mammedov
2022-05-27 16:56 ` [PATCH 3/5] tests/acpi: allow changes for core_count2 test Julia Suvorova
` (2 subsequent siblings)
4 siblings, 2 replies; 28+ messages in thread
From: Julia Suvorova @ 2022-05-27 16:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Igor Mammedov, Ani Sinha, Julia Suvorova
Introduce the 64-bit entry point. Since we no longer have a total
number of structures, stop checking for the new ones at the EOF
structure (type 127).
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
tests/qtest/bios-tables-test.c | 101 ++++++++++++++++++++++++---------
1 file changed, 75 insertions(+), 26 deletions(-)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index a4a46e97f0..0ba9d749a5 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -75,6 +75,14 @@
#define OEM_TEST_ARGS "-machine x-oem-id=" OEM_ID ",x-oem-table-id=" \
OEM_TABLE_ID
+#define SMBIOS_VER21 0
+#define SMBIOS_VER30 1
+
+typedef struct {
+ struct smbios_21_entry_point ep21;
+ struct smbios_30_entry_point ep30;
+} smbios_entry_point;
+
typedef struct {
bool tcg_only;
const char *machine;
@@ -88,8 +96,8 @@ typedef struct {
uint64_t rsdp_addr;
uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
GArray *tables;
- uint32_t smbios_ep_addr;
- struct smbios_21_entry_point smbios_ep_table;
+ uint64_t smbios_ep_addr[2];
+ smbios_entry_point smbios_ep_table;
uint16_t smbios_cpu_max_speed;
uint16_t smbios_cpu_curr_speed;
uint8_t *required_struct_types;
@@ -533,10 +541,10 @@ static void test_acpi_asl(test_data *data)
free_test_data(&exp_data);
}
-static bool smbios_ep_table_ok(test_data *data)
+static bool smbios_ep2_table_ok(test_data *data)
{
- struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
- uint32_t addr = data->smbios_ep_addr;
+ struct smbios_21_entry_point *ep_table = &data->smbios_ep_table.ep21;
+ uint32_t addr = data->smbios_ep_addr[SMBIOS_VER21];
qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
if (memcmp(ep_table->anchor_string, "_SM_", 4)) {
@@ -559,29 +567,59 @@ static bool smbios_ep_table_ok(test_data *data)
return true;
}
-static void test_smbios_entry_point(test_data *data)
+static bool smbios_ep3_table_ok(test_data *data)
+{
+ struct smbios_30_entry_point *ep_table = &data->smbios_ep_table.ep30;
+ uint64_t addr = data->smbios_ep_addr[SMBIOS_VER30];
+
+ qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
+ if (memcmp(ep_table->anchor_string, "_SM3_", 5)) {
+ return false;
+ }
+
+ if (acpi_calc_checksum((uint8_t *)ep_table, sizeof *ep_table)) {
+ return false;
+ }
+
+ return true;
+}
+
+static int test_smbios_entry_point(test_data *data)
{
uint32_t off;
+ bool found_ep2 = false, found_ep3 = false;
/* find smbios entry point structure */
for (off = 0xf0000; off < 0x100000; off += 0x10) {
- uint8_t sig[] = "_SM_";
+ uint8_t sig[] = "_SM3_";
int i;
for (i = 0; i < sizeof sig - 1; ++i) {
sig[i] = qtest_readb(data->qts, off + i);
}
- if (!memcmp(sig, "_SM_", sizeof sig)) {
+ if (!found_ep2 && !memcmp(sig, "_SM_", sizeof sig - 2)) {
/* signature match, but is this a valid entry point? */
- data->smbios_ep_addr = off;
- if (smbios_ep_table_ok(data)) {
- break;
+ data->smbios_ep_addr[SMBIOS_VER21] = off;
+ if (smbios_ep2_table_ok(data)) {
+ found_ep2 = true;
+ }
+ } else if (!found_ep3 && !memcmp(sig, "_SM3_", sizeof sig - 1)) {
+ data->smbios_ep_addr[SMBIOS_VER30] = off;
+ if (smbios_ep3_table_ok(data)) {
+ found_ep3 = true;
}
}
+
+ if (found_ep2 || found_ep3) {
+ break;
+ }
}
- g_assert_cmphex(off, <, 0x100000);
+ g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER21], <, 0x100000);
+ g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER30], <, 0x100000);
+
+ return found_ep3 ? SMBIOS_VER30 : SMBIOS_VER21;
}
static inline bool smbios_single_instance(uint8_t type)
@@ -625,16 +663,23 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
return true;
}
-static void test_smbios_structs(test_data *data)
+static void test_smbios_structs(test_data *data, int ver)
{
DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 };
- struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
- uint32_t addr = le32_to_cpu(ep_table->structure_table_address);
- int i, len, max_len = 0;
+
+ smbios_entry_point *ep_table = &data->smbios_ep_table;
+ int i = 0, len, max_len = 0;
uint8_t type, prv, crt;
+ uint64_t addr;
+
+ if (ver == SMBIOS_VER21) {
+ addr = le32_to_cpu(ep_table->ep21.structure_table_address);
+ } else {
+ addr = le64_to_cpu(ep_table->ep30.structure_table_address);
+ }
/* walk the smbios tables */
- for (i = 0; i < le16_to_cpu(ep_table->number_of_structures); i++) {
+ do {
/* grab type and formatted area length from struct header */
type = qtest_readb(data->qts, addr);
@@ -660,19 +705,23 @@ static void test_smbios_structs(test_data *data)
}
/* keep track of max. struct size */
- if (max_len < len) {
+ if (ver == SMBIOS_VER21 && max_len < len) {
max_len = len;
- g_assert_cmpuint(max_len, <=, ep_table->max_structure_size);
+ g_assert_cmpuint(max_len, <=, ep_table->ep21.max_structure_size);
}
/* start of next structure */
addr += len;
- }
- /* total table length and max struct size must match entry point values */
- g_assert_cmpuint(le16_to_cpu(ep_table->structure_table_length), ==,
- addr - le32_to_cpu(ep_table->structure_table_address));
- g_assert_cmpuint(le16_to_cpu(ep_table->max_structure_size), ==, max_len);
+ } while (ver == SMBIOS_VER21 ?
+ (++i < le16_to_cpu(ep_table->ep21.number_of_structures)) : (type != 127));
+
+ if (ver == SMBIOS_VER21) {
+ /* total table length and max struct size must match entry point values */
+ g_assert_cmpuint(le16_to_cpu(ep_table->ep21.structure_table_length), ==,
+ addr - le32_to_cpu(ep_table->ep21.structure_table_address));
+ g_assert_cmpuint(le16_to_cpu(ep_table->ep21.max_structure_size), ==, max_len);
+ }
/* required struct types must all be present */
for (i = 0; i < data->required_struct_types_len; i++) {
@@ -756,8 +805,8 @@ static void test_acpi_one(const char *params, test_data *data)
* https://bugs.launchpad.net/qemu/+bug/1821884
*/
if (!use_uefi) {
- test_smbios_entry_point(data);
- test_smbios_structs(data);
+ int ver = test_smbios_entry_point(data);
+ test_smbios_structs(data, ver);
}
qtest_quit(data->qts);
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/5] tests/acpi: allow changes for core_count2 test
2022-05-27 16:56 [PATCH 0/5] hw/smbios: add core_count2 to smbios table type 4 Julia Suvorova
2022-05-27 16:56 ` [PATCH 1/5] " Julia Suvorova
2022-05-27 16:56 ` [PATCH 2/5] bios-tables-test: teach test to use smbios 3.0 tables Julia Suvorova
@ 2022-05-27 16:56 ` Julia Suvorova
2022-05-28 5:28 ` Ani Sinha
2022-05-27 16:56 ` [PATCH 4/5] bios-tables-test: add test for number of cores > 255 Julia Suvorova
2022-05-27 16:56 ` [PATCH 5/5] tests/acpi: update tables for new core count test Julia Suvorova
4 siblings, 1 reply; 28+ messages in thread
From: Julia Suvorova @ 2022-05-27 16:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Igor Mammedov, Ani Sinha, Julia Suvorova
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 3 +++
tests/data/acpi/q35/APIC.core-count2 | 0
tests/data/acpi/q35/DSDT.core-count2 | 0
tests/data/acpi/q35/FACP.core-count2 | 0
4 files changed, 3 insertions(+)
create mode 100644 tests/data/acpi/q35/APIC.core-count2
create mode 100644 tests/data/acpi/q35/DSDT.core-count2
create mode 100644 tests/data/acpi/q35/FACP.core-count2
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..e81dc67a2e 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,4 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/APIC.core-count2",
+"tests/data/acpi/q35/DSDT.core-count2",
+"tests/data/acpi/q35/FACP.core-count2",
diff --git a/tests/data/acpi/q35/APIC.core-count2 b/tests/data/acpi/q35/APIC.core-count2
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/q35/DSDT.core-count2 b/tests/data/acpi/q35/DSDT.core-count2
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/data/acpi/q35/FACP.core-count2 b/tests/data/acpi/q35/FACP.core-count2
new file mode 100644
index 0000000000..e69de29bb2
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/5] bios-tables-test: add test for number of cores > 255
2022-05-27 16:56 [PATCH 0/5] hw/smbios: add core_count2 to smbios table type 4 Julia Suvorova
` (2 preceding siblings ...)
2022-05-27 16:56 ` [PATCH 3/5] tests/acpi: allow changes for core_count2 test Julia Suvorova
@ 2022-05-27 16:56 ` Julia Suvorova
2022-05-28 5:22 ` Ani Sinha
` (2 more replies)
2022-05-27 16:56 ` [PATCH 5/5] tests/acpi: update tables for new core count test Julia Suvorova
4 siblings, 3 replies; 28+ messages in thread
From: Julia Suvorova @ 2022-05-27 16:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Igor Mammedov, Ani Sinha, Julia Suvorova
The new test is run with a large number of cpus and checks if the
core_count field in smbios_cpu_test (structure type 4) is correct.
Choose q35 as it allows to run with -smp > 255.
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
tests/qtest/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 0ba9d749a5..f2464adaa0 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -100,6 +100,8 @@ typedef struct {
smbios_entry_point smbios_ep_table;
uint16_t smbios_cpu_max_speed;
uint16_t smbios_cpu_curr_speed;
+ uint8_t smbios_core_count;
+ uint16_t smbios_core_count2;
uint8_t *required_struct_types;
int required_struct_types_len;
QTestState *qts;
@@ -640,8 +642,9 @@ static inline bool smbios_single_instance(uint8_t type)
static bool smbios_cpu_test(test_data *data, uint32_t addr)
{
+ uint8_t real_cc, expect_cc = data->smbios_core_count;
+ uint16_t real, real_cc2, expect_cc2 = data->smbios_core_count2;
uint16_t expect_speed[2];
- uint16_t real;
int offset[2];
int i;
@@ -660,6 +663,20 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
}
}
+ real_cc = qtest_readb(data->qts, addr + offsetof(struct smbios_type_4, core_count));
+ real_cc2 = qtest_readw(data->qts, addr + offsetof(struct smbios_type_4, core_count2));
+
+ if (expect_cc && (real_cc != expect_cc)) {
+ fprintf(stderr, "Unexpected SMBIOS CPU count: real %u expect %u\n",
+ real_cc, expect_cc);
+ return false;
+ }
+ if ((expect_cc == 0xFF) && (real_cc2 != expect_cc2)) {
+ fprintf(stderr, "Unexpected SMBIOS CPU count2: real %u expect %u\n",
+ real_cc2, expect_cc2);
+ return false;
+ }
+
return true;
}
@@ -905,6 +922,21 @@ static void test_acpi_q35_tcg(void)
free_test_data(&data);
}
+static void test_acpi_q35_tcg_core_count2(void)
+{
+ test_data data = {
+ .machine = MACHINE_Q35,
+ .variant = ".core-count2",
+ .required_struct_types = base_required_struct_types,
+ .required_struct_types_len = ARRAY_SIZE(base_required_struct_types),
+ .smbios_core_count = 0xFF,
+ .smbios_core_count2 = 275,
+ };
+
+ test_acpi_one("-machine smbios-entry-point-type=64 -smp 275", &data);
+ free_test_data(&data);
+}
+
static void test_acpi_q35_tcg_bridge(void)
{
test_data data;
@@ -1787,6 +1819,7 @@ int main(int argc, char *argv[])
qtest_add_func("acpi/piix4/pci-hotplug/off",
test_acpi_piix4_no_acpi_pci_hotplug);
qtest_add_func("acpi/q35", test_acpi_q35_tcg);
+ qtest_add_func("acpi/q35/core-count2", test_acpi_q35_tcg_core_count2);
qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
qtest_add_func("acpi/q35/multif-bridge", test_acpi_q35_multif_bridge);
qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 5/5] tests/acpi: update tables for new core count test
2022-05-27 16:56 [PATCH 0/5] hw/smbios: add core_count2 to smbios table type 4 Julia Suvorova
` (3 preceding siblings ...)
2022-05-27 16:56 ` [PATCH 4/5] bios-tables-test: add test for number of cores > 255 Julia Suvorova
@ 2022-05-27 16:56 ` Julia Suvorova
4 siblings, 0 replies; 28+ messages in thread
From: Julia Suvorova @ 2022-05-27 16:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Igor Mammedov, Ani Sinha, Julia Suvorova
Changes in the tables (for 275 cores):
FACP:
+ Use APIC Cluster Model (V4) : 1
APIC:
+[02Ch 0044 1] Subtable Type : 00 [Processor Local APIC]
+[02Dh 0045 1] Length : 08
+[02Eh 0046 1] Processor ID : 00
+[02Fh 0047 1] Local Apic ID : 00
+[030h 0048 4] Flags (decoded below) : 00000001
+ Processor Enabled : 1
...
+
+[81Ch 2076 1] Subtable Type : 00 [Processor Local APIC]
+[81Dh 2077 1] Length : 08
+[81Eh 2078 1] Processor ID : FE
+[81Fh 2079 1] Local Apic ID : FE
+[820h 2080 4] Flags (decoded below) : 00000001
+ Processor Enabled : 1
+ Runtime Online Capable : 0
+
+[824h 2084 1] Subtable Type : 09 [Processor Local x2APIC]
+[825h 2085 1] Length : 10
+[826h 2086 2] Reserved : 0000
+[828h 2088 4] Processor x2Apic ID : 000000FF
+[82Ch 2092 4] Flags (decoded below) : 00000001
+ Processor Enabled : 1
+[830h 2096 4] Processor UID : 000000FF
...
DSDT:
+ Processor (C000, 0x00, 0x00000000, 0x00)
+ {
+ Method (_STA, 0, Serialized) // _STA: Status
+ {
+ Return (CSTA (Zero))
+ }
+
+ Name (_MAT, Buffer (0x08) // _MAT: Multiple APIC Table Entry
+ {
+ 0x00, 0x08, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00 // ........
+ })
+ Method (_OST, 3, Serialized) // _OST: OSPM Status Indication
+ {
+ COST (Zero, Arg0, Arg1, Arg2)
+ }
+ }
...
+ Processor (C0FE, 0xFE, 0x00000000, 0x00)
+ {
+ Method (_STA, 0, Serialized) // _STA: Status
+ {
+ Return (CSTA (0xFE))
+ }
+
+ Name (_MAT, Buffer (0x08) // _MAT: Multiple APIC Table Entry
+ {
+ 0x00, 0x08, 0xFE, 0xFE, 0x01, 0x00, 0x00, 0x00 // ........
+ })
+ Method (_EJ0, 1, NotSerialized) // _EJx: Eject Device, x=0-9
+ {
+ CEJ0 (0xFE)
+ }
+
+ Method (_OST, 3, Serialized) // _OST: OSPM Status Indication
+ {
+ COST (0xFE, Arg0, Arg1, Arg2)
+ }
+ }
+
+ Device (C0FF)
+ {
+ Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
+ Name (_UID, 0xFF) // _UID: Unique ID
+ Method (_STA, 0, Serialized) // _STA: Status
+ {
+ Return (CSTA (0xFF))
+ }
+
+ Name (_MAT, Buffer (0x10) // _MAT: Multiple APIC Table Entry
+ {
+ /* 0000 */ 0x09, 0x10, 0x00, 0x00, 0xFF, 0x00, 0x00, 0x00, // ........
+ /* 0008 */ 0x01, 0x00, 0x00, 0x00, 0xFF, 0x00, 0x00, 0x00 // ........
+ })
+ Method (_EJ0, 1, NotSerialized) // _EJx: Eject Device, x=0-9
+ {
+ CEJ0 (0xFF)
+ }
+
+ Method (_OST, 3, Serialized) // _OST: OSPM Status Indication
+ {
+ COST (0x0101, Arg0, Arg1, Arg2)
+ }
+ }
...
Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
tests/qtest/bios-tables-test-allowed-diff.h | 3 ---
tests/data/acpi/q35/APIC.core-count2 | Bin 0 -> 2478 bytes
tests/data/acpi/q35/DSDT.core-count2 | Bin 0 -> 32429 bytes
tests/data/acpi/q35/FACP.core-count2 | Bin 0 -> 244 bytes
4 files changed, 3 deletions(-)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index e81dc67a2e..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,4 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/APIC.core-count2",
-"tests/data/acpi/q35/DSDT.core-count2",
-"tests/data/acpi/q35/FACP.core-count2",
diff --git a/tests/data/acpi/q35/APIC.core-count2 b/tests/data/acpi/q35/APIC.core-count2
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..a255082ef5bc39f0d92d3e372b91f09dd6d0d9a1 100644
GIT binary patch
literal 2478
zcmXZeWl$AS7=Youz=a#M-K}6ZwgLuNAQ;$~*xjvQcY@uCVs{~Sf`WpLVt2Rbe!ORA
z_B`J^b9R56*&pj2=<ge2)-*$cPk^sqaDJbVK;QiOWzaNDW>M2p(=#;b`y@>U1KQZ2
ztu5Nwq0xx;_UPb%CKH;?XtAKxijI!x<b=-7=;DH|uIT25?(Uc=6K2kgS+Zc(te7nu
zX3vf}a$wG!m@60N&W(BUVBWl#FCTI)nyEkmx?n*pR0s<f#v(<qXi+Ry3_U#1(-Vsq
z#}Xy5WJxSl3QL#9GG(xASu9r$%a_Lr6|iDOtW*grS4J-{tWpK5R>f-7uzGc@Q3Gq%
z#9Fnmc5SRv2fe+~#|M4+PE2*{()H?L{rcFT0s8r&zdtr?h>aRy<Hp#e2{vtt0Rb2o
zh|QW|P!I+OWAo<Nq6M~WiLF{;NC>uWjcwXs+qT%Q9ky?e9Xepgju;w>ojPIX&e)|3
zcI}GYx?%V37#4;-dSK6<*sB-z?u~u=VBfyjuOIgBj{^qaz=1eu5Dp%ULx$kcp*U<9
z4j+yqM&QViIBFD*9*twh;MlP^ZXAvuj}s=~#ECd*5{8FkL<CNrj8mrI)Tuaa8cv^%
zGiKn-nK)|}&Yq2P=HT49IBy=#pN|U`;KGHtXb~=6j7yeaWF$sK;nJnJY#A<Jjw@E+
z%9Xfk6|P>5Yu4b}wYY8_u3wKEHsHpMxM>q^-i%we;MT3UZ5u{M<M!>iV+Y2>;Le@6
zYZva`jeGXs-o3bQAMW3e2M*xDgLvo=9zKjmj^NRwc<dM+KaM9(;K`F18;hq-VO$)Z
zK8<J2;Mucy?i`*!j~6cB#fy095?;QHSFYgIt9b1i#>Znq0$#t4H*R2JA|@r_&6{}Z
z7A7ZSN($b-jd$+g-Me`29^Su?4<6vdhnSj*j~?OU$C#FePoCh@r}*p{K7WocUf|1@
z`05qDevNP5;M=$O?j62=j~_nZ$B+2w6Mp`TU%ueiulVg7e*ca?e&Ela`0E$`{*8bB
z;NQQPo-UeQHSM3S%%ZeJ#vXl<HmDY*ZB&cWwyH&GJJq7JQMD*-uUeFLP%TQEREyGP
z)uOaTwJ2>>ElNA87Nwn3i_*@jMQIn+qO_}OQQA$lDDAE~Lr49*wAgf6Z7ljNgG@%F
cu9Hk={TGbMqHkcbS~Dh#{`5cn(qE|k2lzA_5C8xG
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/q35/DSDT.core-count2 b/tests/data/acpi/q35/DSDT.core-count2
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..2cd0febc7486b9cb22ba92312f70262a2a0e7baa 100644
GIT binary patch
literal 32429
zcmb8&cYKpo7YFbsU8HH+rlo9z3b+;0-MX_nfR@r0QH!=^D8ms2R74btBH)AyZY@~%
z-dk~R#f^LKy>a$E=Xq{&f6uw~^Lgcu=G^q&?>RS3a+)U(O|&`Ma{Y;x<rxs36sc`4
z4Da6<DWrcM%d$A;%RpV)G^=%KG+fi>$x2D{v@XlCWBL;I7VH`gPpR>oH@kg(d;5Y@
z+dUm|Yx>5(y6GM9<ty8#TRdsu^tjK*K_HgX)*NnY?Tq*uBXvP<TWwv`AKL8mdfOsS
zl<I8@MVl;d+myPfRWPw%+oa}5+Uj{$Bs#UIyCJ=6c3yv9ptb(3lHW!S-*^4g1))uQ
zy?)5*dA>B7P)wIzx-5zFm-fZ&z7g%+fwf(8JWF`F{k@{@{%gDJZ#&lo_&f1-zGj2p
zxr(Wxo_VdEmC>e9#I7;Bv7s>-2^LXZk*4Onk-p)l4zesi-#Gm5rrzG(2c6IgEV2Ui
zUT2pvZ*60&C9z8n?TvTUy0o^jrQY&e*5-e*thCl;0Ur0?FV+w(Y;20Q6&94YHMfMU
zIA7Bq=c_u7Z}$b-yZvjr+O4kktlr)pt2<{+SBG^@hu`|(m-HU1$CvBWmgmb^k;%1e
z{>MtQEUISvv0M$`-mRTO>zYGt_DxLbM^Y(`bxo5~eC<AeU_$P=)Wk%d)!JD)AsnU2
z&d(^7s;8<FX$JcS)OgyhcFW_b@yxVl(&eZW%GTOBrhd|t<f`2Mu2o)N`Y67IT04s;
zHntQc>@?@+?QzWw__Av}$E6;Z>h+FVYRkouN%hG!=`@>e%|6y_P2HMmc}E58HQP0{
zM3U8Hsh^)s$E~>K@s6@QbQ3k?>Dy>&W2CvoleNk3^U{xo2NAmMrWDebXkqKp=Gb_8
z6g=to(GM<*2a#|vHlDr_Zm4haWNmhq@gNdzm{MQZSQp*Du+<Zu7@1%dY}XixjZbME
z&7<Dl_-vjLU*wr%vC+B%U)^IB>=L3iea&Jk&R^Qq-1dTKMq{+OKBs%4dT4B}c<Php
zvg7O54EnNu$Rm;ZI9rDu-rhaBYp&Hjq-%~Zg&qM*?E8_Q?zX*FPhP5L8Go@h4C)@x
zHP_cKo}Zp#Wm_Hb_LUu$)zjf^tcz4z1^za=^u%ZLC~)Qh9=@{#B464M<i@6!#{3EM
zm*!`s(Y@1}*f1exIkAW0YAfH**ZWdvP?H#>(xBGL&rYK{=#k{iQzKqQ6LXyJ+3RR(
zV#q7Wx96tVD`~D8@yf*16w6*oQ|*;B<dsy7JvY@}nW!Mu;+2W1sm@B8YOkaruS{g2
z$CWeJcGB#XiK%JM%0%%r#C@i__L*+4OiWF8Rwjz4A?`E7wa*NDWnyZEd!IDKefD$h
zv!839{oMPcA@0-b+NamGPp^BQG{k+<GZ5U;nXY|iy7x&#+^5gAPoHa_KKDLpi2KZP
z?K8`@&n)*oX^8uz=R){-`d$0<yZ1>$+-J6HpV_W`X1n)EL)>SMYo9r;edf6LNkiOc
zu4|vUu6^dZ_en$CXTY`3fNP%t_daQe`^<CgGtafpJoi3ni2KZU?K9uC&wTejX^8vm
z@7ia7*FO8Z_en$CXMt;<1+IM-xc5mz+~)w-J_oq=Il#S78sa_&y7oEHwa<a>ebNy3
zNxw<Zy)W@RG|08jLGFFh5cfIQwa>w>eGYc-lZLp@A@<6UdL|rVuMDdvyCKfXh#vCF
zsG2*}wQ{Iy<xuxZ8sf@fu9d@FD~GvP(hye;cdZ=mS~=Xkl7_f)8`sKhTr0P6ucRTa
z9AU3abeR$M%EWX02xn#DiJOMJGVv1}>B>3Mm2;#!Ck>sPUTUBrXrMLCb(Gp}rXGqZ
zv1*_+ptQ~)l?I8V)Ignic1!YDyed(jhmtx|lMR%HiLFrsb>=1-C`{!B>P$^GP#RJ-
zi3aM-RU^)c4b+*MY@jrxitM@6K%Kd2r1oe}<p%0ZO$>3L)Ic59UP%oUrg8(NVWKN?
z1EpbNpVUB|x%NtGpfHsis58~h$qkf-iG5N7b>`YDse!^&ZlKOodnGqe8YcEh4b+)y
zucQVFQ@MdUQ|*=9Kxvrxc~S#)=GrT%fx=X7pw3i#B{xtSCiY1U)R}9qqy`F8xq&)U
z?Umd>X_(k2HBe`+y^<OzOyvgZOtn{X1EpbNpVUB|x%NtGpfHsis58}G$qkf-iG5N7
zb>`YDse!^&ZlKOodnGqe8YcEh4b+)yucQVFQ@MdUQ|*=9KxvrRCpA!KuDy~PC`{!B
z>P)p)as#DdVxQDNow@c(YM?Nc8>lnYUdauVhKYSr19j%wE2)9PRBoWoRC^^iP#Pxo
zNe$GQYp<jR3RAg(I#cbH+(2oV*e5kmXRf`H8YoQV2I@?;S8@ZTVPc=uK%KevN@}1m
zl^dut)n3UBl!l3YQUi77+AFDn!c=ac&QyCPH&7ZT_DK!YnQO141`1QTfjU#|mE1sS
znAj&ZP-m{ak{T#X<p%0ZwO4WjrD0;9)IgoN_DX7?FqIpqGu2+n4U~q7eNqFZ5j0R5
zVFRThHc%Qu1BI2;Kw%{}P*}+g6jo9Lg_YDmVI?<ESji0(R#F3nmDE6CB{xu5$qf`%
zQUisR)Ieb+H&9r~4HQ;V1Eo>or%Mf#M)VW2Z%l5WG^8IxV#q5Kw=FeL8YOO9YM?Yq
z+_v06X_&Zexq;G<4iyb;qZVp)`@8HHt3CXJbVCMTgj$#FJ}#ACrSee?KHlMJY?>UR
zjypNbUn2Y^>b2<D%j6ix)%>M~ztnmQ`qxdSqXFHSi+xLH(?OQZ9$!{LY2D;VQzlJl
z3g+ebvQ}kSnf8&OMb>J*&X-PK>*9-N`*JOgsE8&y^fIv@{f-by9P;qf;Tk>|<fXEk
z^9K6*ugbA9Q>nXI^!#nnGcg^#D9E*s1NCqTR86QLuPs{FY%Sr~x75BSmybtTG$Ybv
zj|a6iH#K`!&sH^XdcIy|4~y({@Z|r_9)|2;JA0Vw!|dULzO#p0JL`Gx&Y?6q+^6>L
zOZgA!Bal8~r;mvAk$*`aQR$;%^FBJD^?&a^3hASE`lv`B{g?Dnl|ClZr~QZYF-RY?
z)5k>m*uSKYsr1z{efob$Uk&N2?ex_mef7VjuU6@6WcrN%kiG`e*VyT6MEaV4NnfMV
z*UI$${zLj&NMCEGuNCQQ|0R7brBCChGMPT$q)$9~(61HraCeU1y00t9upjbt!H;Xj
z$%nBo&&|>A8V2WF*C%H@F7w?S{Q_ce&UJlq#^a0*0rTT5u`&9s#NeFk`s9qqO@W)E
zUsVjwxvo#ncpMCHbM*U*!8zCU$r<<Sfo_g|sWCX`x;{DMemcm_(Qi5i=UnIJ0>kw2
z?9SZg`Y^rKLT{`1Q|Xw0;Iz?nZ{jy$YJJo6A^*mvSaafT>Z3;|HK*)I<CMbMBNxOE
zTX<;U(X-|sx$x*2v*uf^o=D@w3F_{e?eo=mw)nUBJ$^c@PfdxBop15`{H@D&<Uz<P
zj5Nj?JdwJFW>2K4em@T#0T1$*YLAr~iPT5wD0@E7OpP?hCKl54XsD%-4mQ^{)Oq6;
zU)kEZa&X2pdq-4}Q)PB2(pVP^76rKu=dGjkf{8CgTB;97>GpNawpK)f!QdPYJscLz
z>CWn!?Mvaf*ojj)E^*>Cj!T_5o#Qem&fvJ*iTiO};ly5!E1fu#<0>ciaU62uERMrY
z?B_V*#MvB2oj8Z%m=ou6T<ydGii@axt~rlmRewIms{Z~QtNIH#R`n0ySk*s}V^#kk
zj#d4GIac)#;aJr_lw(!@FpgFI!#P&<Z^N;we+0*>{*fH3`U@#8R`qYov8sO*$EyC(
z9IN`raIEUzj$>8-_8hDFci>pnzaz)0{+&2h_3zBFs(%-bRsFkitm@y5V^#lHj#d5R
zI9BxsDK1g<7jdlWFXmX)U&67fzm#KDe;LQB{&J30{S_Rm`YSnB^;dDM>ffDXRsS9w
ztNQokSk=E5$EyClIac-W!?CJ=Uy4gr{UMH3{b7z({Sl5;{ZWop{V|SJ{nZ?+`fE5=
z_1AK&>aXKi)jytNRsRHzRs9n=R`u6&tm<#zSk*s?V^x16#bv7g$sDWtn>beWH*>7&
zZ{b+gKZRpe|9%{+`loWN>ffJZRsR7TtNIV*Sk>Rkv8sO>$EyB=I9Bx^%(1G!jbl}R
zJH_Ry{tk{+{nI&C_0Ql~)jyMCRsSrGRsDx>tm;3MV^#lbj#d40I9Bz~<yh4}k7HH;
zVH~Ua59e6be+0*>{v$b7^~Wi$Q1u_hv8w-Qj#d5hIac*A;8@kakYiQ<F&wM<kL6g^
ze;miE{zV+C`j6*W)xVfyRsRVbtNNF4tm;3JV^#l29IN`5Qe3I(U&gVjzmsEC|8kC1
z{U>v*>OX~JRsRZ(RsE-Otm<FMv8sO+$EyC-9IN_I<5<;yI>)O1GdNcDpUJVR|16GG
z{by5LrRrb9v8sP9$EyA=j#d5VaIETI$FZt^J;$p4b2(P^pU1JP|9p;B{TFbo>c5a<
zRsTgCtNJhISk-?C$EyBIIac*|QyfzDZ{S$fzma2A|79Gj`g=H5^<U1hs{aa(RsC0T
ztm?mtV^#mv9IN`T;aJswEyt?<>o`{RU(d0s{|1g#{Wo%~>c5HNu&V!Nj#d4)aIEUT
zm19-^Z5*rmZ|7Lme+S2^{yRBV_20#@s{d||RsHvHtm?m)V^#ls9IN{8=UCPM0LQBS
z2RT;tZ=yJ&>VJr1RsX{rtNI_|Sk?b1$EyCvI9ByP&atZh3653$Pjam4e~M#O|I-|+
z`k&!g)&DHVs{ZFVR`oy6v8w+Cj#d4eDUPc8U*uTT{}RWl{+Bsc^}oWgs{d7vRsFAV
ztm=QAV^#kaj#d3{aIETolVer?TO6zU-{x4={|?8h{&zW6^}olls{eh8W2*iSI9Bz4
z$g!&bBaT)5A9Jkg|Ab>z|EC<Q`ak1X)&Duis{SuHR`q|$v8w+oj#d3%bFAwBhGSL#
zw;ZebzvEcd|2@Uks{S81R`vhLv8w+kj#d3XbFAwBg=1C!uN<rTf8$uyzm;QE|L+{D
z`v2fq)&D2Qs{X$?R`vhQv8w+cj#d4=98>-NuG#e7#fnG~|NhSdk3FCr@F2w=&=z=*
zY7b}+JV>(#v<V)h+XLDK4>IflZG#8>>;dhA2VQ$X8{t8wJ)oWNz-JF=D?G@u2ecO+
z`0W90h6ma9fOf-!9D6|9;X$rFQ2Pn+fL{>sck=9k+E2bcD4tVLn?a{4g)&=5PaBt(
zIFE~8)bWcoKG#TIO)sqCR(to*F8K|9xfb{3(m6)PcP38gX)mZx=a+E*Es;l3B=qSQ
z)DDa^M;g5UvJG#fAvQHXr8jQPh<j#ktj_P6>4{r2<DT}o)v>X`e|A^9XWfjf{&Zz~
ze1=SFZ&_YRFG0Hpt&`W%i_jUDcV~8W_<AYP<?+70-#x~-?f6x0e8x(yvcp=~L3M4Y
z=q@v+*{_4?rRoe{YTUSK=$o`|;S4^HGM46?NXg)IYX-l(ok0m&i~8Q2VNpE`r}uqB
z*Jnm1HMjWx<%aJ2{$lua<HhInaofawDe_vLI#*svH*=rWwJxuu7sEk1+(G~7U;y2-
zy*+&YrkDHmp_aTXpVy){SNX7eK@OcG+LFbav1ZSi+qyiKMoWwABl51LbhJCU)Z?+2
zX0@q<5kU_y9-FCUrkLIx=N#F(0zPb=bW&1m$Dee3DRgWwQI9W0OC@x=nUv@VbW%#C
z6Y%V<CMxo!qLiwoQhN7XN^~$eDW#=JDGjAGEtT>64Ptk6TsSGEr%NdvrF1Qo)4S_Z
zqJv0DDI-Hl87O6Fse;~Zml7SVO-lXxNvR)7{j^j`cMvJjiN;CE>y?rhC9jsM>=SC;
z_kv82GVS+&=x&<)*=3@XsihFVyCZIKA4oo*tjULxPfKBbk4Q*aAW_fZZ$nL4C}n9W
zLhs&7$q$m>KF=8?KT3WrMd{sqDP@C{oh@t1Mk!lMF?x4jN;x3q<VYz8r5r6))4Tmr
zqW9jC_k!G9DdnP+o0N)ze3)L`;&dQCDFp&j3ZN9wQV}1G7ZM%APfB@tQp!UqPfNvo
zAYMpx_&zD+=SwLcrF<=w@G*EH(UJS4)W5%!`lHleOQn4HT}X7$J}DIxNT~p&0xgyC
zk#`}{@%p4RV1Secpfo^B<$TaxNOY(^DGeMbrGY37)KUc>Zx<3BrB6zO21#iUN`tgi
zNoOBOX)s8G2TN%%N`tjjMQ0;OX$VL|hDd1$N<*|1qSO1NG!&$vL!~qnrJ-61)7c7A
z8V1s^VNx1~(l9MW=<Ee44F_rXa48K(X}Ff6bT)&OwgG9IZKSjfO512DMrSujX#_|k
zMo4J{N+YyXO=mkuX(UJ^M@nfVN+Xj}aj?*o3PE!IQx86m7ot?CrJ`+3X<LxC-B#AL
zElS&Jsd$trjRI-ZC@GCXX_S^qMw`-TkVcP|(rA=MYpHaMDUAVX%or(+L1~PZ%C<A5
z?LgXYJ1K34(so)Z-`<qA2Wk85rL;Xt+iR&}2UFSsq#bsU(hex?pry(kO=(AvcHB`)
zJEF9sma29#rJX?9X(uV|gwjr03hiu4JA<_I&QjVLrJc1D-o=!50cn?Aq_hi4yJ#u0
zt10aY(yqHoX;+kX)lzgfQ`!xr-FB1GZYb@hrPx?g8Vl0cu~Hg~(pW83k29rlAdMR*
zrEw^YOG+i`<(%ua6umE<?3}@%l!7P)wNzAO)<pj?EGZS)um13vt_Y<fEfp7=QZY!y
z#j>Vil!~=fQesLaAeEFzsRX4GEtQs<QYlEKrBW(IsZ>j4Wu{aHQdyal%1|oPQhB*4
zm4j4XE~Rpm%C%HcVM-MsRa8i+0;LKqRaTl(B}kQ(QmRC$QcG1;rc?z|Rh5*gP^!{W
zXm?ZE9i-iNm(uPi?XIQp9;UPhNPFxdr9DvELral8O=(Y%_S{oSd!n?bmZE!^(q16#
zwU?CkLTN88#r8I(y+PW0Zz=7K(%xFC-p7>o0coFoq_hu8`y{2(;J&7`FG&0DE2Vu=
z+E+_OAycArz>?2Tp^%h9D223C95y97{VOSj!&2h2GLy%j!dfbcm=c}&m6Rg(fAz-a
zOg_OfDMhqY8Z{+4=_@Hkqp~JGS28I@wNw@}B|6_LDaB$^;?o|JQcO$b)uu$JdL^Z5
zI@^d|w87C3K7%nSRcooD#+2wRucTB{BPBi=F)7t(sj}9T=mf8%R9h=0KCdt-)oQ7#
z&Xnpvs;iSy9ZGdt3XM0V@gR*KFQxG)jn`6mf+<Y^X~G04O+aaamLd~PX(C7yCrW7|
zN)xpdtv4n5zxc^pyuMya^(fVADb`?04Ini%NT~s(1}#-jGNnl%O`0U7NhnQ9N@c-D
zQ)&dMu~ABmC^c%SXtF6y25ItSDNROcvX+XQOsNT^rY0#hq12?Ml4et)Q~i=ZyXIyo
zHKWw5rP3BtqCdirlv-M()Phormdd7>(iD)UOp($Ql%{B@d_Pm#52XF}lhS@D?Wd)R
zsirg)q^VP-G!><(TB_XNl=cT{|NW)3KT7*+sp<ezIsl{t4v^9TC>@}s(1E6OAV>!u
zD5V2YI#5gDR#R#PskK!~ttho>DKgEJrhznVnv|xYG)+s<gG}ilkPbRXN(Z5Ikd|Tx
zo6^A`9el8q4o2x<EmgOf68*u9<fEmnO-gMjwI!wUV7n=`gVf$GrFN9swN%t$N*y3|
zbV#WKr4B6>PdBCMAWfexrRgY5*HXz0Q<?$Nj2Tjzfzk{umCiJ!nIO%aDW#bx&D2ua
zEK`~V(yUohnuXFVEtMZ)N{4`S$RScX1f@f?RB@;&9SYK+hf3*Cln&KW<!n=$4btq{
zQksp@Y%NvIF{L>m&6y*mIVjE1QfRIz%>`-hTq(^(X|9&S^Gs<TNb}}NX&y@Rv=lka
zlnw*wu*0Nu7)pm}DSEgm9S+jrhfC>jln&QY><Cjj0;D63kkSz-9igS_BTeZ@kd8c3
zN=Kq}WKyaK#!V>>QamoDI7)FX6&+<tM}c(IQBpbzrK7Y|e6%SY4bss^OX+Boj@DAi
zd{dea(){^Snvc?aEtM`Xr3D}@SRkbZC@s)Z*+NrV2-3oZQd)@8LM@daV@k(>bj&eQ
zItHa<v{Z4dDIE*avByg3Sd@;{Qsr@`bR0;>9VeyZP&!UaRf|k%5lD*`Nof&Ei?kFv
z-jt39>G<QNbUaGOYbm_gloo@uc(IfgqqJB{krPbm1dvWRK}si}bb^+mOH64ANK2MT
zX$eY8v=lqhluiWc#1o}-B1$J}srn>SItiqcPLk3|D4mp)DuYW+X(>ocmr7|VN=vm=
zw9J&2fwXLyl$N2iOiRU`rql^iXQz}pQR>uE$#PR#4$|`FQd*AEaxImfY)U7Cbn?kk
zIvJ&twN!SBDV+k+DW^#36qHWUQuzu~S^?6E6;fJ((h4nAoN7v^f^_PsQaTl-Q?*pN
z(v(($v~s1CR-&|0OI52(X%$GTR!M0UN~^RKT5U?JL0Y|9N~=*?t)=j3rgR!er=2FH
z(@;81OOew}>2#1zKV3?vqjb8KqGy=W86cf;hLp}g=?pE!&NQVnK|1qHDV>SZnOdqo
z%aqOn>8!J)bQVfyC8et1*`{<hNN1ldrL$2wTT4Z2Olb{BYt~3<4N7aYRJ_)d)`GNl
zt(4ZHv{p+cU8d9pQdgIhx=`xUQt3ISbPh=8oFk=kP&!9TW$R379Z2ieNogHQ>$Ft9
z-jvpZw0^ym)}ypuOBLsu(zzg=d#;quMd@5ERi0-`=Ye$Ic~UwLrSr5@b-pQ`57PPP
zOX+-+&eu}t0#mvGqzf*P(gi48pr!DIrgR}l7hWi(3sJgIOOcCA=^~IWx=2bFp>&a!
zq8FRe#UNdLv6L=G>0&L#E-|G`K)U1-DP4lnC0eS!)RZm->C#K3bSX-gCZ$lY+myON
z>h6|OH%i@FD%xO58$jByK}s7?+MuQ4ji$5_q>US;v=OC^S}M8Blr975vdg4&8A_LF
zskFzGdO+&wkx~yzJz6Te+>|Z{>GI2^bU8|wYpMJSQ@R4AE3S~z6)0VyrHU&}=}M5U
zyi!V6qI9K}Dz7r7t3bNyDk)us(p6fjy4sYk2I=anrF1n)S8FMBjVWCN(lyse=^B)-
z(Ng$YQ@R$UYp<2kwJ2SyrO0)rbR9_7T_>gMP`XY_(d$j=dXTQaUP{-abiI~hH<;25
zAl-0-lx{%j1}#<JXi7JNbmNUux)G%tlTtW%lPTQ<(oHu>=_ZtJ(o)gQrgSq%H{UF!
zn^C%1OU1XC(k&p}a*LF1LFpDPmE3Adw}N!*tx~!brCYUBdYdWT2GVV}N$EC}ZqriP
z?WS})NVnfErQ1=uT}$P6n9>~}-EoJM?m+1dEmhoUN_T>E=bciz6Qw(~RC$*v-38KJ
zcS-3kl<v|})!n9aH%NEiEv36rx?4-3dravbknXuhO8202kCwvsn$o=>-FvT;?nUWd
zEk*7#rTaj-?>;Hrhthpoir#NZ_k(o*{ZhIgrTeuMd%%<)0O^4Tr1Stv4``|SK~s7V
zqz50A(t{{Hn3N*HO{TO7q)nTov<an6S}J<TlpX@<p@*dO5K0eesrX@2dKjdKAC}U?
zC_SvDl1EJG5s)5vL`sjK^oW*9A2p>%L3;F2DLsnPqgpC^%#<Dj>9NP8^cYHyX{r2i
zQ+gbv#~+u{<0w6@rHUs^=?RdYctT1~p!9^6DxWl^Cqa7hNhv*v(vw=MddifZ0_mxz
zr1TU@PiZOiv?)Ce($h~%>1mXn)>8NxQ+fuZXP%MLGblZyrO30U^ejlvJ}aeXQF>NO
z(dSI*Igp-vPD;<A^qiJr&zsWoAU*%Ql%7ZFc`a4HU`j85^uh~LdI6;ul2SCd*_1Ye
zw0X0XHlwszOGPi5(u*Ly_@b0vMCnB>6~AOkFM;&ZOHz6XrI)l+^0Fzt4ARRlOX+2l
zUe;3SE2i`cNUyvirB_gTMN4I`n$oKvz51$@UPb9uEtS7!O0R+R+G|pJ4W-w#RPnkg
zy$;gruS@B5lwQ|T<rY)g0@9W(Qrd#j7A;l1VM=d+^u`-fdIP05v=n;Nl->mC%{Qg=
zCQ5H=Dg2fxy#>-+Z%OGbl-|-(<ZV-W8>F}2meSiOy{)C_JErswNbkHOrFT$zM@zAH
zP3c{b-hEd}@1pdsma5+~rT0L3?>#BKhthjVDHeR+l->vF{r9ExK1%Ovsptb!`T(R4
zK9JG}D1D%%;tx&fLy$iFP)Z-7^r4nYJ~E|`K>Fw-DSd>}M_MZV*pxm7>En;3^f5{w
zYpLuLQ~Cs?Pd<^-Cn$ZQrSeZr=~IwC{ZvYyqV%bjDn2u%&p`U@Gbw$B(q~$#{M?j2
z2kG<ArSv&UpKGb=3sd?6q%XdZ(ibRwp{3B5rt~F9Uw$d2FH!nZOX06f=_`=F`btV)
zq4brOB43-**C2iUwUoX_>1!=TzcHn6K>FqzDSd;|H(H8)Yf9gO^zFA&`WB^cwN(9`
zDSZdhci&0rJCwdlO4Y&dP3e1(zW-iI-=p-smWqBbr5`~0;Rh-GfYJ|ID*n-wegx^q
zAEopoN<V6;<R?@538bHXlG0Bo{iLPRpH1m!kbeGIN<X9YvzE$!F{NKX`sEiX{esdj
zS}On5lzs*2*I%XdD@wm=sp2<N`VFMtev{H~DE+3T%B`lf6{M|OrL+~Lty-%3-IRU@
z>G$8I^gBwwYbo@HDg6P`AAd;c50w7UQut3(`V*u-|CG|7DE+CW$X}-P7f65oC8fVm
z`b$gEzfI|HkpBK#N`IsDx0Yi6n9@HW{qv8M{z2&<mOA&%;6J!#|Iujm=a_>Pmi?Eb
zeZ8*#<=gAX^vRu>0n6&8)U5L*{*SC)%+PCRh-UC#aqBAs{e?IBi%rQt3q?a$4o<*1
z_zRVi_`5v3Dv<myRy5LCiu%mracA+kvv^b%zQioF3bJ@0OL3oBQruZm+*wjo7QVzR
zv<kAMK$em|v!uGSq`I@Dsw{koS!fkxNrfz>eP&5>XGwEsNmE()60^`M$dU$G%KFTb
z?#{v=2Fm|qLEXdBRTjR)EVK%;q(heSKC@)Fvt+olWT-5BiCJhBWXXUm6@6yu=g!j4
zou!}3!k3tZRza42kfpNEEM9jOuRDuZW#LQALaQK)7qV3KnI+SmCDWZHQ)S^x%tEUm
zOD1Fq^_j)z&f;@t@u@6)iCJhBWbr|kaGzPS+*z{RS+Z0XzQioF3bJHDmPnsj{O&A%
zcNV|O!k3tZRzVg&WQq2fCEJ}P+nps_W#LQALaQK4He`wQnI*@aCC8m5M`htl%tEUm
zOAch|v$xP6#&kXN=eo1xsw{koS!fkx$<<kk`tC0Qcb0%VOF(7eOUy#6AWHzU^xa?b
z+*$J6S@KjCzQioF3bN!umcIK-zB^04J4?RG!k3tZRza40&QdUpK8B&DG1e9f7F+7W
zBlsg3_)m5QCZ+MWTz8CetDuVJ7FqmPCX=5o?R@$%T+tY5u>TCCYet}cfb*FT_6H2x
zpSwLi%Xi1hY%9gjo!$QZfj%3`v(>+q9yN5mhd$qe{{2SZ-s(?|v+47QE$b)y1H0|-
z^G7W8_Ga-^`j=w4KFZrmABpU|<8N74xxTZBKI7Z@t;yQ7_v&5f<G+>WICSHY+S}`O
zhRLry<`2A%q|uF|?rQvL;-O%<^Se?uUk@Lz;d}^omh)*a_Qz`!W)x<4>`%=epVnC0
z*yP;AZiH^6K0d0$v^M!S`8|W_Bd>RwM(LYN+VV5f_}3STOWX3jX}%O+iajdh@9-`H
zd*~lSYv<VNsS)+CoE)p4LVu&PnAg$=gJZ>k^gud)AV{DgZJPZt(bZ}EQ*7<bY7Pe5
moR4^K?cBLJSm_MtC+4vV$SI0i^a0ou6eofbJILU>i1j}we{{3}
literal 0
HcmV?d00001
diff --git a/tests/data/acpi/q35/FACP.core-count2 b/tests/data/acpi/q35/FACP.core-count2
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..31fa5dd19c213034eef4eeefa6a04e61dadd8a2a 100644
GIT binary patch
literal 244
zcmZ>BbPo8!z`($~*~#D8BUr&HBEVSz2pEB4AU24G0Y(N+hD|^Y6El!tgNU*~X%LSC
z$X0-fGcm9T0LA|E|L2FOWMD92VqjR>!otAF!NBm72O<iWged~jj0!*k$y^{03>bk1
YBHITON2VDSAnpK(F*YFF1LDH~0O^Si0RR91
literal 0
HcmV?d00001
--
2.35.1
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] hw/smbios: add core_count2 to smbios table type 4
2022-05-27 16:56 ` [PATCH 1/5] " Julia Suvorova
@ 2022-05-28 4:34 ` Ani Sinha
2022-05-31 12:40 ` Julia Suvorova
0 siblings, 1 reply; 28+ messages in thread
From: Ani Sinha @ 2022-05-28 4:34 UTC (permalink / raw)
To: Julia Suvorova; +Cc: qemu-devel, Michael S. Tsirkin, Igor Mammedov, Ani Sinha
On Fri, 27 May 2022, Julia Suvorova wrote:
> In order to use the increased number of cpus, we need to bring smbios
> tables in line with the SMBIOS 3.0 specification. This allows us to
> introduce core_count2 which acts as a duplicate of core_count if we have
> fewer cores than 256, and contains the actual core number per socket if
> we have more.
>
> core_enabled2 and thread_count2 fields work the same way.
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
Other than the comment below,
Reviewed-by: Ani Sinha <ani@anisinha.ca>
> ---
> include/hw/firmware/smbios.h | 3 +++
> hw/smbios/smbios.c | 11 +++++++++--
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> index 4b7ad77a44..c427ae5558 100644
> --- a/include/hw/firmware/smbios.h
> +++ b/include/hw/firmware/smbios.h
> @@ -187,6 +187,9 @@ struct smbios_type_4 {
> uint8_t thread_count;
> uint16_t processor_characteristics;
> uint16_t processor_family2;
> + uint16_t core_count2;
> + uint16_t core_enabled2;
> + uint16_t thread_count2;
I would add a comment along the lines of
/* section 7.5, table 21 smbios spec version 3.0.0 */
> } QEMU_PACKED;
>
> /* SMBIOS type 11 - OEM strings */
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 60349ee402..45d7be6b30 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -709,8 +709,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
> SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
> SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
> SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
> - t->core_count = t->core_enabled = ms->smp.cores;
> - t->thread_count = ms->smp.threads;
> +
> + t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
> + t->core_enabled = t->core_count;
> +
> + t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
> +
> + t->thread_count = (ms->smp.threads > 255) ? 0xFF : ms->smp.threads;
> + t->thread_count2 = cpu_to_le16(ms->smp.threads);
> +
> t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
> t->processor_family2 = cpu_to_le16(0x01); /* Other */
>
> --
> 2.35.1
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] bios-tables-test: add test for number of cores > 255
2022-05-27 16:56 ` [PATCH 4/5] bios-tables-test: add test for number of cores > 255 Julia Suvorova
@ 2022-05-28 5:22 ` Ani Sinha
2022-05-31 12:22 ` Julia Suvorova
2022-06-01 5:06 ` Ani Sinha
2022-06-02 15:20 ` Igor Mammedov
2 siblings, 1 reply; 28+ messages in thread
From: Ani Sinha @ 2022-05-28 5:22 UTC (permalink / raw)
To: Julia Suvorova; +Cc: qemu-devel, Michael S. Tsirkin, Igor Mammedov, Ani Sinha
On Fri, 27 May 2022, Julia Suvorova wrote:
> The new test is run with a large number of cpus and checks if the
> core_count field in smbios_cpu_test (structure type 4) is correct.
>
> Choose q35 as it allows to run with -smp > 255.
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> ---
> tests/qtest/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 0ba9d749a5..f2464adaa0 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -100,6 +100,8 @@ typedef struct {
> smbios_entry_point smbios_ep_table;
> uint16_t smbios_cpu_max_speed;
> uint16_t smbios_cpu_curr_speed;
> + uint8_t smbios_core_count;
> + uint16_t smbios_core_count2;
> uint8_t *required_struct_types;
> int required_struct_types_len;
> QTestState *qts;
> @@ -640,8 +642,9 @@ static inline bool smbios_single_instance(uint8_t type)
>
> static bool smbios_cpu_test(test_data *data, uint32_t addr)
> {
> + uint8_t real_cc, expect_cc = data->smbios_core_count;
> + uint16_t real, real_cc2, expect_cc2 = data->smbios_core_count2;
> uint16_t expect_speed[2];
> - uint16_t real;
while you are at it, I suggest renaming this to real_speed or some such so
that its better redeable.
> int offset[2];
> int i;
>
> @@ -660,6 +663,20 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> }
> }
>
> + real_cc = qtest_readb(data->qts, addr + offsetof(struct smbios_type_4, core_count));
> + real_cc2 = qtest_readw(data->qts, addr + offsetof(struct smbios_type_4, core_count2));
> +
> + if (expect_cc && (real_cc != expect_cc)) {
I think better to say if ((expect_cc < 256) && (real_cc != expect_cc))
> + fprintf(stderr, "Unexpected SMBIOS CPU count: real %u expect %u\n",
> + real_cc, expect_cc);
> + return false;
> + }
> + if ((expect_cc == 0xFF) && (real_cc2 != expect_cc2)) {
> + fprintf(stderr, "Unexpected SMBIOS CPU count2: real %u expect %u\n",
> + real_cc2, expect_cc2);
> + return false;
> + }
> +
> return true;
> }
>
> @@ -905,6 +922,21 @@ static void test_acpi_q35_tcg(void)
> free_test_data(&data);
> }
>
> +static void test_acpi_q35_tcg_core_count2(void)
> +{
> + test_data data = {
> + .machine = MACHINE_Q35,
> + .variant = ".core-count2",
> + .required_struct_types = base_required_struct_types,
> + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types),
> + .smbios_core_count = 0xFF,
> + .smbios_core_count2 = 275,
> + };
> +
> + test_acpi_one("-machine smbios-entry-point-type=64 -smp 275", &data);
> + free_test_data(&data);
> +}
> +
> static void test_acpi_q35_tcg_bridge(void)
> {
> test_data data;
> @@ -1787,6 +1819,7 @@ int main(int argc, char *argv[])
> qtest_add_func("acpi/piix4/pci-hotplug/off",
> test_acpi_piix4_no_acpi_pci_hotplug);
> qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> + qtest_add_func("acpi/q35/core-count2", test_acpi_q35_tcg_core_count2);
How about checking thread count as well in the same test or in a
different test?
> qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> qtest_add_func("acpi/q35/multif-bridge", test_acpi_q35_multif_bridge);
> qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
> --
> 2.35.1
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/5] tests/acpi: allow changes for core_count2 test
2022-05-27 16:56 ` [PATCH 3/5] tests/acpi: allow changes for core_count2 test Julia Suvorova
@ 2022-05-28 5:28 ` Ani Sinha
0 siblings, 0 replies; 28+ messages in thread
From: Ani Sinha @ 2022-05-28 5:28 UTC (permalink / raw)
To: Julia Suvorova; +Cc: qemu-devel, Michael S. Tsirkin, Igor Mammedov, Ani Sinha
On Fri, 27 May 2022, Julia Suvorova wrote:
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
Acked-by: Ani Sinha <ani@anisinha.ca>
> ---
> tests/qtest/bios-tables-test-allowed-diff.h | 3 +++
> tests/data/acpi/q35/APIC.core-count2 | 0
> tests/data/acpi/q35/DSDT.core-count2 | 0
> tests/data/acpi/q35/FACP.core-count2 | 0
> 4 files changed, 3 insertions(+)
> create mode 100644 tests/data/acpi/q35/APIC.core-count2
> create mode 100644 tests/data/acpi/q35/DSDT.core-count2
> create mode 100644 tests/data/acpi/q35/FACP.core-count2
>
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523c8b..e81dc67a2e 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,4 @@
> /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/q35/APIC.core-count2",
> +"tests/data/acpi/q35/DSDT.core-count2",
> +"tests/data/acpi/q35/FACP.core-count2",
> diff --git a/tests/data/acpi/q35/APIC.core-count2 b/tests/data/acpi/q35/APIC.core-count2
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/data/acpi/q35/DSDT.core-count2 b/tests/data/acpi/q35/DSDT.core-count2
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/tests/data/acpi/q35/FACP.core-count2 b/tests/data/acpi/q35/FACP.core-count2
> new file mode 100644
> index 0000000000..e69de29bb2
> --
> 2.35.1
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/5] bios-tables-test: teach test to use smbios 3.0 tables
2022-05-27 16:56 ` [PATCH 2/5] bios-tables-test: teach test to use smbios 3.0 tables Julia Suvorova
@ 2022-05-30 6:10 ` Ani Sinha
2022-05-31 12:39 ` Julia Suvorova
2022-06-02 15:04 ` Igor Mammedov
1 sibling, 1 reply; 28+ messages in thread
From: Ani Sinha @ 2022-05-30 6:10 UTC (permalink / raw)
To: Julia Suvorova; +Cc: qemu-devel, Michael S. Tsirkin, Igor Mammedov
On Fri, May 27, 2022 at 10:27 PM Julia Suvorova <jusual@redhat.com> wrote:
>
> Introduce the 64-bit entry point. Since we no longer have a total
> number of structures, stop checking for the new ones at the EOF
> structure (type 127).
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> ---
> tests/qtest/bios-tables-test.c | 101 ++++++++++++++++++++++++---------
> 1 file changed, 75 insertions(+), 26 deletions(-)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index a4a46e97f0..0ba9d749a5 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -75,6 +75,14 @@
> #define OEM_TEST_ARGS "-machine x-oem-id=" OEM_ID ",x-oem-table-id=" \
> OEM_TABLE_ID
>
> +#define SMBIOS_VER21 0
> +#define SMBIOS_VER30 1
> +
> +typedef struct {
> + struct smbios_21_entry_point ep21;
> + struct smbios_30_entry_point ep30;
> +} smbios_entry_point;
> +
> typedef struct {
> bool tcg_only;
> const char *machine;
> @@ -88,8 +96,8 @@ typedef struct {
> uint64_t rsdp_addr;
> uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
> GArray *tables;
> - uint32_t smbios_ep_addr;
> - struct smbios_21_entry_point smbios_ep_table;
> + uint64_t smbios_ep_addr[2];
> + smbios_entry_point smbios_ep_table;
> uint16_t smbios_cpu_max_speed;
> uint16_t smbios_cpu_curr_speed;
> uint8_t *required_struct_types;
> @@ -533,10 +541,10 @@ static void test_acpi_asl(test_data *data)
> free_test_data(&exp_data);
> }
>
> -static bool smbios_ep_table_ok(test_data *data)
> +static bool smbios_ep2_table_ok(test_data *data)
> {
> - struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
> - uint32_t addr = data->smbios_ep_addr;
> + struct smbios_21_entry_point *ep_table = &data->smbios_ep_table.ep21;
> + uint32_t addr = data->smbios_ep_addr[SMBIOS_VER21];
>
> qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
> if (memcmp(ep_table->anchor_string, "_SM_", 4)) {
> @@ -559,29 +567,59 @@ static bool smbios_ep_table_ok(test_data *data)
> return true;
> }
>
> -static void test_smbios_entry_point(test_data *data)
> +static bool smbios_ep3_table_ok(test_data *data)
> +{
> + struct smbios_30_entry_point *ep_table = &data->smbios_ep_table.ep30;
> + uint64_t addr = data->smbios_ep_addr[SMBIOS_VER30];
> +
> + qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
> + if (memcmp(ep_table->anchor_string, "_SM3_", 5)) {
> + return false;
> + }
> +
> + if (acpi_calc_checksum((uint8_t *)ep_table, sizeof *ep_table)) {
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static int test_smbios_entry_point(test_data *data)
> {
> uint32_t off;
> + bool found_ep2 = false, found_ep3 = false;
>
> /* find smbios entry point structure */
> for (off = 0xf0000; off < 0x100000; off += 0x10) {
> - uint8_t sig[] = "_SM_";
> + uint8_t sig[] = "_SM3_";
> int i;
>
> for (i = 0; i < sizeof sig - 1; ++i) {
> sig[i] = qtest_readb(data->qts, off + i);
> }
>
> - if (!memcmp(sig, "_SM_", sizeof sig)) {
> + if (!found_ep2 && !memcmp(sig, "_SM_", sizeof sig - 2)) {
> /* signature match, but is this a valid entry point? */
> - data->smbios_ep_addr = off;
> - if (smbios_ep_table_ok(data)) {
> - break;
> + data->smbios_ep_addr[SMBIOS_VER21] = off;
> + if (smbios_ep2_table_ok(data)) {
> + found_ep2 = true;
> + }
> + } else if (!found_ep3 && !memcmp(sig, "_SM3_", sizeof sig - 1)) {
> + data->smbios_ep_addr[SMBIOS_VER30] = off;
> + if (smbios_ep3_table_ok(data)) {
> + found_ep3 = true;
> }
> }
> +
> + if (found_ep2 || found_ep3) {
> + break;
> + }
> }
>
> - g_assert_cmphex(off, <, 0x100000);
> + g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER21], <, 0x100000);
> + g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER30], <, 0x100000);
> +
> + return found_ep3 ? SMBIOS_VER30 : SMBIOS_VER21;
> }
>
> static inline bool smbios_single_instance(uint8_t type)
> @@ -625,16 +663,23 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> return true;
> }
>
> -static void test_smbios_structs(test_data *data)
> +static void test_smbios_structs(test_data *data, int ver)
> {
> DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 };
> - struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
> - uint32_t addr = le32_to_cpu(ep_table->structure_table_address);
> - int i, len, max_len = 0;
> +
> + smbios_entry_point *ep_table = &data->smbios_ep_table;
> + int i = 0, len, max_len = 0;
> uint8_t type, prv, crt;
> + uint64_t addr;
> +
> + if (ver == SMBIOS_VER21) {
> + addr = le32_to_cpu(ep_table->ep21.structure_table_address);
> + } else {
> + addr = le64_to_cpu(ep_table->ep30.structure_table_address);
> + }
>
> /* walk the smbios tables */
> - for (i = 0; i < le16_to_cpu(ep_table->number_of_structures); i++) {
> + do {
>
> /* grab type and formatted area length from struct header */
> type = qtest_readb(data->qts, addr);
> @@ -660,19 +705,23 @@ static void test_smbios_structs(test_data *data)
> }
>
> /* keep track of max. struct size */
> - if (max_len < len) {
> + if (ver == SMBIOS_VER21 && max_len < len) {
> max_len = len;
> - g_assert_cmpuint(max_len, <=, ep_table->max_structure_size);
> + g_assert_cmpuint(max_len, <=, ep_table->ep21.max_structure_size);
> }
>
> /* start of next structure */
> addr += len;
> - }
>
> - /* total table length and max struct size must match entry point values */
> - g_assert_cmpuint(le16_to_cpu(ep_table->structure_table_length), ==,
> - addr - le32_to_cpu(ep_table->structure_table_address));
> - g_assert_cmpuint(le16_to_cpu(ep_table->max_structure_size), ==, max_len);
> + } while (ver == SMBIOS_VER21 ?
> + (++i < le16_to_cpu(ep_table->ep21.number_of_structures)) : (type != 127));
This is quite an unreadable way to specify the loop condition. I
wonder if there is a better way to structure this.
Secondly, instead of hardcoding 127 why do we not say (type < SMBIOS_MAX_TYPE) ?
> +
> + if (ver == SMBIOS_VER21) {
> + /* total table length and max struct size must match entry point values */
> + g_assert_cmpuint(le16_to_cpu(ep_table->ep21.structure_table_length), ==,
> + addr - le32_to_cpu(ep_table->ep21.structure_table_address));
> + g_assert_cmpuint(le16_to_cpu(ep_table->ep21.max_structure_size), ==, max_len);
> + }
>
> /* required struct types must all be present */
> for (i = 0; i < data->required_struct_types_len; i++) {
> @@ -756,8 +805,8 @@ static void test_acpi_one(const char *params, test_data *data)
> * https://bugs.launchpad.net/qemu/+bug/1821884
> */
> if (!use_uefi) {
> - test_smbios_entry_point(data);
> - test_smbios_structs(data);
> + int ver = test_smbios_entry_point(data);
> + test_smbios_structs(data, ver);
> }
>
> qtest_quit(data->qts);
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] bios-tables-test: add test for number of cores > 255
2022-05-28 5:22 ` Ani Sinha
@ 2022-05-31 12:22 ` Julia Suvorova
2022-05-31 13:14 ` Ani Sinha
0 siblings, 1 reply; 28+ messages in thread
From: Julia Suvorova @ 2022-05-31 12:22 UTC (permalink / raw)
To: Ani Sinha; +Cc: QEMU Developers, Michael S. Tsirkin, Igor Mammedov
On Sat, May 28, 2022 at 7:22 AM Ani Sinha <ani@anisinha.ca> wrote:
>
>
>
> On Fri, 27 May 2022, Julia Suvorova wrote:
>
> > The new test is run with a large number of cpus and checks if the
> > core_count field in smbios_cpu_test (structure type 4) is correct.
> >
> > Choose q35 as it allows to run with -smp > 255.
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > ---
> > tests/qtest/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++-
> > 1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index 0ba9d749a5..f2464adaa0 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -100,6 +100,8 @@ typedef struct {
> > smbios_entry_point smbios_ep_table;
> > uint16_t smbios_cpu_max_speed;
> > uint16_t smbios_cpu_curr_speed;
> > + uint8_t smbios_core_count;
> > + uint16_t smbios_core_count2;
> > uint8_t *required_struct_types;
> > int required_struct_types_len;
> > QTestState *qts;
> > @@ -640,8 +642,9 @@ static inline bool smbios_single_instance(uint8_t type)
> >
> > static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > {
> > + uint8_t real_cc, expect_cc = data->smbios_core_count;
> > + uint16_t real, real_cc2, expect_cc2 = data->smbios_core_count2;
> > uint16_t expect_speed[2];
> > - uint16_t real;
>
> while you are at it, I suggest renaming this to real_speed or some such so
> that its better redeable.
Ok
> > int offset[2];
> > int i;
> >
> > @@ -660,6 +663,20 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > }
> > }
> >
> > + real_cc = qtest_readb(data->qts, addr + offsetof(struct smbios_type_4, core_count));
> > + real_cc2 = qtest_readw(data->qts, addr + offsetof(struct smbios_type_4, core_count2));
> > +
> > + if (expect_cc && (real_cc != expect_cc)) {
>
> I think better to say if ((expect_cc < 256) && (real_cc != expect_cc))
The check is not whether it fits into the field, but whether the field
is initialized.
> > + fprintf(stderr, "Unexpected SMBIOS CPU count: real %u expect %u\n",
> > + real_cc, expect_cc);
> > + return false;
> > + }
> > + if ((expect_cc == 0xFF) && (real_cc2 != expect_cc2)) {
> > + fprintf(stderr, "Unexpected SMBIOS CPU count2: real %u expect %u\n",
> > + real_cc2, expect_cc2);
> > + return false;
> > + }
> > +
> > return true;
> > }
> >
> > @@ -905,6 +922,21 @@ static void test_acpi_q35_tcg(void)
> > free_test_data(&data);
> > }
> >
> > +static void test_acpi_q35_tcg_core_count2(void)
> > +{
> > + test_data data = {
> > + .machine = MACHINE_Q35,
> > + .variant = ".core-count2",
> > + .required_struct_types = base_required_struct_types,
> > + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types),
> > + .smbios_core_count = 0xFF,
> > + .smbios_core_count2 = 275,
> > + };
> > +
> > + test_acpi_one("-machine smbios-entry-point-type=64 -smp 275", &data);
> > + free_test_data(&data);
> > +}
> > +
> > static void test_acpi_q35_tcg_bridge(void)
> > {
> > test_data data;
> > @@ -1787,6 +1819,7 @@ int main(int argc, char *argv[])
> > qtest_add_func("acpi/piix4/pci-hotplug/off",
> > test_acpi_piix4_no_acpi_pci_hotplug);
> > qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> > + qtest_add_func("acpi/q35/core-count2", test_acpi_q35_tcg_core_count2);
>
> How about checking thread count as well in the same test or in a
> different test?
Maybe a different test.
Best regards, Julia Suvorova.
> > qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> > qtest_add_func("acpi/q35/multif-bridge", test_acpi_q35_multif_bridge);
> > qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
> > --
> > 2.35.1
> >
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/5] bios-tables-test: teach test to use smbios 3.0 tables
2022-05-30 6:10 ` Ani Sinha
@ 2022-05-31 12:39 ` Julia Suvorova
0 siblings, 0 replies; 28+ messages in thread
From: Julia Suvorova @ 2022-05-31 12:39 UTC (permalink / raw)
To: Ani Sinha; +Cc: QEMU Developers, Michael S. Tsirkin, Igor Mammedov
On Mon, May 30, 2022 at 8:11 AM Ani Sinha <ani@anisinha.ca> wrote:
>
> On Fri, May 27, 2022 at 10:27 PM Julia Suvorova <jusual@redhat.com> wrote:
> >
> > Introduce the 64-bit entry point. Since we no longer have a total
> > number of structures, stop checking for the new ones at the EOF
> > structure (type 127).
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > ---
> > tests/qtest/bios-tables-test.c | 101 ++++++++++++++++++++++++---------
> > 1 file changed, 75 insertions(+), 26 deletions(-)
> >
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index a4a46e97f0..0ba9d749a5 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -75,6 +75,14 @@
> > #define OEM_TEST_ARGS "-machine x-oem-id=" OEM_ID ",x-oem-table-id=" \
> > OEM_TABLE_ID
> >
> > +#define SMBIOS_VER21 0
> > +#define SMBIOS_VER30 1
> > +
> > +typedef struct {
> > + struct smbios_21_entry_point ep21;
> > + struct smbios_30_entry_point ep30;
> > +} smbios_entry_point;
> > +
> > typedef struct {
> > bool tcg_only;
> > const char *machine;
> > @@ -88,8 +96,8 @@ typedef struct {
> > uint64_t rsdp_addr;
> > uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
> > GArray *tables;
> > - uint32_t smbios_ep_addr;
> > - struct smbios_21_entry_point smbios_ep_table;
> > + uint64_t smbios_ep_addr[2];
> > + smbios_entry_point smbios_ep_table;
> > uint16_t smbios_cpu_max_speed;
> > uint16_t smbios_cpu_curr_speed;
> > uint8_t *required_struct_types;
> > @@ -533,10 +541,10 @@ static void test_acpi_asl(test_data *data)
> > free_test_data(&exp_data);
> > }
> >
> > -static bool smbios_ep_table_ok(test_data *data)
> > +static bool smbios_ep2_table_ok(test_data *data)
> > {
> > - struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
> > - uint32_t addr = data->smbios_ep_addr;
> > + struct smbios_21_entry_point *ep_table = &data->smbios_ep_table.ep21;
> > + uint32_t addr = data->smbios_ep_addr[SMBIOS_VER21];
> >
> > qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
> > if (memcmp(ep_table->anchor_string, "_SM_", 4)) {
> > @@ -559,29 +567,59 @@ static bool smbios_ep_table_ok(test_data *data)
> > return true;
> > }
> >
> > -static void test_smbios_entry_point(test_data *data)
> > +static bool smbios_ep3_table_ok(test_data *data)
> > +{
> > + struct smbios_30_entry_point *ep_table = &data->smbios_ep_table.ep30;
> > + uint64_t addr = data->smbios_ep_addr[SMBIOS_VER30];
> > +
> > + qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
> > + if (memcmp(ep_table->anchor_string, "_SM3_", 5)) {
> > + return false;
> > + }
> > +
> > + if (acpi_calc_checksum((uint8_t *)ep_table, sizeof *ep_table)) {
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +static int test_smbios_entry_point(test_data *data)
> > {
> > uint32_t off;
> > + bool found_ep2 = false, found_ep3 = false;
> >
> > /* find smbios entry point structure */
> > for (off = 0xf0000; off < 0x100000; off += 0x10) {
> > - uint8_t sig[] = "_SM_";
> > + uint8_t sig[] = "_SM3_";
> > int i;
> >
> > for (i = 0; i < sizeof sig - 1; ++i) {
> > sig[i] = qtest_readb(data->qts, off + i);
> > }
> >
> > - if (!memcmp(sig, "_SM_", sizeof sig)) {
> > + if (!found_ep2 && !memcmp(sig, "_SM_", sizeof sig - 2)) {
> > /* signature match, but is this a valid entry point? */
> > - data->smbios_ep_addr = off;
> > - if (smbios_ep_table_ok(data)) {
> > - break;
> > + data->smbios_ep_addr[SMBIOS_VER21] = off;
> > + if (smbios_ep2_table_ok(data)) {
> > + found_ep2 = true;
> > + }
> > + } else if (!found_ep3 && !memcmp(sig, "_SM3_", sizeof sig - 1)) {
> > + data->smbios_ep_addr[SMBIOS_VER30] = off;
> > + if (smbios_ep3_table_ok(data)) {
> > + found_ep3 = true;
> > }
> > }
> > +
> > + if (found_ep2 || found_ep3) {
> > + break;
> > + }
> > }
> >
> > - g_assert_cmphex(off, <, 0x100000);
> > + g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER21], <, 0x100000);
> > + g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER30], <, 0x100000);
> > +
> > + return found_ep3 ? SMBIOS_VER30 : SMBIOS_VER21;
> > }
> >
> > static inline bool smbios_single_instance(uint8_t type)
> > @@ -625,16 +663,23 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > return true;
> > }
> >
> > -static void test_smbios_structs(test_data *data)
> > +static void test_smbios_structs(test_data *data, int ver)
> > {
> > DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 };
> > - struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
> > - uint32_t addr = le32_to_cpu(ep_table->structure_table_address);
> > - int i, len, max_len = 0;
> > +
> > + smbios_entry_point *ep_table = &data->smbios_ep_table;
> > + int i = 0, len, max_len = 0;
> > uint8_t type, prv, crt;
> > + uint64_t addr;
> > +
> > + if (ver == SMBIOS_VER21) {
> > + addr = le32_to_cpu(ep_table->ep21.structure_table_address);
> > + } else {
> > + addr = le64_to_cpu(ep_table->ep30.structure_table_address);
> > + }
> >
> > /* walk the smbios tables */
> > - for (i = 0; i < le16_to_cpu(ep_table->number_of_structures); i++) {
> > + do {
> >
> > /* grab type and formatted area length from struct header */
> > type = qtest_readb(data->qts, addr);
> > @@ -660,19 +705,23 @@ static void test_smbios_structs(test_data *data)
> > }
> >
> > /* keep track of max. struct size */
> > - if (max_len < len) {
> > + if (ver == SMBIOS_VER21 && max_len < len) {
> > max_len = len;
> > - g_assert_cmpuint(max_len, <=, ep_table->max_structure_size);
> > + g_assert_cmpuint(max_len, <=, ep_table->ep21.max_structure_size);
> > }
> >
> > /* start of next structure */
> > addr += len;
> > - }
> >
> > - /* total table length and max struct size must match entry point values */
> > - g_assert_cmpuint(le16_to_cpu(ep_table->structure_table_length), ==,
> > - addr - le32_to_cpu(ep_table->structure_table_address));
> > - g_assert_cmpuint(le16_to_cpu(ep_table->max_structure_size), ==, max_len);
> > + } while (ver == SMBIOS_VER21 ?
> > + (++i < le16_to_cpu(ep_table->ep21.number_of_structures)) : (type != 127));
>
> This is quite an unreadable way to specify the loop condition. I
> wonder if there is a better way to structure this.
> Secondly, instead of hardcoding 127 why do we not say (type < SMBIOS_MAX_TYPE) ?
Here we are looking for a special structure type 127. There can be up
to 256 structure types, and they can be placed before the final
structure.
But it seems like we need to change SMBIOS_MAX_TYPE in the test.
> > +
> > + if (ver == SMBIOS_VER21) {
> > + /* total table length and max struct size must match entry point values */
> > + g_assert_cmpuint(le16_to_cpu(ep_table->ep21.structure_table_length), ==,
> > + addr - le32_to_cpu(ep_table->ep21.structure_table_address));
> > + g_assert_cmpuint(le16_to_cpu(ep_table->ep21.max_structure_size), ==, max_len);
> > + }
> >
> > /* required struct types must all be present */
> > for (i = 0; i < data->required_struct_types_len; i++) {
> > @@ -756,8 +805,8 @@ static void test_acpi_one(const char *params, test_data *data)
> > * https://bugs.launchpad.net/qemu/+bug/1821884
> > */
> > if (!use_uefi) {
> > - test_smbios_entry_point(data);
> > - test_smbios_structs(data);
> > + int ver = test_smbios_entry_point(data);
> > + test_smbios_structs(data, ver);
> > }
> >
> > qtest_quit(data->qts);
> > --
> > 2.35.1
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] hw/smbios: add core_count2 to smbios table type 4
2022-05-28 4:34 ` Ani Sinha
@ 2022-05-31 12:40 ` Julia Suvorova
2022-06-02 14:31 ` Igor Mammedov
0 siblings, 1 reply; 28+ messages in thread
From: Julia Suvorova @ 2022-05-31 12:40 UTC (permalink / raw)
To: Ani Sinha; +Cc: QEMU Developers, Michael S. Tsirkin, Igor Mammedov
On Sat, May 28, 2022 at 6:34 AM Ani Sinha <ani@anisinha.ca> wrote:
>
>
>
> On Fri, 27 May 2022, Julia Suvorova wrote:
>
> > In order to use the increased number of cpus, we need to bring smbios
> > tables in line with the SMBIOS 3.0 specification. This allows us to
> > introduce core_count2 which acts as a duplicate of core_count if we have
> > fewer cores than 256, and contains the actual core number per socket if
> > we have more.
> >
> > core_enabled2 and thread_count2 fields work the same way.
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
>
> Other than the comment below,
> Reviewed-by: Ani Sinha <ani@anisinha.ca>
>
> > ---
> > include/hw/firmware/smbios.h | 3 +++
> > hw/smbios/smbios.c | 11 +++++++++--
> > 2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> > index 4b7ad77a44..c427ae5558 100644
> > --- a/include/hw/firmware/smbios.h
> > +++ b/include/hw/firmware/smbios.h
> > @@ -187,6 +187,9 @@ struct smbios_type_4 {
> > uint8_t thread_count;
> > uint16_t processor_characteristics;
> > uint16_t processor_family2;
> > + uint16_t core_count2;
> > + uint16_t core_enabled2;
> > + uint16_t thread_count2;
>
> I would add a comment along the lines of
> /* section 7.5, table 21 smbios spec version 3.0.0 */
Ok
> > } QEMU_PACKED;
> >
> > /* SMBIOS type 11 - OEM strings */
> > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> > index 60349ee402..45d7be6b30 100644
> > --- a/hw/smbios/smbios.c
> > +++ b/hw/smbios/smbios.c
> > @@ -709,8 +709,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
> > SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
> > SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
> > SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
> > - t->core_count = t->core_enabled = ms->smp.cores;
> > - t->thread_count = ms->smp.threads;
> > +
> > + t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
> > + t->core_enabled = t->core_count;
> > +
> > + t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
> > +
> > + t->thread_count = (ms->smp.threads > 255) ? 0xFF : ms->smp.threads;
> > + t->thread_count2 = cpu_to_le16(ms->smp.threads);
> > +
> > t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
> > t->processor_family2 = cpu_to_le16(0x01); /* Other */
> >
> > --
> > 2.35.1
> >
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] bios-tables-test: add test for number of cores > 255
2022-05-31 12:22 ` Julia Suvorova
@ 2022-05-31 13:14 ` Ani Sinha
2022-05-31 15:05 ` Julia Suvorova
0 siblings, 1 reply; 28+ messages in thread
From: Ani Sinha @ 2022-05-31 13:14 UTC (permalink / raw)
To: Julia Suvorova; +Cc: QEMU Developers, Michael S. Tsirkin, Igor Mammedov
On Tue, May 31, 2022 at 5:53 PM Julia Suvorova <jusual@redhat.com> wrote:
>
> On Sat, May 28, 2022 at 7:22 AM Ani Sinha <ani@anisinha.ca> wrote:
> >
> >
> >
> > On Fri, 27 May 2022, Julia Suvorova wrote:
> >
> > > The new test is run with a large number of cpus and checks if the
> > > core_count field in smbios_cpu_test (structure type 4) is correct.
> > >
> > > Choose q35 as it allows to run with -smp > 255.
> > >
> > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > ---
> > > tests/qtest/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++-
> > > 1 file changed, 34 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > > index 0ba9d749a5..f2464adaa0 100644
> > > --- a/tests/qtest/bios-tables-test.c
> > > +++ b/tests/qtest/bios-tables-test.c
> > > @@ -100,6 +100,8 @@ typedef struct {
> > > smbios_entry_point smbios_ep_table;
> > > uint16_t smbios_cpu_max_speed;
> > > uint16_t smbios_cpu_curr_speed;
> > > + uint8_t smbios_core_count;
> > > + uint16_t smbios_core_count2;
> > > uint8_t *required_struct_types;
> > > int required_struct_types_len;
> > > QTestState *qts;
> > > @@ -640,8 +642,9 @@ static inline bool smbios_single_instance(uint8_t type)
> > >
> > > static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > > {
> > > + uint8_t real_cc, expect_cc = data->smbios_core_count;
> > > + uint16_t real, real_cc2, expect_cc2 = data->smbios_core_count2;
> > > uint16_t expect_speed[2];
> > > - uint16_t real;
> >
> > while you are at it, I suggest renaming this to real_speed or some such so
> > that its better redeable.
>
> Ok
>
> > > int offset[2];
> > > int i;
> > >
> > > @@ -660,6 +663,20 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > > }
> > > }
> > >
> > > + real_cc = qtest_readb(data->qts, addr + offsetof(struct smbios_type_4, core_count));
> > > + real_cc2 = qtest_readw(data->qts, addr + offsetof(struct smbios_type_4, core_count2));
> > > +
> > > + if (expect_cc && (real_cc != expect_cc)) {
> >
> > I think better to say if ((expect_cc < 256) && (real_cc != expect_cc))
>
> The check is not whether it fits into the field, but whether the field
> is initialized.
yes so the real_cc will contain the actual value of core count only
when the core count value is less than 256. This value should be the
same as the expect_cc (the cc value we pass). Is this not what is
being tested?
>
> > > + fprintf(stderr, "Unexpected SMBIOS CPU count: real %u expect %u\n",
> > > + real_cc, expect_cc);
> > > + return false;
> > > + }
> > > + if ((expect_cc == 0xFF) && (real_cc2 != expect_cc2)) {
> > > + fprintf(stderr, "Unexpected SMBIOS CPU count2: real %u expect %u\n",
> > > + real_cc2, expect_cc2);
> > > + return false;
> > > + }
> > > +
> > > return true;
> > > }
> > >
> > > @@ -905,6 +922,21 @@ static void test_acpi_q35_tcg(void)
> > > free_test_data(&data);
> > > }
> > >
> > > +static void test_acpi_q35_tcg_core_count2(void)
> > > +{
> > > + test_data data = {
> > > + .machine = MACHINE_Q35,
> > > + .variant = ".core-count2",
> > > + .required_struct_types = base_required_struct_types,
> > > + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types),
> > > + .smbios_core_count = 0xFF,
> > > + .smbios_core_count2 = 275,
> > > + };
> > > +
> > > + test_acpi_one("-machine smbios-entry-point-type=64 -smp 275", &data);
> > > + free_test_data(&data);
> > > +}
> > > +
> > > static void test_acpi_q35_tcg_bridge(void)
> > > {
> > > test_data data;
> > > @@ -1787,6 +1819,7 @@ int main(int argc, char *argv[])
> > > qtest_add_func("acpi/piix4/pci-hotplug/off",
> > > test_acpi_piix4_no_acpi_pci_hotplug);
> > > qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> > > + qtest_add_func("acpi/q35/core-count2", test_acpi_q35_tcg_core_count2);
> >
> > How about checking thread count as well in the same test or in a
> > different test?
>
> Maybe a different test.
>
> Best regards, Julia Suvorova.
>
> > > qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> > > qtest_add_func("acpi/q35/multif-bridge", test_acpi_q35_multif_bridge);
> > > qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
> > > --
> > > 2.35.1
> > >
> > >
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] bios-tables-test: add test for number of cores > 255
2022-05-31 13:14 ` Ani Sinha
@ 2022-05-31 15:05 ` Julia Suvorova
2022-06-01 5:09 ` Ani Sinha
0 siblings, 1 reply; 28+ messages in thread
From: Julia Suvorova @ 2022-05-31 15:05 UTC (permalink / raw)
To: Ani Sinha; +Cc: QEMU Developers, Michael S. Tsirkin, Igor Mammedov
On Tue, May 31, 2022 at 3:14 PM Ani Sinha <ani@anisinha.ca> wrote:
>
> On Tue, May 31, 2022 at 5:53 PM Julia Suvorova <jusual@redhat.com> wrote:
> >
> > On Sat, May 28, 2022 at 7:22 AM Ani Sinha <ani@anisinha.ca> wrote:
> > >
> > >
> > >
> > > On Fri, 27 May 2022, Julia Suvorova wrote:
> > >
> > > > The new test is run with a large number of cpus and checks if the
> > > > core_count field in smbios_cpu_test (structure type 4) is correct.
> > > >
> > > > Choose q35 as it allows to run with -smp > 255.
> > > >
> > > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > > ---
> > > > tests/qtest/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++-
> > > > 1 file changed, 34 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > > > index 0ba9d749a5..f2464adaa0 100644
> > > > --- a/tests/qtest/bios-tables-test.c
> > > > +++ b/tests/qtest/bios-tables-test.c
> > > > @@ -100,6 +100,8 @@ typedef struct {
> > > > smbios_entry_point smbios_ep_table;
> > > > uint16_t smbios_cpu_max_speed;
> > > > uint16_t smbios_cpu_curr_speed;
> > > > + uint8_t smbios_core_count;
> > > > + uint16_t smbios_core_count2;
> > > > uint8_t *required_struct_types;
> > > > int required_struct_types_len;
> > > > QTestState *qts;
> > > > @@ -640,8 +642,9 @@ static inline bool smbios_single_instance(uint8_t type)
> > > >
> > > > static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > > > {
> > > > + uint8_t real_cc, expect_cc = data->smbios_core_count;
> > > > + uint16_t real, real_cc2, expect_cc2 = data->smbios_core_count2;
> > > > uint16_t expect_speed[2];
> > > > - uint16_t real;
> > >
> > > while you are at it, I suggest renaming this to real_speed or some such so
> > > that its better redeable.
> >
> > Ok
> >
> > > > int offset[2];
> > > > int i;
> > > >
> > > > @@ -660,6 +663,20 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > > > }
> > > > }
> > > >
> > > > + real_cc = qtest_readb(data->qts, addr + offsetof(struct smbios_type_4, core_count));
> > > > + real_cc2 = qtest_readw(data->qts, addr + offsetof(struct smbios_type_4, core_count2));
> > > > +
> > > > + if (expect_cc && (real_cc != expect_cc)) {
> > >
> > > I think better to say if ((expect_cc < 256) && (real_cc != expect_cc))
> >
> > The check is not whether it fits into the field, but whether the field
> > is initialized.
>
> yes so the real_cc will contain the actual value of core count only
> when the core count value is less than 256. This value should be the
> same as the expect_cc (the cc value we pass). Is this not what is
> being tested?
The real_cc should always be equal to expect_cc (which is 0xFF with
-smp 275). So if the core count is less than 256, this checks for the
actual core counter, and if it's over, it checks if real_cc is equal
to 0xFF, which eliminates several unnecessary comparisons. If we
didn't initialize expect_cc in the test, the value is undefined, and
we shouldn't check anything.
Best regards, Julia Suvorova.
> >
> > > > + fprintf(stderr, "Unexpected SMBIOS CPU count: real %u expect %u\n",
> > > > + real_cc, expect_cc);
> > > > + return false;
> > > > + }
> > > > + if ((expect_cc == 0xFF) && (real_cc2 != expect_cc2)) {
> > > > + fprintf(stderr, "Unexpected SMBIOS CPU count2: real %u expect %u\n",
> > > > + real_cc2, expect_cc2);
> > > > + return false;
> > > > + }
> > > > +
> > > > return true;
> > > > }
> > > >
> > > > @@ -905,6 +922,21 @@ static void test_acpi_q35_tcg(void)
> > > > free_test_data(&data);
> > > > }
> > > >
> > > > +static void test_acpi_q35_tcg_core_count2(void)
> > > > +{
> > > > + test_data data = {
> > > > + .machine = MACHINE_Q35,
> > > > + .variant = ".core-count2",
> > > > + .required_struct_types = base_required_struct_types,
> > > > + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types),
> > > > + .smbios_core_count = 0xFF,
> > > > + .smbios_core_count2 = 275,
> > > > + };
> > > > +
> > > > + test_acpi_one("-machine smbios-entry-point-type=64 -smp 275", &data);
> > > > + free_test_data(&data);
> > > > +}
> > > > +
> > > > static void test_acpi_q35_tcg_bridge(void)
> > > > {
> > > > test_data data;
> > > > @@ -1787,6 +1819,7 @@ int main(int argc, char *argv[])
> > > > qtest_add_func("acpi/piix4/pci-hotplug/off",
> > > > test_acpi_piix4_no_acpi_pci_hotplug);
> > > > qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> > > > + qtest_add_func("acpi/q35/core-count2", test_acpi_q35_tcg_core_count2);
> > >
> > > How about checking thread count as well in the same test or in a
> > > different test?
> >
> > Maybe a different test.
> >
> > Best regards, Julia Suvorova.
> >
> > > > qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> > > > qtest_add_func("acpi/q35/multif-bridge", test_acpi_q35_multif_bridge);
> > > > qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
> > > > --
> > > > 2.35.1
> > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] bios-tables-test: add test for number of cores > 255
2022-05-27 16:56 ` [PATCH 4/5] bios-tables-test: add test for number of cores > 255 Julia Suvorova
2022-05-28 5:22 ` Ani Sinha
@ 2022-06-01 5:06 ` Ani Sinha
2022-06-02 15:20 ` Igor Mammedov
2 siblings, 0 replies; 28+ messages in thread
From: Ani Sinha @ 2022-06-01 5:06 UTC (permalink / raw)
To: Julia Suvorova; +Cc: qemu-devel, Michael S. Tsirkin, Igor Mammedov
On Fri, May 27, 2022 at 10:27 PM Julia Suvorova <jusual@redhat.com> wrote:
>
> The new test is run with a large number of cpus and checks if the
> core_count field in smbios_cpu_test (structure type 4) is correct.
>
> Choose q35 as it allows to run with -smp > 255.
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
> ---
> tests/qtest/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 0ba9d749a5..f2464adaa0 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -100,6 +100,8 @@ typedef struct {
> smbios_entry_point smbios_ep_table;
> uint16_t smbios_cpu_max_speed;
> uint16_t smbios_cpu_curr_speed;
> + uint8_t smbios_core_count;
> + uint16_t smbios_core_count2;
> uint8_t *required_struct_types;
> int required_struct_types_len;
> QTestState *qts;
> @@ -640,8 +642,9 @@ static inline bool smbios_single_instance(uint8_t type)
>
> static bool smbios_cpu_test(test_data *data, uint32_t addr)
> {
> + uint8_t real_cc, expect_cc = data->smbios_core_count;
> + uint16_t real, real_cc2, expect_cc2 = data->smbios_core_count2;
> uint16_t expect_speed[2];
> - uint16_t real;
> int offset[2];
> int i;
>
> @@ -660,6 +663,20 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> }
> }
>
> + real_cc = qtest_readb(data->qts, addr + offsetof(struct smbios_type_4, core_count));
> + real_cc2 = qtest_readw(data->qts, addr + offsetof(struct smbios_type_4, core_count2));
> +
> + if (expect_cc && (real_cc != expect_cc)) {
> + fprintf(stderr, "Unexpected SMBIOS CPU count: real %u expect %u\n",
> + real_cc, expect_cc);
> + return false;
> + }
> + if ((expect_cc == 0xFF) && (real_cc2 != expect_cc2)) {
> + fprintf(stderr, "Unexpected SMBIOS CPU count2: real %u expect %u\n",
> + real_cc2, expect_cc2);
> + return false;
> + }
> +
> return true;
> }
>
> @@ -905,6 +922,21 @@ static void test_acpi_q35_tcg(void)
> free_test_data(&data);
> }
>
> +static void test_acpi_q35_tcg_core_count2(void)
> +{
> + test_data data = {
> + .machine = MACHINE_Q35,
> + .variant = ".core-count2",
> + .required_struct_types = base_required_struct_types,
> + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types),
> + .smbios_core_count = 0xFF,
> + .smbios_core_count2 = 275,
> + };
> +
> + test_acpi_one("-machine smbios-entry-point-type=64 -smp 275", &data);
> + free_test_data(&data);
> +}
> +
> static void test_acpi_q35_tcg_bridge(void)
> {
> test_data data;
> @@ -1787,6 +1819,7 @@ int main(int argc, char *argv[])
> qtest_add_func("acpi/piix4/pci-hotplug/off",
> test_acpi_piix4_no_acpi_pci_hotplug);
> qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> + qtest_add_func("acpi/q35/core-count2", test_acpi_q35_tcg_core_count2);
> qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> qtest_add_func("acpi/q35/multif-bridge", test_acpi_q35_multif_bridge);
> qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] bios-tables-test: add test for number of cores > 255
2022-05-31 15:05 ` Julia Suvorova
@ 2022-06-01 5:09 ` Ani Sinha
0 siblings, 0 replies; 28+ messages in thread
From: Ani Sinha @ 2022-06-01 5:09 UTC (permalink / raw)
To: Julia Suvorova; +Cc: QEMU Developers, Michael S. Tsirkin, Igor Mammedov
On Tue, May 31, 2022 at 8:36 PM Julia Suvorova <jusual@redhat.com> wrote:
>
> On Tue, May 31, 2022 at 3:14 PM Ani Sinha <ani@anisinha.ca> wrote:
> >
> > On Tue, May 31, 2022 at 5:53 PM Julia Suvorova <jusual@redhat.com> wrote:
> > >
> > > On Sat, May 28, 2022 at 7:22 AM Ani Sinha <ani@anisinha.ca> wrote:
> > > >
> > > >
> > > >
> > > > On Fri, 27 May 2022, Julia Suvorova wrote:
> > > >
> > > > > The new test is run with a large number of cpus and checks if the
> > > > > core_count field in smbios_cpu_test (structure type 4) is correct.
> > > > >
> > > > > Choose q35 as it allows to run with -smp > 255.
> > > > >
> > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > > > ---
> > > > > tests/qtest/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++-
> > > > > 1 file changed, 34 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > > > > index 0ba9d749a5..f2464adaa0 100644
> > > > > --- a/tests/qtest/bios-tables-test.c
> > > > > +++ b/tests/qtest/bios-tables-test.c
> > > > > @@ -100,6 +100,8 @@ typedef struct {
> > > > > smbios_entry_point smbios_ep_table;
> > > > > uint16_t smbios_cpu_max_speed;
> > > > > uint16_t smbios_cpu_curr_speed;
> > > > > + uint8_t smbios_core_count;
> > > > > + uint16_t smbios_core_count2;
> > > > > uint8_t *required_struct_types;
> > > > > int required_struct_types_len;
> > > > > QTestState *qts;
> > > > > @@ -640,8 +642,9 @@ static inline bool smbios_single_instance(uint8_t type)
> > > > >
> > > > > static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > > > > {
> > > > > + uint8_t real_cc, expect_cc = data->smbios_core_count;
> > > > > + uint16_t real, real_cc2, expect_cc2 = data->smbios_core_count2;
> > > > > uint16_t expect_speed[2];
> > > > > - uint16_t real;
> > > >
> > > > while you are at it, I suggest renaming this to real_speed or some such so
> > > > that its better redeable.
> > >
> > > Ok
> > >
> > > > > int offset[2];
> > > > > int i;
> > > > >
> > > > > @@ -660,6 +663,20 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > > > > }
> > > > > }
> > > > >
> > > > > + real_cc = qtest_readb(data->qts, addr + offsetof(struct smbios_type_4, core_count));
> > > > > + real_cc2 = qtest_readw(data->qts, addr + offsetof(struct smbios_type_4, core_count2));
> > > > > +
> > > > > + if (expect_cc && (real_cc != expect_cc)) {
> > > >
> > > > I think better to say if ((expect_cc < 256) && (real_cc != expect_cc))
> > >
> > > The check is not whether it fits into the field, but whether the field
> > > is initialized.
> >
> > yes so the real_cc will contain the actual value of core count only
> > when the core count value is less than 256. This value should be the
> > same as the expect_cc (the cc value we pass). Is this not what is
> > being tested?
>
> The real_cc should always be equal to expect_cc (which is 0xFF with
> -smp 275). So if the core count is less than 256, this checks for the
> actual core counter, and if it's over, it checks if real_cc is equal
> to 0xFF, which eliminates several unnecessary comparisons. If we
> didn't initialize expect_cc in the test, the value is undefined, and
> we shouldn't check anything.
I have convinced myself that the logic is correct. You are right in
that real_cc should always be equal to expect_cc when expect_cc is set
(either to actual core count or to 0xff when number of cores > 255).
>
> Best regards, Julia Suvorova.
>
> > >
> > > > > + fprintf(stderr, "Unexpected SMBIOS CPU count: real %u expect %u\n",
> > > > > + real_cc, expect_cc);
> > > > > + return false;
> > > > > + }
> > > > > + if ((expect_cc == 0xFF) && (real_cc2 != expect_cc2)) {
> > > > > + fprintf(stderr, "Unexpected SMBIOS CPU count2: real %u expect %u\n",
> > > > > + real_cc2, expect_cc2);
> > > > > + return false;
> > > > > + }
> > > > > +
> > > > > return true;
> > > > > }
> > > > >
> > > > > @@ -905,6 +922,21 @@ static void test_acpi_q35_tcg(void)
> > > > > free_test_data(&data);
> > > > > }
> > > > >
> > > > > +static void test_acpi_q35_tcg_core_count2(void)
> > > > > +{
> > > > > + test_data data = {
> > > > > + .machine = MACHINE_Q35,
> > > > > + .variant = ".core-count2",
> > > > > + .required_struct_types = base_required_struct_types,
> > > > > + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types),
> > > > > + .smbios_core_count = 0xFF,
> > > > > + .smbios_core_count2 = 275,
> > > > > + };
> > > > > +
> > > > > + test_acpi_one("-machine smbios-entry-point-type=64 -smp 275", &data);
> > > > > + free_test_data(&data);
> > > > > +}
> > > > > +
> > > > > static void test_acpi_q35_tcg_bridge(void)
> > > > > {
> > > > > test_data data;
> > > > > @@ -1787,6 +1819,7 @@ int main(int argc, char *argv[])
> > > > > qtest_add_func("acpi/piix4/pci-hotplug/off",
> > > > > test_acpi_piix4_no_acpi_pci_hotplug);
> > > > > qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> > > > > + qtest_add_func("acpi/q35/core-count2", test_acpi_q35_tcg_core_count2);
> > > >
> > > > How about checking thread count as well in the same test or in a
> > > > different test?
> > >
> > > Maybe a different test.
> > >
> > > Best regards, Julia Suvorova.
> > >
> > > > > qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> > > > > qtest_add_func("acpi/q35/multif-bridge", test_acpi_q35_multif_bridge);
> > > > > qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
> > > > > --
> > > > > 2.35.1
> > > > >
> > > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] hw/smbios: add core_count2 to smbios table type 4
2022-05-31 12:40 ` Julia Suvorova
@ 2022-06-02 14:31 ` Igor Mammedov
2022-06-02 14:35 ` Igor Mammedov
0 siblings, 1 reply; 28+ messages in thread
From: Igor Mammedov @ 2022-06-02 14:31 UTC (permalink / raw)
To: Julia Suvorova; +Cc: Ani Sinha, QEMU Developers, Michael S. Tsirkin
On Tue, 31 May 2022 14:40:15 +0200
Julia Suvorova <jusual@redhat.com> wrote:
> On Sat, May 28, 2022 at 6:34 AM Ani Sinha <ani@anisinha.ca> wrote:
> >
> >
> >
> > On Fri, 27 May 2022, Julia Suvorova wrote:
> >
> > > In order to use the increased number of cpus, we need to bring smbios
> > > tables in line with the SMBIOS 3.0 specification. This allows us to
> > > introduce core_count2 which acts as a duplicate of core_count if we have
> > > fewer cores than 256, and contains the actual core number per socket if
> > > we have more.
> > >
> > > core_enabled2 and thread_count2 fields work the same way.
> > >
> > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> >
> > Other than the comment below,
> > Reviewed-by: Ani Sinha <ani@anisinha.ca>
> >
> > > ---
> > > include/hw/firmware/smbios.h | 3 +++
> > > hw/smbios/smbios.c | 11 +++++++++--
> > > 2 files changed, 12 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> > > index 4b7ad77a44..c427ae5558 100644
> > > --- a/include/hw/firmware/smbios.h
> > > +++ b/include/hw/firmware/smbios.h
> > > @@ -187,6 +187,9 @@ struct smbios_type_4 {
> > > uint8_t thread_count;
> > > uint16_t processor_characteristics;
> > > uint16_t processor_family2;
> > > + uint16_t core_count2;
> > > + uint16_t core_enabled2;
> > > + uint16_t thread_count2;
> >
> > I would add a comment along the lines of
> > /* section 7.5, table 21 smbios spec version 3.0.0 */
>
> Ok
With Ani's comment fixed
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
> > > } QEMU_PACKED;
> > >
> > > /* SMBIOS type 11 - OEM strings */
> > > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> > > index 60349ee402..45d7be6b30 100644
> > > --- a/hw/smbios/smbios.c
> > > +++ b/hw/smbios/smbios.c
> > > @@ -709,8 +709,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
> > > SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
> > > SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
> > > SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
> > > - t->core_count = t->core_enabled = ms->smp.cores;
> > > - t->thread_count = ms->smp.threads;
> > > +
> > > + t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
> > > + t->core_enabled = t->core_count;
> > > +
> > > + t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
> > > +
> > > + t->thread_count = (ms->smp.threads > 255) ? 0xFF : ms->smp.threads;
> > > + t->thread_count2 = cpu_to_le16(ms->smp.threads);
> > > +
> > > t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
> > > t->processor_family2 = cpu_to_le16(0x01); /* Other */
> > >
> > > --
> > > 2.35.1
> > >
> > >
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] hw/smbios: add core_count2 to smbios table type 4
2022-06-02 14:31 ` Igor Mammedov
@ 2022-06-02 14:35 ` Igor Mammedov
2022-06-06 11:11 ` Julia Suvorova
0 siblings, 1 reply; 28+ messages in thread
From: Igor Mammedov @ 2022-06-02 14:35 UTC (permalink / raw)
To: Julia Suvorova; +Cc: Ani Sinha, QEMU Developers, Michael S. Tsirkin
On Thu, 2 Jun 2022 16:31:25 +0200
Igor Mammedov <imammedo@redhat.com> wrote:
> On Tue, 31 May 2022 14:40:15 +0200
> Julia Suvorova <jusual@redhat.com> wrote:
>
> > On Sat, May 28, 2022 at 6:34 AM Ani Sinha <ani@anisinha.ca> wrote:
> > >
> > >
> > >
> > > On Fri, 27 May 2022, Julia Suvorova wrote:
> > >
> > > > In order to use the increased number of cpus, we need to bring smbios
> > > > tables in line with the SMBIOS 3.0 specification. This allows us to
> > > > introduce core_count2 which acts as a duplicate of core_count if we have
> > > > fewer cores than 256, and contains the actual core number per socket if
> > > > we have more.
> > > >
> > > > core_enabled2 and thread_count2 fields work the same way.
> > > >
> > > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > >
> > > Other than the comment below,
> > > Reviewed-by: Ani Sinha <ani@anisinha.ca>
> > >
> > > > ---
> > > > include/hw/firmware/smbios.h | 3 +++
> > > > hw/smbios/smbios.c | 11 +++++++++--
> > > > 2 files changed, 12 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> > > > index 4b7ad77a44..c427ae5558 100644
> > > > --- a/include/hw/firmware/smbios.h
> > > > +++ b/include/hw/firmware/smbios.h
> > > > @@ -187,6 +187,9 @@ struct smbios_type_4 {
> > > > uint8_t thread_count;
> > > > uint16_t processor_characteristics;
> > > > uint16_t processor_family2;
> > > > + uint16_t core_count2;
> > > > + uint16_t core_enabled2;
> > > > + uint16_t thread_count2;
on the other hand,
is it ok unconditionally extend type 4 and use v3 structure
if qemu was started with v2 smbios?
> > >
> > > I would add a comment along the lines of
> > > /* section 7.5, table 21 smbios spec version 3.0.0 */
> >
> > Ok
>
> With Ani's comment fixed
>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
> >
> > > > } QEMU_PACKED;
> > > >
> > > > /* SMBIOS type 11 - OEM strings */
> > > > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> > > > index 60349ee402..45d7be6b30 100644
> > > > --- a/hw/smbios/smbios.c
> > > > +++ b/hw/smbios/smbios.c
> > > > @@ -709,8 +709,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
> > > > SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
> > > > SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
> > > > SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
> > > > - t->core_count = t->core_enabled = ms->smp.cores;
> > > > - t->thread_count = ms->smp.threads;
> > > > +
> > > > + t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
> > > > + t->core_enabled = t->core_count;
> > > > +
> > > > + t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
> > > > +
> > > > + t->thread_count = (ms->smp.threads > 255) ? 0xFF : ms->smp.threads;
> > > > + t->thread_count2 = cpu_to_le16(ms->smp.threads);
> > > > +
> > > > t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
> > > > t->processor_family2 = cpu_to_le16(0x01); /* Other */
> > > >
> > > > --
> > > > 2.35.1
> > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/5] bios-tables-test: teach test to use smbios 3.0 tables
2022-05-27 16:56 ` [PATCH 2/5] bios-tables-test: teach test to use smbios 3.0 tables Julia Suvorova
2022-05-30 6:10 ` Ani Sinha
@ 2022-06-02 15:04 ` Igor Mammedov
2022-06-06 10:52 ` Julia Suvorova
1 sibling, 1 reply; 28+ messages in thread
From: Igor Mammedov @ 2022-06-02 15:04 UTC (permalink / raw)
To: Julia Suvorova; +Cc: qemu-devel, Michael S. Tsirkin, Ani Sinha
On Fri, 27 May 2022 18:56:48 +0200
Julia Suvorova <jusual@redhat.com> wrote:
> Introduce the 64-bit entry point. Since we no longer have a total
> number of structures, stop checking for the new ones at the EOF
> structure (type 127).
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> ---
> tests/qtest/bios-tables-test.c | 101 ++++++++++++++++++++++++---------
> 1 file changed, 75 insertions(+), 26 deletions(-)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index a4a46e97f0..0ba9d749a5 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -75,6 +75,14 @@
> #define OEM_TEST_ARGS "-machine x-oem-id=" OEM_ID ",x-oem-table-id=" \
> OEM_TABLE_ID
>
> +#define SMBIOS_VER21 0
> +#define SMBIOS_VER30 1
> +
> +typedef struct {
> + struct smbios_21_entry_point ep21;
> + struct smbios_30_entry_point ep30;
> +} smbios_entry_point;
> +
> typedef struct {
> bool tcg_only;
> const char *machine;
> @@ -88,8 +96,8 @@ typedef struct {
> uint64_t rsdp_addr;
> uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
> GArray *tables;
> - uint32_t smbios_ep_addr;
> - struct smbios_21_entry_point smbios_ep_table;
> + uint64_t smbios_ep_addr[2];
> + smbios_entry_point smbios_ep_table;
> uint16_t smbios_cpu_max_speed;
> uint16_t smbios_cpu_curr_speed;
> uint8_t *required_struct_types;
> @@ -533,10 +541,10 @@ static void test_acpi_asl(test_data *data)
> free_test_data(&exp_data);
> }
>
> -static bool smbios_ep_table_ok(test_data *data)
> +static bool smbios_ep2_table_ok(test_data *data)
> {
> - struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
> - uint32_t addr = data->smbios_ep_addr;
> + struct smbios_21_entry_point *ep_table = &data->smbios_ep_table.ep21;
> + uint32_t addr = data->smbios_ep_addr[SMBIOS_VER21];
>
> qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
> if (memcmp(ep_table->anchor_string, "_SM_", 4)) {
> @@ -559,29 +567,59 @@ static bool smbios_ep_table_ok(test_data *data)
> return true;
> }
>
> -static void test_smbios_entry_point(test_data *data)
> +static bool smbios_ep3_table_ok(test_data *data)
> +{
> + struct smbios_30_entry_point *ep_table = &data->smbios_ep_table.ep30;
> + uint64_t addr = data->smbios_ep_addr[SMBIOS_VER30];
> +
> + qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
> + if (memcmp(ep_table->anchor_string, "_SM3_", 5)) {
> + return false;
> + }
> +
> + if (acpi_calc_checksum((uint8_t *)ep_table, sizeof *ep_table)) {
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static int test_smbios_entry_point(test_data *data)
> {
> uint32_t off;
> + bool found_ep2 = false, found_ep3 = false;
>
> /* find smbios entry point structure */
> for (off = 0xf0000; off < 0x100000; off += 0x10) {
> - uint8_t sig[] = "_SM_";
> + uint8_t sig[] = "_SM3_";
well I'd just add a separate sig3
> int i;
>
> for (i = 0; i < sizeof sig - 1; ++i) {
> sig[i] = qtest_readb(data->qts, off + i);
> }
>
> - if (!memcmp(sig, "_SM_", sizeof sig)) {
> + if (!found_ep2 && !memcmp(sig, "_SM_", sizeof sig - 2)) {
keep original v2 code and just add similar chunk for v3,
drop found_foo locals,
that should make it easier to read/follow
(i.e. less conditions to think about and no magic fiddling with the length of signature)
> /* signature match, but is this a valid entry point? */
> - data->smbios_ep_addr = off;
> - if (smbios_ep_table_ok(data)) {
> - break;
> + data->smbios_ep_addr[SMBIOS_VER21] = off;
> + if (smbios_ep2_table_ok(data)) {
> + found_ep2 = true;
> + }
> + } else if (!found_ep3 && !memcmp(sig, "_SM3_", sizeof sig - 1)) {
> + data->smbios_ep_addr[SMBIOS_VER30] = off;
> + if (smbios_ep3_table_ok(data)) {
> + found_ep3 = true;
> }
> }
> +
> + if (found_ep2 || found_ep3) {
> + break;
> + }
> }
>
> - g_assert_cmphex(off, <, 0x100000);
> + g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER21], <, 0x100000);
> + g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER30], <, 0x100000);
> +
> + return found_ep3 ? SMBIOS_VER30 : SMBIOS_VER21;
and use content of data->smbios_ep_addr[] to return found version
> }
>
> static inline bool smbios_single_instance(uint8_t type)
> @@ -625,16 +663,23 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> return true;
> }
>
> -static void test_smbios_structs(test_data *data)
> +static void test_smbios_structs(test_data *data, int ver)
> {
> DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 };
> - struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
> - uint32_t addr = le32_to_cpu(ep_table->structure_table_address);
> - int i, len, max_len = 0;
> +
> + smbios_entry_point *ep_table = &data->smbios_ep_table;
> + int i = 0, len, max_len = 0;
> uint8_t type, prv, crt;
> + uint64_t addr;
> +
> + if (ver == SMBIOS_VER21) {
> + addr = le32_to_cpu(ep_table->ep21.structure_table_address);
> + } else {
> + addr = le64_to_cpu(ep_table->ep30.structure_table_address);
> + }
>
> /* walk the smbios tables */
> - for (i = 0; i < le16_to_cpu(ep_table->number_of_structures); i++) {
> + do {
>
> /* grab type and formatted area length from struct header */
> type = qtest_readb(data->qts, addr);
> @@ -660,19 +705,23 @@ static void test_smbios_structs(test_data *data)
> }
>
> /* keep track of max. struct size */
> - if (max_len < len) {
> + if (ver == SMBIOS_VER21 && max_len < len) {
> max_len = len;
> - g_assert_cmpuint(max_len, <=, ep_table->max_structure_size);
> + g_assert_cmpuint(max_len, <=, ep_table->ep21.max_structure_size);
> }
>
> /* start of next structure */
> addr += len;
> - }
>
> - /* total table length and max struct size must match entry point values */
> - g_assert_cmpuint(le16_to_cpu(ep_table->structure_table_length), ==,
> - addr - le32_to_cpu(ep_table->structure_table_address));
> - g_assert_cmpuint(le16_to_cpu(ep_table->max_structure_size), ==, max_len);
> + } while (ver == SMBIOS_VER21 ?
> + (++i < le16_to_cpu(ep_table->ep21.number_of_structures)) : (type != 127));
> +
> + if (ver == SMBIOS_VER21) {
> + /* total table length and max struct size must match entry point values */
> + g_assert_cmpuint(le16_to_cpu(ep_table->ep21.structure_table_length), ==,
> + addr - le32_to_cpu(ep_table->ep21.structure_table_address));
> + g_assert_cmpuint(le16_to_cpu(ep_table->ep21.max_structure_size), ==, max_len);
> + }
>
> /* required struct types must all be present */
> for (i = 0; i < data->required_struct_types_len; i++) {
> @@ -756,8 +805,8 @@ static void test_acpi_one(const char *params, test_data *data)
> * https://bugs.launchpad.net/qemu/+bug/1821884
> */
> if (!use_uefi) {
> - test_smbios_entry_point(data);
> - test_smbios_structs(data);
> + int ver = test_smbios_entry_point(data);
> + test_smbios_structs(data, ver);
> }
>
> qtest_quit(data->qts);
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] bios-tables-test: add test for number of cores > 255
2022-05-27 16:56 ` [PATCH 4/5] bios-tables-test: add test for number of cores > 255 Julia Suvorova
2022-05-28 5:22 ` Ani Sinha
2022-06-01 5:06 ` Ani Sinha
@ 2022-06-02 15:20 ` Igor Mammedov
2022-06-02 16:31 ` Ani Sinha
2022-06-06 11:38 ` Julia Suvorova
2 siblings, 2 replies; 28+ messages in thread
From: Igor Mammedov @ 2022-06-02 15:20 UTC (permalink / raw)
To: Julia Suvorova; +Cc: qemu-devel, Michael S. Tsirkin, Ani Sinha
On Fri, 27 May 2022 18:56:50 +0200
Julia Suvorova <jusual@redhat.com> wrote:
> The new test is run with a large number of cpus and checks if the
> core_count field in smbios_cpu_test (structure type 4) is correct.
>
> Choose q35 as it allows to run with -smp > 255.
>
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> ---
> tests/qtest/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 0ba9d749a5..f2464adaa0 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -100,6 +100,8 @@ typedef struct {
> smbios_entry_point smbios_ep_table;
> uint16_t smbios_cpu_max_speed;
> uint16_t smbios_cpu_curr_speed;
> + uint8_t smbios_core_count;
> + uint16_t smbios_core_count2;
> uint8_t *required_struct_types;
> int required_struct_types_len;
> QTestState *qts;
> @@ -640,8 +642,9 @@ static inline bool smbios_single_instance(uint8_t type)
>
> static bool smbios_cpu_test(test_data *data, uint32_t addr)
> {
> + uint8_t real_cc, expect_cc = data->smbios_core_count;
%s/expect/expected/
also I'd s/real_cc/core_count/
> + uint16_t real, real_cc2, expect_cc2 = data->smbios_core_count2;
ditto
> uint16_t expect_speed[2];
> - uint16_t real;
> int offset[2];
> int i;
>
> @@ -660,6 +663,20 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> }
> }
>
> + real_cc = qtest_readb(data->qts, addr + offsetof(struct smbios_type_4, core_count));
> + real_cc2 = qtest_readw(data->qts, addr + offsetof(struct smbios_type_4, core_count2));
> +
> + if (expect_cc && (real_cc != expect_cc)) {
> + fprintf(stderr, "Unexpected SMBIOS CPU count: real %u expect %u\n",
> + real_cc, expect_cc);
> + return false;
since you are rewriting it anyways, how about
if (expect_cc) {
g_assert_cmpuint(...)
}
instead of printing/propagating error
> + }
> + if ((expect_cc == 0xFF) && (real_cc2 != expect_cc2)) {
> + fprintf(stderr, "Unexpected SMBIOS CPU count2: real %u expect %u\n",
> + real_cc2, expect_cc2);
> + return false;
> + }
> +
> return true;
> }
>
> @@ -905,6 +922,21 @@ static void test_acpi_q35_tcg(void)
> free_test_data(&data);
> }
>
> +static void test_acpi_q35_tcg_core_count2(void)
> +{
> + test_data data = {
> + .machine = MACHINE_Q35,
> + .variant = ".core-count2",
> + .required_struct_types = base_required_struct_types,
> + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types),
> + .smbios_core_count = 0xFF,
> + .smbios_core_count2 = 275,
> + };
> +
> + test_acpi_one("-machine smbios-entry-point-type=64 -smp 275", &data);
> + free_test_data(&data);
> +}
> +
> static void test_acpi_q35_tcg_bridge(void)
> {
> test_data data;
> @@ -1787,6 +1819,7 @@ int main(int argc, char *argv[])
> qtest_add_func("acpi/piix4/pci-hotplug/off",
> test_acpi_piix4_no_acpi_pci_hotplug);
> qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> + qtest_add_func("acpi/q35/core-count2", test_acpi_q35_tcg_core_count2);
> qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> qtest_add_func("acpi/q35/multif-bridge", test_acpi_q35_multif_bridge);
> qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] bios-tables-test: add test for number of cores > 255
2022-06-02 15:20 ` Igor Mammedov
@ 2022-06-02 16:31 ` Ani Sinha
2022-06-06 11:38 ` Julia Suvorova
1 sibling, 0 replies; 28+ messages in thread
From: Ani Sinha @ 2022-06-02 16:31 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Julia Suvorova, qemu-devel, Michael S. Tsirkin
On Thu, Jun 2, 2022 at 8:50 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Fri, 27 May 2022 18:56:50 +0200
> Julia Suvorova <jusual@redhat.com> wrote:
>
> > The new test is run with a large number of cpus and checks if the
> > core_count field in smbios_cpu_test (structure type 4) is correct.
> >
> > Choose q35 as it allows to run with -smp > 255.
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > ---
> > tests/qtest/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++-
> > 1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index 0ba9d749a5..f2464adaa0 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -100,6 +100,8 @@ typedef struct {
> > smbios_entry_point smbios_ep_table;
> > uint16_t smbios_cpu_max_speed;
> > uint16_t smbios_cpu_curr_speed;
> > + uint8_t smbios_core_count;
> > + uint16_t smbios_core_count2;
> > uint8_t *required_struct_types;
> > int required_struct_types_len;
> > QTestState *qts;
> > @@ -640,8 +642,9 @@ static inline bool smbios_single_instance(uint8_t type)
> >
> > static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > {
> > + uint8_t real_cc, expect_cc = data->smbios_core_count;
>
> %s/expect/expected/
> also I'd s/real_cc/core_count/
If you are going to go that path, I would suggest real_core_count and
exp_core_count.
>
> > + uint16_t real, real_cc2, expect_cc2 = data->smbios_core_count2;
> ditto
>
> > uint16_t expect_speed[2];
> > - uint16_t real;
> > int offset[2];
> > int i;
> >
> > @@ -660,6 +663,20 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > }
> > }
> >
> > + real_cc = qtest_readb(data->qts, addr + offsetof(struct smbios_type_4, core_count));
> > + real_cc2 = qtest_readw(data->qts, addr + offsetof(struct smbios_type_4, core_count2));
> > +
> > + if (expect_cc && (real_cc != expect_cc)) {
> > + fprintf(stderr, "Unexpected SMBIOS CPU count: real %u expect %u\n",
> > + real_cc, expect_cc);
> > + return false;
>
> since you are rewriting it anyways, how about
> if (expect_cc) {
> g_assert_cmpuint(...)
> }
>
> instead of printing/propagating error
>
> > + }
> > + if ((expect_cc == 0xFF) && (real_cc2 != expect_cc2)) {
> > + fprintf(stderr, "Unexpected SMBIOS CPU count2: real %u expect %u\n",
> > + real_cc2, expect_cc2);
> > + return false;
> > + }
> > +
> > return true;
> > }
> >
> > @@ -905,6 +922,21 @@ static void test_acpi_q35_tcg(void)
> > free_test_data(&data);
> > }
> >
> > +static void test_acpi_q35_tcg_core_count2(void)
> > +{
> > + test_data data = {
> > + .machine = MACHINE_Q35,
> > + .variant = ".core-count2",
> > + .required_struct_types = base_required_struct_types,
> > + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types),
> > + .smbios_core_count = 0xFF,
> > + .smbios_core_count2 = 275,
> > + };
> > +
> > + test_acpi_one("-machine smbios-entry-point-type=64 -smp 275", &data);
> > + free_test_data(&data);
> > +}
> > +
> > static void test_acpi_q35_tcg_bridge(void)
> > {
> > test_data data;
> > @@ -1787,6 +1819,7 @@ int main(int argc, char *argv[])
> > qtest_add_func("acpi/piix4/pci-hotplug/off",
> > test_acpi_piix4_no_acpi_pci_hotplug);
> > qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> > + qtest_add_func("acpi/q35/core-count2", test_acpi_q35_tcg_core_count2);
> > qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> > qtest_add_func("acpi/q35/multif-bridge", test_acpi_q35_multif_bridge);
> > qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/5] bios-tables-test: teach test to use smbios 3.0 tables
2022-06-02 15:04 ` Igor Mammedov
@ 2022-06-06 10:52 ` Julia Suvorova
2022-06-07 9:55 ` Igor Mammedov
0 siblings, 1 reply; 28+ messages in thread
From: Julia Suvorova @ 2022-06-06 10:52 UTC (permalink / raw)
To: Igor Mammedov; +Cc: QEMU Developers, Michael S. Tsirkin, Ani Sinha
On Thu, Jun 2, 2022 at 5:04 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Fri, 27 May 2022 18:56:48 +0200
> Julia Suvorova <jusual@redhat.com> wrote:
>
> > Introduce the 64-bit entry point. Since we no longer have a total
> > number of structures, stop checking for the new ones at the EOF
> > structure (type 127).
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > ---
> > tests/qtest/bios-tables-test.c | 101 ++++++++++++++++++++++++---------
> > 1 file changed, 75 insertions(+), 26 deletions(-)
> >
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index a4a46e97f0..0ba9d749a5 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -75,6 +75,14 @@
> > #define OEM_TEST_ARGS "-machine x-oem-id=" OEM_ID ",x-oem-table-id=" \
> > OEM_TABLE_ID
> >
> > +#define SMBIOS_VER21 0
> > +#define SMBIOS_VER30 1
> > +
> > +typedef struct {
> > + struct smbios_21_entry_point ep21;
> > + struct smbios_30_entry_point ep30;
> > +} smbios_entry_point;
> > +
> > typedef struct {
> > bool tcg_only;
> > const char *machine;
> > @@ -88,8 +96,8 @@ typedef struct {
> > uint64_t rsdp_addr;
> > uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
> > GArray *tables;
> > - uint32_t smbios_ep_addr;
> > - struct smbios_21_entry_point smbios_ep_table;
> > + uint64_t smbios_ep_addr[2];
> > + smbios_entry_point smbios_ep_table;
> > uint16_t smbios_cpu_max_speed;
> > uint16_t smbios_cpu_curr_speed;
> > uint8_t *required_struct_types;
> > @@ -533,10 +541,10 @@ static void test_acpi_asl(test_data *data)
> > free_test_data(&exp_data);
> > }
> >
> > -static bool smbios_ep_table_ok(test_data *data)
> > +static bool smbios_ep2_table_ok(test_data *data)
> > {
> > - struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
> > - uint32_t addr = data->smbios_ep_addr;
> > + struct smbios_21_entry_point *ep_table = &data->smbios_ep_table.ep21;
> > + uint32_t addr = data->smbios_ep_addr[SMBIOS_VER21];
> >
> > qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
> > if (memcmp(ep_table->anchor_string, "_SM_", 4)) {
> > @@ -559,29 +567,59 @@ static bool smbios_ep_table_ok(test_data *data)
> > return true;
> > }
> >
> > -static void test_smbios_entry_point(test_data *data)
> > +static bool smbios_ep3_table_ok(test_data *data)
> > +{
> > + struct smbios_30_entry_point *ep_table = &data->smbios_ep_table.ep30;
> > + uint64_t addr = data->smbios_ep_addr[SMBIOS_VER30];
> > +
> > + qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
> > + if (memcmp(ep_table->anchor_string, "_SM3_", 5)) {
> > + return false;
> > + }
> > +
> > + if (acpi_calc_checksum((uint8_t *)ep_table, sizeof *ep_table)) {
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +static int test_smbios_entry_point(test_data *data)
> > {
> > uint32_t off;
> > + bool found_ep2 = false, found_ep3 = false;
> >
> > /* find smbios entry point structure */
> > for (off = 0xf0000; off < 0x100000; off += 0x10) {
> > - uint8_t sig[] = "_SM_";
> > + uint8_t sig[] = "_SM3_";
>
> well I'd just add a separate sig3
Ok
> > int i;
> >
> > for (i = 0; i < sizeof sig - 1; ++i) {
> > sig[i] = qtest_readb(data->qts, off + i);
> > }
> >
> > - if (!memcmp(sig, "_SM_", sizeof sig)) {
> > + if (!found_ep2 && !memcmp(sig, "_SM_", sizeof sig - 2)) {
>
> keep original v2 code and just add similar chunk for v3,
> drop found_foo locals,
> that should make it easier to read/follow
> (i.e. less conditions to think about and no magic fiddling with the length of signature)
The idea was to reuse existing code, but since it doesn't improve
things much, it makes sense to repeat it.
> > /* signature match, but is this a valid entry point? */
> > - data->smbios_ep_addr = off;
> > - if (smbios_ep_table_ok(data)) {
> > - break;
> > + data->smbios_ep_addr[SMBIOS_VER21] = off;
> > + if (smbios_ep2_table_ok(data)) {
> > + found_ep2 = true;
> > + }
> > + } else if (!found_ep3 && !memcmp(sig, "_SM3_", sizeof sig - 1)) {
> > + data->smbios_ep_addr[SMBIOS_VER30] = off;
> > + if (smbios_ep3_table_ok(data)) {
> > + found_ep3 = true;
> > }
> > }
> > +
> > + if (found_ep2 || found_ep3) {
> > + break;
> > + }
> > }
> >
> > - g_assert_cmphex(off, <, 0x100000);
> > + g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER21], <, 0x100000);
> > + g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER30], <, 0x100000);
> > +
> > + return found_ep3 ? SMBIOS_VER30 : SMBIOS_VER21;
>
> and use content of data->smbios_ep_addr[] to return found version
You mean check if it's initialized?
> > }
> >
> > static inline bool smbios_single_instance(uint8_t type)
> > @@ -625,16 +663,23 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > return true;
> > }
> >
> > -static void test_smbios_structs(test_data *data)
> > +static void test_smbios_structs(test_data *data, int ver)
> > {
> > DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 };
> > - struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
> > - uint32_t addr = le32_to_cpu(ep_table->structure_table_address);
> > - int i, len, max_len = 0;
> > +
> > + smbios_entry_point *ep_table = &data->smbios_ep_table;
> > + int i = 0, len, max_len = 0;
> > uint8_t type, prv, crt;
> > + uint64_t addr;
> > +
> > + if (ver == SMBIOS_VER21) {
> > + addr = le32_to_cpu(ep_table->ep21.structure_table_address);
> > + } else {
> > + addr = le64_to_cpu(ep_table->ep30.structure_table_address);
> > + }
> >
> > /* walk the smbios tables */
> > - for (i = 0; i < le16_to_cpu(ep_table->number_of_structures); i++) {
> > + do {
> >
> > /* grab type and formatted area length from struct header */
> > type = qtest_readb(data->qts, addr);
> > @@ -660,19 +705,23 @@ static void test_smbios_structs(test_data *data)
> > }
> >
> > /* keep track of max. struct size */
> > - if (max_len < len) {
> > + if (ver == SMBIOS_VER21 && max_len < len) {
> > max_len = len;
> > - g_assert_cmpuint(max_len, <=, ep_table->max_structure_size);
> > + g_assert_cmpuint(max_len, <=, ep_table->ep21.max_structure_size);
> > }
> >
> > /* start of next structure */
> > addr += len;
> > - }
> >
> > - /* total table length and max struct size must match entry point values */
> > - g_assert_cmpuint(le16_to_cpu(ep_table->structure_table_length), ==,
> > - addr - le32_to_cpu(ep_table->structure_table_address));
> > - g_assert_cmpuint(le16_to_cpu(ep_table->max_structure_size), ==, max_len);
> > + } while (ver == SMBIOS_VER21 ?
> > + (++i < le16_to_cpu(ep_table->ep21.number_of_structures)) : (type != 127));
> > +
> > + if (ver == SMBIOS_VER21) {
> > + /* total table length and max struct size must match entry point values */
> > + g_assert_cmpuint(le16_to_cpu(ep_table->ep21.structure_table_length), ==,
> > + addr - le32_to_cpu(ep_table->ep21.structure_table_address));
> > + g_assert_cmpuint(le16_to_cpu(ep_table->ep21.max_structure_size), ==, max_len);
> > + }
> >
> > /* required struct types must all be present */
> > for (i = 0; i < data->required_struct_types_len; i++) {
> > @@ -756,8 +805,8 @@ static void test_acpi_one(const char *params, test_data *data)
> > * https://bugs.launchpad.net/qemu/+bug/1821884
> > */
> > if (!use_uefi) {
> > - test_smbios_entry_point(data);
> > - test_smbios_structs(data);
> > + int ver = test_smbios_entry_point(data);
> > + test_smbios_structs(data, ver);
> > }
> >
> > qtest_quit(data->qts);
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] hw/smbios: add core_count2 to smbios table type 4
2022-06-02 14:35 ` Igor Mammedov
@ 2022-06-06 11:11 ` Julia Suvorova
2022-06-07 9:51 ` Igor Mammedov
0 siblings, 1 reply; 28+ messages in thread
From: Julia Suvorova @ 2022-06-06 11:11 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Ani Sinha, QEMU Developers, Michael S. Tsirkin
On Thu, Jun 2, 2022 at 4:35 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Thu, 2 Jun 2022 16:31:25 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
>
> > On Tue, 31 May 2022 14:40:15 +0200
> > Julia Suvorova <jusual@redhat.com> wrote:
> >
> > > On Sat, May 28, 2022 at 6:34 AM Ani Sinha <ani@anisinha.ca> wrote:
> > > >
> > > >
> > > >
> > > > On Fri, 27 May 2022, Julia Suvorova wrote:
> > > >
> > > > > In order to use the increased number of cpus, we need to bring smbios
> > > > > tables in line with the SMBIOS 3.0 specification. This allows us to
> > > > > introduce core_count2 which acts as a duplicate of core_count if we have
> > > > > fewer cores than 256, and contains the actual core number per socket if
> > > > > we have more.
> > > > >
> > > > > core_enabled2 and thread_count2 fields work the same way.
> > > > >
> > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > >
> > > > Other than the comment below,
> > > > Reviewed-by: Ani Sinha <ani@anisinha.ca>
> > > >
> > > > > ---
> > > > > include/hw/firmware/smbios.h | 3 +++
> > > > > hw/smbios/smbios.c | 11 +++++++++--
> > > > > 2 files changed, 12 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> > > > > index 4b7ad77a44..c427ae5558 100644
> > > > > --- a/include/hw/firmware/smbios.h
> > > > > +++ b/include/hw/firmware/smbios.h
> > > > > @@ -187,6 +187,9 @@ struct smbios_type_4 {
> > > > > uint8_t thread_count;
> > > > > uint16_t processor_characteristics;
> > > > > uint16_t processor_family2;
> > > > > + uint16_t core_count2;
> > > > > + uint16_t core_enabled2;
> > > > > + uint16_t thread_count2;
>
> on the other hand,
> is it ok unconditionally extend type 4 and use v3 structure
> if qemu was started with v2 smbios?
We have a flag for the entry point type, not the smbios version per
se. Additional fields added to structures were not previously tracked
(for instance, processor_family2 is v2.6 and status is v2.0). AFAIU it
can affect only the total table length and the maximum structure size
(word). But if you like, I can raise an error (warning) if someone
tries to start a vm with cpus > 255 and smbios v2.
Best regards, Julia Suvorova.
> > > >
> > > > I would add a comment along the lines of
> > > > /* section 7.5, table 21 smbios spec version 3.0.0 */
> > >
> > > Ok
> >
> > With Ani's comment fixed
> >
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> >
> > >
> > > > > } QEMU_PACKED;
> > > > >
> > > > > /* SMBIOS type 11 - OEM strings */
> > > > > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> > > > > index 60349ee402..45d7be6b30 100644
> > > > > --- a/hw/smbios/smbios.c
> > > > > +++ b/hw/smbios/smbios.c
> > > > > @@ -709,8 +709,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
> > > > > SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
> > > > > SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
> > > > > SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
> > > > > - t->core_count = t->core_enabled = ms->smp.cores;
> > > > > - t->thread_count = ms->smp.threads;
> > > > > +
> > > > > + t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
> > > > > + t->core_enabled = t->core_count;
> > > > > +
> > > > > + t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
> > > > > +
> > > > > + t->thread_count = (ms->smp.threads > 255) ? 0xFF : ms->smp.threads;
> > > > > + t->thread_count2 = cpu_to_le16(ms->smp.threads);
> > > > > +
> > > > > t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
> > > > > t->processor_family2 = cpu_to_le16(0x01); /* Other */
> > > > >
> > > > > --
> > > > > 2.35.1
> > > > >
> > > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] bios-tables-test: add test for number of cores > 255
2022-06-02 15:20 ` Igor Mammedov
2022-06-02 16:31 ` Ani Sinha
@ 2022-06-06 11:38 ` Julia Suvorova
2022-06-07 10:08 ` Igor Mammedov
1 sibling, 1 reply; 28+ messages in thread
From: Julia Suvorova @ 2022-06-06 11:38 UTC (permalink / raw)
To: Igor Mammedov; +Cc: QEMU Developers, Michael S. Tsirkin, Ani Sinha
On Thu, Jun 2, 2022 at 5:20 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Fri, 27 May 2022 18:56:50 +0200
> Julia Suvorova <jusual@redhat.com> wrote:
>
> > The new test is run with a large number of cpus and checks if the
> > core_count field in smbios_cpu_test (structure type 4) is correct.
> >
> > Choose q35 as it allows to run with -smp > 255.
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > ---
> > tests/qtest/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++-
> > 1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index 0ba9d749a5..f2464adaa0 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -100,6 +100,8 @@ typedef struct {
> > smbios_entry_point smbios_ep_table;
> > uint16_t smbios_cpu_max_speed;
> > uint16_t smbios_cpu_curr_speed;
> > + uint8_t smbios_core_count;
> > + uint16_t smbios_core_count2;
> > uint8_t *required_struct_types;
> > int required_struct_types_len;
> > QTestState *qts;
> > @@ -640,8 +642,9 @@ static inline bool smbios_single_instance(uint8_t type)
> >
> > static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > {
> > + uint8_t real_cc, expect_cc = data->smbios_core_count;
>
> %s/expect/expected/
> also I'd s/real_cc/core_count/
>
> > + uint16_t real, real_cc2, expect_cc2 = data->smbios_core_count2;
> ditto
>
> > uint16_t expect_speed[2];
> > - uint16_t real;
> > int offset[2];
> > int i;
> >
> > @@ -660,6 +663,20 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > }
> > }
> >
> > + real_cc = qtest_readb(data->qts, addr + offsetof(struct smbios_type_4, core_count));
> > + real_cc2 = qtest_readw(data->qts, addr + offsetof(struct smbios_type_4, core_count2));
> > +
> > + if (expect_cc && (real_cc != expect_cc)) {
> > + fprintf(stderr, "Unexpected SMBIOS CPU count: real %u expect %u\n",
> > + real_cc, expect_cc);
> > + return false;
>
> since you are rewriting it anyways, how about
> if (expect_cc) {
> g_assert_cmpuint(...)
> }
>
> instead of printing/propagating error
That works. But I still need to return something, unless you want to
change the original code too.
Best regards, Julia Suvorova.
> > + }
> > + if ((expect_cc == 0xFF) && (real_cc2 != expect_cc2)) {
> > + fprintf(stderr, "Unexpected SMBIOS CPU count2: real %u expect %u\n",
> > + real_cc2, expect_cc2);
> > + return false;
> > + }
> > +
> > return true;
> > }
> >
> > @@ -905,6 +922,21 @@ static void test_acpi_q35_tcg(void)
> > free_test_data(&data);
> > }
> >
> > +static void test_acpi_q35_tcg_core_count2(void)
> > +{
> > + test_data data = {
> > + .machine = MACHINE_Q35,
> > + .variant = ".core-count2",
> > + .required_struct_types = base_required_struct_types,
> > + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types),
> > + .smbios_core_count = 0xFF,
> > + .smbios_core_count2 = 275,
> > + };
> > +
> > + test_acpi_one("-machine smbios-entry-point-type=64 -smp 275", &data);
> > + free_test_data(&data);
> > +}
> > +
> > static void test_acpi_q35_tcg_bridge(void)
> > {
> > test_data data;
> > @@ -1787,6 +1819,7 @@ int main(int argc, char *argv[])
> > qtest_add_func("acpi/piix4/pci-hotplug/off",
> > test_acpi_piix4_no_acpi_pci_hotplug);
> > qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> > + qtest_add_func("acpi/q35/core-count2", test_acpi_q35_tcg_core_count2);
> > qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> > qtest_add_func("acpi/q35/multif-bridge", test_acpi_q35_multif_bridge);
> > qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] hw/smbios: add core_count2 to smbios table type 4
2022-06-06 11:11 ` Julia Suvorova
@ 2022-06-07 9:51 ` Igor Mammedov
0 siblings, 0 replies; 28+ messages in thread
From: Igor Mammedov @ 2022-06-07 9:51 UTC (permalink / raw)
To: Julia Suvorova; +Cc: Ani Sinha, QEMU Developers, Michael S. Tsirkin
On Mon, 6 Jun 2022 13:11:36 +0200
Julia Suvorova <jusual@redhat.com> wrote:
> On Thu, Jun 2, 2022 at 4:35 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Thu, 2 Jun 2022 16:31:25 +0200
> > Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > > On Tue, 31 May 2022 14:40:15 +0200
> > > Julia Suvorova <jusual@redhat.com> wrote:
> > >
> > > > On Sat, May 28, 2022 at 6:34 AM Ani Sinha <ani@anisinha.ca> wrote:
> > > > >
> > > > >
> > > > >
> > > > > On Fri, 27 May 2022, Julia Suvorova wrote:
> > > > >
> > > > > > In order to use the increased number of cpus, we need to bring smbios
> > > > > > tables in line with the SMBIOS 3.0 specification. This allows us to
> > > > > > introduce core_count2 which acts as a duplicate of core_count if we have
> > > > > > fewer cores than 256, and contains the actual core number per socket if
> > > > > > we have more.
> > > > > >
> > > > > > core_enabled2 and thread_count2 fields work the same way.
> > > > > >
> > > > > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > > >
> > > > > Other than the comment below,
> > > > > Reviewed-by: Ani Sinha <ani@anisinha.ca>
> > > > >
> > > > > > ---
> > > > > > include/hw/firmware/smbios.h | 3 +++
> > > > > > hw/smbios/smbios.c | 11 +++++++++--
> > > > > > 2 files changed, 12 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> > > > > > index 4b7ad77a44..c427ae5558 100644
> > > > > > --- a/include/hw/firmware/smbios.h
> > > > > > +++ b/include/hw/firmware/smbios.h
> > > > > > @@ -187,6 +187,9 @@ struct smbios_type_4 {
> > > > > > uint8_t thread_count;
> > > > > > uint16_t processor_characteristics;
> > > > > > uint16_t processor_family2;
> > > > > > + uint16_t core_count2;
> > > > > > + uint16_t core_enabled2;
> > > > > > + uint16_t thread_count2;
> >
> > on the other hand,
> > is it ok unconditionally extend type 4 and use v3 structure
> > if qemu was started with v2 smbios?
>
> We have a flag for the entry point type, not the smbios version per
> se. Additional fields added to structures were not previously tracked
> (for instance, processor_family2 is v2.6 and status is v2.0). AFAIU it
that's a bug, unless there is a very good reason to keep doing that,
I'd not do that.
> can affect only the total table length and the maximum structure size
table length is fixed depending on used version,
so if we follow it, we should set length and use part of structure
correctly if old version is specified (i.e.
1. old structure + v30 structure,
2. copy only a relevant part of v30 structure and
fixup length when v2 version is asked for
though, I'd prefer #1
> (word). But if you like, I can raise an error (warning) if someone
> tries to start a vm with cpus > 255 and smbios v2.
it's a separate thing, I'd error out
(it will break users that use v2 with too may CPUs but then
they should fix their configuration to use v3)
> Best regards, Julia Suvorova.
>
> > > > >
> > > > > I would add a comment along the lines of
> > > > > /* section 7.5, table 21 smbios spec version 3.0.0 */
> > > >
> > > > Ok
> > >
> > > With Ani's comment fixed
> > >
> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > >
> > > >
> > > > > > } QEMU_PACKED;
> > > > > >
> > > > > > /* SMBIOS type 11 - OEM strings */
> > > > > > diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> > > > > > index 60349ee402..45d7be6b30 100644
> > > > > > --- a/hw/smbios/smbios.c
> > > > > > +++ b/hw/smbios/smbios.c
> > > > > > @@ -709,8 +709,15 @@ static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
> > > > > > SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
> > > > > > SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
> > > > > > SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
> > > > > > - t->core_count = t->core_enabled = ms->smp.cores;
> > > > > > - t->thread_count = ms->smp.threads;
> > > > > > +
> > > > > > + t->core_count = (ms->smp.cores > 255) ? 0xFF : ms->smp.cores;
> > > > > > + t->core_enabled = t->core_count;
> > > > > > +
> > > > > > + t->core_count2 = t->core_enabled2 = cpu_to_le16(ms->smp.cores);
> > > > > > +
> > > > > > + t->thread_count = (ms->smp.threads > 255) ? 0xFF : ms->smp.threads;
> > > > > > + t->thread_count2 = cpu_to_le16(ms->smp.threads);
> > > > > > +
> > > > > > t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
> > > > > > t->processor_family2 = cpu_to_le16(0x01); /* Other */
> > > > > >
> > > > > > --
> > > > > > 2.35.1
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/5] bios-tables-test: teach test to use smbios 3.0 tables
2022-06-06 10:52 ` Julia Suvorova
@ 2022-06-07 9:55 ` Igor Mammedov
0 siblings, 0 replies; 28+ messages in thread
From: Igor Mammedov @ 2022-06-07 9:55 UTC (permalink / raw)
To: Julia Suvorova; +Cc: QEMU Developers, Michael S. Tsirkin, Ani Sinha
On Mon, 6 Jun 2022 12:52:00 +0200
Julia Suvorova <jusual@redhat.com> wrote:
> On Thu, Jun 2, 2022 at 5:04 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Fri, 27 May 2022 18:56:48 +0200
> > Julia Suvorova <jusual@redhat.com> wrote:
> >
> > > Introduce the 64-bit entry point. Since we no longer have a total
> > > number of structures, stop checking for the new ones at the EOF
> > > structure (type 127).
> > >
> > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > ---
> > > tests/qtest/bios-tables-test.c | 101 ++++++++++++++++++++++++---------
> > > 1 file changed, 75 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > > index a4a46e97f0..0ba9d749a5 100644
> > > --- a/tests/qtest/bios-tables-test.c
> > > +++ b/tests/qtest/bios-tables-test.c
> > > @@ -75,6 +75,14 @@
> > > #define OEM_TEST_ARGS "-machine x-oem-id=" OEM_ID ",x-oem-table-id=" \
> > > OEM_TABLE_ID
> > >
> > > +#define SMBIOS_VER21 0
> > > +#define SMBIOS_VER30 1
> > > +
> > > +typedef struct {
> > > + struct smbios_21_entry_point ep21;
> > > + struct smbios_30_entry_point ep30;
> > > +} smbios_entry_point;
> > > +
> > > typedef struct {
> > > bool tcg_only;
> > > const char *machine;
> > > @@ -88,8 +96,8 @@ typedef struct {
> > > uint64_t rsdp_addr;
> > > uint8_t rsdp_table[36 /* ACPI 2.0+ RSDP size */];
> > > GArray *tables;
> > > - uint32_t smbios_ep_addr;
> > > - struct smbios_21_entry_point smbios_ep_table;
> > > + uint64_t smbios_ep_addr[2];
> > > + smbios_entry_point smbios_ep_table;
> > > uint16_t smbios_cpu_max_speed;
> > > uint16_t smbios_cpu_curr_speed;
> > > uint8_t *required_struct_types;
> > > @@ -533,10 +541,10 @@ static void test_acpi_asl(test_data *data)
> > > free_test_data(&exp_data);
> > > }
> > >
> > > -static bool smbios_ep_table_ok(test_data *data)
> > > +static bool smbios_ep2_table_ok(test_data *data)
> > > {
> > > - struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
> > > - uint32_t addr = data->smbios_ep_addr;
> > > + struct smbios_21_entry_point *ep_table = &data->smbios_ep_table.ep21;
> > > + uint32_t addr = data->smbios_ep_addr[SMBIOS_VER21];
> > >
> > > qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
> > > if (memcmp(ep_table->anchor_string, "_SM_", 4)) {
> > > @@ -559,29 +567,59 @@ static bool smbios_ep_table_ok(test_data *data)
> > > return true;
> > > }
> > >
> > > -static void test_smbios_entry_point(test_data *data)
> > > +static bool smbios_ep3_table_ok(test_data *data)
> > > +{
> > > + struct smbios_30_entry_point *ep_table = &data->smbios_ep_table.ep30;
> > > + uint64_t addr = data->smbios_ep_addr[SMBIOS_VER30];
> > > +
> > > + qtest_memread(data->qts, addr, ep_table, sizeof(*ep_table));
> > > + if (memcmp(ep_table->anchor_string, "_SM3_", 5)) {
> > > + return false;
> > > + }
> > > +
> > > + if (acpi_calc_checksum((uint8_t *)ep_table, sizeof *ep_table)) {
> > > + return false;
> > > + }
> > > +
> > > + return true;
> > > +}
> > > +
> > > +static int test_smbios_entry_point(test_data *data)
> > > {
> > > uint32_t off;
> > > + bool found_ep2 = false, found_ep3 = false;
> > >
> > > /* find smbios entry point structure */
> > > for (off = 0xf0000; off < 0x100000; off += 0x10) {
> > > - uint8_t sig[] = "_SM_";
> > > + uint8_t sig[] = "_SM3_";
> >
> > well I'd just add a separate sig3
>
> Ok
>
> > > int i;
> > >
> > > for (i = 0; i < sizeof sig - 1; ++i) {
> > > sig[i] = qtest_readb(data->qts, off + i);
> > > }
> > >
> > > - if (!memcmp(sig, "_SM_", sizeof sig)) {
> > > + if (!found_ep2 && !memcmp(sig, "_SM_", sizeof sig - 2)) {
> >
> > keep original v2 code and just add similar chunk for v3,
> > drop found_foo locals,
> > that should make it easier to read/follow
> > (i.e. less conditions to think about and no magic fiddling with the length of signature)
>
> The idea was to reuse existing code, but since it doesn't improve
> things much, it makes sense to repeat it.
>
> > > /* signature match, but is this a valid entry point? */
> > > - data->smbios_ep_addr = off;
> > > - if (smbios_ep_table_ok(data)) {
> > > - break;
> > > + data->smbios_ep_addr[SMBIOS_VER21] = off;
> > > + if (smbios_ep2_table_ok(data)) {
> > > + found_ep2 = true;
> > > + }
> > > + } else if (!found_ep3 && !memcmp(sig, "_SM3_", sizeof sig - 1)) {
> > > + data->smbios_ep_addr[SMBIOS_VER30] = off;
> > > + if (smbios_ep3_table_ok(data)) {
> > > + found_ep3 = true;
> > > }
> > > }
> > > +
> > > + if (found_ep2 || found_ep3) {
> > > + break;
> > > + }
> > > }
> > >
> > > - g_assert_cmphex(off, <, 0x100000);
> > > + g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER21], <, 0x100000);
> > > + g_assert_cmphex(data->smbios_ep_addr[SMBIOS_VER30], <, 0x100000);
> > > +
> > > + return found_ep3 ? SMBIOS_VER30 : SMBIOS_VER21;
> >
> > and use content of data->smbios_ep_addr[] to return found version
>
> You mean check if it's initialized?
yep, it's zeroed out initially and you can check if it's set to something else
after detection phase
>
> > > }
> > >
> > > static inline bool smbios_single_instance(uint8_t type)
> > > @@ -625,16 +663,23 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > > return true;
> > > }
> > >
> > > -static void test_smbios_structs(test_data *data)
> > > +static void test_smbios_structs(test_data *data, int ver)
> > > {
> > > DECLARE_BITMAP(struct_bitmap, SMBIOS_MAX_TYPE+1) = { 0 };
> > > - struct smbios_21_entry_point *ep_table = &data->smbios_ep_table;
> > > - uint32_t addr = le32_to_cpu(ep_table->structure_table_address);
> > > - int i, len, max_len = 0;
> > > +
> > > + smbios_entry_point *ep_table = &data->smbios_ep_table;
> > > + int i = 0, len, max_len = 0;
> > > uint8_t type, prv, crt;
> > > + uint64_t addr;
> > > +
> > > + if (ver == SMBIOS_VER21) {
> > > + addr = le32_to_cpu(ep_table->ep21.structure_table_address);
> > > + } else {
> > > + addr = le64_to_cpu(ep_table->ep30.structure_table_address);
> > > + }
> > >
> > > /* walk the smbios tables */
> > > - for (i = 0; i < le16_to_cpu(ep_table->number_of_structures); i++) {
> > > + do {
> > >
> > > /* grab type and formatted area length from struct header */
> > > type = qtest_readb(data->qts, addr);
> > > @@ -660,19 +705,23 @@ static void test_smbios_structs(test_data *data)
> > > }
> > >
> > > /* keep track of max. struct size */
> > > - if (max_len < len) {
> > > + if (ver == SMBIOS_VER21 && max_len < len) {
> > > max_len = len;
> > > - g_assert_cmpuint(max_len, <=, ep_table->max_structure_size);
> > > + g_assert_cmpuint(max_len, <=, ep_table->ep21.max_structure_size);
> > > }
> > >
> > > /* start of next structure */
> > > addr += len;
> > > - }
> > >
> > > - /* total table length and max struct size must match entry point values */
> > > - g_assert_cmpuint(le16_to_cpu(ep_table->structure_table_length), ==,
> > > - addr - le32_to_cpu(ep_table->structure_table_address));
> > > - g_assert_cmpuint(le16_to_cpu(ep_table->max_structure_size), ==, max_len);
> > > + } while (ver == SMBIOS_VER21 ?
> > > + (++i < le16_to_cpu(ep_table->ep21.number_of_structures)) : (type != 127));
> > > +
> > > + if (ver == SMBIOS_VER21) {
> > > + /* total table length and max struct size must match entry point values */
> > > + g_assert_cmpuint(le16_to_cpu(ep_table->ep21.structure_table_length), ==,
> > > + addr - le32_to_cpu(ep_table->ep21.structure_table_address));
> > > + g_assert_cmpuint(le16_to_cpu(ep_table->ep21.max_structure_size), ==, max_len);
> > > + }
> > >
> > > /* required struct types must all be present */
> > > for (i = 0; i < data->required_struct_types_len; i++) {
> > > @@ -756,8 +805,8 @@ static void test_acpi_one(const char *params, test_data *data)
> > > * https://bugs.launchpad.net/qemu/+bug/1821884
> > > */
> > > if (!use_uefi) {
> > > - test_smbios_entry_point(data);
> > > - test_smbios_structs(data);
> > > + int ver = test_smbios_entry_point(data);
> > > + test_smbios_structs(data, ver);
> > > }
> > >
> > > qtest_quit(data->qts);
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] bios-tables-test: add test for number of cores > 255
2022-06-06 11:38 ` Julia Suvorova
@ 2022-06-07 10:08 ` Igor Mammedov
0 siblings, 0 replies; 28+ messages in thread
From: Igor Mammedov @ 2022-06-07 10:08 UTC (permalink / raw)
To: Julia Suvorova; +Cc: QEMU Developers, Michael S. Tsirkin, Ani Sinha
On Mon, 6 Jun 2022 13:38:57 +0200
Julia Suvorova <jusual@redhat.com> wrote:
> On Thu, Jun 2, 2022 at 5:20 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Fri, 27 May 2022 18:56:50 +0200
> > Julia Suvorova <jusual@redhat.com> wrote:
> >
> > > The new test is run with a large number of cpus and checks if the
> > > core_count field in smbios_cpu_test (structure type 4) is correct.
> > >
> > > Choose q35 as it allows to run with -smp > 255.
> > >
> > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > ---
> > > tests/qtest/bios-tables-test.c | 35 +++++++++++++++++++++++++++++++++-
> > > 1 file changed, 34 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > > index 0ba9d749a5..f2464adaa0 100644
> > > --- a/tests/qtest/bios-tables-test.c
> > > +++ b/tests/qtest/bios-tables-test.c
> > > @@ -100,6 +100,8 @@ typedef struct {
> > > smbios_entry_point smbios_ep_table;
> > > uint16_t smbios_cpu_max_speed;
> > > uint16_t smbios_cpu_curr_speed;
> > > + uint8_t smbios_core_count;
> > > + uint16_t smbios_core_count2;
> > > uint8_t *required_struct_types;
> > > int required_struct_types_len;
> > > QTestState *qts;
> > > @@ -640,8 +642,9 @@ static inline bool smbios_single_instance(uint8_t type)
> > >
> > > static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > > {
> > > + uint8_t real_cc, expect_cc = data->smbios_core_count;
> >
> > %s/expect/expected/
> > also I'd s/real_cc/core_count/
> >
> > > + uint16_t real, real_cc2, expect_cc2 = data->smbios_core_count2;
> > ditto
> >
> > > uint16_t expect_speed[2];
> > > - uint16_t real;
> > > int offset[2];
> > > int i;
> > >
> > > @@ -660,6 +663,20 @@ static bool smbios_cpu_test(test_data *data, uint32_t addr)
> > > }
> > > }
> > >
> > > + real_cc = qtest_readb(data->qts, addr + offsetof(struct smbios_type_4, core_count));
> > > + real_cc2 = qtest_readw(data->qts, addr + offsetof(struct smbios_type_4, core_count2));
> > > +
> > > + if (expect_cc && (real_cc != expect_cc)) {
> > > + fprintf(stderr, "Unexpected SMBIOS CPU count: real %u expect %u\n",
> > > + real_cc, expect_cc);
> > > + return false;
> >
> > since you are rewriting it anyways, how about
> > if (expect_cc) {
> > g_assert_cmpuint(...)
> > }
> >
> > instead of printing/propagating error
>
> That works. But I still need to return something, unless you want to
> change the original code too.
just change it, as it makes code simpler (maybe a separate patch
that should go before this one)
>
> Best regards, Julia Suvorova.
>
> > > + }
> > > + if ((expect_cc == 0xFF) && (real_cc2 != expect_cc2)) {
> > > + fprintf(stderr, "Unexpected SMBIOS CPU count2: real %u expect %u\n",
> > > + real_cc2, expect_cc2);
> > > + return false;
> > > + }
> > > +
> > > return true;
> > > }
> > >
> > > @@ -905,6 +922,21 @@ static void test_acpi_q35_tcg(void)
> > > free_test_data(&data);
> > > }
> > >
> > > +static void test_acpi_q35_tcg_core_count2(void)
> > > +{
> > > + test_data data = {
> > > + .machine = MACHINE_Q35,
> > > + .variant = ".core-count2",
> > > + .required_struct_types = base_required_struct_types,
> > > + .required_struct_types_len = ARRAY_SIZE(base_required_struct_types),
> > > + .smbios_core_count = 0xFF,
> > > + .smbios_core_count2 = 275,
> > > + };
> > > +
> > > + test_acpi_one("-machine smbios-entry-point-type=64 -smp 275", &data);
> > > + free_test_data(&data);
> > > +}
> > > +
> > > static void test_acpi_q35_tcg_bridge(void)
> > > {
> > > test_data data;
> > > @@ -1787,6 +1819,7 @@ int main(int argc, char *argv[])
> > > qtest_add_func("acpi/piix4/pci-hotplug/off",
> > > test_acpi_piix4_no_acpi_pci_hotplug);
> > > qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> > > + qtest_add_func("acpi/q35/core-count2", test_acpi_q35_tcg_core_count2);
> > > qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> > > qtest_add_func("acpi/q35/multif-bridge", test_acpi_q35_multif_bridge);
> > > qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2022-06-07 10:54 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-27 16:56 [PATCH 0/5] hw/smbios: add core_count2 to smbios table type 4 Julia Suvorova
2022-05-27 16:56 ` [PATCH 1/5] " Julia Suvorova
2022-05-28 4:34 ` Ani Sinha
2022-05-31 12:40 ` Julia Suvorova
2022-06-02 14:31 ` Igor Mammedov
2022-06-02 14:35 ` Igor Mammedov
2022-06-06 11:11 ` Julia Suvorova
2022-06-07 9:51 ` Igor Mammedov
2022-05-27 16:56 ` [PATCH 2/5] bios-tables-test: teach test to use smbios 3.0 tables Julia Suvorova
2022-05-30 6:10 ` Ani Sinha
2022-05-31 12:39 ` Julia Suvorova
2022-06-02 15:04 ` Igor Mammedov
2022-06-06 10:52 ` Julia Suvorova
2022-06-07 9:55 ` Igor Mammedov
2022-05-27 16:56 ` [PATCH 3/5] tests/acpi: allow changes for core_count2 test Julia Suvorova
2022-05-28 5:28 ` Ani Sinha
2022-05-27 16:56 ` [PATCH 4/5] bios-tables-test: add test for number of cores > 255 Julia Suvorova
2022-05-28 5:22 ` Ani Sinha
2022-05-31 12:22 ` Julia Suvorova
2022-05-31 13:14 ` Ani Sinha
2022-05-31 15:05 ` Julia Suvorova
2022-06-01 5:09 ` Ani Sinha
2022-06-01 5:06 ` Ani Sinha
2022-06-02 15:20 ` Igor Mammedov
2022-06-02 16:31 ` Ani Sinha
2022-06-06 11:38 ` Julia Suvorova
2022-06-07 10:08 ` Igor Mammedov
2022-05-27 16:56 ` [PATCH 5/5] tests/acpi: update tables for new core count test Julia Suvorova
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.