From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34494) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yg5GE-0007mD-6x for qemu-devel@nongnu.org; Thu, 09 Apr 2015 01:44:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yg5GA-0000XP-VA for qemu-devel@nongnu.org; Thu, 09 Apr 2015 01:44:18 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:23400) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yg5G9-0000HB-Sl for qemu-devel@nongnu.org; Thu, 09 Apr 2015 01:44:14 -0400 Message-ID: <5526118D.40700@huawei.com> Date: Thu, 9 Apr 2015 13:43:41 +0800 From: Shannon Zhao MIME-Version: 1.0 References: <1428055432-12120-1-git-send-email-zhaoshenglong@huawei.com> <1428055432-12120-4-git-send-email-zhaoshenglong@huawei.com> <87384a4rer.fsf@linaro.org> In-Reply-To: <87384a4rer.fsf@linaro.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v4 03/20] hw/arm/virt-acpi-build: Basic framework for building ACPI tables on ARM List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QWxleCBCZW5uw6ll?= Cc: peter.maydell@linaro.org, hangaohuai@huawei.com, mst@redhat.com, a.spyridakis@virtualopensystems.com, msalter@redhat.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, peter.huangpeng@huawei.com, hanjun.guo@linaro.org, imammedo@redhat.com, pbonzini@redhat.com, lersek@redhat.com, christoffer.dall@linaro.org, shannon.zhao@linaro.org On 2015/4/8 22:37, Alex Bennée wrote: > > Shannon Zhao writes: > >> From: Shannon Zhao >> >> Introduce a preliminary framework in virt-acpi-build.c with the main >> ACPI build functions. It exposes the generated ACPI contents to >> guest over fw_cfg. >> >> The required ACPI v5.1 tables for ARM are: >> - RSDP: Initial table that points to XSDT >> - RSDT: Points to FADT GTDT MADT tables >> - FADT: Generic information about the machine >> - GTDT: Generic timer description table >> - MADT: Multiple APIC description table >> - DSDT: Holds all information about system devices/peripherals, pointed by FADT >> >> Signed-off-by: Shannon Zhao >> Signed-off-by: Shannon Zhao >> --- >> hw/arm/Makefile.objs | 1 + >> hw/arm/virt-acpi-build.c | 198 +++++++++++++++++++++++++++++++++++++++ >> include/hw/arm/virt-acpi-build.h | 65 +++++++++++++ >> 3 files changed, 264 insertions(+) >> create mode 100644 hw/arm/virt-acpi-build.c >> create mode 100644 include/hw/arm/virt-acpi-build.h >> >> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs >> index 2577f68..a1bfb19 100644 >> --- a/hw/arm/Makefile.objs >> +++ b/hw/arm/Makefile.objs >> @@ -3,6 +3,7 @@ obj-$(CONFIG_DIGIC) += digic_boards.o >> obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o >> obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o >> obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o >> +obj-$(CONFIG_ACPI) += virt-acpi-build.o >> obj-y += netduino2.o >> >> obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c >> new file mode 100644 >> index 0000000..388838a >> --- /dev/null >> +++ b/hw/arm/virt-acpi-build.c >> @@ -0,0 +1,198 @@ >> +/* Support for generating ACPI tables and passing them to Guests >> + * >> + * ARM virt ACPI generation >> + * >> + * Copyright (C) 2008-2010 Kevin O'Connor >> + * Copyright (C) 2006 Fabrice Bellard >> + * Copyright (C) 2013 Red Hat Inc >> + * >> + * Author: Michael S. Tsirkin >> + * >> + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD. >> + * >> + * Author: Shannon Zhao >> + * >> + * 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 . >> + */ >> + >> +#include "hw/arm/virt-acpi-build.h" >> +#include >> +#include >> +#include "qemu-common.h" >> +#include "qemu/bitmap.h" >> +#include "qemu/osdep.h" >> +#include "qemu/range.h" >> +#include "qemu/error-report.h" >> +#include "qom/cpu.h" >> +#include "target-arm/cpu.h" >> +#include "hw/acpi/acpi-defs.h" >> +#include "hw/acpi/acpi.h" >> +#include "hw/nvram/fw_cfg.h" >> +#include "hw/acpi/bios-linker-loader.h" >> +#include "hw/loader.h" >> +#include "hw/hw.h" >> + >> +#include "hw/acpi/aml-build.h" >> + >> +#include "qapi/qmp/qint.h" >> +#include "qom/qom-qobject.h" >> +#include "exec/ram_addr.h" >> + >> +/* #define DEBUG_ACPI_BUILD */ >> +#ifdef DEBUG_ACPI_BUILD >> +#define ACPI_BUILD_DPRINTF(fmt, ...) \ >> + do {printf("ACPI_BUILD: " fmt, ## __VA_ARGS__); } while (0) >> +#else >> +#define ACPI_BUILD_DPRINTF(fmt, ...) >> +#endif > > I'd be tempted to rename this to D or at a push VIRT_ACPI_DEBUG just to > make it distinct from where it was copied from. > > You could also consider something like: > > printf("%s: " fmt, __func__, ##__VA_ARGS__); > > So log statements are pre-pended with their source functions. > Thanks, will fix it. >> + >> +typedef >> +struct AcpiBuildState { >> + /* Copy of table in RAM (for patching). */ >> + ram_addr_t table_ram; >> + ram_addr_t rsdp_ram; >> + ram_addr_t linker_ram; >> + /* Is table patched? */ >> + uint8_t patched; >> + VirtGuestInfo *guest_info; >> +} AcpiBuildState; >> + >> +static >> +void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables) >> +{ >> + GArray *table_offsets; >> + >> + table_offsets = g_array_new(false, true /* clear */, >> + sizeof(uint32_t)); >> + >> + bios_linker_loader_alloc(tables->linker, ACPI_BUILD_TABLE_FILE, >> + 64, false /* high memory */); >> + >> + /* >> + * The ACPI v5.1 tables for Hardware-reduced ACPI platform are: >> + * RSDP >> + * RSDT >> + * FADT >> + * GTDT >> + * MADT >> + * DSDT >> + */ >> + >> + /* Cleanup memory that's no longer used. */ >> + g_array_free(table_offsets, true); >> +} >> + >> +static void acpi_ram_update(ram_addr_t ram, GArray *data) >> +{ >> + uint32_t size = acpi_data_len(data); >> + >> + /* Make sure RAM size is correct - in case it got changed >> + * e.g. by migration */ >> + qemu_ram_resize(ram, size, &error_abort); >> + >> + memcpy(qemu_get_ram_ptr(ram), data->data, size); >> + cpu_physical_memory_set_dirty_range_nocode(ram, size); >> +} >> + >> +static void virt_acpi_build_update(void *build_opaque, uint32_t offset) >> +{ >> + AcpiBuildState *build_state = build_opaque; >> + AcpiBuildTables tables; >> + >> + /* No state to update or already patched? Nothing to do. */ >> + if (!build_state || build_state->patched) { >> + return; >> + } >> + build_state->patched = 1; >> + >> + acpi_build_tables_init(&tables); >> + >> + virt_acpi_build(build_state->guest_info, &tables); >> + >> + acpi_ram_update(build_state->table_ram, tables.table_data); >> + acpi_ram_update(build_state->rsdp_ram, tables.rsdp); >> + acpi_ram_update(build_state->linker_ram, tables.linker); >> + >> + >> + acpi_build_tables_cleanup(&tables, true); >> +} >> + >> +static void virt_acpi_build_reset(void *build_opaque) >> +{ >> + AcpiBuildState *build_state = build_opaque; >> + build_state->patched = 0; >> +} >> + >> +static ram_addr_t acpi_add_rom_blob(AcpiBuildState *build_state, GArray *blob, >> + const char *name, uint64_t max_size) >> +{ >> + return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1, >> + name, virt_acpi_build_update, build_state); >> +} >> + >> +static const VMStateDescription vmstate_virt_acpi_build = { >> + .name = "virt_acpi_build", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT8(patched, AcpiBuildState), >> + VMSTATE_END_OF_LIST() >> + }, >> +}; >> + >> +void virt_acpi_setup(VirtGuestInfo *guest_info) >> +{ >> + AcpiBuildTables tables; >> + AcpiBuildState *build_state; >> + >> + if (!guest_info->fw_cfg) { >> + ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n"); >> + return; >> + } >> + >> + if (!acpi_enabled) { >> + ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n"); >> + return; >> + } > > Are these debug errors or something that should be error_report() to the user? > This is not an error. It works when user appends "-no-acpi" option to qemu command line. I should add this option support on ARM in the qemu-options.hx. Will add at next version. diff --git a/qemu-options.hx b/qemu-options.hx index 319d971..82bcc9b 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1352,7 +1352,7 @@ be needed to boot from old floppy disks. ETEXI DEF("no-acpi", 0, QEMU_OPTION_no_acpi, - "-no-acpi disable ACPI\n", QEMU_ARCH_I386) + "-no-acpi disable ACPI\n", QEMU_ARCH_I386 | QEMU_ARCH_ARM) STEXI @item -no-acpi @findex -no-acpi >> + >> + build_state = g_malloc0(sizeof *build_state); >> + build_state->guest_info = guest_info; >> + >> + acpi_build_tables_init(&tables); >> + virt_acpi_build(build_state->guest_info, &tables); >> + >> + /* Now expose it all to Guest */ >> + build_state->table_ram = acpi_add_rom_blob(build_state, tables.table_data, >> + ACPI_BUILD_TABLE_FILE, >> + ACPI_BUILD_TABLE_MAX_SIZE); >> + assert(build_state->table_ram != RAM_ADDR_MAX); >> + >> + build_state->linker_ram = >> + acpi_add_rom_blob(build_state, tables.linker, "etc/table-loader", 0); >> + >> + fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_TPMLOG_FILE, >> + tables.tcpalog->data, acpi_data_len(tables.tcpalog)); >> + >> + build_state->rsdp_ram = acpi_add_rom_blob(build_state, tables.rsdp, >> + ACPI_BUILD_RSDP_FILE, 0); >> + >> + qemu_register_reset(virt_acpi_build_reset, build_state); >> + virt_acpi_build_reset(build_state); >> + vmstate_register(NULL, 0, &vmstate_virt_acpi_build, build_state); >> + >> + /* Cleanup tables but don't free the memory: we track it >> + * in build_state. >> + */ >> + acpi_build_tables_cleanup(&tables, false); > > I'm confused here but I see it comes from the i386 code. What do we need > to keep track of after we've built the tables up? > We track it for updating tables. >> +} >> diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h >> new file mode 100644 >> index 0000000..175eac2 >> --- /dev/null >> +++ b/include/hw/arm/virt-acpi-build.h >> @@ -0,0 +1,65 @@ >> +/* >> + * >> + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD. >> + * >> + * Author: Shannon Zhao >> + * >> + * 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. >> + * >> + * This program is distributed in the hope 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 . >> + */ >> + >> +#ifndef QEMU_VIRT_ACPI_BUILD_H >> +#define QEMU_VIRT_ACPI_BUILD_H >> + >> +#include "qemu-common.h" >> + >> +typedef struct acpi_gtdt_info { >> + uint32_t timer_virt; >> + uint32_t timer_s_el1; >> + uint32_t timer_ns_el1; >> + uint32_t timer_ns_el2; >> +} acpi_gtdt_info; >> + >> +typedef struct acpi_madt_info { >> + const hwaddr *gic_cpu_base_addr; >> + const hwaddr *gic_dist_base_addr; >> +} acpi_madt_info; >> + >> +typedef struct acpi_dsdt_info { >> + const hwaddr *uart_addr; >> + const int *uart_irq; >> + const hwaddr *virtio_mmio_addr; >> + const int *virtio_mmio_irq; >> + int virtio_mmio_num; >> + const hwaddr *rtc_addr; >> + const int *rtc_irq; >> + const hwaddr *flash_addr; >> +} acpi_dsdt_info; >> + >> +typedef struct VirtGuestInfo { >> + int smp_cpus; >> + int max_cpus; >> + FWCfgState *fw_cfg; >> + acpi_madt_info *madt_info; >> + acpi_dsdt_info *dsdt_info; >> + acpi_gtdt_info *gtdt_info; >> +} VirtGuestInfo; >> + >> + >> +typedef struct VirtGuestInfoState { >> + VirtGuestInfo info; >> + Notifier machine_done; >> +} VirtGuestInfoState; >> + >> +void virt_acpi_setup(VirtGuestInfo *guest_info); >> + >> +#endif >