From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33290) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YpEMK-0003Mv-6o for qemu-devel@nongnu.org; Mon, 04 May 2015 07:16:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YpEMF-0003Su-3t for qemu-devel@nongnu.org; Mon, 04 May 2015 07:16:24 -0400 Received: from mail-pa0-f41.google.com ([209.85.220.41]:36844) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YpEME-0003Sh-Rl for qemu-devel@nongnu.org; Mon, 04 May 2015 07:16:19 -0400 Received: by pabsx10 with SMTP id sx10so158012609pab.3 for ; Mon, 04 May 2015 04:16:18 -0700 (PDT) Message-ID: <554754FA.1070301@linaro.org> Date: Mon, 04 May 2015 19:16:10 +0800 From: Shannon Zhao MIME-Version: 1.0 References: <1429104309-3844-1-git-send-email-zhaoshenglong@huawei.com> <1429104309-3844-9-git-send-email-zhaoshenglong@huawei.com> <20150504122125.47606f26@nial.brq.redhat.com> In-Reply-To: <20150504122125.47606f26@nial.brq.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v5 08/20] hw/arm/virt-acpi-build: Generate MADT table List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov , Shannon Zhao Cc: peter.maydell@linaro.org, hangaohuai@huawei.com, mst@redhat.com, a.spyridakis@virtualopensystems.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, peter.huangpeng@huawei.com, alex.bennee@linaro.org, hanjun.guo@linaro.org, pbonzini@redhat.com, lersek@redhat.com, christoffer.dall@linaro.org On 2015/5/4 18:21, Igor Mammedov wrote: > On Wed, 15 Apr 2015 21:24:57 +0800 > Shannon Zhao wrote: > >> From: Shannon Zhao >> >> MADT describes GIC enabled ARM platforms. The GICC and GICD >> subtables are used to define the GIC regions. >> >> Signed-off-by: Shannon Zhao >> Signed-off-by: Shannon Zhao >> Reviewed-by: Alex Bennée >> --- >> hw/arm/virt-acpi-build.c | 61 ++++++++++++++++++++++++++++++++++++++++ >> include/hw/acpi/acpi-defs.h | 38 ++++++++++++++++++++++++- >> include/hw/arm/virt-acpi-build.h | 2 ++ >> 3 files changed, 100 insertions(+), 1 deletion(-) >> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c >> index c72a9c8..94cced0 100644 >> --- a/hw/arm/virt-acpi-build.c >> +++ b/hw/arm/virt-acpi-build.c >> @@ -50,6 +50,20 @@ >> #include "qom/qom-qobject.h" >> #include "exec/ram_addr.h" >> >> +typedef struct VirtAcpiCpuInfo { >> + DECLARE_BITMAP(found_cpus, VIRT_ACPI_CPU_ID_LIMIT); >> +} VirtAcpiCpuInfo; >> + >> +static void virt_acpi_get_cpu_info(VirtAcpiCpuInfo *cpuinfo) >> +{ >> + CPUState *cpu; >> + >> + memset(cpuinfo->found_cpus, 0, sizeof cpuinfo->found_cpus); >> + CPU_FOREACH(cpu) { >> + set_bit(cpu->cpu_index, cpuinfo->found_cpus); >> + } >> +} >> + >> static void acpi_dsdt_add_cpus(Aml *scope, int max_cpus) >> { >> uint16_t i; >> @@ -146,6 +160,47 @@ static void acpi_dsdt_add_virtio(Aml *scope, const MemMap *virtio_mmio_memmap, >> } >> } >> >> +/* MADT */ >> +static void >> +build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info, >> + VirtAcpiCpuInfo *cpuinfo) >> +{ >> + int madt_start = table_data->len; >> + const struct AcpiMadtInfo *info = guest_info->madt_info; >> + AcpiMultipleApicTable *madt; >> + AcpiMadtGenericDistributor *gicd; >> + int i; >> + >> + madt = acpi_data_push(table_data, sizeof *madt); >> + madt->local_apic_address = info->gic_cpu_memmap->addr; > should be safe to drop this since OSPM must ignore this field if > gicc provides base_address. > Ok. >> + madt->flags = cpu_to_le32(1); > is it correct? > Looking at 5.1 spec it's x86 specific PCAT_COMPAT flag, > why do you use it for ARM? > Oh, will fix. Thanks. >> + >> + for (i = 0; i < guest_info->max_cpus; i++) { >> + AcpiMadtGenericInterrupt *gicc = acpi_data_push(table_data, >> + sizeof *gicc); >> + gicc->type = ACPI_APIC_GENERIC_INTERRUPT; >> + gicc->length = sizeof(*gicc); >> + gicc->base_address = info->gic_cpu_memmap->addr; >> + gicc->cpu_interface_number = i; >> + gicc->arm_mpidr = i; >> + gicc->uid = i; >> + if (test_bit(i, cpuinfo->found_cpus)) { >> + gicc->flags = cpu_to_le32(1); > #define ACPI_GICC_ENABLED 1 > >> + } else { >> + gicc->flags = cpu_to_le32(0); >> + } > not necessary else clause, field is zeroed by default. > Ok, will remove. >> + } >> + >> + gicd = acpi_data_push(table_data, sizeof *gicd); >> + gicd->type = ACPI_APIC_GENERIC_DISTRIBUTOR; >> + gicd->length = sizeof(*gicd); >> + gicd->base_address = info->gic_dist_memmap->addr; >> + >> + build_header(linker, table_data, >> + (void *)(table_data->data + madt_start), "APIC", >> + table_data->len - madt_start, 1); > wrong revision? > Thanks, will fix this including other patches. >> +} >> + >> /* FADT */ >> static void >> build_fadt(GArray *table_data, GArray *linker, unsigned dsdt) >> @@ -215,8 +270,11 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables) >> { >> GArray *table_offsets; >> unsigned dsdt; >> + VirtAcpiCpuInfo cpuinfo; >> GArray *tables_blob = tables->table_data; >> >> + virt_acpi_get_cpu_info(&cpuinfo); >> + >> table_offsets = g_array_new(false, true /* clear */, >> sizeof(uint32_t)); >> >> @@ -241,6 +299,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables) >> acpi_add_table(table_offsets, tables_blob); >> build_fadt(tables_blob, tables->linker, dsdt); >> >> + acpi_add_table(table_offsets, tables_blob); >> + build_madt(tables_blob, tables->linker, guest_info, &cpuinfo); >> + >> /* Cleanup memory that's no longer used. */ >> g_array_free(table_offsets, true); >> } >> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h >> index bac981d..4092dc3 100644 >> --- a/include/hw/acpi/acpi-defs.h >> +++ b/include/hw/acpi/acpi-defs.h >> @@ -236,7 +236,13 @@ typedef struct AcpiMultipleApicTable AcpiMultipleApicTable; >> #define ACPI_APIC_IO_SAPIC 6 >> #define ACPI_APIC_LOCAL_SAPIC 7 >> #define ACPI_APIC_XRUPT_SOURCE 8 >> -#define ACPI_APIC_RESERVED 9 /* 9 and greater are reserved */ >> +#define ACPI_APIC_LOCAL_X2APIC 9 >> +#define ACPI_APIC_LOCAL_X2APIC_NMI 10 >> +#define ACPI_APIC_GENERIC_INTERRUPT 11 >> +#define ACPI_APIC_GENERIC_DISTRIBUTOR 12 >> +#define ACPI_APIC_GENERIC_MSI_FRAME 13 >> +#define ACPI_APIC_GENERIC_REDISTRIBUTOR 14 >> +#define ACPI_APIC_RESERVED 15 /* 15 and greater are reserved */ >> >> /* >> * MADT sub-structures (Follow MULTIPLE_APIC_DESCRIPTION_TABLE) >> @@ -284,6 +290,36 @@ struct AcpiMadtLocalNmi { >> } QEMU_PACKED; >> typedef struct AcpiMadtLocalNmi AcpiMadtLocalNmi; >> >> +struct AcpiMadtGenericInterrupt { >> + ACPI_SUB_HEADER_DEF >> + uint16_t reserved; >> + uint32_t cpu_interface_number; >> + uint32_t uid; >> + uint32_t flags; >> + uint32_t parking_version; >> + uint32_t performance_interrupt; >> + uint64_t parked_address; >> + uint64_t base_address; >> + uint64_t gicv_base_address; >> + uint64_t gich_base_address; >> + uint32_t vgic_interrupt; >> + uint64_t gicr_base_address; >> + uint64_t arm_mpidr; >> +} QEMU_PACKED; >> + >> +typedef struct AcpiMadtGenericInterrupt AcpiMadtGenericInterrupt; >> + >> +struct AcpiMadtGenericDistributor { >> + ACPI_SUB_HEADER_DEF >> + uint16_t reserved; >> + uint32_t gic_id; >> + uint64_t base_address; >> + uint32_t global_irq_base; >> + uint32_t reserved2; >> +} QEMU_PACKED; >> + >> +typedef struct AcpiMadtGenericDistributor AcpiMadtGenericDistributor; >> + >> /* >> * HPET Description Table >> */ >> diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h >> index ece67a2..8f0b4a7 100644 >> --- a/include/hw/arm/virt-acpi-build.h >> +++ b/include/hw/arm/virt-acpi-build.h >> @@ -22,6 +22,8 @@ >> >> #include "qemu-common.h" >> >> +#define VIRT_ACPI_CPU_ID_LIMIT 8 >> + >> typedef struct MemMap { >> hwaddr addr; >> hwaddr size; >