All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v7 0/2] Add Arm SBSA Reference Machine
@ 2019-04-18  4:04 ` Hongbo Zhang
  0 siblings, 0 replies; 24+ messages in thread
From: Hongbo Zhang @ 2019-04-18  4:04 UTC (permalink / raw)
  To: peter.maydell, qemu-devel, qemu-arm
  Cc: ard.biesheuvel, leif.lindholm, radoslaw.biernacki, Hongbo Zhang

For the Aarch64, there is one machine 'virt', it is primarily meant to
run on KVM and execute virtualization workloads, but we need an
environment as faithful as possible to physical hardware,  to support
firmware and OS development for pysical Aarch64 machines.

This machine comes with:
 - Re-designed memory map.
 - CPU cortex-a57.
 - EL2 and EL3 enabled.
 - GIC version 3.
 - System bus AHCI controller.
 - System bus XHCI controller.
 - CDROM and hard disc on AHCI bus.
 - E1000E ethernet card on PCIE bus.
 - VGA display adaptor on PCIE bus.
 - Only minimal device tree nodes.
And without:
 - virtio deivces.
 - fw_cfg device.
 - ACPI tables.

Arm Trusted Firmware and UEFI porting to this are done accordingly, and
it should supply ACPI tables to load OS, the minimal device tree nodes
supplied from this platform are only to pass the dynamic info reflecting
command line input to firmware, not for loading OS.

v7 changes:
 - edit memory map for PCIE slightly
 - add another secure UART which can be used for RAS and MM from EL0.

v6 changes:
 - rebased to the latest QEMU tree
 - rechecked all the header files included
 - added the newly introduced system bus EHCI controller
 - removed the machine_done callback due to commit 5614ca80
 - updated block comments styles according to checkpatch.pl
 - use Kconfig to add new file
 - use private SBSA* types defination instead of VIRT* in virt.h
   since nobody else using them so they are in the .c file instead
   of a new .h file

v5 changes:
 - removed more lines derived from virt.c
 - designed a new memory map
 - splitted former one patch into two for easier review
 - cancled previous EHCI and new HXCI coming later separately

V4 changes:
 - rebased to v3.0.0
 - removed timer, uart, rtc, *hci device tree nodes
   (others were removerd in v3)
 - other minore codes clean up, mainly unsed header files, comments etc.

V3 changes:
 - rename the platform 'sbsa-ref'
 - move all the codes to a separate file sbsa-ref.c
 - remove paravirtualized fw_cfg device
 - do not supply ACPI tables, since firmware will do it
 - supply only necessary DT nodes
 - and other minor code clean up

Hongbo Zhang (2):
  hw/arm: Add arm SBSA reference machine, skeleton part
  hw/arm: Add arm SBSA reference machine, devices part

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Kconfig                  |   3 +
 hw/arm/Makefile.objs            |   1 +
 hw/arm/sbsa-ref.c               | 757 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 762 insertions(+)
 create mode 100644 hw/arm/sbsa-ref.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH v7 0/2] Add Arm SBSA Reference Machine
@ 2019-04-18  4:04 ` Hongbo Zhang
  0 siblings, 0 replies; 24+ messages in thread
From: Hongbo Zhang @ 2019-04-18  4:04 UTC (permalink / raw)
  To: peter.maydell, qemu-devel, qemu-arm
  Cc: Hongbo Zhang, radoslaw.biernacki, leif.lindholm, ard.biesheuvel

For the Aarch64, there is one machine 'virt', it is primarily meant to
run on KVM and execute virtualization workloads, but we need an
environment as faithful as possible to physical hardware,  to support
firmware and OS development for pysical Aarch64 machines.

This machine comes with:
 - Re-designed memory map.
 - CPU cortex-a57.
 - EL2 and EL3 enabled.
 - GIC version 3.
 - System bus AHCI controller.
 - System bus XHCI controller.
 - CDROM and hard disc on AHCI bus.
 - E1000E ethernet card on PCIE bus.
 - VGA display adaptor on PCIE bus.
 - Only minimal device tree nodes.
And without:
 - virtio deivces.
 - fw_cfg device.
 - ACPI tables.

Arm Trusted Firmware and UEFI porting to this are done accordingly, and
it should supply ACPI tables to load OS, the minimal device tree nodes
supplied from this platform are only to pass the dynamic info reflecting
command line input to firmware, not for loading OS.

v7 changes:
 - edit memory map for PCIE slightly
 - add another secure UART which can be used for RAS and MM from EL0.

v6 changes:
 - rebased to the latest QEMU tree
 - rechecked all the header files included
 - added the newly introduced system bus EHCI controller
 - removed the machine_done callback due to commit 5614ca80
 - updated block comments styles according to checkpatch.pl
 - use Kconfig to add new file
 - use private SBSA* types defination instead of VIRT* in virt.h
   since nobody else using them so they are in the .c file instead
   of a new .h file

v5 changes:
 - removed more lines derived from virt.c
 - designed a new memory map
 - splitted former one patch into two for easier review
 - cancled previous EHCI and new HXCI coming later separately

V4 changes:
 - rebased to v3.0.0
 - removed timer, uart, rtc, *hci device tree nodes
   (others were removerd in v3)
 - other minore codes clean up, mainly unsed header files, comments etc.

V3 changes:
 - rename the platform 'sbsa-ref'
 - move all the codes to a separate file sbsa-ref.c
 - remove paravirtualized fw_cfg device
 - do not supply ACPI tables, since firmware will do it
 - supply only necessary DT nodes
 - and other minor code clean up

Hongbo Zhang (2):
  hw/arm: Add arm SBSA reference machine, skeleton part
  hw/arm: Add arm SBSA reference machine, devices part

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Kconfig                  |   3 +
 hw/arm/Makefile.objs            |   1 +
 hw/arm/sbsa-ref.c               | 757 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 762 insertions(+)
 create mode 100644 hw/arm/sbsa-ref.c

-- 
2.7.4



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH v7 1/2] hw/arm: Add arm SBSA reference machine, skeleton part
@ 2019-04-18  4:04   ` Hongbo Zhang
  0 siblings, 0 replies; 24+ messages in thread
From: Hongbo Zhang @ 2019-04-18  4:04 UTC (permalink / raw)
  To: peter.maydell, qemu-devel, qemu-arm
  Cc: ard.biesheuvel, leif.lindholm, radoslaw.biernacki, Hongbo Zhang

For the Aarch64, there is one machine 'virt', it is primarily meant to
run on KVM and execute virtualization workloads, but we need an
environment as faithful as possible to physical hardware, for supporting
firmware and OS development for pysical Aarch64 machines.

This patch introduces new machine type 'sbsa-ref' with main features:
 - Based on 'virt' machine type.
 - A new memory map.
 - CPU type cortex-a57.
 - EL2 and EL3 are enabled.
 - GIC version 3.
 - System bus AHCI controller.
 - System bus EHCI controller.
 - CDROM and hard disc on AHCI bus.
 - E1000E ethernet card on PCIE bus.
 - VGA display adaptor on PCIE bus.
 - No virtio deivces.
 - No fw_cfg device.
 - No ACPI table supplied.
 - Only minimal device tree nodes.

Arm Trusted Firmware and UEFI porting to this are done accordingly, and
it should supply ACPI tables to load OS, the minimal device tree nodes
supplied from this platform are only to pass the dynamic info reflecting
command line input to firmware, not for loading OS.

To make the review easier, this task is split into two patches, the
fundamental sceleton part and the peripheral devices part, this patch is
the first part.

Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
---
 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Kconfig                  |   3 +
 hw/arm/Makefile.objs            |   1 +
 hw/arm/sbsa-ref.c               | 306 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 311 insertions(+)
 create mode 100644 hw/arm/sbsa-ref.c

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 613d19a..1a2352e 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -145,6 +145,7 @@ CONFIG_IOH3420=y
 CONFIG_I82801B11=y
 CONFIG_ACPI=y
 CONFIG_ARM_VIRT=y
+CONFIG_SBSA_REF=y
 CONFIG_SMBIOS=y
 CONFIG_ASPEED_SOC=y
 CONFIG_SMBUS_EEPROM=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index d298fbd..6654914 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -38,6 +38,9 @@ config PXA2XX
 config REALVIEW
     bool
 
+config SBSA_REF
+    bool
+
 config STELLARIS
     bool
 
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index fa57c7c..fa812ec 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -12,6 +12,7 @@ obj-$(CONFIG_NSERIES) += nseries.o
 obj-$(CONFIG_OMAP) += omap_sx1.o palm.o
 obj-$(CONFIG_PXA2XX) += gumstix.o spitz.o tosa.o z2.o
 obj-$(CONFIG_REALVIEW) += realview.o
+obj-$(CONFIG_SBSA_REF) += sbsa-ref.o
 obj-$(CONFIG_STELLARIS) += stellaris.o
 obj-$(CONFIG_STRONGARM) += collie.o
 obj-$(CONFIG_VERSATILE) += vexpress.o versatilepb.o
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
new file mode 100644
index 0000000..652ec13
--- /dev/null
+++ b/hw/arm/sbsa-ref.c
@@ -0,0 +1,306 @@
+/*
+ * ARM SBSA Reference Platform emulation
+ *
+ * Copyright (c) 2018 Linaro Limited
+ * Written by Hongbo Zhang <hongbo.zhang@linaro.org>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/units.h"
+#include "sysemu/numa.h"
+#include "sysemu/sysemu.h"
+#include "exec/address-spaces.h"
+#include "exec/hwaddr.h"
+#include "kvm_arm.h"
+#include "hw/arm/arm.h"
+#include "hw/boards.h"
+#include "hw/intc/arm_gicv3_common.h"
+
+#define RAMLIMIT_GB 8192
+#define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
+
+enum {
+    SBSA_FLASH,
+    SBSA_MEM,
+    SBSA_CPUPERIPHS,
+    SBSA_GIC_DIST,
+    SBSA_GIC_REDIST,
+    SBSA_SMMU,
+    SBSA_UART,
+    SBSA_RTC,
+    SBSA_PCIE,
+    SBSA_PCIE_MMIO,
+    SBSA_PCIE_MMIO_HIGH,
+    SBSA_PCIE_PIO,
+    SBSA_PCIE_ECAM,
+    SBSA_GPIO,
+    SBSA_SECURE_UART,
+    SBSA_SECURE_UART_MM,
+    SBSA_SECURE_MEM,
+    SBSA_AHCI,
+    SBSA_EHCI,
+};
+
+typedef struct MemMapEntry {
+    hwaddr base;
+    hwaddr size;
+} MemMapEntry;
+
+typedef struct {
+    MachineState parent;
+    struct arm_boot_info bootinfo;
+    const MemMapEntry *memmap;
+    const int *irqmap;
+    int smp_cpus;
+    void *fdt;
+    int fdt_size;
+    int psci_conduit;
+} SBSAMachineState;
+
+#define TYPE_SBSA_MACHINE   MACHINE_TYPE_NAME("sbsa-ref")
+#define SBSA_MACHINE(obj) \
+    OBJECT_CHECK(SBSAMachineState, (obj), TYPE_SBSA_MACHINE)
+
+static const MemMapEntry sbsa_ref_memmap[] = {
+    /* 512M boot ROM */
+    [SBSA_FLASH] =              {          0, 0x20000000 },
+    /* 512M secure memory */
+    [SBSA_SECURE_MEM] =         { 0x20000000, 0x20000000 },
+    /* Space reserved for CPU peripheral devices */
+    [SBSA_CPUPERIPHS] =         { 0x40000000, 0x00040000 },
+    [SBSA_GIC_DIST] =           { 0x40060000, 0x00010000 },
+    [SBSA_GIC_REDIST] =         { 0x40080000, 0x04000000 },
+    [SBSA_UART] =               { 0x60000000, 0x00001000 },
+    [SBSA_RTC] =                { 0x60010000, 0x00001000 },
+    [SBSA_GPIO] =               { 0x60020000, 0x00001000 },
+    [SBSA_SECURE_UART] =        { 0x60030000, 0x00001000 },
+    [SBSA_SECURE_UART_MM] =     { 0x60040000, 0x00001000 },
+    [SBSA_SMMU] =               { 0x60050000, 0x00020000 },
+    /* Space here reserved for more SMMUs */
+    [SBSA_AHCI] =               { 0x60100000, 0x00010000 },
+    [SBSA_EHCI] =               { 0x60110000, 0x00010000 },
+    /* Space here reserved for other devices */
+    [SBSA_PCIE_PIO] =           { 0x7fff0000, 0x00010000 },
+    /* 32-bit address PCIE MMIO space */
+    [SBSA_PCIE_MMIO] =          { 0x80000000, 0x70000000 },
+    /* 256M PCIE ECAM space */
+    [SBSA_PCIE_ECAM] =          { 0xf0000000, 0x10000000 },
+    /* ~1TB PCIE MMIO space (4GB to 1024GB boundary) */
+    [SBSA_PCIE_MMIO_HIGH] =     { 0x100000000ULL, 0xFF00000000ULL },
+    [SBSA_MEM] =                { 0x10000000000ULL, RAMLIMIT_BYTES },
+};
+
+static const int sbsa_ref_irqmap[] = {
+    [SBSA_UART] = 1,
+    [SBSA_RTC] = 2,
+    [SBSA_PCIE] = 3, /* ... to 6 */
+    [SBSA_GPIO] = 7,
+    [SBSA_SECURE_UART] = 8,
+    [SBSA_SECURE_UART_MM] = 9,
+    [SBSA_AHCI] = 10,
+    [SBSA_EHCI] = 11,
+};
+
+static void sbsa_ref_init(MachineState *machine)
+{
+    SBSAMachineState *vms = SBSA_MACHINE(machine);
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+    MemoryRegion *sysmem = get_system_memory();
+    MemoryRegion *secure_sysmem = NULL;
+    MemoryRegion *ram = g_new(MemoryRegion, 1);
+    bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
+    const CPUArchIdList *possible_cpus;
+    int n, sbsa_max_cpus;
+
+    if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
+        error_report("sbsa-ref: CPU type other than the built-in "
+                     "cortex-a57 not supported");
+        exit(1);
+    }
+
+    if (kvm_enabled()) {
+        error_report("sbsa-ref: KVM is not supported at this machine");
+        exit(1);
+    }
+
+    if (machine->kernel_filename && firmware_loaded) {
+        error_report("sbsa-ref: No fw_cfg device on this machine, "
+                     "so -kernel option is not supported when firmware loaded, "
+                     "please load OS from hard disk instead");
+        exit(1);
+    }
+
+    /*
+     * This machine has EL3 enabled, external firmware should supply PSCI
+     * implementation, so the QEMU's internal PSCI is disabled.
+     */
+    vms->psci_conduit = QEMU_PSCI_CONDUIT_DISABLED;
+
+    sbsa_max_cpus = vms->memmap[SBSA_GIC_REDIST].size / GICV3_REDIST_SIZE;
+
+    if (max_cpus > sbsa_max_cpus) {
+        error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
+                     "supported by machine 'sbsa-ref' (%d)",
+                     max_cpus, sbsa_max_cpus);
+        exit(1);
+    }
+
+    vms->smp_cpus = smp_cpus;
+
+    if (machine->ram_size > vms->memmap[SBSA_MEM].size) {
+        error_report("sbsa-ref: cannot model more than %dGB RAM", RAMLIMIT_GB);
+        exit(1);
+    }
+
+    secure_sysmem = g_new(MemoryRegion, 1);
+    memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory",
+                       UINT64_MAX);
+    memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1);
+
+    possible_cpus = mc->possible_cpu_arch_ids(machine);
+    for (n = 0; n < possible_cpus->len; n++) {
+        Object *cpuobj;
+        CPUState *cs;
+
+        if (n >= smp_cpus) {
+            break;
+        }
+
+        cpuobj = object_new(possible_cpus->cpus[n].type);
+        object_property_set_int(cpuobj, possible_cpus->cpus[n].arch_id,
+                                "mp-affinity", NULL);
+
+        cs = CPU(cpuobj);
+        cs->cpu_index = n;
+
+        numa_cpu_pre_plug(&possible_cpus->cpus[cs->cpu_index], DEVICE(cpuobj),
+                          &error_fatal);
+
+        if (object_property_find(cpuobj, "reset-cbar", NULL)) {
+            object_property_set_int(cpuobj, vms->memmap[SBSA_CPUPERIPHS].base,
+                                    "reset-cbar", &error_abort);
+        }
+
+        object_property_set_link(cpuobj, OBJECT(sysmem), "memory",
+                                 &error_abort);
+
+        object_property_set_link(cpuobj, OBJECT(secure_sysmem),
+                                 "secure-memory", &error_abort);
+
+        object_property_set_bool(cpuobj, true, "realized", &error_fatal);
+        object_unref(cpuobj);
+    }
+
+    memory_region_allocate_system_memory(ram, NULL, "sbsa-ref.ram",
+                                         machine->ram_size);
+    memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram);
+
+    vms->bootinfo.ram_size = machine->ram_size;
+    vms->bootinfo.kernel_filename = machine->kernel_filename;
+    vms->bootinfo.nb_cpus = smp_cpus;
+    vms->bootinfo.board_id = -1;
+    vms->bootinfo.loader_start = vms->memmap[SBSA_MEM].base;
+    vms->bootinfo.firmware_loaded = firmware_loaded;
+    arm_load_kernel(ARM_CPU(first_cpu), &vms->bootinfo);
+}
+
+static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *vms, int idx)
+{
+    uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER;
+    return arm_cpu_mp_affinity(idx, clustersz);
+}
+
+static const CPUArchIdList *sbsa_ref_possible_cpu_arch_ids(MachineState *ms)
+{
+    SBSAMachineState *vms = SBSA_MACHINE(ms);
+    int n;
+
+    if (ms->possible_cpus) {
+        assert(ms->possible_cpus->len == max_cpus);
+        return ms->possible_cpus;
+    }
+
+    ms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
+                                  sizeof(CPUArchId) * max_cpus);
+    ms->possible_cpus->len = max_cpus;
+    for (n = 0; n < ms->possible_cpus->len; n++) {
+        ms->possible_cpus->cpus[n].type = ms->cpu_type;
+        ms->possible_cpus->cpus[n].arch_id =
+            sbsa_ref_cpu_mp_affinity(vms, n);
+        ms->possible_cpus->cpus[n].props.has_thread_id = true;
+        ms->possible_cpus->cpus[n].props.thread_id = n;
+    }
+    return ms->possible_cpus;
+}
+
+static CpuInstanceProperties
+sbsa_ref_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+    const CPUArchIdList *possible_cpus = mc->possible_cpu_arch_ids(ms);
+
+    assert(cpu_index < possible_cpus->len);
+    return possible_cpus->cpus[cpu_index].props;
+}
+
+static int64_t
+sbsa_ref_get_default_cpu_node_id(const MachineState *ms, int idx)
+{
+    return idx % nb_numa_nodes;
+}
+
+static void sbsa_ref_instance_init(Object *obj)
+{
+    SBSAMachineState *vms = SBSA_MACHINE(obj);
+
+    vms->memmap = sbsa_ref_memmap;
+    vms->irqmap = sbsa_ref_irqmap;
+}
+
+static void sbsa_ref_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->init = sbsa_ref_init;
+    mc->desc = "QEMU 'SBSA Reference' ARM Virtual Machine";
+    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a57");
+    mc->max_cpus = 512;
+    mc->pci_allow_0_address = true;
+    mc->minimum_page_bits = 12;
+    mc->block_default_type = IF_IDE;
+    mc->no_cdrom = 1;
+    mc->default_ram_size = 1 * GiB;
+    mc->default_cpus = 4;
+    mc->possible_cpu_arch_ids = sbsa_ref_possible_cpu_arch_ids;
+    mc->cpu_index_to_instance_props = sbsa_ref_cpu_index_to_props;
+    mc->get_default_cpu_node_id = sbsa_ref_get_default_cpu_node_id;
+}
+
+static const TypeInfo sbsa_ref_info = {
+    .name          = TYPE_SBSA_MACHINE,
+    .parent        = TYPE_MACHINE,
+    .instance_size = sizeof(SBSAMachineState),
+    .instance_init = sbsa_ref_instance_init,
+    .class_init    = sbsa_ref_class_init,
+};
+
+static void sbsa_ref_machine_init(void)
+{
+    type_register_static(&sbsa_ref_info);
+}
+
+type_init(sbsa_ref_machine_init);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH v7 1/2] hw/arm: Add arm SBSA reference machine, skeleton part
@ 2019-04-18  4:04   ` Hongbo Zhang
  0 siblings, 0 replies; 24+ messages in thread
From: Hongbo Zhang @ 2019-04-18  4:04 UTC (permalink / raw)
  To: peter.maydell, qemu-devel, qemu-arm
  Cc: Hongbo Zhang, radoslaw.biernacki, leif.lindholm, ard.biesheuvel

For the Aarch64, there is one machine 'virt', it is primarily meant to
run on KVM and execute virtualization workloads, but we need an
environment as faithful as possible to physical hardware, for supporting
firmware and OS development for pysical Aarch64 machines.

This patch introduces new machine type 'sbsa-ref' with main features:
 - Based on 'virt' machine type.
 - A new memory map.
 - CPU type cortex-a57.
 - EL2 and EL3 are enabled.
 - GIC version 3.
 - System bus AHCI controller.
 - System bus EHCI controller.
 - CDROM and hard disc on AHCI bus.
 - E1000E ethernet card on PCIE bus.
 - VGA display adaptor on PCIE bus.
 - No virtio deivces.
 - No fw_cfg device.
 - No ACPI table supplied.
 - Only minimal device tree nodes.

Arm Trusted Firmware and UEFI porting to this are done accordingly, and
it should supply ACPI tables to load OS, the minimal device tree nodes
supplied from this platform are only to pass the dynamic info reflecting
command line input to firmware, not for loading OS.

To make the review easier, this task is split into two patches, the
fundamental sceleton part and the peripheral devices part, this patch is
the first part.

Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
---
 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Kconfig                  |   3 +
 hw/arm/Makefile.objs            |   1 +
 hw/arm/sbsa-ref.c               | 306 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 311 insertions(+)
 create mode 100644 hw/arm/sbsa-ref.c

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 613d19a..1a2352e 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -145,6 +145,7 @@ CONFIG_IOH3420=y
 CONFIG_I82801B11=y
 CONFIG_ACPI=y
 CONFIG_ARM_VIRT=y
+CONFIG_SBSA_REF=y
 CONFIG_SMBIOS=y
 CONFIG_ASPEED_SOC=y
 CONFIG_SMBUS_EEPROM=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index d298fbd..6654914 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -38,6 +38,9 @@ config PXA2XX
 config REALVIEW
     bool
 
+config SBSA_REF
+    bool
+
 config STELLARIS
     bool
 
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index fa57c7c..fa812ec 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -12,6 +12,7 @@ obj-$(CONFIG_NSERIES) += nseries.o
 obj-$(CONFIG_OMAP) += omap_sx1.o palm.o
 obj-$(CONFIG_PXA2XX) += gumstix.o spitz.o tosa.o z2.o
 obj-$(CONFIG_REALVIEW) += realview.o
