* [PATCH v2 01/13] acpi: make build_madt() more generic.
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
@ 2020-05-05 13:42 ` Gerd Hoffmann
2020-05-05 13:54 ` Igor Mammedov
2020-05-05 13:42 ` [PATCH v2 02/13] acpi: create acpi-common.c and move madt code Gerd Hoffmann
` (12 subsequent siblings)
13 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
Remove PCMachineState dependency from build_madt().
Pass AcpiDeviceIf as separate argument instead of
depending on PCMachineState->acpi_dev.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/acpi-build.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 765409a90eb6..fe60c10201ad 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -366,14 +366,13 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
}
static void
-build_madt(GArray *table_data, BIOSLinker *linker, PCMachineState *pcms)
+build_madt(GArray *table_data, BIOSLinker *linker,
+ X86MachineState *x86ms, AcpiDeviceIf *adev)
{
- MachineClass *mc = MACHINE_GET_CLASS(pcms);
- X86MachineState *x86ms = X86_MACHINE(pcms);
- const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(pcms));
+ MachineClass *mc = MACHINE_GET_CLASS(x86ms);
+ const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
int madt_start = table_data->len;
- AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(pcms->acpi_dev);
- AcpiDeviceIf *adev = ACPI_DEVICE_IF(pcms->acpi_dev);
+ AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
bool x2apic_mode = false;
AcpiMultipleApicTable *madt;
@@ -2561,7 +2560,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
aml_len += tables_blob->len - fadt;
acpi_add_table(table_offsets, tables_blob);
- build_madt(tables_blob, tables->linker, pcms);
+ build_madt(tables_blob, tables->linker, x86ms,
+ ACPI_DEVICE_IF(pcms->acpi_dev));
vmgenid_dev = find_vmgenid_dev();
if (vmgenid_dev) {
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 01/13] acpi: make build_madt() more generic.
2020-05-05 13:42 ` [PATCH v2 01/13] acpi: make build_madt() more generic Gerd Hoffmann
@ 2020-05-05 13:54 ` Igor Mammedov
0 siblings, 0 replies; 43+ messages in thread
From: Igor Mammedov @ 2020-05-05 13:54 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, 5 May 2020 15:42:53 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Remove PCMachineState dependency from build_madt().
> Pass AcpiDeviceIf as separate argument instead of
> depending on PCMachineState->acpi_dev.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/acpi-build.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 765409a90eb6..fe60c10201ad 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -366,14 +366,13 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> }
>
> static void
> -build_madt(GArray *table_data, BIOSLinker *linker, PCMachineState *pcms)
> +build_madt(GArray *table_data, BIOSLinker *linker,
> + X86MachineState *x86ms, AcpiDeviceIf *adev)
> {
> - MachineClass *mc = MACHINE_GET_CLASS(pcms);
> - X86MachineState *x86ms = X86_MACHINE(pcms);
> - const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(pcms));
> + MachineClass *mc = MACHINE_GET_CLASS(x86ms);
> + const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
> int madt_start = table_data->len;
> - AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(pcms->acpi_dev);
> - AcpiDeviceIf *adev = ACPI_DEVICE_IF(pcms->acpi_dev);
> + AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
> bool x2apic_mode = false;
>
> AcpiMultipleApicTable *madt;
> @@ -2561,7 +2560,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> aml_len += tables_blob->len - fadt;
>
> acpi_add_table(table_offsets, tables_blob);
> - build_madt(tables_blob, tables->linker, pcms);
> + build_madt(tables_blob, tables->linker, x86ms,
> + ACPI_DEVICE_IF(pcms->acpi_dev));
>
> vmgenid_dev = find_vmgenid_dev();
> if (vmgenid_dev) {
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 02/13] acpi: create acpi-common.c and move madt code
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
2020-05-05 13:42 ` [PATCH v2 01/13] acpi: make build_madt() more generic Gerd Hoffmann
@ 2020-05-05 13:42 ` Gerd Hoffmann
2020-05-05 14:00 ` Igor Mammedov
2020-05-05 13:42 ` [PATCH v2 03/13] acpi: madt: skip pci override on pci-less systems (microvm) Gerd Hoffmann
` (11 subsequent siblings)
13 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/i386/acpi-common.h | 14 ++++
hw/i386/acpi-build.c | 126 +---------------------------------
hw/i386/acpi-common.c | 152 ++++++++++++++++++++++++++++++++++++++++++
hw/i386/Makefile.objs | 1 +
4 files changed, 170 insertions(+), 123 deletions(-)
create mode 100644 hw/i386/acpi-common.h
create mode 100644 hw/i386/acpi-common.c
diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
new file mode 100644
index 000000000000..c30e461f1854
--- /dev/null
+++ b/hw/i386/acpi-common.h
@@ -0,0 +1,14 @@
+#ifndef HW_I386_ACPI_COMMON_H
+#define HW_I386_ACPI_COMMON_H
+#include "include/hw/acpi/acpi_dev_interface.h"
+
+#include "include/hw/acpi/bios-linker-loader.h"
+#include "include/hw/i386/x86.h"
+
+/* Default IOAPIC ID */
+#define ACPI_BUILD_IOAPIC_ID 0x0
+
+void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
+ X86MachineState *x86ms, AcpiDeviceIf *adev);
+
+#endif
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index fe60c10201ad..eb530e5cd56d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -24,6 +24,7 @@
#include "qapi/error.h"
#include "qapi/qmp/qnum.h"
#include "acpi-build.h"
+#include "acpi-common.h"
#include "qemu/bitmap.h"
#include "qemu/error-report.h"
#include "hw/pci/pci.h"
@@ -89,9 +90,6 @@
#define ACPI_BUILD_DPRINTF(fmt, ...)
#endif
-/* Default IOAPIC ID */
-#define ACPI_BUILD_IOAPIC_ID 0x0
-
typedef struct AcpiPmInfo {
bool s3_disabled;
bool s4_disabled;
@@ -327,124 +325,6 @@ build_facs(GArray *table_data)
facs->length = cpu_to_le32(sizeof(*facs));
}
-void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
- const CPUArchIdList *apic_ids, GArray *entry)
-{
- 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 {
- AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic);
-
- apic->type = ACPI_APIC_LOCAL_X2APIC;
- apic->length = sizeof(*apic);
- apic->uid = cpu_to_le32(uid);
- apic->x2apic_id = cpu_to_le32(apic_id);
- if (apic_ids->cpus[uid].cpu != NULL) {
- apic->flags = cpu_to_le32(1);
- } else {
- apic->flags = cpu_to_le32(0);
- }
- }
-}
-
-static void
-build_madt(GArray *table_data, BIOSLinker *linker,
- X86MachineState *x86ms, AcpiDeviceIf *adev)
-{
- MachineClass *mc = MACHINE_GET_CLASS(x86ms);
- const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
- int madt_start = table_data->len;
- AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
- bool x2apic_mode = false;
-
- AcpiMultipleApicTable *madt;
- AcpiMadtIoApic *io_apic;
- AcpiMadtIntsrcovr *intsrcovr;
- int i;
-
- madt = acpi_data_push(table_data, sizeof *madt);
- madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS);
- madt->flags = cpu_to_le32(1);
-
- 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;
- }
- }
-
- io_apic = acpi_data_push(table_data, sizeof *io_apic);
- io_apic->type = ACPI_APIC_IO;
- io_apic->length = sizeof(*io_apic);
- io_apic->io_apic_id = ACPI_BUILD_IOAPIC_ID;
- io_apic->address = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS);
- io_apic->interrupt = cpu_to_le32(0);
-
- if (x86ms->apic_xrupt_override) {
- intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
- intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
- intsrcovr->length = sizeof(*intsrcovr);
- intsrcovr->source = 0;
- intsrcovr->gsi = cpu_to_le32(2);
- intsrcovr->flags = cpu_to_le16(0); /* conforms to bus specifications */
- }
- for (i = 1; i < 16; i++) {
-#define ACPI_BUILD_PCI_IRQS ((1<<5) | (1<<9) | (1<<10) | (1<<11))
- if (!(ACPI_BUILD_PCI_IRQS & (1 << i))) {
- /* No need for a INT source override structure. */
- continue;
- }
- intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
- intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
- intsrcovr->length = sizeof(*intsrcovr);
- intsrcovr->source = i;
- intsrcovr->gsi = cpu_to_le32(i);
- intsrcovr->flags = cpu_to_le16(0xd); /* active high, level triggered */
- }
-
- 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",
- table_data->len - madt_start, 1, NULL, NULL);
-}
-
static void build_append_pcihp_notify_entry(Aml *method, int slot)
{
Aml *if_ctx;
@@ -2560,8 +2440,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
aml_len += tables_blob->len - fadt;
acpi_add_table(table_offsets, tables_blob);
- build_madt(tables_blob, tables->linker, x86ms,
- ACPI_DEVICE_IF(pcms->acpi_dev));
+ acpi_build_madt(tables_blob, tables->linker, x86ms,
+ ACPI_DEVICE_IF(pcms->acpi_dev));
vmgenid_dev = find_vmgenid_dev();
if (vmgenid_dev) {
diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
new file mode 100644
index 000000000000..5caca16a0b59
--- /dev/null
+++ b/hw/i386/acpi-common.c
@@ -0,0 +1,152 @@
+/* Support for generating ACPI tables and passing them to Guests
+ *
+ * Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
+ * Copyright (C) 2006 Fabrice Bellard
+ * Copyright (C) 2013 Red Hat Inc
+ *
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+
+#include "exec/memory.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/utils.h"
+#include "hw/i386/pc.h"
+#include "target/i386/cpu.h"
+
+#include "acpi-build.h"
+#include "acpi-common.h"
+
+void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
+ const CPUArchIdList *apic_ids, GArray *entry)
+{
+ 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 {
+ AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic);
+
+ apic->type = ACPI_APIC_LOCAL_X2APIC;
+ apic->length = sizeof(*apic);
+ apic->uid = cpu_to_le32(uid);
+ apic->x2apic_id = cpu_to_le32(apic_id);
+ if (apic_ids->cpus[uid].cpu != NULL) {
+ apic->flags = cpu_to_le32(1);
+ } else {
+ apic->flags = cpu_to_le32(0);
+ }
+ }
+}
+
+void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
+ X86MachineState *x86ms, AcpiDeviceIf *adev)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(x86ms);
+ const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
+ int madt_start = table_data->len;
+ AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
+ bool x2apic_mode = false;
+
+ AcpiMultipleApicTable *madt;
+ AcpiMadtIoApic *io_apic;
+ AcpiMadtIntsrcovr *intsrcovr;
+ int i;
+
+ madt = acpi_data_push(table_data, sizeof *madt);
+ madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS);
+ madt->flags = cpu_to_le32(1);
+
+ 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;
+ }
+ }
+
+ io_apic = acpi_data_push(table_data, sizeof *io_apic);
+ io_apic->type = ACPI_APIC_IO;
+ io_apic->length = sizeof(*io_apic);
+ io_apic->io_apic_id = ACPI_BUILD_IOAPIC_ID;
+ io_apic->address = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS);
+ io_apic->interrupt = cpu_to_le32(0);
+
+ if (x86ms->apic_xrupt_override) {
+ intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
+ intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
+ intsrcovr->length = sizeof(*intsrcovr);
+ intsrcovr->source = 0;
+ intsrcovr->gsi = cpu_to_le32(2);
+ intsrcovr->flags = cpu_to_le16(0); /* conforms to bus specifications */
+ }
+ for (i = 1; i < 16; i++) {
+#define ACPI_BUILD_PCI_IRQS ((1<<5) | (1<<9) | (1<<10) | (1<<11))
+ if (!(ACPI_BUILD_PCI_IRQS & (1 << i))) {
+ /* No need for a INT source override structure. */
+ continue;
+ }
+ intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
+ intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
+ intsrcovr->length = sizeof(*intsrcovr);
+ intsrcovr->source = i;
+ intsrcovr->gsi = cpu_to_le32(i);
+ intsrcovr->flags = cpu_to_le16(0xd); /* active high, level triggered */
+ }
+
+ 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",
+ table_data->len - madt_start, 1, NULL, NULL);
+}
+
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 8ce1b265335b..6abc74551a72 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -16,4 +16,5 @@ obj-$(CONFIG_VMMOUSE) += vmmouse.o
obj-$(CONFIG_PC) += port92.o
obj-y += kvmvapic.o
+obj-$(CONFIG_ACPI) += acpi-common.o
obj-$(CONFIG_PC) += acpi-build.o
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 02/13] acpi: create acpi-common.c and move madt code
2020-05-05 13:42 ` [PATCH v2 02/13] acpi: create acpi-common.c and move madt code Gerd Hoffmann
@ 2020-05-05 14:00 ` Igor Mammedov
0 siblings, 0 replies; 43+ messages in thread
From: Igor Mammedov @ 2020-05-05 14:00 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, 5 May 2020 15:42:54 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
maybe add here that's it's going to be reused by microvm, hence the movement
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/acpi-common.h | 14 ++++
> hw/i386/acpi-build.c | 126 +---------------------------------
> hw/i386/acpi-common.c | 152 ++++++++++++++++++++++++++++++++++++++++++
> hw/i386/Makefile.objs | 1 +
> 4 files changed, 170 insertions(+), 123 deletions(-)
> create mode 100644 hw/i386/acpi-common.h
> create mode 100644 hw/i386/acpi-common.c
>
> diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
> new file mode 100644
> index 000000000000..c30e461f1854
> --- /dev/null
> +++ b/hw/i386/acpi-common.h
> @@ -0,0 +1,14 @@
> +#ifndef HW_I386_ACPI_COMMON_H
> +#define HW_I386_ACPI_COMMON_H
> +#include "include/hw/acpi/acpi_dev_interface.h"
> +
> +#include "include/hw/acpi/bios-linker-loader.h"
> +#include "include/hw/i386/x86.h"
> +
> +/* Default IOAPIC ID */
> +#define ACPI_BUILD_IOAPIC_ID 0x0
> +
> +void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> + X86MachineState *x86ms, AcpiDeviceIf *adev);
> +
> +#endif
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index fe60c10201ad..eb530e5cd56d 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -24,6 +24,7 @@
> #include "qapi/error.h"
> #include "qapi/qmp/qnum.h"
> #include "acpi-build.h"
> +#include "acpi-common.h"
> #include "qemu/bitmap.h"
> #include "qemu/error-report.h"
> #include "hw/pci/pci.h"
> @@ -89,9 +90,6 @@
> #define ACPI_BUILD_DPRINTF(fmt, ...)
> #endif
>
> -/* Default IOAPIC ID */
> -#define ACPI_BUILD_IOAPIC_ID 0x0
> -
> typedef struct AcpiPmInfo {
> bool s3_disabled;
> bool s4_disabled;
> @@ -327,124 +325,6 @@ build_facs(GArray *table_data)
> facs->length = cpu_to_le32(sizeof(*facs));
> }
>
> -void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> - const CPUArchIdList *apic_ids, GArray *entry)
> -{
> - 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 {
> - AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic);
> -
> - apic->type = ACPI_APIC_LOCAL_X2APIC;
> - apic->length = sizeof(*apic);
> - apic->uid = cpu_to_le32(uid);
> - apic->x2apic_id = cpu_to_le32(apic_id);
> - if (apic_ids->cpus[uid].cpu != NULL) {
> - apic->flags = cpu_to_le32(1);
> - } else {
> - apic->flags = cpu_to_le32(0);
> - }
> - }
> -}
> -
> -static void
> -build_madt(GArray *table_data, BIOSLinker *linker,
> - X86MachineState *x86ms, AcpiDeviceIf *adev)
> -{
> - MachineClass *mc = MACHINE_GET_CLASS(x86ms);
> - const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
> - int madt_start = table_data->len;
> - AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
> - bool x2apic_mode = false;
> -
> - AcpiMultipleApicTable *madt;
> - AcpiMadtIoApic *io_apic;
> - AcpiMadtIntsrcovr *intsrcovr;
> - int i;
> -
> - madt = acpi_data_push(table_data, sizeof *madt);
> - madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS);
> - madt->flags = cpu_to_le32(1);
> -
> - 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;
> - }
> - }
> -
> - io_apic = acpi_data_push(table_data, sizeof *io_apic);
> - io_apic->type = ACPI_APIC_IO;
> - io_apic->length = sizeof(*io_apic);
> - io_apic->io_apic_id = ACPI_BUILD_IOAPIC_ID;
> - io_apic->address = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS);
> - io_apic->interrupt = cpu_to_le32(0);
> -
> - if (x86ms->apic_xrupt_override) {
> - intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
> - intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
> - intsrcovr->length = sizeof(*intsrcovr);
> - intsrcovr->source = 0;
> - intsrcovr->gsi = cpu_to_le32(2);
> - intsrcovr->flags = cpu_to_le16(0); /* conforms to bus specifications */
> - }
> - for (i = 1; i < 16; i++) {
> -#define ACPI_BUILD_PCI_IRQS ((1<<5) | (1<<9) | (1<<10) | (1<<11))
> - if (!(ACPI_BUILD_PCI_IRQS & (1 << i))) {
> - /* No need for a INT source override structure. */
> - continue;
> - }
> - intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
> - intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
> - intsrcovr->length = sizeof(*intsrcovr);
> - intsrcovr->source = i;
> - intsrcovr->gsi = cpu_to_le32(i);
> - intsrcovr->flags = cpu_to_le16(0xd); /* active high, level triggered */
> - }
> -
> - 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",
> - table_data->len - madt_start, 1, NULL, NULL);
> -}
> -
> static void build_append_pcihp_notify_entry(Aml *method, int slot)
> {
> Aml *if_ctx;
> @@ -2560,8 +2440,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> aml_len += tables_blob->len - fadt;
>
> acpi_add_table(table_offsets, tables_blob);
> - build_madt(tables_blob, tables->linker, x86ms,
> - ACPI_DEVICE_IF(pcms->acpi_dev));
> + acpi_build_madt(tables_blob, tables->linker, x86ms,
> + ACPI_DEVICE_IF(pcms->acpi_dev));
>
> vmgenid_dev = find_vmgenid_dev();
> if (vmgenid_dev) {
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> new file mode 100644
> index 000000000000..5caca16a0b59
> --- /dev/null
> +++ b/hw/i386/acpi-common.c
> @@ -0,0 +1,152 @@
> +/* Support for generating ACPI tables and passing them to Guests
> + *
> + * Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
> + * Copyright (C) 2006 Fabrice Bellard
> + * Copyright (C) 2013 Red Hat Inc
> + *
> + * Author: Michael S. Tsirkin <mst@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +
> +#include "exec/memory.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/aml-build.h"
> +#include "hw/acpi/utils.h"
> +#include "hw/i386/pc.h"
> +#include "target/i386/cpu.h"
> +
> +#include "acpi-build.h"
> +#include "acpi-common.h"
> +
> +void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> + const CPUArchIdList *apic_ids, GArray *entry)
> +{
> + 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 {
> + AcpiMadtProcessorX2Apic *apic = acpi_data_push(entry, sizeof *apic);
> +
> + apic->type = ACPI_APIC_LOCAL_X2APIC;
> + apic->length = sizeof(*apic);
> + apic->uid = cpu_to_le32(uid);
> + apic->x2apic_id = cpu_to_le32(apic_id);
> + if (apic_ids->cpus[uid].cpu != NULL) {
> + apic->flags = cpu_to_le32(1);
> + } else {
> + apic->flags = cpu_to_le32(0);
> + }
> + }
> +}
> +
> +void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> + X86MachineState *x86ms, AcpiDeviceIf *adev)
> +{
> + MachineClass *mc = MACHINE_GET_CLASS(x86ms);
> + const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
> + int madt_start = table_data->len;
> + AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(adev);
> + bool x2apic_mode = false;
> +
> + AcpiMultipleApicTable *madt;
> + AcpiMadtIoApic *io_apic;
> + AcpiMadtIntsrcovr *intsrcovr;
> + int i;
> +
> + madt = acpi_data_push(table_data, sizeof *madt);
> + madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS);
> + madt->flags = cpu_to_le32(1);
> +
> + 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;
> + }
> + }
> +
> + io_apic = acpi_data_push(table_data, sizeof *io_apic);
> + io_apic->type = ACPI_APIC_IO;
> + io_apic->length = sizeof(*io_apic);
> + io_apic->io_apic_id = ACPI_BUILD_IOAPIC_ID;
> + io_apic->address = cpu_to_le32(IO_APIC_DEFAULT_ADDRESS);
> + io_apic->interrupt = cpu_to_le32(0);
> +
> + if (x86ms->apic_xrupt_override) {
> + intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
> + intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
> + intsrcovr->length = sizeof(*intsrcovr);
> + intsrcovr->source = 0;
> + intsrcovr->gsi = cpu_to_le32(2);
> + intsrcovr->flags = cpu_to_le16(0); /* conforms to bus specifications */
> + }
> + for (i = 1; i < 16; i++) {
> +#define ACPI_BUILD_PCI_IRQS ((1<<5) | (1<<9) | (1<<10) | (1<<11))
> + if (!(ACPI_BUILD_PCI_IRQS & (1 << i))) {
> + /* No need for a INT source override structure. */
> + continue;
> + }
> + intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
> + intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
> + intsrcovr->length = sizeof(*intsrcovr);
> + intsrcovr->source = i;
> + intsrcovr->gsi = cpu_to_le32(i);
> + intsrcovr->flags = cpu_to_le16(0xd); /* active high, level triggered */
> + }
> +
> + 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",
> + table_data->len - madt_start, 1, NULL, NULL);
> +}
> +
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index 8ce1b265335b..6abc74551a72 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -16,4 +16,5 @@ obj-$(CONFIG_VMMOUSE) += vmmouse.o
> obj-$(CONFIG_PC) += port92.o
>
> obj-y += kvmvapic.o
> +obj-$(CONFIG_ACPI) += acpi-common.o
> obj-$(CONFIG_PC) += acpi-build.o
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 03/13] acpi: madt: skip pci override on pci-less systems (microvm)
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
2020-05-05 13:42 ` [PATCH v2 01/13] acpi: make build_madt() more generic Gerd Hoffmann
2020-05-05 13:42 ` [PATCH v2 02/13] acpi: create acpi-common.c and move madt code Gerd Hoffmann
@ 2020-05-05 13:42 ` Gerd Hoffmann
2020-05-05 14:10 ` Igor Mammedov
2020-05-05 14:17 ` Philippe Mathieu-Daudé
2020-05-05 13:42 ` [PATCH v2 04/13] acpi: move acpi_build_facs to acpi-common.c Gerd Hoffmann
` (10 subsequent siblings)
13 siblings, 2 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/i386/acpi-common.h | 3 ++-
hw/i386/acpi-build.c | 2 +-
hw/i386/acpi-common.c | 26 +++++++++++++++-----------
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
index c30e461f1854..9cac18dddf5b 100644
--- a/hw/i386/acpi-common.h
+++ b/hw/i386/acpi-common.h
@@ -9,6 +9,7 @@
#define ACPI_BUILD_IOAPIC_ID 0x0
void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
- X86MachineState *x86ms, AcpiDeviceIf *adev);
+ X86MachineState *x86ms, AcpiDeviceIf *adev,
+ bool has_pci);
#endif
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index eb530e5cd56d..4cce2192eeb0 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2441,7 +2441,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
acpi_add_table(table_offsets, tables_blob);
acpi_build_madt(tables_blob, tables->linker, x86ms,
- ACPI_DEVICE_IF(pcms->acpi_dev));
+ ACPI_DEVICE_IF(pcms->acpi_dev), true);
vmgenid_dev = find_vmgenid_dev();
if (vmgenid_dev) {
diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 5caca16a0b59..ab9b00581a15 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -72,7 +72,8 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
}
void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
- X86MachineState *x86ms, AcpiDeviceIf *adev)
+ X86MachineState *x86ms, AcpiDeviceIf *adev,
+ bool has_pci)
{
MachineClass *mc = MACHINE_GET_CLASS(x86ms);
const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
@@ -111,18 +112,21 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
intsrcovr->gsi = cpu_to_le32(2);
intsrcovr->flags = cpu_to_le16(0); /* conforms to bus specifications */
}
- for (i = 1; i < 16; i++) {
+
+ if (has_pci) {
+ for (i = 1; i < 16; i++) {
#define ACPI_BUILD_PCI_IRQS ((1<<5) | (1<<9) | (1<<10) | (1<<11))
- if (!(ACPI_BUILD_PCI_IRQS & (1 << i))) {
- /* No need for a INT source override structure. */
- continue;
+ if (!(ACPI_BUILD_PCI_IRQS & (1 << i))) {
+ /* No need for a INT source override structure. */
+ continue;
+ }
+ intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
+ intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
+ intsrcovr->length = sizeof(*intsrcovr);
+ intsrcovr->source = i;
+ intsrcovr->gsi = cpu_to_le32(i);
+ intsrcovr->flags = cpu_to_le16(0xd); /* active high, level triggered */
}
- intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
- intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
- intsrcovr->length = sizeof(*intsrcovr);
- intsrcovr->source = i;
- intsrcovr->gsi = cpu_to_le32(i);
- intsrcovr->flags = cpu_to_le16(0xd); /* active high, level triggered */
}
if (x2apic_mode) {
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 03/13] acpi: madt: skip pci override on pci-less systems (microvm)
2020-05-05 13:42 ` [PATCH v2 03/13] acpi: madt: skip pci override on pci-less systems (microvm) Gerd Hoffmann
@ 2020-05-05 14:10 ` Igor Mammedov
2020-05-05 14:17 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 43+ messages in thread
From: Igor Mammedov @ 2020-05-05 14:10 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, 5 May 2020 15:42:55 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/acpi-common.h | 3 ++-
> hw/i386/acpi-build.c | 2 +-
> hw/i386/acpi-common.c | 26 +++++++++++++++-----------
> 3 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
> index c30e461f1854..9cac18dddf5b 100644
> --- a/hw/i386/acpi-common.h
> +++ b/hw/i386/acpi-common.h
> @@ -9,6 +9,7 @@
> #define ACPI_BUILD_IOAPIC_ID 0x0
>
> void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> - X86MachineState *x86ms, AcpiDeviceIf *adev);
> + X86MachineState *x86ms, AcpiDeviceIf *adev,
> + bool has_pci);
>
> #endif
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index eb530e5cd56d..4cce2192eeb0 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2441,7 +2441,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>
> acpi_add_table(table_offsets, tables_blob);
> acpi_build_madt(tables_blob, tables->linker, x86ms,
> - ACPI_DEVICE_IF(pcms->acpi_dev));
> + ACPI_DEVICE_IF(pcms->acpi_dev), true);
>
> vmgenid_dev = find_vmgenid_dev();
> if (vmgenid_dev) {
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> index 5caca16a0b59..ab9b00581a15 100644
> --- a/hw/i386/acpi-common.c
> +++ b/hw/i386/acpi-common.c
> @@ -72,7 +72,8 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> }
>
> void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> - X86MachineState *x86ms, AcpiDeviceIf *adev)
> + X86MachineState *x86ms, AcpiDeviceIf *adev,
> + bool has_pci)
> {
> MachineClass *mc = MACHINE_GET_CLASS(x86ms);
> const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
> @@ -111,18 +112,21 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> intsrcovr->gsi = cpu_to_le32(2);
> intsrcovr->flags = cpu_to_le16(0); /* conforms to bus specifications */
> }
> - for (i = 1; i < 16; i++) {
> +
> + if (has_pci) {
> + for (i = 1; i < 16; i++) {
> #define ACPI_BUILD_PCI_IRQS ((1<<5) | (1<<9) | (1<<10) | (1<<11))
> - if (!(ACPI_BUILD_PCI_IRQS & (1 << i))) {
> - /* No need for a INT source override structure. */
> - continue;
> + if (!(ACPI_BUILD_PCI_IRQS & (1 << i))) {
> + /* No need for a INT source override structure. */
> + continue;
> + }
> + intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
> + intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
> + intsrcovr->length = sizeof(*intsrcovr);
> + intsrcovr->source = i;
> + intsrcovr->gsi = cpu_to_le32(i);
> + intsrcovr->flags = cpu_to_le16(0xd); /* active high, level triggered */
> }
> - intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
> - intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
> - intsrcovr->length = sizeof(*intsrcovr);
> - intsrcovr->source = i;
> - intsrcovr->gsi = cpu_to_le32(i);
> - intsrcovr->flags = cpu_to_le16(0xd); /* active high, level triggered */
> }
>
> if (x2apic_mode) {
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 03/13] acpi: madt: skip pci override on pci-less systems (microvm)
2020-05-05 13:42 ` [PATCH v2 03/13] acpi: madt: skip pci override on pci-less systems (microvm) Gerd Hoffmann
2020-05-05 14:10 ` Igor Mammedov
@ 2020-05-05 14:17 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 43+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-05 14:17 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Richard Henderson
On 5/5/20 3:42 PM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/i386/acpi-common.h | 3 ++-
> hw/i386/acpi-build.c | 2 +-
> hw/i386/acpi-common.c | 26 +++++++++++++++-----------
> 3 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
> index c30e461f1854..9cac18dddf5b 100644
> --- a/hw/i386/acpi-common.h
> +++ b/hw/i386/acpi-common.h
> @@ -9,6 +9,7 @@
> #define ACPI_BUILD_IOAPIC_ID 0x0
>
> void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> - X86MachineState *x86ms, AcpiDeviceIf *adev);
> + X86MachineState *x86ms, AcpiDeviceIf *adev,
> + bool has_pci);
>
> #endif
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index eb530e5cd56d..4cce2192eeb0 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2441,7 +2441,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>
> acpi_add_table(table_offsets, tables_blob);
> acpi_build_madt(tables_blob, tables->linker, x86ms,
> - ACPI_DEVICE_IF(pcms->acpi_dev));
> + ACPI_DEVICE_IF(pcms->acpi_dev), true);
>
> vmgenid_dev = find_vmgenid_dev();
> if (vmgenid_dev) {
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> index 5caca16a0b59..ab9b00581a15 100644
> --- a/hw/i386/acpi-common.c
> +++ b/hw/i386/acpi-common.c
> @@ -72,7 +72,8 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> }
>
> void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> - X86MachineState *x86ms, AcpiDeviceIf *adev)
> + X86MachineState *x86ms, AcpiDeviceIf *adev,
> + bool has_pci)
> {
> MachineClass *mc = MACHINE_GET_CLASS(x86ms);
> const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(MACHINE(x86ms));
> @@ -111,18 +112,21 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> intsrcovr->gsi = cpu_to_le32(2);
> intsrcovr->flags = cpu_to_le16(0); /* conforms to bus specifications */
> }
> - for (i = 1; i < 16; i++) {
> +
> + if (has_pci) {
> + for (i = 1; i < 16; i++) {
> #define ACPI_BUILD_PCI_IRQS ((1<<5) | (1<<9) | (1<<10) | (1<<11))
> - if (!(ACPI_BUILD_PCI_IRQS & (1 << i))) {
> - /* No need for a INT source override structure. */
> - continue;
> + if (!(ACPI_BUILD_PCI_IRQS & (1 << i))) {
> + /* No need for a INT source override structure. */
> + continue;
> + }
> + intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
> + intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
> + intsrcovr->length = sizeof(*intsrcovr);
> + intsrcovr->source = i;
> + intsrcovr->gsi = cpu_to_le32(i);
> + intsrcovr->flags = cpu_to_le16(0xd); /* active high, level triggered */
> }
> - intsrcovr = acpi_data_push(table_data, sizeof *intsrcovr);
> - intsrcovr->type = ACPI_APIC_XRUPT_OVERRIDE;
> - intsrcovr->length = sizeof(*intsrcovr);
> - intsrcovr->source = i;
> - intsrcovr->gsi = cpu_to_le32(i);
> - intsrcovr->flags = cpu_to_le16(0xd); /* active high, level triggered */
> }
>
> if (x2apic_mode) {
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 04/13] acpi: move acpi_build_facs to acpi-common.c
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
` (2 preceding siblings ...)
2020-05-05 13:42 ` [PATCH v2 03/13] acpi: madt: skip pci override on pci-less systems (microvm) Gerd Hoffmann
@ 2020-05-05 13:42 ` Gerd Hoffmann
2020-05-05 13:49 ` Philippe Mathieu-Daudé
2020-05-05 14:16 ` Igor Mammedov
2020-05-05 13:42 ` [PATCH v2 05/13] acpi: move acpi_init_common_fadt_data " Gerd Hoffmann
` (9 subsequent siblings)
13 siblings, 2 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/i386/acpi-common.h | 1 +
hw/i386/acpi-build.c | 11 +----------
hw/i386/acpi-common.c | 7 +++++++
3 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
index 9cac18dddf5b..583c320bbe7d 100644
--- a/hw/i386/acpi-common.h
+++ b/hw/i386/acpi-common.h
@@ -11,5 +11,6 @@
void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
X86MachineState *x86ms, AcpiDeviceIf *adev,
bool has_pci);
+void acpi_build_facs(GArray *table_data);
#endif
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4cce2192eeb0..a69b85a266e7 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -316,15 +316,6 @@ static void acpi_align_size(GArray *blob, unsigned align)
g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
}
-/* FACS */
-static void
-build_facs(GArray *table_data)
-{
- AcpiFacsDescriptorRev1 *facs = acpi_data_push(table_data, sizeof *facs);
- memcpy(&facs->signature, "FACS", 4);
- facs->length = cpu_to_le32(sizeof(*facs));
-}
-
static void build_append_pcihp_notify_entry(Aml *method, int slot)
{
Aml *if_ctx;
@@ -2417,7 +2408,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
* requirements.
*/
facs = tables_blob->len;
- build_facs(tables_blob);
+ acpi_build_facs(tables_blob);
/* DSDT is pointed to by FADT */
dsdt = tables_blob->len;
diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index ab9b00581a15..5187653893a8 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -154,3 +154,10 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
table_data->len - madt_start, 1, NULL, NULL);
}
+/* FACS */
+void acpi_build_facs(GArray *table_data)
+{
+ AcpiFacsDescriptorRev1 *facs = acpi_data_push(table_data, sizeof *facs);
+ memcpy(&facs->signature, "FACS", 4);
+ facs->length = cpu_to_le32(sizeof(*facs));
+}
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 04/13] acpi: move acpi_build_facs to acpi-common.c
2020-05-05 13:42 ` [PATCH v2 04/13] acpi: move acpi_build_facs to acpi-common.c Gerd Hoffmann
@ 2020-05-05 13:49 ` Philippe Mathieu-Daudé
2020-05-05 14:16 ` Igor Mammedov
1 sibling, 0 replies; 43+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-05 13:49 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Richard Henderson
On 5/5/20 3:42 PM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/i386/acpi-common.h | 1 +
> hw/i386/acpi-build.c | 11 +----------
> hw/i386/acpi-common.c | 7 +++++++
> 3 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
> index 9cac18dddf5b..583c320bbe7d 100644
> --- a/hw/i386/acpi-common.h
> +++ b/hw/i386/acpi-common.h
> @@ -11,5 +11,6 @@
> void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> X86MachineState *x86ms, AcpiDeviceIf *adev,
> bool has_pci);
> +void acpi_build_facs(GArray *table_data);
>
> #endif
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 4cce2192eeb0..a69b85a266e7 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -316,15 +316,6 @@ static void acpi_align_size(GArray *blob, unsigned align)
> g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
> }
>
> -/* FACS */
> -static void
> -build_facs(GArray *table_data)
> -{
> - AcpiFacsDescriptorRev1 *facs = acpi_data_push(table_data, sizeof *facs);
> - memcpy(&facs->signature, "FACS", 4);
> - facs->length = cpu_to_le32(sizeof(*facs));
> -}
> -
> static void build_append_pcihp_notify_entry(Aml *method, int slot)
> {
> Aml *if_ctx;
> @@ -2417,7 +2408,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> * requirements.
> */
> facs = tables_blob->len;
> - build_facs(tables_blob);
> + acpi_build_facs(tables_blob);
>
> /* DSDT is pointed to by FADT */
> dsdt = tables_blob->len;
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> index ab9b00581a15..5187653893a8 100644
> --- a/hw/i386/acpi-common.c
> +++ b/hw/i386/acpi-common.c
> @@ -154,3 +154,10 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> table_data->len - madt_start, 1, NULL, NULL);
> }
>
> +/* FACS */
> +void acpi_build_facs(GArray *table_data)
> +{
> + AcpiFacsDescriptorRev1 *facs = acpi_data_push(table_data, sizeof *facs);
> + memcpy(&facs->signature, "FACS", 4);
> + facs->length = cpu_to_le32(sizeof(*facs));
> +}
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 04/13] acpi: move acpi_build_facs to acpi-common.c
2020-05-05 13:42 ` [PATCH v2 04/13] acpi: move acpi_build_facs to acpi-common.c Gerd Hoffmann
2020-05-05 13:49 ` Philippe Mathieu-Daudé
@ 2020-05-05 14:16 ` Igor Mammedov
1 sibling, 0 replies; 43+ messages in thread
From: Igor Mammedov @ 2020-05-05 14:16 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, 5 May 2020 15:42:56 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
why do you need FACS with reduced profile?
> ---
> hw/i386/acpi-common.h | 1 +
> hw/i386/acpi-build.c | 11 +----------
> hw/i386/acpi-common.c | 7 +++++++
> 3 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
> index 9cac18dddf5b..583c320bbe7d 100644
> --- a/hw/i386/acpi-common.h
> +++ b/hw/i386/acpi-common.h
> @@ -11,5 +11,6 @@
> void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> X86MachineState *x86ms, AcpiDeviceIf *adev,
> bool has_pci);
> +void acpi_build_facs(GArray *table_data);
>
> #endif
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 4cce2192eeb0..a69b85a266e7 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -316,15 +316,6 @@ static void acpi_align_size(GArray *blob, unsigned align)
> g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
> }
>
> -/* FACS */
> -static void
> -build_facs(GArray *table_data)
> -{
> - AcpiFacsDescriptorRev1 *facs = acpi_data_push(table_data, sizeof *facs);
> - memcpy(&facs->signature, "FACS", 4);
> - facs->length = cpu_to_le32(sizeof(*facs));
> -}
> -
> static void build_append_pcihp_notify_entry(Aml *method, int slot)
> {
> Aml *if_ctx;
> @@ -2417,7 +2408,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
> * requirements.
> */
> facs = tables_blob->len;
> - build_facs(tables_blob);
> + acpi_build_facs(tables_blob);
>
> /* DSDT is pointed to by FADT */
> dsdt = tables_blob->len;
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> index ab9b00581a15..5187653893a8 100644
> --- a/hw/i386/acpi-common.c
> +++ b/hw/i386/acpi-common.c
> @@ -154,3 +154,10 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> table_data->len - madt_start, 1, NULL, NULL);
> }
>
> +/* FACS */
> +void acpi_build_facs(GArray *table_data)
> +{
> + AcpiFacsDescriptorRev1 *facs = acpi_data_push(table_data, sizeof *facs);
> + memcpy(&facs->signature, "FACS", 4);
> + facs->length = cpu_to_le32(sizeof(*facs));
> +}
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 05/13] acpi: move acpi_init_common_fadt_data to acpi-common.c
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
` (3 preceding siblings ...)
2020-05-05 13:42 ` [PATCH v2 04/13] acpi: move acpi_build_facs to acpi-common.c Gerd Hoffmann
@ 2020-05-05 13:42 ` Gerd Hoffmann
2020-05-05 14:25 ` Igor Mammedov
2020-05-05 13:42 ` [PATCH v2 06/13] acpi: move acpi_align_size to acpi-common.h Gerd Hoffmann
` (8 subsequent siblings)
13 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/i386/acpi-common.h | 5 ++++-
hw/i386/acpi-build.c | 43 +------------------------------------------
hw/i386/acpi-common.c | 43 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 48 insertions(+), 43 deletions(-)
diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
index 583c320bbe7d..5788a13da9ca 100644
--- a/hw/i386/acpi-common.h
+++ b/hw/i386/acpi-common.h
@@ -1,7 +1,8 @@
#ifndef HW_I386_ACPI_COMMON_H
#define HW_I386_ACPI_COMMON_H
+
+#include "include/hw/acpi/acpi-defs.h"
#include "include/hw/acpi/acpi_dev_interface.h"
-
#include "include/hw/acpi/bios-linker-loader.h"
#include "include/hw/i386/x86.h"
@@ -12,5 +13,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
X86MachineState *x86ms, AcpiDeviceIf *adev,
bool has_pci);
void acpi_build_facs(GArray *table_data);
+void acpi_init_common_fadt_data(MachineState *ms, Object *o,
+ AcpiFadtData *data);
#endif
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a69b85a266e7..d1f14394734e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -132,47 +132,6 @@ const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio = {
.bit_width = NVDIMM_ACPI_IO_LEN << 3
};
-static void init_common_fadt_data(MachineState *ms, Object *o,
- AcpiFadtData *data)
-{
- uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
- AmlAddressSpace as = AML_AS_SYSTEM_IO;
- AcpiFadtData fadt = {
- .rev = 3,
- .flags =
- (1 << ACPI_FADT_F_WBINVD) |
- (1 << ACPI_FADT_F_PROC_C1) |
- (1 << ACPI_FADT_F_SLP_BUTTON) |
- (1 << ACPI_FADT_F_RTC_S4) |
- (1 << ACPI_FADT_F_USE_PLATFORM_CLOCK) |
- /* APIC destination mode ("Flat Logical") has an upper limit of 8
- * CPUs for more than 8 CPUs, "Clustered Logical" mode has to be
- * used
- */
- ((ms->smp.max_cpus > 8) ?
- (1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL) : 0),
- .int_model = 1 /* Multiple APIC */,
- .rtc_century = RTC_CENTURY,
- .plvl2_lat = 0xfff /* C2 state not supported */,
- .plvl3_lat = 0xfff /* C3 state not supported */,
- .smi_cmd = ACPI_PORT_SMI_CMD,
- .sci_int = object_property_get_uint(o, ACPI_PM_PROP_SCI_INT, NULL),
- .acpi_enable_cmd =
- object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL),
- .acpi_disable_cmd =
- object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL),
- .pm1a_evt = { .space_id = as, .bit_width = 4 * 8, .address = io },
- .pm1a_cnt = { .space_id = as, .bit_width = 2 * 8,
- .address = io + 0x04 },
- .pm_tmr = { .space_id = as, .bit_width = 4 * 8, .address = io + 0x08 },
- .gpe0_blk = { .space_id = as, .bit_width =
- object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK_LEN, NULL) * 8,
- .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
- },
- };
- *data = fadt;
-}
-
static Object *object_resolve_type_unambiguous(const char *typename)
{
bool ambig;
@@ -195,7 +154,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
pm->pcihp_io_len = 0;
assert(obj);
- init_common_fadt_data(machine, obj, &pm->fadt);
+ acpi_init_common_fadt_data(machine, obj, &pm->fadt);
if (piix) {
/* w2k requires FADT(rev1) or it won't boot, keep PC compatible */
pm->fadt.rev = 1;
diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
index 5187653893a8..69dfbf0252f3 100644
--- a/hw/i386/acpi-common.c
+++ b/hw/i386/acpi-common.c
@@ -28,6 +28,8 @@
#include "hw/acpi/aml-build.h"
#include "hw/acpi/utils.h"
#include "hw/i386/pc.h"
+#include "hw/isa/apm.h"
+#include "hw/rtc/mc146818rtc_regs.h"
#include "target/i386/cpu.h"
#include "acpi-build.h"
@@ -161,3 +163,44 @@ void acpi_build_facs(GArray *table_data)
memcpy(&facs->signature, "FACS", 4);
facs->length = cpu_to_le32(sizeof(*facs));
}
+
+void acpi_init_common_fadt_data(MachineState *ms, Object *o,
+ AcpiFadtData *data)
+{
+ uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
+ AmlAddressSpace as = AML_AS_SYSTEM_IO;
+ AcpiFadtData fadt = {
+ .rev = 3,
+ .flags =
+ (1 << ACPI_FADT_F_WBINVD) |
+ (1 << ACPI_FADT_F_PROC_C1) |
+ (1 << ACPI_FADT_F_SLP_BUTTON) |
+ (1 << ACPI_FADT_F_RTC_S4) |
+ (1 << ACPI_FADT_F_USE_PLATFORM_CLOCK) |
+ /* APIC destination mode ("Flat Logical") has an upper limit of 8
+ * CPUs for more than 8 CPUs, "Clustered Logical" mode has to be
+ * used
+ */
+ ((ms->smp.max_cpus > 8) ?
+ (1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL) : 0),
+ .int_model = 1 /* Multiple APIC */,
+ .rtc_century = RTC_CENTURY,
+ .plvl2_lat = 0xfff /* C2 state not supported */,
+ .plvl3_lat = 0xfff /* C3 state not supported */,
+ .smi_cmd = ACPI_PORT_SMI_CMD,
+ .sci_int = object_property_get_uint(o, ACPI_PM_PROP_SCI_INT, NULL),
+ .acpi_enable_cmd =
+ object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL),
+ .acpi_disable_cmd =
+ object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL),
+ .pm1a_evt = { .space_id = as, .bit_width = 4 * 8, .address = io },
+ .pm1a_cnt = { .space_id = as, .bit_width = 2 * 8,
+ .address = io + 0x04 },
+ .pm_tmr = { .space_id = as, .bit_width = 4 * 8, .address = io + 0x08 },
+ .gpe0_blk = { .space_id = as, .bit_width =
+ object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK_LEN, NULL) * 8,
+ .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
+ },
+ };
+ *data = fadt;
+}
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 05/13] acpi: move acpi_init_common_fadt_data to acpi-common.c
2020-05-05 13:42 ` [PATCH v2 05/13] acpi: move acpi_init_common_fadt_data " Gerd Hoffmann
@ 2020-05-05 14:25 ` Igor Mammedov
2020-05-06 10:03 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Igor Mammedov @ 2020-05-05 14:25 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, 5 May 2020 15:42:57 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
the same question like with FACS, why legacy data are needed for with reduced profile?
it mostly initializes data for fixed-hw model.
I'd preffer if you drop FACS and use minimal FADT like build_fadt_rev5() does
without pulling along a bunch of legacy junk (unless there is a good justification for it).
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/i386/acpi-common.h | 5 ++++-
> hw/i386/acpi-build.c | 43 +------------------------------------------
> hw/i386/acpi-common.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 48 insertions(+), 43 deletions(-)
>
> diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
> index 583c320bbe7d..5788a13da9ca 100644
> --- a/hw/i386/acpi-common.h
> +++ b/hw/i386/acpi-common.h
> @@ -1,7 +1,8 @@
> #ifndef HW_I386_ACPI_COMMON_H
> #define HW_I386_ACPI_COMMON_H
> +
> +#include "include/hw/acpi/acpi-defs.h"
> #include "include/hw/acpi/acpi_dev_interface.h"
> -
> #include "include/hw/acpi/bios-linker-loader.h"
> #include "include/hw/i386/x86.h"
>
> @@ -12,5 +13,7 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> X86MachineState *x86ms, AcpiDeviceIf *adev,
> bool has_pci);
> void acpi_build_facs(GArray *table_data);
> +void acpi_init_common_fadt_data(MachineState *ms, Object *o,
> + AcpiFadtData *data);
>
> #endif
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index a69b85a266e7..d1f14394734e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -132,47 +132,6 @@ const struct AcpiGenericAddress x86_nvdimm_acpi_dsmio = {
> .bit_width = NVDIMM_ACPI_IO_LEN << 3
> };
>
> -static void init_common_fadt_data(MachineState *ms, Object *o,
> - AcpiFadtData *data)
> -{
> - uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
> - AmlAddressSpace as = AML_AS_SYSTEM_IO;
> - AcpiFadtData fadt = {
> - .rev = 3,
> - .flags =
> - (1 << ACPI_FADT_F_WBINVD) |
> - (1 << ACPI_FADT_F_PROC_C1) |
> - (1 << ACPI_FADT_F_SLP_BUTTON) |
> - (1 << ACPI_FADT_F_RTC_S4) |
> - (1 << ACPI_FADT_F_USE_PLATFORM_CLOCK) |
> - /* APIC destination mode ("Flat Logical") has an upper limit of 8
> - * CPUs for more than 8 CPUs, "Clustered Logical" mode has to be
> - * used
> - */
> - ((ms->smp.max_cpus > 8) ?
> - (1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL) : 0),
> - .int_model = 1 /* Multiple APIC */,
> - .rtc_century = RTC_CENTURY,
> - .plvl2_lat = 0xfff /* C2 state not supported */,
> - .plvl3_lat = 0xfff /* C3 state not supported */,
> - .smi_cmd = ACPI_PORT_SMI_CMD,
> - .sci_int = object_property_get_uint(o, ACPI_PM_PROP_SCI_INT, NULL),
> - .acpi_enable_cmd =
> - object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL),
> - .acpi_disable_cmd =
> - object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL),
> - .pm1a_evt = { .space_id = as, .bit_width = 4 * 8, .address = io },
> - .pm1a_cnt = { .space_id = as, .bit_width = 2 * 8,
> - .address = io + 0x04 },
> - .pm_tmr = { .space_id = as, .bit_width = 4 * 8, .address = io + 0x08 },
> - .gpe0_blk = { .space_id = as, .bit_width =
> - object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK_LEN, NULL) * 8,
> - .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
> - },
> - };
> - *data = fadt;
> -}
> -
> static Object *object_resolve_type_unambiguous(const char *typename)
> {
> bool ambig;
> @@ -195,7 +154,7 @@ static void acpi_get_pm_info(MachineState *machine, AcpiPmInfo *pm)
> pm->pcihp_io_len = 0;
>
> assert(obj);
> - init_common_fadt_data(machine, obj, &pm->fadt);
> + acpi_init_common_fadt_data(machine, obj, &pm->fadt);
> if (piix) {
> /* w2k requires FADT(rev1) or it won't boot, keep PC compatible */
> pm->fadt.rev = 1;
> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c
> index 5187653893a8..69dfbf0252f3 100644
> --- a/hw/i386/acpi-common.c
> +++ b/hw/i386/acpi-common.c
> @@ -28,6 +28,8 @@
> #include "hw/acpi/aml-build.h"
> #include "hw/acpi/utils.h"
> #include "hw/i386/pc.h"
> +#include "hw/isa/apm.h"
> +#include "hw/rtc/mc146818rtc_regs.h"
> #include "target/i386/cpu.h"
>
> #include "acpi-build.h"
> @@ -161,3 +163,44 @@ void acpi_build_facs(GArray *table_data)
> memcpy(&facs->signature, "FACS", 4);
> facs->length = cpu_to_le32(sizeof(*facs));
> }
> +
> +void acpi_init_common_fadt_data(MachineState *ms, Object *o,
> + AcpiFadtData *data)
> +{
> + uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
> + AmlAddressSpace as = AML_AS_SYSTEM_IO;
> + AcpiFadtData fadt = {
> + .rev = 3,
> + .flags =
> + (1 << ACPI_FADT_F_WBINVD) |
> + (1 << ACPI_FADT_F_PROC_C1) |
> + (1 << ACPI_FADT_F_SLP_BUTTON) |
> + (1 << ACPI_FADT_F_RTC_S4) |
> + (1 << ACPI_FADT_F_USE_PLATFORM_CLOCK) |
> + /* APIC destination mode ("Flat Logical") has an upper limit of 8
> + * CPUs for more than 8 CPUs, "Clustered Logical" mode has to be
> + * used
> + */
> + ((ms->smp.max_cpus > 8) ?
> + (1 << ACPI_FADT_F_FORCE_APIC_CLUSTER_MODEL) : 0),
> + .int_model = 1 /* Multiple APIC */,
> + .rtc_century = RTC_CENTURY,
> + .plvl2_lat = 0xfff /* C2 state not supported */,
> + .plvl3_lat = 0xfff /* C3 state not supported */,
> + .smi_cmd = ACPI_PORT_SMI_CMD,
> + .sci_int = object_property_get_uint(o, ACPI_PM_PROP_SCI_INT, NULL),
> + .acpi_enable_cmd =
> + object_property_get_uint(o, ACPI_PM_PROP_ACPI_ENABLE_CMD, NULL),
> + .acpi_disable_cmd =
> + object_property_get_uint(o, ACPI_PM_PROP_ACPI_DISABLE_CMD, NULL),
> + .pm1a_evt = { .space_id = as, .bit_width = 4 * 8, .address = io },
> + .pm1a_cnt = { .space_id = as, .bit_width = 2 * 8,
> + .address = io + 0x04 },
> + .pm_tmr = { .space_id = as, .bit_width = 4 * 8, .address = io + 0x08 },
> + .gpe0_blk = { .space_id = as, .bit_width =
> + object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK_LEN, NULL) * 8,
> + .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
> + },
> + };
> + *data = fadt;
> +}
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 05/13] acpi: move acpi_init_common_fadt_data to acpi-common.c
2020-05-05 14:25 ` Igor Mammedov
@ 2020-05-06 10:03 ` Gerd Hoffmann
0 siblings, 0 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-06 10:03 UTC (permalink / raw)
To: Igor Mammedov
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, May 05, 2020 at 04:25:55PM +0200, Igor Mammedov wrote:
> On Tue, 5 May 2020 15:42:57 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> the same question like with FACS, why legacy data are needed for with reduced profile?
> it mostly initializes data for fixed-hw model.
>
> I'd preffer if you drop FACS and use minimal FADT like build_fadt_rev5() does
> without pulling along a bunch of legacy junk (unless there is a good justification for it).
Leftover from the isa-acpi version, it is indeed not needed any more
(same for facs).
take care,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 06/13] acpi: move acpi_align_size to acpi-common.h
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
` (4 preceding siblings ...)
2020-05-05 13:42 ` [PATCH v2 05/13] acpi: move acpi_init_common_fadt_data " Gerd Hoffmann
@ 2020-05-05 13:42 ` Gerd Hoffmann
2020-05-05 13:53 ` Philippe Mathieu-Daudé
2020-05-05 14:30 ` Igor Mammedov
2020-05-05 13:42 ` [PATCH v2 07/13] acpi: fadt: add hw-reduced sleep register support Gerd Hoffmann
` (7 subsequent siblings)
13 siblings, 2 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/i386/acpi-common.h | 19 +++++++++++++++++++
hw/i386/acpi-build.c | 18 ------------------
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
index 5788a13da9ca..f837bb17163b 100644
--- a/hw/i386/acpi-common.h
+++ b/hw/i386/acpi-common.h
@@ -3,12 +3,31 @@
#include "include/hw/acpi/acpi-defs.h"
#include "include/hw/acpi/acpi_dev_interface.h"
+#include "include/hw/acpi/aml-build.h"
#include "include/hw/acpi/bios-linker-loader.h"
#include "include/hw/i386/x86.h"
+/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
+ * -M pc-i440fx-2.0. Even if the actual amount of AML generated grows
+ * a little bit, there should be plenty of free space since the DSDT
+ * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
+ */
+#define ACPI_BUILD_LEGACY_CPU_AML_SIZE 97
+#define ACPI_BUILD_ALIGN_SIZE 0x1000
+
+#define ACPI_BUILD_TABLE_SIZE 0x20000
+
/* Default IOAPIC ID */
#define ACPI_BUILD_IOAPIC_ID 0x0
+static inline void acpi_align_size(GArray *blob, unsigned align)
+{
+ /* Align size to multiple of given size. This reduces the chance
+ * we need to change size in the future (breaking cross version migration).
+ */
+ g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
+}
+
void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
X86MachineState *x86ms, AcpiDeviceIf *adev,
bool has_pci);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index d1f14394734e..dc3b62468439 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -72,16 +72,6 @@
#include "hw/acpi/ipmi.h"
#include "hw/acpi/hmat.h"
-/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
- * -M pc-i440fx-2.0. Even if the actual amount of AML generated grows
- * a little bit, there should be plenty of free space since the DSDT
- * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
- */
-#define ACPI_BUILD_LEGACY_CPU_AML_SIZE 97
-#define ACPI_BUILD_ALIGN_SIZE 0x1000
-
-#define ACPI_BUILD_TABLE_SIZE 0x20000
-
/* #define DEBUG_ACPI_BUILD */
#ifdef DEBUG_ACPI_BUILD
#define ACPI_BUILD_DPRINTF(fmt, ...) \
@@ -267,14 +257,6 @@ static void acpi_get_pci_holes(Range *hole, Range *hole64)
NULL));
}
-static void acpi_align_size(GArray *blob, unsigned align)
-{
- /* Align size to multiple of given size. This reduces the chance
- * we need to change size in the future (breaking cross version migration).
- */
- g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
-}
-
static void build_append_pcihp_notify_entry(Aml *method, int slot)
{
Aml *if_ctx;
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 06/13] acpi: move acpi_align_size to acpi-common.h
2020-05-05 13:42 ` [PATCH v2 06/13] acpi: move acpi_align_size to acpi-common.h Gerd Hoffmann
@ 2020-05-05 13:53 ` Philippe Mathieu-Daudé
2020-05-05 14:30 ` Igor Mammedov
1 sibling, 0 replies; 43+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-05 13:53 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Paolo Bonzini,
Igor Mammedov, Richard Henderson
Hi Gerd,
On 5/5/20 3:42 PM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/i386/acpi-common.h | 19 +++++++++++++++++++
> hw/i386/acpi-build.c | 18 ------------------
> 2 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
> index 5788a13da9ca..f837bb17163b 100644
> --- a/hw/i386/acpi-common.h
> +++ b/hw/i386/acpi-common.h
> @@ -3,12 +3,31 @@
>
> #include "include/hw/acpi/acpi-defs.h"
> #include "include/hw/acpi/acpi_dev_interface.h"
> +#include "include/hw/acpi/aml-build.h"
> #include "include/hw/acpi/bios-linker-loader.h"
> #include "include/hw/i386/x86.h"
>
> +/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
> + * -M pc-i440fx-2.0. Even if the actual amount of AML generated grows
> + * a little bit, there should be plenty of free space since the DSDT
> + * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
> + */
> +#define ACPI_BUILD_LEGACY_CPU_AML_SIZE 97
> +#define ACPI_BUILD_ALIGN_SIZE 0x1000
> +
> +#define ACPI_BUILD_TABLE_SIZE 0x20000
> +
> /* Default IOAPIC ID */
> #define ACPI_BUILD_IOAPIC_ID 0x0
>
> +static inline void acpi_align_size(GArray *blob, unsigned align)
> +{
> + /* Align size to multiple of given size. This reduces the chance
> + * we need to change size in the future (breaking cross version migration).
> + */
> + g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
> +}
> +
> void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> X86MachineState *x86ms, AcpiDeviceIf *adev,
> bool has_pci);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index d1f14394734e..dc3b62468439 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -72,16 +72,6 @@
> #include "hw/acpi/ipmi.h"
> #include "hw/acpi/hmat.h"
>
> -/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
> - * -M pc-i440fx-2.0. Even if the actual amount of AML generated grows
> - * a little bit, there should be plenty of free space since the DSDT
> - * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
> - */
> -#define ACPI_BUILD_LEGACY_CPU_AML_SIZE 97
Can we keep the ACPI_BUILD_LEGACY_CPU_AML_SIZE definition in this file?
The rest of the patch is OK.
> -#define ACPI_BUILD_ALIGN_SIZE 0x1000
> -
> -#define ACPI_BUILD_TABLE_SIZE 0x20000
> -
> /* #define DEBUG_ACPI_BUILD */
> #ifdef DEBUG_ACPI_BUILD
> #define ACPI_BUILD_DPRINTF(fmt, ...) \
> @@ -267,14 +257,6 @@ static void acpi_get_pci_holes(Range *hole, Range *hole64)
> NULL));
> }
>
> -static void acpi_align_size(GArray *blob, unsigned align)
> -{
> - /* Align size to multiple of given size. This reduces the chance
> - * we need to change size in the future (breaking cross version migration).
> - */
> - g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
> -}
> -
> static void build_append_pcihp_notify_entry(Aml *method, int slot)
> {
> Aml *if_ctx;
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 06/13] acpi: move acpi_align_size to acpi-common.h
2020-05-05 13:42 ` [PATCH v2 06/13] acpi: move acpi_align_size to acpi-common.h Gerd Hoffmann
2020-05-05 13:53 ` Philippe Mathieu-Daudé
@ 2020-05-05 14:30 ` Igor Mammedov
1 sibling, 0 replies; 43+ messages in thread
From: Igor Mammedov @ 2020-05-05 14:30 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, 5 May 2020 15:42:58 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
patch probably isn't needed, see my comment on 9/13
> ---
> hw/i386/acpi-common.h | 19 +++++++++++++++++++
> hw/i386/acpi-build.c | 18 ------------------
> 2 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/hw/i386/acpi-common.h b/hw/i386/acpi-common.h
> index 5788a13da9ca..f837bb17163b 100644
> --- a/hw/i386/acpi-common.h
> +++ b/hw/i386/acpi-common.h
> @@ -3,12 +3,31 @@
>
> #include "include/hw/acpi/acpi-defs.h"
> #include "include/hw/acpi/acpi_dev_interface.h"
> +#include "include/hw/acpi/aml-build.h"
> #include "include/hw/acpi/bios-linker-loader.h"
> #include "include/hw/i386/x86.h"
>
> +/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
> + * -M pc-i440fx-2.0. Even if the actual amount of AML generated grows
> + * a little bit, there should be plenty of free space since the DSDT
> + * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
> + */
> +#define ACPI_BUILD_LEGACY_CPU_AML_SIZE 97
> +#define ACPI_BUILD_ALIGN_SIZE 0x1000
> +
> +#define ACPI_BUILD_TABLE_SIZE 0x20000
> +
> /* Default IOAPIC ID */
> #define ACPI_BUILD_IOAPIC_ID 0x0
>
> +static inline void acpi_align_size(GArray *blob, unsigned align)
> +{
> + /* Align size to multiple of given size. This reduces the chance
> + * we need to change size in the future (breaking cross version migration).
> + */
> + g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
> +}
> +
> void acpi_build_madt(GArray *table_data, BIOSLinker *linker,
> X86MachineState *x86ms, AcpiDeviceIf *adev,
> bool has_pci);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index d1f14394734e..dc3b62468439 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -72,16 +72,6 @@
> #include "hw/acpi/ipmi.h"
> #include "hw/acpi/hmat.h"
>
> -/* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
> - * -M pc-i440fx-2.0. Even if the actual amount of AML generated grows
> - * a little bit, there should be plenty of free space since the DSDT
> - * shrunk by ~1.5k between QEMU 2.0 and QEMU 2.1.
> - */
> -#define ACPI_BUILD_LEGACY_CPU_AML_SIZE 97
> -#define ACPI_BUILD_ALIGN_SIZE 0x1000
> -
> -#define ACPI_BUILD_TABLE_SIZE 0x20000
> -
> /* #define DEBUG_ACPI_BUILD */
> #ifdef DEBUG_ACPI_BUILD
> #define ACPI_BUILD_DPRINTF(fmt, ...) \
> @@ -267,14 +257,6 @@ static void acpi_get_pci_holes(Range *hole, Range *hole64)
> NULL));
> }
>
> -static void acpi_align_size(GArray *blob, unsigned align)
> -{
> - /* Align size to multiple of given size. This reduces the chance
> - * we need to change size in the future (breaking cross version migration).
> - */
> - g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align));
> -}
> -
> static void build_append_pcihp_notify_entry(Aml *method, int slot)
> {
> Aml *if_ctx;
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 07/13] acpi: fadt: add hw-reduced sleep register support
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
` (5 preceding siblings ...)
2020-05-05 13:42 ` [PATCH v2 06/13] acpi: move acpi_align_size to acpi-common.h Gerd Hoffmann
@ 2020-05-05 13:42 ` Gerd Hoffmann
2020-05-05 14:35 ` Igor Mammedov
2020-05-05 13:43 ` [PATCH v2 08/13] acpi: generic event device for x86 Gerd Hoffmann
` (6 subsequent siblings)
13 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
Add fields to struct AcpiFadtData and update build_fadt() to properly
generate sleep register entries.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
include/hw/acpi/acpi-defs.h | 2 ++
hw/acpi/aml-build.c | 4 ++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index c13327fa7867..3be9ab504968 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -88,6 +88,8 @@ typedef struct AcpiFadtData {
struct AcpiGenericAddress pm_tmr; /* PM_TMR_BLK */
struct AcpiGenericAddress gpe0_blk; /* GPE0_BLK */
struct AcpiGenericAddress reset_reg; /* RESET_REG */
+ struct AcpiGenericAddress sleep_ctl; /* SLEEP_CONTROL_REG */
+ struct AcpiGenericAddress sleep_sts; /* SLEEP_STATUS_REG */
uint8_t reset_val; /* RESET_VALUE */
uint8_t rev; /* Revision */
uint32_t flags; /* Flags */
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 2c3702b8825b..c159b0d30022 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1863,9 +1863,9 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
}
/* SLEEP_CONTROL_REG */
- build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
+ build_append_gas_from_struct(tbl, &f->sleep_ctl);
/* SLEEP_STATUS_REG */
- build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
+ build_append_gas_from_struct(tbl, &f->sleep_sts);
/* TODO: extra fields need to be added to support revisions above rev5 */
assert(f->rev == 5);
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 07/13] acpi: fadt: add hw-reduced sleep register support
2020-05-05 13:42 ` [PATCH v2 07/13] acpi: fadt: add hw-reduced sleep register support Gerd Hoffmann
@ 2020-05-05 14:35 ` Igor Mammedov
0 siblings, 0 replies; 43+ messages in thread
From: Igor Mammedov @ 2020-05-05 14:35 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, 5 May 2020 15:42:59 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Add fields to struct AcpiFadtData and update build_fadt() to properly
> generate sleep register entries.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> include/hw/acpi/acpi-defs.h | 2 ++
> hw/acpi/aml-build.c | 4 ++--
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index c13327fa7867..3be9ab504968 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -88,6 +88,8 @@ typedef struct AcpiFadtData {
> struct AcpiGenericAddress pm_tmr; /* PM_TMR_BLK */
> struct AcpiGenericAddress gpe0_blk; /* GPE0_BLK */
> struct AcpiGenericAddress reset_reg; /* RESET_REG */
> + struct AcpiGenericAddress sleep_ctl; /* SLEEP_CONTROL_REG */
> + struct AcpiGenericAddress sleep_sts; /* SLEEP_STATUS_REG */
> uint8_t reset_val; /* RESET_VALUE */
> uint8_t rev; /* Revision */
> uint32_t flags; /* Flags */
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 2c3702b8825b..c159b0d30022 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1863,9 +1863,9 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
> }
>
> /* SLEEP_CONTROL_REG */
> - build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> + build_append_gas_from_struct(tbl, &f->sleep_ctl);
> /* SLEEP_STATUS_REG */
> - build_append_gas(tbl, AML_AS_SYSTEM_MEMORY, 0 , 0, 0, 0);
> + build_append_gas_from_struct(tbl, &f->sleep_sts);
>
> /* TODO: extra fields need to be added to support revisions above rev5 */
> assert(f->rev == 5);
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 08/13] acpi: generic event device for x86
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
` (6 preceding siblings ...)
2020-05-05 13:42 ` [PATCH v2 07/13] acpi: fadt: add hw-reduced sleep register support Gerd Hoffmann
@ 2020-05-05 13:43 ` Gerd Hoffmann
2020-05-05 15:03 ` Igor Mammedov
2020-05-05 13:43 ` [PATCH v2 09/13] microvm: add minimal acpi support Gerd Hoffmann
` (5 subsequent siblings)
13 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:43 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
Uses TYPE_ACPI_GED as QOM parent for code reuse.
Add registers for sleep states and reset.
Add powerdown notifier for power button events.
Set AcpiDeviceIfClass->madt_cpu.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
include/hw/acpi/generic_event_device.h | 10 +++
hw/i386/generic_event_device_x86.c | 114 +++++++++++++++++++++++++
hw/i386/Makefile.objs | 1 +
3 files changed, 125 insertions(+)
create mode 100644 hw/i386/generic_event_device_x86.c
diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
index 9eb86ca4fd94..16d1f2b3cd30 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -68,9 +68,19 @@
#define ACPI_GED(obj) \
OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED)
+#define TYPE_ACPI_GED_X86 "acpi-ged-x86"
+#define ACPI_GED_X86(obj) \
+ OBJECT_CHECK(AcpiGedX86State, (obj), TYPE_ACPI_GED_X86)
+
#define ACPI_GED_EVT_SEL_OFFSET 0x0
#define ACPI_GED_EVT_SEL_LEN 0x4
+#define ACPI_GED_X86_REG_SLEEP_CTL 0x00
+#define ACPI_GED_X86_REG_SLEEP_STS 0x01
+#define ACPI_GED_X86_REG_RESET 0x02
+#define ACPI_GED_X86_RESET_VALUE 0x42
+#define ACPI_GED_X86_REG_COUNT 0x03
+
#define GED_DEVICE "GED"
#define AML_GED_EVT_REG "EREG"
#define AML_GED_EVT_SEL "ESEL"
diff --git a/hw/i386/generic_event_device_x86.c b/hw/i386/generic_event_device_x86.c
new file mode 100644
index 000000000000..8ae4a63084f3
--- /dev/null
+++ b/hw/i386/generic_event_device_x86.c
@@ -0,0 +1,114 @@
+/*
+ * x86 variant of the generic event device for hw reduced acpi
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "exec/address-spaces.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/generic_event_device.h"
+#include "hw/i386/pc.h"
+#include "hw/irq.h"
+#include "hw/mem/pc-dimm.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "qemu/error-report.h"
+#include "sysemu/runstate.h"
+
+typedef struct AcpiGedX86State {
+ struct AcpiGedState parent_obj;
+ MemoryRegion regs;
+ Notifier powerdown_req;
+} AcpiGedX86State;
+
+static uint64_t acpi_ged_x86_regs_read(void *opaque, hwaddr addr, unsigned size)
+{
+ return 0;
+}
+
+static void acpi_ged_x86_regs_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned int size)
+{
+ bool slp_en;
+ int slp_typ;
+
+ switch (addr) {
+ case ACPI_GED_X86_REG_SLEEP_CTL:
+ slp_typ = (data >> 2) & 0x07;
+ slp_en = (data >> 5) & 0x01;
+ if (slp_en && slp_typ == 5) {
+ qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+ }
+ return;
+ case ACPI_GED_X86_REG_SLEEP_STS:
+ return;
+ case ACPI_GED_X86_REG_RESET:
+ if (data == ACPI_GED_X86_RESET_VALUE) {
+ qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+ }
+ return;
+ }
+}
+
+static const MemoryRegionOps acpi_ged_x86_regs_ops = {
+ .read = acpi_ged_x86_regs_read,
+ .write = acpi_ged_x86_regs_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 1,
+ },
+};
+
+static void acpi_ged_x86_powerdown_req(Notifier *n, void *opaque)
+{
+ AcpiGedX86State *s = container_of(n, AcpiGedX86State, powerdown_req);
+ AcpiDeviceIf *adev = ACPI_DEVICE_IF(s);
+ AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(OBJECT(s));
+
+ adevc->send_event(adev, ACPI_POWER_DOWN_STATUS);
+}
+
+static void acpi_ged_x86_initfn(Object *obj)
+{
+ AcpiGedX86State *s = ACPI_GED_X86(obj);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+
+ memory_region_init_io(&s->regs, obj, &acpi_ged_x86_regs_ops,
+ obj, "acpi-regs", ACPI_GED_X86_REG_COUNT);
+ sysbus_init_mmio(sbd, &s->regs);
+
+ s->powerdown_req.notify = acpi_ged_x86_powerdown_req;
+ qemu_register_powerdown_notifier(&s->powerdown_req);
+}
+
+static void acpi_ged_x86_class_init(ObjectClass *class, void *data)
+{
+ AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
+
+ adevc->madt_cpu = pc_madt_cpu_entry;
+}
+
+static const TypeInfo acpi_ged_x86_info = {
+ .name = TYPE_ACPI_GED_X86,
+ .parent = TYPE_ACPI_GED,
+ .instance_size = sizeof(AcpiGedX86State),
+ .instance_init = acpi_ged_x86_initfn,
+ .class_init = acpi_ged_x86_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_HOTPLUG_HANDLER },
+ { TYPE_ACPI_DEVICE_IF },
+ { }
+ }
+};
+
+static void acpi_ged_x86_register_types(void)
+{
+ type_register_static(&acpi_ged_x86_info);
+}
+
+type_init(acpi_ged_x86_register_types)
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 6abc74551a72..622739305882 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -17,4 +17,5 @@ obj-$(CONFIG_PC) += port92.o
obj-y += kvmvapic.o
obj-$(CONFIG_ACPI) += acpi-common.o
+obj-$(CONFIG_ACPI_HW_REDUCED) += generic_event_device_x86.o
obj-$(CONFIG_PC) += acpi-build.o
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 08/13] acpi: generic event device for x86
2020-05-05 13:43 ` [PATCH v2 08/13] acpi: generic event device for x86 Gerd Hoffmann
@ 2020-05-05 15:03 ` Igor Mammedov
2020-05-06 10:31 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Igor Mammedov @ 2020-05-05 15:03 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, 5 May 2020 15:43:00 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Uses TYPE_ACPI_GED as QOM parent for code reuse.
> Add registers for sleep states and reset.
> Add powerdown notifier for power button events.
registers aren't x86 specific per spec,
can we put these registers into TYPE_ACPI_GED
and enable the optionally like is done with memory hotplug registers
in acpi_ged_initfn()
> Set AcpiDeviceIfClass->madt_cpu.
that's the only reason I could justify adding x86 specific flavour.
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> include/hw/acpi/generic_event_device.h | 10 +++
> hw/i386/generic_event_device_x86.c | 114 +++++++++++++++++++++++++
> hw/i386/Makefile.objs | 1 +
> 3 files changed, 125 insertions(+)
> create mode 100644 hw/i386/generic_event_device_x86.c
>
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> index 9eb86ca4fd94..16d1f2b3cd30 100644
> --- a/include/hw/acpi/generic_event_device.h
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -68,9 +68,19 @@
> #define ACPI_GED(obj) \
> OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED)
>
> +#define TYPE_ACPI_GED_X86 "acpi-ged-x86"
> +#define ACPI_GED_X86(obj) \
> + OBJECT_CHECK(AcpiGedX86State, (obj), TYPE_ACPI_GED_X86)
> +
> #define ACPI_GED_EVT_SEL_OFFSET 0x0
> #define ACPI_GED_EVT_SEL_LEN 0x4
>
> +#define ACPI_GED_X86_REG_SLEEP_CTL 0x00
> +#define ACPI_GED_X86_REG_SLEEP_STS 0x01
> +#define ACPI_GED_X86_REG_RESET 0x02
> +#define ACPI_GED_X86_RESET_VALUE 0x42
> +#define ACPI_GED_X86_REG_COUNT 0x03
> +
> #define GED_DEVICE "GED"
> #define AML_GED_EVT_REG "EREG"
> #define AML_GED_EVT_SEL "ESEL"
> diff --git a/hw/i386/generic_event_device_x86.c b/hw/i386/generic_event_device_x86.c
> new file mode 100644
> index 000000000000..8ae4a63084f3
> --- /dev/null
> +++ b/hw/i386/generic_event_device_x86.c
> @@ -0,0 +1,114 @@
> +/*
> + * x86 variant of the generic event device for hw reduced acpi
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "exec/address-spaces.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/generic_event_device.h"
> +#include "hw/i386/pc.h"
> +#include "hw/irq.h"
> +#include "hw/mem/pc-dimm.h"
> +#include "hw/qdev-properties.h"
> +#include "migration/vmstate.h"
> +#include "qemu/error-report.h"
> +#include "sysemu/runstate.h"
> +
> +typedef struct AcpiGedX86State {
> + struct AcpiGedState parent_obj;
> + MemoryRegion regs;
> + Notifier powerdown_req;
> +} AcpiGedX86State;
> +
> +static uint64_t acpi_ged_x86_regs_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + return 0;
> +}
> +
> +static void acpi_ged_x86_regs_write(void *opaque, hwaddr addr, uint64_t data,
> + unsigned int size)
> +{
> + bool slp_en;
> + int slp_typ;
> +
> + switch (addr) {
> + case ACPI_GED_X86_REG_SLEEP_CTL:
> + slp_typ = (data >> 2) & 0x07;
> + slp_en = (data >> 5) & 0x01;
> + if (slp_en && slp_typ == 5) {
> + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> + }
> + return;
> + case ACPI_GED_X86_REG_SLEEP_STS:
> + return;
> + case ACPI_GED_X86_REG_RESET:
> + if (data == ACPI_GED_X86_RESET_VALUE) {
> + qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> + }
> + return;
> + }
> +}
> +
> +static const MemoryRegionOps acpi_ged_x86_regs_ops = {
> + .read = acpi_ged_x86_regs_read,
> + .write = acpi_ged_x86_regs_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> + .valid = {
> + .min_access_size = 1,
> + .max_access_size = 1,
> + },
> +};
> +
> +static void acpi_ged_x86_powerdown_req(Notifier *n, void *opaque)
> +{
> + AcpiGedX86State *s = container_of(n, AcpiGedX86State, powerdown_req);
> + AcpiDeviceIf *adev = ACPI_DEVICE_IF(s);
> + AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(OBJECT(s));
> +
> + adevc->send_event(adev, ACPI_POWER_DOWN_STATUS);
> +}
> +
> +static void acpi_ged_x86_initfn(Object *obj)
> +{
> + AcpiGedX86State *s = ACPI_GED_X86(obj);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +
> + memory_region_init_io(&s->regs, obj, &acpi_ged_x86_regs_ops,
> + obj, "acpi-regs", ACPI_GED_X86_REG_COUNT);
> + sysbus_init_mmio(sbd, &s->regs);
> +
> + s->powerdown_req.notify = acpi_ged_x86_powerdown_req;
> + qemu_register_powerdown_notifier(&s->powerdown_req);
> +}
> +
> +static void acpi_ged_x86_class_init(ObjectClass *class, void *data)
> +{
> + AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
> +
> + adevc->madt_cpu = pc_madt_cpu_entry;
> +}
> +
> +static const TypeInfo acpi_ged_x86_info = {
> + .name = TYPE_ACPI_GED_X86,
> + .parent = TYPE_ACPI_GED,
> + .instance_size = sizeof(AcpiGedX86State),
> + .instance_init = acpi_ged_x86_initfn,
> + .class_init = acpi_ged_x86_class_init,
> + .interfaces = (InterfaceInfo[]) {
> + { TYPE_HOTPLUG_HANDLER },
> + { TYPE_ACPI_DEVICE_IF },
> + { }
> + }
> +};
> +
> +static void acpi_ged_x86_register_types(void)
> +{
> + type_register_static(&acpi_ged_x86_info);
> +}
> +
> +type_init(acpi_ged_x86_register_types)
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index 6abc74551a72..622739305882 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -17,4 +17,5 @@ obj-$(CONFIG_PC) += port92.o
>
> obj-y += kvmvapic.o
> obj-$(CONFIG_ACPI) += acpi-common.o
> +obj-$(CONFIG_ACPI_HW_REDUCED) += generic_event_device_x86.o
> obj-$(CONFIG_PC) += acpi-build.o
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 08/13] acpi: generic event device for x86
2020-05-05 15:03 ` Igor Mammedov
@ 2020-05-06 10:31 ` Gerd Hoffmann
2020-05-06 12:41 ` Igor Mammedov
0 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-06 10:31 UTC (permalink / raw)
To: Igor Mammedov
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, May 05, 2020 at 05:03:16PM +0200, Igor Mammedov wrote:
> On Tue, 5 May 2020 15:43:00 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > Uses TYPE_ACPI_GED as QOM parent for code reuse.
> > Add registers for sleep states and reset.
> > Add powerdown notifier for power button events.
> registers aren't x86 specific per spec,
> can we put these registers into TYPE_ACPI_GED
> and enable the optionally like is done with memory hotplug registers
> in acpi_ged_initfn()
Sure, will do.
> > Set AcpiDeviceIfClass->madt_cpu.
> that's the only reason I could justify adding x86 specific flavour.
Also the powerdown notifier. Seems the workflow is slightly different
on x86 (acpi device registers notifier) and arm (machine registers
notifier and calls acpidev->send_event).
take care,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 08/13] acpi: generic event device for x86
2020-05-06 10:31 ` Gerd Hoffmann
@ 2020-05-06 12:41 ` Igor Mammedov
2020-05-07 14:03 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Igor Mammedov @ 2020-05-06 12:41 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Wed, 6 May 2020 12:31:06 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Tue, May 05, 2020 at 05:03:16PM +0200, Igor Mammedov wrote:
> > On Tue, 5 May 2020 15:43:00 +0200
> > Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > > Uses TYPE_ACPI_GED as QOM parent for code reuse.
> > > Add registers for sleep states and reset.
> > > Add powerdown notifier for power button events.
> > registers aren't x86 specific per spec,
> > can we put these registers into TYPE_ACPI_GED
> > and enable the optionally like is done with memory hotplug registers
> > in acpi_ged_initfn()
>
> Sure, will do.
>
> > > Set AcpiDeviceIfClass->madt_cpu.
> > that's the only reason I could justify adding x86 specific flavour.
>
> Also the powerdown notifier. Seems the workflow is slightly different
> on x86 (acpi device registers notifier) and arm (machine registers
> notifier and calls acpidev->send_event).
Is it possible to use ARM's approach, without notifier?
>
> take care,
> Gerd
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 08/13] acpi: generic event device for x86
2020-05-06 12:41 ` Igor Mammedov
@ 2020-05-07 14:03 ` Gerd Hoffmann
0 siblings, 0 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-07 14:03 UTC (permalink / raw)
To: Igor Mammedov
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
Hi,
> > Also the powerdown notifier. Seems the workflow is slightly different
> > on x86 (acpi device registers notifier) and arm (machine registers
> > notifier and calls acpidev->send_event).
>
> Is it possible to use ARM's approach, without notifier?
Without notifier isn't going to work, it has to be somewhere.
(Moving the notifier from ged to microvm shouldn't be much of
an issue, didn't try that yet though).
take care,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 09/13] microvm: add minimal acpi support
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
` (7 preceding siblings ...)
2020-05-05 13:43 ` [PATCH v2 08/13] acpi: generic event device for x86 Gerd Hoffmann
@ 2020-05-05 13:43 ` Gerd Hoffmann
2020-05-05 15:20 ` Igor Mammedov
2020-05-05 13:43 ` [PATCH v2 10/13] microvm: disable virtio-mmio cmdline hack Gerd Hoffmann
` (4 subsequent siblings)
13 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:43 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
$subject says all. Can be disabled using the usual -no-acpi switch.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/i386/acpi-microvm.h | 6 ++
include/hw/i386/microvm.h | 8 ++
hw/i386/acpi-microvm.c | 198 ++++++++++++++++++++++++++++++++++++++
hw/i386/microvm.c | 22 +++++
hw/i386/Kconfig | 1 +
hw/i386/Makefile.objs | 1 +
6 files changed, 236 insertions(+)
create mode 100644 hw/i386/acpi-microvm.h
create mode 100644 hw/i386/acpi-microvm.c
diff --git a/hw/i386/acpi-microvm.h b/hw/i386/acpi-microvm.h
new file mode 100644
index 000000000000..6a6c2967102b
--- /dev/null
+++ b/hw/i386/acpi-microvm.h
@@ -0,0 +1,6 @@
+#ifndef HW_I386_ACPI_MICROVM_H
+#define HW_I386_ACPI_MICROVM_H
+
+void acpi_setup_microvm(void);
+
+#endif
diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
index ba68d1f22bb3..55f5984cfaa1 100644
--- a/include/hw/i386/microvm.h
+++ b/include/hw/i386/microvm.h
@@ -24,6 +24,7 @@
#include "hw/boards.h"
#include "hw/i386/x86.h"
+#include "hw/acpi/acpi_dev_interface.h"
/* Platform virtio definitions */
#define VIRTIO_MMIO_BASE 0xc0000000
@@ -31,6 +32,11 @@
#define VIRTIO_NUM_TRANSPORTS 8
#define VIRTIO_CMDLINE_MAXLEN 64
+#define GED_MMIO_BASE 0xc1000000
+#define GED_MMIO_BASE_MEMHP (GED_MMIO_BASE + 0x100)
+#define GED_MMIO_BASE_REGS (GED_MMIO_BASE + 0x200)
+#define GED_MMIO_IRQ 9
+
/* Machine type options */
#define MICROVM_MACHINE_PIT "pit"
#define MICROVM_MACHINE_PIC "pic"
@@ -58,6 +64,8 @@ typedef struct {
/* Machine state */
bool kernel_cmdline_fixed;
+ Notifier machine_done;
+ AcpiDeviceIf *acpi_dev;
} MicrovmMachineState;
#define TYPE_MICROVM_MACHINE MACHINE_TYPE_NAME("microvm")
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
new file mode 100644
index 000000000000..ce5ab86d642c
--- /dev/null
+++ b/hw/i386/acpi-microvm.c
@@ -0,0 +1,198 @@
+/* Support for generating ACPI tables and passing them to Guests
+ *
+ * Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
+ * Copyright (C) 2006 Fabrice Bellard
+ * Copyright (C) 2013 Red Hat Inc
+ *
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+
+#include "exec/memory.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/bios-linker-loader.h"
+#include "hw/acpi/generic_event_device.h"
+#include "hw/acpi/utils.h"
+#include "hw/boards.h"
+#include "hw/i386/fw_cfg.h"
+#include "hw/i386/microvm.h"
+
+#include "acpi-common.h"
+#include "acpi-microvm.h"
+
+/* FIXME: copy & paste */
+static void acpi_dsdt_add_power_button(Aml *scope)
+{
+ Aml *dev = aml_device(ACPI_POWER_BUTTON_DEVICE);
+ aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C")));
+ aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+ aml_append(scope, dev);
+}
+
+static void
+build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
+ MicrovmMachineState *mms)
+{
+ X86MachineState *x86ms = X86_MACHINE(mms);
+ Aml *dsdt, *sb_scope, *scope, *pkg;
+ bool ambiguous;
+ Object *isabus;
+
+ isabus = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
+ assert(isabus);
+ assert(!ambiguous);
+
+ dsdt = init_aml_allocator();
+
+ /* Reserve space for header */
+ acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader));
+
+ sb_scope = aml_scope("_SB");
+ fw_cfg_add_acpi_dsdt(sb_scope, x86ms->fw_cfg);
+ isa_build_aml(ISA_BUS(isabus), sb_scope);
+ build_ged_aml(sb_scope, GED_DEVICE, HOTPLUG_HANDLER(mms->acpi_dev),
+ GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
+ acpi_dsdt_add_power_button(sb_scope);
+ aml_append(dsdt, sb_scope);
+
+ scope = aml_scope("\\");
+ pkg = aml_package(4);
+ aml_append(pkg, aml_int(5)); /* SLEEP_CONTROL_REG.SLP_TYP */
+ aml_append(pkg, aml_int(0)); /* ignored */
+ aml_append(pkg, aml_int(0)); /* reserved */
+ aml_append(pkg, aml_int(0)); /* reserved */
+ aml_append(scope, aml_name_decl("_S5", pkg));
+ aml_append(dsdt, scope);
+
+ /* copy AML table into ACPI tables blob and patch header there */
+ g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
+ build_header(linker, table_data,
+ (void *)(table_data->data + table_data->len - dsdt->buf->len),
+ "DSDT", dsdt->buf->len, 5, NULL, NULL);
+ free_aml_allocator();
+}
+
+static void acpi_build_microvm(AcpiBuildTables *tables,
+ MicrovmMachineState *mms)
+{
+ MachineState *machine = MACHINE(mms);
+ GArray *table_offsets;
+ GArray *tables_blob = tables->table_data;
+ unsigned facs, dsdt, xsdt;
+ AcpiFadtData pmfadt = {
+ .rev = 5,
+ .minor_ver = 1,
+ .flags = ((1 << ACPI_FADT_F_HW_REDUCED_ACPI) |
+ (1 << ACPI_FADT_F_RESET_REG_SUP)),
+ .sleep_ctl = {
+ .space_id = AML_AS_SYSTEM_MEMORY,
+ .bit_width = 8,
+ .address = GED_MMIO_BASE_REGS + ACPI_GED_X86_REG_SLEEP_CTL,
+ },
+ .sleep_sts = {
+ .space_id = AML_AS_SYSTEM_MEMORY,
+ .bit_width = 8,
+ .address = GED_MMIO_BASE_REGS + ACPI_GED_X86_REG_SLEEP_STS,
+ },
+ .reset_reg = {
+ .space_id = AML_AS_SYSTEM_MEMORY,
+ .bit_width = 8,
+ .address = GED_MMIO_BASE_REGS + ACPI_GED_X86_REG_RESET,
+ },
+ .reset_val = ACPI_GED_X86_RESET_VALUE,
+ };
+
+ table_offsets = g_array_new(false, true /* clear */,
+ sizeof(uint32_t));
+ bios_linker_loader_alloc(tables->linker,
+ ACPI_BUILD_TABLE_FILE, tables_blob,
+ 64 /* Ensure FACS is aligned */,
+ false /* high memory */);
+
+ facs = tables_blob->len;
+ acpi_build_facs(tables_blob);
+
+ dsdt = tables_blob->len;
+ build_dsdt_microvm(tables_blob, tables->linker, mms);
+
+ pmfadt.facs_tbl_offset = &facs;
+ pmfadt.dsdt_tbl_offset = &dsdt;
+ pmfadt.xdsdt_tbl_offset = &dsdt;
+ acpi_add_table(table_offsets, tables_blob);
+ build_fadt(tables_blob, tables->linker, &pmfadt, NULL, NULL);
+
+ acpi_add_table(table_offsets, tables_blob);
+ acpi_build_madt(tables_blob, tables->linker, X86_MACHINE(machine),
+ mms->acpi_dev, false);
+
+ xsdt = tables_blob->len;
+ build_xsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
+
+ /* RSDP is in FSEG memory, so allocate it separately */
+ {
+ AcpiRsdpData rsdp_data = {
+ .revision = 2,
+ .oem_id = ACPI_BUILD_APPNAME6,
+ .xsdt_tbl_offset = &xsdt,
+ .rsdt_tbl_offset = NULL,
+ };
+ build_rsdp(tables->rsdp, tables->linker, &rsdp_data);
+ }
+
+ acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
+ acpi_align_size(tables->linker->cmd_blob, ACPI_BUILD_ALIGN_SIZE);
+
+ /* Cleanup memory that's no longer used. */
+ g_array_free(table_offsets, true);
+}
+
+static void acpi_build_no_update(void *build_opaque)
+{
+ /* nothing, microvm tables don't change at runtime */
+}
+
+void acpi_setup_microvm(void)
+{
+ MicrovmMachineState *mms = MICROVM_MACHINE(qdev_get_machine());
+ X86MachineState *x86ms = X86_MACHINE(mms);
+ AcpiBuildTables tables;
+
+ assert(x86ms->fw_cfg);
+
+ if (!x86_machine_is_acpi_enabled(x86ms)) {
+ return;
+ }
+
+ acpi_build_tables_init(&tables);
+ acpi_build_microvm(&tables, mms);
+
+ /* Now expose it all to Guest */
+ acpi_add_rom_blob(acpi_build_no_update, NULL,
+ tables.table_data,
+ ACPI_BUILD_TABLE_FILE,
+ ACPI_BUILD_TABLE_MAX_SIZE);
+ acpi_add_rom_blob(acpi_build_no_update, NULL,
+ tables.linker->cmd_blob,
+ "etc/table-loader", 0);
+ acpi_add_rom_blob(acpi_build_no_update, NULL,
+ tables.rsdp,
+ ACPI_BUILD_RSDP_FILE, 0);
+
+ acpi_build_tables_cleanup(&tables, false);
+}
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 38d8e5170323..6ba2d9d3f028 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -26,6 +26,7 @@
#include "sysemu/cpus.h"
#include "sysemu/numa.h"
#include "sysemu/reset.h"
+#include "acpi-microvm.h"
#include "hw/loader.h"
#include "hw/irq.h"
@@ -41,6 +42,8 @@
#include "hw/i386/e820_memory_layout.h"
#include "hw/i386/fw_cfg.h"
#include "hw/virtio/virtio-mmio.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/generic_event_device.h"
#include "cpu.h"
#include "elf.h"
@@ -128,6 +131,17 @@ static void microvm_devices_init(MicrovmMachineState *mms)
}
/* Optional and legacy devices */
+ if (x86_machine_is_acpi_enabled(x86ms)) {
+ DeviceState *dev = qdev_create(NULL, TYPE_ACPI_GED_X86);
+ qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
+ sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
+ /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP); */
+ sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, GED_MMIO_BASE_REGS);
+ sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
+ x86ms->gsi[GED_MMIO_IRQ]);
+ qdev_init_nofail(dev);
+ mms->acpi_dev = ACPI_DEVICE_IF(dev);
+ }
if (mms->pic == ON_OFF_AUTO_ON || mms->pic == ON_OFF_AUTO_AUTO) {
qemu_irq *i8259;
@@ -468,6 +482,11 @@ static void microvm_machine_set_auto_kernel_cmdline(Object *obj, bool value,
mms->auto_kernel_cmdline = value;
}
+static void microvm_machine_done(Notifier *notifier, void *data)
+{
+ acpi_setup_microvm();
+}
+
static void microvm_machine_initfn(Object *obj)
{
MicrovmMachineState *mms = MICROVM_MACHINE(obj);
@@ -482,6 +501,9 @@ static void microvm_machine_initfn(Object *obj)
/* State */
mms->kernel_cmdline_fixed = false;
+
+ mms->machine_done.notify = microvm_machine_done;
+ qemu_add_machine_init_done_notifier(&mms->machine_done);
}
static void microvm_class_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index c93f32f6579d..be746bcb49eb 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -102,6 +102,7 @@ config MICROVM
select I8259
select MC146818RTC
select VIRTIO_MMIO
+ select ACPI_HW_REDUCED
config X86_IOMMU
bool
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 622739305882..bbb2fe78f3cd 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -19,3 +19,4 @@ obj-y += kvmvapic.o
obj-$(CONFIG_ACPI) += acpi-common.o
obj-$(CONFIG_ACPI_HW_REDUCED) += generic_event_device_x86.o
obj-$(CONFIG_PC) += acpi-build.o
+obj-$(CONFIG_MICROVM) += acpi-microvm.o
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 09/13] microvm: add minimal acpi support
2020-05-05 13:43 ` [PATCH v2 09/13] microvm: add minimal acpi support Gerd Hoffmann
@ 2020-05-05 15:20 ` Igor Mammedov
2020-05-06 10:35 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Igor Mammedov @ 2020-05-05 15:20 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, 5 May 2020 15:43:01 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> $subject says all. Can be disabled using the usual -no-acpi switch.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/i386/acpi-microvm.h | 6 ++
> include/hw/i386/microvm.h | 8 ++
> hw/i386/acpi-microvm.c | 198 ++++++++++++++++++++++++++++++++++++++
> hw/i386/microvm.c | 22 +++++
> hw/i386/Kconfig | 1 +
> hw/i386/Makefile.objs | 1 +
> 6 files changed, 236 insertions(+)
> create mode 100644 hw/i386/acpi-microvm.h
> create mode 100644 hw/i386/acpi-microvm.c
>
> diff --git a/hw/i386/acpi-microvm.h b/hw/i386/acpi-microvm.h
> new file mode 100644
> index 000000000000..6a6c2967102b
> --- /dev/null
> +++ b/hw/i386/acpi-microvm.h
> @@ -0,0 +1,6 @@
> +#ifndef HW_I386_ACPI_MICROVM_H
> +#define HW_I386_ACPI_MICROVM_H
> +
> +void acpi_setup_microvm(void);
> +
> +#endif
> diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
> index ba68d1f22bb3..55f5984cfaa1 100644
> --- a/include/hw/i386/microvm.h
> +++ b/include/hw/i386/microvm.h
> @@ -24,6 +24,7 @@
>
> #include "hw/boards.h"
> #include "hw/i386/x86.h"
> +#include "hw/acpi/acpi_dev_interface.h"
>
> /* Platform virtio definitions */
> #define VIRTIO_MMIO_BASE 0xc0000000
> @@ -31,6 +32,11 @@
> #define VIRTIO_NUM_TRANSPORTS 8
> #define VIRTIO_CMDLINE_MAXLEN 64
>
> +#define GED_MMIO_BASE 0xc1000000
> +#define GED_MMIO_BASE_MEMHP (GED_MMIO_BASE + 0x100)
> +#define GED_MMIO_BASE_REGS (GED_MMIO_BASE + 0x200)
> +#define GED_MMIO_IRQ 9
> +
> /* Machine type options */
> #define MICROVM_MACHINE_PIT "pit"
> #define MICROVM_MACHINE_PIC "pic"
> @@ -58,6 +64,8 @@ typedef struct {
>
> /* Machine state */
> bool kernel_cmdline_fixed;
> + Notifier machine_done;
> + AcpiDeviceIf *acpi_dev;
> } MicrovmMachineState;
>
> #define TYPE_MICROVM_MACHINE MACHINE_TYPE_NAME("microvm")
> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
> new file mode 100644
> index 000000000000..ce5ab86d642c
> --- /dev/null
> +++ b/hw/i386/acpi-microvm.c
> @@ -0,0 +1,198 @@
> +/* Support for generating ACPI tables and passing them to Guests
> + *
> + * Copyright (C) 2008-2010 Kevin O'Connor <kevin@koconnor.net>
> + * Copyright (C) 2006 Fabrice Bellard
> + * Copyright (C) 2013 Red Hat Inc
> + *
> + * Author: Michael S. Tsirkin <mst@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +
> +#include "exec/memory.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/aml-build.h"
> +#include "hw/acpi/bios-linker-loader.h"
> +#include "hw/acpi/generic_event_device.h"
> +#include "hw/acpi/utils.h"
> +#include "hw/boards.h"
> +#include "hw/i386/fw_cfg.h"
> +#include "hw/i386/microvm.h"
> +
> +#include "acpi-common.h"
> +#include "acpi-microvm.h"
> +
> +/* FIXME: copy & paste */
> +static void acpi_dsdt_add_power_button(Aml *scope)
> +{
> + Aml *dev = aml_device(ACPI_POWER_BUTTON_DEVICE);
> + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C")));
> + aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> + aml_append(scope, dev);
> +}
could be unified with ARM's version
> +
> +static void
> +build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
> + MicrovmMachineState *mms)
> +{
> + X86MachineState *x86ms = X86_MACHINE(mms);
> + Aml *dsdt, *sb_scope, *scope, *pkg;
> + bool ambiguous;
> + Object *isabus;
> +
> + isabus = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
> + assert(isabus);
> + assert(!ambiguous);
> +
> + dsdt = init_aml_allocator();
> +
> + /* Reserve space for header */
> + acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader));
> +
> + sb_scope = aml_scope("_SB");
> + fw_cfg_add_acpi_dsdt(sb_scope, x86ms->fw_cfg);
> + isa_build_aml(ISA_BUS(isabus), sb_scope);
> + build_ged_aml(sb_scope, GED_DEVICE, HOTPLUG_HANDLER(mms->acpi_dev),
> + GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
> + acpi_dsdt_add_power_button(sb_scope);
> + aml_append(dsdt, sb_scope);
> +
> + scope = aml_scope("\\");
> + pkg = aml_package(4);
> + aml_append(pkg, aml_int(5)); /* SLEEP_CONTROL_REG.SLP_TYP */
> + aml_append(pkg, aml_int(0)); /* ignored */
> + aml_append(pkg, aml_int(0)); /* reserved */
> + aml_append(pkg, aml_int(0)); /* reserved */
> + aml_append(scope, aml_name_decl("_S5", pkg));
> + aml_append(dsdt, scope);
> +
> + /* copy AML table into ACPI tables blob and patch header there */
> + g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
> + build_header(linker, table_data,
> + (void *)(table_data->data + table_data->len - dsdt->buf->len),
> + "DSDT", dsdt->buf->len, 5, NULL, NULL);
> + free_aml_allocator();
> +}
> +
> +static void acpi_build_microvm(AcpiBuildTables *tables,
> + MicrovmMachineState *mms)
> +{
> + MachineState *machine = MACHINE(mms);
> + GArray *table_offsets;
> + GArray *tables_blob = tables->table_data;
> + unsigned facs, dsdt, xsdt;
> + AcpiFadtData pmfadt = {
> + .rev = 5,
> + .minor_ver = 1,
> + .flags = ((1 << ACPI_FADT_F_HW_REDUCED_ACPI) |
> + (1 << ACPI_FADT_F_RESET_REG_SUP)),
> + .sleep_ctl = {
> + .space_id = AML_AS_SYSTEM_MEMORY,
> + .bit_width = 8,
> + .address = GED_MMIO_BASE_REGS + ACPI_GED_X86_REG_SLEEP_CTL,
> + },
> + .sleep_sts = {
> + .space_id = AML_AS_SYSTEM_MEMORY,
> + .bit_width = 8,
> + .address = GED_MMIO_BASE_REGS + ACPI_GED_X86_REG_SLEEP_STS,
> + },
> + .reset_reg = {
> + .space_id = AML_AS_SYSTEM_MEMORY,
> + .bit_width = 8,
> + .address = GED_MMIO_BASE_REGS + ACPI_GED_X86_REG_RESET,
> + },
> + .reset_val = ACPI_GED_X86_RESET_VALUE,
> + };
> +
> + table_offsets = g_array_new(false, true /* clear */,
> + sizeof(uint32_t));
> + bios_linker_loader_alloc(tables->linker,
> + ACPI_BUILD_TABLE_FILE, tables_blob,
> + 64 /* Ensure FACS is aligned */,
> + false /* high memory */);
> +
> + facs = tables_blob->len;
> + acpi_build_facs(tables_blob);
> +
> + dsdt = tables_blob->len;
> + build_dsdt_microvm(tables_blob, tables->linker, mms);
> +
> + pmfadt.facs_tbl_offset = &facs;
> + pmfadt.dsdt_tbl_offset = &dsdt;
> + pmfadt.xdsdt_tbl_offset = &dsdt;
> + acpi_add_table(table_offsets, tables_blob);
> + build_fadt(tables_blob, tables->linker, &pmfadt, NULL, NULL);
> +
> + acpi_add_table(table_offsets, tables_blob);
> + acpi_build_madt(tables_blob, tables->linker, X86_MACHINE(machine),
> + mms->acpi_dev, false);
> +
> + xsdt = tables_blob->len;
> + build_xsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
> +
> + /* RSDP is in FSEG memory, so allocate it separately */
> + {
> + AcpiRsdpData rsdp_data = {
> + .revision = 2,
> + .oem_id = ACPI_BUILD_APPNAME6,
> + .xsdt_tbl_offset = &xsdt,
> + .rsdt_tbl_offset = NULL,
> + };
> + build_rsdp(tables->rsdp, tables->linker, &rsdp_data);
> + }
> +
> + acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
> + acpi_align_size(tables->linker->cmd_blob, ACPI_BUILD_ALIGN_SIZE);
I'd drop these as that was mostly to counter various migration issues on legacy
and model after virt_acpi_setup()
> +
> + /* Cleanup memory that's no longer used. */
> + g_array_free(table_offsets, true);
> +}
> +
> +static void acpi_build_no_update(void *build_opaque)
> +{
> + /* nothing, microvm tables don't change at runtime */
> +}
> +
> +void acpi_setup_microvm(void)
> +{
> + MicrovmMachineState *mms = MICROVM_MACHINE(qdev_get_machine());
> + X86MachineState *x86ms = X86_MACHINE(mms);
> + AcpiBuildTables tables;
> +
> + assert(x86ms->fw_cfg);
> +
> + if (!x86_machine_is_acpi_enabled(x86ms)) {
> + return;
> + }
> +
> + acpi_build_tables_init(&tables);
> + acpi_build_microvm(&tables, mms);
> +
> + /* Now expose it all to Guest */
> + acpi_add_rom_blob(acpi_build_no_update, NULL,
> + tables.table_data,
> + ACPI_BUILD_TABLE_FILE,
> + ACPI_BUILD_TABLE_MAX_SIZE);
> + acpi_add_rom_blob(acpi_build_no_update, NULL,
> + tables.linker->cmd_blob,
> + "etc/table-loader", 0);
> + acpi_add_rom_blob(acpi_build_no_update, NULL,
> + tables.rsdp,
> + ACPI_BUILD_RSDP_FILE, 0);
> +
> + acpi_build_tables_cleanup(&tables, false);
> +}
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 38d8e5170323..6ba2d9d3f028 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -26,6 +26,7 @@
> #include "sysemu/cpus.h"
> #include "sysemu/numa.h"
> #include "sysemu/reset.h"
> +#include "acpi-microvm.h"
>
> #include "hw/loader.h"
> #include "hw/irq.h"
> @@ -41,6 +42,8 @@
> #include "hw/i386/e820_memory_layout.h"
> #include "hw/i386/fw_cfg.h"
> #include "hw/virtio/virtio-mmio.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/generic_event_device.h"
>
> #include "cpu.h"
> #include "elf.h"
> @@ -128,6 +131,17 @@ static void microvm_devices_init(MicrovmMachineState *mms)
> }
>
> /* Optional and legacy devices */
> + if (x86_machine_is_acpi_enabled(x86ms)) {
> + DeviceState *dev = qdev_create(NULL, TYPE_ACPI_GED_X86);
> + qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
> + /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP); */
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, GED_MMIO_BASE_REGS);
> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
> + x86ms->gsi[GED_MMIO_IRQ]);
> + qdev_init_nofail(dev);
> + mms->acpi_dev = ACPI_DEVICE_IF(dev);
> + }
>
> if (mms->pic == ON_OFF_AUTO_ON || mms->pic == ON_OFF_AUTO_AUTO) {
> qemu_irq *i8259;
> @@ -468,6 +482,11 @@ static void microvm_machine_set_auto_kernel_cmdline(Object *obj, bool value,
> mms->auto_kernel_cmdline = value;
> }
>
> +static void microvm_machine_done(Notifier *notifier, void *data)
> +{
> + acpi_setup_microvm();
> +}
> +
> static void microvm_machine_initfn(Object *obj)
> {
> MicrovmMachineState *mms = MICROVM_MACHINE(obj);
> @@ -482,6 +501,9 @@ static void microvm_machine_initfn(Object *obj)
>
> /* State */
> mms->kernel_cmdline_fixed = false;
> +
> + mms->machine_done.notify = microvm_machine_done;
> + qemu_add_machine_init_done_notifier(&mms->machine_done);
> }
>
> static void microvm_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index c93f32f6579d..be746bcb49eb 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -102,6 +102,7 @@ config MICROVM
> select I8259
> select MC146818RTC
> select VIRTIO_MMIO
> + select ACPI_HW_REDUCED
>
> config X86_IOMMU
> bool
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index 622739305882..bbb2fe78f3cd 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -19,3 +19,4 @@ obj-y += kvmvapic.o
> obj-$(CONFIG_ACPI) += acpi-common.o
> obj-$(CONFIG_ACPI_HW_REDUCED) += generic_event_device_x86.o
> obj-$(CONFIG_PC) += acpi-build.o
> +obj-$(CONFIG_MICROVM) += acpi-microvm.o
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 09/13] microvm: add minimal acpi support
2020-05-05 15:20 ` Igor Mammedov
@ 2020-05-06 10:35 ` Gerd Hoffmann
2020-05-06 14:14 ` Igor Mammedov
0 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-06 10:35 UTC (permalink / raw)
To: Igor Mammedov
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
> > +/* FIXME: copy & paste */
> > +static void acpi_dsdt_add_power_button(Aml *scope)
> > +{
> > + Aml *dev = aml_device(ACPI_POWER_BUTTON_DEVICE);
> > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C")));
> > + aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> > + aml_append(scope, dev);
> > +}
>
> could be unified with ARM's version
Yep. Suggestions for a good place? hw/acpi/aml-build.c ?
> > + acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
> > + acpi_align_size(tables->linker->cmd_blob, ACPI_BUILD_ALIGN_SIZE);
> I'd drop these as that was mostly to counter various migration issues on legacy
Dropped.
take care,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 09/13] microvm: add minimal acpi support
2020-05-06 10:35 ` Gerd Hoffmann
@ 2020-05-06 14:14 ` Igor Mammedov
2020-05-07 13:39 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Igor Mammedov @ 2020-05-06 14:14 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Wed, 6 May 2020 12:35:49 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> > > +/* FIXME: copy & paste */
> > > +static void acpi_dsdt_add_power_button(Aml *scope)
> > > +{
> > > + Aml *dev = aml_device(ACPI_POWER_BUTTON_DEVICE);
> > > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C")));
> > > + aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> > > + aml_append(scope, dev);
> > > +}
> >
> > could be unified with ARM's version
>
> Yep. Suggestions for a good place? hw/acpi/aml-build.c ?
sounds good to me
>
> > > + acpi_align_size(tables_blob, ACPI_BUILD_TABLE_SIZE);
> > > + acpi_align_size(tables->linker->cmd_blob, ACPI_BUILD_ALIGN_SIZE);
> > I'd drop these as that was mostly to counter various migration issues on legacy
>
> Dropped.
>
> take care,
> Gerd
>
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 09/13] microvm: add minimal acpi support
2020-05-06 14:14 ` Igor Mammedov
@ 2020-05-07 13:39 ` Gerd Hoffmann
0 siblings, 0 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-07 13:39 UTC (permalink / raw)
To: Igor Mammedov
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Wed, May 06, 2020 at 04:14:19PM +0200, Igor Mammedov wrote:
> On Wed, 6 May 2020 12:35:49 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > > > +/* FIXME: copy & paste */
> > > > +static void acpi_dsdt_add_power_button(Aml *scope)
> > > > +{
> > > > + Aml *dev = aml_device(ACPI_POWER_BUTTON_DEVICE);
> > > > + aml_append(dev, aml_name_decl("_HID", aml_string("PNP0C0C")));
> > > > + aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> > > > + aml_append(scope, dev);
> > > > +}
> > >
> > > could be unified with ARM's version
> >
> > Yep. Suggestions for a good place? hw/acpi/aml-build.c ?
> sounds good to me
Hmm, tried that, but ACPI_POWER_BUTTON_DEVICE is defined in ged header
file, so ged.c looks like a better fit ;)
take care,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 10/13] microvm: disable virtio-mmio cmdline hack
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
` (8 preceding siblings ...)
2020-05-05 13:43 ` [PATCH v2 09/13] microvm: add minimal acpi support Gerd Hoffmann
@ 2020-05-05 13:43 ` Gerd Hoffmann
2020-05-05 15:22 ` Igor Mammedov
2020-05-05 13:43 ` [PATCH v2 11/13] microvm: add acpi_dsdt_add_virtio() for x86 Gerd Hoffmann
` (3 subsequent siblings)
13 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:43 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
... in case we are using ACPI.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/i386/microvm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 6ba2d9d3f028..a3708fdf1e39 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -369,7 +369,8 @@ static void microvm_machine_reset(MachineState *machine)
CPUState *cs;
X86CPU *cpu;
- if (machine->kernel_filename != NULL &&
+ if (!x86_machine_is_acpi_enabled(X86_MACHINE(machine)) &&
+ machine->kernel_filename != NULL &&
mms->auto_kernel_cmdline && !mms->kernel_cmdline_fixed) {
microvm_fix_kernel_cmdline(machine);
mms->kernel_cmdline_fixed = true;
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 10/13] microvm: disable virtio-mmio cmdline hack
2020-05-05 13:43 ` [PATCH v2 10/13] microvm: disable virtio-mmio cmdline hack Gerd Hoffmann
@ 2020-05-05 15:22 ` Igor Mammedov
0 siblings, 0 replies; 43+ messages in thread
From: Igor Mammedov @ 2020-05-05 15:22 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, qemu-devel,
Paolo Bonzini, Richard Henderson
On Tue, 5 May 2020 15:43:02 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> ... in case we are using ACPI.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/microvm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 6ba2d9d3f028..a3708fdf1e39 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -369,7 +369,8 @@ static void microvm_machine_reset(MachineState *machine)
> CPUState *cs;
> X86CPU *cpu;
>
> - if (machine->kernel_filename != NULL &&
> + if (!x86_machine_is_acpi_enabled(X86_MACHINE(machine)) &&
> + machine->kernel_filename != NULL &&
> mms->auto_kernel_cmdline && !mms->kernel_cmdline_fixed) {
> microvm_fix_kernel_cmdline(machine);
> mms->kernel_cmdline_fixed = true;
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 11/13] microvm: add acpi_dsdt_add_virtio() for x86
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
` (9 preceding siblings ...)
2020-05-05 13:43 ` [PATCH v2 10/13] microvm: disable virtio-mmio cmdline hack Gerd Hoffmann
@ 2020-05-05 13:43 ` Gerd Hoffmann
2020-05-06 7:27 ` Sergio Lopez
2020-05-05 13:43 ` [PATCH v2 12/13] microvm: make virtio irq base runtime configurable Gerd Hoffmann
` (2 subsequent siblings)
13 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:43 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
Makes x86 linux kernel find virtio-mmio devices automatically.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/i386/acpi-microvm.c | 51 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
index ce5ab86d642c..4d91ac9360ce 100644
--- a/hw/i386/acpi-microvm.c
+++ b/hw/i386/acpi-microvm.c
@@ -21,6 +21,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/cutils.h"
#include "qapi/error.h"
#include "exec/memory.h"
@@ -32,6 +33,7 @@
#include "hw/boards.h"
#include "hw/i386/fw_cfg.h"
#include "hw/i386/microvm.h"
+#include "hw/virtio/virtio-mmio.h"
#include "acpi-common.h"
#include "acpi-microvm.h"
@@ -45,6 +47,54 @@ static void acpi_dsdt_add_power_button(Aml *scope)
aml_append(scope, dev);
}
+static void acpi_dsdt_add_virtio(Aml *scope)
+{
+ gchar *separator;
+ long int index;
+ BusState *bus;
+ BusChild *kid;
+
+ bus = sysbus_get_default();
+ QTAILQ_FOREACH(kid, &bus->children, sibling) {
+ DeviceState *dev = kid->child;
+ Object *obj = object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MMIO);
+
+ if (obj) {
+ VirtIOMMIOProxy *mmio = VIRTIO_MMIO(obj);
+ VirtioBusState *mmio_virtio_bus = &mmio->bus;
+ BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
+
+ if (QTAILQ_EMPTY(&mmio_bus->children)) {
+ continue;
+ }
+ separator = g_strrstr(mmio_bus->name, ".");
+ if (!separator) {
+ continue;
+ }
+ if (qemu_strtol(separator + 1, NULL, 10, &index) != 0) {
+ continue;
+ }
+
+ uint32_t irq = VIRTIO_IRQ_BASE + index;
+ hwaddr base = VIRTIO_MMIO_BASE + index * 512;
+ hwaddr size = 512;
+
+ Aml *dev = aml_device("VR%02u", (unsigned)index);
+ aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
+ aml_append(dev, aml_name_decl("_UID", aml_int(index)));
+ aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
+
+ Aml *crs = aml_resource_template();
+ aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
+ aml_append(crs,
+ aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
+ AML_EXCLUSIVE, &irq, 1));
+ aml_append(dev, aml_name_decl("_CRS", crs));
+ aml_append(scope, dev);
+ }
+ }
+}
+
static void
build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
MicrovmMachineState *mms)
@@ -69,6 +119,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
build_ged_aml(sb_scope, GED_DEVICE, HOTPLUG_HANDLER(mms->acpi_dev),
GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
acpi_dsdt_add_power_button(sb_scope);
+ acpi_dsdt_add_virtio(sb_scope);
aml_append(dsdt, sb_scope);
scope = aml_scope("\\");
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 11/13] microvm: add acpi_dsdt_add_virtio() for x86
2020-05-05 13:43 ` [PATCH v2 11/13] microvm: add acpi_dsdt_add_virtio() for x86 Gerd Hoffmann
@ 2020-05-06 7:27 ` Sergio Lopez
0 siblings, 0 replies; 43+ messages in thread
From: Sergio Lopez @ 2020-05-06 7:27 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Igor Mammedov,
Paolo Bonzini, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 3407 bytes --]
On Tue, May 05, 2020 at 03:43:03PM +0200, Gerd Hoffmann wrote:
> Makes x86 linux kernel find virtio-mmio devices automatically.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/i386/acpi-microvm.c | 51 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
Reviewed-by: Sergio Lopez <slp@redhat.com>
> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
> index ce5ab86d642c..4d91ac9360ce 100644
> --- a/hw/i386/acpi-microvm.c
> +++ b/hw/i386/acpi-microvm.c
> @@ -21,6 +21,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
> #include "qapi/error.h"
>
> #include "exec/memory.h"
> @@ -32,6 +33,7 @@
> #include "hw/boards.h"
> #include "hw/i386/fw_cfg.h"
> #include "hw/i386/microvm.h"
> +#include "hw/virtio/virtio-mmio.h"
>
> #include "acpi-common.h"
> #include "acpi-microvm.h"
> @@ -45,6 +47,54 @@ static void acpi_dsdt_add_power_button(Aml *scope)
> aml_append(scope, dev);
> }
>
> +static void acpi_dsdt_add_virtio(Aml *scope)
> +{
> + gchar *separator;
> + long int index;
> + BusState *bus;
> + BusChild *kid;
> +
> + bus = sysbus_get_default();
> + QTAILQ_FOREACH(kid, &bus->children, sibling) {
> + DeviceState *dev = kid->child;
> + Object *obj = object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MMIO);
> +
> + if (obj) {
> + VirtIOMMIOProxy *mmio = VIRTIO_MMIO(obj);
> + VirtioBusState *mmio_virtio_bus = &mmio->bus;
> + BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
> +
> + if (QTAILQ_EMPTY(&mmio_bus->children)) {
> + continue;
> + }
> + separator = g_strrstr(mmio_bus->name, ".");
> + if (!separator) {
> + continue;
> + }
> + if (qemu_strtol(separator + 1, NULL, 10, &index) != 0) {
> + continue;
> + }
> +
> + uint32_t irq = VIRTIO_IRQ_BASE + index;
> + hwaddr base = VIRTIO_MMIO_BASE + index * 512;
> + hwaddr size = 512;
> +
> + Aml *dev = aml_device("VR%02u", (unsigned)index);
> + aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> + aml_append(dev, aml_name_decl("_UID", aml_int(index)));
> + aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +
> + Aml *crs = aml_resource_template();
> + aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE));
> + aml_append(crs,
> + aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> + AML_EXCLUSIVE, &irq, 1));
> + aml_append(dev, aml_name_decl("_CRS", crs));
> + aml_append(scope, dev);
> + }
> + }
> +}
> +
> static void
> build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
> MicrovmMachineState *mms)
> @@ -69,6 +119,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
> build_ged_aml(sb_scope, GED_DEVICE, HOTPLUG_HANDLER(mms->acpi_dev),
> GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
> acpi_dsdt_add_power_button(sb_scope);
> + acpi_dsdt_add_virtio(sb_scope);
> aml_append(dsdt, sb_scope);
>
> scope = aml_scope("\\");
> --
> 2.18.4
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 12/13] microvm: make virtio irq base runtime configurable
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
` (10 preceding siblings ...)
2020-05-05 13:43 ` [PATCH v2 11/13] microvm: add acpi_dsdt_add_virtio() for x86 Gerd Hoffmann
@ 2020-05-05 13:43 ` Gerd Hoffmann
2020-05-06 7:28 ` Sergio Lopez
2020-05-05 13:43 ` [PATCH v2 13/13] microvm/acpi: use GSI 16-23 for virtio Gerd Hoffmann
2020-05-05 14:04 ` [PATCH v2 00/13] microvm: add acpi support Michael S. Tsirkin
13 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:43 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
include/hw/i386/microvm.h | 2 +-
hw/i386/acpi-microvm.c | 6 +++---
hw/i386/microvm.c | 11 +++++++----
3 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
index 55f5984cfaa1..878d2a8011f4 100644
--- a/include/hw/i386/microvm.h
+++ b/include/hw/i386/microvm.h
@@ -28,7 +28,6 @@
/* Platform virtio definitions */
#define VIRTIO_MMIO_BASE 0xc0000000
-#define VIRTIO_IRQ_BASE 5
#define VIRTIO_NUM_TRANSPORTS 8
#define VIRTIO_CMDLINE_MAXLEN 64
@@ -63,6 +62,7 @@ typedef struct {
bool auto_kernel_cmdline;
/* Machine state */
+ uint32_t virtio_irq_base;
bool kernel_cmdline_fixed;
Notifier machine_done;
AcpiDeviceIf *acpi_dev;
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
index 4d91ac9360ce..1230080c45cd 100644
--- a/hw/i386/acpi-microvm.c
+++ b/hw/i386/acpi-microvm.c
@@ -47,7 +47,7 @@ static void acpi_dsdt_add_power_button(Aml *scope)
aml_append(scope, dev);
}
-static void acpi_dsdt_add_virtio(Aml *scope)
+static void acpi_dsdt_add_virtio(Aml *scope, MicrovmMachineState *mms)
{
gchar *separator;
long int index;
@@ -75,7 +75,7 @@ static void acpi_dsdt_add_virtio(Aml *scope)
continue;
}
- uint32_t irq = VIRTIO_IRQ_BASE + index;
+ uint32_t irq = mms->virtio_irq_base + index;
hwaddr base = VIRTIO_MMIO_BASE + index * 512;
hwaddr size = 512;
@@ -119,7 +119,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
build_ged_aml(sb_scope, GED_DEVICE, HOTPLUG_HANDLER(mms->acpi_dev),
GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
acpi_dsdt_add_power_button(sb_scope);
- acpi_dsdt_add_virtio(sb_scope);
+ acpi_dsdt_add_virtio(sb_scope, mms);
aml_append(dsdt, sb_scope);
scope = aml_scope("\\");
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index a3708fdf1e39..2aa2804e4ca0 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -124,10 +124,11 @@ static void microvm_devices_init(MicrovmMachineState *mms)
kvmclock_create();
+ mms->virtio_irq_base = 8;
for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) {
sysbus_create_simple("virtio-mmio",
VIRTIO_MMIO_BASE + i * 512,
- x86ms->gsi[VIRTIO_IRQ_BASE + i]);
+ x86ms->gsi[mms->virtio_irq_base + i]);
}
/* Optional and legacy devices */
@@ -274,7 +275,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
x86ms->ioapic_as = &address_space_memory;
}
-static gchar *microvm_get_mmio_cmdline(gchar *name)
+static gchar *microvm_get_mmio_cmdline(gchar *name, uint32_t virtio_irq_base)
{
gchar *cmdline;
gchar *separator;
@@ -294,7 +295,7 @@ static gchar *microvm_get_mmio_cmdline(gchar *name)
ret = g_snprintf(cmdline, VIRTIO_CMDLINE_MAXLEN,
" virtio_mmio.device=512@0x%lx:%ld",
VIRTIO_MMIO_BASE + index * 512,
- VIRTIO_IRQ_BASE + index);
+ virtio_irq_base + index);
if (ret < 0 || ret >= VIRTIO_CMDLINE_MAXLEN) {
g_free(cmdline);
return NULL;
@@ -306,6 +307,7 @@ static gchar *microvm_get_mmio_cmdline(gchar *name)
static void microvm_fix_kernel_cmdline(MachineState *machine)
{
X86MachineState *x86ms = X86_MACHINE(machine);
+ MicrovmMachineState *mms = MICROVM_MACHINE(machine);
BusState *bus;
BusChild *kid;
char *cmdline;
@@ -329,7 +331,8 @@ static void microvm_fix_kernel_cmdline(MachineState *machine)
BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
if (!QTAILQ_EMPTY(&mmio_bus->children)) {
- gchar *mmio_cmdline = microvm_get_mmio_cmdline(mmio_bus->name);
+ gchar *mmio_cmdline = microvm_get_mmio_cmdline
+ (mmio_bus->name, mms->virtio_irq_base);
if (mmio_cmdline) {
char *newcmd = g_strjoin(NULL, cmdline, mmio_cmdline, NULL);
g_free(mmio_cmdline);
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 12/13] microvm: make virtio irq base runtime configurable
2020-05-05 13:43 ` [PATCH v2 12/13] microvm: make virtio irq base runtime configurable Gerd Hoffmann
@ 2020-05-06 7:28 ` Sergio Lopez
0 siblings, 0 replies; 43+ messages in thread
From: Sergio Lopez @ 2020-05-06 7:28 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Igor Mammedov,
Paolo Bonzini, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 4650 bytes --]
On Tue, May 05, 2020 at 03:43:04PM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> include/hw/i386/microvm.h | 2 +-
> hw/i386/acpi-microvm.c | 6 +++---
> hw/i386/microvm.c | 11 +++++++----
> 3 files changed, 11 insertions(+), 8 deletions(-)
Reviewed-by: Sergio Lopez <slp@redhat.com>
> diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
> index 55f5984cfaa1..878d2a8011f4 100644
> --- a/include/hw/i386/microvm.h
> +++ b/include/hw/i386/microvm.h
> @@ -28,7 +28,6 @@
>
> /* Platform virtio definitions */
> #define VIRTIO_MMIO_BASE 0xc0000000
> -#define VIRTIO_IRQ_BASE 5
> #define VIRTIO_NUM_TRANSPORTS 8
> #define VIRTIO_CMDLINE_MAXLEN 64
>
> @@ -63,6 +62,7 @@ typedef struct {
> bool auto_kernel_cmdline;
>
> /* Machine state */
> + uint32_t virtio_irq_base;
> bool kernel_cmdline_fixed;
> Notifier machine_done;
> AcpiDeviceIf *acpi_dev;
> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
> index 4d91ac9360ce..1230080c45cd 100644
> --- a/hw/i386/acpi-microvm.c
> +++ b/hw/i386/acpi-microvm.c
> @@ -47,7 +47,7 @@ static void acpi_dsdt_add_power_button(Aml *scope)
> aml_append(scope, dev);
> }
>
> -static void acpi_dsdt_add_virtio(Aml *scope)
> +static void acpi_dsdt_add_virtio(Aml *scope, MicrovmMachineState *mms)
> {
> gchar *separator;
> long int index;
> @@ -75,7 +75,7 @@ static void acpi_dsdt_add_virtio(Aml *scope)
> continue;
> }
>
> - uint32_t irq = VIRTIO_IRQ_BASE + index;
> + uint32_t irq = mms->virtio_irq_base + index;
> hwaddr base = VIRTIO_MMIO_BASE + index * 512;
> hwaddr size = 512;
>
> @@ -119,7 +119,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
> build_ged_aml(sb_scope, GED_DEVICE, HOTPLUG_HANDLER(mms->acpi_dev),
> GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
> acpi_dsdt_add_power_button(sb_scope);
> - acpi_dsdt_add_virtio(sb_scope);
> + acpi_dsdt_add_virtio(sb_scope, mms);
> aml_append(dsdt, sb_scope);
>
> scope = aml_scope("\\");
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index a3708fdf1e39..2aa2804e4ca0 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -124,10 +124,11 @@ static void microvm_devices_init(MicrovmMachineState *mms)
>
> kvmclock_create();
>
> + mms->virtio_irq_base = 8;
> for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) {
> sysbus_create_simple("virtio-mmio",
> VIRTIO_MMIO_BASE + i * 512,
> - x86ms->gsi[VIRTIO_IRQ_BASE + i]);
> + x86ms->gsi[mms->virtio_irq_base + i]);
> }
>
> /* Optional and legacy devices */
> @@ -274,7 +275,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
> x86ms->ioapic_as = &address_space_memory;
> }
>
> -static gchar *microvm_get_mmio_cmdline(gchar *name)
> +static gchar *microvm_get_mmio_cmdline(gchar *name, uint32_t virtio_irq_base)
> {
> gchar *cmdline;
> gchar *separator;
> @@ -294,7 +295,7 @@ static gchar *microvm_get_mmio_cmdline(gchar *name)
> ret = g_snprintf(cmdline, VIRTIO_CMDLINE_MAXLEN,
> " virtio_mmio.device=512@0x%lx:%ld",
> VIRTIO_MMIO_BASE + index * 512,
> - VIRTIO_IRQ_BASE + index);
> + virtio_irq_base + index);
> if (ret < 0 || ret >= VIRTIO_CMDLINE_MAXLEN) {
> g_free(cmdline);
> return NULL;
> @@ -306,6 +307,7 @@ static gchar *microvm_get_mmio_cmdline(gchar *name)
> static void microvm_fix_kernel_cmdline(MachineState *machine)
> {
> X86MachineState *x86ms = X86_MACHINE(machine);
> + MicrovmMachineState *mms = MICROVM_MACHINE(machine);
> BusState *bus;
> BusChild *kid;
> char *cmdline;
> @@ -329,7 +331,8 @@ static void microvm_fix_kernel_cmdline(MachineState *machine)
> BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
>
> if (!QTAILQ_EMPTY(&mmio_bus->children)) {
> - gchar *mmio_cmdline = microvm_get_mmio_cmdline(mmio_bus->name);
> + gchar *mmio_cmdline = microvm_get_mmio_cmdline
> + (mmio_bus->name, mms->virtio_irq_base);
> if (mmio_cmdline) {
> char *newcmd = g_strjoin(NULL, cmdline, mmio_cmdline, NULL);
> g_free(mmio_cmdline);
> --
> 2.18.4
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 13/13] microvm/acpi: use GSI 16-23 for virtio
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
` (11 preceding siblings ...)
2020-05-05 13:43 ` [PATCH v2 12/13] microvm: make virtio irq base runtime configurable Gerd Hoffmann
@ 2020-05-05 13:43 ` Gerd Hoffmann
2020-05-06 7:28 ` Sergio Lopez
2020-05-05 14:04 ` [PATCH v2 00/13] microvm: add acpi support Michael S. Tsirkin
13 siblings, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-05 13:43 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Sergio Lopez, Michael S. Tsirkin, Gerd Hoffmann,
Igor Mammedov, Paolo Bonzini, Richard Henderson
With ACPI enabled and IO-APIC being properly declared in the ACPI tables
we can use interrupt lines 16-23 for virtio and avoid shared interrupts.
With acpi disabled we continue to use lines 8-15.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/i386/microvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 2aa2804e4ca0..08ed2a17f2ca 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -124,7 +124,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
kvmclock_create();
- mms->virtio_irq_base = 8;
+ mms->virtio_irq_base = x86_machine_is_acpi_enabled(x86ms) ? 16 : 8;
for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) {
sysbus_create_simple("virtio-mmio",
VIRTIO_MMIO_BASE + i * 512,
--
2.18.4
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 13/13] microvm/acpi: use GSI 16-23 for virtio
2020-05-05 13:43 ` [PATCH v2 13/13] microvm/acpi: use GSI 16-23 for virtio Gerd Hoffmann
@ 2020-05-06 7:28 ` Sergio Lopez
0 siblings, 0 replies; 43+ messages in thread
From: Sergio Lopez @ 2020-05-06 7:28 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Igor Mammedov,
Paolo Bonzini, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]
On Tue, May 05, 2020 at 03:43:05PM +0200, Gerd Hoffmann wrote:
> With ACPI enabled and IO-APIC being properly declared in the ACPI tables
> we can use interrupt lines 16-23 for virtio and avoid shared interrupts.
>
> With acpi disabled we continue to use lines 8-15.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/i386/microvm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Sergio Lopez <slp@redhat.com>
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 2aa2804e4ca0..08ed2a17f2ca 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -124,7 +124,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
>
> kvmclock_create();
>
> - mms->virtio_irq_base = 8;
> + mms->virtio_irq_base = x86_machine_is_acpi_enabled(x86ms) ? 16 : 8;
> for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) {
> sysbus_create_simple("virtio-mmio",
> VIRTIO_MMIO_BASE + i * 512,
> --
> 2.18.4
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 00/13] microvm: add acpi support
2020-05-05 13:42 [PATCH v2 00/13] microvm: add acpi support Gerd Hoffmann
` (12 preceding siblings ...)
2020-05-05 13:43 ` [PATCH v2 13/13] microvm/acpi: use GSI 16-23 for virtio Gerd Hoffmann
@ 2020-05-05 14:04 ` Michael S. Tsirkin
2020-05-05 14:16 ` Philippe Mathieu-Daudé
2020-05-06 11:46 ` Gerd Hoffmann
13 siblings, 2 replies; 43+ messages in thread
From: Michael S. Tsirkin @ 2020-05-05 14:04 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, qemu-devel, Igor Mammedov,
Paolo Bonzini, Richard Henderson
On Tue, May 05, 2020 at 03:42:52PM +0200, Gerd Hoffmann wrote:
> I know that not supporting ACPI in microvm is intentional. If you still
> don't want ACPI this is perfectly fine, you can use the usual -no-acpi
> switch to toggle ACPI support.
>
> These are the advantages you are going to loose then:
>
> (1) virtio-mmio device discovery without command line hacks (tweaking
> the command line is a problem when not using direct kernel boot).
> (2) Better IO-APIC support, we can use IRQ lines 16-23.
> (3) ACPI power button (aka powerdown request) works.
> (4) machine poweroff (aka S5 state) works.
Questions
- what's the tradeoff in startup time?
- what should be the default?
Based on above I'd be inclined to say default should stay no acpi and
users should enable acpi with an option.
> Together with seabios patches for virtio-mmio support this allows to
> boot standard fedora images (cloud, coreos, workstation live) with the
> microvm machine type.
>
> git branch for testing (including updated seabios):
> https://git.kraxel.org/cgit/qemu/log/?h=sirius/microvm
>
> changes in v2:
> * some acpi cleanups are an separate patch series now.
> * switched to hw reduced acpi & generic event device.
> * misc fixes here and there.
>
> cheers,
> Gerd
>
> Gerd Hoffmann (13):
> acpi: make build_madt() more generic.
> acpi: create acpi-common.c and move madt code
> acpi: madt: skip pci override on pci-less systems (microvm)
> acpi: move acpi_build_facs to acpi-common.c
> acpi: move acpi_init_common_fadt_data to acpi-common.c
> acpi: move acpi_align_size to acpi-common.h
> acpi: fadt: add hw-reduced sleep register support
> acpi: generic event device for x86
> microvm: add minimal acpi support
> microvm: disable virtio-mmio cmdline hack
> microvm: add acpi_dsdt_add_virtio() for x86
> microvm: make virtio irq base runtime configurable
> microvm/acpi: use GSI 16-23 for virtio
>
> hw/i386/acpi-common.h | 38 ++++
> hw/i386/acpi-microvm.h | 6 +
> include/hw/acpi/acpi-defs.h | 2 +
> include/hw/acpi/generic_event_device.h | 10 +
> include/hw/i386/microvm.h | 10 +-
> hw/acpi/aml-build.c | 4 +-
> hw/i386/acpi-build.c | 198 +-------------------
> hw/i386/acpi-common.c | 206 ++++++++++++++++++++
> hw/i386/acpi-microvm.c | 249 +++++++++++++++++++++++++
> hw/i386/generic_event_device_x86.c | 114 +++++++++++
> hw/i386/microvm.c | 36 +++-
> hw/i386/Kconfig | 1 +
> hw/i386/Makefile.objs | 3 +
> 13 files changed, 676 insertions(+), 201 deletions(-)
> create mode 100644 hw/i386/acpi-common.h
> create mode 100644 hw/i386/acpi-microvm.h
> create mode 100644 hw/i386/acpi-common.c
> create mode 100644 hw/i386/acpi-microvm.c
> create mode 100644 hw/i386/generic_event_device_x86.c
>
> --
> 2.18.4
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 00/13] microvm: add acpi support
2020-05-05 14:04 ` [PATCH v2 00/13] microvm: add acpi support Michael S. Tsirkin
@ 2020-05-05 14:16 ` Philippe Mathieu-Daudé
2020-05-06 7:25 ` Sergio Lopez
2020-05-06 11:46 ` Gerd Hoffmann
1 sibling, 1 reply; 43+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-05 14:16 UTC (permalink / raw)
To: Michael S. Tsirkin, Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, qemu-devel, Paolo Bonzini,
Igor Mammedov, Richard Henderson
On 5/5/20 4:04 PM, Michael S. Tsirkin wrote:
> On Tue, May 05, 2020 at 03:42:52PM +0200, Gerd Hoffmann wrote:
>> I know that not supporting ACPI in microvm is intentional. If you still
>> don't want ACPI this is perfectly fine, you can use the usual -no-acpi
>> switch to toggle ACPI support.
>>
>> These are the advantages you are going to loose then:
>>
>> (1) virtio-mmio device discovery without command line hacks (tweaking
>> the command line is a problem when not using direct kernel boot).
>> (2) Better IO-APIC support, we can use IRQ lines 16-23.
>> (3) ACPI power button (aka powerdown request) works.
>> (4) machine poweroff (aka S5 state) works.
>
> Questions
>
> - what's the tradeoff in startup time?
> - what should be the default?
>
> Based on above I'd be inclined to say default should stay no acpi and
> users should enable acpi with an option.
As this machine was added to have the least minimum hardware required,
I'd keep the default with no ACPI and have user requiring it to use an
option. My 2 cents obviously.
>
>> Together with seabios patches for virtio-mmio support this allows to
>> boot standard fedora images (cloud, coreos, workstation live) with the
>> microvm machine type.
>>
>> git branch for testing (including updated seabios):
>> https://git.kraxel.org/cgit/qemu/log/?h=sirius/microvm
>>
>> changes in v2:
>> * some acpi cleanups are an separate patch series now.
>> * switched to hw reduced acpi & generic event device.
>> * misc fixes here and there.
>>
>> cheers,
>> Gerd
>>
>> Gerd Hoffmann (13):
>> acpi: make build_madt() more generic.
>> acpi: create acpi-common.c and move madt code
>> acpi: madt: skip pci override on pci-less systems (microvm)
>> acpi: move acpi_build_facs to acpi-common.c
>> acpi: move acpi_init_common_fadt_data to acpi-common.c
>> acpi: move acpi_align_size to acpi-common.h
>> acpi: fadt: add hw-reduced sleep register support
>> acpi: generic event device for x86
>> microvm: add minimal acpi support
>> microvm: disable virtio-mmio cmdline hack
>> microvm: add acpi_dsdt_add_virtio() for x86
>> microvm: make virtio irq base runtime configurable
>> microvm/acpi: use GSI 16-23 for virtio
>>
>> hw/i386/acpi-common.h | 38 ++++
>> hw/i386/acpi-microvm.h | 6 +
>> include/hw/acpi/acpi-defs.h | 2 +
>> include/hw/acpi/generic_event_device.h | 10 +
>> include/hw/i386/microvm.h | 10 +-
>> hw/acpi/aml-build.c | 4 +-
>> hw/i386/acpi-build.c | 198 +-------------------
>> hw/i386/acpi-common.c | 206 ++++++++++++++++++++
>> hw/i386/acpi-microvm.c | 249 +++++++++++++++++++++++++
>> hw/i386/generic_event_device_x86.c | 114 +++++++++++
>> hw/i386/microvm.c | 36 +++-
>> hw/i386/Kconfig | 1 +
>> hw/i386/Makefile.objs | 3 +
>> 13 files changed, 676 insertions(+), 201 deletions(-)
>> create mode 100644 hw/i386/acpi-common.h
>> create mode 100644 hw/i386/acpi-microvm.h
>> create mode 100644 hw/i386/acpi-common.c
>> create mode 100644 hw/i386/acpi-microvm.c
>> create mode 100644 hw/i386/generic_event_device_x86.c
>>
>> --
>> 2.18.4
>
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 00/13] microvm: add acpi support
2020-05-05 14:16 ` Philippe Mathieu-Daudé
@ 2020-05-06 7:25 ` Sergio Lopez
0 siblings, 0 replies; 43+ messages in thread
From: Sergio Lopez @ 2020-05-06 7:25 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Gerd Hoffmann,
Paolo Bonzini, Igor Mammedov, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 3871 bytes --]
On Tue, May 05, 2020 at 04:16:00PM +0200, Philippe Mathieu-Daudé wrote:
> On 5/5/20 4:04 PM, Michael S. Tsirkin wrote:
> > On Tue, May 05, 2020 at 03:42:52PM +0200, Gerd Hoffmann wrote:
> > > I know that not supporting ACPI in microvm is intentional. If you still
> > > don't want ACPI this is perfectly fine, you can use the usual -no-acpi
> > > switch to toggle ACPI support.
> > >
> > > These are the advantages you are going to loose then:
> > >
> > > (1) virtio-mmio device discovery without command line hacks (tweaking
> > > the command line is a problem when not using direct kernel boot).
> > > (2) Better IO-APIC support, we can use IRQ lines 16-23.
> > > (3) ACPI power button (aka powerdown request) works.
> > > (4) machine poweroff (aka S5 state) works.
> >
> > Questions
> >
> > - what's the tradeoff in startup time?
> > - what should be the default?
> >
> > Based on above I'd be inclined to say default should stay no acpi and
> > users should enable acpi with an option.
>
> As this machine was added to have the least minimum hardware required, I'd
> keep the default with no ACPI and have user requiring it to use an option.
> My 2 cents obviously.
I also share this opinion. And I would prefer it to be a machine type
option, defaulting to "off", but I guess that's a matter of taste.
Sergio.
> >
> > > Together with seabios patches for virtio-mmio support this allows to
> > > boot standard fedora images (cloud, coreos, workstation live) with the
> > > microvm machine type.
> > >
> > > git branch for testing (including updated seabios):
> > > https://git.kraxel.org/cgit/qemu/log/?h=sirius/microvm
> > >
> > > changes in v2:
> > > * some acpi cleanups are an separate patch series now.
> > > * switched to hw reduced acpi & generic event device.
> > > * misc fixes here and there.
> > >
> > > cheers,
> > > Gerd
> > >
> > > Gerd Hoffmann (13):
> > > acpi: make build_madt() more generic.
> > > acpi: create acpi-common.c and move madt code
> > > acpi: madt: skip pci override on pci-less systems (microvm)
> > > acpi: move acpi_build_facs to acpi-common.c
> > > acpi: move acpi_init_common_fadt_data to acpi-common.c
> > > acpi: move acpi_align_size to acpi-common.h
> > > acpi: fadt: add hw-reduced sleep register support
> > > acpi: generic event device for x86
> > > microvm: add minimal acpi support
> > > microvm: disable virtio-mmio cmdline hack
> > > microvm: add acpi_dsdt_add_virtio() for x86
> > > microvm: make virtio irq base runtime configurable
> > > microvm/acpi: use GSI 16-23 for virtio
> > >
> > > hw/i386/acpi-common.h | 38 ++++
> > > hw/i386/acpi-microvm.h | 6 +
> > > include/hw/acpi/acpi-defs.h | 2 +
> > > include/hw/acpi/generic_event_device.h | 10 +
> > > include/hw/i386/microvm.h | 10 +-
> > > hw/acpi/aml-build.c | 4 +-
> > > hw/i386/acpi-build.c | 198 +-------------------
> > > hw/i386/acpi-common.c | 206 ++++++++++++++++++++
> > > hw/i386/acpi-microvm.c | 249 +++++++++++++++++++++++++
> > > hw/i386/generic_event_device_x86.c | 114 +++++++++++
> > > hw/i386/microvm.c | 36 +++-
> > > hw/i386/Kconfig | 1 +
> > > hw/i386/Makefile.objs | 3 +
> > > 13 files changed, 676 insertions(+), 201 deletions(-)
> > > create mode 100644 hw/i386/acpi-common.h
> > > create mode 100644 hw/i386/acpi-microvm.h
> > > create mode 100644 hw/i386/acpi-common.c
> > > create mode 100644 hw/i386/acpi-microvm.c
> > > create mode 100644 hw/i386/generic_event_device_x86.c
> > >
> > > --
> > > 2.18.4
> >
> >
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 00/13] microvm: add acpi support
2020-05-05 14:04 ` [PATCH v2 00/13] microvm: add acpi support Michael S. Tsirkin
2020-05-05 14:16 ` Philippe Mathieu-Daudé
@ 2020-05-06 11:46 ` Gerd Hoffmann
2020-05-06 11:50 ` Michael S. Tsirkin
1 sibling, 1 reply; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-06 11:46 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eduardo Habkost, Sergio Lopez, qemu-devel, Igor Mammedov,
Paolo Bonzini, Richard Henderson
On Tue, May 05, 2020 at 10:04:02AM -0400, Michael S. Tsirkin wrote:
> On Tue, May 05, 2020 at 03:42:52PM +0200, Gerd Hoffmann wrote:
> > I know that not supporting ACPI in microvm is intentional. If you still
> > don't want ACPI this is perfectly fine, you can use the usual -no-acpi
> > switch to toggle ACPI support.
> >
> > These are the advantages you are going to loose then:
> >
> > (1) virtio-mmio device discovery without command line hacks (tweaking
> > the command line is a problem when not using direct kernel boot).
> > (2) Better IO-APIC support, we can use IRQ lines 16-23.
> > (3) ACPI power button (aka powerdown request) works.
> > (4) machine poweroff (aka S5 state) works.
>
> Questions
>
> - what's the tradeoff in startup time?
In the noise. 0.28-0.29 seconds on my hardware to the "i8042: PNP: No
PS/2 controller found" message, no matter whenever acpi is on or off.
With "quiet" (acpi prints more and logging to the serial console is
slow).
At that point -no-acpi takes one second to figure the ps2 controller
really isn't there (as discussed before).
Another interesting difference is interrupt handling.
The -no-acpi version:
CPU0
2: 0 XT-PIC cascade
4: 284 IO-APIC 4-edge ttyS0
8: 0 IO-APIC 8-edge rtc0
14: 5399 IO-APIC 14-edge virtio1
15: 58 IO-APIC 15-edge virtio0
NMI: 0 Non-maskable interrupts
[ ... ]
The acpi version:
CPU0
1: 0 IO-APIC 9-edge ACPI:Ged
2: 231 IO-APIC 23-fasteoi virtio0
3: 6291 IO-APIC 22-fasteoi virtio1
4: 1758 IO-APIC 4-edge ttyS0
5: 0 IO-APIC 8-edge rtc0
NMI: 0 Non-maskable interrupts
[ ... ]
> - what should be the default?
IMO it makes sense to enable it by default. You get working
power management. You can boot stock cloud images (patched
seabios parses the dsdt to find virtio-mmio devices to boot
from virtio-mmio disks).
It's easier to leave behind legacy stuff: The kernel trusts the
firmware and doesn't go into "trying harder to find ps2 kbd" mode.
Also what is this "cascade" thing in /proc/interrupts above? [1]
I expect dropping the rtc is easier with acpi too, the kernel probably
wouldn't try to find it then. Right now seabios needs rtc cmos for
ram size probing, so I didn't test that yet.
On the other hand I don't really see any disadvantages. The tables are
small ...
# find /sys/firmware/acpi/tables/ -type f | xargs ls -l
-r--------. 1 root root 70 May 6 06:48 /sys/firmware/acpi/tables/APIC
-r--------. 1 root root 472 May 6 06:48 /sys/firmware/acpi/tables/DSDT
-r--------. 1 root root 268 May 6 06:48 /sys/firmware/acpi/tables/FACP
... and simple (no methods) so you can hardly call that "bloat".
> Based on above I'd be inclined to say default should stay no acpi and
> users should enable acpi with an option.
I disagree, but I can live with off by default too. We already have
acpi=OnOffAuto for X86MachineState, so it is just a matter of handling
microvm.acpi=auto accordingly in x86_machine_is_acpi_enabled().
take care,
Gerd
[1] Rhetorical question, I know what it is. [2]
[2] I don't want remember though.
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 00/13] microvm: add acpi support
2020-05-06 11:46 ` Gerd Hoffmann
@ 2020-05-06 11:50 ` Michael S. Tsirkin
2020-05-07 13:57 ` Gerd Hoffmann
0 siblings, 1 reply; 43+ messages in thread
From: Michael S. Tsirkin @ 2020-05-06 11:50 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Sergio Lopez, qemu-devel, Igor Mammedov,
Paolo Bonzini, Richard Henderson
On Wed, May 06, 2020 at 01:46:35PM +0200, Gerd Hoffmann wrote:
> On Tue, May 05, 2020 at 10:04:02AM -0400, Michael S. Tsirkin wrote:
> > On Tue, May 05, 2020 at 03:42:52PM +0200, Gerd Hoffmann wrote:
> > > I know that not supporting ACPI in microvm is intentional. If you still
> > > don't want ACPI this is perfectly fine, you can use the usual -no-acpi
> > > switch to toggle ACPI support.
> > >
> > > These are the advantages you are going to loose then:
> > >
> > > (1) virtio-mmio device discovery without command line hacks (tweaking
> > > the command line is a problem when not using direct kernel boot).
> > > (2) Better IO-APIC support, we can use IRQ lines 16-23.
> > > (3) ACPI power button (aka powerdown request) works.
> > > (4) machine poweroff (aka S5 state) works.
> >
> > Questions
> >
> > - what's the tradeoff in startup time?
>
> In the noise. 0.28-0.29 seconds on my hardware to the "i8042: PNP: No
> PS/2 controller found" message, no matter whenever acpi is on or off.
> With "quiet" (acpi prints more and logging to the serial console is
> slow).
>
> At that point -no-acpi takes one second to figure the ps2 controller
> really isn't there (as discussed before).
>
> Another interesting difference is interrupt handling.
>
> The -no-acpi version:
>
> CPU0
> 2: 0 XT-PIC cascade
> 4: 284 IO-APIC 4-edge ttyS0
> 8: 0 IO-APIC 8-edge rtc0
> 14: 5399 IO-APIC 14-edge virtio1
> 15: 58 IO-APIC 15-edge virtio0
> NMI: 0 Non-maskable interrupts
> [ ... ]
>
> The acpi version:
>
> CPU0
> 1: 0 IO-APIC 9-edge ACPI:Ged
> 2: 231 IO-APIC 23-fasteoi virtio0
> 3: 6291 IO-APIC 22-fasteoi virtio1
> 4: 1758 IO-APIC 4-edge ttyS0
> 5: 0 IO-APIC 8-edge rtc0
> NMI: 0 Non-maskable interrupts
> [ ... ]
>
> > - what should be the default?
>
> IMO it makes sense to enable it by default. You get working
> power management. You can boot stock cloud images (patched
> seabios parses the dsdt to find virtio-mmio devices to boot
> from virtio-mmio disks).
>
> It's easier to leave behind legacy stuff: The kernel trusts the
> firmware and doesn't go into "trying harder to find ps2 kbd" mode.
> Also what is this "cascade" thing in /proc/interrupts above? [1]
>
> I expect dropping the rtc is easier with acpi too, the kernel probably
> wouldn't try to find it then. Right now seabios needs rtc cmos for
> ram size probing, so I didn't test that yet.
>
> On the other hand I don't really see any disadvantages. The tables are
> small ...
>
> # find /sys/firmware/acpi/tables/ -type f | xargs ls -l
> -r--------. 1 root root 70 May 6 06:48 /sys/firmware/acpi/tables/APIC
> -r--------. 1 root root 472 May 6 06:48 /sys/firmware/acpi/tables/DSDT
> -r--------. 1 root root 268 May 6 06:48 /sys/firmware/acpi/tables/FACP
>
> ... and simple (no methods) so you can hardly call that "bloat".
>
> > Based on above I'd be inclined to say default should stay no acpi and
> > users should enable acpi with an option.
>
> I disagree, but I can live with off by default too. We already have
> acpi=OnOffAuto for X86MachineState, so it is just a matter of handling
> microvm.acpi=auto accordingly in x86_machine_is_acpi_enabled().
>
> take care,
> Gerd
>
> [1] Rhetorical question, I know what it is. [2]
> [2] I don't want remember though.
Let's leave flipping the default as a separate patch, to be
decided on merits after a bunch of people test with/without.
--
MST
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 00/13] microvm: add acpi support
2020-05-06 11:50 ` Michael S. Tsirkin
@ 2020-05-07 13:57 ` Gerd Hoffmann
0 siblings, 0 replies; 43+ messages in thread
From: Gerd Hoffmann @ 2020-05-07 13:57 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Eduardo Habkost, Sergio Lopez, qemu-devel, Igor Mammedov,
Paolo Bonzini, Richard Henderson
Hi,
> > I expect dropping the rtc is easier with acpi too, the kernel probably
> > wouldn't try to find it then. Right now seabios needs rtc cmos for
> > ram size probing, so I didn't test that yet.
Confirmed. With rtc=off I get this ...
CPU0
1: 0 IO-APIC 9-edge ACPI:Ged
2: 62 IO-APIC 23-fasteoi virtio0
3: 5664 IO-APIC 22-fasteoi virtio1
4: 1854 IO-APIC 4-edge ttyS0
NMI: 0 Non-maskable interrupts
[ ... ]
... i.e. rtc irq is gone.
> Let's leave flipping the default as a separate patch, to be
> decided on merits after a bunch of people test with/without.
Sure.
take care,
Gerd
^ permalink raw reply [flat|nested] 43+ messages in thread