On Thu, Oct 13, 2016 at 11:52:35AM +0200, Igor Mammedov wrote: > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
Reviewed-by: Eduardo Habkost <ehabk...@redhat.com> But I have a few questions below that are not directly related to this patch: > --- > include/hw/acpi/acpi-defs.h | 18 +++++++++++ > hw/i386/acpi-build.c | 78 > +++++++++++++++++++++++++++++++-------------- > 2 files changed, 72 insertions(+), 24 deletions(-) > > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h > index 9c1b7cb..e94123c 100644 > --- a/include/hw/acpi/acpi-defs.h > +++ b/include/hw/acpi/acpi-defs.h > @@ -343,6 +343,24 @@ struct AcpiMadtLocalNmi { > } QEMU_PACKED; > typedef struct AcpiMadtLocalNmi AcpiMadtLocalNmi; > > +struct AcpiMadtProcessorX2Apic { > + ACPI_SUB_HEADER_DEF > + uint16_t reserved; > + uint32_t x2apic_id; /* Processor's local x2APIC ID */ > + uint32_t flags; > + uint32_t uid; /* Processor object _UID */ > +} QEMU_PACKED; > +typedef struct AcpiMadtProcessorX2Apic AcpiMadtProcessorX2Apic; > + > +struct AcpiMadtLocalX2ApicNmi { > + ACPI_SUB_HEADER_DEF > + uint16_t flags; /* MPS INTI flags */ > + uint32_t uid; /* Processor object _UID */ > + uint8_t lint; /* Local APIC LINT# */ > + uint8_t reserved[3]; /* Local APIC LINT# */ > +} QEMU_PACKED; > +typedef struct AcpiMadtLocalX2ApicNmi AcpiMadtLocalX2ApicNmi; > + > struct AcpiMadtGenericInterrupt { > ACPI_SUB_HEADER_DEF > uint16_t reserved; > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index e999654..702d254 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -340,24 +340,38 @@ build_fadt(GArray *table_data, BIOSLinker *linker, > AcpiPmInfo *pm, > void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid, > CPUArchIdList *apic_ids, GArray *entry) > { > - int apic_id; > - AcpiMadtProcessorApic *apic = acpi_data_push(entry, sizeof *apic); > - > - apic_id = apic_ids->cpus[uid].arch_id; > - apic->type = ACPI_APIC_PROCESSOR; > - 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); Shouldn't length, processor_id, and local_apic_id be converted to little-endian too? > + uint32_t apic_id = apic_ids->cpus[uid].arch_id; > + > + /* ACPI spec says that LAPIC entry for non present > + * CPU may be omitted from MADT or it must be marked > + * as disabled. However omitting non present CPU from > + * MADT breaks hotplug on linux. So possible CPUs > + * should be put in MADT but kept disabled. > + */ > + if (apic_id < 255) { > + AcpiMadtProcessorApic *apic = acpi_data_push(entry, sizeof *apic); > + > + apic->type = ACPI_APIC_PROCESSOR; > + 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); > + } > } else { > - /* ACPI spec says that LAPIC entry for non present > - * CPU may be omitted from MADT or it must be marked > - * as disabled. However omitting non present CPU from > - * MADT breaks hotplug on linux. So possible CPUs > - * should be put in MADT but kept disabled. > - */ > - apic->flags = cpu_to_le32(0); > + AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic); > + > + apic->type = ACPI_APIC_LOCAL_X2APIC; > + apic->length = sizeof(*apic); > + apic->uid = uid; > + apic->x2apic_id = apic_id; > + if (apic_ids->cpus[uid].cpu != NULL) { > + apic->flags = cpu_to_le32(1); > + } else { > + apic->flags = cpu_to_le32(0); > + } > } > } > > @@ -369,11 +383,11 @@ build_madt(GArray *table_data, BIOSLinker *linker, > PCMachineState *pcms) > int madt_start = table_data->len; > AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(pcms->acpi_dev); > AcpiDeviceIf *adev = ACPI_DEVICE_IF(pcms->acpi_dev); > + bool x2apic_mode = false; > > AcpiMultipleApicTable *madt; > AcpiMadtIoApic *io_apic; > AcpiMadtIntsrcovr *intsrcovr; > - AcpiMadtLocalNmi *local_nmi; > int i; > > madt = acpi_data_push(table_data, sizeof *madt); > @@ -382,6 +396,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, > PCMachineState *pcms) > > for (i = 0; i < apic_ids->len; i++) { > adevc->madt_cpu(adev, i, apic_ids, table_data); Why adevc->madt_cpu exists if all devices use pc_madt_cpu_entry()? > + if (apic_ids->cpus[i].arch_id > 254) { > + x2apic_mode = true; > + } > } > g_free(apic_ids); > > @@ -414,12 +431,25 @@ build_madt(GArray *table_data, BIOSLinker *linker, > PCMachineState *pcms) > intsrcovr->flags = cpu_to_le16(0xd); /* active high, level > triggered */ > } > > - local_nmi = acpi_data_push(table_data, sizeof *local_nmi); > - local_nmi->type = ACPI_APIC_LOCAL_NMI; > - local_nmi->length = sizeof(*local_nmi); > - local_nmi->processor_id = 0xff; /* all processors */ > - local_nmi->flags = cpu_to_le16(0); > - local_nmi->lint = 1; /* ACPI_LINT1 */ > + if (x2apic_mode) { > + AcpiMadtLocalX2ApicNmi *local_nmi; > + > + local_nmi = acpi_data_push(table_data, sizeof *local_nmi); > + local_nmi->type = ACPI_APIC_LOCAL_X2APIC_NMI; > + local_nmi->length = sizeof(*local_nmi); > + local_nmi->uid = 0xFFFFFFFF; /* all processors */ > + local_nmi->flags = cpu_to_le16(0); > + local_nmi->lint = 1; /* ACPI_LINT1 */ > + } else { > + AcpiMadtLocalNmi *local_nmi; > + > + local_nmi = acpi_data_push(table_data, sizeof *local_nmi); > + local_nmi->type = ACPI_APIC_LOCAL_NMI; > + local_nmi->length = sizeof(*local_nmi); > + local_nmi->processor_id = 0xff; /* all processors */ > + local_nmi->flags = cpu_to_le16(0); > + local_nmi->lint = 1; /* ACPI_LINT1 */ > + } > > build_header(linker, table_data, > (void *)(table_data->data + madt_start), "APIC", > -- > 2.7.4 > -- Eduardo