+obj-$(CONFIG_SBSA_REF) += sbsa-ref.o
 obj-$(CONFIG_STELLARIS) += stellaris.o
 obj-$(CONFIG_STRONGARM) += collie.o
 obj-$(CONFIG_VERSATILE) += vexpress.o versatilepb.o
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
new file mode 100644
index 0000000..652ec13
--- /dev/null
+++ b/hw/arm/sbsa-ref.c
@@ -0,0 +1,306 @@
+/*
+ * ARM SBSA Reference Platform emulation
+ *
+ * Copyright (c) 2018 Linaro Limited
+ * Written by Hongbo Zhang <hongbo.zhang@linaro.org>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/units.h"
+#include "sysemu/numa.h"
+#include "sysemu/sysemu.h"
+#include "exec/address-spaces.h"
+#include "exec/hwaddr.h"
+#include "kvm_arm.h"
+#include "hw/arm/arm.h"
+#include "hw/boards.h"
+#include "hw/intc/arm_gicv3_common.h"
+
+#define RAMLIMIT_GB 8192
+#define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
+
+enum {
+    SBSA_FLASH,
+    SBSA_MEM,
+    SBSA_CPUPERIPHS,
+    SBSA_GIC_DIST,
+    SBSA_GIC_REDIST,
+    SBSA_SMMU,
+    SBSA_UART,
+    SBSA_RTC,
+    SBSA_PCIE,
+    SBSA_PCIE_MMIO,
+    SBSA_PCIE_MMIO_HIGH,
+    SBSA_PCIE_PIO,
+    SBSA_PCIE_ECAM,
+    SBSA_GPIO,
+    SBSA_SECURE_UART,
+    SBSA_SECURE_UART_MM,
+    SBSA_SECURE_MEM,
+    SBSA_AHCI,
+    SBSA_EHCI,
+};
+
+typedef struct MemMapEntry {
+    hwaddr base;
+    hwaddr size;
+} MemMapEntry;
+
+typedef struct {
+    MachineState parent;
+    struct arm_boot_info bootinfo;
+    const MemMapEntry *memmap;
+    const int *irqmap;
+    int smp_cpus;
+    void *fdt;
+    int fdt_size;
+    int psci_conduit;
+} SBSAMachineState;
+
+#define TYPE_SBSA_MACHINE   MACHINE_TYPE_NAME("sbsa-ref")
+#define SBSA_MACHINE(obj) \
+    OBJECT_CHECK(SBSAMachineState, (obj), TYPE_SBSA_MACHINE)
+
+static const MemMapEntry sbsa_ref_memmap[] = {
+    /* 512M boot ROM */
+    [SBSA_FLASH] =              {          0, 0x20000000 },
+    /* 512M secure memory */
+    [SBSA_SECURE_MEM] =         { 0x20000000, 0x20000000 },
+    /* Space reserved for CPU peripheral devices */
+    [SBSA_CPUPERIPHS] =         { 0x40000000, 0x00040000 },
+    [SBSA_GIC_DIST] =           { 0x40060000, 0x00010000 },
+    [SBSA_GIC_REDIST] =         { 0x40080000, 0x04000000 },
+    [SBSA_UART] =               { 0x60000000, 0x00001000 },
+    [SBSA_RTC] =                { 0x60010000, 0x00001000 },
+    [SBSA_GPIO] =               { 0x60020000, 0x00001000 },
+    [SBSA_SECURE_UART] =        { 0x60030000, 0x00001000 },
+    [SBSA_SECURE_UART_MM] =     { 0x60040000, 0x00001000 },
+    [SBSA_SMMU] =               { 0x60050000, 0x00020000 },
+    /* Space here reserved for more SMMUs */
+    [SBSA_AHCI] =               { 0x60100000, 0x00010000 },
+    [SBSA_EHCI] =               { 0x60110000, 0x00010000 },
+    /* Space here reserved for other devices */
+    [SBSA_PCIE_PIO] =           { 0x7fff0000, 0x00010000 },
+    /* 32-bit address PCIE MMIO space */
+    [SBSA_PCIE_MMIO] =          { 0x80000000, 0x70000000 },
+    /* 256M PCIE ECAM space */
+    [SBSA_PCIE_ECAM] =          { 0xf0000000, 0x10000000 },
+    /* ~1TB PCIE MMIO space (4GB to 1024GB boundary) */
+    [SBSA_PCIE_MMIO_HIGH] =     { 0x100000000ULL, 0xFF00000000ULL },
+    [SBSA_MEM] =                { 0x10000000000ULL, RAMLIMIT_BYTES },
+};
+
+static const int sbsa_ref_irqmap[] = {
+    [SBSA_UART] = 1,
+    [SBSA_RTC] = 2,
+    [SBSA_PCIE] = 3, /* ... to 6 */
+    [SBSA_GPIO] = 7,
+    [SBSA_SECURE_UART] = 8,
+    [SBSA_SECURE_UART_MM] = 9,
+    [SBSA_AHCI] = 10,
+    [SBSA_EHCI] = 11,
+};
+
+static void sbsa_ref_init(MachineState *machine)
+{
+    SBSAMachineState *vms = SBSA_MACHINE(machine);
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+    MemoryRegion *sysmem = get_system_memory();
+    MemoryRegion *secure_sysmem = NULL;
+    MemoryRegion *ram = g_new(MemoryRegion, 1);
+    bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
+    const CPUArchIdList *possible_cpus;
+    int n, sbsa_max_cpus;
+
+    if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
+        error_report("sbsa-ref: CPU type other than the built-in "
+                     "cortex-a57 not supported");
+        exit(1);
+    }
+
+    if (kvm_enabled()) {
+        error_report("sbsa-ref: KVM is not supported at this machine");
+        exit(1);
+    }
+
+    if (machine->kernel_filename && firmware_loaded) {
+        error_report("sbsa-ref: No fw_cfg device on this machine, "
+                     "so -kernel option is not supported when firmware loaded, "
+                     "please load OS from hard disk instead");
+        exit(1);
+    }
+
+    /*
+     * This machine has EL3 enabled, external firmware should supply PSCI
+     * implementation, so the QEMU's internal PSCI is disabled.
+     */
+    vms->psci_conduit = QEMU_PSCI_CONDUIT_DISABLED;
+
+    sbsa_max_cpus = vms->memmap[SBSA_GIC_REDIST].size / GICV3_REDIST_SIZE;
+
+    if (max_cpus > sbsa_max_cpus) {
+        error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
+                     "supported by machine 'sbsa-ref' (%d)",
+                     max_cpus, sbsa_max_cpus);
+        exit(1);
+    }
+
+    vms->smp_cpus = smp_cpus;
+
+    if (machine->ram_size > vms->memmap[SBSA_MEM].size) {
+        error_report("sbsa-ref: cannot model more than %dGB RAM", RAMLIMIT_GB);
+        exit(1);
+    }
+
+    secure_sysmem = g_new(MemoryRegion, 1);
+    memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory",
+                       UINT64_MAX);
+    memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1);
+
+    possible_cpus = mc->possible_cpu_arch_ids(machine);
+    for (n = 0; n < possible_cpus->len; n++) {
+        Object *cpuobj;
+        CPUState *cs;
+
+        if (n >= smp_cpus) {
+            break;
+        }
+
+        cpuobj = object_new(possible_cpus->cpus[n].type);
+        object_property_set_int(cpuobj, possible_cpus->cpus[n].arch_id,
+                                "mp-affinity", NULL);
+
+        cs = CPU(cpuobj);
+        cs->cpu_index = n;
+
+        numa_cpu_pre_plug(&possible_cpus->cpus[cs->cpu_index], DEVICE(cpuobj),
+                          &error_fatal);
+
+        if (object_property_find(cpuobj, "reset-cbar", NULL)) {
+            object_property_set_int(cpuobj, vms->memmap[SBSA_CPUPERIPHS].base,
+                                    "reset-cbar", &error_abort);
+        }
+
+        object_property_set_link(cpuobj, OBJECT(sysmem), "memory",
+                                 &error_abort);
+
+        object_property_set_link(cpuobj, OBJECT(secure_sysmem),
+                                 "secure-memory", &error_abort);
+
+        object_property_set_bool(cpuobj, true, "realized", &error_fatal);
+        object_unref(cpuobj);
+    }
+
+    memory_region_allocate_system_memory(ram, NULL, "sbsa-ref.ram",
+                                         machine->ram_size);
+    memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram);
+
+    vms->bootinfo.ram_size = machine->ram_size;
+    vms->bootinfo.kernel_filename = machine->kernel_filename;
+    vms->bootinfo.nb_cpus = smp_cpus;
+    vms->bootinfo.board_id = -1;
+    vms->bootinfo.loader_start = vms->memmap[SBSA_MEM].base;
+    vms->bootinfo.firmware_loaded = firmware_loaded;
+    arm_load_kernel(ARM_CPU(first_cpu), &vms->bootinfo);
+}
+
+static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *vms, int idx)
+{
+    uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER;
+    return arm_cpu_mp_affinity(idx, clustersz);
+}
+
+static const CPUArchIdList *sbsa_ref_possible_cpu_arch_ids(MachineState *ms)
+{
+    SBSAMachineState *vms = SBSA_MACHINE(ms);
+    int n;
+
+    if (ms->possible_cpus) {
+        assert(ms->possible_cpus->len == max_cpus);
+        return ms->possible_cpus;
+    }
+
+    ms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
+                                  sizeof(CPUArchId) * max_cpus);
+    ms->possible_cpus->len = max_cpus;
+    for (n = 0; n < ms->possible_cpus->len; n++) {
+        ms->possible_cpus->cpus[n].type = ms->cpu_type;
+        ms->possible_cpus->cpus[n].arch_id =
+            sbsa_ref_cpu_mp_affinity(vms, n);
+        ms->possible_cpus->cpus[n].props.has_thread_id = true;
+        ms->possible_cpus->cpus[n].props.thread_id = n;
+    }
+    return ms->possible_cpus;
+}
+
+static CpuInstanceProperties
+sbsa_ref_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+    const CPUArchIdList *possible_cpus = mc->possible_cpu_arch_ids(ms);
+
+    assert(cpu_index < possible_cpus->len);
+    return possible_cpus->cpus[cpu_index].props;
+}
+
+static int64_t
+sbsa_ref_get_default_cpu_node_id(const MachineState *ms, int idx)
+{
+    return idx % nb_numa_nodes;
+}
+
+static void sbsa_ref_instance_init(Object *obj)
+{
+    SBSAMachineState *vms = SBSA_MACHINE(obj);
+
+    vms->memmap = sbsa_ref_memmap;
+    vms->irqmap = sbsa_ref_irqmap;
+}
+
+static void sbsa_ref_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->init = sbsa_ref_init;
+    mc->desc = "QEMU 'SBSA Reference' ARM Virtual Machine";
+    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a57");
+    mc->max_cpus = 512;
+    mc->pci_allow_0_address = true;
+    mc->minimum_page_bits = 12;
+    mc->block_default_type = IF_IDE;
+    mc->no_cdrom = 1;
+    mc->default_ram_size = 1 * GiB;
+    mc->default_cpus = 4;
+    mc->possible_cpu_arch_ids = sbsa_ref_possible_cpu_arch_ids;
+    mc->cpu_index_to_instance_props = sbsa_ref_cpu_index_to_props;
+    mc->get_default_cpu_node_id = sbsa_ref_get_default_cpu_node_id;
+}
+
+static const TypeInfo sbsa_ref_info = {
+    .name          = TYPE_SBSA_MACHINE,
+    .parent        = TYPE_MACHINE,
+    .instance_size = sizeof(SBSAMachineState),
+    .instance_init = sbsa_ref_instance_init,
+    .class_init    = sbsa_ref_class_init,
+};
+
+static void sbsa_ref_machine_init(void)
+{
+    type_register_static(&sbsa_ref_info);
+}
+
+type_init(sbsa_ref_machine_init);
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
@ 2019-04-18  4:04   ` Hongbo Zhang
  0 siblings, 0 replies; 24+ messages in thread
From: Hongbo Zhang @ 2019-04-18  4:04 UTC (permalink / raw)
  To: peter.maydell, qemu-devel, qemu-arm
  Cc: ard.biesheuvel, leif.lindholm, radoslaw.biernacki, Hongbo Zhang

Following the previous patch, this patch adds peripheral devices to the
newly introduced SBSA-ref machine.

Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
---
 hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 451 insertions(+)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 652ec13..3fb0027 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -21,6 +21,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/units.h"
+#include "sysemu/device_tree.h"
 #include "sysemu/numa.h"
 #include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
@@ -28,11 +29,28 @@
 #include "kvm_arm.h"
 #include "hw/arm/arm.h"
 #include "hw/boards.h"
+#include "hw/ide/internal.h"
+#include "hw/ide/ahci_internal.h"
 #include "hw/intc/arm_gicv3_common.h"
+#include "hw/loader.h"
+#include "hw/pci-host/gpex.h"
+#include "hw/usb.h"
+#include "net/net.h"
 
 #define RAMLIMIT_GB 8192
 #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
 
+#define NUM_IRQS        256
+#define NUM_SMMU_IRQS   4
+#define NUM_SATA_PORTS  6
+
+#define VIRTUAL_PMU_IRQ        7
+#define ARCH_GIC_MAINT_IRQ     9
+#define ARCH_TIMER_VIRT_IRQ    11
+#define ARCH_TIMER_S_EL1_IRQ   13
+#define ARCH_TIMER_NS_EL1_IRQ  14
+#define ARCH_TIMER_NS_EL2_IRQ  10
+
 enum {
     SBSA_FLASH,
     SBSA_MEM,
@@ -115,6 +133,415 @@ static const int sbsa_ref_irqmap[] = {
     [SBSA_EHCI] = 11,
 };
 
+/*
+ * Firmware on this machine only uses ACPI table to load OS, these limited
+ * device tree nodes are just to let firmware know the info which varies from
+ * command line parameters, so it is not necessary to be fully compatible
+ * with the kernel CPU and NUMA binding rules.
+ */
+static void create_fdt(SBSAMachineState *vms)
+{
+    void *fdt = create_device_tree(&vms->fdt_size);
+    const MachineState *ms = MACHINE(vms);
+    int cpu;
+
+    if (!fdt) {
+        error_report("create_device_tree() failed");
+        exit(1);
+    }
+
+    vms->fdt = fdt;
+
+    qemu_fdt_setprop_string(fdt, "/", "compatible", "linux,sbsa-ref");
+    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x2);
+    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2);
+
+    if (have_numa_distance) {
+        int size = nb_numa_nodes * nb_numa_nodes * 3 * sizeof(uint32_t);
+        uint32_t *matrix = g_malloc0(size);
+        int idx, i, j;
+
+        for (i = 0; i < nb_numa_nodes; i++) {
+            for (j = 0; j < nb_numa_nodes; j++) {
+                idx = (i * nb_numa_nodes + j) * 3;
+                matrix[idx + 0] = cpu_to_be32(i);
+                matrix[idx + 1] = cpu_to_be32(j);
+                matrix[idx + 2] = cpu_to_be32(numa_info[i].distance[j]);
+            }
+        }
+
+        qemu_fdt_add_subnode(fdt, "/distance-map");
+        qemu_fdt_setprop(fdt, "/distance-map", "distance-matrix",
+                         matrix, size);
+        g_free(matrix);
+    }
+
+    qemu_fdt_add_subnode(vms->fdt, "/cpus");
+
+    for (cpu = vms->smp_cpus - 1; cpu >= 0; cpu--) {
+        char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
+        ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));
+        CPUState *cs = CPU(armcpu);
+
+        qemu_fdt_add_subnode(vms->fdt, nodename);
+
+        if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) {
+            qemu_fdt_setprop_cell(vms->fdt, nodename, "numa-node-id",
+                ms->possible_cpus->cpus[cs->cpu_index].props.node_id);
+        }
+
+        g_free(nodename);
+    }
+}
+
+static void create_one_flash(const char *name, hwaddr flashbase,
+                             hwaddr flashsize, const char *file,
+                             MemoryRegion *sysmem)
+{
+    /*
+     * Create and map a single flash device. We use the same
+     * parameters as the flash devices on the Versatile Express board.
+     */
+    DriveInfo *dinfo = drive_get_next(IF_PFLASH);
+    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    const uint64_t sectorlength = 256 * 1024;
+
+    if (dinfo) {
+        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
+                            &error_abort);
+    }
+
+    qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
+    qdev_prop_set_uint64(dev, "sector-length", sectorlength);
+    qdev_prop_set_uint8(dev, "width", 4);
+    qdev_prop_set_uint8(dev, "device-width", 2);
+    qdev_prop_set_bit(dev, "big-endian", false);
+    qdev_prop_set_uint16(dev, "id0", 0x89);
+    qdev_prop_set_uint16(dev, "id1", 0x18);
+    qdev_prop_set_uint16(dev, "id2", 0x00);
+    qdev_prop_set_uint16(dev, "id3", 0x00);
+    qdev_prop_set_string(dev, "name", name);
+    qdev_init_nofail(dev);
+
+    memory_region_add_subregion(sysmem, flashbase,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
+
+    if (file) {
+        char *fn;
+        int image_size;
+
+        if (drive_get(IF_PFLASH, 0, 0)) {
+            error_report("The contents of the first flash device may be "
+                         "specified with -bios or with -drive if=pflash... "
+                         "but you cannot use both options at once");
+            exit(1);
+        }
+        fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, file);
+        if (!fn) {
+            error_report("Could not find ROM image '%s'", file);
+            exit(1);
+        }
+        image_size = load_image_mr(fn, sysbus_mmio_get_region(sbd, 0));
+        g_free(fn);
+        if (image_size < 0) {
+            error_report("Could not load ROM image '%s'", file);
+            exit(1);
+        }
+    }
+}
+
+static void create_flash(const SBSAMachineState *vms,
+                         MemoryRegion *sysmem,
+                         MemoryRegion *secure_sysmem)
+{
+    /*
+     * Create one secure and nonsecure flash devices to fill SBSA_FLASH
+     * space in the memmap, file passed via -bios goes in the first one.
+     */
+    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
+    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
+
+    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
+                     bios_name, secure_sysmem);
+    create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
+                     NULL, sysmem);
+}
+
+static void create_secure_ram(SBSAMachineState *vms,
+                              MemoryRegion *secure_sysmem)
+{
+    MemoryRegion *secram = g_new(MemoryRegion, 1);
+    hwaddr base = vms->memmap[SBSA_SECURE_MEM].base;
+    hwaddr size = vms->memmap[SBSA_SECURE_MEM].size;
+
+    memory_region_init_ram(secram, NULL, "sbsa-ref.secure-ram", size,
+                           &error_fatal);
+    memory_region_add_subregion(secure_sysmem, base, secram);
+}
+
+static void create_gic(SBSAMachineState *vms, qemu_irq *pic)
+{
+    DeviceState *gicdev;
+    SysBusDevice *gicbusdev;
+    const char *gictype;
+    uint32_t redist0_capacity, redist0_count;
+    int i;
+
+    gictype = gicv3_class_name();
+
+    gicdev = qdev_create(NULL, gictype);
+    qdev_prop_set_uint32(gicdev, "revision", 3);
+    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).
+     */
+    qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
+    qdev_prop_set_bit(gicdev, "has-security-extensions", true);
+
+    redist0_capacity =
+                vms->memmap[SBSA_GIC_REDIST].size / GICV3_REDIST_SIZE;
+    redist0_count = MIN(smp_cpus, redist0_capacity);
+
+    qdev_prop_set_uint32(gicdev, "len-redist-region-count", 1);
+    qdev_prop_set_uint32(gicdev, "redist-region-count[0]", redist0_count);
+
+    qdev_init_nofail(gicdev);
+    gicbusdev = SYS_BUS_DEVICE(gicdev);
+    sysbus_mmio_map(gicbusdev, 0, vms->memmap[SBSA_GIC_DIST].base);
+    sysbus_mmio_map(gicbusdev, 1, vms->memmap[SBSA_GIC_REDIST].base);
+
+    /*
+     * Wire the outputs from each CPU's generic timer and the GICv3
+     * maintenance interrupt signal to the appropriate GIC PPI inputs,
+     * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
+     */
+    for (i = 0; i < smp_cpus; i++) {
+        DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
+        int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
+        int irq;
+        /*
+         * Mapping from the output timer irq lines from the CPU to the
+         * GIC PPI inputs used for this board.
+         */
+        const int timer_irq[] = {
+            [GTIMER_PHYS] = ARCH_TIMER_NS_EL1_IRQ,
+            [GTIMER_VIRT] = ARCH_TIMER_VIRT_IRQ,
+            [GTIMER_HYP]  = ARCH_TIMER_NS_EL2_IRQ,
+            [GTIMER_SEC]  = ARCH_TIMER_S_EL1_IRQ,
+        };
+
+        for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
+            qdev_connect_gpio_out(cpudev, irq,
+                                  qdev_get_gpio_in(gicdev,
+                                                   ppibase + timer_irq[irq]));
+        }
+
+        qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0,
+                                    qdev_get_gpio_in(gicdev, ppibase
+                                                     + ARCH_GIC_MAINT_IRQ));
+        qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0,
+                                    qdev_get_gpio_in(gicdev, ppibase
+                                                     + VIRTUAL_PMU_IRQ));
+
+        sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
+        sysbus_connect_irq(gicbusdev, i + smp_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
+        sysbus_connect_irq(gicbusdev, i + 2 * smp_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
+        sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
+    }
+
+    for (i = 0; i < NUM_IRQS; i++) {
+        pic[i] = qdev_get_gpio_in(gicdev, i);
+    }
+}
+
+static void create_uart(const SBSAMachineState *vms, qemu_irq *pic, int uart,
+                        MemoryRegion *mem, Chardev *chr)
+{
+    hwaddr base = vms->memmap[uart].base;
+    int irq = vms->irqmap[uart];
+    DeviceState *dev = qdev_create(NULL, "pl011");
+    SysBusDevice *s = SYS_BUS_DEVICE(dev);
+
+    qdev_prop_set_chr(dev, "chardev", chr);
+    qdev_init_nofail(dev);
+    memory_region_add_subregion(mem, base,
+                                sysbus_mmio_get_region(s, 0));
+    sysbus_connect_irq(s, 0, pic[irq]);
+}
+
+static void create_rtc(const SBSAMachineState *vms, qemu_irq *pic)
+{
+    hwaddr base = vms->memmap[SBSA_RTC].base;
+    int irq = vms->irqmap[SBSA_RTC];
+
+    sysbus_create_simple("pl031", base, pic[irq]);
+}
+
+static DeviceState *gpio_key_dev;
+static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
+{
+    /* use gpio Pin 3 for power button event */
+    qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
+}
+
+static Notifier sbsa_ref_powerdown_notifier = {
+    .notify = sbsa_ref_powerdown_req
+};
+
+static void create_gpio(const SBSAMachineState *vms, qemu_irq *pic)
+{
+    DeviceState *pl061_dev;
+    hwaddr base = vms->memmap[SBSA_GPIO].base;
+    int irq = vms->irqmap[SBSA_GPIO];
+
+    pl061_dev = sysbus_create_simple("pl061", base, pic[irq]);
+
+    gpio_key_dev = sysbus_create_simple("gpio-key", -1,
+                                        qdev_get_gpio_in(pl061_dev, 3));
+
+    /* connect powerdown request */
+    qemu_register_powerdown_notifier(&sbsa_ref_powerdown_notifier);
+}
+
+static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic)
+{
+    hwaddr base = vms->memmap[SBSA_AHCI].base;
+    int irq = vms->irqmap[SBSA_AHCI];
+    DeviceState *dev;
+    DriveInfo *hd[NUM_SATA_PORTS];
+    SysbusAHCIState *sysahci;
+    AHCIState *ahci;
+    int i;
+
+    dev = qdev_create(NULL, "sysbus-ahci");
+    qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
+
+    sysahci = SYSBUS_AHCI(dev);
+    ahci = &sysahci->ahci;
+    ide_drive_get(hd, ARRAY_SIZE(hd));
+    for (i = 0; i < ahci->ports; i++) {
+        if (hd[i] == NULL) {
+            continue;
+        }
+        ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
+    }
+}
+
+static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic)
+{
+    hwaddr base = vms->memmap[SBSA_EHCI].base;
+    int irq = vms->irqmap[SBSA_EHCI];
+    USBBus *usb_bus;
+
+    sysbus_create_simple("platform-ehci-usb", base, pic[irq]);
+
+    usb_bus = usb_bus_find(-1);
+    usb_create_simple(usb_bus, "usb-kbd");
+    usb_create_simple(usb_bus, "usb-mouse");
+}
+
+static void create_smmu(const SBSAMachineState *vms, qemu_irq *pic,
+                        PCIBus *bus)
+{
+    hwaddr base = vms->memmap[SBSA_SMMU].base;
+    int irq =  vms->irqmap[SBSA_SMMU];
+    DeviceState *dev;
+    int i;
+
+    dev = qdev_create(NULL, "arm-smmuv3");
+
+    object_property_set_link(OBJECT(dev), OBJECT(bus), "primary-bus",
+                             &error_abort);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+    for (i = 0; i < NUM_SMMU_IRQS; i++) {
+        sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
+    }
+}
+
+static void create_pcie(SBSAMachineState *vms, qemu_irq *pic)
+{
+    hwaddr base_ecam = vms->memmap[SBSA_PCIE_ECAM].base;
+    hwaddr size_ecam = vms->memmap[SBSA_PCIE_ECAM].size;
+    hwaddr base_mmio = vms->memmap[SBSA_PCIE_MMIO].base;
+    hwaddr size_mmio = vms->memmap[SBSA_PCIE_MMIO].size;
+    hwaddr base_mmio_high = vms->memmap[SBSA_PCIE_MMIO_HIGH].base;
+    hwaddr size_mmio_high = vms->memmap[SBSA_PCIE_MMIO_HIGH].size;
+    hwaddr base_pio = vms->memmap[SBSA_PCIE_PIO].base;
+    int irq = vms->irqmap[SBSA_PCIE];
+    MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
+    MemoryRegion *ecam_alias, *ecam_reg;
+    DeviceState *dev;
+    PCIHostState *pci;
+    int i;
+
+    dev = qdev_create(NULL, TYPE_GPEX_HOST);
+    qdev_init_nofail(dev);
+
+    /* Map ECAM space */
+    ecam_alias = g_new0(MemoryRegion, 1);
+    ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+    memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
+                             ecam_reg, 0, size_ecam);
+    memory_region_add_subregion(get_system_memory(), base_ecam, ecam_alias);
+
+    /* Map the MMIO space */
+    mmio_alias = g_new0(MemoryRegion, 1);
+    mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+    memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
+                             mmio_reg, base_mmio, size_mmio);
+    memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
+
+    /* Map the MMIO_HIGH space */
+    mmio_alias_high = g_new0(MemoryRegion, 1);
+    memory_region_init_alias(mmio_alias_high, OBJECT(dev), "pcie-mmio-high",
+                             mmio_reg, base_mmio_high, size_mmio_high);
+    memory_region_add_subregion(get_system_memory(), base_mmio_high,
+                                mmio_alias_high);
+
+    /* Map IO port space */
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio);
+
+    for (i = 0; i < GPEX_NUM_IRQS; i++) {
+        sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
+        gpex_set_irq_num(GPEX_HOST(dev), i, irq + i);
+    }
+
+    pci = PCI_HOST_BRIDGE(dev);
+    if (pci->bus) {
+        for (i = 0; i < nb_nics; i++) {
+            NICInfo *nd = &nd_table[i];
+
+            if (!nd->model) {
+                nd->model = g_strdup("e1000e");
+            }
+
+            pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);
+        }
+    }
+
+    pci_create_simple(pci->bus, -1, "VGA");
+
+    create_smmu(vms, pic, pci->bus);
+}
+
+static void *sbsa_ref_dtb(const struct arm_boot_info *binfo, int *fdt_size)
+{
+    const SBSAMachineState *board = container_of(binfo, SBSAMachineState,
+                                                 bootinfo);
+
+    *fdt_size = board->fdt_size;
+    return board->fdt;
+}
+
 static void sbsa_ref_init(MachineState *machine)
 {
     SBSAMachineState *vms = SBSA_MACHINE(machine);
@@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine)
     bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
     const CPUArchIdList *possible_cpus;
     int n, sbsa_max_cpus;
+    qemu_irq pic[NUM_IRQS];
 
     if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
         error_report("sbsa-ref: CPU type other than the built-in "
@@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine)
                                          machine->ram_size);
     memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram);
 
+    create_fdt(vms);
+
+    create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
+
+    create_secure_ram(vms, secure_sysmem);
+
+    create_gic(vms, pic);
+
+    create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0));
+    create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1));
+    create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2));
+
+    create_rtc(vms, pic);
+
+    create_gpio(vms, pic);
+
+    create_ahci(vms, pic);
+
+    create_ehci(vms, pic);
+
+    create_pcie(vms, pic);
+
     vms->bootinfo.ram_size = machine->ram_size;
     vms->bootinfo.kernel_filename = machine->kernel_filename;
     vms->bootinfo.nb_cpus = smp_cpus;
     vms->bootinfo.board_id = -1;
     vms->bootinfo.loader_start = vms->memmap[SBSA_MEM].base;
+    vms->bootinfo.get_dtb = sbsa_ref_dtb;
     vms->bootinfo.firmware_loaded = firmware_loaded;
     arm_load_kernel(ARM_CPU(first_cpu), &vms->bootinfo);
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
@ 2019-04-18  4:04   ` Hongbo Zhang
  0 siblings, 0 replies; 24+ messages in thread
