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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.