| Message ID | 1476352367-69400-2-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 13, 2016 at 11:52:35AM +0200, Igor Mammedov wrote: > Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@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 >
On Tue, 18 Oct 2016 10:47:02 -0200 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Thu, Oct 13, 2016 at 11:52:35AM +0200, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > Reviewed-by: Eduardo Habkost <ehabkost@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? it's 1 byte fields only. > > > + 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()? it's been added with ARM target in mind, so that it could be possible to have generic hotplug code which uses this callback as well and maybe some day to generalize build_madt() as well. > > + 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 > > >
On Tue, Oct 18, 2016 at 10:47:02AM -0200, Eduardo Habkost wrote: > On Thu, Oct 13, 2016 at 11:52:35AM +0200, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by withdrawn. See below: [...] > > 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? Erm, those fields are all 8-bit. Nevermind. But that means the new x2apic code below seems wrong: > > > + 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; cpu_to_le32()? > > + apic->x2apic_id = apic_id; cpu_to_le32()? > > + if (apic_ids->cpus[uid].cpu != NULL) { > > + apic->flags = cpu_to_le32(1); > > + } else { > > + apic->flags = cpu_to_le32(0); > > + } > > } > > } [...]
On Tue, 18 Oct 2016 11:05:47 -0200 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Tue, Oct 18, 2016 at 10:47:02AM -0200, Eduardo Habkost wrote: > > On Thu, Oct 13, 2016 at 11:52:35AM +0200, Igor Mammedov wrote: > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > > Reviewed-by withdrawn. See below: > > [...] > > > 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? > > Erm, those fields are all 8-bit. Nevermind. But that means the > new x2apic code below seems wrong: Thanks for noticing, it needs cpu_to_le32() at some places. I'll fix it and post v4 here. > > > > > > + 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; > > cpu_to_le32()? > > > > + apic->x2apic_id = apic_id; > > cpu_to_le32()? > > > > + if (apic_ids->cpus[uid].cpu != NULL) { > > > + apic->flags = cpu_to_le32(1); > > > + } else { > > > + apic->flags = cpu_to_le32(0); > > > + } > > > } > > > } > [...] >
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); + 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); + 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",
Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- include/hw/acpi/acpi-defs.h | 18 +++++++++++ hw/i386/acpi-build.c | 78 +++++++++++++++++++++++++++++++-------------- 2 files changed, 72 insertions(+), 24 deletions(-)