From: Hongbo Zhang @ 2019-04-18  4:04 UTC (permalink / raw)
  To: peter.maydell, qemu-devel, qemu-arm
  Cc: Hongbo Zhang, radoslaw.biernacki, leif.lindholm, ard.biesheuvel

Following the previous patch, this patch adds peripheral devices to the
newly introduced SBSA-ref machine.

Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
---
 hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 451 insertions(+)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 652ec13..3fb0027 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -21,6 +21,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/units.h"
+#include "sysemu/device_tree.h"
 #include "sysemu/numa.h"
 #include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
@@ -28,11 +29,28 @@
 #include "kvm_arm.h"
 #include "hw/arm/arm.h"
 #include "hw/boards.h"
+#include "hw/ide/internal.h"
+#include "hw/ide/ahci_internal.h"
 #include "hw/intc/arm_gicv3_common.h"
+#include "hw/loader.h"
+#include "hw/pci-host/gpex.h"
+#include "hw/usb.h"
+#include "net/net.h"
 
 #define RAMLIMIT_GB 8192
 #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
 
+#define NUM_IRQS        256
+#define NUM_SMMU_IRQS   4
+#define NUM_SATA_PORTS  6
+
+#define VIRTUAL_PMU_IRQ        7
+#define ARCH_GIC_MAINT_IRQ     9
+#define ARCH_TIMER_VIRT_IRQ    11
+#define ARCH_TIMER_S_EL1_IRQ   13
+#define ARCH_TIMER_NS_EL1_IRQ  14
+#define ARCH_TIMER_NS_EL2_IRQ  10
+
 enum {
     SBSA_FLASH,
     SBSA_MEM,
@@ -115,6 +133,415 @@ static const int sbsa_ref_irqmap[] = {
     [SBSA_EHCI] = 11,
 };
 
+/*
+ * Firmware on this machine only uses ACPI table to load OS, these limited
+ * device tree nodes are just to let firmware know the info which varies from
+ * command line parameters, so it is not necessary to be fully compatible
+ * with the kernel CPU and NUMA binding rules.
+ */
+static void create_fdt(SBSAMachineState *vms)
+{
+    void *fdt = create_device_tree(&vms->fdt_size);
+    const MachineState *ms = MACHINE(vms);
+    int cpu;
+
+    if (!fdt) {
+        error_report("create_device_tree() failed");
+        exit(1);
+    }
+
+    vms->fdt = fdt;
+
+    qemu_fdt_setprop_string(fdt, "/", "compatible", "linux,sbsa-ref");
+    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x2);
+    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2);
+
+    if (have_numa_distance) {
+        int size = nb_numa_nodes * nb_numa_nodes * 3 * sizeof(uint32_t);
+        uint32_t *matrix = g_malloc0(size);
+        int idx, i, j;
+
+        for (i = 0; i < nb_numa_nodes; i++) {
+            for (j = 0; j < nb_numa_nodes; j++) {
+                idx = (i * nb_numa_nodes + j) * 3;
+                matrix[idx + 0] = cpu_to_be32(i);
+                matrix[idx + 1] = cpu_to_be32(j);
+                matrix[idx + 2] = cpu_to_be32(numa_info[i].distance[j]);
+            }
+        }
+
+        qemu_fdt_add_subnode(fdt, "/distance-map");
+        qemu_fdt_setprop(fdt, "/distance-map", "distance-matrix",
+                         matrix, size);
+        g_free(matrix);
+    }
+
+    qemu_fdt_add_subnode(vms->fdt, "/cpus");
+
+    for (cpu = vms->smp_cpus - 1; cpu >= 0; cpu--) {
+        char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
+        ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));
+        CPUState *cs = CPU(armcpu);
+
+        qemu_fdt_add_subnode(vms->fdt, nodename);
+
+        if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) {
+            qemu_fdt_setprop_cell(vms->fdt, nodename, "numa-node-id",
+                ms->possible_cpus->cpus[cs->cpu_index].props.node_id);
+        }
+
+        g_free(nodename);
+    }
+}
+
+static void create_one_flash(const char *name, hwaddr flashbase,
+                             hwaddr flashsize, const char *file,
+                             MemoryRegion *sysmem)
+{
+    /*
+     * Create and map a single flash device. We use the same
+     * parameters as the flash devices on the Versatile Express board.
+     */
+    DriveInfo *dinfo = drive_get_next(IF_PFLASH);
+    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    const uint64_t sectorlength = 256 * 1024;
+
+    if (dinfo) {
+        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
+                            &error_abort);
+    }
+
+    qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
+    qdev_prop_set_uint64(dev, "sector-length", sectorlength);
+    qdev_prop_set_uint8(dev, "width", 4);
+    qdev_prop_set_uint8(dev, "device-width", 2);
+    qdev_prop_set_bit(dev, "big-endian", false);
+    qdev_prop_set_uint16(dev, "id0", 0x89);
+    qdev_prop_set_uint16(dev, "id1", 0x18);
+    qdev_prop_set_uint16(dev, "id2", 0x00);
+    qdev_prop_set_uint16(dev, "id3", 0x00);
+    qdev_prop_set_string(dev, "name", name);
+    qdev_init_nofail(dev);
+
+    memory_region_add_subregion(sysmem, flashbase,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
+
+    if (file) {
+        char *fn;
+        int image_size;
+
+        if (drive_get(IF_PFLASH, 0, 0)) {
+            error_report("The contents of the first flash device may be "
+                         "specified with -bios or with -drive if=pflash... "
+                         "but you cannot use both options at once");
+            exit(1);
+        }
+        fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, file);
+        if (!fn) {
+            error_report("Could not find ROM image '%s'", file);
+            exit(1);
+        }
+        image_size = load_image_mr(fn, sysbus_mmio_get_region(sbd, 0));
+        g_free(fn);
+        if (image_size < 0) {
+            error_report("Could not load ROM image '%s'", file);
+            exit(1);
+        }
+    }
+}
+
+static void create_flash(const SBSAMachineState *vms,
+                         MemoryRegion *sysmem,
+                         MemoryRegion *secure_sysmem)
+{
+    /*
+     * Create one secure and nonsecure flash devices to fill SBSA_FLASH
+     * space in the memmap, file passed via -bios goes in the first one.
+     */
+    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
+    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
+
+    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
+                     bios_name, secure_sysmem);
+    create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
+                     NULL, sysmem);
+}
+
+static void create_secure_ram(SBSAMachineState *vms,
+                              MemoryRegion *secure_sysmem)
+{
+    MemoryRegion *secram = g_new(MemoryRegion, 1);
+    hwaddr base = vms->memmap[SBSA_SECURE_MEM].base;
+    hwaddr size = vms->memmap[SBSA_SECURE_MEM].size;
+
+    memory_region_init_ram(secram, NULL, "sbsa-ref.secure-ram", size,
+                           &error_fatal);
+    memory_region_add_subregion(secure_sysmem, base, secram);
+}
+
+static void create_gic(SBSAMachineState *vms, qemu_irq *pic)
+{
+    DeviceState *gicdev;
+    SysBusDevice *gicbusdev;
+    const char *gictype;
+    uint32_t redist0_capacity, redist0_count;
+    int i;
+
+    gictype = gicv3_class_name();
+
+    gicdev = qdev_create(NULL, gictype);
+    qdev_prop_set_uint32(gicdev, "revision", 3);
+    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).
+     */
+    qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
+    qdev_prop_set_bit(gicdev, "has-security-extensions", true);
+
+    redist0_capacity =
+                vms->memmap[SBSA_GIC_REDIST].size / GICV3_REDIST_SIZE;
+    redist0_count = MIN(smp_cpus, redist0_capacity);
+
+    qdev_prop_set_uint32(gicdev, "len-redist-region-count", 1);
+    qdev_prop_set_uint32(gicdev, "redist-region-count[0]", redist0_count);
+
+    qdev_init_nofail(gicdev);
+    gicbusdev = SYS_BUS_DEVICE(gicdev);
+    sysbus_mmio_map(gicbusdev, 0, vms->memmap[SBSA_GIC_DIST].base);
+    sysbus_mmio_map(gicbusdev, 1, vms->memmap[SBSA_GIC_REDIST].base);
+
+    /*
+     * Wire the outputs from each CPU's generic timer and the GICv3
+     * maintenance interrupt signal to the appropriate GIC PPI inputs,
+     * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
+     */
+    for (i = 0; i < smp_cpus; i++) {
+        DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
+        int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
+        int irq;
+        /*
+         * Mapping from the output timer irq lines from the CPU to the
+         * GIC PPI inputs used for this board.
+         */
+        const int timer_irq[] = {
+            [GTIMER_PHYS] = ARCH_TIMER_NS_EL1_IRQ,
+            [GTIMER_VIRT] = ARCH_TIMER_VIRT_IRQ,
+            [GTIMER_HYP]  = ARCH_TIMER_NS_EL2_IRQ,
+            [GTIMER_SEC]  = ARCH_TIMER_S_EL1_IRQ,
+        };
+
+        for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
+            qdev_connect_gpio_out(cpudev, irq,
+                                  qdev_get_gpio_in(gicdev,
+                                                   ppibase + timer_irq[irq]));
+        }
+
+        qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0,
+                                    qdev_get_gpio_in(gicdev, ppibase
+                                                     + ARCH_GIC_MAINT_IRQ));
+        qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0,
+                                    qdev_get_gpio_in(gicdev, ppibase
+                                                     + VIRTUAL_PMU_IRQ));
+
+        sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
+        sysbus_connect_irq(gicbusdev, i + smp_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
+        sysbus_connect_irq(gicbusdev, i + 2 * smp_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
+        sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
+    }
+
+    for (i = 0; i < NUM_IRQS; i++) {
+        pic[i] = qdev_get_gpio_in(gicdev, i);
+    }
+}
+
+static void create_uart(const SBSAMachineState *vms, qemu_irq *pic, int uart,
+                        MemoryRegion *mem, Chardev *chr)
+{
+    hwaddr base = vms->memmap[uart].base;
+    int irq = vms->irqmap[uart];
+    DeviceState *dev = qdev_create(NULL, "pl011");
+    SysBusDevice *s = SYS_BUS_DEVICE(dev);
+
+    qdev_prop_set_chr(dev, "chardev", chr);
+    qdev_init_nofail(dev);
+    memory_region_add_subregion(mem, base,
+                                sysbus_mmio_get_region(s, 0));
+    sysbus_connect_irq(s, 0, pic[irq]);
+}
+
+static void create_rtc(const SBSAMachineState *vms, qemu_irq *pic)
+{
+    hwaddr base = vms->memmap[SBSA_RTC].base;
+    int irq = vms->irqmap[SBSA_RTC];
+
+    sysbus_create_simple("pl031", base, pic[irq]);
+}
+
+static DeviceState *gpio_key_dev;
+static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
+{
+    /* use gpio Pin 3 for power button event */
+    qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
+}
+
+static Notifier sbsa_ref_powerdown_notifier = {
+    .notify = sbsa_ref_powerdown_req
+};
+
+static void create_gpio(const SBSAMachineState *vms, qemu_irq *pic)
+{
+    DeviceState *pl061_dev;
+    hwaddr base = vms->memmap[SBSA_GPIO].base;
+    int irq = vms->irqmap[SBSA_GPIO];
+
+    pl061_dev = sysbus_create_simple("pl061", base, pic[irq]);
+
+    gpio_key_dev = sysbus_create_simple("gpio-key", -1,
+                                        qdev_get_gpio_in(pl061_dev, 3));
+
+    /* connect powerdown request */
+    qemu_register_powerdown_notifier(&sbsa_ref_powerdown_notifier);
+}
+
+static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic)
+{
+    hwaddr base = vms->memmap[SBSA_AHCI].base;
+    int irq = vms->irqmap[SBSA_AHCI];
+    DeviceState *dev;
+    DriveInfo *hd[NUM_SATA_PORTS];
+    SysbusAHCIState *sysahci;
+    AHCIState *ahci;
+    int i;
+
+    dev = qdev_create(NULL, "sysbus-ahci");
+    qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
+
+    sysahci = SYSBUS_AHCI(dev);
+    ahci = &sysahci->ahci;
+    ide_drive_get(hd, ARRAY_SIZE(hd));
+    for (i = 0; i < ahci->ports; i++) {
+        if (hd[i] == NULL) {
+            continue;
+        }
+        ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
+    }
+}
+
+static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic)
+{
+    hwaddr base = vms->memmap[SBSA_EHCI].base;
+    int irq = vms->irqmap[SBSA_EHCI];
+    USBBus *usb_bus;
+
+    sysbus_create_simple("platform-ehci-usb", base, pic[irq]);
+
+    usb_bus = usb_bus_find(-1);
+    usb_create_simple(usb_bus, "usb-kbd");
+    usb_create_simple(usb_bus, "usb-mouse");
+}
+
+static void create_smmu(const SBSAMachineState *vms, qemu_irq *pic,
+                        PCIBus *bus)
+{
+    hwaddr base = vms->memmap[SBSA_SMMU].base;
+    int irq =  vms->irqmap[SBSA_SMMU];
+    DeviceState *dev;
+    int i;
+
+    dev = qdev_create(NULL, "arm-smmuv3");
+
+    object_property_set_link(OBJECT(dev), OBJECT(bus), "primary-bus",
+                             &error_abort);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+    for (i = 0; i < NUM_SMMU_IRQS; i++) {
+        sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
+    }
+}
+
+static void create_pcie(SBSAMachineState *vms, qemu_irq *pic)
+{
+    hwaddr base_ecam = vms->memmap[SBSA_PCIE_ECAM].base;
+    hwaddr size_ecam = vms->memmap[SBSA_PCIE_ECAM].size;
+    hwaddr base_mmio = vms->memmap[SBSA_PCIE_MMIO].base;
+    hwaddr size_mmio = vms->memmap[SBSA_PCIE_MMIO].size;
+    hwaddr base_mmio_high = vms->memmap[SBSA_PCIE_MMIO_HIGH].base;
+    hwaddr size_mmio_high = vms->memmap[SBSA_PCIE_MMIO_HIGH].size;
+    hwaddr base_pio = vms->memmap[SBSA_PCIE_PIO].base;
+    int irq = vms->irqmap[SBSA_PCIE];
+    MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
+    MemoryRegion *ecam_alias, *ecam_reg;
+    DeviceState *dev;
+    PCIHostState *pci;
+    int i;
+
+    dev = qdev_create(NULL, TYPE_GPEX_HOST);
+    qdev_init_nofail(dev);
+
+    /* Map ECAM space */
+    ecam_alias = g_new0(MemoryRegion, 1);
+    ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+    memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
+                             ecam_reg, 0, size_ecam);
+    memory_region_add_subregion(get_system_memory(), base_ecam, ecam_alias);
+
+    /* Map the MMIO space */
+    mmio_alias = g_new0(MemoryRegion, 1);
+    mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+    memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
+                             mmio_reg, base_mmio, size_mmio);
+    memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
+
+    /* Map the MMIO_HIGH space */
+    mmio_alias_high = g_new0(MemoryRegion, 1);
+    memory_region_init_alias(mmio_alias_high, OBJECT(dev), "pcie-mmio-high",
+                             mmio_reg, base_mmio_high, size_mmio_high);
+    memory_region_add_subregion(get_system_memory(), base_mmio_high,
+                                mmio_alias_high);
+
+    /* Map IO port space */
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio);
+
+    for (i = 0; i < GPEX_NUM_IRQS; i++) {
+        sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
+        gpex_set_irq_num(GPEX_HOST(dev), i, irq + i);
+    }
+
+    pci = PCI_HOST_BRIDGE(dev);
+    if (pci->bus) {
+        for (i = 0; i < nb_nics; i++) {
+            NICInfo *nd = &nd_table[i];
+
+            if (!nd->model) {
+                nd->model = g_strdup("e1000e");
+            }
+
+            pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);
+        }
+    }
+
+    pci_create_simple(pci->bus, -1, "VGA");
+
+    create_smmu(vms, pic, pci->bus);
+}
+
+static void *sbsa_ref_dtb(const struct arm_boot_info *binfo, int *fdt_size)
+{
+    const SBSAMachineState *board = container_of(binfo, SBSAMachineState,
+                                                 bootinfo);
+
+    *fdt_size = board->fdt_size;
+    return board->fdt;
+}
+
 static void sbsa_ref_init(MachineState *machine)
 {
     SBSAMachineState *vms = SBSA_MACHINE(machine);
@@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine)
     bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
     const CPUArchIdList *possible_cpus;
     int n, sbsa_max_cpus;
+    qemu_irq pic[NUM_IRQS];
 
     if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
         error_report("sbsa-ref: CPU type other than the built-in "
@@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine)
                                          machine->ram_size);
     memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram);
 
+    create_fdt(vms);
+
+    create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
+
+    create_secure_ram(vms, secure_sysmem);
+
+    create_gic(vms, pic);
+
+    create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0));
+    create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1));
+    create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2));
+
+    create_rtc(vms, pic);
+
+    create_gpio(vms, pic);
+
+    create_ahci(vms, pic);
+
+    create_ehci(vms, pic);
+
+    create_pcie(vms, pic);
+
     vms->bootinfo.ram_size = machine->ram_size;
     vms->bootinfo.kernel_filename = machine->kernel_filename;
     vms->bootinfo.nb_cpus = smp_cpus;
     vms->bootinfo.board_id = -1;
     vms->bootinfo.loader_start = vms->memmap[SBSA_MEM].base;
+    vms->bootinfo.get_dtb = sbsa_ref_dtb;
     vms->bootinfo.firmware_loaded = firmware_loaded;
     arm_load_kernel(ARM_CPU(first_cpu), &vms->bootinfo);
 }
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 1/2] hw/arm: Add arm SBSA reference machine, skeleton part
@ 2019-04-30 14:03     ` Peter Maydell
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2019-04-30 14:03 UTC (permalink / raw)
  To: Hongbo Zhang
  Cc: QEMU Developers, qemu-arm, Ard Biesheuvel, Leif Lindholm,
	Radoslaw Biernacki, Igor Mammedov, Eric Auger

On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>
> For the Aarch64, there is one machine 'virt', it is primarily meant to
> run on KVM and execute virtualization workloads, but we need an
> environment as faithful as possible to physical hardware, for supporting
> firmware and OS development for pysical Aarch64 machines.
>
> This patch introduces new machine type 'sbsa-ref' with main features:
>  - Based on 'virt' machine type.
>  - A new memory map.
>  - CPU type cortex-a57.
>  - EL2 and EL3 are enabled.
>  - GIC version 3.
>  - System bus AHCI controller.
>  - System bus EHCI controller.
>  - CDROM and hard disc on AHCI bus.
>  - E1000E ethernet card on PCIE bus.
>  - VGA display adaptor on PCIE bus.
>  - No virtio deivces.
>  - No fw_cfg device.
>  - No ACPI table supplied.
>  - Only minimal device tree nodes.
>
> Arm Trusted Firmware and UEFI porting to this are done accordingly, and
> it should supply ACPI tables to load OS, the minimal device tree nodes
> supplied from this platform are only to pass the dynamic info reflecting
> command line input to firmware, not for loading OS.
>
> To make the review easier, this task is split into two patches, the
> fundamental sceleton part and the peripheral devices part, this patch is
> the first part.
>
> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

Hi. This patch looks good to me. I have a couple of very minor
comments below.

The only other thing I'm not sure about is whether the recent work
(both in master and in pending patchset) to add support for memory
hotplug and nvdimms to the 'virt' board is applicable here. I've
cc'd Igor and Eric to ask their opinion on that question.

> +static const MemMapEntry sbsa_ref_memmap[] = {
> +    /* 512M boot ROM */
> +    [SBSA_FLASH] =              {          0, 0x20000000 },
> +    /* 512M secure memory */
> +    [SBSA_SECURE_MEM] =         { 0x20000000, 0x20000000 },
> +    /* Space reserved for CPU peripheral devices */
> +    [SBSA_CPUPERIPHS] =         { 0x40000000, 0x00040000 },
> +    [SBSA_GIC_DIST] =           { 0x40060000, 0x00010000 },
> +    [SBSA_GIC_REDIST] =         { 0x40080000, 0x04000000 },
> +    [SBSA_UART] =               { 0x60000000, 0x00001000 },
> +    [SBSA_RTC] =                { 0x60010000, 0x00001000 },
> +    [SBSA_GPIO] =               { 0x60020000, 0x00001000 },
> +    [SBSA_SECURE_UART] =        { 0x60030000, 0x00001000 },
> +    [SBSA_SECURE_UART_MM] =     { 0x60040000, 0x00001000 },
> +    [SBSA_SMMU] =               { 0x60050000, 0x00020000 },
> +    /* Space here reserved for more SMMUs */
> +    [SBSA_AHCI] =               { 0x60100000, 0x00010000 },
> +    [SBSA_EHCI] =               { 0x60110000, 0x00010000 },
> +    /* Space here reserved for other devices */
> +    [SBSA_PCIE_PIO] =           { 0x7fff0000, 0x00010000 },
> +    /* 32-bit address PCIE MMIO space */
> +    [SBSA_PCIE_MMIO] =          { 0x80000000, 0x70000000 },
> +    /* 256M PCIE ECAM space */
> +    [SBSA_PCIE_ECAM] =          { 0xf0000000, 0x10000000 },
> +    /* ~1TB PCIE MMIO space (4GB to 1024GB boundary) */
> +    [SBSA_PCIE_MMIO_HIGH] =     { 0x100000000ULL, 0xFF00000000ULL },
> +    [SBSA_MEM] =                { 0x10000000000ULL, RAMLIMIT_BYTES },
> +};
> +
> +static const int sbsa_ref_irqmap[] = {
> +    [SBSA_UART] = 1,
> +    [SBSA_RTC] = 2,
> +    [SBSA_PCIE] = 3, /* ... to 6 */
> +    [SBSA_GPIO] = 7,
> +    [SBSA_SECURE_UART] = 8,
> +    [SBSA_SECURE_UART_MM] = 9,
> +    [SBSA_AHCI] = 10,
> +    [SBSA_EHCI] = 11,
> +};

Since we only have one memory map and one irqmap, I think that
rather than setting up vms->memmap[x] and vms->irqmap[x] and then
always using those, we should just refer directly to the globals.
The indirection in virt is originally because I was thinking we
might want to have more than one layout (and because the code
derives from the vexpress boards, which really do have two
different layouts depending on the version), and then it turned
out to be useful that we could pass the VirtMachineState* to
the ACPI table generation code and let it get at the addresses
and IRQ numbers that way. But neither of those applies here, I think.

> +
> +static void sbsa_ref_init(MachineState *machine)
> +{
> +    SBSAMachineState *vms = SBSA_MACHINE(machine);

This is a very trivial nitpick, but I think we should call
this variable 'sms', not 'vms', since we changed the name of
the struct type. (Same applies in some other functions later.)

> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
> +    MemoryRegion *sysmem = get_system_memory();
> +    MemoryRegion *secure_sysmem = NULL;
> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
> +    bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> +    const CPUArchIdList *possible_cpus;
> +    int n, sbsa_max_cpus;
> +
> +    if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
> +        error_report("sbsa-ref: CPU type other than the built-in "
> +                     "cortex-a57 not supported");
> +        exit(1);
> +    }
> +
> +    if (kvm_enabled()) {
> +        error_report("sbsa-ref: KVM is not supported at this machine");

"for this machine".

> +        exit(1);
> +    }

thanks
-- PMM

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 1/2] hw/arm: Add arm SBSA reference machine, skeleton part
@ 2019-04-30 14:03     ` Peter Maydell
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2019-04-30 14:03 UTC (permalink / raw)
  To: Hongbo Zhang
  Cc: Radoslaw Biernacki, Ard Biesheuvel, QEMU Developers,
	Leif Lindholm, Eric Auger, qemu-arm, Igor Mammedov

