From: Shannon Zhao <shannon.zhao@linaro.org>
To: Pavel Fedin <p.fedin@samsung.com>, qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
Shlomo Pongratz <shlomopongratz@gmail.com>,
Shlomo Pongratz <shlomo.pongratz@huawei.com>
Subject: Re: [Qemu-devel] [PATCH v13 5/5] hw/arm/virt: Add gic-version option to virt machine
Date: Wed, 09 Sep 2015 23:04:29 +0800 [thread overview]
Message-ID: <55F04A7D.1060301@linaro.org> (raw)
In-Reply-To: <4c5da6faa513ac27ab818c69a7ba47011baf76ee.1441702954.git.p.fedin@samsung.com>
On 2015/9/8 17:06, Pavel Fedin wrote:
> Add gic_version to VirtMachineState, set it to value of the option
> and pass it around where necessary. Instantiate devices and fdt
> nodes according to the choice.
>
> max_cpus for virt machine increased to 123 (calculated from redistributor
> space available in the memory map). GICv2 compatibility check happens
> inside arm_gic_common_realize().
>
> ITS region is added to the memory map too, however currently it not used,
> just reserved.
>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Tested-by: Ashok kumar <ashoks@broadcom.com>
> ---
> dtc | 2 +-
> hw/arm/virt-acpi-build.c | 54 ++++++++++-------
> hw/arm/virt.c | 124 +++++++++++++++++++++++++++++++--------
> include/hw/acpi/acpi-defs.h | 9 +++
> include/hw/arm/virt-acpi-build.h | 1 +
> include/hw/arm/virt.h | 4 +-
> 6 files changed, 147 insertions(+), 47 deletions(-)
>
> diff --git a/dtc b/dtc
> index 65cc4d2..bc895d6 160000
> --- a/dtc
> +++ b/dtc
> @@ -1 +1 @@
> -Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
> +Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 9088248..0dd7fce 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -443,33 +443,43 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info,
>
> madt = acpi_data_push(table_data, sizeof *madt);
>
> - for (i = 0; i < guest_info->smp_cpus; i++) {
> - AcpiMadtGenericInterrupt *gicc = acpi_data_push(table_data,
> - sizeof *gicc);
> - gicc->type = ACPI_APIC_GENERIC_INTERRUPT;
> - gicc->length = sizeof(*gicc);
> - gicc->base_address = memmap[VIRT_GIC_CPU].base;
> - gicc->cpu_interface_number = i;
> - gicc->arm_mpidr = i;
> - gicc->uid = i;
> - if (test_bit(i, cpuinfo->found_cpus)) {
> - gicc->flags = cpu_to_le32(ACPI_GICC_ENABLED);
> - }
> - }
> -
> gicd = acpi_data_push(table_data, sizeof *gicd);
> gicd->type = ACPI_APIC_GENERIC_DISTRIBUTOR;
> gicd->length = sizeof(*gicd);
> gicd->base_address = memmap[VIRT_GIC_DIST].base;
>
> - gic_msi = acpi_data_push(table_data, sizeof *gic_msi);
> - gic_msi->type = ACPI_APIC_GENERIC_MSI_FRAME;
> - gic_msi->length = sizeof(*gic_msi);
> - gic_msi->gic_msi_frame_id = 0;
> - gic_msi->base_address = cpu_to_le64(memmap[VIRT_GIC_V2M].base);
> - gic_msi->flags = cpu_to_le32(1);
> - gic_msi->spi_count = cpu_to_le16(NUM_GICV2M_SPIS);
> - gic_msi->spi_base = cpu_to_le16(irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE);
> + if (guest_info->gic_version == 3) {
> + AcpiMadtGenericRedistributor *gicr = acpi_data_push(table_data,
> + sizeof *gicr);
> +
> + gicr->type = ACPI_APIC_GENERIC_REDISTRIBUTOR;
> + gicr->length = sizeof(*gicr);
> + gicr->base_address = memmap[VIRT_GIC_REDIST].base;
> + gicr->range_length = memmap[VIRT_GIC_REDIST].size;
These should cpu_to_le for the >1 byte members.
> + } else {
> + for (i = 0; i < guest_info->smp_cpus; i++) {
> + AcpiMadtGenericInterrupt *gicc = acpi_data_push(table_data,
> + sizeof *gicc);
> + gicc->type = ACPI_APIC_GENERIC_INTERRUPT;
> + gicc->length = sizeof(*gicc);
> + gicc->base_address = memmap[VIRT_GIC_CPU].base;
> + gicc->cpu_interface_number = i;
> + gicc->arm_mpidr = i;
> + gicc->uid = i;
> + if (test_bit(i, cpuinfo->found_cpus)) {
> + gicc->flags = cpu_to_le32(ACPI_GICC_ENABLED);
> + }
> + }
> +
> + gic_msi = acpi_data_push(table_data, sizeof *gic_msi);
> + gic_msi->type = ACPI_APIC_GENERIC_MSI_FRAME;
> + gic_msi->length = sizeof(*gic_msi);
> + gic_msi->gic_msi_frame_id = 0;
> + gic_msi->base_address = cpu_to_le64(memmap[VIRT_GIC_V2M].base);
> + gic_msi->flags = cpu_to_le32(1);
> + gic_msi->spi_count = cpu_to_le16(NUM_GICV2M_SPIS);
> + gic_msi->spi_base = cpu_to_le16(irqmap[VIRT_GIC_V2M] + ARM_SPI_BASE);
> + }
>
> build_header(linker, table_data,
> (void *)(table_data->data + madt_start), "APIC",
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 91e45e0..470d5ff 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -51,6 +51,7 @@
> #include "hw/intc/arm_gic_common.h"
> #include "kvm_arm.h"
> #include "hw/smbios/smbios.h"
> +#include "qapi/visitor.h"
>
> /* Number of external interrupt lines to configure the GIC with */
> #define NUM_IRQS 256
> @@ -81,6 +82,7 @@ typedef struct {
> MachineState parent;
> bool secure;
> bool highmem;
> + int32_t gic_version;
> } VirtMachineState;
>
> #define TYPE_VIRT_MACHINE "virt"
> @@ -111,6 +113,10 @@ static const MemMapEntry a15memmap[] = {
> [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 },
> [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 },
> [VIRT_GIC_V2M] = { 0x08020000, 0x00001000 },
> + /* The space in between here is reserved for GICv3 CPU/vCPU/HYP */
> + [VIRT_GIC_ITS] = { 0x08080000, 0x00020000 },
> + /* This redistributor space allows up to 2*64kB*123 CPUs */
> + [VIRT_GIC_REDIST] = { 0x080A0000, 0x00F60000 },
> [VIRT_UART] = { 0x09000000, 0x00001000 },
> [VIRT_RTC] = { 0x09010000, 0x00001000 },
> [VIRT_FW_CFG] = { 0x09020000, 0x0000000a },
> @@ -255,7 +261,7 @@ static void fdt_add_psci_node(const VirtBoardInfo *vbi)
> qemu_fdt_setprop_cell(fdt, "/psci", "migrate", migrate_fn);
> }
>
> -static void fdt_add_timer_nodes(const VirtBoardInfo *vbi)
> +static void fdt_add_timer_nodes(const VirtBoardInfo *vbi, int gictype)
> {
> /* Note that on A15 h/w these interrupts are level-triggered,
> * but for the GIC implementation provided by both QEMU and KVM
> @@ -264,8 +270,11 @@ static void fdt_add_timer_nodes(const VirtBoardInfo *vbi)
> ARMCPU *armcpu;
> uint32_t irqflags = GIC_FDT_IRQ_FLAGS_EDGE_LO_HI;
>
> - irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> - GIC_FDT_IRQ_PPI_CPU_WIDTH, (1 << vbi->smp_cpus) - 1);
> + if (gictype == 2) {
> + irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> + GIC_FDT_IRQ_PPI_CPU_WIDTH,
> + (1 << vbi->smp_cpus) - 1);
> + }
>
> qemu_fdt_add_subnode(vbi->fdt, "/timer");
>
> @@ -355,25 +364,36 @@ static void fdt_add_v2m_gic_node(VirtBoardInfo *vbi)
> qemu_fdt_setprop_cell(vbi->fdt, "/intc/v2m", "phandle", vbi->v2m_phandle);
> }
>
> -static void fdt_add_gic_node(VirtBoardInfo *vbi)
> +static void fdt_add_gic_node(VirtBoardInfo *vbi, int type)
> {
> vbi->gic_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
> qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", vbi->gic_phandle);
>
> qemu_fdt_add_subnode(vbi->fdt, "/intc");
> - /* 'cortex-a15-gic' means 'GIC v2' */
> - qemu_fdt_setprop_string(vbi->fdt, "/intc", "compatible",
> - "arm,cortex-a15-gic");
> qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#interrupt-cells", 3);
> qemu_fdt_setprop(vbi->fdt, "/intc", "interrupt-controller", NULL, 0);
> - qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc", "reg",
> - 2, vbi->memmap[VIRT_GIC_DIST].base,
> - 2, vbi->memmap[VIRT_GIC_DIST].size,
> - 2, vbi->memmap[VIRT_GIC_CPU].base,
> - 2, vbi->memmap[VIRT_GIC_CPU].size);
> qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#address-cells", 0x2);
> qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#size-cells", 0x2);
> qemu_fdt_setprop(vbi->fdt, "/intc", "ranges", NULL, 0);
> + if (type == 3) {
> + qemu_fdt_setprop_string(vbi->fdt, "/intc", "compatible",
> + "arm,gic-v3");
> + qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc", "reg",
> + 2, vbi->memmap[VIRT_GIC_DIST].base,
> + 2, vbi->memmap[VIRT_GIC_DIST].size,
> + 2, vbi->memmap[VIRT_GIC_REDIST].base,
> + 2, vbi->memmap[VIRT_GIC_REDIST].size);
> + } else {
> + /* 'cortex-a15-gic' means 'GIC v2' */
> + qemu_fdt_setprop_string(vbi->fdt, "/intc", "compatible",
> + "arm,cortex-a15-gic");
> + qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc", "reg",
> + 2, vbi->memmap[VIRT_GIC_DIST].base,
> + 2, vbi->memmap[VIRT_GIC_DIST].size,
> + 2, vbi->memmap[VIRT_GIC_CPU].base,
> + 2, vbi->memmap[VIRT_GIC_CPU].size);
> + }
> +
> qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);
> }
>
> @@ -396,18 +416,18 @@ static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
> fdt_add_v2m_gic_node(vbi);
> }
>
> -static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
> +static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type)
> {
> - /* We create a standalone GIC v2 */
> + /* We create a standalone GIC */
> DeviceState *gicdev;
> SysBusDevice *gicbusdev;
> const char *gictype;
> int i;
>
> - gictype = gic_class_name();
> + gictype = (type == 3) ? gicv3_class_name() : gic_class_name();
>
> gicdev = qdev_create(NULL, gictype);
> - qdev_prop_set_uint32(gicdev, "revision", 2);
> + qdev_prop_set_uint32(gicdev, "revision", type);
> qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus);
> /* Note that the num-irq property counts both internal and external
> * interrupts; there are always 32 of the former (mandated by GIC spec).
> @@ -416,7 +436,11 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
> qdev_init_nofail(gicdev);
> gicbusdev = SYS_BUS_DEVICE(gicdev);
> sysbus_mmio_map(gicbusdev, 0, vbi->memmap[VIRT_GIC_DIST].base);
> - sysbus_mmio_map(gicbusdev, 1, vbi->memmap[VIRT_GIC_CPU].base);
> + if (type == 3) {
> + sysbus_mmio_map(gicbusdev, 1, vbi->memmap[VIRT_GIC_REDIST].base);
> + } else {
> + sysbus_mmio_map(gicbusdev, 1, vbi->memmap[VIRT_GIC_CPU].base);
> + }
>
> /* Wire the outputs from each CPU's generic timer to the
> * appropriate GIC PPI inputs, and the GIC's IRQ output to
> @@ -451,9 +475,11 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
> pic[i] = qdev_get_gpio_in(gicdev, i);
> }
>
> - fdt_add_gic_node(vbi);
> + fdt_add_gic_node(vbi, type);
>
> - create_v2m(vbi, pic);
> + if (type == 2) {
> + create_v2m(vbi, pic);
> + }
> }
>
> static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
> @@ -770,7 +796,10 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
> qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0,
> nr_pcie_buses - 1);
>
> - qemu_fdt_setprop_cells(vbi->fdt, nodename, "msi-parent", vbi->v2m_phandle);
> + if (vbi->v2m_phandle) {
> + qemu_fdt_setprop_cells(vbi->fdt, nodename, "msi-parent",
> + vbi->v2m_phandle);
> + }
>
> qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
> 2, base_ecam, 2, size_ecam);
> @@ -885,6 +914,7 @@ static void machvirt_init(MachineState *machine)
> VirtMachineState *vms = VIRT_MACHINE(machine);
> qemu_irq pic[NUM_IRQS];
> MemoryRegion *sysmem = get_system_memory();
> + int gic_version = vms->gic_version;
> int n;
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> const char *cpu_model = machine->cpu_model;
> @@ -897,6 +927,18 @@ static void machvirt_init(MachineState *machine)
> cpu_model = "cortex-a15";
> }
>
> + /* We can probe only here because during property set
> + * KVM is not available yet
> + */
> + if (!gic_version) {
> + gic_version = kvm_arm_vgic_probe();
> + if (!gic_version) {
> + error_report("Unable to determine GIC version supported by host\n"
> + "Probably KVM acceleration is not supported\n");
> + exit(1);
> + }
> + }
> +
> /* Separate the actual CPU model name from any appended features */
> cpustr = g_strsplit(cpu_model, ",", 2);
>
> @@ -957,7 +999,7 @@ static void machvirt_init(MachineState *machine)
> object_property_set_bool(cpuobj, true, "realized", NULL);
> }
> g_strfreev(cpustr);
> - fdt_add_timer_nodes(vbi);
> + fdt_add_timer_nodes(vbi, gic_version);
> fdt_add_cpu_nodes(vbi);
> fdt_add_psci_node(vbi);
>
> @@ -967,7 +1009,7 @@ static void machvirt_init(MachineState *machine)
>
> create_flash(vbi);
>
> - create_gic(vbi, pic);
> + create_gic(vbi, pic, gic_version);
>
> create_uart(vbi, pic);
>
> @@ -989,6 +1031,7 @@ static void machvirt_init(MachineState *machine)
> guest_info->memmap = vbi->memmap;
> guest_info->irqmap = vbi->irqmap;
> guest_info->use_highmem = vms->highmem;
> + guest_info->gic_version = gic_version;
> guest_info_state->machine_done.notify = virt_guest_info_machine_done;
> qemu_add_machine_init_done_notifier(&guest_info_state->machine_done);
>
> @@ -1040,6 +1083,31 @@ static void virt_set_highmem(Object *obj, bool value, Error **errp)
> vms->highmem = value;
> }
>
> +static char *virt_get_gic_version(Object *obj, Error **errp)
> +{
> + VirtMachineState *vms = VIRT_MACHINE(obj);
> + const char *val = vms->gic_version == 3 ? "3" : "2";
> +
> + return g_strdup(val);
> +}
> +
> +static void virt_set_gic_version(Object *obj, const char *value, Error **errp)
> +{
> + VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> + if (!strcmp(value, "3")) {
> + vms->gic_version = 3;
> + } else if (!strcmp(value, "2")) {
> + vms->gic_version = 2;
Maybe it could define macro or enum for these magic numbers 2, 3, like
GICv2, GICv3
> + } else if (!strcmp(value, "host")) {
> + vms->gic_version = 0; /* Will probe later */
> + } else {
> + error_report("Invalid gic-version option value\n"
> + "Allowed values are: 3, 2, host\n");
> + exit(1);
> + }
> +}
> +
> static void virt_instance_init(Object *obj)
> {
> VirtMachineState *vms = VIRT_MACHINE(obj);
> @@ -1061,6 +1129,13 @@ static void virt_instance_init(Object *obj)
> "Set on/off to enable/disable using "
> "physical address space above 32 bits",
> NULL);
> + /* Default GIC type is v2 */
> + vms->gic_version = 2;
> + object_property_add_str(obj, "gic-version", virt_get_gic_version,
> + virt_set_gic_version, NULL);
> + object_property_set_description(obj, "gic-version",
> + "Set GIC version. "
> + "Valid values are 2, 3 and host", NULL);
> }
>
> static void virt_class_init(ObjectClass *oc, void *data)
> @@ -1070,7 +1145,10 @@ static void virt_class_init(ObjectClass *oc, void *data)
> mc->name = TYPE_VIRT_MACHINE;
> mc->desc = "ARM Virtual Machine",
> mc->init = machvirt_init;
> - mc->max_cpus = 8;
> + /* Our maximum number of CPUs depends on how many redistributors
> + * we can fit into memory map
> + */
> + mc->max_cpus = a15memmap[VIRT_GIC_REDIST].size / 0x20000;
> mc->has_dynamic_sysbus = true;
> mc->block_default_type = IF_VIRTIO;
> mc->no_cdrom = 1;
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 2b431e6..c7a03d4 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -384,6 +384,15 @@ struct AcpiMadtGenericMsiFrame {
>
> typedef struct AcpiMadtGenericMsiFrame AcpiMadtGenericMsiFrame;
>
> +struct AcpiMadtGenericRedistributor {
> + ACPI_SUB_HEADER_DEF
> + uint16_t reserved;
> + uint64_t base_address;
> + uint32_t range_length;
> +} QEMU_PACKED;
> +
> +typedef struct AcpiMadtGenericRedistributor AcpiMadtGenericRedistributor;
> +
> /*
> * Generic Timer Description Table (GTDT)
> */
> diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
> index 19b68a4..744b666 100644
> --- a/include/hw/arm/virt-acpi-build.h
> +++ b/include/hw/arm/virt-acpi-build.h
> @@ -32,6 +32,7 @@ typedef struct VirtGuestInfo {
> const MemMapEntry *memmap;
> const int *irqmap;
> bool use_highmem;
> + int gic_version;
> } VirtGuestInfo;
>
>
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 808753f..f464586 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -46,6 +46,9 @@ enum {
> VIRT_CPUPERIPHS,
> VIRT_GIC_DIST,
> VIRT_GIC_CPU,
> + VIRT_GIC_V2M,
> + VIRT_GIC_ITS,
> + VIRT_GIC_REDIST,
> VIRT_UART,
> VIRT_MMIO,
> VIRT_RTC,
> @@ -54,7 +57,6 @@ enum {
> VIRT_PCIE_MMIO,
> VIRT_PCIE_PIO,
> VIRT_PCIE_ECAM,
> - VIRT_GIC_V2M,
> VIRT_PLATFORM_BUS,
> VIRT_PCIE_MMIO_HIGH,
> };
>
--
Shannon
prev parent reply other threads:[~2015-09-09 15:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-08 9:06 [Qemu-devel] [PATCH v13 0/5] vGICv3 support Pavel Fedin
2015-09-08 9:06 ` [Qemu-devel] [PATCH v13 1/5] hw/intc: Implement GIC-500 base class Pavel Fedin
2015-09-08 9:06 ` [Qemu-devel] [PATCH v13 2/5] intc/gic: Extract some reusable vGIC code Pavel Fedin
2015-09-08 9:06 ` [Qemu-devel] [PATCH v13 3/5] arm_kvm: Do not assume particular GIC type in kvm_arch_irqchip_create() Pavel Fedin
2015-09-08 9:06 ` [Qemu-devel] [PATCH v13 4/5] hw/intc: Initial implementation of vGICv3 Pavel Fedin
2015-09-08 9:06 ` [Qemu-devel] [PATCH v13 5/5] hw/arm/virt: Add gic-version option to virt machine Pavel Fedin
2015-09-08 10:49 ` Shlomo Pongratz
2015-09-09 6:50 ` Pavel Fedin
2015-09-09 15:04 ` Shannon Zhao [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55F04A7D.1060301@linaro.org \
--to=shannon.zhao@linaro.org \
--cc=p.fedin@samsung.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=shlomo.pongratz@huawei.com \
--cc=shlomopongratz@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).