Message ID | 20210907144814.741785-24-imammedo@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | acpi: refactor error prone build_header() and packed structures usage in ACPI tables | expand |
On 9/7/21 4:48 PM, Igor Mammedov wrote: > Instead of composing disabled _MAT entry and then later on > patching it to enabled for hotpluggbale CPUs in DSDT, > set it to enabled at the time _MAT entry is built. > > It will allow to drop usage of packed structures in > following patches when build_madt() is switched to use > build_append_int_noprefix() API. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > CC: marcel.apfelbaum@gmail.com > --- > include/hw/acpi/acpi_dev_interface.h | 3 ++- > include/hw/i386/pc.h | 6 +++--- > hw/acpi/acpi-x86-stub.c | 3 ++- > hw/acpi/cpu.c | 17 ++--------------- > hw/i386/acpi-common.c | 18 ++++++------------ > 5 files changed, 15 insertions(+), 32 deletions(-) > > diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h > index 769ff55c7e..ea6056ab92 100644 > --- a/include/hw/acpi/acpi_dev_interface.h > +++ b/include/hw/acpi/acpi_dev_interface.h > @@ -53,6 +53,7 @@ struct AcpiDeviceIfClass { > void (*ospm_status)(AcpiDeviceIf *adev, ACPIOSTInfoList ***list); > void (*send_event)(AcpiDeviceIf *adev, AcpiEventStatusBits ev); > void (*madt_cpu)(AcpiDeviceIf *adev, int uid, > - const CPUArchIdList *apic_ids, GArray *entry); > + const CPUArchIdList *apic_ids, GArray *entry, > + bool force_enabled); > }; > #endif > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 4d2e35a152..82cf7b7e30 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -187,10 +187,10 @@ bool pc_system_ovmf_table_find(const char *entry, uint8_t **data, > int *data_len); > void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size); > > - > -/* acpi-build.c */ > +/* hw/i386/acpi-common.c */ > void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, > - const CPUArchIdList *apic_ids, GArray *entry); > + const CPUArchIdList *apic_ids, GArray *entry, > + bool force_enabled); > > extern GlobalProperty pc_compat_6_1[]; > extern const size_t pc_compat_6_1_len; > diff --git a/hw/acpi/acpi-x86-stub.c b/hw/acpi/acpi-x86-stub.c > index e9e46c5c5f..3df1e090f4 100644 > --- a/hw/acpi/acpi-x86-stub.c > +++ b/hw/acpi/acpi-x86-stub.c > @@ -3,7 +3,8 @@ > #include "hw/i386/acpi-build.h" > > void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, > - const CPUArchIdList *apic_ids, GArray *entry) > + const CPUArchIdList *apic_ids, GArray *entry, > + bool force_enabled) > { > } > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c > index f82e9512fd..b20903ea30 100644 > --- a/hw/acpi/cpu.c > +++ b/hw/acpi/cpu.c > @@ -669,21 +669,8 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, > > /* build _MAT object */ > assert(adevc && adevc->madt_cpu); > - adevc->madt_cpu(adev, i, arch_ids, madt_buf); > - switch (madt_buf->data[0]) { > - case ACPI_APIC_PROCESSOR: { > - AcpiMadtProcessorApic *apic = (void *)madt_buf->data; > - apic->flags = cpu_to_le32(1); > - break; > - } > - case ACPI_APIC_LOCAL_X2APIC: { > - AcpiMadtProcessorX2Apic *apic = (void *)madt_buf->data; > - apic->flags = cpu_to_le32(1); > - break; > - } > - default: > - assert(0); > - } > + adevc->madt_cpu(adev, i, arch_ids, madt_buf, > + true); /* set enabled flag */ > aml_append(dev, aml_name_decl("_MAT", > aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data))); > g_array_free(madt_buf, true); > diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c > index a0cde1d874..7983a13a93 100644 > --- a/hw/i386/acpi-common.c > +++ b/hw/i386/acpi-common.c > @@ -34,9 +34,11 @@ > #include "acpi-common.h" > > void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, > - const CPUArchIdList *apic_ids, GArray *entry) > + const CPUArchIdList *apic_ids, GArray *entry, > + bool force_enabled) > { > uint32_t apic_id = apic_ids->cpus[uid].arch_id; > + uint32_t flags = apic_ids->cpus[uid].cpu != NULL || force_enabled ? 1 : 0; > > /* ACPI spec says that LAPIC entry for non present > * CPU may be omitted from MADT or it must be marked > @@ -51,11 +53,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, > apic->length = sizeof(*apic); > apic->processor_id = uid; > apic->local_apic_id = apic_id; > - if (apic_ids->cpus[uid].cpu != NULL) { > - apic->flags = cpu_to_le32(1); > - } else { > - apic->flags = cpu_to_le32(0); > - } > + apic->flags = cpu_to_le32(flags); > } else { > AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic); > > @@ -63,11 +61,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, > apic->length = sizeof(*apic); > apic->uid = cpu_to_le32(uid); > apic->x2apic_id = cpu_to_le32(apic_id); > - if (apic_ids->cpus[uid].cpu != NULL) { > - apic->flags = cpu_to_le32(1); > - } else { > - apic->flags = cpu_to_le32(0); > - } > + apic->flags = cpu_to_le32(flags); > } > } > > @@ -96,7 +90,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker, > build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */ > > for (i = 0; i < apic_ids->len; i++) { > - adevc->madt_cpu(adev, i, apic_ids, table_data); > + adevc->madt_cpu(adev, i, apic_ids, table_data, false); > if (apic_ids->cpus[i].arch_id > 254) { > x2apic_mode = true; > } > Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric
diff --git a/include/hw/acpi/acpi_dev_interface.h b/include/hw/acpi/acpi_dev_interface.h index 769ff55c7e..ea6056ab92 100644 --- a/include/hw/acpi/acpi_dev_interface.h +++ b/include/hw/acpi/acpi_dev_interface.h @@ -53,6 +53,7 @@ struct AcpiDeviceIfClass { void (*ospm_status)(AcpiDeviceIf *adev, ACPIOSTInfoList ***list); void (*send_event)(AcpiDeviceIf *adev, AcpiEventStatusBits ev); void (*madt_cpu)(AcpiDeviceIf *adev, int uid, - const CPUArchIdList *apic_ids, GArray *entry); + const CPUArchIdList *apic_ids, GArray *entry, + bool force_enabled); }; #endif diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 4d2e35a152..82cf7b7e30 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -187,10 +187,10 @@ bool pc_system_ovmf_table_find(const char *entry, uint8_t **data, int *data_len); void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size); - -/* acpi-build.c */ +/* hw/i386/acpi-common.c */ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, - const CPUArchIdList *apic_ids, GArray *entry); + const CPUArchIdList *apic_ids, GArray *entry, + bool force_enabled); extern GlobalProperty pc_compat_6_1[]; extern const size_t pc_compat_6_1_len; diff --git a/hw/acpi/acpi-x86-stub.c b/hw/acpi/acpi-x86-stub.c index e9e46c5c5f..3df1e090f4 100644 --- a/hw/acpi/acpi-x86-stub.c +++ b/hw/acpi/acpi-x86-stub.c @@ -3,7 +3,8 @@ #include "hw/i386/acpi-build.h" void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, - const CPUArchIdList *apic_ids, GArray *entry) + const CPUArchIdList *apic_ids, GArray *entry, + bool force_enabled) { } diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c index f82e9512fd..b20903ea30 100644 --- a/hw/acpi/cpu.c +++ b/hw/acpi/cpu.c @@ -669,21 +669,8 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts, /* build _MAT object */ assert(adevc && adevc->madt_cpu); - adevc->madt_cpu(adev, i, arch_ids, madt_buf); - switch (madt_buf->data[0]) { - case ACPI_APIC_PROCESSOR: { - AcpiMadtProcessorApic *apic = (void *)madt_buf->data; - apic->flags = cpu_to_le32(1); - break; - } - case ACPI_APIC_LOCAL_X2APIC: { - AcpiMadtProcessorX2Apic *apic = (void *)madt_buf->data; - apic->flags = cpu_to_le32(1); - break; - } - default: - assert(0); - } + adevc->madt_cpu(adev, i, arch_ids, madt_buf, + true); /* set enabled flag */ aml_append(dev, aml_name_decl("_MAT", aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data))); g_array_free(madt_buf, true); diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c index a0cde1d874..7983a13a93 100644 --- a/hw/i386/acpi-common.c +++ b/hw/i386/acpi-common.c @@ -34,9 +34,11 @@ #include "acpi-common.h" void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, - const CPUArchIdList *apic_ids, GArray *entry) + const CPUArchIdList *apic_ids, GArray *entry, + bool force_enabled) { uint32_t apic_id = apic_ids->cpus[uid].arch_id; + uint32_t flags = apic_ids->cpus[uid].cpu != NULL || force_enabled ? 1 : 0; /* ACPI spec says that LAPIC entry for non present * CPU may be omitted from MADT or it must be marked @@ -51,11 +53,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, apic->length = sizeof(*apic); apic->processor_id = uid; apic->local_apic_id = apic_id; - if (apic_ids->cpus[uid].cpu != NULL) { - apic->flags = cpu_to_le32(1); - } else { - apic->flags = cpu_to_le32(0); - } + apic->flags = cpu_to_le32(flags); } else { AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic); @@ -63,11 +61,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, apic->length = sizeof(*apic); apic->uid = cpu_to_le32(uid); apic->x2apic_id = cpu_to_le32(apic_id); - if (apic_ids->cpus[uid].cpu != NULL) { - apic->flags = cpu_to_le32(1); - } else { - apic->flags = cpu_to_le32(0); - } + apic->flags = cpu_to_le32(flags); } } @@ -96,7 +90,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker, build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */ for (i = 0; i < apic_ids->len; i++) { - adevc->madt_cpu(adev, i, apic_ids, table_data); + adevc->madt_cpu(adev, i, apic_ids, table_data, false); if (apic_ids->cpus[i].arch_id > 254) { x2apic_mode = true; }
Instead of composing disabled _MAT entry and then later on patching it to enabled for hotpluggbale CPUs in DSDT, set it to enabled at the time _MAT entry is built. It will allow to drop usage of packed structures in following patches when build_madt() is switched to use build_append_int_noprefix() API. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- CC: marcel.apfelbaum@gmail.com --- include/hw/acpi/acpi_dev_interface.h | 3 ++- include/hw/i386/pc.h | 6 +++--- hw/acpi/acpi-x86-stub.c | 3 ++- hw/acpi/cpu.c | 17 ++--------------- hw/i386/acpi-common.c | 18 ++++++------------ 5 files changed, 15 insertions(+), 32 deletions(-)