On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>
> For the Aarch64, there is one machine 'virt', it is primarily meant to
> run on KVM and execute virtualization workloads, but we need an
> environment as faithful as possible to physical hardware, for supporting
> firmware and OS development for pysical Aarch64 machines.
>
> This patch introduces new machine type 'sbsa-ref' with main features:
>  - Based on 'virt' machine type.
>  - A new memory map.
>  - CPU type cortex-a57.
>  - EL2 and EL3 are enabled.
>  - GIC version 3.
>  - System bus AHCI controller.
>  - System bus EHCI controller.
>  - CDROM and hard disc on AHCI bus.
>  - E1000E ethernet card on PCIE bus.
>  - VGA display adaptor on PCIE bus.
>  - No virtio deivces.
>  - No fw_cfg device.
>  - No ACPI table supplied.
>  - Only minimal device tree nodes.
>
> Arm Trusted Firmware and UEFI porting to this are done accordingly, and
> it should supply ACPI tables to load OS, the minimal device tree nodes
> supplied from this platform are only to pass the dynamic info reflecting
> command line input to firmware, not for loading OS.
>
> To make the review easier, this task is split into two patches, the
> fundamental sceleton part and the peripheral devices part, this patch is
> the first part.
>
> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

Hi. This patch looks good to me. I have a couple of very minor
comments below.

The only other thing I'm not sure about is whether the recent work
(both in master and in pending patchset) to add support for memory
hotplug and nvdimms to the 'virt' board is applicable here. I've
cc'd Igor and Eric to ask their opinion on that question.

> +static const MemMapEntry sbsa_ref_memmap[] = {
> +    /* 512M boot ROM */
> +    [SBSA_FLASH] =              {          0, 0x20000000 },
> +    /* 512M secure memory */
> +    [SBSA_SECURE_MEM] =         { 0x20000000, 0x20000000 },
> +    /* Space reserved for CPU peripheral devices */
> +    [SBSA_CPUPERIPHS] =         { 0x40000000, 0x00040000 },
> +    [SBSA_GIC_DIST] =           { 0x40060000, 0x00010000 },
> +    [SBSA_GIC_REDIST] =         { 0x40080000, 0x04000000 },
> +    [SBSA_UART] =               { 0x60000000, 0x00001000 },
> +    [SBSA_RTC] =                { 0x60010000, 0x00001000 },
> +    [SBSA_GPIO] =               { 0x60020000, 0x00001000 },
> +    [SBSA_SECURE_UART] =        { 0x60030000, 0x00001000 },
> +    [SBSA_SECURE_UART_MM] =     { 0x60040000, 0x00001000 },
> +    [SBSA_SMMU] =               { 0x60050000, 0x00020000 },
> +    /* Space here reserved for more SMMUs */
> +    [SBSA_AHCI] =               { 0x60100000, 0x00010000 },
> +    [SBSA_EHCI] =               { 0x60110000, 0x00010000 },
> +    /* Space here reserved for other devices */
> +    [SBSA_PCIE_PIO] =           { 0x7fff0000, 0x00010000 },
> +    /* 32-bit address PCIE MMIO space */
> +    [SBSA_PCIE_MMIO] =          { 0x80000000, 0x70000000 },
> +    /* 256M PCIE ECAM space */
> +    [SBSA_PCIE_ECAM] =          { 0xf0000000, 0x10000000 },
> +    /* ~1TB PCIE MMIO space (4GB to 1024GB boundary) */
> +    [SBSA_PCIE_MMIO_HIGH] =     { 0x100000000ULL, 0xFF00000000ULL },
> +    [SBSA_MEM] =                { 0x10000000000ULL, RAMLIMIT_BYTES },
> +};
> +
> +static const int sbsa_ref_irqmap[] = {
> +    [SBSA_UART] = 1,
> +    [SBSA_RTC] = 2,
> +    [SBSA_PCIE] = 3, /* ... to 6 */
> +    [SBSA_GPIO] = 7,
> +    [SBSA_SECURE_UART] = 8,
> +    [SBSA_SECURE_UART_MM] = 9,
> +    [SBSA_AHCI] = 10,
> +    [SBSA_EHCI] = 11,
> +};

Since we only have one memory map and one irqmap, I think that
rather than setting up vms->memmap[x] and vms->irqmap[x] and then
always using those, we should just refer directly to the globals.
The indirection in virt is originally because I was thinking we
might want to have more than one layout (and because the code
derives from the vexpress boards, which really do have two
different layouts depending on the version), and then it turned
out to be useful that we could pass the VirtMachineState* to
the ACPI table generation code and let it get at the addresses
and IRQ numbers that way. But neither of those applies here, I think.

> +
> +static void sbsa_ref_init(MachineState *machine)
> +{
> +    SBSAMachineState *vms = SBSA_MACHINE(machine);

This is a very trivial nitpick, but I think we should call
this variable 'sms', not 'vms', since we changed the name of
the struct type. (Same applies in some other functions later.)

> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
> +    MemoryRegion *sysmem = get_system_memory();
> +    MemoryRegion *secure_sysmem = NULL;
> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
> +    bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> +    const CPUArchIdList *possible_cpus;
> +    int n, sbsa_max_cpus;
> +
> +    if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
> +        error_report("sbsa-ref: CPU type other than the built-in "
> +                     "cortex-a57 not supported");
> +        exit(1);
> +    }
> +
> +    if (kvm_enabled()) {
> +        error_report("sbsa-ref: KVM is not supported at this machine");

"for this machine".

> +        exit(1);
> +    }

thanks
-- PMM


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
@ 2019-04-30 14:16     ` Peter Maydell
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2019-04-30 14:16 UTC (permalink / raw)
  To: Hongbo Zhang
  Cc: QEMU Developers, qemu-arm, Ard Biesheuvel, Leif Lindholm,
	Radoslaw Biernacki, Markus Armbruster

On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>
> Following the previous patch, this patch adds peripheral devices to the
> newly introduced SBSA-ref machine.
>
> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> ---
>  hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 451 insertions(+)

Some fairly minor comments on this one.

> +static void create_flash(const SBSAMachineState *vms,
> +                         MemoryRegion *sysmem,
> +                         MemoryRegion *secure_sysmem)
> +{
> +    /*
> +     * Create one secure and nonsecure flash devices to fill SBSA_FLASH
> +     * space in the memmap, file passed via -bios goes in the first one.
> +     */
> +    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
> +    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
> +
> +    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
> +                     bios_name, secure_sysmem);
> +    create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
> +                     NULL, sysmem);
> +}

I think Markus might have an opinion on the best way to create
flash devices on a new board model. Is "just create two flash
devices the way the virt board does" the right thing?

> +static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic)
> +{
> +    hwaddr base = vms->memmap[SBSA_AHCI].base;
> +    int irq = vms->irqmap[SBSA_AHCI];
> +    DeviceState *dev;
> +    DriveInfo *hd[NUM_SATA_PORTS];
> +    SysbusAHCIState *sysahci;
> +    AHCIState *ahci;
> +    int i;
> +
> +    dev = qdev_create(NULL, "sysbus-ahci");
> +    qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS);
> +    qdev_init_nofail(dev);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> +
> +    sysahci = SYSBUS_AHCI(dev);
> +    ahci = &sysahci->ahci;
> +    ide_drive_get(hd, ARRAY_SIZE(hd));
> +    for (i = 0; i < ahci->ports; i++) {
> +        if (hd[i] == NULL) {
> +            continue;
> +        }
> +        ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
> +    }
> +}
> +
> +static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic)
> +{
> +    hwaddr base = vms->memmap[SBSA_EHCI].base;
> +    int irq = vms->irqmap[SBSA_EHCI];
> +    USBBus *usb_bus;
> +
> +    sysbus_create_simple("platform-ehci-usb", base, pic[irq]);
> +
> +    usb_bus = usb_bus_find(-1);
> +    usb_create_simple(usb_bus, "usb-kbd");
> +    usb_create_simple(usb_bus, "usb-mouse");

I don't think we should automatically create the usb keyboard
and mouse devices. The user can do it on the command line if they
want them.

>  static void sbsa_ref_init(MachineState *machine)
>  {
>      SBSAMachineState *vms = SBSA_MACHINE(machine);
> @@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine)
>      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
>      const CPUArchIdList *possible_cpus;
>      int n, sbsa_max_cpus;
> +    qemu_irq pic[NUM_IRQS];
>
>      if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
>          error_report("sbsa-ref: CPU type other than the built-in "
> @@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine)
>                                           machine->ram_size);
>      memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram);
>
> +    create_fdt(vms);
> +
> +    create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
> +
> +    create_secure_ram(vms, secure_sysmem);
> +
> +    create_gic(vms, pic);
> +
> +    create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0));
> +    create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1));
> +    create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2));

What's the third UART for (ie what is the name intended to mean)?
Should we have more than one non-secure UART?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
@ 2019-04-30 14:16     ` Peter Maydell
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2019-04-30 14:16 UTC (permalink / raw)
  To: Hongbo Zhang
  Cc: Radoslaw Biernacki, Ard Biesheuvel, Markus Armbruster,
	Leif Lindholm, QEMU Developers, qemu-arm

On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>
> Following the previous patch, this patch adds peripheral devices to the
> newly introduced SBSA-ref machine.
>
> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> ---
>  hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 451 insertions(+)

Some fairly minor comments on this one.

> +static void create_flash(const SBSAMachineState *vms,
> +                         MemoryRegion *sysmem,
> +                         MemoryRegion *secure_sysmem)
> +{
> +    /*
> +     * Create one secure and nonsecure flash devices to fill SBSA_FLASH
> +     * space in the memmap, file passed via -bios goes in the first one.
> +     */
> +    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
> +    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
> +
> +    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
> +                     bios_name, secure_sysmem);
> +    create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
> +                     NULL, sysmem);
> +}

I think Markus might have an opinion on the best way to create
flash devices on a new board model. Is "just create two flash
devices the way the virt board does" the right thing?

> +static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic)
> +{
> +    hwaddr base = vms->memmap[SBSA_AHCI].base;
> +    int irq = vms->irqmap[SBSA_AHCI];
> +    DeviceState *dev;
> +    DriveInfo *hd[NUM_SATA_PORTS];
> +    SysbusAHCIState *sysahci;
> +    AHCIState *ahci;
> +    int i;
> +
> +    dev = qdev_create(NULL, "sysbus-ahci");
> +    qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS);
> +    qdev_init_nofail(dev);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> +
> +    sysahci = SYSBUS_AHCI(dev);
> +    ahci = &sysahci->ahci;
> +    ide_drive_get(hd, ARRAY_SIZE(hd));
> +    for (i = 0; i < ahci->ports; i++) {
> +        if (hd[i] == NULL) {
> +            continue;
> +        }
> +        ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
> +    }
> +}
> +
> +static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic)
> +{
> +    hwaddr base = vms->memmap[SBSA_EHCI].base;
> +    int irq = vms->irqmap[SBSA_EHCI];
> +    USBBus *usb_bus;
> +
> +    sysbus_create_simple("platform-ehci-usb", base, pic[irq]);
> +
> +    usb_bus = usb_bus_find(-1);
> +    usb_create_simple(usb_bus, "usb-kbd");
> +    usb_create_simple(usb_bus, "usb-mouse");

I don't think we should automatically create the usb keyboard
and mouse devices. The user can do it on the command line if they
want them.

>  static void sbsa_ref_init(MachineState *machine)
>  {
>      SBSAMachineState *vms = SBSA_MACHINE(machine);
> @@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine)
>      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
>      const CPUArchIdList *possible_cpus;
>      int n, sbsa_max_cpus;
> +    qemu_irq pic[NUM_IRQS];
>
>      if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
>          error_report("sbsa-ref: CPU type other than the built-in "
> @@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine)
>                                           machine->ram_size);
>      memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram);
>
> +    create_fdt(vms);
> +
> +    create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
> +
> +    create_secure_ram(vms, secure_sysmem);
> +
> +    create_gic(vms, pic);
> +
> +    create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0));
> +    create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1));
> +    create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2));

What's the third UART for (ie what is the name intended to mean)?
Should we have more than one non-secure UART?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/2] Add Arm SBSA Reference Machine
@ 2019-04-30 14:18   ` Peter Maydell
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2019-04-30 14:18 UTC (permalink / raw)
  To: Hongbo Zhang
  Cc: QEMU Developers, qemu-arm, Ard Biesheuvel, Leif Lindholm,
	Radoslaw Biernacki

On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>
> For the Aarch64, there is one machine 'virt', it is primarily meant to
> run on KVM and execute virtualization workloads, but we need an
> environment as faithful as possible to physical hardware,  to support
> firmware and OS development for pysical Aarch64 machines.
>
> This machine comes with:
>  - Re-designed memory map.
>  - CPU cortex-a57.
>  - EL2 and EL3 enabled.
>  - GIC version 3.
>  - System bus AHCI controller.
>  - System bus XHCI controller.
>  - CDROM and hard disc on AHCI bus.
>  - E1000E ethernet card on PCIE bus.
>  - VGA display adaptor on PCIE bus.
>  - Only minimal device tree nodes.
> And without:
>  - virtio deivces.
>  - fw_cfg device.
>  - ACPI tables.

I've had a look through these patches -- my comments are all
pretty minor. There are a few places where I don't really know
the right answer and have asked other people to weigh in on
whether changes (proposed or recent) for the virt board should
be applied here too.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 0/2] Add Arm SBSA Reference Machine
@ 2019-04-30 14:18   ` Peter Maydell
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2019-04-30 14:18 UTC (permalink / raw)
  To: Hongbo Zhang
  Cc: qemu-arm, Radoslaw Biernacki, QEMU Developers, Leif Lindholm,
	Ard Biesheuvel

On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>
> For the Aarch64, there is one machine 'virt', it is primarily meant to
> run on KVM and execute virtualization workloads, but we need an
> environment as faithful as possible to physical hardware,  to support
> firmware and OS development for pysical Aarch64 machines.
>
> This machine comes with:
>  - Re-designed memory map.
>  - CPU cortex-a57.
>  - EL2 and EL3 enabled.
>  - GIC version 3.
>  - System bus AHCI controller.
>  - System bus XHCI controller.
>  - CDROM and hard disc on AHCI bus.
>  - E1000E ethernet card on PCIE bus.
>  - VGA display adaptor on PCIE bus.
>  - Only minimal device tree nodes.
> And without:
>  - virtio deivces.
>  - fw_cfg device.
>  - ACPI tables.

I've had a look through these patches -- my comments are all
pretty minor. There are a few places where I don't really know
the right answer and have asked other people to weigh in on
whether changes (proposed or recent) for the virt board should
be applied here too.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 1/2] hw/arm: Add arm SBSA reference machine, skeleton part
  2019-04-30 14:03     ` Peter Maydell
  (?)
@ 2019-05-08 11:22     ` Hongbo Zhang
  2019-05-09 14:27       ` Igor Mammedov
  -1 siblings, 1 reply; 24+ messages in thread
From: Hongbo Zhang @ 2019-05-08 11:22 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Radoslaw Biernacki, Ard Biesheuvel, QEMU Developers,
	Leif Lindholm, Eric Auger, qemu-arm, Igor Mammedov

On Tue, 30 Apr 2019 at 22:04, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> >
> > For the Aarch64, there is one machine 'virt', it is primarily meant to
> > run on KVM and execute virtualization workloads, but we need an
> > environment as faithful as possible to physical hardware, for supporting
> > firmware and OS development for pysical Aarch64 machines.
> >
> > This patch introduces new machine type 'sbsa-ref' with main features:
> >  - Based on 'virt' machine type.
> >  - A new memory map.
> >  - CPU type cortex-a57.
> >  - EL2 and EL3 are enabled.
> >  - GIC version 3.
> >  - System bus AHCI controller.
> >  - System bus EHCI controller.
> >  - CDROM and hard disc on AHCI bus.
> >  - E1000E ethernet card on PCIE bus.
> >  - VGA display adaptor on PCIE bus.
> >  - No virtio deivces.
> >  - No fw_cfg device.
> >  - No ACPI table supplied.
> >  - Only minimal device tree nodes.
> >
> > Arm Trusted Firmware and UEFI porting to this are done accordingly, and
> > it should supply ACPI tables to load OS, the minimal device tree nodes
> > supplied from this platform are only to pass the dynamic info reflecting
> > command line input to firmware, not for loading OS.
> >
> > To make the review easier, this task is split into two patches, the
> > fundamental sceleton part and the peripheral devices part, this patch is
> > the first part.
> >
> > Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>
> Hi. This patch looks good to me. I have a couple of very minor
> comments below.
>
> The only other thing I'm not sure about is whether the recent work
> (both in master and in pending patchset) to add support for memory
> hotplug and nvdimms to the 'virt' board is applicable here. I've
> cc'd Igor and Eric to ask their opinion on that question.
>
My opinnion is, if we don't have conclusion before I send out next
iteration, we can just abandon it at this time, currently from my side
I don't see any reqirement for such features, what's more even if in
the future we need it, we can still add it by seperate patch, maybe we
probably have other features to be added in future too.

> > +static const MemMapEntry sbsa_ref_memmap[] = {
> > +    /* 512M boot ROM */
> > +    [SBSA_FLASH] =              {          0, 0x20000000 },
> > +    /* 512M secure memory */
> > +    [SBSA_SECURE_MEM] =         { 0x20000000, 0x20000000 },
> > +    /* Space reserved for CPU peripheral devices */
> > +    [SBSA_CPUPERIPHS] =         { 0x40000000, 0x00040000 },
> > +    [SBSA_GIC_DIST] =           { 0x40060000, 0x00010000 },
> > +    [SBSA_GIC_REDIST] =         { 0x40080000, 0x04000000 },
> > +    [SBSA_UART] =               { 0x60000000, 0x00001000 },
> > +    [SBSA_RTC] =                { 0x60010000, 0x00001000 },
> > +    [SBSA_GPIO] =               { 0x60020000, 0x00001000 },
> > +    [SBSA_SECURE_UART] =        { 0x60030000, 0x00001000 },
> > +    [SBSA_SECURE_UART_MM] =     { 0x60040000, 0x00001000 },
> > +    [SBSA_SMMU] =               { 0x60050000, 0x00020000 },
> > +    /* Space here reserved for more SMMUs */
> > +    [SBSA_AHCI] =               { 0x60100000, 0x00010000 },
> > +    [SBSA_EHCI] =               { 0x60110000, 0x00010000 },
> > +    /* Space here reserved for other devices */
> > +    [SBSA_PCIE_PIO] =           { 0x7fff0000, 0x00010000 },
> > +    /* 32-bit address PCIE MMIO space */
> > +    [SBSA_PCIE_MMIO] =          { 0x80000000, 0x70000000 },
> > +    /* 256M PCIE ECAM space */
> > +    [SBSA_PCIE_ECAM] =          { 0xf0000000, 0x10000000 },
> > +    /* ~1TB PCIE MMIO space (4GB to 1024GB boundary) */
> > +    [SBSA_PCIE_MMIO_HIGH] =     { 0x100000000ULL, 0xFF00000000ULL },
> > +    [SBSA_MEM] =                { 0x10000000000ULL, RAMLIMIT_BYTES },
> > +};
> > +
> > +static const int sbsa_ref_irqmap[] = {
> > +    [SBSA_UART] = 1,
> > +    [SBSA_RTC] = 2,
> > +    [SBSA_PCIE] = 3, /* ... to 6 */
> > +    [SBSA_GPIO] = 7,
> > +    [SBSA_SECURE_UART] = 8,
> > +    [SBSA_SECURE_UART_MM] = 9,
> > +    [SBSA_AHCI] = 10,
> > +    [SBSA_EHCI] = 11,
> > +};
>
> Since we only have one memory map and one irqmap, I think that
> rather than setting up vms->memmap[x] and vms->irqmap[x] and then
> always using those, we should just refer directly to the globals.
> The indirection in virt is originally because I was thinking we
> might want to have more than one layout (and because the code
> derives from the vexpress boards, which really do have two
> different layouts depending on the version), and then it turned
> out to be useful that we could pass the VirtMachineState* to
> the ACPI table generation code and let it get at the addresses
> and IRQ numbers that way. But neither of those applies here, I think.
>
Yes, good.

> > +
> > +static void sbsa_ref_init(MachineState *machine)
> > +{
> > +    SBSAMachineState *vms = SBSA_MACHINE(machine);
>
> This is a very trivial nitpick, but I think we should call
> this variable 'sms', not 'vms', since we changed the name of
> the struct type. (Same applies in some other functions later.)
>
Good catch, this is not 'trivial' I think, they should be changed obviously.

> > +    MachineClass *mc = MACHINE_GET_CLASS(machine);
> > +    MemoryRegion *sysmem = get_system_memory();
> > +    MemoryRegion *secure_sysmem = NULL;
> > +    MemoryRegion *ram = g_new(MemoryRegion, 1);
> > +    bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> > +    const CPUArchIdList *possible_cpus;
> > +    int n, sbsa_max_cpus;
> > +
> > +    if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
> > +        error_report("sbsa-ref: CPU type other than the built-in "
> > +                     "cortex-a57 not supported");
> > +        exit(1);
> > +    }
> > +
> > +    if (kvm_enabled()) {
> > +        error_report("sbsa-ref: KVM is not supported at this machine");
>
> "for this machine".
>
> > +        exit(1);
> > +    }
>
> thanks
> -- PMM


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
  2019-04-30 14:16     ` Peter Maydell
  (?)
@ 2019-05-08 11:30     ` Hongbo Zhang
  2019-05-08 17:48       ` Radoslaw Biernacki
  -1 siblings, 1 reply; 24+ messages in thread
From: Hongbo Zhang @ 2019-05-08 11:30 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Radoslaw Biernacki, Ard Biesheuvel, Markus Armbruster,
	Leif Lindholm, QEMU Developers, qemu-arm

On Tue, 30 Apr 2019 at 22:17, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> >
> > Following the previous patch, this patch adds peripheral devices to the
> > newly introduced SBSA-ref machine.
> >
> > Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> > ---
> >  hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 451 insertions(+)
>
> Some fairly minor comments on this one.
>
> > +static void create_flash(const SBSAMachineState *vms,
> > +                         MemoryRegion *sysmem,
> > +                         MemoryRegion *secure_sysmem)
> > +{
> > +    /*
> > +     * Create one secure and nonsecure flash devices to fill SBSA_FLASH
> > +     * space in the memmap, file passed via -bios goes in the first one.
> > +     */
> > +    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
> > +    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
> > +
> > +    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
> > +                     bios_name, secure_sysmem);
> > +    create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
> > +                     NULL, sysmem);
> > +}
>
> I think Markus might have an opinion on the best way to create
> flash devices on a new board model. Is "just create two flash
> devices the way the virt board does" the right thing?
>
For the firmware part, we are using two flashes, one secure and
another non-secure.

> > +static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    hwaddr base = vms->memmap[SBSA_AHCI].base;
> > +    int irq = vms->irqmap[SBSA_AHCI];
> > +    DeviceState *dev;
> > +    DriveInfo *hd[NUM_SATA_PORTS];
> > +    SysbusAHCIState *sysahci;
> > +    AHCIState *ahci;
> > +    int i;
> > +
> > +    dev = qdev_create(NULL, "sysbus-ahci");
> > +    qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS);
> > +    qdev_init_nofail(dev);
> > +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> > +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> > +
> > +    sysahci = SYSBUS_AHCI(dev);
> > +    ahci = &sysahci->ahci;
> > +    ide_drive_get(hd, ARRAY_SIZE(hd));
> > +    for (i = 0; i < ahci->ports; i++) {
> > +        if (hd[i] == NULL) {
> > +            continue;
> > +        }
> > +        ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
> > +    }
> > +}
> > +
> > +static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    hwaddr base = vms->memmap[SBSA_EHCI].base;
> > +    int irq = vms->irqmap[SBSA_EHCI];
> > +    USBBus *usb_bus;
> > +
> > +    sysbus_create_simple("platform-ehci-usb", base, pic[irq]);
> > +
> > +    usb_bus = usb_bus_find(-1);
> > +    usb_create_simple(usb_bus, "usb-kbd");
> > +    usb_create_simple(usb_bus, "usb-mouse");
>
> I don't think we should automatically create the usb keyboard
> and mouse devices. The user can do it on the command line if they
> want them.
>
OK.

