* [Qemu-devel] [PATCH v3 01/13] pc: acpi: x2APIC support for MADT table
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-18 12:47 ` Eduardo Habkost
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 02/13] pc: acpi: x2APIC support for SRAT table Igor Mammedov
` (14 subsequent siblings)
15 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
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(-)
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",
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 01/13] pc: acpi: x2APIC support for MADT table
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 01/13] pc: acpi: x2APIC support for MADT table Igor Mammedov
@ 2016-10-18 12:47 ` Eduardo Habkost
2016-10-18 13:00 ` Igor Mammedov
2016-10-18 13:05 ` Eduardo Habkost
0 siblings, 2 replies; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 12:47 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
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
>
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 01/13] pc: acpi: x2APIC support for MADT table
2016-10-18 12:47 ` Eduardo Habkost
@ 2016-10-18 13:00 ` Igor Mammedov
2016-10-18 13:05 ` Eduardo Habkost
1 sibling, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 13:00 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
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
> >
>
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 01/13] pc: acpi: x2APIC support for MADT table
2016-10-18 12:47 ` Eduardo Habkost
2016-10-18 13:00 ` Igor Mammedov
@ 2016-10-18 13:05 ` Eduardo Habkost
2016-10-18 13:42 ` Igor Mammedov
1 sibling, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 13:05 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
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);
> > + }
> > }
> > }
[...]
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 01/13] pc: acpi: x2APIC support for MADT table
2016-10-18 13:05 ` Eduardo Habkost
@ 2016-10-18 13:42 ` Igor Mammedov
0 siblings, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 13:42 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
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);
> > > + }
> > > }
> > > }
> [...]
>
^ permalink raw reply [flat|nested] 68+ messages in thread
* [Qemu-devel] [PATCH v3 02/13] pc: acpi: x2APIC support for SRAT table
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 01/13] pc: acpi: x2APIC support for MADT table Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-18 13:07 ` Eduardo Habkost
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 03/13] acpi: cphp: support x2APIC entry in cpu._MAT Igor Mammedov
` (13 subsequent siblings)
15 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
rebase on top in 2.7 + updated cpu PXM patches
---
include/hw/acpi/acpi-defs.h | 11 +++++++++++
hw/i386/acpi-build.c | 34 ++++++++++++++++++++++++----------
2 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index e94123c..fa89abc 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -503,6 +503,17 @@ struct AcpiSratProcessorAffinity
} QEMU_PACKED;
typedef struct AcpiSratProcessorAffinity AcpiSratProcessorAffinity;
+struct AcpiSratProcessorX2ApicAffinity {
+ ACPI_SUB_HEADER_DEF
+ uint16_t reserved;
+ uint32_t proximity_domain;
+ uint32_t x2apic_id;
+ uint32_t flags;
+ uint32_t clk_domain;
+ uint32_t reserved2;
+} QEMU_PACKED;
+typedef struct AcpiSratProcessorX2ApicAffinity AcpiSratProcessorX2ApicAffinity;
+
struct AcpiSratMemoryAffinity
{
ACPI_SUB_HEADER_DEF
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 702d254..b9819f4 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2421,7 +2421,6 @@ static void
build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
{
AcpiSystemResourceAffinityTable *srat;
- AcpiSratProcessorAffinity *core;
AcpiSratMemoryAffinity *numamem;
int i;
@@ -2441,18 +2440,33 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
for (i = 0; i < apic_ids->len; i++) {
int j = numa_get_node_for_cpu(i);
- int apic_id = apic_ids->cpus[i].arch_id;
+ uint32_t apic_id = apic_ids->cpus[i].arch_id;
- core = acpi_data_push(table_data, sizeof *core);
- core->type = ACPI_SRAT_PROCESSOR_APIC;
- core->length = sizeof(*core);
- core->local_apic_id = apic_id;
- if (j < nb_numa_nodes) {
+ if (apic_id < 255) {
+ AcpiSratProcessorAffinity *core;
+
+ core = acpi_data_push(table_data, sizeof *core);
+ core->type = ACPI_SRAT_PROCESSOR_APIC;
+ core->length = sizeof(*core);
+ core->local_apic_id = apic_id;
+ if (j < nb_numa_nodes) {
core->proximity_lo = j;
+ }
+ memset(core->proximity_hi, 0, 3);
+ core->local_sapic_eid = 0;
+ core->flags = cpu_to_le32(1);
+ } else {
+ AcpiSratProcessorX2ApicAffinity *core;
+
+ core = acpi_data_push(table_data, sizeof *core);
+ core->type = ACPI_SRAT_PROCESSOR_x2APIC;
+ core->length = sizeof(*core);
+ core->x2apic_id = apic_id;
+ if (j < nb_numa_nodes) {
+ core->proximity_domain = cpu_to_le32(j);
+ }
+ core->flags = cpu_to_le32(1);
}
- memset(core->proximity_hi, 0, 3);
- core->local_sapic_eid = 0;
- core->flags = cpu_to_le32(1);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 02/13] pc: acpi: x2APIC support for SRAT table
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 02/13] pc: acpi: x2APIC support for SRAT table Igor Mammedov
@ 2016-10-18 13:07 ` Eduardo Habkost
2016-10-18 13:47 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 13:07 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Thu, Oct 13, 2016 at 11:52:36AM +0200, Igor Mammedov wrote
[...]
> @@ -2441,18 +2440,33 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>
> for (i = 0; i < apic_ids->len; i++) {
> int j = numa_get_node_for_cpu(i);
> - int apic_id = apic_ids->cpus[i].arch_id;
> + uint32_t apic_id = apic_ids->cpus[i].arch_id;
>
> - core = acpi_data_push(table_data, sizeof *core);
> - core->type = ACPI_SRAT_PROCESSOR_APIC;
> - core->length = sizeof(*core);
> - core->local_apic_id = apic_id;
> - if (j < nb_numa_nodes) {
> + if (apic_id < 255) {
> + AcpiSratProcessorAffinity *core;
> +
> + core = acpi_data_push(table_data, sizeof *core);
> + core->type = ACPI_SRAT_PROCESSOR_APIC;
> + core->length = sizeof(*core);
> + core->local_apic_id = apic_id;
> + if (j < nb_numa_nodes) {
> core->proximity_lo = j;
> + }
> + memset(core->proximity_hi, 0, 3);
> + core->local_sapic_eid = 0;
> + core->flags = cpu_to_le32(1);
> + } else {
> + AcpiSratProcessorX2ApicAffinity *core;
> +
> + core = acpi_data_push(table_data, sizeof *core);
> + core->type = ACPI_SRAT_PROCESSOR_x2APIC;
> + core->length = sizeof(*core);
> + core->x2apic_id = apic_id;
cpu_to_le32()?
> + if (j < nb_numa_nodes) {
> + core->proximity_domain = cpu_to_le32(j);
> + }
> + core->flags = cpu_to_le32(1);
> }
> - memset(core->proximity_hi, 0, 3);
> - core->local_sapic_eid = 0;
> - core->flags = cpu_to_le32(1);
> }
>
>
> --
> 2.7.4
>
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 02/13] pc: acpi: x2APIC support for SRAT table
2016-10-18 13:07 ` Eduardo Habkost
@ 2016-10-18 13:47 ` Igor Mammedov
0 siblings, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 13:47 UTC (permalink / raw)
To: Eduardo Habkost
Cc: liuxiaojian6, mst, rkrcmar, qemu-devel, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 11:07:21 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Thu, Oct 13, 2016 at 11:52:36AM +0200, Igor Mammedov wrote
> [...]
> > @@ -2441,18 +2440,33 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
> >
> > for (i = 0; i < apic_ids->len; i++) {
> > int j = numa_get_node_for_cpu(i);
> > - int apic_id = apic_ids->cpus[i].arch_id;
> > + uint32_t apic_id = apic_ids->cpus[i].arch_id;
> >
> > - core = acpi_data_push(table_data, sizeof *core);
> > - core->type = ACPI_SRAT_PROCESSOR_APIC;
> > - core->length = sizeof(*core);
> > - core->local_apic_id = apic_id;
> > - if (j < nb_numa_nodes) {
> > + if (apic_id < 255) {
> > + AcpiSratProcessorAffinity *core;
> > +
> > + core = acpi_data_push(table_data, sizeof *core);
> > + core->type = ACPI_SRAT_PROCESSOR_APIC;
> > + core->length = sizeof(*core);
> > + core->local_apic_id = apic_id;
> > + if (j < nb_numa_nodes) {
> > core->proximity_lo = j;
> > + }
> > + memset(core->proximity_hi, 0, 3);
> > + core->local_sapic_eid = 0;
> > + core->flags = cpu_to_le32(1);
> > + } else {
> > + AcpiSratProcessorX2ApicAffinity *core;
> > +
> > + core = acpi_data_push(table_data, sizeof *core);
> > + core->type = ACPI_SRAT_PROCESSOR_x2APIC;
> > + core->length = sizeof(*core);
> > + core->x2apic_id = apic_id;
>
> cpu_to_le32()?
Sure,
will fix and respin series as a lot of fixes are accumulated by now.
>
> > + if (j < nb_numa_nodes) {
> > + core->proximity_domain = cpu_to_le32(j);
> > + }
> > + core->flags = cpu_to_le32(1);
> > }
> > - memset(core->proximity_hi, 0, 3);
> > - core->local_sapic_eid = 0;
> > - core->flags = cpu_to_le32(1);
> > }
> >
> >
> > --
> > 2.7.4
> >
>
^ permalink raw reply [flat|nested] 68+ messages in thread
* [Qemu-devel] [PATCH v3 03/13] acpi: cphp: support x2APIC entry in cpu._MAT
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 01/13] pc: acpi: x2APIC support for MADT table Igor Mammedov
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 02/13] pc: acpi: x2APIC support for SRAT table Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-18 13:34 ` Eduardo Habkost
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254 Igor Mammedov
` (12 subsequent siblings)
15 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/cpu.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 902f5c9..5ac89fe 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -531,6 +531,11 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
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);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 03/13] acpi: cphp: support x2APIC entry in cpu._MAT
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 03/13] acpi: cphp: support x2APIC entry in cpu._MAT Igor Mammedov
@ 2016-10-18 13:34 ` Eduardo Habkost
2016-10-18 13:46 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 13:34 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Thu, Oct 13, 2016 at 11:52:37AM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/acpi/cpu.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 902f5c9..5ac89fe 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -531,6 +531,11 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> 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;
> + }
Shouldn't this patch be applied before 01/13 to avoid triggering
the assert() below?
> default:
> assert(0);
> }
> --
> 2.7.4
>
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 03/13] acpi: cphp: support x2APIC entry in cpu._MAT
2016-10-18 13:34 ` Eduardo Habkost
@ 2016-10-18 13:46 ` Igor Mammedov
2016-10-18 13:47 ` Eduardo Habkost
0 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 13:46 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 11:34:55 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Thu, Oct 13, 2016 at 11:52:37AM +0200, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > hw/acpi/cpu.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> > index 902f5c9..5ac89fe 100644
> > --- a/hw/acpi/cpu.c
> > +++ b/hw/acpi/cpu.c
> > @@ -531,6 +531,11 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> > 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;
> > + }
>
> Shouldn't this patch be applied before 01/13 to avoid triggering
> the assert() below?
There is no AcpiMadtProcessorX2Apic before 1/13,
how about squashing this patch into 1/13, that should be cleaner.
>
> > default:
> > assert(0);
> > }
> > --
> > 2.7.4
> >
>
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 03/13] acpi: cphp: support x2APIC entry in cpu._MAT
2016-10-18 13:46 ` Igor Mammedov
@ 2016-10-18 13:47 ` Eduardo Habkost
2016-10-18 14:02 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 13:47 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, Oct 18, 2016 at 03:46:05PM +0200, Igor Mammedov wrote:
> On Tue, 18 Oct 2016 11:34:55 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Thu, Oct 13, 2016 at 11:52:37AM +0200, Igor Mammedov wrote:
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > > hw/acpi/cpu.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> > > index 902f5c9..5ac89fe 100644
> > > --- a/hw/acpi/cpu.c
> > > +++ b/hw/acpi/cpu.c
> > > @@ -531,6 +531,11 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> > > 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;
> > > + }
> >
> > Shouldn't this patch be applied before 01/13 to avoid triggering
> > the assert() below?
> There is no AcpiMadtProcessorX2Apic before 1/13,
>
> how about squashing this patch into 1/13, that should be cleaner.
It would work, too. I assume you will squash it on v4 of 01/13.
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 03/13] acpi: cphp: support x2APIC entry in cpu._MAT
2016-10-18 13:47 ` Eduardo Habkost
@ 2016-10-18 14:02 ` Igor Mammedov
0 siblings, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 14:02 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 11:47:11 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, Oct 18, 2016 at 03:46:05PM +0200, Igor Mammedov wrote:
> > On Tue, 18 Oct 2016 11:34:55 -0200
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Thu, Oct 13, 2016 at 11:52:37AM +0200, Igor Mammedov wrote:
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > ---
> > > > hw/acpi/cpu.c | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> > > > index 902f5c9..5ac89fe 100644
> > > > --- a/hw/acpi/cpu.c
> > > > +++ b/hw/acpi/cpu.c
> > > > @@ -531,6 +531,11 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> > > > 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;
> > > > + }
> > >
> > > Shouldn't this patch be applied before 01/13 to avoid triggering
> > > the assert() below?
> > There is no AcpiMadtProcessorX2Apic before 1/13,
> >
> > how about squashing this patch into 1/13, that should be cleaner.
>
> It would work, too. I assume you will squash it on v4 of 01/13.
>
yep
^ permalink raw reply [flat|nested] 68+ messages in thread
* [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (2 preceding siblings ...)
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 03/13] acpi: cphp: support x2APIC entry in cpu._MAT Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-18 13:38 ` Eduardo Habkost
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code Igor Mammedov
` (11 subsequent siblings)
15 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
Switch to modern cpu hotplug at machine startup time if
a cpu present at boot has apic-id in range unsupported
by legacy cpu hotplug interface (i.e. > 254), to avoid
killing QEMU from legacy cpu hotplug code with error:
"acpi: invalid cpu id: #apic-id#"
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/cpu_hotplug.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index e19d902..c2ab9b8 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
cpu_id = k->get_arch_id(cpu);
if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
- error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
+ object_property_set_bool(g->device, false, "cpu-hotplug-legacy",
+ &error_abort);
return;
}
@@ -85,13 +86,14 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
{
CPUState *cpu;
- CPU_FOREACH(cpu) {
- acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
- }
memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
memory_region_add_subregion(parent, base, &gpe_cpu->io);
gpe_cpu->device = owner;
+
+ CPU_FOREACH(cpu) {
+ acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
+ }
}
void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu,
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254 Igor Mammedov
@ 2016-10-18 13:38 ` Eduardo Habkost
2016-10-18 14:34 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 13:38 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:
> Switch to modern cpu hotplug at machine startup time if
> a cpu present at boot has apic-id in range unsupported
> by legacy cpu hotplug interface (i.e. > 254), to avoid
> killing QEMU from legacy cpu hotplug code with error:
> "acpi: invalid cpu id: #apic-id#"
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/acpi/cpu_hotplug.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index e19d902..c2ab9b8 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
>
> cpu_id = k->get_arch_id(cpu);
> if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> - error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> + object_property_set_bool(g->device, false, "cpu-hotplug-legacy",
> + &error_abort);
What happens we are in legacy mode and this is triggered during
hotplug instead of machine init? Would it break something, or is
it safe?
Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
assert(!value). I assume this means we must replace the QOM
property with something that the user can't fiddle with, right?
> return;
> }
>
> @@ -85,13 +86,14 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
> {
> CPUState *cpu;
>
> - CPU_FOREACH(cpu) {
> - acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
> - }
> memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
> gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
> memory_region_add_subregion(parent, base, &gpe_cpu->io);
> gpe_cpu->device = owner;
> +
> + CPU_FOREACH(cpu) {
> + acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
> + }
> }
>
> void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu,
> --
> 2.7.4
>
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254
2016-10-18 13:38 ` Eduardo Habkost
@ 2016-10-18 14:34 ` Igor Mammedov
2016-10-18 15:05 ` Eduardo Habkost
0 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 14:34 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 11:38:31 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:
> > Switch to modern cpu hotplug at machine startup time if
> > a cpu present at boot has apic-id in range unsupported
> > by legacy cpu hotplug interface (i.e. > 254), to avoid
> > killing QEMU from legacy cpu hotplug code with error:
> > "acpi: invalid cpu id: #apic-id#"
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > hw/acpi/cpu_hotplug.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> > index e19d902..c2ab9b8 100644
> > --- a/hw/acpi/cpu_hotplug.c
> > +++ b/hw/acpi/cpu_hotplug.c
> > @@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
> >
> > cpu_id = k->get_arch_id(cpu);
> > if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> > - error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> > + object_property_set_bool(g->device, false, "cpu-hotplug-legacy",
> > + &error_abort);
>
> What happens we are in legacy mode and this is triggered during
> hotplug instead of machine init? Would it break something, or is
> it safe?
case 1: guest with legacy hotplug AML (migrated for example) would use
legacy interface and it won't be possible to trigger this path
as target should be started with the same CLI as source
(hence < 255 cpus)
case 2: guest started on new QEMU will have new hotplug AML which switches
QEMU to modern cpu hotplug interface at ACPI tables _INI time
so this path is unreachable.
Originally it's been static rule:
since 2.7 use new hotplug interface and old one for older machine types
Well it's complex but Michael insisted on keeping legacy hotplug
by default and do dynamic switching, so here we are.
this behavior is since 2.7:
commit 679dd1a957df418453efdd3ed2914dba5cd73773
pc: use new CPU hotplug interface since 2.7 machine type
> Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
> assert(!value). I assume this means we must replace the QOM
> property with something that the user can't fiddle with, right?
it's readonly to user after machine starts, but allows user
to play modern hotplug interface on old machine types if needed.
assert is there to trap mistake of switching to legacy mode
(which is default) from compat_properties.
>
> > return;
> > }
> >
> > @@ -85,13 +86,14 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
> > {
> > CPUState *cpu;
> >
> > - CPU_FOREACH(cpu) {
> > - acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
> > - }
> > memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
> > gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
> > memory_region_add_subregion(parent, base, &gpe_cpu->io);
> > gpe_cpu->device = owner;
> > +
> > + CPU_FOREACH(cpu) {
> > + acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
> > + }
> > }
> >
> > void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu,
> > --
> > 2.7.4
> >
>
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254
2016-10-18 14:34 ` Igor Mammedov
@ 2016-10-18 15:05 ` Eduardo Habkost
2016-10-18 15:23 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 15:05 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, Oct 18, 2016 at 04:34:53PM +0200, Igor Mammedov wrote:
> On Tue, 18 Oct 2016 11:38:31 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:
> > > Switch to modern cpu hotplug at machine startup time if
> > > a cpu present at boot has apic-id in range unsupported
> > > by legacy cpu hotplug interface (i.e. > 254), to avoid
> > > killing QEMU from legacy cpu hotplug code with error:
> > > "acpi: invalid cpu id: #apic-id#"
> > >
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > > hw/acpi/cpu_hotplug.c | 10 ++++++----
> > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> > > index e19d902..c2ab9b8 100644
> > > --- a/hw/acpi/cpu_hotplug.c
> > > +++ b/hw/acpi/cpu_hotplug.c
> > > @@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
> > >
> > > cpu_id = k->get_arch_id(cpu);
> > > if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> > > - error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> > > + object_property_set_bool(g->device, false, "cpu-hotplug-legacy",
> > > + &error_abort);
> >
> > What happens we are in legacy mode and this is triggered during
> > hotplug instead of machine init? Would it break something, or is
> > it safe?
> case 1: guest with legacy hotplug AML (migrated for example) would use
> legacy interface and it won't be possible to trigger this path
> as target should be started with the same CLI as source
> (hence < 255 cpus)
> case 2: guest started on new QEMU will have new hotplug AML which switches
> QEMU to modern cpu hotplug interface at ACPI tables _INI time
> so this path is unreachable.
I see. Thanks for the explanation!
>
> Originally it's been static rule:
> since 2.7 use new hotplug interface and old one for older machine types
> Well it's complex but Michael insisted on keeping legacy hotplug
> by default and do dynamic switching, so here we are.
>
This means PCMachineClass::legacy_cpu_hotplug means "legacy CPU
hotplug _only_", because legacy CPU hotplug is always available
on startup, right?
> this behavior is since 2.7:
> commit 679dd1a957df418453efdd3ed2914dba5cd73773
> pc: use new CPU hotplug interface since 2.7 machine type
>
>
> > Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
> > assert(!value). I assume this means we must replace the QOM
> > property with something that the user can't fiddle with, right?
> it's readonly to user after machine starts, but allows user
> to play modern hotplug interface on old machine types if needed.
> assert is there to trap mistake of switching to legacy mode
> (which is default) from compat_properties.
What exactly makes the property read-only to the user? Maybe we
should make the setter return an error instead, as all
object_property_set_bool("cpu-hotplug-legacy") calls already use
&error_abort?
>
> >
> > > return;
> > > }
> > >
> > > @@ -85,13 +86,14 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
> > > {
> > > CPUState *cpu;
> > >
> > > - CPU_FOREACH(cpu) {
> > > - acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
> > > - }
> > > memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
> > > gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
> > > memory_region_add_subregion(parent, base, &gpe_cpu->io);
> > > gpe_cpu->device = owner;
> > > +
> > > + CPU_FOREACH(cpu) {
> > > + acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
> > > + }
> > > }
> > >
> > > void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu,
> > > --
> > > 2.7.4
> > >
> >
>
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254
2016-10-18 15:05 ` Eduardo Habkost
@ 2016-10-18 15:23 ` Igor Mammedov
2016-10-18 16:37 ` Eduardo Habkost
0 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 15:23 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 13:05:51 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, Oct 18, 2016 at 04:34:53PM +0200, Igor Mammedov wrote:
> > On Tue, 18 Oct 2016 11:38:31 -0200
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:
> > > > Switch to modern cpu hotplug at machine startup time if
> > > > a cpu present at boot has apic-id in range unsupported
> > > > by legacy cpu hotplug interface (i.e. > 254), to avoid
> > > > killing QEMU from legacy cpu hotplug code with error:
> > > > "acpi: invalid cpu id: #apic-id#"
> > > >
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > ---
> > > > hw/acpi/cpu_hotplug.c | 10 ++++++----
> > > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> > > > index e19d902..c2ab9b8 100644
> > > > --- a/hw/acpi/cpu_hotplug.c
> > > > +++ b/hw/acpi/cpu_hotplug.c
> > > > @@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
> > > >
> > > > cpu_id = k->get_arch_id(cpu);
> > > > if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> > > > - error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> > > > + object_property_set_bool(g->device, false, "cpu-hotplug-legacy",
> > > > + &error_abort);
> > >
> > > What happens we are in legacy mode and this is triggered during
> > > hotplug instead of machine init? Would it break something, or is
> > > it safe?
> > case 1: guest with legacy hotplug AML (migrated for example) would use
> > legacy interface and it won't be possible to trigger this path
> > as target should be started with the same CLI as source
> > (hence < 255 cpus)
> > case 2: guest started on new QEMU will have new hotplug AML which switches
> > QEMU to modern cpu hotplug interface at ACPI tables _INI time
> > so this path is unreachable.
>
> I see. Thanks for the explanation!
>
> >
> > Originally it's been static rule:
> > since 2.7 use new hotplug interface and old one for older machine types
> > Well it's complex but Michael insisted on keeping legacy hotplug
> > by default and do dynamic switching, so here we are.
> >
>
> This means PCMachineClass::legacy_cpu_hotplug means "legacy CPU
> hotplug _only_", because legacy CPU hotplug is always available
> on startup, right?
Sorry, I can't parse question, could you rephrase?
>
> > this behavior is since 2.7:
> > commit 679dd1a957df418453efdd3ed2914dba5cd73773
> > pc: use new CPU hotplug interface since 2.7 machine type
> >
> >
> > > Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
> > > assert(!value). I assume this means we must replace the QOM
> > > property with something that the user can't fiddle with, right?
> > it's readonly to user after machine starts, but allows user
> > to play modern hotplug interface on old machine types if needed.
> > assert is there to trap mistake of switching to legacy mode
> > (which is default) from compat_properties.
>
> What exactly makes the property read-only to the user? Maybe we
> should make the setter return an error instead, as all
> object_property_set_bool("cpu-hotplug-legacy") calls already use
> &error_abort?
My mistake,
it's dynamic property with custom setters in piix4/ich9_pm and user
potentially can write there.
I was under impression that errors are ignored if property comes from
compat_props, if returning error would prevent machine from starting
when property comes from compat_props I can fix cpu-hotplug-legacy to
return error.
>
> >
> > >
> > > > return;
> > > > }
> > > >
> > > > @@ -85,13 +86,14 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
> > > > {
> > > > CPUState *cpu;
> > > >
> > > > - CPU_FOREACH(cpu) {
> > > > - acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
> > > > - }
> > > > memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
> > > > gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
> > > > memory_region_add_subregion(parent, base, &gpe_cpu->io);
> > > > gpe_cpu->device = owner;
> > > > +
> > > > + CPU_FOREACH(cpu) {
> > > > + acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
> > > > + }
> > > > }
> > > >
> > > > void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu,
> > > > --
> > > > 2.7.4
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254
2016-10-18 15:23 ` Igor Mammedov
@ 2016-10-18 16:37 ` Eduardo Habkost
2016-10-19 10:35 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 16:37 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, Oct 18, 2016 at 05:23:04PM +0200, Igor Mammedov wrote:
> On Tue, 18 Oct 2016 13:05:51 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Tue, Oct 18, 2016 at 04:34:53PM +0200, Igor Mammedov wrote:
> > > On Tue, 18 Oct 2016 11:38:31 -0200
> > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > >
> > > > On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:
> > > > > Switch to modern cpu hotplug at machine startup time if
> > > > > a cpu present at boot has apic-id in range unsupported
> > > > > by legacy cpu hotplug interface (i.e. > 254), to avoid
> > > > > killing QEMU from legacy cpu hotplug code with error:
> > > > > "acpi: invalid cpu id: #apic-id#"
> > > > >
> > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > > ---
> > > > > hw/acpi/cpu_hotplug.c | 10 ++++++----
> > > > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> > > > > index e19d902..c2ab9b8 100644
> > > > > --- a/hw/acpi/cpu_hotplug.c
> > > > > +++ b/hw/acpi/cpu_hotplug.c
> > > > > @@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
> > > > >
> > > > > cpu_id = k->get_arch_id(cpu);
> > > > > if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> > > > > - error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> > > > > + object_property_set_bool(g->device, false, "cpu-hotplug-legacy",
> > > > > + &error_abort);
> > > >
> > > > What happens we are in legacy mode and this is triggered during
> > > > hotplug instead of machine init? Would it break something, or is
> > > > it safe?
> > > case 1: guest with legacy hotplug AML (migrated for example) would use
> > > legacy interface and it won't be possible to trigger this path
> > > as target should be started with the same CLI as source
> > > (hence < 255 cpus)
> > > case 2: guest started on new QEMU will have new hotplug AML which switches
> > > QEMU to modern cpu hotplug interface at ACPI tables _INI time
> > > so this path is unreachable.
> >
> > I see. Thanks for the explanation!
> >
> > >
> > > Originally it's been static rule:
> > > since 2.7 use new hotplug interface and old one for older machine types
> > > Well it's complex but Michael insisted on keeping legacy hotplug
> > > by default and do dynamic switching, so here we are.
Do you rememer the reasoning for that?
> > >
> >
> > This means PCMachineClass::legacy_cpu_hotplug means "legacy CPU
> > hotplug _only_", because legacy CPU hotplug is always available
> > on startup, right?
> Sorry, I can't parse question, could you rephrase?
I was just trying to clarify the meaning of
PCMachineClass::legacy_cpu_hotplug.
I thought legacy CPU hotplug was available only if
PCMC::legacy_cpu_hotplug was set. But legacy hotplug is still
available even on pc-2.7 (and then switched dynamically).
So the difference is that PCMC::legacy_cpu_hotplug only disables
the ability to dynamically switch to modern hotplug, but doesn't
disable legacy hotplug completely. Correct?
>
> >
> > > this behavior is since 2.7:
> > > commit 679dd1a957df418453efdd3ed2914dba5cd73773
> > > pc: use new CPU hotplug interface since 2.7 machine type
> > >
> > >
> > > > Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
> > > > assert(!value). I assume this means we must replace the QOM
> > > > property with something that the user can't fiddle with, right?
> > > it's readonly to user after machine starts, but allows user
> > > to play modern hotplug interface on old machine types if needed.
> > > assert is there to trap mistake of switching to legacy mode
> > > (which is default) from compat_properties.
> >
> > What exactly makes the property read-only to the user? Maybe we
> > should make the setter return an error instead, as all
> > object_property_set_bool("cpu-hotplug-legacy") calls already use
> > &error_abort?
> My mistake,
> it's dynamic property with custom setters in piix4/ich9_pm and user
> potentially can write there.
>
> I was under impression that errors are ignored if property comes from
> compat_props, if returning error would prevent machine from starting
> when property comes from compat_props I can fix cpu-hotplug-legacy to
> return error.
compat_props set errp to &error_abort, see
machine_register_compat_props().
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254
2016-10-18 16:37 ` Eduardo Habkost
@ 2016-10-19 10:35 ` Igor Mammedov
0 siblings, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-19 10:35 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 14:37:39 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, Oct 18, 2016 at 05:23:04PM +0200, Igor Mammedov wrote:
> > On Tue, 18 Oct 2016 13:05:51 -0200
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Tue, Oct 18, 2016 at 04:34:53PM +0200, Igor Mammedov wrote:
> > > > On Tue, 18 Oct 2016 11:38:31 -0200
> > > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > >
> > > > > On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:
> > > > > > Switch to modern cpu hotplug at machine startup time if
> > > > > > a cpu present at boot has apic-id in range unsupported
> > > > > > by legacy cpu hotplug interface (i.e. > 254), to avoid
> > > > > > killing QEMU from legacy cpu hotplug code with error:
> > > > > > "acpi: invalid cpu id: #apic-id#"
> > > > > >
> > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > > > ---
> > > > > > hw/acpi/cpu_hotplug.c | 10 ++++++----
> > > > > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> > > > > > index e19d902..c2ab9b8 100644
> > > > > > --- a/hw/acpi/cpu_hotplug.c
> > > > > > +++ b/hw/acpi/cpu_hotplug.c
> > > > > > @@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
> > > > > >
> > > > > > cpu_id = k->get_arch_id(cpu);
> > > > > > if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> > > > > > - error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> > > > > > + object_property_set_bool(g->device, false, "cpu-hotplug-legacy",
> > > > > > + &error_abort);
> > > > >
> > > > > What happens we are in legacy mode and this is triggered during
> > > > > hotplug instead of machine init? Would it break something, or is
> > > > > it safe?
> > > > case 1: guest with legacy hotplug AML (migrated for example) would use
> > > > legacy interface and it won't be possible to trigger this path
> > > > as target should be started with the same CLI as source
> > > > (hence < 255 cpus)
> > > > case 2: guest started on new QEMU will have new hotplug AML which switches
> > > > QEMU to modern cpu hotplug interface at ACPI tables _INI time
> > > > so this path is unreachable.
> > >
> > > I see. Thanks for the explanation!
> > >
> > > >
> > > > Originally it's been static rule:
> > > > since 2.7 use new hotplug interface and old one for older machine types
> > > > Well it's complex but Michael insisted on keeping legacy hotplug
> > > > by default and do dynamic switching, so here we are.
>
> Do you rememer the reasoning for that?
Ancient SeaBIOS that doesn't get ACPI tables from QEMU supports only
legacy interface. So it's been argued that new machines should start
in legacy mode so that CPU hotplug would be operational with old SeaBIOS.
That's not applicable in practice as QEMU with new hotplug support ships
with SeaBIOS supporting ACPI tables from QEMU.
So the only case, when it could happen, is user overrides default bios
image with old one for purposes of debugging old bios with new machine
type. (that user could have used old machine type for that purpose and
we could have kept code simpler, but train already left and we are
stuck with supporting dynamic switching).
> > > >
> > >
> > > This means PCMachineClass::legacy_cpu_hotplug means "legacy CPU
> > > hotplug _only_", because legacy CPU hotplug is always available
> > > on startup, right?
> > Sorry, I can't parse question, could you rephrase?
>
> I was just trying to clarify the meaning of
> PCMachineClass::legacy_cpu_hotplug.
>
> I thought legacy CPU hotplug was available only if
> PCMC::legacy_cpu_hotplug was set. But legacy hotplug is still
> available even on pc-2.7 (and then switched dynamically).
both hoptlug methods available even on pre pc-2.7 machines,
and legacy hotplug is used by default. User can one way switch
mode to modern hotplug by setting cpu-hotplug-legacy prop to false
which is done by writing 0 at the beginning of legacy CPU bitmap
from AML by guest.
>
> So the difference is that PCMC::legacy_cpu_hotplug only disables
> the ability to dynamically switch to modern hotplug, but doesn't
> disable legacy hotplug completely. Correct?
yep,
more exactly PCMC::legacy_cpu_hotplug picks whether legacy or modern
AML code should be generated by QEMU.
After guest is switched to modern mode, it can't switch back
'abort(!value)' takes care about that.
>
> >
> > >
> > > > this behavior is since 2.7:
> > > > commit 679dd1a957df418453efdd3ed2914dba5cd73773
> > > > pc: use new CPU hotplug interface since 2.7 machine type
> > > >
> > > >
> > > > > Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
> > > > > assert(!value). I assume this means we must replace the QOM
> > > > > property with something that the user can't fiddle with, right?
> > > > it's readonly to user after machine starts, but allows user
> > > > to play modern hotplug interface on old machine types if needed.
> > > > assert is there to trap mistake of switching to legacy mode
> > > > (which is default) from compat_properties.
> > >
> > > What exactly makes the property read-only to the user? Maybe we
> > > should make the setter return an error instead, as all
> > > object_property_set_bool("cpu-hotplug-legacy") calls already use
> > > &error_abort?
> > My mistake,
> > it's dynamic property with custom setters in piix4/ich9_pm and user
> > potentially can write there.
> >
> > I was under impression that errors are ignored if property comes from
> > compat_props, if returning error would prevent machine from starting
> > when property comes from compat_props I can fix cpu-hotplug-legacy to
> > return error.
>
> compat_props set errp to &error_abort, see
> machine_register_compat_props().
Than I guess that assert could be replaced by error_setg(),
I'll post a separate patch for this (as it's unrelated to this series).
^ permalink raw reply [flat|nested] 68+ messages in thread
* [Qemu-devel] [PATCH v3 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (3 preceding siblings ...)
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254 Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-17 21:44 ` Eduardo Habkost
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 06/13] pc: apic_common: extend APIC ID property to 32bit Igor Mammedov
` (10 subsequent siblings)
15 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
that's enough to make old code that depends on it
to prevent QEMU starting with more than 255 CPUs.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/cpu_hotplug.c | 7 ++++++-
hw/i386/pc.c | 7 -------
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index c2ab9b8..f15a240 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -15,6 +15,7 @@
#include "qapi/error.h"
#include "qom/cpu.h"
#include "hw/i386/pc.h"
+#include "qemu/error-report.h"
#define CPU_EJECT_METHOD "CPEJ"
#define CPU_MAT_METHOD "CPMA"
@@ -236,7 +237,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
/* The current AML generator can cover the APIC ID range [0..255],
* inclusive, for VCPU hotplug. */
QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
- g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
+ if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
+ error_report("max_cpus is too large. APIC ID of last CPU is %u",
+ pcms->apic_id_limit - 1);
+ exit(1);
+ }
/* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 93ff49c..f1c1013 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -778,7 +778,6 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
for (i = 0; i < max_cpus; i++) {
unsigned int apic_id = x86_cpu_apic_id_from_index(i);
- assert(apic_id < pcms->apic_id_limit);
j = numa_get_node_for_cpu(i);
if (j < nb_numa_nodes) {
numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
@@ -1190,12 +1189,6 @@ void pc_cpus_init(PCMachineState *pcms)
* This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
*/
pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
- if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
- error_report("max_cpus is too large. APIC ID of last CPU is %u",
- pcms->apic_id_limit - 1);
- exit(1);
- }
-
pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
sizeof(CPUArchId) * max_cpus);
for (i = 0; i < max_cpus; i++) {
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code Igor Mammedov
@ 2016-10-17 21:44 ` Eduardo Habkost
2016-10-18 9:02 ` Igor Mammedov
2016-10-18 9:12 ` [Qemu-devel] [PATCH v3 " Igor Mammedov
0 siblings, 2 replies; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-17 21:44 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Thu, Oct 13, 2016 at 11:52:39AM +0200, Igor Mammedov wrote:
[...]
> @@ -236,7 +237,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
> /* The current AML generator can cover the APIC ID range [0..255],
> * inclusive, for VCPU hotplug. */
> QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
> - g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
> + if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> + error_report("max_cpus is too large. APIC ID of last CPU is %u",
> + pcms->apic_id_limit - 1);
> + exit(1);
> + }
Moving the check here seems to make sense, but:
>
> /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
> dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 93ff49c..f1c1013 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -778,7 +778,6 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
[Added more context below to show the code around the change]
> numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
> numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
> for (i = 0; i < max_cpus; i++) {
> unsigned int apic_id = x86_cpu_apic_id_from_index(i);
> - assert(apic_id < pcms->apic_id_limit);
If you really needed to remove this assert, that means you can
write beyond the end of numa_fw_fg[] below. Are you sure you need
to remove it?
> j = numa_get_node_for_cpu(i);
> if (j < nb_numa_nodes) {
> numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
^^^^^^^^^^^ here
> }
> }
>
> @@ -1190,12 +1189,6 @@ void pc_cpus_init(PCMachineState *pcms)
> * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
> */
> pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
> - if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> - error_report("max_cpus is too large. APIC ID of last CPU is %u",
> - pcms->apic_id_limit - 1);
> - exit(1);
> - }
> -
> pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> sizeof(CPUArchId) * max_cpus);
> for (i = 0; i < max_cpus; i++) {
> --
> 2.7.4
>
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code
2016-10-17 21:44 ` Eduardo Habkost
@ 2016-10-18 9:02 ` Igor Mammedov
2016-10-18 10:31 ` Eduardo Habkost
2016-10-18 9:12 ` [Qemu-devel] [PATCH v3 " Igor Mammedov
1 sibling, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 9:02 UTC (permalink / raw)
To: Eduardo Habkost
Cc: liuxiaojian6, mst, rkrcmar, qemu-devel, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
On Mon, 17 Oct 2016 19:44:52 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Thu, Oct 13, 2016 at 11:52:39AM +0200, Igor Mammedov wrote:
> [...]
> > @@ -236,7 +237,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
> > /* The current AML generator can cover the APIC ID range [0..255],
> > * inclusive, for VCPU hotplug. */
> > QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
> > - g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
> > + if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> > + error_report("max_cpus is too large. APIC ID of last CPU is %u",
> > + pcms->apic_id_limit - 1);
> > + exit(1);
> > + }
>
> Moving the check here seems to make sense, but:
>
> >
> > /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
> > dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 93ff49c..f1c1013 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -778,7 +778,6 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
>
> [Added more context below to show the code around the change]
>
> > numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
> > numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
> > for (i = 0; i < max_cpus; i++) {
> > unsigned int apic_id = x86_cpu_apic_id_from_index(i);
> > - assert(apic_id < pcms->apic_id_limit);
>
> If you really needed to remove this assert, that means you can
> write beyond the end of numa_fw_fg[] below. Are you sure you need
> to remove it?
>
> > j = numa_get_node_for_cpu(i);
> > if (j < nb_numa_nodes) {
> > numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
>
> ^^^^^^^^^^^ here
Shouldn't above
numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
allocate sufficiently sized array?
That's aside, the assert could be kept as it doesn't get in a way
if you'd prefer it that way.
> > }
> > }
> >
> > @@ -1190,12 +1189,6 @@ void pc_cpus_init(PCMachineState *pcms)
> > * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
> > */
> > pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
> > - if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> > - error_report("max_cpus is too large. APIC ID of last CPU is %u",
> > - pcms->apic_id_limit - 1);
> > - exit(1);
> > - }
> > -
> > pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> > sizeof(CPUArchId) * max_cpus);
> > for (i = 0; i < max_cpus; i++) {
> > --
> > 2.7.4
> >
>
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code
2016-10-18 9:02 ` Igor Mammedov
@ 2016-10-18 10:31 ` Eduardo Habkost
2016-10-18 11:37 ` [Qemu-devel] [PATCH v4 " Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 10:31 UTC (permalink / raw)
To: Igor Mammedov
Cc: liuxiaojian6, mst, rkrcmar, qemu-devel, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
On Tue, Oct 18, 2016 at 11:02:54AM +0200, Igor Mammedov wrote:
> On Mon, 17 Oct 2016 19:44:52 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Thu, Oct 13, 2016 at 11:52:39AM +0200, Igor Mammedov wrote:
> > [...]
> > > @@ -236,7 +237,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
> > > /* The current AML generator can cover the APIC ID range [0..255],
> > > * inclusive, for VCPU hotplug. */
> > > QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
> > > - g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
> > > + if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> > > + error_report("max_cpus is too large. APIC ID of last CPU is %u",
> > > + pcms->apic_id_limit - 1);
> > > + exit(1);
> > > + }
> >
> > Moving the check here seems to make sense, but:
> >
> > >
> > > /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
> > > dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
> > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > index 93ff49c..f1c1013 100644
> > > --- a/hw/i386/pc.c
> > > +++ b/hw/i386/pc.c
> > > @@ -778,7 +778,6 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
> >
> > [Added more context below to show the code around the change]
> >
> > > numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
> > > numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
> > > for (i = 0; i < max_cpus; i++) {
> > > unsigned int apic_id = x86_cpu_apic_id_from_index(i);
> > > - assert(apic_id < pcms->apic_id_limit);
> >
> > If you really needed to remove this assert, that means you can
> > write beyond the end of numa_fw_fg[] below. Are you sure you need
> > to remove it?
> >
> > > j = numa_get_node_for_cpu(i);
> > > if (j < nb_numa_nodes) {
> > > numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
> >
> > ^^^^^^^^^^^ here
> Shouldn't above
> numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
> allocate sufficiently sized array?
I believe it should, but that would mean the assert() is still
valid.
>
> That's aside, the assert could be kept as it doesn't get in a way
> if you'd prefer it that way.
Yes, please. The assert() removal seems unnecessary (and
confusing, because it made me believe that the condition was not
going to be valid anymore).
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* [Qemu-devel] [PATCH v4 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code
2016-10-18 10:31 ` Eduardo Habkost
@ 2016-10-18 11:37 ` Igor Mammedov
2016-10-18 12:01 ` Eduardo Habkost
0 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 11:37 UTC (permalink / raw)
To: qemu-devel
Cc: liuxiaojian6, mst, rkrcmar, peterx, kevin, kraxel, pbonzini,
lersek, chao.gao, ehabkost
that's enough to make old code that depends on it
to prevent QEMU starting with more than 255 CPUs.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v4:
- keep assert() as it doesn't affect x2APIC cpus (Eduardo)
---
hw/acpi/cpu_hotplug.c | 7 ++++++-
hw/i386/pc.c | 6 ------
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index c2ab9b8..f15a240 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -15,6 +15,7 @@
#include "qapi/error.h"
#include "qom/cpu.h"
#include "hw/i386/pc.h"
+#include "qemu/error-report.h"
#define CPU_EJECT_METHOD "CPEJ"
#define CPU_MAT_METHOD "CPMA"
@@ -236,7 +237,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
/* The current AML generator can cover the APIC ID range [0..255],
* inclusive, for VCPU hotplug. */
QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
- g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
+ if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
+ error_report("max_cpus is too large. APIC ID of last CPU is %u",
+ pcms->apic_id_limit - 1);
+ exit(1);
+ }
/* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 93ff49c..2045525 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1190,12 +1190,6 @@ void pc_cpus_init(PCMachineState *pcms)
* This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
*/
pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
- if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
- error_report("max_cpus is too large. APIC ID of last CPU is %u",
- pcms->apic_id_limit - 1);
- exit(1);
- }
-
pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
sizeof(CPUArchId) * max_cpus);
for (i = 0; i < max_cpus; i++) {
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v4 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code
2016-10-18 11:37 ` [Qemu-devel] [PATCH v4 " Igor Mammedov
@ 2016-10-18 12:01 ` Eduardo Habkost
0 siblings, 0 replies; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 12:01 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, liuxiaojian6, mst, rkrcmar, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
On Tue, Oct 18, 2016 at 01:37:46PM +0200, Igor Mammedov wrote:
> that's enough to make old code that depends on it
> to prevent QEMU starting with more than 255 CPUs.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [Qemu-devel] [PATCH v3 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code
2016-10-17 21:44 ` Eduardo Habkost
2016-10-18 9:02 ` Igor Mammedov
@ 2016-10-18 9:12 ` Igor Mammedov
2016-10-18 10:39 ` Eduardo Habkost
1 sibling, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 9:12 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Mon, 17 Oct 2016 19:44:52 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Thu, Oct 13, 2016 at 11:52:39AM +0200, Igor Mammedov wrote:
> [...]
> > @@ -236,7 +237,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
> > /* The current AML generator can cover the APIC ID range [0..255],
> > * inclusive, for VCPU hotplug. */
> > QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
> > - g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
> > + if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> > + error_report("max_cpus is too large. APIC ID of last CPU is %u",
> > + pcms->apic_id_limit - 1);
> > + exit(1);
> > + }
>
> Moving the check here seems to make sense, but:
>
> >
> > /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
> > dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 93ff49c..f1c1013 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -778,7 +778,6 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
>
> [Added more context below to show the code around the change]
>
> > numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
> > numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
> > for (i = 0; i < max_cpus; i++) {
> > unsigned int apic_id = x86_cpu_apic_id_from_index(i);
> > - assert(apic_id < pcms->apic_id_limit);
>
> If you really needed to remove this assert, that means you can
> write beyond the end of numa_fw_fg[] below. Are you sure you need
> to remove it?
>
> > j = numa_get_node_for_cpu(i);
> > if (j < nb_numa_nodes) {
> > numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
>
> ^^^^^^^^^^^ here
>
> > }
Another more radical way to deal with legacy FW_CFG_NUMA
could be to remove it altogether if machine has x2APIC cpus.
It'd be possible to do due to BIOS using QEMU provided
BIOS tables and not using/calling code for built in/legacy
ACPI tables.
Could do it as patch on top if that's sounds ok.
> > }
> >
> > @@ -1190,12 +1189,6 @@ void pc_cpus_init(PCMachineState *pcms)
> > * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
> > */
> > pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
> > - if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> > - error_report("max_cpus is too large. APIC ID of last CPU is %u",
> > - pcms->apic_id_limit - 1);
> > - exit(1);
> > - }
> > -
> > pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
> > sizeof(CPUArchId) * max_cpus);
> > for (i = 0; i < max_cpus; i++) {
> > --
> > 2.7.4
> >
>
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code
2016-10-18 9:12 ` [Qemu-devel] [PATCH v3 " Igor Mammedov
@ 2016-10-18 10:39 ` Eduardo Habkost
2016-10-18 12:10 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 10:39 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, Oct 18, 2016 at 11:12:04AM +0200, Igor Mammedov wrote:
> On Mon, 17 Oct 2016 19:44:52 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Thu, Oct 13, 2016 at 11:52:39AM +0200, Igor Mammedov wrote:
> > [...]
> > > @@ -236,7 +237,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
> > > /* The current AML generator can cover the APIC ID range [0..255],
> > > * inclusive, for VCPU hotplug. */
> > > QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
> > > - g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
> > > + if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> > > + error_report("max_cpus is too large. APIC ID of last CPU is %u",
> > > + pcms->apic_id_limit - 1);
> > > + exit(1);
> > > + }
> >
> > Moving the check here seems to make sense, but:
> >
> > >
> > > /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
> > > dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
> > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > index 93ff49c..f1c1013 100644
> > > --- a/hw/i386/pc.c
> > > +++ b/hw/i386/pc.c
> > > @@ -778,7 +778,6 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
> >
> > [Added more context below to show the code around the change]
> >
> > > numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
> > > numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
> > > for (i = 0; i < max_cpus; i++) {
> > > unsigned int apic_id = x86_cpu_apic_id_from_index(i);
> > > - assert(apic_id < pcms->apic_id_limit);
> >
> > If you really needed to remove this assert, that means you can
> > write beyond the end of numa_fw_fg[] below. Are you sure you need
> > to remove it?
> >
> > > j = numa_get_node_for_cpu(i);
> > > if (j < nb_numa_nodes) {
> > > numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
> >
> > ^^^^^^^^^^^ here
> >
> > > }
> Another more radical way to deal with legacy FW_CFG_NUMA
> could be to remove it altogether if machine has x2APIC cpus.
> It'd be possible to do due to BIOS using QEMU provided
> BIOS tables and not using/calling code for built in/legacy
> ACPI tables.
>
> Could do it as patch on top if that's sounds ok.
I don't mind either way. Initializing the fields even if they are
not going to be used seems harmless, but if you believe skipping
it would make things simpler, go ahead.
In this case, what would happen if x2apic CPUs are available but
the BIOS is too old?
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code
2016-10-18 10:39 ` Eduardo Habkost
@ 2016-10-18 12:10 ` Igor Mammedov
0 siblings, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 12:10 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 08:39:05 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, Oct 18, 2016 at 11:12:04AM +0200, Igor Mammedov wrote:
> > On Mon, 17 Oct 2016 19:44:52 -0200
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Thu, Oct 13, 2016 at 11:52:39AM +0200, Igor Mammedov wrote:
> > > [...]
> > > > @@ -236,7 +237,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState *machine,
> > > > /* The current AML generator can cover the APIC ID range [0..255],
> > > > * inclusive, for VCPU hotplug. */
> > > > QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
> > > > - g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
> > > > + if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
> > > > + error_report("max_cpus is too large. APIC ID of last CPU is %u",
> > > > + pcms->apic_id_limit - 1);
> > > > + exit(1);
> > > > + }
> > >
> > > Moving the check here seems to make sense, but:
> > >
> > > >
> > > > /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
> > > > dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
> > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > > index 93ff49c..f1c1013 100644
> > > > --- a/hw/i386/pc.c
> > > > +++ b/hw/i386/pc.c
> > > > @@ -778,7 +778,6 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
> > >
> > > [Added more context below to show the code around the change]
> > >
> > > > numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
> > > > numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
> > > > for (i = 0; i < max_cpus; i++) {
> > > > unsigned int apic_id = x86_cpu_apic_id_from_index(i);
> > > > - assert(apic_id < pcms->apic_id_limit);
> > >
> > > If you really needed to remove this assert, that means you can
> > > write beyond the end of numa_fw_fg[] below. Are you sure you need
> > > to remove it?
> > >
> > > > j = numa_get_node_for_cpu(i);
> > > > if (j < nb_numa_nodes) {
> > > > numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
> > >
> > > ^^^^^^^^^^^ here
> > >
> > > > }
> > Another more radical way to deal with legacy FW_CFG_NUMA
> > could be to remove it altogether if machine has x2APIC cpus.
> > It'd be possible to do due to BIOS using QEMU provided
> > BIOS tables and not using/calling code for built in/legacy
> > ACPI tables.
> >
> > Could do it as patch on top if that's sounds ok.
>
> I don't mind either way. Initializing the fields even if they are
> not going to be used seems harmless, but if you believe skipping
> it would make things simpler, go ahead.
Keying of >255 cpus is a chance to drop legacy FW_CFG_NUMA which isn't
used by modern QEMU/SeaBIOS.
We can't do that for < 255 as it would break migration/BIOS where
it is already present.
I'll look at it some more from SeaBIOS perspective if it's feasible/useful.
> In this case, what would happen if x2apic CPUs are available but
> the BIOS is too old?
Old BIOS is unfixable is there are more than 255,
most often old BIOS would hang in smp_setup() waiting
cmos_smp_count CPUs to wakeup or crash later if smp_scan() lucky and
wins the race.
Or BIOS could fail earlier/later during buffer overflow/allocation
when initializing legacy ACPI tables/pir/mp_table if etc/max-cpus
is big enough.
Corresponding SeaBIOS series is mentioned in cover letter.
This series or dropping FW_CFG_NUMA should not affect (existing) VMs
with less that 255 CPUs though.
^ permalink raw reply [flat|nested] 68+ messages in thread
* [Qemu-devel] [PATCH v3 06/13] pc: apic_common: extend APIC ID property to 32bit
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (4 preceding siblings ...)
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 05/13] pc: leave max apic_id_limit only in legacy cpu hotplug code Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-18 10:56 ` Eduardo Habkost
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 07/13] pc: apic_common: restore APIC ID to initial ID on reset Igor Mammedov
` (9 subsequent siblings)
15 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
ACPI ID is 32 bit wide on CPUs with x2APIC support.
Extend 'id' property to support it.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
keep original behaviour where 'id' is readonly after
object is realized (pbonzini)
---
include/hw/i386/apic_internal.h | 3 ++-
target-i386/cpu.h | 1 +
hw/intc/apic_common.c | 46 ++++++++++++++++++++++++++++++++++++++++-
target-i386/cpu.c | 2 +-
4 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index cdd11fb..1209eb4 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -160,7 +160,8 @@ struct APICCommonState {
MemoryRegion io_memory;
X86CPU *cpu;
uint32_t apicbase;
- uint8_t id;
+ uint8_t id; /* legacy APIC ID */
+ uint32_t initial_apic_id;
uint8_t version;
uint8_t arb_id;
uint8_t tpr;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index e645698..6303d65 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -325,6 +325,7 @@
#define MSR_IA32_APICBASE 0x1b
#define MSR_IA32_APICBASE_BSP (1<<8)
#define MSR_IA32_APICBASE_ENABLE (1<<11)
+#define MSR_IA32_APICBASE_EXTD (1 << 10)
#define MSR_IA32_APICBASE_BASE (0xfffffU<<12)
#define MSR_IA32_FEATURE_CONTROL 0x0000003a
#define MSR_TSC_ADJUST 0x0000003b
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 8d01c9c..30f2af0 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -22,6 +22,7 @@
#include "qapi/error.h"
#include "qemu-common.h"
#include "cpu.h"
+#include "qapi/visitor.h"
#include "hw/i386/apic.h"
#include "hw/i386/apic_internal.h"
#include "trace.h"
@@ -428,7 +429,6 @@ static const VMStateDescription vmstate_apic_common = {
};
static Property apic_properties_common[] = {
- DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
DEFINE_PROP_UINT8("version", APICCommonState, version, 0x14),
DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
true),
@@ -437,6 +437,49 @@ static Property apic_properties_common[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static void apic_common_get_id(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ APICCommonState *s = APIC_COMMON(obj);
+ int64_t value;
+
+ value = s->apicbase & MSR_IA32_APICBASE_EXTD ? s->initial_apic_id : s->id;
+ visit_type_int(v, name, &value, errp);
+}
+
+static void apic_common_set_id(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ APICCommonState *s = APIC_COMMON(obj);
+ DeviceState *dev = DEVICE(obj);
+ Error *local_err = NULL;
+ int64_t value;
+
+ if (dev->realized) {
+ qdev_prop_set_after_realize(dev, name, errp);
+ return;
+ }
+
+ visit_type_int(v, name, &value, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ s->initial_apic_id = value;
+ s->id = (uint8_t)value;
+}
+
+static void apic_common_initfn(Object *obj)
+{
+ APICCommonState *s = APIC_COMMON(obj);
+
+ s->id = s->initial_apic_id = -1;
+ object_property_add(obj, "id", "int",
+ apic_common_get_id,
+ apic_common_set_id, NULL, NULL, NULL);
+}
+
static void apic_common_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -456,6 +499,7 @@ static const TypeInfo apic_common_type = {
.name = TYPE_APIC_COMMON,
.parent = TYPE_DEVICE,
.instance_size = sizeof(APICCommonState),
+ .instance_init = apic_common_initfn,
.class_size = sizeof(APICCommonClass),
.class_init = apic_common_class_init,
.abstract = true,
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 13505ab..b4b4342 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2872,7 +2872,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
OBJECT(cpu->apic_state), &error_abort);
object_unref(OBJECT(cpu->apic_state));
- qdev_prop_set_uint8(cpu->apic_state, "id", cpu->apic_id);
+ qdev_prop_set_uint32(cpu->apic_state, "id", cpu->apic_id);
/* TODO: convert to link<> */
apic = APIC_COMMON(cpu->apic_state);
apic->cpu = cpu;
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/13] pc: apic_common: extend APIC ID property to 32bit
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 06/13] pc: apic_common: extend APIC ID property to 32bit Igor Mammedov
@ 2016-10-18 10:56 ` Eduardo Habkost
2016-10-18 12:36 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 10:56 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Thu, Oct 13, 2016 at 11:52:40AM +0200, Igor Mammedov wrote:
> ACPI ID is 32 bit wide on CPUs with x2APIC support.
> Extend 'id' property to support it.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v3:
> keep original behaviour where 'id' is readonly after
> object is realized (pbonzini)
> ---
[...]
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 8d01c9c..30f2af0 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -428,7 +429,6 @@ static const VMStateDescription vmstate_apic_common = {
> };
>
> static Property apic_properties_common[] = {
> - DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
> DEFINE_PROP_UINT8("version", APICCommonState, version, 0x14),
> DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
> true),
> @@ -437,6 +437,49 @@ static Property apic_properties_common[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +static void apic_common_get_id(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + APICCommonState *s = APIC_COMMON(obj);
> + int64_t value;
> +
> + value = s->apicbase & MSR_IA32_APICBASE_EXTD ? s->initial_apic_id : s->id;
> + visit_type_int(v, name, &value, errp);
> +}
Who exactly is going to read this property and require this logic
to be in the property getter?
Do we really need to expose this to the outside as a magic
property that changes depending on hardware state? Returning
initial_apic_id sounds much simpler.
> +
> +static void apic_common_set_id(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + APICCommonState *s = APIC_COMMON(obj);
> + DeviceState *dev = DEVICE(obj);
> + Error *local_err = NULL;
> + int64_t value;
> +
> + if (dev->realized) {
> + qdev_prop_set_after_realize(dev, name, errp);
> + return;
> + }
> +
> + visit_type_int(v, name, &value, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> + s->initial_apic_id = value;
> + s->id = (uint8_t)value;
Do we really need to change s->id here too? Won't it be set
automatically to initial_apic_id on reset?
I'm asking this because making it read/write only initial_apic_id
would make it easier to eventually convert the property to a
field-based getter/setter API (maybe even keep it using the
static property system).
Or, even better: do we really need a writeable property named
"id" at all? Is there any valid use case for the user to set it
directly? We could make the code that creates the APIC set
apic->initial_apic_id directly (or use a clearer
"initial-apic-id" property name).
> +}
> +
> +static void apic_common_initfn(Object *obj)
> +{
> + APICCommonState *s = APIC_COMMON(obj);
> +
> + s->id = s->initial_apic_id = -1;
> + object_property_add(obj, "id", "int",
> + apic_common_get_id,
> + apic_common_set_id, NULL, NULL, NULL);
If you are going to add new properties, please register them
using object_class_property_add*().
> +}
> +
> static void apic_common_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -456,6 +499,7 @@ static const TypeInfo apic_common_type = {
> .name = TYPE_APIC_COMMON,
> .parent = TYPE_DEVICE,
> .instance_size = sizeof(APICCommonState),
> + .instance_init = apic_common_initfn,
> .class_size = sizeof(APICCommonClass),
> .class_init = apic_common_class_init,
> .abstract = true,
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 13505ab..b4b4342 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2872,7 +2872,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
> OBJECT(cpu->apic_state), &error_abort);
> object_unref(OBJECT(cpu->apic_state));
>
> - qdev_prop_set_uint8(cpu->apic_state, "id", cpu->apic_id);
> + qdev_prop_set_uint32(cpu->apic_state, "id", cpu->apic_id);
> /* TODO: convert to link<> */
> apic = APIC_COMMON(cpu->apic_state);
> apic->cpu = cpu;
> --
> 2.7.4
>
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/13] pc: apic_common: extend APIC ID property to 32bit
2016-10-18 10:56 ` Eduardo Habkost
@ 2016-10-18 12:36 ` Igor Mammedov
2016-10-18 12:59 ` Eduardo Habkost
0 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 12:36 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 08:56:28 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Thu, Oct 13, 2016 at 11:52:40AM +0200, Igor Mammedov wrote:
> > ACPI ID is 32 bit wide on CPUs with x2APIC support.
> > Extend 'id' property to support it.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v3:
> > keep original behaviour where 'id' is readonly after
> > object is realized (pbonzini)
> > ---
> [...]
> > diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> > index 8d01c9c..30f2af0 100644
> > --- a/hw/intc/apic_common.c
> > +++ b/hw/intc/apic_common.c
> > @@ -428,7 +429,6 @@ static const VMStateDescription vmstate_apic_common = {
> > };
> >
> > static Property apic_properties_common[] = {
> > - DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
> > DEFINE_PROP_UINT8("version", APICCommonState, version, 0x14),
> > DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
> > true),
> > @@ -437,6 +437,49 @@ static Property apic_properties_common[] = {
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > +static void apic_common_get_id(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + APICCommonState *s = APIC_COMMON(obj);
> > + int64_t value;
> > +
> > + value = s->apicbase & MSR_IA32_APICBASE_EXTD ? s->initial_apic_id : s->id;
> > + visit_type_int(v, name, &value, errp);
> > +}
>
> Who exactly is going to read this property and require this logic
> to be in the property getter?
As it's set/read only from CPU we don't actually have to expose it
as property.
However, I've kept it as read/write property because it has already
been this way and been exposed to external users as some magic property.
Not sure is anyone cares.
> Do we really need to expose this to the outside as a magic
> property that changes depending on hardware state? Returning
> initial_apic_id sounds much simpler.
Well that's what it is now, so I've kept current behavior.
If we decide to change property behavior or drop it altogether
I can do it on top.
>
> > +
> > +static void apic_common_set_id(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + APICCommonState *s = APIC_COMMON(obj);
> > + DeviceState *dev = DEVICE(obj);
> > + Error *local_err = NULL;
> > + int64_t value;
> > +
> > + if (dev->realized) {
> > + qdev_prop_set_after_realize(dev, name, errp);
> > + return;
> > + }
> > +
> > + visit_type_int(v, name, &value, &local_err);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + return;
> > + }
> > +
> > + s->initial_apic_id = value;
> > + s->id = (uint8_t)value;
>
> Do we really need to change s->id here too? Won't it be set
> automatically to initial_apic_id on reset?
it will after following patch,
[PATCH v3 07/13] pc: apic_common: restore APIC ID to initial ID on reset
but I'd prefer to set it here as well to make consistent and clear
where it comes from.
> I'm asking this because making it read/write only initial_apic_id
> would make it easier to eventually convert the property to a
> field-based getter/setter API (maybe even keep it using the
> static property system).
If we don't care about keeping current behavior then I can make
it static property like it used to be.
> Or, even better: do we really need a writeable property named
> "id" at all? Is there any valid use case for the user to set it
> directly?
if user does it, it likely would make VM unsable,
I don't see a way for user to dot it though as he/she is going to
see APIC object only in realized state where it's forbidden to
change property.
> We could make the code that creates the APIC set
> apic->initial_apic_id directly (or use a clearer
> "initial-apic-id" property name).
So we probably fine with making it readonly (still not static
but a bit less code in this patch) and set apic_id directly
by calling new callback.
apic_common::set_apic_id(uint32_t new_apic_id){
this->id = this->initial_apic_id = new_apic_id;
}
> > +}
> > +
> > +static void apic_common_initfn(Object *obj)
> > +{
> > + APICCommonState *s = APIC_COMMON(obj);
> > +
> > + s->id = s->initial_apic_id = -1;
> > + object_property_add(obj, "id", "int",
> > + apic_common_get_id,
> > + apic_common_set_id, NULL, NULL, NULL);
>
> If you are going to add new properties, please register them
> using object_class_property_add*().
Sure, will fix.
>
> > +}
> > +
> > static void apic_common_class_init(ObjectClass *klass, void *data)
> > {
> > DeviceClass *dc = DEVICE_CLASS(klass);
> > @@ -456,6 +499,7 @@ static const TypeInfo apic_common_type = {
> > .name = TYPE_APIC_COMMON,
> > .parent = TYPE_DEVICE,
> > .instance_size = sizeof(APICCommonState),
> > + .instance_init = apic_common_initfn,
> > .class_size = sizeof(APICCommonClass),
> > .class_init = apic_common_class_init,
> > .abstract = true,
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 13505ab..b4b4342 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -2872,7 +2872,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
> > OBJECT(cpu->apic_state), &error_abort);
> > object_unref(OBJECT(cpu->apic_state));
> >
> > - qdev_prop_set_uint8(cpu->apic_state, "id", cpu->apic_id);
> > + qdev_prop_set_uint32(cpu->apic_state, "id", cpu->apic_id);
> > /* TODO: convert to link<> */
> > apic = APIC_COMMON(cpu->apic_state);
> > apic->cpu = cpu;
> > --
> > 2.7.4
> >
>
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/13] pc: apic_common: extend APIC ID property to 32bit
2016-10-18 12:36 ` Igor Mammedov
@ 2016-10-18 12:59 ` Eduardo Habkost
2016-10-18 14:01 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 12:59 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, Oct 18, 2016 at 02:36:10PM +0200, Igor Mammedov wrote:
> On Tue, 18 Oct 2016 08:56:28 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Thu, Oct 13, 2016 at 11:52:40AM +0200, Igor Mammedov wrote:
> > > ACPI ID is 32 bit wide on CPUs with x2APIC support.
> > > Extend 'id' property to support it.
> > >
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > > v3:
> > > keep original behaviour where 'id' is readonly after
> > > object is realized (pbonzini)
> > > ---
> > [...]
> > > diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> > > index 8d01c9c..30f2af0 100644
> > > --- a/hw/intc/apic_common.c
> > > +++ b/hw/intc/apic_common.c
> > > @@ -428,7 +429,6 @@ static const VMStateDescription vmstate_apic_common = {
> > > };
> > >
> > > static Property apic_properties_common[] = {
> > > - DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
> > > DEFINE_PROP_UINT8("version", APICCommonState, version, 0x14),
> > > DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
> > > true),
> > > @@ -437,6 +437,49 @@ static Property apic_properties_common[] = {
> > > DEFINE_PROP_END_OF_LIST(),
> > > };
> > >
> > > +static void apic_common_get_id(Object *obj, Visitor *v, const char *name,
> > > + void *opaque, Error **errp)
> > > +{
> > > + APICCommonState *s = APIC_COMMON(obj);
> > > + int64_t value;
> > > +
> > > + value = s->apicbase & MSR_IA32_APICBASE_EXTD ? s->initial_apic_id : s->id;
> > > + visit_type_int(v, name, &value, errp);
> > > +}
> >
> > Who exactly is going to read this property and require this logic
> > to be in the property getter?
> As it's set/read only from CPU we don't actually have to expose it
> as property.
> However, I've kept it as read/write property because it has already
> been this way and been exposed to external users as some magic property.
> Not sure is anyone cares.
>
>
> > Do we really need to expose this to the outside as a magic
> > property that changes depending on hardware state? Returning
> > initial_apic_id sounds much simpler.
> Well that's what it is now, so I've kept current behavior.
> If we decide to change property behavior or drop it altogether
> I can do it on top.
>
I agree to make them static properties as follow-up patch. This
way, if the change breaks anything we can revert only that patch.
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/13] pc: apic_common: extend APIC ID property to 32bit
2016-10-18 12:59 ` Eduardo Habkost
@ 2016-10-18 14:01 ` Igor Mammedov
2016-10-18 14:14 ` Eduardo Habkost
0 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 14:01 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 10:59:17 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, Oct 18, 2016 at 02:36:10PM +0200, Igor Mammedov wrote:
> > On Tue, 18 Oct 2016 08:56:28 -0200
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Thu, Oct 13, 2016 at 11:52:40AM +0200, Igor Mammedov wrote:
> > > > ACPI ID is 32 bit wide on CPUs with x2APIC support.
> > > > Extend 'id' property to support it.
> > > >
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > ---
> > > > v3:
> > > > keep original behaviour where 'id' is readonly after
> > > > object is realized (pbonzini)
> > > > ---
> > > [...]
> > > > diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> > > > index 8d01c9c..30f2af0 100644
> > > > --- a/hw/intc/apic_common.c
> > > > +++ b/hw/intc/apic_common.c
> > > > @@ -428,7 +429,6 @@ static const VMStateDescription vmstate_apic_common = {
> > > > };
> > > >
> > > > static Property apic_properties_common[] = {
> > > > - DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
> > > > DEFINE_PROP_UINT8("version", APICCommonState, version, 0x14),
> > > > DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
> > > > true),
> > > > @@ -437,6 +437,49 @@ static Property apic_properties_common[] = {
> > > > DEFINE_PROP_END_OF_LIST(),
> > > > };
> > > >
> > > > +static void apic_common_get_id(Object *obj, Visitor *v, const char *name,
> > > > + void *opaque, Error **errp)
> > > > +{
> > > > + APICCommonState *s = APIC_COMMON(obj);
> > > > + int64_t value;
> > > > +
> > > > + value = s->apicbase & MSR_IA32_APICBASE_EXTD ? s->initial_apic_id : s->id;
> > > > + visit_type_int(v, name, &value, errp);
> > > > +}
> > >
> > > Who exactly is going to read this property and require this logic
> > > to be in the property getter?
> > As it's set/read only from CPU we don't actually have to expose it
> > as property.
> > However, I've kept it as read/write property because it has already
> > been this way and been exposed to external users as some magic property.
> > Not sure is anyone cares.
> >
> >
> > > Do we really need to expose this to the outside as a magic
> > > property that changes depending on hardware state? Returning
> > > initial_apic_id sounds much simpler.
> > Well that's what it is now, so I've kept current behavior.
> > If we decide to change property behavior or drop it altogether
> > I can do it on top.
> >
>
> I agree to make them static properties as follow-up patch. This
> way, if the change breaks anything we can revert only that patch.
Static property won't work here as it should show APIC ID
depending on current CPU mode.
I could make it readonly property on respin,
and set apic.id/initial_apic_id directly from CPU.
Change would be
- apic_common_set_id()
+ apic_common::set_apic_id() callback
It won't get us less LOC, more likely it will take even more
code to do so.
As it's in this patch, it's at least consistent in a way
values are get/set. And effectively it's readonly due to check:
+ if (dev->realized) {
+ qdev_prop_set_after_realize(dev, name, errp);
+ return;
+ }
as external user can see only realized apic device.
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/13] pc: apic_common: extend APIC ID property to 32bit
2016-10-18 14:01 ` Igor Mammedov
@ 2016-10-18 14:14 ` Eduardo Habkost
2016-10-18 14:38 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 14:14 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, Oct 18, 2016 at 04:01:56PM +0200, Igor Mammedov wrote:
> On Tue, 18 Oct 2016 10:59:17 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Tue, Oct 18, 2016 at 02:36:10PM +0200, Igor Mammedov wrote:
> > > On Tue, 18 Oct 2016 08:56:28 -0200
> > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > >
> > > > On Thu, Oct 13, 2016 at 11:52:40AM +0200, Igor Mammedov wrote:
> > > > > ACPI ID is 32 bit wide on CPUs with x2APIC support.
> > > > > Extend 'id' property to support it.
> > > > >
> > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > > ---
> > > > > v3:
> > > > > keep original behaviour where 'id' is readonly after
> > > > > object is realized (pbonzini)
> > > > > ---
> > > > [...]
> > > > > diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> > > > > index 8d01c9c..30f2af0 100644
> > > > > --- a/hw/intc/apic_common.c
> > > > > +++ b/hw/intc/apic_common.c
> > > > > @@ -428,7 +429,6 @@ static const VMStateDescription vmstate_apic_common = {
> > > > > };
> > > > >
> > > > > static Property apic_properties_common[] = {
> > > > > - DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
> > > > > DEFINE_PROP_UINT8("version", APICCommonState, version, 0x14),
> > > > > DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
> > > > > true),
> > > > > @@ -437,6 +437,49 @@ static Property apic_properties_common[] = {
> > > > > DEFINE_PROP_END_OF_LIST(),
> > > > > };
> > > > >
> > > > > +static void apic_common_get_id(Object *obj, Visitor *v, const char *name,
> > > > > + void *opaque, Error **errp)
> > > > > +{
> > > > > + APICCommonState *s = APIC_COMMON(obj);
> > > > > + int64_t value;
> > > > > +
> > > > > + value = s->apicbase & MSR_IA32_APICBASE_EXTD ? s->initial_apic_id : s->id;
> > > > > + visit_type_int(v, name, &value, errp);
> > > > > +}
> > > >
> > > > Who exactly is going to read this property and require this logic
> > > > to be in the property getter?
> > > As it's set/read only from CPU we don't actually have to expose it
> > > as property.
> > > However, I've kept it as read/write property because it has already
> > > been this way and been exposed to external users as some magic property.
> > > Not sure is anyone cares.
> > >
> > >
> > > > Do we really need to expose this to the outside as a magic
> > > > property that changes depending on hardware state? Returning
> > > > initial_apic_id sounds much simpler.
> > > Well that's what it is now, so I've kept current behavior.
> > > If we decide to change property behavior or drop it altogether
> > > I can do it on top.
> > >
> >
> > I agree to make them static properties as follow-up patch. This
> > way, if the change breaks anything we can revert only that patch.
> Static property won't work here as it should show APIC ID
> depending on current CPU mode.
My suggestion is to _not_ show a different ID depending on the
current mode, to keep it simple. We can just have
"initial-apic-id" and "id" properties.
But, anyway, I didn't mean to suggest static properties
specifically. Where I say "static property" above, please read
"simple property that returns just a simple struct field and
don't need custom getter/setter code". That means either using a
static property (in case we still want it to be writeable, which
I doubt) or using object_property_add_uint*_ptr().
>
> I could make it readonly property on respin,
> and set apic.id/initial_apic_id directly from CPU.
> Change would be
> - apic_common_set_id()
> + apic_common::set_apic_id() callback
> It won't get us less LOC, more likely it will take even more
> code to do so.
> As it's in this patch, it's at least consistent in a way
> values are get/set. And effectively it's readonly due to check:
Don't worry about doing it on the respin. I'm OK with keeping the
current version by now, and change it in a follow-up patch.
>
> + if (dev->realized) {
> + qdev_prop_set_after_realize(dev, name, errp);
> + return;
> + }
>
> as external user can see only realized apic device.
That's true. But we could avoid all the extra getter/setter code
if we just use a static property or a read-only
object_property_add_uint*_ptr() property.
(All I say above are suggestions for a follow-up patch. I'm OK
with keeping the existing behavior like you do in this patch, to
keep this series simple.)
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 06/13] pc: apic_common: extend APIC ID property to 32bit
2016-10-18 14:14 ` Eduardo Habkost
@ 2016-10-18 14:38 ` Igor Mammedov
0 siblings, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 14:38 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 12:14:43 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, Oct 18, 2016 at 04:01:56PM +0200, Igor Mammedov wrote:
> > On Tue, 18 Oct 2016 10:59:17 -0200
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Tue, Oct 18, 2016 at 02:36:10PM +0200, Igor Mammedov wrote:
> > > > On Tue, 18 Oct 2016 08:56:28 -0200
> > > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > >
> > > > > On Thu, Oct 13, 2016 at 11:52:40AM +0200, Igor Mammedov wrote:
> > > > > > ACPI ID is 32 bit wide on CPUs with x2APIC support.
> > > > > > Extend 'id' property to support it.
> > > > > >
> > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > > > ---
> > > > > > v3:
> > > > > > keep original behaviour where 'id' is readonly after
> > > > > > object is realized (pbonzini)
> > > > > > ---
> > > > > [...]
> > > > > > diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> > > > > > index 8d01c9c..30f2af0 100644
> > > > > > --- a/hw/intc/apic_common.c
> > > > > > +++ b/hw/intc/apic_common.c
> > > > > > @@ -428,7 +429,6 @@ static const VMStateDescription vmstate_apic_common = {
> > > > > > };
> > > > > >
> > > > > > static Property apic_properties_common[] = {
> > > > > > - DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
> > > > > > DEFINE_PROP_UINT8("version", APICCommonState, version, 0x14),
> > > > > > DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
> > > > > > true),
> > > > > > @@ -437,6 +437,49 @@ static Property apic_properties_common[] = {
> > > > > > DEFINE_PROP_END_OF_LIST(),
> > > > > > };
> > > > > >
> > > > > > +static void apic_common_get_id(Object *obj, Visitor *v, const char *name,
> > > > > > + void *opaque, Error **errp)
> > > > > > +{
> > > > > > + APICCommonState *s = APIC_COMMON(obj);
> > > > > > + int64_t value;
> > > > > > +
> > > > > > + value = s->apicbase & MSR_IA32_APICBASE_EXTD ? s->initial_apic_id : s->id;
> > > > > > + visit_type_int(v, name, &value, errp);
> > > > > > +}
> > > > >
> > > > > Who exactly is going to read this property and require this logic
> > > > > to be in the property getter?
> > > > As it's set/read only from CPU we don't actually have to expose it
> > > > as property.
> > > > However, I've kept it as read/write property because it has already
> > > > been this way and been exposed to external users as some magic property.
> > > > Not sure is anyone cares.
> > > >
> > > >
> > > > > Do we really need to expose this to the outside as a magic
> > > > > property that changes depending on hardware state? Returning
> > > > > initial_apic_id sounds much simpler.
> > > > Well that's what it is now, so I've kept current behavior.
> > > > If we decide to change property behavior or drop it altogether
> > > > I can do it on top.
> > > >
> > >
> > > I agree to make them static properties as follow-up patch. This
> > > way, if the change breaks anything we can revert only that patch.
> > Static property won't work here as it should show APIC ID
> > depending on current CPU mode.
>
> My suggestion is to _not_ show a different ID depending on the
> current mode, to keep it simple. We can just have
> "initial-apic-id" and "id" properties.
>
> But, anyway, I didn't mean to suggest static properties
> specifically. Where I say "static property" above, please read
> "simple property that returns just a simple struct field and
> don't need custom getter/setter code". That means either using a
> static property (in case we still want it to be writeable, which
> I doubt) or using object_property_add_uint*_ptr().
>
> >
> > I could make it readonly property on respin,
> > and set apic.id/initial_apic_id directly from CPU.
> > Change would be
> > - apic_common_set_id()
> > + apic_common::set_apic_id() callback
> > It won't get us less LOC, more likely it will take even more
> > code to do so.
> > As it's in this patch, it's at least consistent in a way
> > values are get/set. And effectively it's readonly due to check:
>
> Don't worry about doing it on the respin. I'm OK with keeping the
> current version by now, and change it in a follow-up patch.
>
> >
> > + if (dev->realized) {
> > + qdev_prop_set_after_realize(dev, name, errp);
> > + return;
> > + }
> >
> > as external user can see only realized apic device.
>
> That's true. But we could avoid all the extra getter/setter code
> if we just use a static property or a read-only
> object_property_add_uint*_ptr() property.
>
> (All I say above are suggestions for a follow-up patch. I'm OK
> with keeping the existing behavior like you do in this patch, to
> keep this series simple.)
Ok, I'll respin this patch as is and think/try
the way you are suggesting in follow up.
^ permalink raw reply [flat|nested] 68+ messages in thread
* [Qemu-devel] [PATCH v3 07/13] pc: apic_common: restore APIC ID to initial ID on reset
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (5 preceding siblings ...)
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 06/13] pc: apic_common: extend APIC ID property to 32bit Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 08/13] pc: apic_common: reset APIC ID to initial ID when switching into x2APIC mode Igor Mammedov
` (8 subsequent siblings)
15 siblings, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
APIC ID should be restored to initial APIC ID
state after Reset and Power-On.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/intc/apic_common.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 30f2af0..ea3c8ca 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -243,6 +243,7 @@ static void apic_reset_common(DeviceState *dev)
bsp = s->apicbase & MSR_IA32_APICBASE_BSP;
s->apicbase = APIC_DEFAULT_ADDRESS | bsp | MSR_IA32_APICBASE_ENABLE;
+ s->id = s->initial_apic_id;
s->vapic_paddr = 0;
info->vapic_base_update(s);
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* [Qemu-devel] [PATCH v3 08/13] pc: apic_common: reset APIC ID to initial ID when switching into x2APIC mode
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (6 preceding siblings ...)
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 07/13] pc: apic_common: restore APIC ID to initial ID on reset Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-13 14:11 ` Radim Krčmář
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode Igor Mammedov
` (7 subsequent siblings)
15 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
SDM: x2APIC State Transitions:
State Changes From xAPIC Mode to x2APIC Mode
"
Any APIC ID value written to the memory-mapped
local APIC ID register is not preserved
"
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/intc/apic_common.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index ea3c8ca..d78c885 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -40,6 +40,11 @@ void cpu_set_apic_base(DeviceState *dev, uint64_t val)
if (dev) {
APICCommonState *s = APIC_COMMON(dev);
APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
+ /* switching to x2APIC, reset possibly modified xAPIC ID */
+ if (!(s->apicbase & MSR_IA32_APICBASE_EXTD) &&
+ (val & MSR_IA32_APICBASE_EXTD)) {
+ s->id = s->initial_apic_id;
+ }
info->set_base(s, val);
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 08/13] pc: apic_common: reset APIC ID to initial ID when switching into x2APIC mode
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 08/13] pc: apic_common: reset APIC ID to initial ID when switching into x2APIC mode Igor Mammedov
@ 2016-10-13 14:11 ` Radim Krčmář
0 siblings, 0 replies; 68+ messages in thread
From: Radim Krčmář @ 2016-10-13 14:11 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, ehabkost, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
2016-10-13 11:52+0200, Igor Mammedov:
> SDM: x2APIC State Transitions:
> State Changes From xAPIC Mode to x2APIC Mode
> "
> Any APIC ID value written to the memory-mapped
> local APIC ID register is not preserved
> "
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> hw/intc/apic_common.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index ea3c8ca..d78c885 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -40,6 +40,11 @@ void cpu_set_apic_base(DeviceState *dev, uint64_t val)
> if (dev) {
> APICCommonState *s = APIC_COMMON(dev);
> APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
> + /* switching to x2APIC, reset possibly modified xAPIC ID */
> + if (!(s->apicbase & MSR_IA32_APICBASE_EXTD) &&
> + (val & MSR_IA32_APICBASE_EXTD)) {
> + s->id = s->initial_apic_id;
> + }
> info->set_base(s, val);
> }
> }
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 68+ messages in thread
* [Qemu-devel] [PATCH v3 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (7 preceding siblings ...)
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 08/13] pc: apic_common: reset APIC ID to initial ID when switching into x2APIC mode Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-13 14:08 ` Radim Krčmář
2016-10-17 21:51 ` [Qemu-devel] [PATCH v3 " Eduardo Habkost
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 10/13] pc: clarify FW_CFG_MAX_CPUS usage comment Igor Mammedov
` (6 subsequent siblings)
15 siblings, 2 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v4:
- drop kvm_has_x2apic_api() and reuse kvm_enable_x2apic() instead
---
hw/i386/kvm/apic.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index be55102..9a7dd03 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -34,7 +34,11 @@ static void kvm_put_apic_state(APICCommonState *s, struct kvm_lapic_state *kapic
int i;
memset(kapic, 0, sizeof(*kapic));
- kvm_apic_set_reg(kapic, 0x2, s->id << 24);
+ if (kvm_enable_x2apic() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
+ kvm_apic_set_reg(kapic, 0x2, s->initial_apic_id);
+ } else {
+ kvm_apic_set_reg(kapic, 0x2, s->id << 24);
+ }
kvm_apic_set_reg(kapic, 0x8, s->tpr);
kvm_apic_set_reg(kapic, 0xd, s->log_dest << 24);
kvm_apic_set_reg(kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
@@ -59,7 +63,11 @@ void kvm_get_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
APICCommonState *s = APIC_COMMON(dev);
int i, v;
- s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
+ if (kvm_enable_x2apic() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
+ assert(kvm_apic_get_reg(kapic, 0x2) == s->initial_apic_id);
+ } else {
+ s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
+ }
s->tpr = kvm_apic_get_reg(kapic, 0x8);
s->arb_id = kvm_apic_get_reg(kapic, 0x9);
s->log_dest = kvm_apic_get_reg(kapic, 0xd) >> 24;
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode Igor Mammedov
@ 2016-10-13 14:08 ` Radim Krčmář
2016-10-14 11:21 ` [Qemu-devel] [PATCH v4 " Igor Mammedov
2016-10-17 21:51 ` [Qemu-devel] [PATCH v3 " Eduardo Habkost
1 sibling, 1 reply; 68+ messages in thread
From: Radim Krčmář @ 2016-10-13 14:08 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, ehabkost, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
2016-10-13 11:52+0200, Igor Mammedov:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v4:
> - drop kvm_has_x2apic_api() and reuse kvm_enable_x2apic() instead
> ---
Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> hw/i386/kvm/apic.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> index be55102..9a7dd03 100644
> --- a/hw/i386/kvm/apic.c
> +++ b/hw/i386/kvm/apic.c
> @@ -34,7 +34,11 @@ static void kvm_put_apic_state(APICCommonState *s, struct kvm_lapic_state *kapic
> int i;
>
> memset(kapic, 0, sizeof(*kapic));
> - kvm_apic_set_reg(kapic, 0x2, s->id << 24);
> + if (kvm_enable_x2apic() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
This will enable x2apic broadcast quirk even if a machine has no way of
using the standard x2APIC mode, so we'd change behavior if an OS
configured a device for broadcast.
I think Linux is the only user of the paravirtual x2APIC mode and it
doesn't configure devices for broadcast, so I think we're safe with it.
Thinking back, I could have just given it a non-controllable capability
in KVM ...
> + kvm_apic_set_reg(kapic, 0x2, s->initial_apic_id);
> + } else {
> + kvm_apic_set_reg(kapic, 0x2, s->id << 24);
> + }
> kvm_apic_set_reg(kapic, 0x8, s->tpr);
> kvm_apic_set_reg(kapic, 0xd, s->log_dest << 24);
> kvm_apic_set_reg(kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
> @@ -59,7 +63,11 @@ void kvm_get_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
> APICCommonState *s = APIC_COMMON(dev);
> int i, v;
>
> - s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
> + if (kvm_enable_x2apic() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
> + assert(kvm_apic_get_reg(kapic, 0x2) == s->initial_apic_id);
> + } else {
> + s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
> + }
> s->tpr = kvm_apic_get_reg(kapic, 0x8);
> s->arb_id = kvm_apic_get_reg(kapic, 0x9);
> s->log_dest = kvm_apic_get_reg(kapic, 0xd) >> 24;
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 68+ messages in thread* [Qemu-devel] [PATCH v4 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode
2016-10-13 14:08 ` Radim Krčmář
@ 2016-10-14 11:21 ` Igor Mammedov
2016-10-17 12:35 ` Radim Krčmář
2016-10-18 14:56 ` Eduardo Habkost
0 siblings, 2 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-14 11:21 UTC (permalink / raw)
To: qemu-devel
Cc: rkrcmar, ehabkost, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v4:
- restore kvm_has_x2apic_api() and use it to avoid side-effects
of kvm_enable_x2apic(). x2APIC API will be enabled by iommu
if it's present or not enabled at all.
v3:
- drop kvm_has_x2apic_api() and reuse kvm_enable_x2apic() instead
---
target-i386/kvm_i386.h | 1 +
hw/i386/kvm/apic.c | 12 ++++++++++--
target-i386/kvm.c | 13 ++++++++++---
3 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 5c369b1..7607929 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -44,4 +44,5 @@ int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
void kvm_put_apicbase(X86CPU *cpu, uint64_t value);
bool kvm_enable_x2apic(void);
+bool kvm_has_x2apic_api(void);
#endif
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index be55102..39b73e7 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -34,7 +34,11 @@ static void kvm_put_apic_state(APICCommonState *s, struct kvm_lapic_state *kapic
int i;
memset(kapic, 0, sizeof(*kapic));
- kvm_apic_set_reg(kapic, 0x2, s->id << 24);
+ if (kvm_has_x2apic_api() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
+ kvm_apic_set_reg(kapic, 0x2, s->initial_apic_id);
+ } else {
+ kvm_apic_set_reg(kapic, 0x2, s->id << 24);
+ }
kvm_apic_set_reg(kapic, 0x8, s->tpr);
kvm_apic_set_reg(kapic, 0xd, s->log_dest << 24);
kvm_apic_set_reg(kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
@@ -59,7 +63,11 @@ void kvm_get_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
APICCommonState *s = APIC_COMMON(dev);
int i, v;
- s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
+ if (kvm_has_x2apic_api() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
+ assert(kvm_apic_get_reg(kapic, 0x2) == s->initial_apic_id);
+ } else {
+ s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
+ }
s->tpr = kvm_apic_get_reg(kapic, 0x8);
s->arb_id = kvm_apic_get_reg(kapic, 0x9);
s->log_dest = kvm_apic_get_reg(kapic, 0xd) >> 24;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 0472f45..86b41a9 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -129,9 +129,8 @@ static bool kvm_x2apic_api_set_flags(uint64_t flags)
return !kvm_vm_enable_cap(s, KVM_CAP_X2APIC_API, 0, flags);
}
-#define MEMORIZE(fn) \
+#define MEMORIZE(fn, _result) \
({ \
- static typeof(fn) _result; \
static bool _memorized; \
\
if (_memorized) { \
@@ -141,11 +140,19 @@ static bool kvm_x2apic_api_set_flags(uint64_t flags)
_result = fn; \
})
+static bool has_x2apic_api;
+
+bool kvm_has_x2apic_api(void)
+{
+ return has_x2apic_api;
+}
+
bool kvm_enable_x2apic(void)
{
return MEMORIZE(
kvm_x2apic_api_set_flags(KVM_X2APIC_API_USE_32BIT_IDS |
- KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK));
+ KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK),
+ has_x2apic_api);
}
static int kvm_get_tsc(CPUState *cs)
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v4 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode
2016-10-14 11:21 ` [Qemu-devel] [PATCH v4 " Igor Mammedov
@ 2016-10-17 12:35 ` Radim Krčmář
2016-10-18 14:56 ` Eduardo Habkost
1 sibling, 0 replies; 68+ messages in thread
From: Radim Krčmář @ 2016-10-17 12:35 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, ehabkost, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
2016-10-14 13:21+0200, Igor Mammedov:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> v4:
> - restore kvm_has_x2apic_api() and use it to avoid side-effects
> of kvm_enable_x2apic(). x2APIC API will be enabled by iommu
> if it's present or not enabled at all.
> v3:
> - drop kvm_has_x2apic_api() and reuse kvm_enable_x2apic() instead
> ---
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [Qemu-devel] [PATCH v4 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode
2016-10-14 11:21 ` [Qemu-devel] [PATCH v4 " Igor Mammedov
2016-10-17 12:35 ` Radim Krčmář
@ 2016-10-18 14:56 ` Eduardo Habkost
2016-10-18 16:26 ` Radim Krčmář
1 sibling, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 14:56 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, rkrcmar, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
On Fri, Oct 14, 2016 at 01:21:55PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v4:
> - restore kvm_has_x2apic_api() and use it to avoid side-effects
> of kvm_enable_x2apic(). x2APIC API will be enabled by iommu
> if it's present or not enabled at all.
> v3:
> - drop kvm_has_x2apic_api() and reuse kvm_enable_x2apic() instead
> ---
> target-i386/kvm_i386.h | 1 +
> hw/i386/kvm/apic.c | 12 ++++++++++--
> target-i386/kvm.c | 13 ++++++++++---
> 3 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
> index 5c369b1..7607929 100644
> --- a/target-i386/kvm_i386.h
> +++ b/target-i386/kvm_i386.h
> @@ -44,4 +44,5 @@ int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
> void kvm_put_apicbase(X86CPU *cpu, uint64_t value);
>
> bool kvm_enable_x2apic(void);
> +bool kvm_has_x2apic_api(void);
> #endif
> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> index be55102..39b73e7 100644
> --- a/hw/i386/kvm/apic.c
> +++ b/hw/i386/kvm/apic.c
> @@ -34,7 +34,11 @@ static void kvm_put_apic_state(APICCommonState *s, struct kvm_lapic_state *kapic
> int i;
>
> memset(kapic, 0, sizeof(*kapic));
> - kvm_apic_set_reg(kapic, 0x2, s->id << 24);
> + if (kvm_has_x2apic_api() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
> + kvm_apic_set_reg(kapic, 0x2, s->initial_apic_id);
What happens if:
* x2apic is enabled on CPUID;
* guest sets MSR_IA32_APICBASE_EXTD; an
* the x2apic API is not enabled?
Does that mean kvm_{put,get}_apic_state() was already broken, or
is the x2apic ID translated to the old format by the kernel when
the x2apic API is disabled?
> + } else {
> + kvm_apic_set_reg(kapic, 0x2, s->id << 24);
> + }
> kvm_apic_set_reg(kapic, 0x8, s->tpr);
> kvm_apic_set_reg(kapic, 0xd, s->log_dest << 24);
> kvm_apic_set_reg(kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
> @@ -59,7 +63,11 @@ void kvm_get_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
> APICCommonState *s = APIC_COMMON(dev);
> int i, v;
>
> - s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
> + if (kvm_has_x2apic_api() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
> + assert(kvm_apic_get_reg(kapic, 0x2) == s->initial_apic_id);
> + } else {
> + s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
> + }
> s->tpr = kvm_apic_get_reg(kapic, 0x8);
> s->arb_id = kvm_apic_get_reg(kapic, 0x9);
> s->log_dest = kvm_apic_get_reg(kapic, 0xd) >> 24;
[...]
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v4 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode
2016-10-18 14:56 ` Eduardo Habkost
@ 2016-10-18 16:26 ` Radim Krčmář
2016-10-18 18:04 ` Eduardo Habkost
0 siblings, 1 reply; 68+ messages in thread
From: Radim Krčmář @ 2016-10-18 16:26 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Igor Mammedov, qemu-devel, liuxiaojian6, mst, peterx, kevin,
kraxel, pbonzini, lersek, chao.gao
2016-10-18 12:56-0200, Eduardo Habkost:
> On Fri, Oct 14, 2016 at 01:21:55PM +0200, Igor Mammedov wrote:
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>> v4:
>> - restore kvm_has_x2apic_api() and use it to avoid side-effects
>> of kvm_enable_x2apic(). x2APIC API will be enabled by iommu
>> if it's present or not enabled at all.
>> v3:
>> - drop kvm_has_x2apic_api() and reuse kvm_enable_x2apic() instead
>> ---
>> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
>> @@ -34,7 +34,11 @@ static void kvm_put_apic_state(APICCommonState *s, struct kvm_lapic_state *kapic
>> int i;
>>
>> memset(kapic, 0, sizeof(*kapic));
>> - kvm_apic_set_reg(kapic, 0x2, s->id << 24);
>> + if (kvm_has_x2apic_api() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
>> + kvm_apic_set_reg(kapic, 0x2, s->initial_apic_id);
>
> What happens if:
>
> * x2apic is enabled on CPUID;
> * guest sets MSR_IA32_APICBASE_EXTD; an
> * the x2apic API is not enabled?
KVM expects APIC ID to be in upper 8 bits of the register then.
Guest APIC mode does not come into play if the x2APIC API is not
enabled. This is to keep compatibility with old KVMs that used xAPIC
format regardless of APIC mode.
> Does that mean kvm_{put,get}_apic_state() was already broken, or
> is the x2apic ID translated to the old format by the kernel when
> the x2apic API is disabled?
The latter. KVM stores the 8 bits in an appropriate format, but it
doesn't really matter to QEMU: the exchange format without enabled
x2APIC API is defined to be the xAPIC one. (KVM used to keep always
keep ID in xAPIC format and trapped x2APIC ID reads to shift the value.)
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v4 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode
2016-10-18 16:26 ` Radim Krčmář
@ 2016-10-18 18:04 ` Eduardo Habkost
0 siblings, 0 replies; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 18:04 UTC (permalink / raw)
To: Radim Krčmář
Cc: Igor Mammedov, qemu-devel, liuxiaojian6, mst, peterx, kevin,
kraxel, pbonzini, lersek, chao.gao
On Tue, Oct 18, 2016 at 06:26:54PM +0200, Radim Krčmář wrote:
> 2016-10-18 12:56-0200, Eduardo Habkost:
> > On Fri, Oct 14, 2016 at 01:21:55PM +0200, Igor Mammedov wrote:
> >> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >> ---
> >> v4:
> >> - restore kvm_has_x2apic_api() and use it to avoid side-effects
> >> of kvm_enable_x2apic(). x2APIC API will be enabled by iommu
> >> if it's present or not enabled at all.
> >> v3:
> >> - drop kvm_has_x2apic_api() and reuse kvm_enable_x2apic() instead
> >> ---
> >> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> >> @@ -34,7 +34,11 @@ static void kvm_put_apic_state(APICCommonState *s, struct kvm_lapic_state *kapic
> >> int i;
> >>
> >> memset(kapic, 0, sizeof(*kapic));
> >> - kvm_apic_set_reg(kapic, 0x2, s->id << 24);
> >> + if (kvm_has_x2apic_api() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
> >> + kvm_apic_set_reg(kapic, 0x2, s->initial_apic_id);
> >
> > What happens if:
> >
> > * x2apic is enabled on CPUID;
> > * guest sets MSR_IA32_APICBASE_EXTD; an
> > * the x2apic API is not enabled?
>
> KVM expects APIC ID to be in upper 8 bits of the register then.
> Guest APIC mode does not come into play if the x2APIC API is not
> enabled. This is to keep compatibility with old KVMs that used xAPIC
> format regardless of APIC mode.
>
> > Does that mean kvm_{put,get}_apic_state() was already broken, or
> > is the x2apic ID translated to the old format by the kernel when
> > the x2apic API is disabled?
>
> The latter. KVM stores the 8 bits in an appropriate format, but it
> doesn't really matter to QEMU: the exchange format without enabled
> x2APIC API is defined to be the xAPIC one. (KVM used to keep always
> keep ID in xAPIC format and trapped x2APIC ID reads to shift the value.)
Thanks for the clarification!
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [Qemu-devel] [PATCH v3 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode Igor Mammedov
2016-10-13 14:08 ` Radim Krčmář
@ 2016-10-17 21:51 ` Eduardo Habkost
2016-10-18 7:17 ` Igor Mammedov
1 sibling, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-17 21:51 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Thu, Oct 13, 2016 at 11:52:43AM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v4:
> - drop kvm_has_x2apic_api() and reuse kvm_enable_x2apic() instead
> ---
> hw/i386/kvm/apic.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> index be55102..9a7dd03 100644
> --- a/hw/i386/kvm/apic.c
> +++ b/hw/i386/kvm/apic.c
> @@ -34,7 +34,11 @@ static void kvm_put_apic_state(APICCommonState *s, struct kvm_lapic_state *kapic
> int i;
>
> memset(kapic, 0, sizeof(*kapic));
> - kvm_apic_set_reg(kapic, 0x2, s->id << 24);
> + if (kvm_enable_x2apic() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
This is going to enable x2apic unconditionally (not just check if
x2apic was enabled). Is this really what you want to do?
> + kvm_apic_set_reg(kapic, 0x2, s->initial_apic_id);
> + } else {
> + kvm_apic_set_reg(kapic, 0x2, s->id << 24);
> + }
> kvm_apic_set_reg(kapic, 0x8, s->tpr);
> kvm_apic_set_reg(kapic, 0xd, s->log_dest << 24);
> kvm_apic_set_reg(kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
> @@ -59,7 +63,11 @@ void kvm_get_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
> APICCommonState *s = APIC_COMMON(dev);
> int i, v;
>
> - s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
> + if (kvm_enable_x2apic() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
> + assert(kvm_apic_get_reg(kapic, 0x2) == s->initial_apic_id);
> + } else {
> + s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
> + }
> s->tpr = kvm_apic_get_reg(kapic, 0x8);
> s->arb_id = kvm_apic_get_reg(kapic, 0x9);
> s->log_dest = kvm_apic_get_reg(kapic, 0xd) >> 24;
> --
> 2.7.4
>
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode
2016-10-17 21:51 ` [Qemu-devel] [PATCH v3 " Eduardo Habkost
@ 2016-10-18 7:17 ` Igor Mammedov
2016-10-18 10:40 ` Eduardo Habkost
0 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 7:17 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Mon, 17 Oct 2016 19:51:12 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Thu, Oct 13, 2016 at 11:52:43AM +0200, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v4:
> > - drop kvm_has_x2apic_api() and reuse kvm_enable_x2apic() instead
> > ---
> > hw/i386/kvm/apic.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> > index be55102..9a7dd03 100644
> > --- a/hw/i386/kvm/apic.c
> > +++ b/hw/i386/kvm/apic.c
> > @@ -34,7 +34,11 @@ static void kvm_put_apic_state(APICCommonState *s, struct kvm_lapic_state *kapic
> > int i;
> >
> > memset(kapic, 0, sizeof(*kapic));
> > - kvm_apic_set_reg(kapic, 0x2, s->id << 24);
> > + if (kvm_enable_x2apic() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
>
> This is going to enable x2apic unconditionally (not just check if
> x2apic was enabled). Is this really what you want to do?
Fixed,
v4 is in reply to the same note from Radim
>
>
> > + kvm_apic_set_reg(kapic, 0x2, s->initial_apic_id);
> > + } else {
> > + kvm_apic_set_reg(kapic, 0x2, s->id << 24);
> > + }
> > kvm_apic_set_reg(kapic, 0x8, s->tpr);
> > kvm_apic_set_reg(kapic, 0xd, s->log_dest << 24);
> > kvm_apic_set_reg(kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
> > @@ -59,7 +63,11 @@ void kvm_get_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
> > APICCommonState *s = APIC_COMMON(dev);
> > int i, v;
> >
> > - s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
> > + if (kvm_enable_x2apic() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
> > + assert(kvm_apic_get_reg(kapic, 0x2) == s->initial_apic_id);
> > + } else {
> > + s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
> > + }
> > s->tpr = kvm_apic_get_reg(kapic, 0x8);
> > s->arb_id = kvm_apic_get_reg(kapic, 0x9);
> > s->log_dest = kvm_apic_get_reg(kapic, 0xd) >> 24;
> > --
> > 2.7.4
> >
>
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode
2016-10-18 7:17 ` Igor Mammedov
@ 2016-10-18 10:40 ` Eduardo Habkost
0 siblings, 0 replies; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 10:40 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, kraxel, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
On Tue, Oct 18, 2016 at 09:17:48AM +0200, Igor Mammedov wrote:
> On Mon, 17 Oct 2016 19:51:12 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Thu, Oct 13, 2016 at 11:52:43AM +0200, Igor Mammedov wrote:
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > > v4:
> > > - drop kvm_has_x2apic_api() and reuse kvm_enable_x2apic() instead
> > > ---
> > > hw/i386/kvm/apic.c | 12 ++++++++++--
> > > 1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> > > index be55102..9a7dd03 100644
> > > --- a/hw/i386/kvm/apic.c
> > > +++ b/hw/i386/kvm/apic.c
> > > @@ -34,7 +34,11 @@ static void kvm_put_apic_state(APICCommonState *s, struct kvm_lapic_state *kapic
> > > int i;
> > >
> > > memset(kapic, 0, sizeof(*kapic));
> > > - kvm_apic_set_reg(kapic, 0x2, s->id << 24);
> > > + if (kvm_enable_x2apic() && s->apicbase & MSR_IA32_APICBASE_EXTD) {
> >
> > This is going to enable x2apic unconditionally (not just check if
> > x2apic was enabled). Is this really what you want to do?
> Fixed,
> v4 is in reply to the same note from Radim
Oops, didn't notice v4 was already there. Sorry for the noise.
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread
* [Qemu-devel] [PATCH v3 10/13] pc: clarify FW_CFG_MAX_CPUS usage comment
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (8 preceding siblings ...)
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 09/13] pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 11/13] increase MAX_CPUMASK_BITS from 255 to 288 Igor Mammedov
` (5 subsequent siblings)
15 siblings, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/pc.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f1c1013..f807932 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -746,17 +746,15 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
/* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
*
- * SeaBIOS needs FW_CFG_MAX_CPUS for CPU hotplug, but the CPU hotplug
- * QEMU<->SeaBIOS interface is not based on the "CPU index", but on the APIC
- * ID of hotplugged CPUs[1]. This means that FW_CFG_MAX_CPUS is not the
- * "maximum number of CPUs", but the "limit to the APIC ID values SeaBIOS
- * may see".
+ * For machine types prior to 1.8, SeaBIOS needs FW_CFG_MAX_CPUS for
+ * building MPTable, ACPI MADT, ACPI CPU hotplug and ACPI SRAT table,
+ * that tables are based on xAPIC ID and QEMU<->SeaBIOS interface
+ * for CPU hotplug also uses APIC ID and not "CPU index".
+ * This means that FW_CFG_MAX_CPUS is not the "maximum number of CPUs",
+ * but the "limit to the APIC ID values SeaBIOS may see".
*
- * So, this means we must not use max_cpus, here, but the maximum possible
- * APIC ID value, plus one.
- *
- * [1] The only kind of "CPU identifier" used between SeaBIOS and QEMU is
- * the APIC ID, not the "CPU index"
+ * So for compatibility reasons with old BIOSes we are stuck with
+ * "etc/max-cpus" actually being apic_id_limit
*/
fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit);
fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* [Qemu-devel] [PATCH v3 11/13] increase MAX_CPUMASK_BITS from 255 to 288
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (9 preceding siblings ...)
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 10/13] pc: clarify FW_CFG_MAX_CPUS usage comment Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-13 13:01 ` Andrew Jones
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 12/13] pc: add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs Igor Mammedov
` (4 subsequent siblings)
15 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao, david, peter.maydell, drjones
so that it would be possible to increase maxcpus limit
for x86 target. Keep spapr/virt_arm at limit they used
to have 255.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
CC: david@gibson.dropbear.id.au
CC: peter.maydell@linaro.org
CC: drjones@redhat.com
include/sysemu/sysemu.h | 2 +-
hw/arm/virt.c | 2 +-
hw/ppc/spapr.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b668833..66c6f15 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -173,7 +173,7 @@ extern int mem_prealloc;
*
* Note that cpu->get_arch_id() may be larger than MAX_CPUMASK_BITS.
*/
-#define MAX_CPUMASK_BITS 255
+#define MAX_CPUMASK_BITS 288
#define MAX_OPTION_ROMS 16
typedef struct QEMUOptionRom {
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 795740d..186b5bd 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1491,7 +1491,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
* it later in machvirt_init, where we have more information about the
* configuration of the particular instance.
*/
- mc->max_cpus = MAX_CPUMASK_BITS;
+ mc->max_cpus = 255;
mc->has_dynamic_sysbus = true;
mc->block_default_type = IF_VIRTIO;
mc->no_cdrom = 1;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 03e3803..fb3b498 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2388,7 +2388,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
mc->init = ppc_spapr_init;
mc->reset = ppc_spapr_reset;
mc->block_default_type = IF_SCSI;
- mc->max_cpus = MAX_CPUMASK_BITS;
+ mc->max_cpus = 255;
mc->no_parallel = 1;
mc->default_boot_order = "";
mc->default_ram_size = 512 * M_BYTE;
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 11/13] increase MAX_CPUMASK_BITS from 255 to 288
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 11/13] increase MAX_CPUMASK_BITS from 255 to 288 Igor Mammedov
@ 2016-10-13 13:01 ` Andrew Jones
0 siblings, 0 replies; 68+ messages in thread
From: Andrew Jones @ 2016-10-13 13:01 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, peter.maydell, ehabkost, liuxiaojian6, mst, rkrcmar,
peterx, kevin, kraxel, pbonzini, david, lersek, chao.gao
On Thu, Oct 13, 2016 at 11:52:45AM +0200, Igor Mammedov wrote:
> so that it would be possible to increase maxcpus limit
> for x86 target. Keep spapr/virt_arm at limit they used
> to have 255.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> CC: david@gibson.dropbear.id.au
> CC: peter.maydell@linaro.org
> CC: drjones@redhat.com
>
> include/sysemu/sysemu.h | 2 +-
> hw/arm/virt.c | 2 +-
> hw/ppc/spapr.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index b668833..66c6f15 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -173,7 +173,7 @@ extern int mem_prealloc;
> *
> * Note that cpu->get_arch_id() may be larger than MAX_CPUMASK_BITS.
> */
> -#define MAX_CPUMASK_BITS 255
> +#define MAX_CPUMASK_BITS 288
>
> #define MAX_OPTION_ROMS 16
> typedef struct QEMUOptionRom {
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 795740d..186b5bd 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1491,7 +1491,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
> * it later in machvirt_init, where we have more information about the
> * configuration of the particular instance.
> */
> - mc->max_cpus = MAX_CPUMASK_BITS;
> + mc->max_cpus = 255;
> mc->has_dynamic_sysbus = true;
> mc->block_default_type = IF_VIRTIO;
> mc->no_cdrom = 1;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 03e3803..fb3b498 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2388,7 +2388,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> mc->init = ppc_spapr_init;
> mc->reset = ppc_spapr_reset;
> mc->block_default_type = IF_SCSI;
> - mc->max_cpus = MAX_CPUMASK_BITS;
> + mc->max_cpus = 255;
> mc->no_parallel = 1;
> mc->default_boot_order = "";
> mc->default_ram_size = 512 * M_BYTE;
> --
> 2.7.4
>
>
Reviewed-by: Andrew Jones <drjones@redhat.com>
^ permalink raw reply [flat|nested] 68+ messages in thread
* [Qemu-devel] [PATCH v3 12/13] pc: add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (10 preceding siblings ...)
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 11/13] increase MAX_CPUMASK_BITS from 255 to 288 Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 13/13] pc: require IRQ remapping and EIM if there could be x2APIC CPUs Igor Mammedov
` (3 subsequent siblings)
15 siblings, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
Currently firmware uses 1 byte at 0x5F offset in RTC CMOS
to get number of CPUs present at boot. However 1 byte is
not enough to handle more than 255 CPUs. So add a new
fw_cfg file that would allow QEMU to tell it.
For compat reasons add file only for machine types that
support more than 255 CPUs.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
* make boot_cpus to PCMachineState so it could be kept uptodate
when a cpu is added/removed and BIOS would read current value
on system reset
---
include/hw/i386/pc.h | 2 ++
hw/i386/pc.c | 36 +++++++++++++++++++++---------------
2 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index b16c448..17fff80 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -37,6 +37,7 @@
/**
* PCMachineState:
* @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
+ * @boot_cpus_le: number of present VCPUs, referenced by 'etc/boot-cpus' fw_cfg
*/
struct PCMachineState {
/*< private >*/
@@ -69,6 +70,7 @@ struct PCMachineState {
bool apic_xrupt_override;
unsigned apic_id_limit;
CPUArchIdList *possible_cpus;
+ uint16_t boot_cpus_le;
/* NUMA information: */
uint64_t numa_nodes;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f807932..40eb43b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1084,17 +1084,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
}
}
-static int pc_present_cpus_count(PCMachineState *pcms)
-{
- int i, boot_cpus = 0;
- for (i = 0; i < pcms->possible_cpus->len; i++) {
- if (pcms->possible_cpus->cpus[i].cpu) {
- boot_cpus++;
- }
- }
- return boot_cpus;
-}
-
static X86CPU *pc_new_cpu(const char *typename, int64_t apic_id,
Error **errp)
{
@@ -1231,6 +1220,11 @@ static void pc_build_feature_control_file(PCMachineState *pcms)
fw_cfg_add_file(pcms->fw_cfg, "etc/msr_feature_control", val, sizeof(*val));
}
+static void rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count)
+{
+ rtc_set_memory(rtc, 0x5f, cpus_count - 1);
+}
+
static
void pc_machine_done(Notifier *notifier, void *data)
{
@@ -1239,7 +1233,7 @@ void pc_machine_done(Notifier *notifier, void *data)
PCIBus *bus = pcms->bus;
/* set the number of CPUs */
- rtc_set_memory(pcms->rtc, 0x5f, pc_present_cpus_count(pcms) - 1);
+ rtc_set_cpus_count(pcms->rtc, le16_to_cpu(pcms->boot_cpus_le));
if (bus) {
int extra_hosts = 0;
@@ -1260,8 +1254,15 @@ void pc_machine_done(Notifier *notifier, void *data)
acpi_setup();
if (pcms->fw_cfg) {
+ MachineClass *mc = MACHINE_GET_CLASS(pcms);
+
pc_build_smbios(pcms->fw_cfg);
pc_build_feature_control_file(pcms);
+
+ if (mc->max_cpus > 255) {
+ fw_cfg_add_file(pcms->fw_cfg, "etc/boot-cpus", &pcms->boot_cpus_le,
+ sizeof(pcms->boot_cpus_le));
+ }
}
}
@@ -1785,9 +1786,11 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
}
}
+ /* increment the number of CPUs */
+ pcms->boot_cpus_le = cpu_to_le16(le16_to_cpu(pcms->boot_cpus_le) + 1);
if (dev->hotplugged) {
- /* increment the number of CPUs */
- rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1);
+ /* Update the number of CPUs in CMOS */
+ rtc_set_cpus_count(pcms->rtc, le16_to_cpu(pcms->boot_cpus_le));
}
found_cpu = pc_find_cpu_slot(pcms, CPU(dev), NULL);
@@ -1841,7 +1844,10 @@ static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev,
found_cpu->cpu = NULL;
object_unparent(OBJECT(dev));
- rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) - 1);
+ /* decrement the number of CPUs */
+ pcms->boot_cpus_le = cpu_to_le16(le16_to_cpu(pcms->boot_cpus_le) - 1);
+ /* Update the number of CPUs in CMOS */
+ rtc_set_cpus_count(pcms->rtc, le16_to_cpu(pcms->boot_cpus_le));
out:
error_propagate(errp, local_err);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* [Qemu-devel] [PATCH v3 13/13] pc: require IRQ remapping and EIM if there could be x2APIC CPUs
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (11 preceding siblings ...)
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 12/13] pc: add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs Igor Mammedov
@ 2016-10-13 9:52 ` Igor Mammedov
2016-10-13 13:56 ` Radim Krčmář
2016-10-13 10:01 ` [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Paolo Bonzini
` (2 subsequent siblings)
15 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 9:52 UTC (permalink / raw)
To: qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
pbonzini, lersek, chao.gao
it would prevent starting guest with incorrect configs
where interrupts couldn't be delivered to CPUs with
APIC IDs > 254.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/pc.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 40eb43b..f7070e0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -68,6 +68,7 @@
#include "qapi-visit.h"
#include "qom/cpu.h"
#include "hw/nmi.h"
+#include "hw/i386/intel_iommu.h"
/* debug PC/ISA interrupts */
//#define DEBUG_IRQ
@@ -1264,6 +1265,18 @@ void pc_machine_done(Notifier *notifier, void *data)
sizeof(pcms->boot_cpus_le));
}
}
+
+ if (pcms->apic_id_limit > 255) {
+ IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());
+
+ if (!iommu || !iommu->x86_iommu.intr_supported ||
+ iommu->intr_eim != ON_OFF_AUTO_ON) {
+ error_report("current -smp configuration requires "
+ "Extended Interrupt Mode enabled. "
+ "IOMMU should have eim=on option set");
+ exit(EXIT_FAILURE);
+ }
+ }
}
void pc_guest_info_init(PCMachineState *pcms)
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 13/13] pc: require IRQ remapping and EIM if there could be x2APIC CPUs
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 13/13] pc: require IRQ remapping and EIM if there could be x2APIC CPUs Igor Mammedov
@ 2016-10-13 13:56 ` Radim Krčmář
2016-10-14 11:25 ` [Qemu-devel] [PATCH v4 " Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Radim Krčmář @ 2016-10-13 13:56 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, ehabkost, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
2016-10-13 11:52+0200, Igor Mammedov:
> it would prevent starting guest with incorrect configs
> where interrupts couldn't be delivered to CPUs with
> APIC IDs > 254.
off-by-one.
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> hw/i386/pc.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 40eb43b..f7070e0 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -68,6 +68,7 @@
> #include "qapi-visit.h"
> #include "qom/cpu.h"
> #include "hw/nmi.h"
> +#include "hw/i386/intel_iommu.h"
>
> /* debug PC/ISA interrupts */
> //#define DEBUG_IRQ
> @@ -1264,6 +1265,18 @@ void pc_machine_done(Notifier *notifier, void *data)
> sizeof(pcms->boot_cpus_le));
> }
> }
> +
> + if (pcms->apic_id_limit > 255) {
> + IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());
> +
> + if (!iommu || !iommu->x86_iommu.intr_supported ||
> + iommu->intr_eim != ON_OFF_AUTO_ON) {
> + error_report("current -smp configuration requires "
> + "Extended Interrupt Mode enabled. "
> + "IOMMU should have eim=on option set");
> + exit(EXIT_FAILURE);
> + }
> + }
> }
>
> void pc_guest_info_init(PCMachineState *pcms)
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 68+ messages in thread* [Qemu-devel] [PATCH v4 13/13] pc: require IRQ remapping and EIM if there could be x2APIC CPUs
2016-10-13 13:56 ` Radim Krčmář
@ 2016-10-14 11:25 ` Igor Mammedov
2016-10-18 11:27 ` Eduardo Habkost
0 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-14 11:25 UTC (permalink / raw)
To: qemu-devel
Cc: rkrcmar, ehabkost, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
it would prevent starting guest with incorrect configs
where interrupts couldn't be delivered to CPUs with
APIC IDs > 255.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
---
v4:
- s/254/255/ in commit message (Radim)
---
hw/i386/pc.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 40eb43b..f7070e0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -68,6 +68,7 @@
#include "qapi-visit.h"
#include "qom/cpu.h"
#include "hw/nmi.h"
+#include "hw/i386/intel_iommu.h"
/* debug PC/ISA interrupts */
//#define DEBUG_IRQ
@@ -1264,6 +1265,18 @@ void pc_machine_done(Notifier *notifier, void *data)
sizeof(pcms->boot_cpus_le));
}
}
+
+ if (pcms->apic_id_limit > 255) {
+ IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());
+
+ if (!iommu || !iommu->x86_iommu.intr_supported ||
+ iommu->intr_eim != ON_OFF_AUTO_ON) {
+ error_report("current -smp configuration requires "
+ "Extended Interrupt Mode enabled. "
+ "IOMMU should have eim=on option set");
+ exit(EXIT_FAILURE);
+ }
+ }
}
void pc_guest_info_init(PCMachineState *pcms)
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v4 13/13] pc: require IRQ remapping and EIM if there could be x2APIC CPUs
2016-10-14 11:25 ` [Qemu-devel] [PATCH v4 " Igor Mammedov
@ 2016-10-18 11:27 ` Eduardo Habkost
2016-10-18 12:44 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 11:27 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, rkrcmar, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
On Fri, Oct 14, 2016 at 01:25:35PM +0200, Igor Mammedov wrote:
> it would prevent starting guest with incorrect configs
> where interrupts couldn't be delivered to CPUs with
> APIC IDs > 255.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
> v4:
> - s/254/255/ in commit message (Radim)
> ---
> hw/i386/pc.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 40eb43b..f7070e0 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -68,6 +68,7 @@
> #include "qapi-visit.h"
> #include "qom/cpu.h"
> #include "hw/nmi.h"
> +#include "hw/i386/intel_iommu.h"
>
> /* debug PC/ISA interrupts */
> //#define DEBUG_IRQ
> @@ -1264,6 +1265,18 @@ void pc_machine_done(Notifier *notifier, void *data)
> sizeof(pcms->boot_cpus_le));
> }
> }
> +
> + if (pcms->apic_id_limit > 255) {
> + IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());
> +
> + if (!iommu || !iommu->x86_iommu.intr_supported ||
> + iommu->intr_eim != ON_OFF_AUTO_ON) {
> + error_report("current -smp configuration requires "
> + "Extended Interrupt Mode enabled. "
> + "IOMMU should have eim=on option set");
Suggestion for a follow-up patch:
* Error message explaining how to set eim=on if the iommu is
available
* Error message explaining how to make sure the iommu is created,
in case it was not even created.
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v4 13/13] pc: require IRQ remapping and EIM if there could be x2APIC CPUs
2016-10-18 11:27 ` Eduardo Habkost
@ 2016-10-18 12:44 ` Igor Mammedov
2016-10-18 12:55 ` Eduardo Habkost
0 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 12:44 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, rkrcmar, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 09:27:37 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Fri, Oct 14, 2016 at 01:25:35PM +0200, Igor Mammedov wrote:
> > it would prevent starting guest with incorrect configs
> > where interrupts couldn't be delivered to CPUs with
> > APIC IDs > 255.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> > ---
> > v4:
> > - s/254/255/ in commit message (Radim)
> > ---
> > hw/i386/pc.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 40eb43b..f7070e0 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -68,6 +68,7 @@
> > #include "qapi-visit.h"
> > #include "qom/cpu.h"
> > #include "hw/nmi.h"
> > +#include "hw/i386/intel_iommu.h"
> >
> > /* debug PC/ISA interrupts */
> > //#define DEBUG_IRQ
> > @@ -1264,6 +1265,18 @@ void pc_machine_done(Notifier *notifier, void *data)
> > sizeof(pcms->boot_cpus_le));
> > }
> > }
> > +
> > + if (pcms->apic_id_limit > 255) {
> > + IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());
> > +
> > + if (!iommu || !iommu->x86_iommu.intr_supported ||
> > + iommu->intr_eim != ON_OFF_AUTO_ON) {
> > + error_report("current -smp configuration requires "
> > + "Extended Interrupt Mode enabled. "
> > + "IOMMU should have eim=on option set");
>
> Suggestion for a follow-up patch:
>
> * Error message explaining how to set eim=on if the iommu is
> available
> * Error message explaining how to make sure the iommu is created,
> in case it was not even created.
Reason I didn't include how to create iommu/CLI example is that
it could be some other iommu in future so that message could
bit rot over time.
But I can add description if you'd prefer it.
How about something like this:
+ error_report("current -smp configuration requires "
+ "Intel IOMMU with Extended Interrupt Mode enabled. "
+ "To enable IOMMU add to command line: "
+ "-device intel-iommu,intremap=on,eim=on");
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v4 13/13] pc: require IRQ remapping and EIM if there could be x2APIC CPUs
2016-10-18 12:44 ` Igor Mammedov
@ 2016-10-18 12:55 ` Eduardo Habkost
2016-10-18 14:39 ` Igor Mammedov
0 siblings, 1 reply; 68+ messages in thread
From: Eduardo Habkost @ 2016-10-18 12:55 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, rkrcmar, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
On Tue, Oct 18, 2016 at 02:44:55PM +0200, Igor Mammedov wrote:
> On Tue, 18 Oct 2016 09:27:37 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Fri, Oct 14, 2016 at 01:25:35PM +0200, Igor Mammedov wrote:
> > > it would prevent starting guest with incorrect configs
> > > where interrupts couldn't be delivered to CPUs with
> > > APIC IDs > 255.
> > >
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> > > ---
> > > v4:
> > > - s/254/255/ in commit message (Radim)
> > > ---
> > > hw/i386/pc.c | 13 +++++++++++++
> > > 1 file changed, 13 insertions(+)
> > >
> > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > index 40eb43b..f7070e0 100644
> > > --- a/hw/i386/pc.c
> > > +++ b/hw/i386/pc.c
> > > @@ -68,6 +68,7 @@
> > > #include "qapi-visit.h"
> > > #include "qom/cpu.h"
> > > #include "hw/nmi.h"
> > > +#include "hw/i386/intel_iommu.h"
> > >
> > > /* debug PC/ISA interrupts */
> > > //#define DEBUG_IRQ
> > > @@ -1264,6 +1265,18 @@ void pc_machine_done(Notifier *notifier, void *data)
> > > sizeof(pcms->boot_cpus_le));
> > > }
> > > }
> > > +
> > > + if (pcms->apic_id_limit > 255) {
> > > + IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());
> > > +
> > > + if (!iommu || !iommu->x86_iommu.intr_supported ||
> > > + iommu->intr_eim != ON_OFF_AUTO_ON) {
> > > + error_report("current -smp configuration requires "
> > > + "Extended Interrupt Mode enabled. "
> > > + "IOMMU should have eim=on option set");
> >
> > Suggestion for a follow-up patch:
> >
> > * Error message explaining how to set eim=on if the iommu is
> > available
> > * Error message explaining how to make sure the iommu is created,
> > in case it was not even created.
> Reason I didn't include how to create iommu/CLI example is that
> it could be some other iommu in future so that message could
> bit rot over time.
I see.
>
> But I can add description if you'd prefer it.
> How about something like this:
> + error_report("current -smp configuration requires "
> + "Intel IOMMU with Extended Interrupt Mode enabled. "
> + "To enable IOMMU add to command line: "
> + "-device intel-iommu,intremap=on,eim=on");
If there could be other iommu devices in the future, we can show
it as an example, but not an instruction to be followed as-is.
Maybe we can just say "You can add an IOMMU using:
-device intel-iommu,intremap=on,eim=on". It would mean it is one
way to add an IOMMU, but not necessarily the only way.
BTW, are there any plans to make machine code create an IOMMU
automatically if the VM config requires it?
--
Eduardo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v4 13/13] pc: require IRQ remapping and EIM if there could be x2APIC CPUs
2016-10-18 12:55 ` Eduardo Habkost
@ 2016-10-18 14:39 ` Igor Mammedov
0 siblings, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-18 14:39 UTC (permalink / raw)
To: Eduardo Habkost
Cc: qemu-devel, rkrcmar, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
On Tue, 18 Oct 2016 10:55:57 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, Oct 18, 2016 at 02:44:55PM +0200, Igor Mammedov wrote:
> > On Tue, 18 Oct 2016 09:27:37 -0200
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Fri, Oct 14, 2016 at 01:25:35PM +0200, Igor Mammedov wrote:
> > > > it would prevent starting guest with incorrect configs
> > > > where interrupts couldn't be delivered to CPUs with
> > > > APIC IDs > 255.
> > > >
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> > > > ---
> > > > v4:
> > > > - s/254/255/ in commit message (Radim)
> > > > ---
> > > > hw/i386/pc.c | 13 +++++++++++++
> > > > 1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > > index 40eb43b..f7070e0 100644
> > > > --- a/hw/i386/pc.c
> > > > +++ b/hw/i386/pc.c
> > > > @@ -68,6 +68,7 @@
> > > > #include "qapi-visit.h"
> > > > #include "qom/cpu.h"
> > > > #include "hw/nmi.h"
> > > > +#include "hw/i386/intel_iommu.h"
> > > >
> > > > /* debug PC/ISA interrupts */
> > > > //#define DEBUG_IRQ
> > > > @@ -1264,6 +1265,18 @@ void pc_machine_done(Notifier *notifier, void *data)
> > > > sizeof(pcms->boot_cpus_le));
> > > > }
> > > > }
> > > > +
> > > > + if (pcms->apic_id_limit > 255) {
> > > > + IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());
> > > > +
> > > > + if (!iommu || !iommu->x86_iommu.intr_supported ||
> > > > + iommu->intr_eim != ON_OFF_AUTO_ON) {
> > > > + error_report("current -smp configuration requires "
> > > > + "Extended Interrupt Mode enabled. "
> > > > + "IOMMU should have eim=on option set");
> > >
> > > Suggestion for a follow-up patch:
> > >
> > > * Error message explaining how to set eim=on if the iommu is
> > > available
> > > * Error message explaining how to make sure the iommu is created,
> > > in case it was not even created.
> > Reason I didn't include how to create iommu/CLI example is that
> > it could be some other iommu in future so that message could
> > bit rot over time.
>
> I see.
>
> >
> > But I can add description if you'd prefer it.
> > How about something like this:
> > + error_report("current -smp configuration requires "
> > + "Intel IOMMU with Extended Interrupt Mode enabled. "
> > + "To enable IOMMU add to command line: "
> > + "-device intel-iommu,intremap=on,eim=on");
>
> If there could be other iommu devices in the future, we can show
> it as an example, but not an instruction to be followed as-is.
>
> Maybe we can just say "You can add an IOMMU using:
> -device intel-iommu,intremap=on,eim=on". It would mean it is one
> way to add an IOMMU, but not necessarily the only way.
ok, I'll amend error message.
>
> BTW, are there any plans to make machine code create an IOMMU
> automatically if the VM config requires it?
not that I know of
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (12 preceding siblings ...)
2016-10-13 9:52 ` [Qemu-devel] [PATCH v3 13/13] pc: require IRQ remapping and EIM if there could be x2APIC CPUs Igor Mammedov
@ 2016-10-13 10:01 ` Paolo Bonzini
2016-10-13 10:15 ` Igor Mammedov
2016-10-13 10:28 ` Gerd Hoffmann
2016-10-13 13:24 ` [Qemu-devel] [PATCH v3 14/13] pc: q35: bump max_cpus to 288 Igor Mammedov
2016-10-14 4:05 ` [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode no-reply
15 siblings, 2 replies; 68+ messages in thread
From: Paolo Bonzini @ 2016-10-13 10:01 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin,
lersek, chao.gao
On 13/10/2016 11:52, Igor Mammedov wrote:
> To play with the feature, one would also need x2apic enabled
> seabios counterpart:
> https://github.com/imammedo/seabios.git x2apic_v4
Is this going to make it into seabios 1.10?
Paolo
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode
2016-10-13 10:01 ` [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Paolo Bonzini
@ 2016-10-13 10:15 ` Igor Mammedov
2016-10-13 10:28 ` Gerd Hoffmann
1 sibling, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 10:15 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, kraxel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx,
kevin, lersek, chao.gao
On Thu, 13 Oct 2016 12:01:48 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 13/10/2016 11:52, Igor Mammedov wrote:
> > To play with the feature, one would also need x2apic enabled
> > seabios counterpart:
> > https://github.com/imammedo/seabios.git x2apic_v4
>
> Is this going to make it into seabios 1.10?
The only dependency here is a new fwcfg file etc/boot-cpus
which so far (RFC-v2) haven't risen any questions/complains
from QEMU/SeabIOS sides, so we could assume that it won't
change and merge SeaBIOS side first.
BTW:
I'm going to respin SeaBIOS side today dropping per Kevin's suggestion
[PATCH v4 3/5] error out if present cpus count changed during SMP bringup
> Paolo
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode
2016-10-13 10:01 ` [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Paolo Bonzini
2016-10-13 10:15 ` Igor Mammedov
@ 2016-10-13 10:28 ` Gerd Hoffmann
1 sibling, 0 replies; 68+ messages in thread
From: Gerd Hoffmann @ 2016-10-13 10:28 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Igor Mammedov, qemu-devel, ehabkost, liuxiaojian6, mst, rkrcmar,
peterx, kevin, lersek, chao.gao
On Do, 2016-10-13 at 12:01 +0200, Paolo Bonzini wrote:
>
> On 13/10/2016 11:52, Igor Mammedov wrote:
> > To play with the feature, one would also need x2apic enabled
> > seabios counterpart:
> > https://github.com/imammedo/seabios.git x2apic_v4
>
> Is this going to make it into seabios 1.10?
seabios patches are basically ready and are waiting for the qemu patches
being merged. So, yes, if this series gets reviewed and merged quickly
it should be able to make 1.10
cheers,
Gerd
^ permalink raw reply [flat|nested] 68+ messages in thread
* [Qemu-devel] [PATCH v3 14/13] pc: q35: bump max_cpus to 288
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (13 preceding siblings ...)
2016-10-13 10:01 ` [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Paolo Bonzini
@ 2016-10-13 13:24 ` Igor Mammedov
2016-10-13 13:53 ` Radim Krčmář
2016-10-14 4:05 ` [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode no-reply
15 siblings, 1 reply; 68+ messages in thread
From: Igor Mammedov @ 2016-10-13 13:24 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, liuxiaojian6, mst, rkrcmar, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
along with it for machine versions 2.7 and older keep
it at 255.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
Forgot to inlude the last patch that
permits using more than 255 cpus since q35-2.8 machine type
v2:
- make 288 cpus available only since q35-2.8 machine type
---
hw/i386/pc_q35.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 0b214f2..b40d19e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -291,6 +291,7 @@ static void pc_q35_machine_options(MachineClass *m)
m->default_display = "std";
m->no_floppy = 1;
m->has_dynamic_sysbus = true;
+ m->max_cpus = 288;
}
static void pc_q35_2_8_machine_options(MachineClass *m)
@@ -306,6 +307,7 @@ static void pc_q35_2_7_machine_options(MachineClass *m)
{
pc_q35_2_8_machine_options(m);
m->alias = NULL;
+ m->max_cpus = 255;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 14/13] pc: q35: bump max_cpus to 288
2016-10-13 13:24 ` [Qemu-devel] [PATCH v3 14/13] pc: q35: bump max_cpus to 288 Igor Mammedov
@ 2016-10-13 13:53 ` Radim Krčmář
0 siblings, 0 replies; 68+ messages in thread
From: Radim Krčmář @ 2016-10-13 13:53 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, ehabkost, liuxiaojian6, mst, peterx, kevin, kraxel,
pbonzini, lersek, chao.gao
2016-10-13 15:24+0200, Igor Mammedov:
> along with it for machine versions 2.7 and older keep
> it at 255.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
Seems nicer than touching max_cpus in pc_machine_class_init(),
Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> Forgot to inlude the last patch that
> permits using more than 255 cpus since q35-2.8 machine type
>
>
> v2:
> - make 288 cpus available only since q35-2.8 machine type
> ---
> hw/i386/pc_q35.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 0b214f2..b40d19e 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -291,6 +291,7 @@ static void pc_q35_machine_options(MachineClass *m)
> m->default_display = "std";
> m->no_floppy = 1;
> m->has_dynamic_sysbus = true;
> + m->max_cpus = 288;
> }
>
> static void pc_q35_2_8_machine_options(MachineClass *m)
> @@ -306,6 +307,7 @@ static void pc_q35_2_7_machine_options(MachineClass *m)
> {
> pc_q35_2_8_machine_options(m);
> m->alias = NULL;
> + m->max_cpus = 255;
> SET_MACHINE_COMPAT(m, PC_COMPAT_2_7);
> }
>
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode
2016-10-13 9:52 [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode Igor Mammedov
` (14 preceding siblings ...)
2016-10-13 13:24 ` [Qemu-devel] [PATCH v3 14/13] pc: q35: bump max_cpus to 288 Igor Mammedov
@ 2016-10-14 4:05 ` no-reply
2016-10-14 7:59 ` Igor Mammedov
15 siblings, 1 reply; 68+ messages in thread
From: no-reply @ 2016-10-14 4:05 UTC (permalink / raw)
To: imammedo
Cc: famz, qemu-devel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx,
kevin, kraxel, pbonzini, lersek, chao.gao
Hi,
Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.
Type: series
Message-id: 1476352367-69400-1-git-send-email-imammedo@redhat.com
Subject: [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode
=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=16
make docker-test-quick@centos6
make docker-test-mingw@fedora
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
bca661e pc: q35: bump max_cpus to 288
17f6729 pc: require IRQ remapping and EIM if there could be x2APIC CPUs
8fc45a4 pc: add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs
a7d6417 increase MAX_CPUMASK_BITS from 255 to 288
b03673c pc: clarify FW_CFG_MAX_CPUS usage comment
f71d194 pc: kvm_apic: pass APIC ID depending on xAPIC/x2APIC mode
87d9001 pc: apic_common: reset APIC ID to initial ID when switching into x2APIC mode
426abcf pc: apic_common: restore APIC ID to initial ID on reset
9cb56de pc: apic_common: extend APIC ID property to 32bit
489db69 pc: leave max apic_id_limit only in legacy cpu hotplug code
a503ed4 acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254
4899cb1 acpi: cphp: support x2APIC entry in cpu._MAT
b9fe39d pc: acpi: x2APIC support for SRAT table
4ffb548 pc: acpi: x2APIC support for MADT table
=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
BUILD centos6
ARCHIVE qemu.tgz
ARCHIVE dtc.tgz
COPY RUNNER
RUN test-quick in centos6
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
gcc-4.4.7-17.el6.x86_64
git-1.7.1-4.el6_7.1.x86_64
glib2-devel-2.28.8-5.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64
Environment variables:
PACKAGES=libfdt-devel ccache tar git make gcc g++ zlib-devel glib2-devel SDL-devel pixman-devel epel-release
HOSTNAME=b7cb6513802f
TERM=xterm
MAKEFLAGS= -j16
HISTSIZE=1000
J=16
USER=root
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env
Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix /var/tmp/qemu-build/install
BIOS directory /var/tmp/qemu-build/install/share/qemu
binary directory /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory /var/tmp/qemu-build/install/etc
local state directory /var/tmp/qemu-build/install/var
Manual directory /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path /tmp/qemu-test/src
C compiler cc
Host C compiler cc
C++ compiler
Objective-C compiler cc
ARFLAGS rv
CFLAGS -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g
QEMU_CFLAGS -I/usr/include/pixman-1 -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all
LDFLAGS -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g
make make
install install
python python -B
smbd /usr/sbin/smbd
module support no
host CPU x86_64
host big endian no
target list x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabled no
strip binaries yes
profiler no
static build no
pixman system
SDL support yes (1.2.14)
GTK support no
GTK GL support no
VTE support no
TLS priority NORMAL
GNUTLS support no
GNUTLS rnd no
libgcrypt no
libgcrypt kdf no
nettle no
nettle kdf no
libtasn1 no
curses support no
virgl support no
curl support no
mingw32 support no
Audio drivers oss
Block whitelist (rw)
Block whitelist (ro)
VirtFS support no
VNC support yes
VNC SASL support no
VNC JPEG support no
VNC PNG support no
xen support no
brlapi support no
bluez support no
Documentation no
PIE yes
vde support no
netmap support no
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support yes
RDMA support no
TCG interpreter no
fdt support yes
preadv support yes
fdatasync yes
madvise yes
posix_madvise yes
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
vhost-vsock support yes
Trace backends log
spice support no
rbd support no
xfsctl support no
smartcard support no
libusb no
usb net redir no
OpenGL support no
OpenGL dmabufs no
libiscsi support no
libnfs support no
build guest agent yes
QGA VSS support no
QGA w32 disk info no
QGA MSI support no
seccomp support no
coroutine backend ucontext
coroutine pool yes
debug stack usage no
GlusterFS support no
Archipelago support no
gcov gcov
gcov enabled no
TPM support yes
libssh2 support no
TPM passthrough yes
QOM debugging yes
lzo support no
snappy support no
bzip2 support no
NUMA host support no
tcmalloc support no
jemalloc support no
avx2 optimization no
replication support yes
GEN x86_64-softmmu/config-devices.mak.tmp
GEN aarch64-softmmu/config-devices.mak.tmp
GEN config-host.h
GEN qemu-options.def
GEN qmp-commands.h
GEN qapi-types.h
GEN qapi-visit.h
GEN qapi-event.h
GEN qmp-introspect.h
GEN module_block.h
GEN x86_64-softmmu/config-devices.mak
GEN tests/test-qapi-types.h
GEN tests/test-qapi-visit.h
GEN tests/test-qmp-commands.h
GEN tests/test-qapi-event.h
GEN tests/test-qmp-introspect.h
GEN aarch64-softmmu/config-devices.mak
GEN trace/generated-tracers.h
GEN trace/generated-tcg-tracers.h
GEN trace/generated-helpers-wrappers.h
GEN trace/generated-helpers.h
GEN config-all-devices.mak
CC tests/qemu-iotests/socket_scm_helper.o
GEN qga/qapi-generated/qga-qapi-types.h
GEN qga/qapi-generated/qga-qapi-visit.h
GEN qga/qapi-generated/qga-qmp-commands.h
GEN qga/qapi-generated/qga-qapi-types.c
GEN qga/qapi-generated/qga-qapi-visit.c
GEN qga/qapi-generated/qga-qmp-marshal.c
GEN qmp-introspect.c
GEN qapi-types.c
GEN qapi-visit.c
GEN qapi-event.c
CC qapi/qapi-visit-core.o
CC qapi/qapi-dealloc-visitor.o
CC qapi/qmp-input-visitor.o
CC qapi/qmp-output-visitor.o
CC qapi/qmp-registry.o
CC qapi/qmp-dispatch.o
CC qapi/string-input-visitor.o
CC qapi/string-output-visitor.o
CC qapi/opts-visitor.o
CC qapi/qapi-clone-visitor.o
CC qapi/qmp-event.o
CC qapi/qapi-util.o
CC qobject/qnull.o
CC qobject/qint.o
CC qobject/qstring.o
CC qobject/qdict.o
CC qobject/qlist.o
CC qobject/qfloat.o
CC qobject/qbool.o
CC qobject/qjson.o
CC qobject/qobject.o
CC qobject/json-lexer.o
CC qobject/json-streamer.o
CC qobject/json-parser.o
GEN trace/generated-tracers.c
CC trace/control.o
CC trace/qmp.o
CC util/osdep.o
CC util/cutils.o
CC util/unicode.o
CC util/qemu-timer-common.o
CC util/bufferiszero.o
CC util/compatfd.o
CC util/event_notifier-posix.o
CC util/mmap-alloc.o
CC util/oslib-posix.o
CC util/qemu-thread-posix.o
CC util/qemu-openpty.o
CC util/envlist.o
CC util/memfd.o
CC util/path.o
CC util/bitmap.o
CC util/module.o
CC util/bitops.o
CC util/hbitmap.o
CC util/fifo8.o
CC util/acl.o
CC util/error.o
CC util/qemu-error.o
CC util/id.o
CC util/iov.o
CC util/qemu-config.o
CC util/uri.o
CC util/qemu-sockets.o
CC util/notify.o
CC util/qemu-option.o
CC util/qemu-progress.o
CC util/hexdump.o
CC util/crc32c.o
CC util/uuid.o
CC util/throttle.o
CC util/getauxval.o
CC util/readline.o
CC util/rfifolock.o
CC util/rcu.o
CC util/qemu-coroutine.o
CC util/qemu-coroutine-lock.o
CC util/qemu-coroutine-io.o
CC util/qemu-coroutine-sleep.o
CC util/coroutine-ucontext.o
CC util/base64.o
CC util/buffer.o
CC util/timed-average.o
CC util/log.o
CC util/qdist.o
CC util/qht.o
CC util/range.o
CC crypto/pbkdf-stub.o
CC stubs/arch-query-cpu-def.o
CC stubs/arch-query-cpu-model-expansion.o
CC stubs/arch-query-cpu-model-comparison.o
CC stubs/arch-query-cpu-model-baseline.o
CC stubs/bdrv-next-monitor-owned.o
CC stubs/blk-commit-all.o
CC stubs/blockdev-close-all-bdrv-states.o
CC stubs/clock-warp.o
CC stubs/cpu-get-clock.o
CC stubs/cpu-get-icount.o
CC stubs/dump.o
CC stubs/fdset-add-fd.o
CC stubs/fdset-find-fd.o
CC stubs/fdset-get-fd.o
CC stubs/fdset-remove-fd.o
CC stubs/get-fd.o
CC stubs/gdbstub.o
CC stubs/get-next-serial.o
CC stubs/get-vm-name.o
CC stubs/iothread-lock.o
CC stubs/is-daemonized.o
CC stubs/machine-init-done.o
CC stubs/migr-blocker.o
CC stubs/mon-is-qmp.o
CC stubs/mon-printf.o
CC stubs/monitor-init.o
CC stubs/notify-event.o
CC stubs/qtest.o
CC stubs/replay.o
CC stubs/replay-user.o
CC stubs/reset.o
CC stubs/runstate-check.o
CC stubs/slirp.o
CC stubs/set-fd-handler.o
CC stubs/sysbus.o
CC stubs/trace-control.o
CC stubs/uuid.o
CC stubs/vm-stop.o
CC stubs/vmstate.o
CC stubs/cpus.o
CC stubs/kvm.o
CC stubs/qmp_pc_dimm_device_list.o
CC stubs/target-monitor-defs.o
CC stubs/target-get-monitor-def.o
CC stubs/vhost.o
CC stubs/iohandler.o
CC stubs/ipmi.o
CC stubs/smbios_type_38.o
CC stubs/pc_madt_cpu_entry.o
CC contrib/ivshmem-client/ivshmem-client.o
CC contrib/ivshmem-client/main.o
CC contrib/ivshmem-server/ivshmem-server.o
CC contrib/ivshmem-server/main.o
CC qemu-nbd.o
CC async.o
CC thread-pool.o
CC block.o
CC blockjob.o
CC main-loop.o
CC iohandler.o
CC qemu-timer.o
CC aio-posix.o
CC qemu-io-cmds.o
CC replication.o
CC block/raw_bsd.o
CC block/qcow.o
CC block/vdi.o
CC block/vmdk.o
CC block/cloop.o
CC block/bochs.o
CC block/vpc.o
CC block/vvfat.o
CC block/dmg.o
CC block/qcow2.o
CC block/qcow2-refcount.o
CC block/qcow2-cluster.o
CC block/qcow2-snapshot.o
CC block/qed.o
CC block/qcow2-cache.o
CC block/qed-gencb.o
CC block/qed-l2-cache.o
CC block/qed-table.o
CC block/qed-cluster.o
CC block/qed-check.o
CC block/vhdx.o
CC block/vhdx-endian.o
CC block/vhdx-log.o
CC block/quorum.o
CC block/parallels.o
CC block/blkdebug.o
CC block/blkverify.o
CC block/blkreplay.o
CC block/block-backend.o
CC block/snapshot.o
CC block/qapi.o
CC block/raw-posix.o
CC block/null.o
CC block/mirror.o
CC block/commit.o
CC block/io.o
CC block/throttle-groups.o
CC block/nbd-client.o
CC block/nbd.o
CC block/sheepdog.o
CC block/accounting.o
CC block/dirty-bitmap.o
CC block/write-threshold.o
CC block/backup.o
CC block/crypto.o
CC block/replication.o
CC nbd/server.o
CC nbd/client.o
CC nbd/common.o
CC crypto/init.o
CC crypto/aes.o
CC crypto/hash.o
CC crypto/hash-glib.o
CC crypto/desrfb.o
CC crypto/cipher.o
CC crypto/tlscreds.o
CC crypto/tlscredsanon.o
CC crypto/tlscredsx509.o
CC crypto/tlssession.o
CC crypto/secret.o
CC crypto/random-platform.o
CC crypto/pbkdf.o
CC crypto/ivgen-plain.o
CC crypto/ivgen.o
CC crypto/ivgen-essiv.o
CC crypto/ivgen-plain64.o
CC crypto/afsplit.o
CC crypto/xts.o
CC crypto/block.o
CC crypto/block-luks.o
CC crypto/block-qcow.o
CC io/channel-buffer.o
CC io/channel.o
CC io/channel-command.o
CC io/channel-file.o
CC io/channel-socket.o
CC io/channel-tls.o
CC io/channel-watch.o
CC io/channel-websock.o
CC io/channel-util.o
CC qom/object.o
CC io/task.o
CC qom/container.o
CC qom/qom-qobject.o
CC qom/object_interfaces.o
GEN qemu-img-cmds.h
CC qemu-bridge-helper.o
CC blockdev.o
CC qemu-io.o
CC blockdev-nbd.o
CC iothread.o
CC device-hotplug.o
CC qdev-monitor.o
CC os-posix.o
CC qemu-char.o
CC page_cache.o
CC accel.o
CC bt-host.o
CC bt-vhci.o
CC dma-helpers.o
CC vl.o
CC tpm.o
CC qmp.o
CC device_tree.o
GEN qmp-marshal.c
CC hmp.o
CC tcg-runtime.o
CC cpus-common.o
CC audio/audio.o
CC audio/noaudio.o
CC audio/wavaudio.o
CC audio/mixeng.o
CC audio/sdlaudio.o
CC audio/ossaudio.o
CC audio/wavcapture.o
CC backends/rng.o
CC backends/rng-random.o
CC backends/rng-egd.o
CC backends/msmouse.o
CC backends/testdev.o
CC backends/tpm.o
CC backends/hostmem-ram.o
CC backends/hostmem.o
CC backends/hostmem-file.o
CC block/stream.o
CC disas/arm.o
CC disas/i386.o
CC fsdev/qemu-fsdev-opts.o
CC fsdev/qemu-fsdev-dummy.o
CC hw/acpi/core.o
CC hw/acpi/piix4.o
CC hw/acpi/pcihp.o
CC hw/acpi/ich9.o
CC hw/acpi/tco.o
CC hw/acpi/cpu_hotplug.o
CC hw/acpi/memory_hotplug.o
CC hw/acpi/memory_hotplug_acpi_table.o
CC hw/acpi/cpu.o
CC hw/acpi/acpi_interface.o
CC hw/acpi/bios-linker-loader.o
CC hw/acpi/aml-build.o
CC hw/acpi/ipmi.o
CC hw/audio/sb16.o
CC hw/audio/es1370.o
CC hw/audio/ac97.o
CC hw/audio/fmopl.o
CC hw/audio/adlib.o
CC hw/audio/gus.o
CC hw/audio/gusemu_hal.o
CC hw/audio/gusemu_mixer.o
CC hw/audio/cs4231a.o
CC hw/audio/intel-hda.o
CC hw/audio/hda-codec.o
CC hw/audio/pcspk.o
CC hw/audio/wm8750.o
CC hw/audio/pl041.o
CC hw/audio/lm4549.o
CC hw/audio/marvell_88w8618.o
CC hw/block/block.o
CC hw/block/cdrom.o
CC hw/block/hd-geometry.o
CC hw/block/fdc.o
CC hw/block/m25p80.o
CC hw/block/nand.o
CC hw/block/pflash_cfi01.o
CC hw/block/pflash_cfi02.o
CC hw/block/ecc.o
CC hw/block/onenand.o
CC hw/block/nvme.o
CC hw/bt/core.o
CC hw/bt/l2cap.o
CC hw/bt/sdp.o
CC hw/bt/hci.o
CC hw/bt/hid.o
CC hw/bt/hci-csr.o
CC hw/char/ipoctal232.o
CC hw/char/parallel.o
CC hw/char/pl011.o
CC hw/char/serial.o
CC hw/char/serial-isa.o
CC hw/char/serial-pci.o
CC hw/char/virtio-console.o
CC hw/char/cadence_uart.o
CC hw/char/debugcon.o
CC hw/char/imx_serial.o
CC hw/core/qdev.o
CC hw/core/qdev-properties.o
CC hw/core/bus.o
CC hw/core/fw-path-provider.o
CC hw/core/irq.o
CC hw/core/hotplug.o
CC hw/core/ptimer.o
CC hw/core/sysbus.o
CC hw/core/machine.o
CC hw/core/null-machine.o
CC hw/core/qdev-properties-system.o
CC hw/core/loader.o
CC hw/core/register.o
CC hw/core/or-irq.o
CC hw/core/platform-bus.o
CC hw/display/ads7846.o
CC hw/display/cirrus_vga.o
CC hw/display/ssd0303.o
CC hw/display/pl110.o
CC hw/display/ssd0323.o
CC hw/display/vga-pci.o
CC hw/display/vga-isa.o
CC hw/display/vmware_vga.o
CC hw/display/blizzard.o
CC hw/display/exynos4210_fimd.o
CC hw/display/framebuffer.o
CC hw/display/tc6393xb.o
CC hw/dma/pl080.o
CC hw/dma/pl330.o
CC hw/dma/xlnx-zynq-devcfg.o
CC hw/dma/i8257.o
CC hw/gpio/max7310.o
CC hw/gpio/pl061.o
CC hw/gpio/zaurus.o
CC hw/gpio/gpio_key.o
CC hw/i2c/core.o
CC hw/i2c/smbus.o
CC hw/i2c/smbus_eeprom.o
CC hw/i2c/i2c-ddc.o
CC hw/i2c/versatile_i2c.o
CC hw/i2c/smbus_ich9.o
CC hw/i2c/pm_smbus.o
CC hw/i2c/bitbang_i2c.o
CC hw/i2c/exynos4210_i2c.o
CC hw/i2c/imx_i2c.o
CC hw/ide/core.o
CC hw/i2c/aspeed_i2c.o
CC hw/ide/atapi.o
CC hw/ide/qdev.o
CC hw/ide/pci.o
CC hw/ide/isa.o
CC hw/ide/piix.o
CC hw/ide/microdrive.o
CC hw/ide/ahci.o
CC hw/ide/ich.o
CC hw/input/hid.o
CC hw/input/lm832x.o
CC hw/input/pckbd.o
CC hw/input/pl050.o
CC hw/input/ps2.o
CC hw/input/stellaris_input.o
CC hw/input/tsc2005.o
CC hw/input/vmmouse.o
CC hw/input/virtio-input.o
CC hw/input/virtio-input-hid.o
CC hw/input/virtio-input-host.o
CC hw/intc/i8259_common.o
CC hw/intc/i8259.o
CC hw/intc/pl190.o
CC hw/intc/imx_avic.o
CC hw/intc/realview_gic.o
CC hw/intc/ioapic_common.o
CC hw/intc/arm_gic_common.o
CC hw/intc/arm_gicv2m.o
CC hw/intc/arm_gic.o
CC hw/intc/arm_gicv3_common.o
CC hw/intc/arm_gicv3.o
CC hw/intc/arm_gicv3_dist.o
CC hw/intc/arm_gicv3_redist.o
CC hw/intc/arm_gicv3_its_common.o
CC hw/intc/intc.o
CC hw/ipack/ipack.o
CC hw/ipack/tpci200.o
CC hw/ipmi/ipmi_bmc_sim.o
CC hw/ipmi/ipmi.o
CC hw/ipmi/isa_ipmi_bt.o
CC hw/isa/isa-bus.o
CC hw/ipmi/isa_ipmi_kcs.o
CC hw/ipmi/ipmi_bmc_extern.o
CC hw/isa/apm.o
CC hw/mem/pc-dimm.o
CC hw/mem/nvdimm.o
CC hw/misc/applesmc.o
CC hw/misc/max111x.o
CC hw/misc/tmp105.o
CC hw/misc/debugexit.o
CC hw/misc/sga.o
CC hw/misc/pc-testdev.o
CC hw/misc/pci-testdev.o
CC hw/misc/arm_l2x0.o
CC hw/misc/arm_integrator_debug.o
CC hw/misc/a9scu.o
CC hw/misc/arm11scu.o
CC hw/net/ne2000.o
CC hw/net/eepro100.o
CC hw/net/pcnet-pci.o
CC hw/net/pcnet.o
CC hw/net/e1000.o
CC hw/net/e1000x_common.o
CC hw/net/net_tx_pkt.o
CC hw/net/net_rx_pkt.o
CC hw/net/e1000e.o
CC hw/net/e1000e_core.o
CC hw/net/rtl8139.o
CC hw/net/vmxnet3.o
CC hw/net/smc91c111.o
CC hw/net/lan9118.o
CC hw/net/ne2000-isa.o
CC hw/net/xgmac.o
CC hw/net/allwinner_emac.o
CC hw/net/imx_fec.o
CC hw/net/cadence_gem.o
CC hw/net/stellaris_enet.o
CC hw/net/rocker/rocker.o
CC hw/net/rocker/rocker_fp.o
CC hw/net/rocker/rocker_desc.o
CC hw/net/rocker/rocker_world.o
CC hw/nvram/eeprom93xx.o
CC hw/net/rocker/rocker_of_dpa.o
CC hw/nvram/fw_cfg.o
CC hw/pci-bridge/pci_bridge_dev.o
CC hw/pci-bridge/pci_expander_bridge.o
CC hw/pci-bridge/xio3130_downstream.o
CC hw/pci-bridge/xio3130_upstream.o
CC hw/pci-bridge/ioh3420.o
CC hw/pci-bridge/i82801b11.o
CC hw/pci-host/pam.o
CC hw/pci-host/versatile.o
CC hw/pci-host/piix.o
CC hw/pci/pci.o
CC hw/pci-host/q35.o
CC hw/pci-host/gpex.o
CC hw/pci/pci_bridge.o
CC hw/pci/msix.o
CC hw/pci/msi.o
CC hw/pci/shpc.o
CC hw/pci/slotid_cap.o
CC hw/pci/pci_host.o
CC hw/pci/pcie_host.o
CC hw/pci/pcie.o
CC hw/pci/pcie_aer.o
CC hw/pci/pcie_port.o
CC hw/pci/pci-stub.o
CC hw/pcmcia/pcmcia.o
CC hw/scsi/scsi-disk.o
CC hw/scsi/scsi-generic.o
CC hw/scsi/scsi-bus.o
CC hw/scsi/lsi53c895a.o
CC hw/scsi/mptsas.o
CC hw/scsi/mptconfig.o
/tmp/qemu-test/src/hw/nvram/fw_cfg.c: In function ‘fw_cfg_dma_transfer’:
/tmp/qemu-test/src/hw/nvram/fw_cfg.c:330: warning: ‘read’ may be used uninitialized in this function
CC hw/scsi/megasas.o
CC hw/scsi/mptendian.o
CC hw/scsi/vmw_pvscsi.o
CC hw/scsi/esp.o
CC hw/scsi/esp-pci.o
CC hw/sd/pl181.o
CC hw/sd/ssi-sd.o
CC hw/sd/sd.o
CC hw/sd/core.o
CC hw/sd/sdhci.o
CC hw/smbios/smbios.o
CC hw/smbios/smbios_type_38.o
CC hw/ssi/pl022.o
CC hw/ssi/ssi.o
CC hw/ssi/xilinx_spips.o
CC hw/ssi/aspeed_smc.o
CC hw/ssi/stm32f2xx_spi.o
CC hw/timer/arm_timer.o
CC hw/timer/arm_mptimer.o
CC hw/timer/a9gtimer.o
CC hw/timer/cadence_ttc.o
CC hw/timer/ds1338.o
CC hw/timer/hpet.o
CC hw/timer/i8254_common.o
CC hw/timer/i8254.o
CC hw/timer/pl031.o
CC hw/timer/twl92230.o
CC hw/timer/imx_epit.o
CC hw/timer/imx_gpt.o
CC hw/timer/stm32f2xx_timer.o
CC hw/timer/aspeed_timer.o
CC hw/tpm/tpm_tis.o
CC hw/tpm/tpm_passthrough.o
CC hw/tpm/tpm_util.o
CC hw/usb/core.o
CC hw/usb/combined-packet.o
CC hw/usb/bus.o
CC hw/usb/libhw.o
CC hw/usb/desc-msos.o
CC hw/usb/desc.o
CC hw/usb/hcd-uhci.o
CC hw/usb/hcd-ohci.o
CC hw/usb/hcd-ehci.o
CC hw/usb/hcd-ehci-sysbus.o
CC hw/usb/hcd-ehci-pci.o
CC hw/usb/hcd-xhci.o
CC hw/usb/dev-hub.o
CC hw/usb/hcd-musb.o
CC hw/usb/dev-hid.o
CC hw/usb/dev-wacom.o
CC hw/usb/dev-uas.o
CC hw/usb/dev-audio.o
CC hw/usb/dev-storage.o
CC hw/usb/dev-serial.o
CC hw/usb/dev-bluetooth.o
CC hw/usb/dev-network.o
CC hw/usb/dev-smartcard-reader.o
CC hw/usb/dev-mtp.o
CC hw/usb/host-stub.o
CC hw/virtio/virtio-rng.o
CC hw/virtio/virtio-pci.o
CC hw/virtio/virtio-bus.o
CC hw/virtio/virtio-mmio.o
CC hw/watchdog/watchdog.o
CC hw/watchdog/wdt_i6300esb.o
CC hw/watchdog/wdt_ib700.o
CC migration/socket.o
CC migration/migration.o
CC migration/fd.o
CC migration/exec.o
CC migration/tls.o
CC migration/vmstate.o
CC migration/qemu-file-channel.o
CC migration/qemu-file.o
CC migration/xbzrle.o
CC migration/postcopy-ram.o
CC migration/qjson.o
CC migration/block.o
CC net/net.o
CC net/queue.o
CC net/checksum.o
CC net/util.o
CC net/hub.o
CC net/socket.o
CC net/dump.o
CC net/eth.o
CC net/l2tpv3.o
CC net/tap.o
CC net/vhost-user.o
CC net/tap-linux.o
CC net/slirp.o
CC net/filter.o
CC net/filter-buffer.o
CC net/filter-mirror.o
CC net/colo-compare.o
CC net/colo.o
CC net/filter-rewriter.o
CC qom/cpu.o
CC replay/replay.o
CC replay/replay-internal.o
CC replay/replay-time.o
CC replay/replay-events.o
CC replay/replay-input.o
CC replay/replay-char.o
CC replay/replay-snapshot.o
CC slirp/cksum.o
CC slirp/if.o
CC slirp/ip_icmp.o
CC slirp/ip6_icmp.o
CC slirp/ip6_input.o
CC slirp/ip6_output.o
CC slirp/ip_input.o
CC slirp/ip_output.o
CC slirp/dnssearch.o
/tmp/qemu-test/src/replay/replay-internal.c: In function ‘replay_put_array’:
/tmp/qemu-test/src/replay/replay-internal.c:65: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result
CC slirp/slirp.o
CC slirp/dhcpv6.o
CC slirp/mbuf.o
CC slirp/misc.o
CC slirp/sbuf.o
CC slirp/socket.o
CC slirp/tcp_input.o
/tmp/qemu-test/src/slirp/tcp_input.c: In function ‘tcp_input’:
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_p’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_len’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_tos’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_id’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_off’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_ttl’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_sum’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_src.s_addr’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_dst.s_addr’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:220: warning: ‘save_ip6.ip_nh’ may be used uninitialized in this function
CC slirp/tcp_output.o
CC slirp/tcp_timer.o
CC slirp/tcp_subr.o
CC slirp/bootp.o
CC slirp/udp.o
CC slirp/tftp.o
CC slirp/udp6.o
CC slirp/arp_table.o
CC ui/keymaps.o
CC slirp/ndp_table.o
CC ui/console.o
CC ui/cursor.o
CC ui/qemu-pixman.o
CC ui/input.o
CC ui/input-legacy.o
CC ui/input-linux.o
CC ui/input-keymap.o
CC ui/sdl_zoom.o
CC ui/sdl.o
CC ui/x_keymap.o
CC ui/vnc.o
CC ui/vnc-enc-zlib.o
CC ui/vnc-enc-hextile.o
CC ui/vnc-enc-tight.o
CC ui/vnc-palette.o
CC ui/vnc-enc-zrle.o
CC ui/vnc-auth-vencrypt.o
CC ui/vnc-ws.o
CC ui/vnc-jobs.o
CC qga/commands.o
LINK tests/qemu-iotests/socket_scm_helper
CC qga/guest-agent-command-state.o
CC qga/main.o
CC qga/commands-posix.o
CC qga/channel-posix.o
CC qga/qapi-generated/qga-qapi-types.o
CC qga/qapi-generated/qga-qapi-visit.o
CC qga/qapi-generated/qga-qmp-marshal.o
CC qmp-introspect.o
CC qapi-types.o
CC qapi-visit.o
CC qapi-event.o
AR libqemustub.a
CC qemu-img.o
CC trace/generated-tracers.o
CC qmp-marshal.o
AR libqemuutil.a
CC optionrom/linuxboot_dma.o
AS optionrom/kvmvapic.o
AS optionrom/multiboot.o
AS optionrom/linuxboot.o
cc: unrecognized option '-no-integrated-as'
cc: unrecognized option '-no-integrated-as'
BUILD optionrom/linuxboot_dma.img
BUILD optionrom/linuxboot_dma.raw
BUILD optionrom/multiboot.img
BUILD optionrom/kvmvapic.img
BUILD optionrom/linuxboot.img
BUILD optionrom/multiboot.raw
BUILD optionrom/kvmvapic.raw
BUILD optionrom/linuxboot.raw
SIGN optionrom/kvmvapic.bin
SIGN optionrom/linuxboot.bin
SIGN optionrom/multiboot.bin
SIGN optionrom/linuxboot_dma.bin
LINK qemu-ga
LINK ivshmem-client
LINK ivshmem-server
LINK qemu-nbd
LINK qemu-img
LINK qemu-io
LINK qemu-bridge-helper
GEN x86_64-softmmu/hmp-commands.h
GEN x86_64-softmmu/hmp-commands-info.h
GEN x86_64-softmmu/config-target.h
GEN aarch64-softmmu/hmp-commands.h
GEN aarch64-softmmu/hmp-commands-info.h
GEN aarch64-softmmu/config-target.h
CC x86_64-softmmu/exec.o
CC x86_64-softmmu/translate-all.o
CC x86_64-softmmu/cpu-exec.o
CC x86_64-softmmu/cpu-exec-common.o
CC x86_64-softmmu/translate-common.o
CC x86_64-softmmu/tcg/tcg-op.o
CC x86_64-softmmu/tcg/tcg.o
CC x86_64-softmmu/tcg/optimize.o
CC x86_64-softmmu/tcg/tcg-common.o
CC x86_64-softmmu/fpu/softfloat.o
CC x86_64-softmmu/disas.o
CC x86_64-softmmu/arch_init.o
CC x86_64-softmmu/cpus.o
CC x86_64-softmmu/monitor.o
CC x86_64-softmmu/gdbstub.o
CC x86_64-softmmu/balloon.o
CC aarch64-softmmu/exec.o
CC aarch64-softmmu/translate-all.o
CC aarch64-softmmu/cpu-exec.o
CC aarch64-softmmu/translate-common.o
CC aarch64-softmmu/cpu-exec-common.o
CC aarch64-softmmu/tcg/tcg.o
CC x86_64-softmmu/ioport.o
CC aarch64-softmmu/tcg/tcg-op.o
CC aarch64-softmmu/tcg/optimize.o
CC aarch64-softmmu/tcg/tcg-common.o
CC aarch64-softmmu/fpu/softfloat.o
CC aarch64-softmmu/disas.o
GEN aarch64-softmmu/gdbstub-xml.c
CC aarch64-softmmu/kvm-stub.o
CC aarch64-softmmu/cpus.o
CC aarch64-softmmu/arch_init.o
CC aarch64-softmmu/monitor.o
CC aarch64-softmmu/gdbstub.o
CC x86_64-softmmu/numa.o
CC aarch64-softmmu/balloon.o
CC x86_64-softmmu/qtest.o
CC x86_64-softmmu/bootdevice.o
CC aarch64-softmmu/ioport.o
CC aarch64-softmmu/numa.o
CC aarch64-softmmu/qtest.o
CC aarch64-softmmu/bootdevice.o
CC aarch64-softmmu/memory.o
CC aarch64-softmmu/cputlb.o
CC aarch64-softmmu/memory_mapping.o
CC x86_64-softmmu/kvm-all.o
CC aarch64-softmmu/dump.o
CC aarch64-softmmu/migration/ram.o
CC aarch64-softmmu/migration/savevm.o
CC aarch64-softmmu/xen-common-stub.o
CC aarch64-softmmu/xen-hvm-stub.o
CC x86_64-softmmu/memory.o
CC x86_64-softmmu/cputlb.o
CC x86_64-softmmu/memory_mapping.o
CC aarch64-softmmu/hw/adc/stm32f2xx_adc.o
CC aarch64-softmmu/hw/block/virtio-blk.o
CC aarch64-softmmu/hw/block/dataplane/virtio-blk.o
CC x86_64-softmmu/dump.o
CC x86_64-softmmu/migration/ram.o
CC aarch64-softmmu/hw/char/exynos4210_uart.o
CC x86_64-softmmu/migration/savevm.o
CC x86_64-softmmu/xen-common-stub.o
CC x86_64-softmmu/xen-hvm-stub.o
CC x86_64-softmmu/hw/acpi/nvdimm.o
CC aarch64-softmmu/hw/char/omap_uart.o
CC aarch64-softmmu/hw/char/digic-uart.o
CC aarch64-softmmu/hw/char/stm32f2xx_usart.o
CC aarch64-softmmu/hw/char/bcm2835_aux.o
CC x86_64-softmmu/hw/block/virtio-blk.o
CC x86_64-softmmu/hw/block/dataplane/virtio-blk.o
CC x86_64-softmmu/hw/char/virtio-serial-bus.o
CC x86_64-softmmu/hw/core/nmi.o
CC x86_64-softmmu/hw/core/generic-loader.o
CC aarch64-softmmu/hw/char/virtio-serial-bus.o
CC aarch64-softmmu/hw/core/nmi.o
CC aarch64-softmmu/hw/core/generic-loader.o
CC aarch64-softmmu/hw/cpu/arm11mpcore.o
CC aarch64-softmmu/hw/cpu/realview_mpcore.o
CC x86_64-softmmu/hw/cpu/core.o
CC aarch64-softmmu/hw/cpu/a9mpcore.o
CC aarch64-softmmu/hw/cpu/a15mpcore.o
CC aarch64-softmmu/hw/cpu/core.o
CC aarch64-softmmu/hw/display/omap_dss.o
CC x86_64-softmmu/hw/display/vga.o
CC aarch64-softmmu/hw/display/omap_lcdc.o
CC x86_64-softmmu/hw/display/virtio-gpu.o
CC aarch64-softmmu/hw/display/pxa2xx_lcd.o
CC x86_64-softmmu/hw/display/virtio-gpu-3d.o
CC aarch64-softmmu/hw/display/bcm2835_fb.o
CC aarch64-softmmu/hw/display/vga.o
CC x86_64-softmmu/hw/display/virtio-gpu-pci.o
CC x86_64-softmmu/hw/display/virtio-vga.o
CC aarch64-softmmu/hw/display/virtio-gpu.o
CC x86_64-softmmu/hw/intc/apic.o
CC aarch64-softmmu/hw/display/virtio-gpu-3d.o
CC aarch64-softmmu/hw/display/virtio-gpu-pci.o
CC aarch64-softmmu/hw/display/dpcd.o
CC aarch64-softmmu/hw/display/xlnx_dp.o
CC x86_64-softmmu/hw/intc/apic_common.o
CC aarch64-softmmu/hw/dma/xlnx_dpdma.o
CC x86_64-softmmu/hw/intc/ioapic.o
CC aarch64-softmmu/hw/dma/omap_dma.o
CC x86_64-softmmu/hw/isa/lpc_ich9.o
CC x86_64-softmmu/hw/misc/vmport.o
CC aarch64-softmmu/hw/dma/soc_dma.o
CC aarch64-softmmu/hw/dma/pxa2xx_dma.o
CC aarch64-softmmu/hw/dma/bcm2835_dma.o
CC x86_64-softmmu/hw/misc/ivshmem.o
CC aarch64-softmmu/hw/gpio/omap_gpio.o
CC aarch64-softmmu/hw/gpio/imx_gpio.o
CC x86_64-softmmu/hw/misc/pvpanic.o
CC aarch64-softmmu/hw/i2c/omap_i2c.o
CC x86_64-softmmu/hw/misc/edu.o
CC x86_64-softmmu/hw/misc/hyperv_testdev.o
CC aarch64-softmmu/hw/input/pxa2xx_keypad.o
CC aarch64-softmmu/hw/input/tsc210x.o
CC aarch64-softmmu/hw/intc/armv7m_nvic.o
CC aarch64-softmmu/hw/intc/exynos4210_gic.o
CC aarch64-softmmu/hw/intc/exynos4210_combiner.o
CC aarch64-softmmu/hw/intc/omap_intc.o
CC aarch64-softmmu/hw/intc/bcm2835_ic.o
CC aarch64-softmmu/hw/intc/bcm2836_control.o
CC x86_64-softmmu/hw/net/virtio-net.o
CC x86_64-softmmu/hw/net/vhost_net.o
CC aarch64-softmmu/hw/intc/allwinner-a10-pic.o
CC aarch64-softmmu/hw/intc/aspeed_vic.o
CC aarch64-softmmu/hw/misc/ivshmem.o
CC aarch64-softmmu/hw/intc/arm_gicv3_cpuif.o
CC x86_64-softmmu/hw/scsi/virtio-scsi.o
CC aarch64-softmmu/hw/misc/arm_sysctl.o
CC x86_64-softmmu/hw/scsi/virtio-scsi-dataplane.o
CC aarch64-softmmu/hw/misc/cbus.o
CC aarch64-softmmu/hw/misc/exynos4210_pmu.o
CC x86_64-softmmu/hw/scsi/vhost-scsi.o
CC x86_64-softmmu/hw/timer/mc146818rtc.o
CC x86_64-softmmu/hw/vfio/common.o
CC x86_64-softmmu/hw/vfio/pci.o
CC x86_64-softmmu/hw/vfio/pci-quirks.o
CC x86_64-softmmu/hw/vfio/platform.o
CC x86_64-softmmu/hw/vfio/calxeda-xgmac.o
CC x86_64-softmmu/hw/vfio/amd-xgbe.o
CC x86_64-softmmu/hw/vfio/spapr.o
CC aarch64-softmmu/hw/misc/imx_ccm.o
CC x86_64-softmmu/hw/virtio/virtio.o
CC x86_64-softmmu/hw/virtio/virtio-balloon.o
CC x86_64-softmmu/hw/virtio/vhost.o
CC aarch64-softmmu/hw/misc/imx31_ccm.o
CC x86_64-softmmu/hw/virtio/vhost-backend.o
CC x86_64-softmmu/hw/virtio/vhost-user.o
CC aarch64-softmmu/hw/misc/imx25_ccm.o
CC x86_64-softmmu/hw/virtio/vhost-vsock.o
CC x86_64-softmmu/hw/i386/multiboot.o
CC x86_64-softmmu/hw/i386/pc.o
CC x86_64-softmmu/hw/i386/pc_piix.o
CC aarch64-softmmu/hw/misc/imx6_ccm.o
CC x86_64-softmmu/hw/i386/pc_q35.o
CC x86_64-softmmu/hw/i386/pc_sysfw.o
CC x86_64-softmmu/hw/i386/x86-iommu.o
CC aarch64-softmmu/hw/misc/imx6_src.o
CC x86_64-softmmu/hw/i386/intel_iommu.o
CC aarch64-softmmu/hw/misc/mst_fpga.o
CC aarch64-softmmu/hw/misc/omap_clk.o
CC aarch64-softmmu/hw/misc/omap_gpmc.o
CC x86_64-softmmu/hw/i386/amd_iommu.o
CC aarch64-softmmu/hw/misc/omap_l4.o
CC aarch64-softmmu/hw/misc/omap_sdrc.o
CC aarch64-softmmu/hw/misc/omap_tap.o
CC x86_64-softmmu/hw/i386/kvmvapic.o
CC x86_64-softmmu/hw/i386/acpi-build.o
CC aarch64-softmmu/hw/misc/bcm2835_mbox.o
/tmp/qemu-test/src/hw/i386/pc.c: In function ‘pc_machine_done’:
/tmp/qemu-test/src/hw/i386/pc.c:1273: error: ‘IntelIOMMUState’ has no member named ‘intr_eim’
make[1]: *** [hw/i386/pc.o] Error 1
make[1]: *** Waiting for unfinished jobs....
CC aarch64-softmmu/hw/misc/bcm2835_property.o
CC aarch64-softmmu/hw/misc/zynq_slcr.o
CC aarch64-softmmu/hw/misc/zynq-xadc.o
CC aarch64-softmmu/hw/misc/stm32f2xx_syscfg.o
CC aarch64-softmmu/hw/misc/edu.o
CC aarch64-softmmu/hw/misc/auxbus.o
CC aarch64-softmmu/hw/misc/aspeed_scu.o
CC aarch64-softmmu/hw/misc/aspeed_sdmc.o
CC aarch64-softmmu/hw/net/vhost_net.o
CC aarch64-softmmu/hw/net/virtio-net.o
CC aarch64-softmmu/hw/pcmcia/pxa2xx.o
CC aarch64-softmmu/hw/scsi/virtio-scsi.o
CC aarch64-softmmu/hw/scsi/virtio-scsi-dataplane.o
CC aarch64-softmmu/hw/scsi/vhost-scsi.o
CC aarch64-softmmu/hw/sd/omap_mmc.o
CC aarch64-softmmu/hw/sd/pxa2xx_mmci.o
CC aarch64-softmmu/hw/ssi/omap_spi.o
CC aarch64-softmmu/hw/timer/exynos4210_mct.o
CC aarch64-softmmu/hw/ssi/imx_spi.o
CC aarch64-softmmu/hw/timer/exynos4210_pwm.o
CC aarch64-softmmu/hw/timer/exynos4210_rtc.o
CC aarch64-softmmu/hw/timer/omap_gptimer.o
CC aarch64-softmmu/hw/timer/omap_synctimer.o
CC aarch64-softmmu/hw/timer/pxa2xx_timer.o
CC aarch64-softmmu/hw/timer/digic-timer.o
CC aarch64-softmmu/hw/timer/allwinner-a10-pit.o
CC aarch64-softmmu/hw/usb/tusb6010.o
CC aarch64-softmmu/hw/vfio/common.o
CC aarch64-softmmu/hw/vfio/pci.o
CC aarch64-softmmu/hw/vfio/pci-quirks.o
CC aarch64-softmmu/hw/vfio/platform.o
/tmp/qemu-test/src/hw/i386/pc_piix.c: In function ‘igd_passthrough_isa_bridge_create’:
/tmp/qemu-test/src/hw/i386/pc_piix.c:1046: warning: ‘pch_rev_id’ may be used uninitialized in this function
CC aarch64-softmmu/hw/vfio/calxeda-xgmac.o
CC aarch64-softmmu/hw/vfio/amd-xgbe.o
CC aarch64-softmmu/hw/vfio/spapr.o
CC aarch64-softmmu/hw/virtio/virtio.o
CC aarch64-softmmu/hw/virtio/vhost.o
CC aarch64-softmmu/hw/virtio/virtio-balloon.o
CC aarch64-softmmu/hw/virtio/vhost-backend.o
CC aarch64-softmmu/hw/virtio/vhost-user.o
CC aarch64-softmmu/hw/arm/boot.o
CC aarch64-softmmu/hw/virtio/vhost-vsock.o
CC aarch64-softmmu/hw/arm/exynos4_boards.o
CC aarch64-softmmu/hw/arm/collie.o
CC aarch64-softmmu/hw/arm/gumstix.o
CC aarch64-softmmu/hw/arm/highbank.o
CC aarch64-softmmu/hw/arm/digic_boards.o
CC aarch64-softmmu/hw/arm/integratorcp.o
CC aarch64-softmmu/hw/arm/mainstone.o
CC aarch64-softmmu/hw/arm/musicpal.o
CC aarch64-softmmu/hw/arm/nseries.o
CC aarch64-softmmu/hw/arm/omap_sx1.o
CC aarch64-softmmu/hw/arm/palm.o
CC aarch64-softmmu/hw/arm/realview.o
CC aarch64-softmmu/hw/arm/spitz.o
CC aarch64-softmmu/hw/arm/stellaris.o
CC aarch64-softmmu/hw/arm/tosa.o
CC aarch64-softmmu/hw/arm/versatilepb.o
CC aarch64-softmmu/hw/arm/vexpress.o
CC aarch64-softmmu/hw/arm/virt.o
CC aarch64-softmmu/hw/arm/xilinx_zynq.o
CC aarch64-softmmu/hw/arm/z2.o
CC aarch64-softmmu/hw/arm/virt-acpi-build.o
CC aarch64-softmmu/hw/arm/netduino2.o
CC aarch64-softmmu/hw/arm/sysbus-fdt.o
CC aarch64-softmmu/hw/arm/armv7m.o
CC aarch64-softmmu/hw/arm/exynos4210.o
CC aarch64-softmmu/hw/arm/pxa2xx.o
CC aarch64-softmmu/hw/arm/pxa2xx_gpio.o
CC aarch64-softmmu/hw/arm/pxa2xx_pic.o
CC aarch64-softmmu/hw/arm/digic.o
CC aarch64-softmmu/hw/arm/omap1.o
CC aarch64-softmmu/hw/arm/omap2.o
CC aarch64-softmmu/hw/arm/strongarm.o
CC aarch64-softmmu/hw/arm/allwinner-a10.o
CC aarch64-softmmu/hw/arm/cubieboard.o
CC aarch64-softmmu/hw/arm/bcm2835_peripherals.o
CC aarch64-softmmu/hw/arm/bcm2836.o
CC aarch64-softmmu/hw/arm/raspi.o
CC aarch64-softmmu/hw/arm/stm32f205_soc.o
CC aarch64-softmmu/hw/arm/xlnx-zynqmp.o
CC aarch64-softmmu/hw/arm/xlnx-ep108.o
CC aarch64-softmmu/hw/arm/fsl-imx25.o
CC aarch64-softmmu/hw/arm/imx25_pdk.o
CC aarch64-softmmu/hw/arm/fsl-imx31.o
CC aarch64-softmmu/hw/arm/fsl-imx6.o
CC aarch64-softmmu/hw/arm/kzm.o
CC aarch64-softmmu/hw/arm/sabrelite.o
/tmp/qemu-test/src/hw/i386/acpi-build.c: In function ‘build_append_pci_bus_devices’:
/tmp/qemu-test/src/hw/i386/acpi-build.c:502: warning: ‘notify_method’ may be used uninitialized in this function
CC aarch64-softmmu/hw/arm/aspeed_soc.o
CC aarch64-softmmu/hw/arm/aspeed.o
CC aarch64-softmmu/target-arm/arm-semi.o
CC aarch64-softmmu/target-arm/machine.o
CC aarch64-softmmu/target-arm/psci.o
CC aarch64-softmmu/target-arm/arch_dump.o
CC aarch64-softmmu/target-arm/monitor.o
CC aarch64-softmmu/target-arm/kvm-stub.o
CC aarch64-softmmu/target-arm/translate.o
CC aarch64-softmmu/target-arm/op_helper.o
CC aarch64-softmmu/target-arm/helper.o
CC aarch64-softmmu/target-arm/cpu.o
CC aarch64-softmmu/target-arm/neon_helper.o
CC aarch64-softmmu/target-arm/iwmmxt_helper.o
CC aarch64-softmmu/target-arm/gdbstub.o
CC aarch64-softmmu/target-arm/cpu64.o
CC aarch64-softmmu/target-arm/translate-a64.o
CC aarch64-softmmu/target-arm/helper-a64.o
CC aarch64-softmmu/target-arm/gdbstub64.o
CC aarch64-softmmu/target-arm/arm-powerctl.o
CC aarch64-softmmu/target-arm/crypto_helper.o
make: *** [subdir-x86_64-softmmu] Error 2
make: *** Waiting for unfinished jobs....
/tmp/qemu-test/src/target-arm/translate-a64.c: In function ‘handle_shri_with_rndacc’:
/tmp/qemu-test/src/target-arm/translate-a64.c:6333: warning: ‘tcg_src_hi’ may be used uninitialized in this function
/tmp/qemu-test/src/target-arm/translate-a64.c: In function ‘disas_simd_scalar_two_reg_misc’:
/tmp/qemu-test/src/target-arm/translate-a64.c:8060: warning: ‘rmode’ may be used uninitialized in this function
GEN trace/generated-helpers.c
CC aarch64-softmmu/trace/control-target.o
CC aarch64-softmmu/gdbstub-xml.o
CC aarch64-softmmu/trace/generated-helpers.o
LINK aarch64-softmmu/qemu-system-aarch64
tests/docker/Makefile.include:107: recipe for target 'docker-run-test-quick@centos6' failed
make: *** [docker-run-test-quick@centos6] Error 2
=== OUTPUT END ===
Test command exited with code: 2
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 68+ messages in thread* Re: [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode
2016-10-14 4:05 ` [Qemu-devel] [PATCH v3 00/13] pc: q35: x2APIC support in kvm_apic mode no-reply
@ 2016-10-14 7:59 ` Igor Mammedov
0 siblings, 0 replies; 68+ messages in thread
From: Igor Mammedov @ 2016-10-14 7:59 UTC (permalink / raw)
To: no-reply
Cc: famz, qemu-devel, ehabkost, liuxiaojian6, mst, rkrcmar, peterx,
kevin, kraxel, pbonzini, lersek, chao.gao
On Thu, 13 Oct 2016 21:05:54 -0700 (PDT)
no-reply@ec2-52-6-146-230.compute-1.amazonaws.com wrote:
> Hi,
>
> Your series failed automatic build test. Please find the testing commands and
> their output below. If you have docker installed, you can probably reproduce it
> locally.
[..]
> /tmp/qemu-test/src/hw/i386/pc.c: In function ‘pc_machine_done’:
> /tmp/qemu-test/src/hw/i386/pc.c:1273: error: ‘IntelIOMMUState’ has no member named ‘intr_eim’
> make[1]: *** [hw/i386/pc.o] Error 1
That's due to dependency on not yet merged EIM fixes series
^ permalink raw reply [flat|nested] 68+ messages in thread