> >  static void sbsa_ref_init(MachineState *machine)
> >  {
> >      SBSAMachineState *vms = SBSA_MACHINE(machine);
> > @@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine)
> >      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> >      const CPUArchIdList *possible_cpus;
> >      int n, sbsa_max_cpus;
> > +    qemu_irq pic[NUM_IRQS];
> >
> >      if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
> >          error_report("sbsa-ref: CPU type other than the built-in "
> > @@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine)
> >                                           machine->ram_size);
> >      memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram);
> >
> > +    create_fdt(vms);
> > +
> > +    create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
> > +
> > +    create_secure_ram(vms, secure_sysmem);
> > +
> > +    create_gic(vms, pic);
> > +
> > +    create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0));
> > +    create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1));
> > +    create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2));
>
> What's the third UART for (ie what is the name intended to mean)?
> Should we have more than one non-secure UART?
>
Yes, this is called " Standalone Management Mode", I will add comment
for it, this is needed by server RAS feature.

> thanks
> -- PMM


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
  2019-04-30 14:16     ` Peter Maydell
  (?)
  (?)
@ 2019-05-08 13:59     ` Markus Armbruster
  2019-06-02  3:16       ` Hongbo Zhang
  -1 siblings, 1 reply; 24+ messages in thread
From: Markus Armbruster @ 2019-05-08 13:59 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Hongbo Zhang, Radoslaw Biernacki, Ard Biesheuvel,
	QEMU Developers, Leif Lindholm, qemu-arm

Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>>
>> Following the previous patch, this patch adds peripheral devices to the
>> newly introduced SBSA-ref machine.
>>
>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>> ---
>>  hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 451 insertions(+)
>
> Some fairly minor comments on this one.
>
>> +static void create_flash(const SBSAMachineState *vms,
>> +                         MemoryRegion *sysmem,
>> +                         MemoryRegion *secure_sysmem)
>> +{
>> +    /*
>> +     * Create one secure and nonsecure flash devices to fill SBSA_FLASH
>> +     * space in the memmap, file passed via -bios goes in the first one.
>> +     */
>> +    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
>> +    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
>> +
>> +    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
>> +                     bios_name, secure_sysmem);
>> +    create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
>> +                     NULL, sysmem);
>> +}
>
> I think Markus might have an opinion on the best way to create
> flash devices on a new board model. Is "just create two flash
> devices the way the virt board does" the right thing?

Short answer: create flash devices the way the ARM virt board does now,
after commit e0561e60f17, merged into master today.  Possibly less
backward compatibility stuff you don't need.  As is, your patch creates
them the way the ARM virt board did before commit e0561e60f17.  Please
consider updating.

Longer answer:

The old way to configure block backends is -drive.

The newer -blockdev is more flexible.  Libvirt is in the process of
transitioning from -drive to -blockdev entirely.  Other users with
similar needs for flexibility may do the same.  We hope to deprecate
-drive eventually.

The traditional way to configure onboard flash is -drive if=pflash.
Works, but we need a way to configure with -blockdev for full
flexibility, and to support libvirt ditching -drive entirely.

I recently improved the i386 PC machine types (commit ebc29e1beab) and
the ARM virt machine types (commit e0561e60f17) to support flash
configuration with -blockdev.

I recommend new boards support flash configuration with -blockdev from
the start.

Questions?


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
  2019-05-08 11:30     ` Hongbo Zhang
@ 2019-05-08 17:48       ` Radoslaw Biernacki
  2019-05-09  8:46         ` Peter Maydell
  0 siblings, 1 reply; 24+ messages in thread
From: Radoslaw Biernacki @ 2019-05-08 17:48 UTC (permalink / raw)
  To: Hongbo Zhang
  Cc: Peter Maydell, Ard Biesheuvel, Markus Armbruster, Leif Lindholm,
	QEMU Developers, qemu-arm

On Wed, 8 May 2019 at 13:30, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:

> On Tue, 30 Apr 2019 at 22:17, Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >
> > On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org>
> wrote:
> > >
> > > Following the previous patch, this patch adds peripheral devices to the
> > > newly introduced SBSA-ref machine.
> > >
> > > Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> > > ---
> > >  hw/arm/sbsa-ref.c | 451
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 451 insertions(+)
> >
> > Some fairly minor comments on this one.
> >
> > > +static void create_flash(const SBSAMachineState *vms,
> > > +                         MemoryRegion *sysmem,
> > > +                         MemoryRegion *secure_sysmem)
> > > +{
> > > +    /*
> > > +     * Create one secure and nonsecure flash devices to fill
> SBSA_FLASH
> > > +     * space in the memmap, file passed via -bios goes in the first
> one.
> > > +     */
> > > +    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
> > > +    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
> > > +
> > > +    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
> > > +                     bios_name, secure_sysmem);
> > > +    create_one_flash("sbsa-ref.flash1", flashbase + flashsize,
> flashsize,
> > > +                     NULL, sysmem);
> > > +}
> >
> > I think Markus might have an opinion on the best way to create
> > flash devices on a new board model. Is "just create two flash
> > devices the way the virt board does" the right thing?
> >
> For the firmware part, we are using two flashes, one secure and
> another non-secure.
>
> > > +static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic)
> > > +{
> > > +    hwaddr base = vms->memmap[SBSA_AHCI].base;
> > > +    int irq = vms->irqmap[SBSA_AHCI];
> > > +    DeviceState *dev;
> > > +    DriveInfo *hd[NUM_SATA_PORTS];
> > > +    SysbusAHCIState *sysahci;
> > > +    AHCIState *ahci;
> > > +    int i;
> > > +
> > > +    dev = qdev_create(NULL, "sysbus-ahci");
> > > +    qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS);
> > > +    qdev_init_nofail(dev);
> > > +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> > > +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> > > +
> > > +    sysahci = SYSBUS_AHCI(dev);
> > > +    ahci = &sysahci->ahci;
> > > +    ide_drive_get(hd, ARRAY_SIZE(hd));
> > > +    for (i = 0; i < ahci->ports; i++) {
> > > +        if (hd[i] == NULL) {
> > > +            continue;
> > > +        }
> > > +        ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
> > > +    }
> > > +}
> > > +
> > > +static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic)
> > > +{
> > > +    hwaddr base = vms->memmap[SBSA_EHCI].base;
> > > +    int irq = vms->irqmap[SBSA_EHCI];
> > > +    USBBus *usb_bus;
> > > +
> > > +    sysbus_create_simple("platform-ehci-usb", base, pic[irq]);
> > > +
> > > +    usb_bus = usb_bus_find(-1);
> > > +    usb_create_simple(usb_bus, "usb-kbd");
> > > +    usb_create_simple(usb_bus, "usb-mouse");
> >
> > I don't think we should automatically create the usb keyboard
> > and mouse devices. The user can do it on the command line if they
> > want them.
> >
> OK.
>

Actually I need to rise an objection to this one.
As we trying to make SBSA machine as close as possible to real machine, we
should have keyboard and mouse.
Those have the same requirement as for VGA. It's just an expected piece of
HW when you for e.g. installing a server.
We also do a lot of FW work so it is expected to have keyboard (and even
mouse) in UEFI.


>
> > >  static void sbsa_ref_init(MachineState *machine)
> > >  {
> > >      SBSAMachineState *vms = SBSA_MACHINE(machine);
> > > @@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine)
> > >      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> > >      const CPUArchIdList *possible_cpus;
> > >      int n, sbsa_max_cpus;
> > > +    qemu_irq pic[NUM_IRQS];
> > >
> > >      if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
> > >          error_report("sbsa-ref: CPU type other than the built-in "
> > > @@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine)
> > >                                           machine->ram_size);
> > >      memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base,
> ram);
> > >
> > > +    create_fdt(vms);
> > > +
> > > +    create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
> > > +
> > > +    create_secure_ram(vms, secure_sysmem);
> > > +
> > > +    create_gic(vms, pic);
> > > +
> > > +    create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0));
> > > +    create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem,
> serial_hd(1));
> > > +    create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem,
> serial_hd(2));
> >
> > What's the third UART for (ie what is the name intended to mean)?
> > Should we have more than one non-secure UART?
> >
> Yes, this is called " Standalone Management Mode", I will add comment
> for it, this is needed by server RAS feature.
>
> > thanks
> > -- PMM
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
  2019-05-08 17:48       ` Radoslaw Biernacki
@ 2019-05-09  8:46         ` Peter Maydell
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2019-05-09  8:46 UTC (permalink / raw)
  To: Radoslaw Biernacki
  Cc: Hongbo Zhang, Ard Biesheuvel, Markus Armbruster, Leif Lindholm,
	QEMU Developers, qemu-arm

On Wed, 8 May 2019 at 18:48, Radoslaw Biernacki
<radoslaw.biernacki@linaro.org> wrote:
>
>
>
> On Wed, 8 May 2019 at 13:30, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>>
>> On Tue, 30 Apr 2019 at 22:17, Peter Maydell <peter.maydell@linaro.org> wrote:
>> > I don't think we should automatically create the usb keyboard
>> > and mouse devices. The user can do it on the command line if they
>> > want them.
>> >
>> OK.
>
>
> Actually I need to rise an objection to this one.
> As we trying to make SBSA machine as close as possible to real machine, we should have keyboard and mouse.
> Those have the same requirement as for VGA. It's just an expected piece of HW when you for e.g. installing a server.
> We also do a lot of FW work so it is expected to have keyboard (and even mouse) in UEFI.

Real hardware doesn't have the keyboard and mouse built in --
when you unpack the machine from the box you have to plug in
the keyboard and mouse yourself (and often you have to
buy the keyboard and mouse and monitor and maybe the
PCI video card separately).

But more seriously, the philosophy of the QEMU command line
is not "do what the user probably wants automatically". It
is "provide the user with full manual control of everything,
using a complicated but orthogonal set of options". We expect
that if users want a more "friendly" interface to setting
up VMs then they will use a "management layer" on top of
QEMU (such as libvirt).

thanks
-- PMM


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 1/2] hw/arm: Add arm SBSA reference machine, skeleton part
  2019-05-08 11:22     ` Hongbo Zhang
@ 2019-05-09 14:27       ` Igor Mammedov
  2019-05-09 14:40         ` Peter Maydell
  0 siblings, 1 reply; 24+ messages in thread
From: Igor Mammedov @ 2019-05-09 14:27 UTC (permalink / raw)
  To: Hongbo Zhang
  Cc: Peter Maydell, Radoslaw Biernacki, Ard Biesheuvel,
	QEMU Developers, Leif Lindholm, Eric Auger, qemu-arm

On Wed, 8 May 2019 19:22:53 +0800
Hongbo Zhang <hongbo.zhang@linaro.org> wrote:

> On Tue, 30 Apr 2019 at 22:04, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> > >
> > > For the Aarch64, there is one machine 'virt', it is primarily meant to
> > > run on KVM and execute virtualization workloads, but we need an
> > > environment as faithful as possible to physical hardware, for supporting
> > > firmware and OS development for pysical Aarch64 machines.
> > >
> > > This patch introduces new machine type 'sbsa-ref' with main features:
> > >  - Based on 'virt' machine type.
> > >  - A new memory map.
> > >  - CPU type cortex-a57.
> > >  - EL2 and EL3 are enabled.
> > >  - GIC version 3.
> > >  - System bus AHCI controller.
> > >  - System bus EHCI controller.
> > >  - CDROM and hard disc on AHCI bus.
> > >  - E1000E ethernet card on PCIE bus.
> > >  - VGA display adaptor on PCIE bus.
> > >  - No virtio deivces.
> > >  - No fw_cfg device.
> > >  - No ACPI table supplied.
> > >  - Only minimal device tree nodes.
> > >
> > > Arm Trusted Firmware and UEFI porting to this are done accordingly, and
> > > it should supply ACPI tables to load OS, the minimal device tree nodes
> > > supplied from this platform are only to pass the dynamic info reflecting
> > > command line input to firmware, not for loading OS.
> > >
> > > To make the review easier, this task is split into two patches, the
> > > fundamental sceleton part and the peripheral devices part, this patch is
> > > the first part.
> > >
> > > Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> >
> > Hi. This patch looks good to me. I have a couple of very minor
> > comments below.
> >
> > The only other thing I'm not sure about is whether the recent work
> > (both in master and in pending patchset) to add support for memory
> > hotplug and nvdimms to the 'virt' board is applicable here. I've
> > cc'd Igor and Eric to ask their opinion on that question.
> >
> My opinnion is, if we don't have conclusion before I send out next
> iteration, we can just abandon it at this time, currently from my side
> I don't see any reqirement for such features, what's more even if in
> the future we need it, we can still add it by seperate patch, maybe we
> probably have other features to be added in future too.

It should be possible to add nvdimm and memory hotplug later.
(I don't see big issues here, as long as it would be possible
to carve out continuous range in address space for it.

Considering ABI to guest, one could reuse QEMU's notification/enumeration
code for that or implement their own.

PS:
However there will be the same issues that we've had with Seabios when
ACPI tables were generated there (i.e. cease-less invention of interfaces
to to pass information from QEMU to firmware). But if I recall correctly,
it seems that it's intentional design decision for this board,
where user supplies/builds in firmware a board description (acpi/dt/...)
manually.

 
> > > +static const MemMapEntry sbsa_ref_memmap[] = {
> > > +    /* 512M boot ROM */
> > > +    [SBSA_FLASH] =              {          0, 0x20000000 },
> > > +    /* 512M secure memory */
> > > +    [SBSA_SECURE_MEM] =         { 0x20000000, 0x20000000 },
> > > +    /* Space reserved for CPU peripheral devices */
> > > +    [SBSA_CPUPERIPHS] =         { 0x40000000, 0x00040000 },
> > > +    [SBSA_GIC_DIST] =           { 0x40060000, 0x00010000 },
> > > +    [SBSA_GIC_REDIST] =         { 0x40080000, 0x04000000 },
> > > +    [SBSA_UART] =               { 0x60000000, 0x00001000 },
> > > +    [SBSA_RTC] =                { 0x60010000, 0x00001000 },
> > > +    [SBSA_GPIO] =               { 0x60020000, 0x00001000 },
> > > +    [SBSA_SECURE_UART] =        { 0x60030000, 0x00001000 },
> > > +    [SBSA_SECURE_UART_MM] =     { 0x60040000, 0x00001000 },
> > > +    [SBSA_SMMU] =               { 0x60050000, 0x00020000 },
> > > +    /* Space here reserved for more SMMUs */
> > > +    [SBSA_AHCI] =               { 0x60100000, 0x00010000 },
> > > +    [SBSA_EHCI] =               { 0x60110000, 0x00010000 },
> > > +    /* Space here reserved for other devices */
> > > +    [SBSA_PCIE_PIO] =           { 0x7fff0000, 0x00010000 },
> > > +    /* 32-bit address PCIE MMIO space */
> > > +    [SBSA_PCIE_MMIO] =          { 0x80000000, 0x70000000 },
> > > +    /* 256M PCIE ECAM space */
> > > +    [SBSA_PCIE_ECAM] =          { 0xf0000000, 0x10000000 },
> > > +    /* ~1TB PCIE MMIO space (4GB to 1024GB boundary) */
> > > +    [SBSA_PCIE_MMIO_HIGH] =     { 0x100000000ULL, 0xFF00000000ULL },
> > > +    [SBSA_MEM] =                { 0x10000000000ULL, RAMLIMIT_BYTES },
> > > +};
> > > +
> > > +static const int sbsa_ref_irqmap[] = {
> > > +    [SBSA_UART] = 1,
> > > +    [SBSA_RTC] = 2,
> > > +    [SBSA_PCIE] = 3, /* ... to 6 */
> > > +    [SBSA_GPIO] = 7,
> > > +    [SBSA_SECURE_UART] = 8,
> > > +    [SBSA_SECURE_UART_MM] = 9,
> > > +    [SBSA_AHCI] = 10,
> > > +    [SBSA_EHCI] = 11,
> > > +};
> >
> > Since we only have one memory map and one irqmap, I think that
> > rather than setting up vms->memmap[x] and vms->irqmap[x] and then
> > always using those, we should just refer directly to the globals.
> > The indirection in virt is originally because I was thinking we
> > might want to have more than one layout (and because the code
> > derives from the vexpress boards, which really do have two
> > different layouts depending on the version), and then it turned
> > out to be useful that we could pass the VirtMachineState* to
> > the ACPI table generation code and let it get at the addresses
> > and IRQ numbers that way. But neither of those applies here, I think.
> >
> Yes, good.
> 
> > > +
> > > +static void sbsa_ref_init(MachineState *machine)
> > > +{
> > > +    SBSAMachineState *vms = SBSA_MACHINE(machine);
> >
> > This is a very trivial nitpick, but I think we should call
> > this variable 'sms', not 'vms', since we changed the name of
> > the struct type. (Same applies in some other functions later.)
> >
> Good catch, this is not 'trivial' I think, they should be changed obviously.
> 
> > > +    MachineClass *mc = MACHINE_GET_CLASS(machine);
> > > +    MemoryRegion *sysmem = get_system_memory();
> > > +    MemoryRegion *secure_sysmem = NULL;
> > > +    MemoryRegion *ram = g_new(MemoryRegion, 1);
> > > +    bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> > > +    const CPUArchIdList *possible_cpus;
> > > +    int n, sbsa_max_cpus;
> > > +
> > > +    if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
> > > +        error_report("sbsa-ref: CPU type other than the built-in "
> > > +                     "cortex-a57 not supported");
> > > +        exit(1);
> > > +    }
> > > +
> > > +    if (kvm_enabled()) {
> > > +        error_report("sbsa-ref: KVM is not supported at this machine");
> >
> > "for this machine".
> >
> > > +        exit(1);
> > > +    }
> >
> > thanks
> > -- PMM



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 1/2] hw/arm: Add arm SBSA reference machine, skeleton part
  2019-05-09 14:27       ` Igor Mammedov
@ 2019-05-09 14:40         ` Peter Maydell
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2019-05-09 14:40 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Hongbo Zhang, Radoslaw Biernacki, Ard Biesheuvel,
	QEMU Developers, Leif Lindholm, Eric Auger, qemu-arm

On Thu, 9 May 2019 at 15:27, Igor Mammedov <imammedo@redhat.com> wrote:

> It should be possible to add nvdimm and memory hotplug later.
> (I don't see big issues here, as long as it would be possible
> to carve out continuous range in address space for it.
>
> Considering ABI to guest, one could reuse QEMU's notification/enumeration
> code for that or implement their own.
>
> PS:
> However there will be the same issues that we've had with Seabios when
> ACPI tables were generated there (i.e. cease-less invention of interfaces
> to to pass information from QEMU to firmware). But if I recall correctly,
> it seems that it's intentional design decision for this board,
> where user supplies/builds in firmware a board description (acpi/dt/...)
> manually.

This board is supposed to be very close to real hardware,
where the firmware knows what it's running on and very
little is configurable from the command line, yes.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
  2019-05-08 13:59     ` Markus Armbruster
@ 2019-06-02  3:16       ` Hongbo Zhang
  0 siblings, 0 replies; 24+ messages in thread
From: Hongbo Zhang @ 2019-06-02  3:16 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Peter Maydell, Radoslaw Biernacki, Ard Biesheuvel,
	QEMU Developers, Leif Lindholm, qemu-arm

On Wed, 8 May 2019 at 21:59, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> >>
> >> Following the previous patch, this patch adds peripheral devices to the
> >> newly introduced SBSA-ref machine.
> >>
> >> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> >> ---
> >>  hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 451 insertions(+)
> >
> > Some fairly minor comments on this one.
> >
> >> +static void create_flash(const SBSAMachineState *vms,
> >> +                         MemoryRegion *sysmem,
> >> +                         MemoryRegion *secure_sysmem)
> >> +{
> >> +    /*
> >> +     * Create one secure and nonsecure flash devices to fill SBSA_FLASH
> >> +     * space in the memmap, file passed via -bios goes in the first one.
> >> +     */
> >> +    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
> >> +    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
> >> +
> >> +    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
> >> +                     bios_name, secure_sysmem);
> >> +    create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
> >> +                     NULL, sysmem);
> >> +}
> >
> > I think Markus might have an opinion on the best way to create
> > flash devices on a new board model. Is "just create two flash
> > devices the way the virt board does" the right thing?
>
> Short answer: create flash devices the way the ARM virt board does now,
> after commit e0561e60f17, merged into master today.  Possibly less
> backward compatibility stuff you don't need.  As is, your patch creates
> them the way the ARM virt board did before commit e0561e60f17.  Please
> consider updating.
>
> Longer answer:
>
> The old way to configure block backends is -drive.
>
> The newer -blockdev is more flexible.  Libvirt is in the process of
> transitioning from -drive to -blockdev entirely.  Other users with
> similar needs for flexibility may do the same.  We hope to deprecate
> -drive eventually.
>
> The traditional way to configure onboard flash is -drive if=pflash.
> Works, but we need a way to configure with -blockdev for full
> flexibility, and to support libvirt ditching -drive entirely.
>
> I recently improved the i386 PC machine types (commit ebc29e1beab) and
> the ARM virt machine types (commit e0561e60f17) to support flash
> configuration with -blockdev.
>
> I recommend new boards support flash configuration with -blockdev from
> the start.
>
> Questions?

Sorry for the late response.
Thank you for the detailed explanation, and I'll follow the new
pattern in my next version of patch which will be sent out in a few
days.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
  2019-04-18  4:04   ` Hongbo Zhang
  (?)
  (?)
@ 2019-06-03 10:54   ` Philippe Mathieu-Daudé
  2019-06-16 11:41     ` Hongbo Zhang
  2019-06-17  2:44     ` Hongbo Zhang
  -1 siblings, 2 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-03 10:54 UTC (permalink / raw)
  To: Hongbo Zhang, peter.maydell, qemu-devel, qemu-arm
  Cc: radoslaw.biernacki, Laszlo Ersek, leif.lindholm, ard.biesheuvel

Hi Hongbo, Ard.

On 4/18/19 6:04 AM, Hongbo Zhang wrote:
> Following the previous patch, this patch adds peripheral devices to the
> newly introduced SBSA-ref machine.
> 
> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> ---
>  hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 451 insertions(+)
> 
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index 652ec13..3fb0027 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -21,6 +21,7 @@
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "qemu/units.h"
> +#include "sysemu/device_tree.h"
>  #include "sysemu/numa.h"
>  #include "sysemu/sysemu.h"
>  #include "exec/address-spaces.h"
> @@ -28,11 +29,28 @@
>  #include "kvm_arm.h"
>  #include "hw/arm/arm.h"
>  #include "hw/boards.h"
> +#include "hw/ide/internal.h"
> +#include "hw/ide/ahci_internal.h"
>  #include "hw/intc/arm_gicv3_common.h"
> +#include "hw/loader.h"
> +#include "hw/pci-host/gpex.h"
> +#include "hw/usb.h"
> +#include "net/net.h"
>  
>  #define RAMLIMIT_GB 8192
>  #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
>  
> +#define NUM_IRQS        256
> +#define NUM_SMMU_IRQS   4
> +#define NUM_SATA_PORTS  6
> +
> +#define VIRTUAL_PMU_IRQ        7
> +#define ARCH_GIC_MAINT_IRQ     9
> +#define ARCH_TIMER_VIRT_IRQ    11
> +#define ARCH_TIMER_S_EL1_IRQ   13
> +#define ARCH_TIMER_NS_EL1_IRQ  14
> +#define ARCH_TIMER_NS_EL2_IRQ  10
> +
>  enum {
>      SBSA_FLASH,
>      SBSA_MEM,
> @@ -115,6 +133,415 @@ static const int sbsa_ref_irqmap[] = {
>      [SBSA_EHCI] = 11,
>  };
>  
> +/*
> + * Firmware on this machine only uses ACPI table to load OS, these limited
> + * device tree nodes are just to let firmware know the info which varies from
> + * command line parameters, so it is not necessary to be fully compatible
> + * with the kernel CPU and NUMA binding rules.
> + */
> +static void create_fdt(SBSAMachineState *vms)
> +{
> +    void *fdt = create_device_tree(&vms->fdt_size);
> +    const MachineState *ms = MACHINE(vms);
> +    int cpu;
> +
> +    if (!fdt) {
> +        error_report("create_device_tree() failed");
> +        exit(1);
> +    }
> +
> +    vms->fdt = fdt;
> +
> +    qemu_fdt_setprop_string(fdt, "/", "compatible", "linux,sbsa-ref");
> +    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x2);
> +    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2);
> +
> +    if (have_numa_distance) {
> +        int size = nb_numa_nodes * nb_numa_nodes * 3 * sizeof(uint32_t);
> +        uint32_t *matrix = g_malloc0(size);
> +        int idx, i, j;
> +
> +        for (i = 0; i < nb_numa_nodes; i++) {
> +            for (j = 0; j < nb_numa_nodes; j++) {
> +                idx = (i * nb_numa_nodes + j) * 3;
> +                matrix[idx + 0] = cpu_to_be32(i);
> +                matrix[idx + 1] = cpu_to_be32(j);
> +                matrix[idx + 2] = cpu_to_be32(numa_info[i].distance[j]);
> +            }
> +        }
> +
> +        qemu_fdt_add_subnode(fdt, "/distance-map");
> +        qemu_fdt_setprop(fdt, "/distance-map", "distance-matrix",
> +                         matrix, size);
> +        g_free(matrix);
> +    }
> +
> +    qemu_fdt_add_subnode(vms->fdt, "/cpus");
> +
> +    for (cpu = vms->smp_cpus - 1; cpu >= 0; cpu--) {
> +        char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
> +        ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));
> +        CPUState *cs = CPU(armcpu);
> +
> +        qemu_fdt_add_subnode(vms->fdt, nodename);
> +
> +        if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) {
> +            qemu_fdt_setprop_cell(vms->fdt, nodename, "numa-node-id",
> +                ms->possible_cpus->cpus[cs->cpu_index].props.node_id);
> +        }
> +
> +        g_free(nodename);
> +    }
> +}
> +
> +static void create_one_flash(const char *name, hwaddr flashbase,
> +                             hwaddr flashsize, const char *file,
> +                             MemoryRegion *sysmem)
> +{
> +    /*
> +     * Create and map a single flash device. We use the same
> +     * parameters as the flash devices on the Versatile Express board.
> +     */
> +    DriveInfo *dinfo = drive_get_next(IF_PFLASH);
> +    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");

Please use TYPE_PFLASH_CFI01 instead of "cfi.pflash01".

I wanted to ask "does it has to be CFI01?" because this device model is
in bad shape, but I guess I answered myself looking at the EDK2 platform
code:

- P30_CFI_ADDR_VENDOR_ID is not used
- NorFlashDxe::NorFlashReadCfiData() is not implemented
- All commands in NorFlashDxe uses:
    SEND_NOR_COMMAND(..., P30_CMD_...)
  which are specific to the Intel P30 Nor flash family (CFI01).

> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    const uint64_t sectorlength = 256 * 1024;
> +
> +    if (dinfo) {
> +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
> +                            &error_abort);
> +    }
> +
> +    qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
> +    qdev_prop_set_uint64(dev, "sector-length", sectorlength);
> +    qdev_prop_set_uint8(dev, "width", 4);
> +    qdev_prop_set_uint8(dev, "device-width", 2);
> +    qdev_prop_set_bit(dev, "big-endian", false);
> +    qdev_prop_set_uint16(dev, "id0", 0x89);
> +    qdev_prop_set_uint16(dev, "id1", 0x18);
> +    qdev_prop_set_uint16(dev, "id2", 0x00);
> +    qdev_prop_set_uint16(dev, "id3", 0x00);
> +    qdev_prop_set_string(dev, "name", name);
> +    qdev_init_nofail(dev);
> +
> +    memory_region_add_subregion(sysmem, flashbase,
> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
> +
> +    if (file) {
> +        char *fn;
> +        int image_size;
> +
> +        if (drive_get(IF_PFLASH, 0, 0)) {
> +            error_report("The contents of the first flash device may be "
> +                         "specified with -bios or with -drive if=pflash... "
> +                         "but you cannot use both options at once");
> +            exit(1);
> +        }
> +        fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, file);
> +        if (!fn) {
> +            error_report("Could not find ROM image '%s'", file);
> +            exit(1);
> +        }
> +        image_size = load_image_mr(fn, sysbus_mmio_get_region(sbd, 0));
> +        g_free(fn);
> +        if (image_size < 0) {
> +            error_report("Could not load ROM image '%s'", file);
> +            exit(1);
> +        }
> +    }
> +}
> +
> +static void create_flash(const SBSAMachineState *vms,
> +                         MemoryRegion *sysmem,
> +                         MemoryRegion *secure_sysmem)
> +{
> +    /*
> +     * Create one secure and nonsecure flash devices to fill SBSA_FLASH
> +     * space in the memmap, file passed via -bios goes in the first one.
> +     */
> +    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
> +    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
> +
> +    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
> +                     bios_name, secure_sysmem);
> +    create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
> +                     NULL, sysmem);

static const MemMapEntry base_memmap[] = {
    /* Space up to 0x8000000 is reserved for a boot ROM */
    [VIRT_FLASH] =              {          0, 0x08000000 },

So you are creating 2 identical flashes of 128MiB/2 = 64 MiB each which
are the biggest flash you can have:

"The P30 family provides density upgrades from 64-Mbit through
512-Mbit." On Intel, the 512-Mib case is particular in that it is built
of 2x 256-Mib on the same die, with a virtual chip enable. It is simpler
to use a Micron or Numonyx model.

I plan to use a whitelist of supported (and tested...) models, the one
you use seems the Micron JS28F512P30EF ('E' for 'Symetrically Blocked',
since the current model doesn't support bottom/top blocks layout), or in
short: 28F512P30E.
Ard, is that OK?

Checking EDK2 git history, the driver is part of ArmPlatformPkg,
imported in commit 1d5d0ae92d9541, based on 'Versatile Express'.

On the Versatile Express and the RealView Emulation Baseboard user
guides, I only find reference of "64MB of NOR flash" with no specific model.

Peter, do you have physical access to tell me what flashes are used on
real hardware? I googled for Linux console boot log but the kernel
doesn't seem to care about detecting/mapping the flash.

QEMU added the flash to the Versatile board in commit 964c695a54ceda3a,
with the following description:

    - add support for the 64MB NOR CFI01 flash available at
    0x34000000 on the versatilepb board

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0225d/BBAJIHEC.html

However on this link I only see "SSMC Chip Select 1, normally NOR flash
(During boot remapping, this can be NOR flash, Disk-on-Chip, or static
expansion memory)". Again, nothing specific (which makes sense, why
restrict the users to a particuliar family, as long as the pinout matches).

The Manufacturer/Device ID used in QEMU (0x0089, 0x0018) correspond to
the Micron 28F128J3D (128-Mbit, 128 symmetrical blocks of 128-KiB).
Neither the flash size (64 vs 16) nor the block size (256 vs 128) match.

The safer fix here is to find a CFI01 flash of 256 sectors of 256-KiB
and update the Manufacturer/Device IDs in QEMU. Luckily this matches the
28F512P30E cited previously :)

Regards,

Phil.

> +}
> +
> +static void create_secure_ram(SBSAMachineState *vms,
> +                              MemoryRegion *secure_sysmem)
> +{
> +    MemoryRegion *secram = g_new(MemoryRegion, 1);
> +    hwaddr base = vms->memmap[SBSA_SECURE_MEM].base;
> +    hwaddr size = vms->memmap[SBSA_SECURE_MEM].size;
> +
> +    memory_region_init_ram(secram, NULL, "sbsa-ref.secure-ram", size,
> +                           &error_fatal);
> +    memory_region_add_subregion(secure_sysmem, base, secram);
> +}
> +
> +static void create_gic(SBSAMachineState *vms, qemu_irq *pic)
> +{
> +    DeviceState *gicdev;
> +    SysBusDevice *gicbusdev;
> +    const char *gictype;
> +    uint32_t redist0_capacity, redist0_count;
> +    int i;
> +
> +    gictype = gicv3_class_name();
> +
> +    gicdev = qdev_create(NULL, gictype);
> +    qdev_prop_set_uint32(gicdev, "revision", 3);
> +    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).
> +     */
> +    qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
> +    qdev_prop_set_bit(gicdev, "has-security-extensions", true);
> +
> +    redist0_capacity =
> +                vms->memmap[SBSA_GIC_REDIST].size / GICV3_REDIST_SIZE;
> +    redist0_count = MIN(smp_cpus, redist0_capacity);
> +
> +    qdev_prop_set_uint32(gicdev, "len-redist-region-count", 1);
> +    qdev_prop_set_uint32(gicdev, "redist-region-count[0]", redist0_count);
> +
> +    qdev_init_nofail(gicdev);
> +    gicbusdev = SYS_BUS_DEVICE(gicdev);
> +    sysbus_mmio_map(gicbusdev, 0, vms->memmap[SBSA_GIC_DIST].base);
> +    sysbus_mmio_map(gicbusdev, 1, vms->memmap[SBSA_GIC_REDIST].base);
> +
> +    /*
> +     * Wire the outputs from each CPU's generic timer and the GICv3
> +     * maintenance interrupt signal to the appropriate GIC PPI inputs,
> +     * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
> +     */
> +    for (i = 0; i < smp_cpus; i++) {
> +        DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
> +        int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
> +        int irq;
> +        /*
> +         * Mapping from the output timer irq lines from the CPU to the
> +         * GIC PPI inputs used for this board.
> +         */
> +        const int timer_irq[] = {
> +            [GTIMER_PHYS] = ARCH_TIMER_NS_EL1_IRQ,
> +            [GTIMER_VIRT] = ARCH_TIMER_VIRT_IRQ,
> +            [GTIMER_HYP]  = ARCH_TIMER_NS_EL2_IRQ,
> +            [GTIMER_SEC]  = ARCH_TIMER_S_EL1_IRQ,
> +        };
> +
> +        for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
> +            qdev_connect_gpio_out(cpudev, irq,
> +                                  qdev_get_gpio_in(gicdev,
> +                                                   ppibase + timer_irq[irq]));
> +        }
> +
> +        qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0,
> +                                    qdev_get_gpio_in(gicdev, ppibase
> +                                                     + ARCH_GIC_MAINT_IRQ));
> +        qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0,
> +                                    qdev_get_gpio_in(gicdev, ppibase
> +                                                     + VIRTUAL_PMU_IRQ));
> +
> +        sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
> +        sysbus_connect_irq(gicbusdev, i + smp_cpus,
> +                           qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> +        sysbus_connect_irq(gicbusdev, i + 2 * smp_cpus,
> +                           qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
> +        sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
> +                           qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
> +    }
> +
> +    for (i = 0; i < NUM_IRQS; i++) {
> +        pic[i] = qdev_get_gpio_in(gicdev, i);
> +    }
> +}
> +
> +static void create_uart(const SBSAMachineState *vms, qemu_irq *pic, int uart,
> +                        MemoryRegion *mem, Chardev *chr)
> +{
> +    hwaddr base = vms->memmap[uart].base;
> +    int irq = vms->irqmap[uart];
> +    DeviceState *dev = qdev_create(NULL, "pl011");
> +    SysBusDevice *s = SYS_BUS_DEVICE(dev);
> +
> +    qdev_prop_set_chr(dev, "chardev", chr);
> +    qdev_init_nofail(dev);
> +    memory_region_add_subregion(mem, base,
> +                                sysbus_mmio_get_region(s, 0));
> +    sysbus_connect_irq(s, 0, pic[irq]);
> +}
> +
> +static void create_rtc(const SBSAMachineState *vms, qemu_irq *pic)
> +{
> +    hwaddr base = vms->memmap[SBSA_RTC].base;
> +    int irq = vms->irqmap[SBSA_RTC];
> +
> +    sysbus_create_simple("pl031", base, pic[irq]);
> +}
> +
> +static DeviceState *gpio_key_dev;
> +static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
> +{
> +    /* use gpio Pin 3 for power button event */
> +    qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
> +}
> +
> +static Notifier sbsa_ref_powerdown_notifier = {
> +    .notify = sbsa_ref_powerdown_req
> +};
> +
> +static void create_gpio(const SBSAMachineState *vms, qemu_irq *pic)
> +{
> +    DeviceState *pl061_dev;
> +    hwaddr base = vms->memmap[SBSA_GPIO].base;
> +    int irq = vms->irqmap[SBSA_GPIO];
> +
> +    pl061_dev = sysbus_create_simple("pl061", base, pic[irq]);
> +
> +    gpio_key_dev = sysbus_create_simple("gpio-key", -1,
> +                                        qdev_get_gpio_in(pl061_dev, 3));
> +
> +    /* connect powerdown request */
> +    qemu_register_powerdown_notifier(&sbsa_ref_powerdown_notifier);
> +}
> +
> +static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic)
> +{
> +    hwaddr base = vms->memmap[SBSA_AHCI].base;
> +    int irq = vms->irqmap[SBSA_AHCI];
> +    DeviceState *dev;
> +    DriveInfo *hd[NUM_SATA_PORTS];
> +    SysbusAHCIState *sysahci;
> +    AHCIState *ahci;
> +    int i;
> +
> +    dev = qdev_create(NULL, "sysbus-ahci");
> +    qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS);
> +    qdev_init_nofail(dev);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> +
> +    sysahci = SYSBUS_AHCI(dev);
> +    ahci = &sysahci->ahci;
> +    ide_drive_get(hd, ARRAY_SIZE(hd));
> +    for (i = 0; i < ahci->ports; i++) {
> +        if (hd[i] == NULL) {
> +            continue;
> +        }
> +        ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
> +    }
> +}
> +
> +static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic)
> +{
> +    hwaddr base = vms->memmap[SBSA_EHCI].base;
> +    int irq = vms->irqmap[SBSA_EHCI];
> +    USBBus *usb_bus;
> +
> +    sysbus_create_simple("platform-ehci-usb", base, pic[irq]);
> +
> +    usb_bus = usb_bus_find(-1);
> +    usb_create_simple(usb_bus, "usb-kbd");
> +    usb_create_simple(usb_bus, "usb-mouse");
> +}
> +
> +static void create_smmu(const SBSAMachineState *vms, qemu_irq *pic,
> +                        PCIBus *bus)
> +{
> +    hwaddr base = vms->memmap[SBSA_SMMU].base;
> +    int irq =  vms->irqmap[SBSA_SMMU];
> +    DeviceState *dev;
> +    int i;
> +
> +    dev = qdev_create(NULL, "arm-smmuv3");
> +
> +    object_property_set_link(OBJECT(dev), OBJECT(bus), "primary-bus",
> +                             &error_abort);
> +    qdev_init_nofail(dev);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> +    for (i = 0; i < NUM_SMMU_IRQS; i++) {
> +        sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
> +    }
> +}
> +
> +static void create_pcie(SBSAMachineState *vms, qemu_irq *pic)
> +{
> +    hwaddr base_ecam = vms->memmap[SBSA_PCIE_ECAM].base;
> +    hwaddr size_ecam = vms->memmap[SBSA_PCIE_ECAM].size;
> +    hwaddr base_mmio = vms->memmap[SBSA_PCIE_MMIO].base;
> +    hwaddr size_mmio = vms->memmap[SBSA_PCIE_MMIO].size;
> +    hwaddr base_mmio_high = vms->memmap[SBSA_PCIE_MMIO_HIGH].base;
> +    hwaddr size_mmio_high = vms->memmap[SBSA_PCIE_MMIO_HIGH].size;
> +    hwaddr base_pio = vms->memmap[SBSA_PCIE_PIO].base;
> +    int irq = vms->irqmap[SBSA_PCIE];
> +    MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
> +    MemoryRegion *ecam_alias, *ecam_reg;
> +    DeviceState *dev;
> +    PCIHostState *pci;
> +    int i;
> +
> +    dev = qdev_create(NULL, TYPE_GPEX_HOST);
> +    qdev_init_nofail(dev);
> +
> +    /* Map ECAM space */
> +    ecam_alias = g_new0(MemoryRegion, 1);
> +    ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> +    memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
> +                             ecam_reg, 0, size_ecam);
> +    memory_region_add_subregion(get_system_memory(), base_ecam, ecam_alias);
> +
> +    /* Map the MMIO space */
> +    mmio_alias = g_new0(MemoryRegion, 1);
> +    mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
> +    memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
> +                             mmio_reg, base_mmio, size_mmio);
> +    memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
> +
> +    /* Map the MMIO_HIGH space */
> +    mmio_alias_high = g_new0(MemoryRegion, 1);
> +    memory_region_init_alias(mmio_alias_high, OBJECT(dev), "pcie-mmio-high",
> +                             mmio_reg, base_mmio_high, size_mmio_high);
> +    memory_region_add_subregion(get_system_memory(), base_mmio_high,
> +                                mmio_alias_high);
> +
> +    /* Map IO port space */
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio);
> +
> +    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> +        sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
> +        gpex_set_irq_num(GPEX_HOST(dev), i, irq + i);
> +    }
> +
> +    pci = PCI_HOST_BRIDGE(dev);
> +    if (pci->bus) {
> +        for (i = 0; i < nb_nics; i++) {
> +            NICInfo *nd = &nd_table[i];
> +
> +            if (!nd->model) {
> +                nd->model = g_strdup("e1000e");
> +            }
> +
> +            pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);
> +        }
> +    }
> +
> +    pci_create_simple(pci->bus, -1, "VGA");
> +
> +    create_smmu(vms, pic, pci->bus);
> +}
> +
> +static void *sbsa_ref_dtb(const struct arm_boot_info *binfo, int *fdt_size)
> +{
> +    const SBSAMachineState *board = container_of(binfo, SBSAMachineState,
> +                                                 bootinfo);
> +
> +    *fdt_size = board->fdt_size;
> +    return board->fdt;
> +}
> +
>  static void sbsa_ref_init(MachineState *machine)
>  {
>      SBSAMachineState *vms = SBSA_MACHINE(machine);
> @@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine)
>      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
>      const CPUArchIdList *possible_cpus;
>      int n, sbsa_max_cpus;
> +    qemu_irq pic[NUM_IRQS];
>  
>      if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
>          error_report("sbsa-ref: CPU type other than the built-in "
> @@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine)
>                                           machine->ram_size);
>      memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram);
>  
> +    create_fdt(vms);
> +
> +    create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
> +
> +    create_secure_ram(vms, secure_sysmem);
> +
> +    create_gic(vms, pic);
> +
> +    create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0));
> +    create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1));
> +    create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2));
> +
> +    create_rtc(vms, pic);
> +
> +    create_gpio(vms, pic);
> +
> +    create_ahci(vms, pic);
> +
> +    create_ehci(vms, pic);
> +
> +    create_pcie(vms, pic);
> +
>      vms->bootinfo.ram_size = machine->ram_size;
>      vms->bootinfo.kernel_filename = machine->kernel_filename;
>      vms->bootinfo.nb_cpus = smp_cpus;
>      vms->bootinfo.board_id = -1;
>      vms->bootinfo.loader_start = vms->memmap[SBSA_MEM].base;
> +    vms->bootinfo.get_dtb = sbsa_ref_dtb;
>      vms->bootinfo.firmware_loaded = firmware_loaded;
>      arm_load_kernel(ARM_CPU(first_cpu), &vms->bootinfo);
>  }
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
  2019-06-03 10:54   ` Philippe Mathieu-Daudé
@ 2019-06-16 11:41     ` Hongbo Zhang
  2019-06-17 11:08       ` Philippe Mathieu-Daudé
  2019-06-17  2:44     ` Hongbo Zhang
  1 sibling, 1 reply; 24+ messages in thread
From: Hongbo Zhang @ 2019-06-16 11:41 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Radoslaw Biernacki, Ard Biesheuvel,
	QEMU Developers, Leif Lindholm, qemu-arm, Laszlo Ersek

On Mon, 3 Jun 2019 at 18:54, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Hi Hongbo, Ard.
>
> On 4/18/19 6:04 AM, Hongbo Zhang wrote:
> > Following the previous patch, this patch adds peripheral devices to the
> > newly introduced SBSA-ref machine.
> >
> > Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> > ---
> >  hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 451 insertions(+)
> >
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index 652ec13..3fb0027 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -21,6 +21,7 @@
> >  #include "qapi/error.h"
> >  #include "qemu/error-report.h"
> >  #include "qemu/units.h"
> > +#include "sysemu/device_tree.h"
> >  #include "sysemu/numa.h"
> >  #include "sysemu/sysemu.h"
> >  #include "exec/address-spaces.h"
> > @@ -28,11 +29,28 @@
> >  #include "kvm_arm.h"
> >  #include "hw/arm/arm.h"
> >  #include "hw/boards.h"
> > +#include "hw/ide/internal.h"
> > +#include "hw/ide/ahci_internal.h"
> >  #include "hw/intc/arm_gicv3_common.h"
> > +#include "hw/loader.h"
> > +#include "hw/pci-host/gpex.h"
> > +#include "hw/usb.h"
> > +#include "net/net.h"
> >
> >  #define RAMLIMIT_GB 8192
> >  #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
> >
> > +#define NUM_IRQS        256
> > +#define NUM_SMMU_IRQS   4
> > +#define NUM_SATA_PORTS  6
> > +
> > +#define VIRTUAL_PMU_IRQ        7
> > +#define ARCH_GIC_MAINT_IRQ     9
> > +#define ARCH_TIMER_VIRT_IRQ    11
> > +#define ARCH_TIMER_S_EL1_IRQ   13
> > +#define ARCH_TIMER_NS_EL1_IRQ  14
> > +#define ARCH_TIMER_NS_EL2_IRQ  10
> > +
> >  enum {
> >      SBSA_FLASH,
> >      SBSA_MEM,
> > @@ -115,6 +133,415 @@ static const int sbsa_ref_irqmap[] = {
> >      [SBSA_EHCI] = 11,
> >  };
> >
> > +/*
> > + * Firmware on this machine only uses ACPI table to load OS, these limited
> > + * device tree nodes are just to let firmware know the info which varies from
> > + * command line parameters, so it is not necessary to be fully compatible
> > + * with the kernel CPU and NUMA binding rules.
> > + */
> > +static void create_fdt(SBSAMachineState *vms)
> > +{
> > +    void *fdt = create_device_tree(&vms->fdt_size);
> > +    const MachineState *ms = MACHINE(vms);
> > +    int cpu;
> > +
> > +    if (!fdt) {
> > +        error_report("create_device_tree() failed");
> > +        exit(1);
> > +    }
> > +
> > +    vms->fdt = fdt;
> > +
> > +    qemu_fdt_setprop_string(fdt, "/", "compatible", "linux,sbsa-ref");
> > +    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x2);
> > +    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2);
> > +
> > +    if (have_numa_distance) {
> > +        int size = nb_numa_nodes * nb_numa_nodes * 3 * sizeof(uint32_t);
> > +        uint32_t *matrix = g_malloc0(size);
> > +        int idx, i, j;
> > +
> > +        for (i = 0; i < nb_numa_nodes; i++) {
> > +            for (j = 0; j < nb_numa_nodes; j++) {
> > +                idx = (i * nb_numa_nodes + j) * 3;
> > +                matrix[idx + 0] = cpu_to_be32(i);
> > +                matrix[idx + 1] = cpu_to_be32(j);
> > +                matrix[idx + 2] = cpu_to_be32(numa_info[i].distance[j]);
> > +            }
> > +        }
> > +
> > +        qemu_fdt_add_subnode(fdt, "/distance-map");
> > +        qemu_fdt_setprop(fdt, "/distance-map", "distance-matrix",
> > +                         matrix, size);
> > +        g_free(matrix);
> > +    }
> > +
> > +    qemu_fdt_add_subnode(vms->fdt, "/cpus");
> > +
> > +    for (cpu = vms->smp_cpus - 1; cpu >= 0; cpu--) {
> > +        char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
> > +        ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));
> > +        CPUState *cs = CPU(armcpu);
> > +
> > +        qemu_fdt_add_subnode(vms->fdt, nodename);
> > +
> > +        if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) {
> > +            qemu_fdt_setprop_cell(vms->fdt, nodename, "numa-node-id",
> > +                ms->possible_cpus->cpus[cs->cpu_index].props.node_id);
> > +        }
> > +
> > +        g_free(nodename);
> > +    }
> > +}
> > +
> > +static void create_one_flash(const char *name, hwaddr flashbase,
> > +                             hwaddr flashsize, const char *file,
> > +                             MemoryRegion *sysmem)
> > +{
> > +    /*
> > +     * Create and map a single flash device. We use the same
> > +     * parameters as the flash devices on the Versatile Express board.
> > +     */
> > +    DriveInfo *dinfo = drive_get_next(IF_PFLASH);
> > +    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
>
> Please use TYPE_PFLASH_CFI01 instead of "cfi.pflash01".
>
> I wanted to ask "does it has to be CFI01?" because this device model is
> in bad shape, but I guess I answered myself looking at the EDK2 platform
> code:
>
> - P30_CFI_ADDR_VENDOR_ID is not used
> - NorFlashDxe::NorFlashReadCfiData() is not implemented
> - All commands in NorFlashDxe uses:
>     SEND_NOR_COMMAND(..., P30_CMD_...)
>   which are specific to the Intel P30 Nor flash family (CFI01).
>
> > +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > +    const uint64_t sectorlength = 256 * 1024;
> > +
> > +    if (dinfo) {
> > +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
> > +                            &error_abort);
> > +    }
> > +
> > +    qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
> > +    qdev_prop_set_uint64(dev, "sector-length", sectorlength);
> > +    qdev_prop_set_uint8(dev, "width", 4);
> > +    qdev_prop_set_uint8(dev, "device-width", 2);
> > +    qdev_prop_set_bit(dev, "big-endian", false);
> > +    qdev_prop_set_uint16(dev, "id0", 0x89);
> > +    qdev_prop_set_uint16(dev, "id1", 0x18);
> > +    qdev_prop_set_uint16(dev, "id2", 0x00);
> > +    qdev_prop_set_uint16(dev, "id3", 0x00);
> > +    qdev_prop_set_string(dev, "name", name);
> > +    qdev_init_nofail(dev);
> > +
> > +    memory_region_add_subregion(sysmem, flashbase,
> > +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
> > +
> > +    if (file) {
> > +        char *fn;
> > +        int image_size;
> > +
> > +        if (drive_get(IF_PFLASH, 0, 0)) {
> > +            error_report("The contents of the first flash device may be "
> > +                         "specified with -bios or with -drive if=pflash... "
> > +                         "but you cannot use both options at once");
> > +            exit(1);
> > +        }
> > +        fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, file);
> > +        if (!fn) {
> > +            error_report("Could not find ROM image '%s'", file);
> > +            exit(1);
> > +        }
> > +        image_size = load_image_mr(fn, sysbus_mmio_get_region(sbd, 0));
> > +        g_free(fn);
> > +        if (image_size < 0) {
> > +            error_report("Could not load ROM image '%s'", file);
> > +            exit(1);
> > +        }
> > +    }
> > +}
> > +
> > +static void create_flash(const SBSAMachineState *vms,
> > +                         MemoryRegion *sysmem,
> > +                         MemoryRegion *secure_sysmem)
> > +{
> > +    /*
> > +     * Create one secure and nonsecure flash devices to fill SBSA_FLASH
> > +     * space in the memmap, file passed via -bios goes in the first one.
> > +     */
> > +    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
> > +    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
> > +
> > +    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
> > +                     bios_name, secure_sysmem);
> > +    create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
> > +                     NULL, sysmem);
>
> static const MemMapEntry base_memmap[] = {
>     /* Space up to 0x8000000 is reserved for a boot ROM */
>     [VIRT_FLASH] =              {          0, 0x08000000 },
>
Hi Philippe,
Thank you for the long comments.
Some parts of this machine are based on the 'virt' machine, but I use
this flash memory map:
[SBSA_FLASH] =              {          0, 0x20000000 },
that are 256M *2 flashes.
Franky I didn't consider the product part number etc, just use the
original design in 'virt' and change the size large enough as I think.

Peter, Ard, do we need more considerations here?

> So you are creating 2 identical flashes of 128MiB/2 = 64 MiB each which
> are the biggest flash you can have:
>
> "The P30 family provides density upgrades from 64-Mbit through
> 512-Mbit." On Intel, the 512-Mib case is particular in that it is built
> of 2x 256-Mib on the same die, with a virtual chip enable. It is simpler
> to use a Micron or Numonyx model.
>
> I plan to use a whitelist of supported (and tested...) models, the one
> you use seems the Micron JS28F512P30EF ('E' for 'Symetrically Blocked',
> since the current model doesn't support bottom/top blocks layout), or in
> short: 28F512P30E.
> Ard, is that OK?
>
> Checking EDK2 git history, the driver is part of ArmPlatformPkg,
> imported in commit 1d5d0ae92d9541, based on 'Versatile Express'.
>
> On the Versatile Express and the RealView Emulation Baseboard user
> guides, I only find reference of "64MB of NOR flash" with no specific model.
>
> Peter, do you have physical access to tell me what flashes are used on
> real hardware? I googled for Linux console boot log but the kernel
> doesn't seem to care about detecting/mapping the flash.
>
> QEMU added the flash to the Versatile board in commit 964c695a54ceda3a,
> with the following description:
>
>     - add support for the 64MB NOR CFI01 flash available at
>     0x34000000 on the versatilepb board
>
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0225d/BBAJIHEC.html
>
> However on this link I only see "SSMC Chip Select 1, normally NOR flash
> (During boot remapping, this can be NOR flash, Disk-on-Chip, or static
> expansion memory)". Again, nothing specific (which makes sense, why
> restrict the users to a particuliar family, as long as the pinout matches).
>
> The Manufacturer/Device ID used in QEMU (0x0089, 0x0018) correspond to
> the Micron 28F128J3D (128-Mbit, 128 symmetrical blocks of 128-KiB).
> Neither the flash size (64 vs 16) nor the block size (256 vs 128) match.
>
> The safer fix here is to find a CFI01 flash of 256 sectors of 256-KiB
> and update the Manufacturer/Device IDs in QEMU. Luckily this matches the
> 28F512P30E cited previously :)
>
> Regards,
>
> Phil.
>
> > +}
> > +
> > +static void create_secure_ram(SBSAMachineState *vms,
> > +                              MemoryRegion *secure_sysmem)
> > +{
> > +    MemoryRegion *secram = g_new(MemoryRegion, 1);
> > +    hwaddr base = vms->memmap[SBSA_SECURE_MEM].base;
> > +    hwaddr size = vms->memmap[SBSA_SECURE_MEM].size;
> > +
> > +    memory_region_init_ram(secram, NULL, "sbsa-ref.secure-ram", size,
> > +                           &error_fatal);
> > +    memory_region_add_subregion(secure_sysmem, base, secram);
> > +}
> > +
> > +static void create_gic(SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    DeviceState *gicdev;
> > +    SysBusDevice *gicbusdev;
> > +    const char *gictype;
> > +    uint32_t redist0_capacity, redist0_count;
> > +    int i;
> > +
> > +    gictype = gicv3_class_name();
> > +
> > +    gicdev = qdev_create(NULL, gictype);
> > +    qdev_prop_set_uint32(gicdev, "revision", 3);
> > +    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).
> > +     */
> > +    qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
> > +    qdev_prop_set_bit(gicdev, "has-security-extensions", true);
> > +
> > +    redist0_capacity =
> > +                vms->memmap[SBSA_GIC_REDIST].size / GICV3_REDIST_SIZE;
> > +    redist0_count = MIN(smp_cpus, redist0_capacity);
> > +
> > +    qdev_prop_set_uint32(gicdev, "len-redist-region-count", 1);
> > +    qdev_prop_set_uint32(gicdev, "redist-region-count[0]", redist0_count);
> > +
> > +    qdev_init_nofail(gicdev);
> > +    gicbusdev = SYS_BUS_DEVICE(gicdev);
> > +    sysbus_mmio_map(gicbusdev, 0, vms->memmap[SBSA_GIC_DIST].base);
> > +    sysbus_mmio_map(gicbusdev, 1, vms->memmap[SBSA_GIC_REDIST].base);
> > +
> > +    /*
> > +     * Wire the outputs from each CPU's generic timer and the GICv3
> > +     * maintenance interrupt signal to the appropriate GIC PPI inputs,
> > +     * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
> > +     */
> > +    for (i = 0; i < smp_cpus; i++) {
> > +        DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
> > +        int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
> > +        int irq;
> > +        /*
> > +         * Mapping from the output timer irq lines from the CPU to the
> > +         * GIC PPI inputs used for this board.
> > +         */
> > +        const int timer_irq[] = {
> > +            [GTIMER_PHYS] = ARCH_TIMER_NS_EL1_IRQ,
> > +            [GTIMER_VIRT] = ARCH_TIMER_VIRT_IRQ,
> > +            [GTIMER_HYP]  = ARCH_TIMER_NS_EL2_IRQ,
> > +            [GTIMER_SEC]  = ARCH_TIMER_S_EL1_IRQ,
> > +        };
> > +
> > +        for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
> > +            qdev_connect_gpio_out(cpudev, irq,
> > +                                  qdev_get_gpio_in(gicdev,
> > +                                                   ppibase + timer_irq[irq]));
> > +        }
> > +
> > +        qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0,
> > +                                    qdev_get_gpio_in(gicdev, ppibase
> > +                                                     + ARCH_GIC_MAINT_IRQ));
> > +        qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0,
> > +                                    qdev_get_gpio_in(gicdev, ppibase
> > +                                                     + VIRTUAL_PMU_IRQ));
> > +
> > +        sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
> > +        sysbus_connect_irq(gicbusdev, i + smp_cpus,
> > +                           qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> > +        sysbus_connect_irq(gicbusdev, i + 2 * smp_cpus,
> > +                           qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
> > +        sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
> > +                           qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
> > +    }
> > +
> > +    for (i = 0; i < NUM_IRQS; i++) {
> > +        pic[i] = qdev_get_gpio_in(gicdev, i);
> > +    }
> > +}
> > +
> > +static void create_uart(const SBSAMachineState *vms, qemu_irq *pic, int uart,
> > +                        MemoryRegion *mem, Chardev *chr)
> > +{
> > +    hwaddr base = vms->memmap[uart].base;
> > +    int irq = vms->irqmap[uart];
> > +    DeviceState *dev = qdev_create(NULL, "pl011");
> > +    SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > +
> > +    qdev_prop_set_chr(dev, "chardev", chr);
> > +    qdev_init_nofail(dev);
> > +    memory_region_add_subregion(mem, base,
> > +                                sysbus_mmio_get_region(s, 0));
> > +    sysbus_connect_irq(s, 0, pic[irq]);
> > +}
> > +
> > +static void create_rtc(const SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    hwaddr base = vms->memmap[SBSA_RTC].base;
> > +    int irq = vms->irqmap[SBSA_RTC];
> > +
> > +    sysbus_create_simple("pl031", base, pic[irq]);
> > +}
> > +
> > +static DeviceState *gpio_key_dev;
> > +static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
> > +{
> > +    /* use gpio Pin 3 for power button event */
> > +    qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
> > +}
> > +
> > +static Notifier sbsa_ref_powerdown_notifier = {
> > +    .notify = sbsa_ref_powerdown_req
> > +};
> > +
> > +static void create_gpio(const SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    DeviceState *pl061_dev;
> > +    hwaddr base = vms->memmap[SBSA_GPIO].base;
> > +    int irq = vms->irqmap[SBSA_GPIO];
> > +
> > +    pl061_dev = sysbus_create_simple("pl061", base, pic[irq]);
> > +
> > +    gpio_key_dev = sysbus_create_simple("gpio-key", -1,
> > +                                        qdev_get_gpio_in(pl061_dev, 3));
> > +
> > +    /* connect powerdown request */
> > +    qemu_register_powerdown_notifier(&sbsa_ref_powerdown_notifier);
> > +}
> > +
> > +static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    hwaddr base = vms->memmap[SBSA_AHCI].base;
> > +    int irq = vms->irqmap[SBSA_AHCI];
> > +    DeviceState *dev;
> > +    DriveInfo *hd[NUM_SATA_PORTS];
> > +    SysbusAHCIState *sysahci;
> > +    AHCIState *ahci;
> > +    int i;
> > +
> > +    dev = qdev_create(NULL, "sysbus-ahci");
> > +    qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS);
> > +    qdev_init_nofail(dev);
> > +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> > +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> > +
> > +    sysahci = SYSBUS_AHCI(dev);
> > +    ahci = &sysahci->ahci;
> > +    ide_drive_get(hd, ARRAY_SIZE(hd));
> > +    for (i = 0; i < ahci->ports; i++) {
> > +        if (hd[i] == NULL) {
> > +            continue;
> > +        }
> > +        ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
> > +    }
> > +}
> > +
> > +static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    hwaddr base = vms->memmap[SBSA_EHCI].base;
> > +    int irq = vms->irqmap[SBSA_EHCI];
> > +    USBBus *usb_bus;
> > +
> > +    sysbus_create_simple("platform-ehci-usb", base, pic[irq]);
> > +
> > +    usb_bus = usb_bus_find(-1);
> > +    usb_create_simple(usb_bus, "usb-kbd");
> > +    usb_create_simple(usb_bus, "usb-mouse");
> > +}
> > +
> > +static void create_smmu(const SBSAMachineState *vms, qemu_irq *pic,
> > +                        PCIBus *bus)
> > +{
> > +    hwaddr base = vms->memmap[SBSA_SMMU].base;
> > +    int irq =  vms->irqmap[SBSA_SMMU];
> > +    DeviceState *dev;
> > +    int i;
> > +
> > +    dev = qdev_create(NULL, "arm-smmuv3");
> > +
> > +    object_property_set_link(OBJECT(dev), OBJECT(bus), "primary-bus",
> > +                             &error_abort);
> > +    qdev_init_nofail(dev);
> > +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> > +    for (i = 0; i < NUM_SMMU_IRQS; i++) {
> > +        sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
> > +    }
> > +}
> > +
> > +static void create_pcie(SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    hwaddr base_ecam = vms->memmap[SBSA_PCIE_ECAM].base;
> > +    hwaddr size_ecam = vms->memmap[SBSA_PCIE_ECAM].size;
> > +    hwaddr base_mmio = vms->memmap[SBSA_PCIE_MMIO].base;
> > +    hwaddr size_mmio = vms->memmap[SBSA_PCIE_MMIO].size;
> > +    hwaddr base_mmio_high = vms->memmap[SBSA_PCIE_MMIO_HIGH].base;
> > +    hwaddr size_mmio_high = vms->memmap[SBSA_PCIE_MMIO_HIGH].size;
> > +    hwaddr base_pio = vms->memmap[SBSA_PCIE_PIO].base;
> > +    int irq = vms->irqmap[SBSA_PCIE];
> > +    MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
> > +    MemoryRegion *ecam_alias, *ecam_reg;
> > +    DeviceState *dev;
> > +    PCIHostState *pci;
> > +    int i;
> > +
> > +    dev = qdev_create(NULL, TYPE_GPEX_HOST);
> > +    qdev_init_nofail(dev);
> > +
> > +    /* Map ECAM space */
> > +    ecam_alias = g_new0(MemoryRegion, 1);
> > +    ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> > +    memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
> > +                             ecam_reg, 0, size_ecam);
> > +    memory_region_add_subregion(get_system_memory(), base_ecam, ecam_alias);
> > +
> > +    /* Map the MMIO space */
> > +    mmio_alias = g_new0(MemoryRegion, 1);
> > +    mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
> > +    memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
> > +                             mmio_reg, base_mmio, size_mmio);
> > +    memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
> > +
> > +    /* Map the MMIO_HIGH space */
> > +    mmio_alias_high = g_new0(MemoryRegion, 1);
> > +    memory_region_init_alias(mmio_alias_high, OBJECT(dev), "pcie-mmio-high",
> > +                             mmio_reg, base_mmio_high, size_mmio_high);
> > +    memory_region_add_subregion(get_system_memory(), base_mmio_high,
> > +                                mmio_alias_high);
> > +
> > +    /* Map IO port space */
> > +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio);
> > +
> > +    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > +        sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
> > +        gpex_set_irq_num(GPEX_HOST(dev), i, irq + i);
> > +    }
> > +
> > +    pci = PCI_HOST_BRIDGE(dev);
> > +    if (pci->bus) {
> > +        for (i = 0; i < nb_nics; i++) {
> > +            NICInfo *nd = &nd_table[i];
> > +
> > +            if (!nd->model) {
> > +                nd->model = g_strdup("e1000e");
> > +            }
> > +
> > +            pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);
> > +        }
> > +    }
> > +
> > +    pci_create_simple(pci->bus, -1, "VGA");
> > +
> > +    create_smmu(vms, pic, pci->bus);
> > +}
> > +
> > +static void *sbsa_ref_dtb(const struct arm_boot_info *binfo, int *fdt_size)
> > +{
> > +    const SBSAMachineState *board = container_of(binfo, SBSAMachineState,
> > +                                                 bootinfo);
> > +
> > +    *fdt_size = board->fdt_size;
> > +    return board->fdt;
> > +}
> > +
> >  static void sbsa_ref_init(MachineState *machine)
> >  {
> >      SBSAMachineState *vms = SBSA_MACHINE(machine);
> > @@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine)
> >      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> >      const CPUArchIdList *possible_cpus;
> >      int n, sbsa_max_cpus;
> > +    qemu_irq pic[NUM_IRQS];
> >
> >      if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
> >          error_report("sbsa-ref: CPU type other than the built-in "
> > @@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine)
> >                                           machine->ram_size);
> >      memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram);
> >
> > +    create_fdt(vms);
> > +
> > +    create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
> > +
> > +    create_secure_ram(vms, secure_sysmem);
> > +
> > +    create_gic(vms, pic);
> > +
> > +    create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0));
> > +    create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1));
> > +    create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2));
> > +
> > +    create_rtc(vms, pic);
> > +
> > +    create_gpio(vms, pic);
> > +
> > +    create_ahci(vms, pic);
> > +
> > +    create_ehci(vms, pic);
> > +
> > +    create_pcie(vms, pic);
> > +
> >      vms->bootinfo.ram_size = machine->ram_size;
> >      vms->bootinfo.kernel_filename = machine->kernel_filename;
> >      vms->bootinfo.nb_cpus = smp_cpus;
> >      vms->bootinfo.board_id = -1;
> >      vms->bootinfo.loader_start = vms->memmap[SBSA_MEM].base;
> > +    vms->bootinfo.get_dtb = sbsa_ref_dtb;
> >      vms->bootinfo.firmware_loaded = firmware_loaded;
> >      arm_load_kernel(ARM_CPU(first_cpu), &vms->bootinfo);
> >  }
> >


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
  2019-06-03 10:54   ` Philippe Mathieu-Daudé
  2019-06-16 11:41     ` Hongbo Zhang
@ 2019-06-17  2:44     ` Hongbo Zhang
  1 sibling, 0 replies; 24+ messages in thread
From: Hongbo Zhang @ 2019-06-17  2:44 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Radoslaw Biernacki, Ard Biesheuvel,
	QEMU Developers, Leif Lindholm, qemu-arm, Laszlo Ersek

On Mon, 3 Jun 2019 at 18:54, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Hi Hongbo, Ard.
>
> On 4/18/19 6:04 AM, Hongbo Zhang wrote:
> > Following the previous patch, this patch adds peripheral devices to the
> > newly introduced SBSA-ref machine.
> >
> > Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> > ---
> >  hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 451 insertions(+)
> >
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index 652ec13..3fb0027 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -21,6 +21,7 @@
> >  #include "qapi/error.h"
> >  #include "qemu/error-report.h"
> >  #include "qemu/units.h"
> > +#include "sysemu/device_tree.h"
> >  #include "sysemu/numa.h"
> >  #include "sysemu/sysemu.h"
> >  #include "exec/address-spaces.h"
> > @@ -28,11 +29,28 @@
> >  #include "kvm_arm.h"
> >  #include "hw/arm/arm.h"
> >  #include "hw/boards.h"
> > +#include "hw/ide/internal.h"
> > +#include "hw/ide/ahci_internal.h"
> >  #include "hw/intc/arm_gicv3_common.h"
> > +#include "hw/loader.h"
> > +#include "hw/pci-host/gpex.h"
> > +#include "hw/usb.h"
> > +#include "net/net.h"
> >
> >  #define RAMLIMIT_GB 8192
> >  #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
> >
> > +#define NUM_IRQS        256
> > +#define NUM_SMMU_IRQS   4
> > +#define NUM_SATA_PORTS  6
> > +
> > +#define VIRTUAL_PMU_IRQ        7
> > +#define ARCH_GIC_MAINT_IRQ     9
> > +#define ARCH_TIMER_VIRT_IRQ    11
> > +#define ARCH_TIMER_S_EL1_IRQ   13
> > +#define ARCH_TIMER_NS_EL1_IRQ  14
> > +#define ARCH_TIMER_NS_EL2_IRQ  10
> > +
> >  enum {
> >      SBSA_FLASH,
> >      SBSA_MEM,
> > @@ -115,6 +133,415 @@ static const int sbsa_ref_irqmap[] = {
> >      [SBSA_EHCI] = 11,
> >  };
> >
> > +/*
> > + * Firmware on this machine only uses ACPI table to load OS, these limited
> > + * device tree nodes are just to let firmware know the info which varies from
> > + * command line parameters, so it is not necessary to be fully compatible
> > + * with the kernel CPU and NUMA binding rules.
> > + */
> > +static void create_fdt(SBSAMachineState *vms)
> > +{
> > +    void *fdt = create_device_tree(&vms->fdt_size);
> > +    const MachineState *ms = MACHINE(vms);
> > +    int cpu;
> > +
> > +    if (!fdt) {
> > +        error_report("create_device_tree() failed");
> > +        exit(1);
> > +    }
> > +
> > +    vms->fdt = fdt;
> > +
> > +    qemu_fdt_setprop_string(fdt, "/", "compatible", "linux,sbsa-ref");
> > +    qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x2);
> > +    qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2);
> > +
> > +    if (have_numa_distance) {
> > +        int size = nb_numa_nodes * nb_numa_nodes * 3 * sizeof(uint32_t);
> > +        uint32_t *matrix = g_malloc0(size);
> > +        int idx, i, j;
> > +
> > +        for (i = 0; i < nb_numa_nodes; i++) {
> > +            for (j = 0; j < nb_numa_nodes; j++) {
> > +                idx = (i * nb_numa_nodes + j) * 3;
> > +                matrix[idx + 0] = cpu_to_be32(i);
> > +                matrix[idx + 1] = cpu_to_be32(j);
> > +                matrix[idx + 2] = cpu_to_be32(numa_info[i].distance[j]);
> > +            }
> > +        }
> > +
> > +        qemu_fdt_add_subnode(fdt, "/distance-map");
> > +        qemu_fdt_setprop(fdt, "/distance-map", "distance-matrix",
> > +                         matrix, size);
> > +        g_free(matrix);
> > +    }
> > +
> > +    qemu_fdt_add_subnode(vms->fdt, "/cpus");
> > +
> > +    for (cpu = vms->smp_cpus - 1; cpu >= 0; cpu--) {
> > +        char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
> > +        ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));
> > +        CPUState *cs = CPU(armcpu);
> > +
> > +        qemu_fdt_add_subnode(vms->fdt, nodename);
> > +
> > +        if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) {
> > +            qemu_fdt_setprop_cell(vms->fdt, nodename, "numa-node-id",
> > +                ms->possible_cpus->cpus[cs->cpu_index].props.node_id);
> > +        }
> > +
> > +        g_free(nodename);
> > +    }
> > +}
> > +
> > +static void create_one_flash(const char *name, hwaddr flashbase,
> > +                             hwaddr flashsize, const char *file,
> > +                             MemoryRegion *sysmem)
> > +{
> > +    /*
> > +     * Create and map a single flash device. We use the same
> > +     * parameters as the flash devices on the Versatile Express board.
> > +     */
> > +    DriveInfo *dinfo = drive_get_next(IF_PFLASH);
> > +    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
>
> Please use TYPE_PFLASH_CFI01 instead of "cfi.pflash01".
>
And as reviewed by Markus, I will update to the new method of create
flash, as commit e0561e60f17, TYPE_PFLASH_CFI01 is used there.

> I wanted to ask "does it has to be CFI01?" because this device model is
> in bad shape, but I guess I answered myself looking at the EDK2 platform
> code:
>
> - P30_CFI_ADDR_VENDOR_ID is not used
> - NorFlashDxe::NorFlashReadCfiData() is not implemented
> - All commands in NorFlashDxe uses:
>     SEND_NOR_COMMAND(..., P30_CMD_...)
>   which are specific to the Intel P30 Nor flash family (CFI01).
>
> > +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > +    const uint64_t sectorlength = 256 * 1024;
> > +
> > +    if (dinfo) {
> > +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
> > +                            &error_abort);
> > +    }
> > +
> > +    qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
> > +    qdev_prop_set_uint64(dev, "sector-length", sectorlength);
> > +    qdev_prop_set_uint8(dev, "width", 4);
> > +    qdev_prop_set_uint8(dev, "device-width", 2);
> > +    qdev_prop_set_bit(dev, "big-endian", false);
> > +    qdev_prop_set_uint16(dev, "id0", 0x89);
> > +    qdev_prop_set_uint16(dev, "id1", 0x18);
> > +    qdev_prop_set_uint16(dev, "id2", 0x00);
> > +    qdev_prop_set_uint16(dev, "id3", 0x00);
> > +    qdev_prop_set_string(dev, "name", name);
> > +    qdev_init_nofail(dev);
> > +
> > +    memory_region_add_subregion(sysmem, flashbase,
> > +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
> > +
> > +    if (file) {
> > +        char *fn;
> > +        int image_size;
> > +
> > +        if (drive_get(IF_PFLASH, 0, 0)) {
> > +            error_report("The contents of the first flash device may be "
> > +                         "specified with -bios or with -drive if=pflash... "
> > +                         "but you cannot use both options at once");
> > +            exit(1);
> > +        }
> > +        fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, file);
> > +        if (!fn) {
> > +            error_report("Could not find ROM image '%s'", file);
> > +            exit(1);
> > +        }
> > +        image_size = load_image_mr(fn, sysbus_mmio_get_region(sbd, 0));
> > +        g_free(fn);
> > +        if (image_size < 0) {
> > +            error_report("Could not load ROM image '%s'", file);
> > +            exit(1);
> > +        }
> > +    }
> > +}
> > +
> > +static void create_flash(const SBSAMachineState *vms,
> > +                         MemoryRegion *sysmem,
> > +                         MemoryRegion *secure_sysmem)
> > +{
> > +    /*
> > +     * Create one secure and nonsecure flash devices to fill SBSA_FLASH
> > +     * space in the memmap, file passed via -bios goes in the first one.
> > +     */
> > +    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
> > +    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
> > +
> > +    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
> > +                     bios_name, secure_sysmem);
> > +    create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
> > +                     NULL, sysmem);
>
> static const MemMapEntry base_memmap[] = {
>     /* Space up to 0x8000000 is reserved for a boot ROM */
>     [VIRT_FLASH] =              {          0, 0x08000000 },
>
> So you are creating 2 identical flashes of 128MiB/2 = 64 MiB each which
> are the biggest flash you can have:
>
> "The P30 family provides density upgrades from 64-Mbit through
> 512-Mbit." On Intel, the 512-Mib case is particular in that it is built
> of 2x 256-Mib on the same die, with a virtual chip enable. It is simpler
> to use a Micron or Numonyx model.
>
> I plan to use a whitelist of supported (and tested...) models, the one
> you use seems the Micron JS28F512P30EF ('E' for 'Symetrically Blocked',
> since the current model doesn't support bottom/top blocks layout), or in
> short: 28F512P30E.
> Ard, is that OK?
>
> Checking EDK2 git history, the driver is part of ArmPlatformPkg,
> imported in commit 1d5d0ae92d9541, based on 'Versatile Express'.
>
> On the Versatile Express and the RealView Emulation Baseboard user
> guides, I only find reference of "64MB of NOR flash" with no specific model.
>
> Peter, do you have physical access to tell me what flashes are used on
> real hardware? I googled for Linux console boot log but the kernel
> doesn't seem to care about detecting/mapping the flash.
>
> QEMU added the flash to the Versatile board in commit 964c695a54ceda3a,
> with the following description:
>
>     - add support for the 64MB NOR CFI01 flash available at
>     0x34000000 on the versatilepb board
>
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0225d/BBAJIHEC.html
>
> However on this link I only see "SSMC Chip Select 1, normally NOR flash
> (During boot remapping, this can be NOR flash, Disk-on-Chip, or static
> expansion memory)". Again, nothing specific (which makes sense, why
> restrict the users to a particuliar family, as long as the pinout matches).
>
> The Manufacturer/Device ID used in QEMU (0x0089, 0x0018) correspond to
> the Micron 28F128J3D (128-Mbit, 128 symmetrical blocks of 128-KiB).
> Neither the flash size (64 vs 16) nor the block size (256 vs 128) match.
>
> The safer fix here is to find a CFI01 flash of 256 sectors of 256-KiB
> and update the Manufacturer/Device IDs in QEMU. Luckily this matches the
> 28F512P30E cited previously :)
>
> Regards,
>
> Phil.
>
> > +}
> > +
> > +static void create_secure_ram(SBSAMachineState *vms,
> > +                              MemoryRegion *secure_sysmem)
> > +{
> > +    MemoryRegion *secram = g_new(MemoryRegion, 1);
> > +    hwaddr base = vms->memmap[SBSA_SECURE_MEM].base;
> > +    hwaddr size = vms->memmap[SBSA_SECURE_MEM].size;
> > +
> > +    memory_region_init_ram(secram, NULL, "sbsa-ref.secure-ram", size,
> > +                           &error_fatal);
> > +    memory_region_add_subregion(secure_sysmem, base, secram);
> > +}
> > +
> > +static void create_gic(SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    DeviceState *gicdev;
> > +    SysBusDevice *gicbusdev;
> > +    const char *gictype;
> > +    uint32_t redist0_capacity, redist0_count;
> > +    int i;
> > +
> > +    gictype = gicv3_class_name();
> > +
> > +    gicdev = qdev_create(NULL, gictype);
> > +    qdev_prop_set_uint32(gicdev, "revision", 3);
> > +    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).
> > +     */
> > +    qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32);
> > +    qdev_prop_set_bit(gicdev, "has-security-extensions", true);
> > +
> > +    redist0_capacity =
> > +                vms->memmap[SBSA_GIC_REDIST].size / GICV3_REDIST_SIZE;
> > +    redist0_count = MIN(smp_cpus, redist0_capacity);
> > +
> > +    qdev_prop_set_uint32(gicdev, "len-redist-region-count", 1);
> > +    qdev_prop_set_uint32(gicdev, "redist-region-count[0]", redist0_count);
> > +
> > +    qdev_init_nofail(gicdev);
> > +    gicbusdev = SYS_BUS_DEVICE(gicdev);
> > +    sysbus_mmio_map(gicbusdev, 0, vms->memmap[SBSA_GIC_DIST].base);
> > +    sysbus_mmio_map(gicbusdev, 1, vms->memmap[SBSA_GIC_REDIST].base);
> > +
> > +    /*
> > +     * Wire the outputs from each CPU's generic timer and the GICv3
> > +     * maintenance interrupt signal to the appropriate GIC PPI inputs,
> > +     * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
> > +     */
> > +    for (i = 0; i < smp_cpus; i++) {
> > +        DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
> > +        int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
> > +        int irq;
> > +        /*
> > +         * Mapping from the output timer irq lines from the CPU to the
> > +         * GIC PPI inputs used for this board.
> > +         */
> > +        const int timer_irq[] = {
> > +            [GTIMER_PHYS] = ARCH_TIMER_NS_EL1_IRQ,
> > +            [GTIMER_VIRT] = ARCH_TIMER_VIRT_IRQ,
> > +            [GTIMER_HYP]  = ARCH_TIMER_NS_EL2_IRQ,
> > +            [GTIMER_SEC]  = ARCH_TIMER_S_EL1_IRQ,
> > +        };
> > +
> > +        for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
> > +            qdev_connect_gpio_out(cpudev, irq,
> > +                                  qdev_get_gpio_in(gicdev,
> > +                                                   ppibase + timer_irq[irq]));
> > +        }
> > +
> > +        qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0,
> > +                                    qdev_get_gpio_in(gicdev, ppibase
> > +                                                     + ARCH_GIC_MAINT_IRQ));
> > +        qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0,
> > +                                    qdev_get_gpio_in(gicdev, ppibase
> > +                                                     + VIRTUAL_PMU_IRQ));
> > +
> > +        sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
> > +        sysbus_connect_irq(gicbusdev, i + smp_cpus,
> > +                           qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> > +        sysbus_connect_irq(gicbusdev, i + 2 * smp_cpus,
> > +                           qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
> > +        sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
> > +                           qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
> > +    }
> > +
> > +    for (i = 0; i < NUM_IRQS; i++) {
> > +        pic[i] = qdev_get_gpio_in(gicdev, i);
> > +    }
> > +}
> > +
> > +static void create_uart(const SBSAMachineState *vms, qemu_irq *pic, int uart,
> > +                        MemoryRegion *mem, Chardev *chr)
> > +{
> > +    hwaddr base = vms->memmap[uart].base;
> > +    int irq = vms->irqmap[uart];
> > +    DeviceState *dev = qdev_create(NULL, "pl011");
> > +    SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > +
> > +    qdev_prop_set_chr(dev, "chardev", chr);
> > +    qdev_init_nofail(dev);
> > +    memory_region_add_subregion(mem, base,
> > +                                sysbus_mmio_get_region(s, 0));
> > +    sysbus_connect_irq(s, 0, pic[irq]);
> > +}
> > +
> > +static void create_rtc(const SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    hwaddr base = vms->memmap[SBSA_RTC].base;
> > +    int irq = vms->irqmap[SBSA_RTC];
> > +
> > +    sysbus_create_simple("pl031", base, pic[irq]);
> > +}
> > +
> > +static DeviceState *gpio_key_dev;
> > +static void sbsa_ref_powerdown_req(Notifier *n, void *opaque)
> > +{
> > +    /* use gpio Pin 3 for power button event */
> > +    qemu_set_irq(qdev_get_gpio_in(gpio_key_dev, 0), 1);
> > +}
> > +
> > +static Notifier sbsa_ref_powerdown_notifier = {
> > +    .notify = sbsa_ref_powerdown_req
> > +};
> > +
> > +static void create_gpio(const SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    DeviceState *pl061_dev;
> > +    hwaddr base = vms->memmap[SBSA_GPIO].base;
> > +    int irq = vms->irqmap[SBSA_GPIO];
> > +
> > +    pl061_dev = sysbus_create_simple("pl061", base, pic[irq]);
> > +
> > +    gpio_key_dev = sysbus_create_simple("gpio-key", -1,
> > +                                        qdev_get_gpio_in(pl061_dev, 3));
> > +
> > +    /* connect powerdown request */
> > +    qemu_register_powerdown_notifier(&sbsa_ref_powerdown_notifier);
> > +}
> > +
> > +static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    hwaddr base = vms->memmap[SBSA_AHCI].base;
> > +    int irq = vms->irqmap[SBSA_AHCI];
> > +    DeviceState *dev;
> > +    DriveInfo *hd[NUM_SATA_PORTS];
> > +    SysbusAHCIState *sysahci;
> > +    AHCIState *ahci;
> > +    int i;
> > +
> > +    dev = qdev_create(NULL, "sysbus-ahci");
> > +    qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS);
> > +    qdev_init_nofail(dev);
> > +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> > +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> > +
> > +    sysahci = SYSBUS_AHCI(dev);
> > +    ahci = &sysahci->ahci;
> > +    ide_drive_get(hd, ARRAY_SIZE(hd));
> > +    for (i = 0; i < ahci->ports; i++) {
> > +        if (hd[i] == NULL) {
> > +            continue;
> > +        }
> > +        ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
> > +    }
> > +}
> > +
> > +static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    hwaddr base = vms->memmap[SBSA_EHCI].base;
> > +    int irq = vms->irqmap[SBSA_EHCI];
> > +    USBBus *usb_bus;
> > +
> > +    sysbus_create_simple("platform-ehci-usb", base, pic[irq]);
> > +
> > +    usb_bus = usb_bus_find(-1);
> > +    usb_create_simple(usb_bus, "usb-kbd");
> > +    usb_create_simple(usb_bus, "usb-mouse");
> > +}
> > +
> > +static void create_smmu(const SBSAMachineState *vms, qemu_irq *pic,
> > +                        PCIBus *bus)
> > +{
> > +    hwaddr base = vms->memmap[SBSA_SMMU].base;
> > +    int irq =  vms->irqmap[SBSA_SMMU];
> > +    DeviceState *dev;
> > +    int i;
> > +
> > +    dev = qdev_create(NULL, "arm-smmuv3");
> > +
> > +    object_property_set_link(OBJECT(dev), OBJECT(bus), "primary-bus",
> > +                             &error_abort);
> > +    qdev_init_nofail(dev);
> > +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> > +    for (i = 0; i < NUM_SMMU_IRQS; i++) {
> > +        sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
> > +    }
> > +}
> > +
> > +static void create_pcie(SBSAMachineState *vms, qemu_irq *pic)
> > +{
> > +    hwaddr base_ecam = vms->memmap[SBSA_PCIE_ECAM].base;
> > +    hwaddr size_ecam = vms->memmap[SBSA_PCIE_ECAM].size;
> > +    hwaddr base_mmio = vms->memmap[SBSA_PCIE_MMIO].base;
> > +    hwaddr size_mmio = vms->memmap[SBSA_PCIE_MMIO].size;
> > +    hwaddr base_mmio_high = vms->memmap[SBSA_PCIE_MMIO_HIGH].base;
> > +    hwaddr size_mmio_high = vms->memmap[SBSA_PCIE_MMIO_HIGH].size;
> > +    hwaddr base_pio = vms->memmap[SBSA_PCIE_PIO].base;
> > +    int irq = vms->irqmap[SBSA_PCIE];
> > +    MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
> > +    MemoryRegion *ecam_alias, *ecam_reg;
> > +    DeviceState *dev;
> > +    PCIHostState *pci;
> > +    int i;
> > +
> > +    dev = qdev_create(NULL, TYPE_GPEX_HOST);
> > +    qdev_init_nofail(dev);
> > +
> > +    /* Map ECAM space */
> > +    ecam_alias = g_new0(MemoryRegion, 1);
> > +    ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> > +    memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
> > +                             ecam_reg, 0, size_ecam);
> > +    memory_region_add_subregion(get_system_memory(), base_ecam, ecam_alias);
> > +
> > +    /* Map the MMIO space */
> > +    mmio_alias = g_new0(MemoryRegion, 1);
> > +    mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
> > +    memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
> > +                             mmio_reg, base_mmio, size_mmio);
> > +    memory_region_add_subregion(get_system_memory(), base_mmio, mmio_alias);
> > +
> > +    /* Map the MMIO_HIGH space */
> > +    mmio_alias_high = g_new0(MemoryRegion, 1);
> > +    memory_region_init_alias(mmio_alias_high, OBJECT(dev), "pcie-mmio-high",
> > +                             mmio_reg, base_mmio_high, size_mmio_high);
> > +    memory_region_add_subregion(get_system_memory(), base_mmio_high,
> > +                                mmio_alias_high);
> > +
> > +    /* Map IO port space */
> > +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio);
> > +
> > +    for (i = 0; i < GPEX_NUM_IRQS; i++) {
> > +        sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
> > +        gpex_set_irq_num(GPEX_HOST(dev), i, irq + i);
> > +    }
> > +
> > +    pci = PCI_HOST_BRIDGE(dev);
> > +    if (pci->bus) {
> > +        for (i = 0; i < nb_nics; i++) {
> > +            NICInfo *nd = &nd_table[i];
> > +
> > +            if (!nd->model) {
> > +                nd->model = g_strdup("e1000e");
> > +            }
> > +
> > +            pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);
> > +        }
> > +    }
> > +
> > +    pci_create_simple(pci->bus, -1, "VGA");
> > +
> > +    create_smmu(vms, pic, pci->bus);
> > +}
> > +
> > +static void *sbsa_ref_dtb(const struct arm_boot_info *binfo, int *fdt_size)
> > +{
> > +    const SBSAMachineState *board = container_of(binfo, SBSAMachineState,
> > +                                                 bootinfo);
> > +
> > +    *fdt_size = board->fdt_size;
> > +    return board->fdt;
> > +}
> > +
> >  static void sbsa_ref_init(MachineState *machine)
> >  {
> >      SBSAMachineState *vms = SBSA_MACHINE(machine);
> > @@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine)
> >      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> >      const CPUArchIdList *possible_cpus;
> >      int n, sbsa_max_cpus;
> > +    qemu_irq pic[NUM_IRQS];
> >
> >      if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
> >          error_report("sbsa-ref: CPU type other than the built-in "
> > @@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine)
> >                                           machine->ram_size);
> >      memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram);
> >
> > +    create_fdt(vms);
> > +
> > +    create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
> > +
> > +    create_secure_ram(vms, secure_sysmem);
> > +
> > +    create_gic(vms, pic);
> > +
> > +    create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0));
> > +    create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1));
> > +    create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2));
> > +
> > +    create_rtc(vms, pic);
> > +
> > +    create_gpio(vms, pic);
> > +
> > +    create_ahci(vms, pic);
> > +
> > +    create_ehci(vms, pic);
> > +
> > +    create_pcie(vms, pic);
> > +
> >      vms->bootinfo.ram_size = machine->ram_size;
> >      vms->bootinfo.kernel_filename = machine->kernel_filename;
> >      vms->bootinfo.nb_cpus = smp_cpus;
> >      vms->bootinfo.board_id = -1;
> >      vms->bootinfo.loader_start = vms->memmap[SBSA_MEM].base;
> > +    vms->bootinfo.get_dtb = sbsa_ref_dtb;
> >      vms->bootinfo.firmware_loaded = firmware_loaded;
> >      arm_load_kernel(ARM_CPU(first_cpu), &vms->bootinfo);
> >  }
> >


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part
  2019-06-16 11:41     ` Hongbo Zhang
@ 2019-06-17 11:08       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-17 11:08 UTC (permalink / raw)
  To: Hongbo Zhang
  Cc: Peter Maydell, Radoslaw Biernacki, Ard Biesheuvel,
	QEMU Developers, Leif Lindholm, qemu-arm, Laszlo Ersek


[-- Attachment #1.1: Type: text/plain, Size: 7355 bytes --]

On 6/16/19 1:41 PM, Hongbo Zhang wrote:
> On Mon, 3 Jun 2019 at 18:54, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> Hi Hongbo, Ard.
>>
>> On 4/18/19 6:04 AM, Hongbo Zhang wrote:
>>> Following the previous patch, this patch adds peripheral devices to the
>>> newly introduced SBSA-ref machine.
>>>
>>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>>> ---
>>>  hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 451 insertions(+)
>>>
>>> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
[...]
>>> +static void create_one_flash(const char *name, hwaddr flashbase,
>>> +                             hwaddr flashsize, const char *file,
>>> +                             MemoryRegion *sysmem)
>>> +{
>>> +    /*
>>> +     * Create and map a single flash device. We use the same
>>> +     * parameters as the flash devices on the Versatile Express board.
>>> +     */
>>> +    DriveInfo *dinfo = drive_get_next(IF_PFLASH);
>>> +    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
>>
>> Please use TYPE_PFLASH_CFI01 instead of "cfi.pflash01".
>>
>> I wanted to ask "does it has to be CFI01?" because this device model is
>> in bad shape, but I guess I answered myself looking at the EDK2 platform
>> code:
>>
>> - P30_CFI_ADDR_VENDOR_ID is not used
>> - NorFlashDxe::NorFlashReadCfiData() is not implemented
>> - All commands in NorFlashDxe uses:
>>     SEND_NOR_COMMAND(..., P30_CMD_...)
>>   which are specific to the Intel P30 Nor flash family (CFI01).
>>
>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> +    const uint64_t sectorlength = 256 * 1024;
>>> +
>>> +    if (dinfo) {
>>> +        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
>>> +                            &error_abort);
>>> +    }
>>> +
>>> +    qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
>>> +    qdev_prop_set_uint64(dev, "sector-length", sectorlength);
>>> +    qdev_prop_set_uint8(dev, "width", 4);
>>> +    qdev_prop_set_uint8(dev, "device-width", 2);
>>> +    qdev_prop_set_bit(dev, "big-endian", false);
>>> +    qdev_prop_set_uint16(dev, "id0", 0x89);
>>> +    qdev_prop_set_uint16(dev, "id1", 0x18);
>>> +    qdev_prop_set_uint16(dev, "id2", 0x00);
>>> +    qdev_prop_set_uint16(dev, "id3", 0x00);
>>> +    qdev_prop_set_string(dev, "name", name);
>>> +    qdev_init_nofail(dev);
>>> +
>>> +    memory_region_add_subregion(sysmem, flashbase,
>>> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
>>> +
>>> +    if (file) {
>>> +        char *fn;
>>> +        int image_size;
>>> +
>>> +        if (drive_get(IF_PFLASH, 0, 0)) {
>>> +            error_report("The contents of the first flash device may be "
>>> +                         "specified with -bios or with -drive if=pflash... "
>>> +                         "but you cannot use both options at once");
>>> +            exit(1);
>>> +        }
>>> +        fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, file);
>>> +        if (!fn) {
>>> +            error_report("Could not find ROM image '%s'", file);
>>> +            exit(1);
>>> +        }
>>> +        image_size = load_image_mr(fn, sysbus_mmio_get_region(sbd, 0));
>>> +        g_free(fn);
>>> +        if (image_size < 0) {
>>> +            error_report("Could not load ROM image '%s'", file);
>>> +            exit(1);
>>> +        }
>>> +    }
>>> +}
>>> +
>>> +static void create_flash(const SBSAMachineState *vms,
>>> +                         MemoryRegion *sysmem,
>>> +                         MemoryRegion *secure_sysmem)
>>> +{
>>> +    /*
>>> +     * Create one secure and nonsecure flash devices to fill SBSA_FLASH
>>> +     * space in the memmap, file passed via -bios goes in the first one.
>>> +     */
>>> +    hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
>>> +    hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
>>> +
>>> +    create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
>>> +                     bios_name, secure_sysmem);
>>> +    create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
>>> +                     NULL, sysmem);
>>
>> static const MemMapEntry base_memmap[] = {
>>     /* Space up to 0x8000000 is reserved for a boot ROM */
>>     [VIRT_FLASH] =              {          0, 0x08000000 },
>>
> Hi Philippe,
> Thank you for the long comments.
> Some parts of this machine are based on the 'virt' machine, but I use
> this flash memory map:
> [SBSA_FLASH] =              {          0, 0x20000000 },
> that are 256M *2 flashes.
> Franky I didn't consider the product part number etc, just use the
> original design in 'virt' and change the size large enough as I think.

I guess we are very lucky... The Micron PC28F00BP33EF is a 2Gib
symmetrical blocks NOR flash, and the only CFI01 one I could find a
datasheet :)

"The 2Gb device employs a virtual chip enable feature, which combines
two 1Gb die with a common chip enable".

> Peter, Ard, do we need more considerations here?
> 
>> So you are creating 2 identical flashes of 128MiB/2 = 64 MiB each which
>> are the biggest flash you can have:
>>
>> "The P30 family provides density upgrades from 64-Mbit through
>> 512-Mbit." On Intel, the 512-Mib case is particular in that it is built
>> of 2x 256-Mib on the same die, with a virtual chip enable. It is simpler
>> to use a Micron or Numonyx model.
>>
>> I plan to use a whitelist of supported (and tested...) models, the one
>> you use seems the Micron JS28F512P30EF ('E' for 'Symetrically Blocked',
>> since the current model doesn't support bottom/top blocks layout), or in
>> short: 28F512P30E.
>> Ard, is that OK?
>>
>> Checking EDK2 git history, the driver is part of ArmPlatformPkg,
>> imported in commit 1d5d0ae92d9541, based on 'Versatile Express'.
>>
>> On the Versatile Express and the RealView Emulation Baseboard user
>> guides, I only find reference of "64MB of NOR flash" with no specific model.
>>
>> Peter, do you have physical access to tell me what flashes are used on
>> real hardware? I googled for Linux console boot log but the kernel
>> doesn't seem to care about detecting/mapping the flash.
>>
>> QEMU added the flash to the Versatile board in commit 964c695a54ceda3a,
>> with the following description:
>>
>>     - add support for the 64MB NOR CFI01 flash available at
>>     0x34000000 on the versatilepb board
>>
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0225d/BBAJIHEC.html
>>
>> However on this link I only see "SSMC Chip Select 1, normally NOR flash
>> (During boot remapping, this can be NOR flash, Disk-on-Chip, or static
>> expansion memory)". Again, nothing specific (which makes sense, why
>> restrict the users to a particuliar family, as long as the pinout matches).
>>
>> The Manufacturer/Device ID used in QEMU (0x0089, 0x0018) correspond to
>> the Micron 28F128J3D (128-Mbit, 128 symmetrical blocks of 128-KiB).
>> Neither the flash size (64 vs 16) nor the block size (256 vs 128) match.
>>
>> The safer fix here is to find a CFI01 flash of 256 sectors of 256-KiB
>> and update the Manufacturer/Device IDs in QEMU. Luckily this matches the
>> 28F512P30E cited previously :)
>>
>> Regards,
>>
>> Phil.
[...]


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2019-06-17 11:10 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-18  4:04 [Qemu-devel] [PATCH v7 0/2] Add Arm SBSA Reference Machine Hongbo Zhang
2019-04-18  4:04 ` Hongbo Zhang
2019-04-18  4:04 ` [Qemu-devel] [PATCH v7 1/2] hw/arm: Add arm SBSA reference machine, skeleton part Hongbo Zhang
2019-04-18  4:04   ` Hongbo Zhang
2019-04-30 14:03   ` Peter Maydell
2019-04-30 14:03     ` Peter Maydell
2019-05-08 11:22     ` Hongbo Zhang
2019-05-09 14:27       ` Igor Mammedov
2019-05-09 14:40         ` Peter Maydell
2019-04-18  4:04 ` [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part Hongbo Zhang
2019-04-18  4:04   ` Hongbo Zhang
2019-04-30 14:16   ` Peter Maydell
2019-04-30 14:16     ` Peter Maydell
2019-05-08 11:30     ` Hongbo Zhang
2019-05-08 17:48       ` Radoslaw Biernacki
2019-05-09  8:46         ` Peter Maydell
2019-05-08 13:59     ` Markus Armbruster
2019-06-02  3:16       ` Hongbo Zhang
2019-06-03 10:54   ` Philippe Mathieu-Daudé
2019-06-16 11:41     ` Hongbo Zhang
2019-06-17 11:08       ` Philippe Mathieu-Daudé
2019-06-17  2:44     ` Hongbo Zhang
2019-04-30 14:18 ` [Qemu-devel] [PATCH v7 0/2] Add Arm SBSA Reference Machine Peter Maydell
2019-04-30 14:18   ` Peter Maydell

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.