[Qemu-devel] [PATCH v2 1/2] hw/arm: check fw_cfg return value before using it

[Qemu-devel] [PATCH v2 1/2] hw/arm: check fw_cfg return value before using it

[Hongbo Zhang]

The fw_cfg value returned from fw_cfg_find() may be NULL, so check it
before using.

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

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index e09201c..43b217f 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -930,6 +930,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
     hwaddr entry;
     static const ARMInsnFixup *primary_loader;
     AddressSpace *as = arm_boot_address_space(cpu, info);
+    FWCfgState *fw_cfg;
 
     /* CPU objects (unlike devices) are not automatically reset on system
      * reset, so we must always register a handler to do so. If we're
@@ -960,11 +961,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
             info->dtb_start = info->loader_start;
         }
 
-        if (info->kernel_filename) {
-            FWCfgState *fw_cfg;
+        fw_cfg = fw_cfg_find();
+        if (info->kernel_filename && fw_cfg) {
             bool try_decompressing_kernel;
 
-            fw_cfg = fw_cfg_find();
             try_decompressing_kernel = arm_feature(&cpu->env,
                                                    ARM_FEATURE_AARCH64);
 
-- 
2.7.4

[Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[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 'Enterprise' with main features:
 - Based on 'virt' machine type.
 - Re-designed memory map.
 - EL2 and EL3 are enabled by default.
 - GIC version 3 by default.
 - AHCI controller attached to system bus, and then CDROM and hard disc
   can be added to it.
 - EHCI controller attached to system bus, with USB mouse and key board
   installed by default.
 - E1000E ethernet card on PCIE bus.
 - VGA display adaptor on PCIE bus.
 - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
 - No virtio functions enabled, since this is to emulate real hardware.
 - No paravirtualized fw_cfg device either.

Arm Trusted Firmware and UEFI porting to this are done accordingly.

Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
---
Changes since v1:
 - rebase on v3.0.0-rc0
 - introduce another auxillary patch as 1/2, so this is 2/2
 - rename 'sbsa' to 'enterprise'
 - remove paravirualized fw_cfg
 - set gic_vertion to 3 instead of 2
 - edit commit message to describe purpose of this platform

 hw/arm/virt-acpi-build.c |  59 +++++++++++++-
 hw/arm/virt.c            | 199 ++++++++++++++++++++++++++++++++++++++++++++++-
 include/hw/arm/virt.h    |   3 +
 3 files changed, 255 insertions(+), 6 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 6ea47e2..212832e 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -84,6 +84,52 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
     aml_append(scope, dev);
 }
 
+static void acpi_dsdt_add_ahci(Aml *scope, const MemMapEntry *ahci_memmap,
+                                           uint32_t ahci_irq)
+{
+    Aml *dev = aml_device("AHC0");
+    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO001E")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
+
+    Aml *crs = aml_resource_template();
+    aml_append(crs, aml_memory32_fixed(ahci_memmap->base,
+                                       ahci_memmap->size, AML_READ_WRITE));
+    aml_append(crs,
+               aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
+                             AML_EXCLUSIVE, &ahci_irq, 1));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+
+    Aml *pkg = aml_package(3);
+    aml_append(pkg, aml_int(0x1));
+    aml_append(pkg, aml_int(0x6));
+    aml_append(pkg, aml_int(0x1));
+
+    /* append the SATA class id */
+    aml_append(dev, aml_name_decl("_CLS", pkg));
+
+    aml_append(scope, dev);
+}
+
+static void acpi_dsdt_add_ehci(Aml *scope, const MemMapEntry *ehci_memmap,
+                                           uint32_t ehci_irq)
+{
+    Aml *dev = aml_device("EHC0");
+    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0D20")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
+
+    Aml *crs = aml_resource_template();
+    aml_append(crs, aml_memory32_fixed(ehci_memmap->base,
+                                       ehci_memmap->size, AML_READ_WRITE));
+    aml_append(crs,
+               aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
+                             AML_EXCLUSIVE, &ehci_irq, 1));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+
+    aml_append(scope, dev);
+}
+
 static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
 {
     Aml *dev = aml_device("FWCF");
@@ -768,14 +814,23 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
                        (irqmap[VIRT_UART] + ARM_SPI_BASE));
     acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
     acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
-    acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
-                    (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
+    if (!vms->enterprise) {
+        acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
+                   (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
+    }
     acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
                       vms->highmem, vms->highmem_ecam);
     acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
                        (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
     acpi_dsdt_add_power_button(scope);
 
+    if (vms->enterprise) {
+        acpi_dsdt_add_ahci(scope, &memmap[VIRT_AHCI],
+                           (irqmap[VIRT_AHCI] + ARM_SPI_BASE));
+        acpi_dsdt_add_ehci(scope, &memmap[VIRT_EHCI],
+                           (irqmap[VIRT_EHCI] + ARM_SPI_BASE));
+    }
+
     aml_append(dsdt, scope);
 
     /* copy AML table into ACPI tables blob and patch header there */
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 281ddcd..498b526 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -59,6 +59,10 @@
 #include "qapi/visitor.h"
 #include "standard-headers/linux/input.h"
 #include "hw/arm/smmuv3.h"
+#include "hw/ide/internal.h"
+#include "hw/ide/ahci_internal.h"
+#include "hw/usb.h"
+#include "qemu/units.h"
 
 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
     static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -94,6 +98,8 @@
 
 #define PLATFORM_BUS_NUM_IRQS 64
 
+#define SATA_NUM_PORTS 6
+
 /* RAM limit in GB. Since VIRT_MEM starts at the 1GB mark, this means
  * RAM can go up to the 256GB mark, leaving 256GB of the physical
  * address space unallocated and free for future use between 256G and 512G.
@@ -168,6 +174,47 @@ static const int a15irqmap[] = {
     [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
 };
 
+static const MemMapEntry enterprise_memmap[] = {
+    /* Space up to 0x8000000 is reserved for a boot ROM */
+    [VIRT_FLASH] =              {          0, 0x08000000 },
+    [VIRT_CPUPERIPHS] =         { 0x08000000, 0x00020000 },
+    /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
+    [VIRT_GIC_DIST] =           { 0x08000000, 0x00010000 },
+    [VIRT_GIC_CPU] =            { 0x08010000, 0x00010000 },
+    [VIRT_GIC_V2M] =            { 0x08020000, 0x00001000 },
+    /* The space in between here is reserved for GICv3 CPU/vCPU/HYP */
+    [VIRT_GIC_ITS] =            { 0x08080000, 0x00020000 },
+    /* This redistributor space allows up to 2*64kB*123 CPUs */
+    [VIRT_GIC_REDIST] =         { 0x080A0000, 0x00F60000 },
+    [VIRT_UART] =               { 0x09000000, 0x00001000 },
+    [VIRT_RTC] =                { 0x09010000, 0x00001000 },
+    [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
+    [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
+    [VIRT_SECURE_UART] =        { 0x09040000, 0x00001000 },
+    [VIRT_AHCI] =               { 0x09050000, 0x00010000 },
+    [VIRT_EHCI] =               { 0x09060000, 0x00010000 },
+    [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
+    [VIRT_SECURE_MEM] =         { 0x0e000000, 0x01000000 },
+    [VIRT_PCIE_MMIO] =          { 0x10000000, 0x7fff0000 },
+    [VIRT_PCIE_PIO] =           { 0x8fff0000, 0x00010000 },
+    [VIRT_PCIE_ECAM] =          { 0x90000000, 0x10000000 },
+    /* Second PCIe window, 508GB wide at the 4GB boundary */
+    [VIRT_PCIE_MMIO_HIGH] =     { 0x100000000ULL, 0x7F00000000ULL },
+    [VIRT_MEM] =                { 0x8000000000ULL, RAMLIMIT_BYTES },
+};
+
+static const int enterprise_irqmap[] = {
+    [VIRT_UART] = 1,
+    [VIRT_RTC] = 2,
+    [VIRT_PCIE] = 3, /* ... to 6 */
+    [VIRT_GPIO] = 7,
+    [VIRT_SECURE_UART] = 8,
+    [VIRT_AHCI] = 9,
+    [VIRT_EHCI] = 10,
+    [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
+    [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
+};
+
 static const char *valid_cpus[] = {
     ARM_CPU_TYPE_NAME("cortex-a15"),
     ARM_CPU_TYPE_NAME("cortex-a53"),
@@ -706,6 +753,72 @@ static void create_rtc(const VirtMachineState *vms, qemu_irq *pic)
     g_free(nodename);
 }
 
+static void create_ahci(const VirtMachineState *vms, qemu_irq *pic)
+{
+    char *nodename;
+    hwaddr base = vms->memmap[VIRT_AHCI].base;
+    hwaddr size = vms->memmap[VIRT_AHCI].size;
+    int irq = vms->irqmap[VIRT_AHCI];
+    const char compat[] = "qemu,mach-virt-ahci\0generic-ahci";
+    DeviceState *dev;
+    DriveInfo *hd[SATA_NUM_PORTS];
+    SysbusAHCIState *sysahci;
+    AHCIState *ahci;
+    int i;
+
+    dev = qdev_create(NULL, "sysbus-ahci");
+    qdev_prop_set_uint32(dev, "num-ports", SATA_NUM_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]);
+    }
+
+    nodename = g_strdup_printf("/sata@%" PRIx64, base);
+    qemu_fdt_add_subnode(vms->fdt, nodename);
+    qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
+                                 2, base, 2, size);
+    qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_SPI, irq,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+    g_free(nodename);
+}
+
+static void create_ehci(const VirtMachineState *vms, qemu_irq *pic)
+{
+    char *nodename;
+    hwaddr base = vms->memmap[VIRT_EHCI].base;
+    hwaddr size = vms->memmap[VIRT_EHCI].size;
+    int irq = vms->irqmap[VIRT_EHCI];
+    const char compat[] = "qemu,mach-virt-ehci\0generic-ehci";
+    USBBus *usb_bus;
+
+    sysbus_create_simple("exynos4210-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");
+
+    nodename = g_strdup_printf("/ehci@%" PRIx64, base);
+    qemu_fdt_add_subnode(vms->fdt, nodename);
+    qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
+                                 2, base, 2, size);
+    qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_SPI, irq,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+    g_free(nodename);
+}
+
 static DeviceState *gpio_key_dev;
 static void virt_powerdown_req(Notifier *n, void *opaque)
 {
@@ -1117,13 +1230,21 @@ static void create_pcie(VirtMachineState *vms, qemu_irq *pic)
             NICInfo *nd = &nd_table[i];
 
             if (!nd->model) {
-                nd->model = g_strdup("virtio");
+                if (vms->enterprise) {
+                    nd->model = g_strdup("e1000e");
+                } else {
+                    nd->model = g_strdup("virtio");
+                }
             }
 
             pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);
         }
     }
 
+    if (vms->enterprise) {
+        pci_create_simple(pci->bus, -1, "VGA");
+    }
+
     nodename = g_strdup_printf("/pcie@%" PRIx64, base);
     qemu_fdt_add_subnode(vms->fdt, nodename);
     qemu_fdt_setprop_string(vms->fdt, nodename,
@@ -1512,14 +1633,21 @@ static void machvirt_init(MachineState *machine)
 
     create_gpio(vms, pic);
 
+    if (vms->enterprise) {
+        create_ahci(vms, pic);
+        create_ehci(vms, pic);
+    }
+
     /* Create mmio transports, so the user can create virtio backends
      * (which will be automatically plugged in to the transports). If
      * no backend is created the transport will just sit harmlessly idle.
      */
-    create_virtio_devices(vms, pic);
+    if (!vms->enterprise) {
+        create_virtio_devices(vms, pic);
 
-    vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);
-    rom_set_fw(vms->fw_cfg);
+        vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);
+        rom_set_fw(vms->fw_cfg);
+    }
 
     create_platform_bus(vms, pic);
 
@@ -1828,6 +1956,7 @@ static void virt_3_0_instance_init(Object *obj)
 
     vms->memmap = a15memmap;
     vms->irqmap = a15irqmap;
+    vms->enterprise = false;
 }
 
 static void virt_machine_3_0_options(MachineClass *mc)
@@ -1960,3 +2089,65 @@ static void virt_machine_2_6_options(MachineClass *mc)
     vmc->no_pmu = true;
 }
 DEFINE_VIRT_MACHINE(2, 6)
+
+static void enterprise_instance_init(Object *obj)
+{
+    VirtMachineState *vms = VIRT_MACHINE(obj);
+
+    vms->secure = true;
+    object_property_add_bool(obj, "secure", virt_get_secure,
+                             virt_set_secure, NULL);
+    object_property_set_description(obj, "secure",
+                                    "Set on/off to enable/disable the ARM "
+                                    "Security Extensions (TrustZone)",
+                                    NULL);
+
+    vms->virt = true;
+    object_property_add_bool(obj, "virtualization", virt_get_virt,
+                             virt_set_virt, NULL);
+    object_property_set_description(obj, "virtualization",
+                                    "Set on/off to enable/disable emulating a "
+                                    "guest CPU which implements the ARM "
+                                    "Virtualization Extensions",
+                                    NULL);
+
+    vms->highmem = true;
+
+    vms->gic_version = 3;
+    object_property_add_str(obj, "gic-version", virt_get_gic_version,
+                        virt_set_gic_version, NULL);
+    object_property_set_description(obj, "gic-version",
+                                    "Set GIC version. "
+                                    "Valid values are 2, 3, host, max", NULL);
+    vms->its = true;
+
+    vms->memmap = enterprise_memmap;
+    vms->irqmap = enterprise_irqmap;
+    vms->enterprise = true;
+}
+
+static void enterprise_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->max_cpus = 246;
+    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a57");
+    mc->block_default_type = IF_IDE;
+    mc->default_ram_size = 1 * GiB;
+    mc->default_cpus = 4;
+    mc->desc = "QEMU 'Enterprise' ARM Virtual Machine";
+}
+
+static const TypeInfo enterprise_info = {
+    .name          = MACHINE_TYPE_NAME("enterprise"),
+    .parent        = TYPE_VIRT_MACHINE,
+    .instance_init = enterprise_instance_init,
+    .class_init    = enterprise_class_init,
+};
+
+static void enterprise_machine_init(void)
+{
+    type_register_static(&enterprise_info);
+}
+
+type_init(enterprise_machine_init);
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 9a870cc..e65d478 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -78,6 +78,8 @@ enum {
     VIRT_GPIO,
     VIRT_SECURE_UART,
     VIRT_SECURE_MEM,
+    VIRT_AHCI,
+    VIRT_EHCI,
 };
 
 typedef enum VirtIOMMUType {
@@ -124,6 +126,7 @@ typedef struct {
     uint32_t msi_phandle;
     uint32_t iommu_phandle;
     int psci_conduit;
+    bool enterprise;
 } VirtMachineState;
 
 #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Shannon Zhao]

Hi Hongbo,

On 2018/7/25 13:30, Hongbo Zhang 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 'Enterprise' with main features:
>  - Based on 'virt' machine type.
>  - Re-designed memory map.
>  - EL2 and EL3 are enabled by default.
>  - GIC version 3 by default.
>  - AHCI controller attached to system bus, and then CDROM and hard disc
>    can be added to it.
>  - EHCI controller attached to system bus, with USB mouse and key board
>    installed by default.
>  - E1000E ethernet card on PCIE bus.
>  - VGA display adaptor on PCIE bus.
>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>  - No virtio functions enabled, since this is to emulate real hardware.
>  - No paravirtualized fw_cfg device either.
> 
Does it support run on KVM or just TCG?

> Arm Trusted Firmware and UEFI porting to this are done accordingly.
> 
> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> ---
> Changes since v1:
>  - rebase on v3.0.0-rc0
>  - introduce another auxillary patch as 1/2, so this is 2/2
>  - rename 'sbsa' to 'enterprise'
>  - remove paravirualized fw_cfg
>  - set gic_vertion to 3 instead of 2
>  - edit commit message to describe purpose of this platform
> 
>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-
>  hw/arm/virt.c            | 199 ++++++++++++++++++++++++++++++++++++++++++++++-
>  include/hw/arm/virt.h    |   3 +
>  3 files changed, 255 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 6ea47e2..212832e 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -84,6 +84,52 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>      aml_append(scope, dev);
>  }
>  
> +static void acpi_dsdt_add_ahci(Aml *scope, const MemMapEntry *ahci_memmap,
> +                                           uint32_t ahci_irq)
> +{
> +    Aml *dev = aml_device("AHC0");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO001E")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +
> +    Aml *crs = aml_resource_template();
> +    aml_append(crs, aml_memory32_fixed(ahci_memmap->base,
> +                                       ahci_memmap->size, AML_READ_WRITE));
> +    aml_append(crs,
> +               aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> +                             AML_EXCLUSIVE, &ahci_irq, 1));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +    Aml *pkg = aml_package(3);
> +    aml_append(pkg, aml_int(0x1));
> +    aml_append(pkg, aml_int(0x6));
> +    aml_append(pkg, aml_int(0x1));
> +
> +    /* append the SATA class id */
> +    aml_append(dev, aml_name_decl("_CLS", pkg));
> +
> +    aml_append(scope, dev);
> +}
> +
> +static void acpi_dsdt_add_ehci(Aml *scope, const MemMapEntry *ehci_memmap,
> +                                           uint32_t ehci_irq)
> +{
> +    Aml *dev = aml_device("EHC0");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0D20")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +
> +    Aml *crs = aml_resource_template();
> +    aml_append(crs, aml_memory32_fixed(ehci_memmap->base,
> +                                       ehci_memmap->size, AML_READ_WRITE));
> +    aml_append(crs,
> +               aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> +                             AML_EXCLUSIVE, &ehci_irq, 1));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +    aml_append(scope, dev);
> +}
> +
>  static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
>  {
>      Aml *dev = aml_device("FWCF");
> @@ -768,14 +814,23 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>                         (irqmap[VIRT_UART] + ARM_SPI_BASE));
>      acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
>      acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
As you don't create the fw_cfg, no need to add it ACPI here.

> -    acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> -                    (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
> +    if (!vms->enterprise) {
> +        acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> +                   (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
> +    }
>      acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
>                        vms->highmem, vms->highmem_ecam);
>      acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
>                         (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
>      acpi_dsdt_add_power_button(scope);
>  
> +    if (vms->enterprise) {
> +        acpi_dsdt_add_ahci(scope, &memmap[VIRT_AHCI],
> +                           (irqmap[VIRT_AHCI] + ARM_SPI_BASE));
> +        acpi_dsdt_add_ehci(scope, &memmap[VIRT_EHCI],
> +                           (irqmap[VIRT_EHCI] + ARM_SPI_BASE));
> +    }
> +
>      aml_append(dsdt, scope);
>  
>      /* copy AML table into ACPI tables blob and patch header there */
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 281ddcd..498b526 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -59,6 +59,10 @@
>  #include "qapi/visitor.h"
>  #include "standard-headers/linux/input.h"
>  #include "hw/arm/smmuv3.h"
> +#include "hw/ide/internal.h"
> +#include "hw/ide/ahci_internal.h"
> +#include "hw/usb.h"
> +#include "qemu/units.h"
>  
>  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>      static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
> @@ -94,6 +98,8 @@
>  
>  #define PLATFORM_BUS_NUM_IRQS 64
>  
> +#define SATA_NUM_PORTS 6
> +
>  /* RAM limit in GB. Since VIRT_MEM starts at the 1GB mark, this means
>   * RAM can go up to the 256GB mark, leaving 256GB of the physical
>   * address space unallocated and free for future use between 256G and 512G.
> @@ -168,6 +174,47 @@ static const int a15irqmap[] = {
>      [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
>  };
>  
> +static const MemMapEntry enterprise_memmap[] = {
> +    /* Space up to 0x8000000 is reserved for a boot ROM */
> +    [VIRT_FLASH] =              {          0, 0x08000000 },
> +    [VIRT_CPUPERIPHS] =         { 0x08000000, 0x00020000 },
> +    /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
> +    [VIRT_GIC_DIST] =           { 0x08000000, 0x00010000 },
> +    [VIRT_GIC_CPU] =            { 0x08010000, 0x00010000 },
> +    [VIRT_GIC_V2M] =            { 0x08020000, 0x00001000 },
> +    /* The space in between here is reserved for GICv3 CPU/vCPU/HYP */
> +    [VIRT_GIC_ITS] =            { 0x08080000, 0x00020000 },
> +    /* This redistributor space allows up to 2*64kB*123 CPUs */
> +    [VIRT_GIC_REDIST] =         { 0x080A0000, 0x00F60000 },
> +    [VIRT_UART] =               { 0x09000000, 0x00001000 },
> +    [VIRT_RTC] =                { 0x09010000, 0x00001000 },
> +    [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
no need to reserve mmio for fw_cfg.

> +    [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
> +    [VIRT_SECURE_UART] =        { 0x09040000, 0x00001000 },
> +    [VIRT_AHCI] =               { 0x09050000, 0x00010000 },
> +    [VIRT_EHCI] =               { 0x09060000, 0x00010000 },
Between VIRT_EHCI and VIRT_PLATFORM_BUS, there are still some spaces,
can we reuse it?

Also, is it necessary to create VIRT_PLATFORM_BUS?

> +    [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
> +    [VIRT_SECURE_MEM] =         { 0x0e000000, 0x01000000 },
> +    [VIRT_PCIE_MMIO] =          { 0x10000000, 0x7fff0000 },
> +    [VIRT_PCIE_PIO] =           { 0x8fff0000, 0x00010000 },
> +    [VIRT_PCIE_ECAM] =          { 0x90000000, 0x10000000 },
> +    /* Second PCIe window, 508GB wide at the 4GB boundary */
> +    [VIRT_PCIE_MMIO_HIGH] =     { 0x100000000ULL, 0x7F00000000ULL },
> +    [VIRT_MEM] =                { 0x8000000000ULL, RAMLIMIT_BYTES },
> +};
> +
> +static const int enterprise_irqmap[] = {
> +    [VIRT_UART] = 1,
> +    [VIRT_RTC] = 2,
> +    [VIRT_PCIE] = 3, /* ... to 6 */
> +    [VIRT_GPIO] = 7,
> +    [VIRT_SECURE_UART] = 8,
> +    [VIRT_AHCI] = 9,
> +    [VIRT_EHCI] = 10,
> +    [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
> +    [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
> +};
> +
>  static const char *valid_cpus[] = {
>      ARM_CPU_TYPE_NAME("cortex-a15"),
>      ARM_CPU_TYPE_NAME("cortex-a53"),
> @@ -706,6 +753,72 @@ static void create_rtc(const VirtMachineState *vms, qemu_irq *pic)
>      g_free(nodename);
>  }
>  
> +static void create_ahci(const VirtMachineState *vms, qemu_irq *pic)
> +{
> +    char *nodename;
> +    hwaddr base = vms->memmap[VIRT_AHCI].base;
> +    hwaddr size = vms->memmap[VIRT_AHCI].size;
> +    int irq = vms->irqmap[VIRT_AHCI];
> +    const char compat[] = "qemu,mach-virt-ahci\0generic-ahci";
> +    DeviceState *dev;
> +    DriveInfo *hd[SATA_NUM_PORTS];
> +    SysbusAHCIState *sysahci;
> +    AHCIState *ahci;
> +    int i;
> +
> +    dev = qdev_create(NULL, "sysbus-ahci");
> +    qdev_prop_set_uint32(dev, "num-ports", SATA_NUM_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]);
> +    }
> +
> +    nodename = g_strdup_printf("/sata@%" PRIx64, base);
> +    qemu_fdt_add_subnode(vms->fdt, nodename);
> +    qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat, sizeof(compat));
> +    qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
> +                                 2, base, 2, size);
> +    qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
> +                           GIC_FDT_IRQ_TYPE_SPI, irq,
> +                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> +    g_free(nodename);
> +}
> +
> +static void create_ehci(const VirtMachineState *vms, qemu_irq *pic)
> +{
> +    char *nodename;
> +    hwaddr base = vms->memmap[VIRT_EHCI].base;
> +    hwaddr size = vms->memmap[VIRT_EHCI].size;
> +    int irq = vms->irqmap[VIRT_EHCI];
> +    const char compat[] = "qemu,mach-virt-ehci\0generic-ehci";
> +    USBBus *usb_bus;
> +
> +    sysbus_create_simple("exynos4210-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");
> +
> +    nodename = g_strdup_printf("/ehci@%" PRIx64, base);
> +    qemu_fdt_add_subnode(vms->fdt, nodename);
> +    qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat, sizeof(compat));
> +    qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
> +                                 2, base, 2, size);
> +    qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
> +                           GIC_FDT_IRQ_TYPE_SPI, irq,
> +                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> +    g_free(nodename);
> +}
> +
>  static DeviceState *gpio_key_dev;
>  static void virt_powerdown_req(Notifier *n, void *opaque)
>  {
> @@ -1117,13 +1230,21 @@ static void create_pcie(VirtMachineState *vms, qemu_irq *pic)
>              NICInfo *nd = &nd_table[i];
>  
>              if (!nd->model) {
> -                nd->model = g_strdup("virtio");
> +                if (vms->enterprise) {
> +                    nd->model = g_strdup("e1000e");
> +                } else {
> +                    nd->model = g_strdup("virtio");
> +                }
>              }
>  
>              pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);
>          }
>      }
>  
> +    if (vms->enterprise) {
> +        pci_create_simple(pci->bus, -1, "VGA");
> +    }
> +
>      nodename = g_strdup_printf("/pcie@%" PRIx64, base);
>      qemu_fdt_add_subnode(vms->fdt, nodename);
>      qemu_fdt_setprop_string(vms->fdt, nodename,
> @@ -1512,14 +1633,21 @@ static void machvirt_init(MachineState *machine)
>  
>      create_gpio(vms, pic);
>  
> +    if (vms->enterprise) {
> +        create_ahci(vms, pic);
> +        create_ehci(vms, pic);
> +    }
> +
>      /* Create mmio transports, so the user can create virtio backends
>       * (which will be automatically plugged in to the transports). If
>       * no backend is created the transport will just sit harmlessly idle.
>       */
> -    create_virtio_devices(vms, pic);
> +    if (!vms->enterprise) {
> +        create_virtio_devices(vms, pic);
>  
> -    vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);
> -    rom_set_fw(vms->fw_cfg);
> +        vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);
> +        rom_set_fw(vms->fw_cfg);
> +    }
>  
>      create_platform_bus(vms, pic);
>  
> @@ -1828,6 +1956,7 @@ static void virt_3_0_instance_init(Object *obj)
>  
>      vms->memmap = a15memmap;
>      vms->irqmap = a15irqmap;
> +    vms->enterprise = false;
>  }
>  
>  static void virt_machine_3_0_options(MachineClass *mc)
> @@ -1960,3 +2089,65 @@ static void virt_machine_2_6_options(MachineClass *mc)
>      vmc->no_pmu = true;
>  }
>  DEFINE_VIRT_MACHINE(2, 6)
> +
> +static void enterprise_instance_init(Object *obj)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +    vms->secure = true;
> +    object_property_add_bool(obj, "secure", virt_get_secure,
> +                             virt_set_secure, NULL);
> +    object_property_set_description(obj, "secure",
> +                                    "Set on/off to enable/disable the ARM "
> +                                    "Security Extensions (TrustZone)",
> +                                    NULL);
> +
> +    vms->virt = true;
> +    object_property_add_bool(obj, "virtualization", virt_get_virt,
> +                             virt_set_virt, NULL);
> +    object_property_set_description(obj, "virtualization",
> +                                    "Set on/off to enable/disable emulating a "
> +                                    "guest CPU which implements the ARM "
> +                                    "Virtualization Extensions",
> +                                    NULL);
> +
> +    vms->highmem = true;
> +
> +    vms->gic_version = 3;
> +    object_property_add_str(obj, "gic-version", virt_get_gic_version,
> +                        virt_set_gic_version, NULL);
> +    object_property_set_description(obj, "gic-version",
> +                                    "Set GIC version. "
> +                                    "Valid values are 2, 3, host, max", NULL);
> +    vms->its = true;
> +
> +    vms->memmap = enterprise_memmap;
> +    vms->irqmap = enterprise_irqmap;
> +    vms->enterprise = true;
> +}
> +
> +static void enterprise_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    mc->max_cpus = 246;
Why 246?

> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a57");
> +    mc->block_default_type = IF_IDE;
> +    mc->default_ram_size = 1 * GiB;
> +    mc->default_cpus = 4;
> +    mc->desc = "QEMU 'Enterprise' ARM Virtual Machine";
> +}
> +
> +static const TypeInfo enterprise_info = {
> +    .name          = MACHINE_TYPE_NAME("enterprise"),
> +    .parent        = TYPE_VIRT_MACHINE,
> +    .instance_init = enterprise_instance_init,
> +    .class_init    = enterprise_class_init,
> +};
> +
> +static void enterprise_machine_init(void)
> +{
> +    type_register_static(&enterprise_info);
> +}
> +
> +type_init(enterprise_machine_init);
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 9a870cc..e65d478 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -78,6 +78,8 @@ enum {
>      VIRT_GPIO,
>      VIRT_SECURE_UART,
>      VIRT_SECURE_MEM,
> +    VIRT_AHCI,
> +    VIRT_EHCI,
>  };
>  
>  typedef enum VirtIOMMUType {
> @@ -124,6 +126,7 @@ typedef struct {
>      uint32_t msi_phandle;
>      uint32_t iommu_phandle;
>      int psci_conduit;
> +    bool enterprise;
>  } VirtMachineState;
>  
>  #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)
> 

-- 
Shannon

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 15:20, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> Hi Hongbo,
>
Hi Shannon,

> On 2018/7/25 13:30, Hongbo Zhang 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 'Enterprise' with main features:
>>  - Based on 'virt' machine type.
>>  - Re-designed memory map.
>>  - EL2 and EL3 are enabled by default.
>>  - GIC version 3 by default.
>>  - AHCI controller attached to system bus, and then CDROM and hard disc
>>    can be added to it.
>>  - EHCI controller attached to system bus, with USB mouse and key board
>>    installed by default.
>>  - E1000E ethernet card on PCIE bus.
>>  - VGA display adaptor on PCIE bus.
>>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>>  - No virtio functions enabled, since this is to emulate real hardware.
>>  - No paravirtualized fw_cfg device either.
>>
> Does it support run on KVM or just TCG?
>
TCG is suggested to be used, but KVM isn't disabled now, it is up to
the user to use it or not.
(I would like to run some SBSA/SBBR test suite to see if there is some
difference later)

>> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>>
>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>> ---
>> Changes since v1:
>>  - rebase on v3.0.0-rc0
>>  - introduce another auxillary patch as 1/2, so this is 2/2
>>  - rename 'sbsa' to 'enterprise'
>>  - remove paravirualized fw_cfg
>>  - set gic_vertion to 3 instead of 2
>>  - edit commit message to describe purpose of this platform
>>
>>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-
>>  hw/arm/virt.c            | 199 ++++++++++++++++++++++++++++++++++++++++++++++-
>>  include/hw/arm/virt.h    |   3 +
>>  3 files changed, 255 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 6ea47e2..212832e 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -84,6 +84,52 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>>      aml_append(scope, dev);
>>  }
>>
>> +static void acpi_dsdt_add_ahci(Aml *scope, const MemMapEntry *ahci_memmap,
>> +                                           uint32_t ahci_irq)
>> +{
>> +    Aml *dev = aml_device("AHC0");
>> +    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO001E")));
>> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>> +    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>> +
>> +    Aml *crs = aml_resource_template();
>> +    aml_append(crs, aml_memory32_fixed(ahci_memmap->base,
>> +                                       ahci_memmap->size, AML_READ_WRITE));
>> +    aml_append(crs,
>> +               aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
>> +                             AML_EXCLUSIVE, &ahci_irq, 1));
>> +    aml_append(dev, aml_name_decl("_CRS", crs));
>> +
>> +    Aml *pkg = aml_package(3);
>> +    aml_append(pkg, aml_int(0x1));
>> +    aml_append(pkg, aml_int(0x6));
>> +    aml_append(pkg, aml_int(0x1));
>> +
>> +    /* append the SATA class id */
>> +    aml_append(dev, aml_name_decl("_CLS", pkg));
>> +
>> +    aml_append(scope, dev);
>> +}
>> +
>> +static void acpi_dsdt_add_ehci(Aml *scope, const MemMapEntry *ehci_memmap,
>> +                                           uint32_t ehci_irq)
>> +{
>> +    Aml *dev = aml_device("EHC0");
>> +    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0D20")));
>> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>> +    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
>> +
>> +    Aml *crs = aml_resource_template();
>> +    aml_append(crs, aml_memory32_fixed(ehci_memmap->base,
>> +                                       ehci_memmap->size, AML_READ_WRITE));
>> +    aml_append(crs,
>> +               aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
>> +                             AML_EXCLUSIVE, &ehci_irq, 1));
>> +    aml_append(dev, aml_name_decl("_CRS", crs));
>> +
>> +    aml_append(scope, dev);
>> +}
>> +
>>  static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
>>  {
>>      Aml *dev = aml_device("FWCF");
>> @@ -768,14 +814,23 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>                         (irqmap[VIRT_UART] + ARM_SPI_BASE));
>>      acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
>>      acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
> As you don't create the fw_cfg, no need to add it ACPI here.
>
Forgot this in this v2 patch, will move the if (!vms->enterprise) to include it.

>> -    acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
>> -                    (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
>> +    if (!vms->enterprise) {
>> +        acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
>> +                   (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
>> +    }
>>      acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),
>>                        vms->highmem, vms->highmem_ecam);
>>      acpi_dsdt_add_gpio(scope, &memmap[VIRT_GPIO],
>>                         (irqmap[VIRT_GPIO] + ARM_SPI_BASE));
>>      acpi_dsdt_add_power_button(scope);
>>
>> +    if (vms->enterprise) {
>> +        acpi_dsdt_add_ahci(scope, &memmap[VIRT_AHCI],
>> +                           (irqmap[VIRT_AHCI] + ARM_SPI_BASE));
>> +        acpi_dsdt_add_ehci(scope, &memmap[VIRT_EHCI],
>> +                           (irqmap[VIRT_EHCI] + ARM_SPI_BASE));
>> +    }
>> +
>>      aml_append(dsdt, scope);
>>
>>      /* copy AML table into ACPI tables blob and patch header there */
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 281ddcd..498b526 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -59,6 +59,10 @@
>>  #include "qapi/visitor.h"
>>  #include "standard-headers/linux/input.h"
>>  #include "hw/arm/smmuv3.h"
>> +#include "hw/ide/internal.h"
>> +#include "hw/ide/ahci_internal.h"
>> +#include "hw/usb.h"
>> +#include "qemu/units.h"
>>
>>  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>>      static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
>> @@ -94,6 +98,8 @@
>>
>>  #define PLATFORM_BUS_NUM_IRQS 64
>>
>> +#define SATA_NUM_PORTS 6
>> +
>>  /* RAM limit in GB. Since VIRT_MEM starts at the 1GB mark, this means
>>   * RAM can go up to the 256GB mark, leaving 256GB of the physical
>>   * address space unallocated and free for future use between 256G and 512G.
>> @@ -168,6 +174,47 @@ static const int a15irqmap[] = {
>>      [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
>>  };
>>
>> +static const MemMapEntry enterprise_memmap[] = {
>> +    /* Space up to 0x8000000 is reserved for a boot ROM */
>> +    [VIRT_FLASH] =              {          0, 0x08000000 },
>> +    [VIRT_CPUPERIPHS] =         { 0x08000000, 0x00020000 },
>> +    /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
>> +    [VIRT_GIC_DIST] =           { 0x08000000, 0x00010000 },
>> +    [VIRT_GIC_CPU] =            { 0x08010000, 0x00010000 },
>> +    [VIRT_GIC_V2M] =            { 0x08020000, 0x00001000 },
>> +    /* The space in between here is reserved for GICv3 CPU/vCPU/HYP */
>> +    [VIRT_GIC_ITS] =            { 0x08080000, 0x00020000 },
>> +    /* This redistributor space allows up to 2*64kB*123 CPUs */
>> +    [VIRT_GIC_REDIST] =         { 0x080A0000, 0x00F60000 },
>> +    [VIRT_UART] =               { 0x09000000, 0x00001000 },
>> +    [VIRT_RTC] =                { 0x09010000, 0x00001000 },
>> +    [VIRT_FW_CFG] =             { 0x09020000, 0x00000018 },
> no need to reserve mmio for fw_cfg.
>
Right, payed much attention to verify patch 1/2 and ignored here :(

>> +    [VIRT_GPIO] =               { 0x09030000, 0x00001000 },
>> +    [VIRT_SECURE_UART] =        { 0x09040000, 0x00001000 },
>> +    [VIRT_AHCI] =               { 0x09050000, 0x00010000 },
>> +    [VIRT_EHCI] =               { 0x09060000, 0x00010000 },
> Between VIRT_EHCI and VIRT_PLATFORM_BUS, there are still some spaces,
> can we reuse it?
>
This derives from virt machine a15memmap, and I see even the a15memmap
has some blank spaces, so I just simply leave spaces as well.

> Also, is it necessary to create VIRT_PLATFORM_BUS?
>
Good catch, not necessary to keep it, I've tested that qemu works fine
without it.

>> +    [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
>> +    [VIRT_SECURE_MEM] =         { 0x0e000000, 0x01000000 },
>> +    [VIRT_PCIE_MMIO] =          { 0x10000000, 0x7fff0000 },
>> +    [VIRT_PCIE_PIO] =           { 0x8fff0000, 0x00010000 },
>> +    [VIRT_PCIE_ECAM] =          { 0x90000000, 0x10000000 },
>> +    /* Second PCIe window, 508GB wide at the 4GB boundary */
>> +    [VIRT_PCIE_MMIO_HIGH] =     { 0x100000000ULL, 0x7F00000000ULL },
>> +    [VIRT_MEM] =                { 0x8000000000ULL, RAMLIMIT_BYTES },
>> +};
>> +
>> +static const int enterprise_irqmap[] = {
>> +    [VIRT_UART] = 1,
>> +    [VIRT_RTC] = 2,
>> +    [VIRT_PCIE] = 3, /* ... to 6 */
>> +    [VIRT_GPIO] = 7,
>> +    [VIRT_SECURE_UART] = 8,
>> +    [VIRT_AHCI] = 9,
>> +    [VIRT_EHCI] = 10,
>> +    [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
>> +    [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
>> +};
>> +
>>  static const char *valid_cpus[] = {
>>      ARM_CPU_TYPE_NAME("cortex-a15"),
>>      ARM_CPU_TYPE_NAME("cortex-a53"),
>> @@ -706,6 +753,72 @@ static void create_rtc(const VirtMachineState *vms, qemu_irq *pic)
>>      g_free(nodename);
>>  }
>>
>> +static void create_ahci(const VirtMachineState *vms, qemu_irq *pic)
>> +{
>> +    char *nodename;
>> +    hwaddr base = vms->memmap[VIRT_AHCI].base;
>> +    hwaddr size = vms->memmap[VIRT_AHCI].size;
>> +    int irq = vms->irqmap[VIRT_AHCI];
>> +    const char compat[] = "qemu,mach-virt-ahci\0generic-ahci";
>> +    DeviceState *dev;
>> +    DriveInfo *hd[SATA_NUM_PORTS];
>> +    SysbusAHCIState *sysahci;
>> +    AHCIState *ahci;
>> +    int i;
>> +
>> +    dev = qdev_create(NULL, "sysbus-ahci");
>> +    qdev_prop_set_uint32(dev, "num-ports", SATA_NUM_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]);
>> +    }
>> +
>> +    nodename = g_strdup_printf("/sata@%" PRIx64, base);
>> +    qemu_fdt_add_subnode(vms->fdt, nodename);
>> +    qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat, sizeof(compat));
>> +    qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
>> +                                 2, base, 2, size);
>> +    qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
>> +                           GIC_FDT_IRQ_TYPE_SPI, irq,
>> +                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
>> +    g_free(nodename);
>> +}
>> +
>> +static void create_ehci(const VirtMachineState *vms, qemu_irq *pic)
>> +{
>> +    char *nodename;
>> +    hwaddr base = vms->memmap[VIRT_EHCI].base;
>> +    hwaddr size = vms->memmap[VIRT_EHCI].size;
>> +    int irq = vms->irqmap[VIRT_EHCI];
>> +    const char compat[] = "qemu,mach-virt-ehci\0generic-ehci";
>> +    USBBus *usb_bus;
>> +
>> +    sysbus_create_simple("exynos4210-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");
>> +
>> +    nodename = g_strdup_printf("/ehci@%" PRIx64, base);
>> +    qemu_fdt_add_subnode(vms->fdt, nodename);
>> +    qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat, sizeof(compat));
>> +    qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
>> +                                 2, base, 2, size);
>> +    qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
>> +                           GIC_FDT_IRQ_TYPE_SPI, irq,
>> +                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
>> +    g_free(nodename);
>> +}
>> +
>>  static DeviceState *gpio_key_dev;
>>  static void virt_powerdown_req(Notifier *n, void *opaque)
>>  {
>> @@ -1117,13 +1230,21 @@ static void create_pcie(VirtMachineState *vms, qemu_irq *pic)
>>              NICInfo *nd = &nd_table[i];
>>
>>              if (!nd->model) {
>> -                nd->model = g_strdup("virtio");
>> +                if (vms->enterprise) {
>> +                    nd->model = g_strdup("e1000e");
>> +                } else {
>> +                    nd->model = g_strdup("virtio");
>> +                }
>>              }
>>
>>              pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);
>>          }
>>      }
>>
>> +    if (vms->enterprise) {
>> +        pci_create_simple(pci->bus, -1, "VGA");
>> +    }
>> +
>>      nodename = g_strdup_printf("/pcie@%" PRIx64, base);
>>      qemu_fdt_add_subnode(vms->fdt, nodename);
>>      qemu_fdt_setprop_string(vms->fdt, nodename,
>> @@ -1512,14 +1633,21 @@ static void machvirt_init(MachineState *machine)
>>
>>      create_gpio(vms, pic);
>>
>> +    if (vms->enterprise) {
>> +        create_ahci(vms, pic);
>> +        create_ehci(vms, pic);
>> +    }
>> +
>>      /* Create mmio transports, so the user can create virtio backends
>>       * (which will be automatically plugged in to the transports). If
>>       * no backend is created the transport will just sit harmlessly idle.
>>       */
>> -    create_virtio_devices(vms, pic);
>> +    if (!vms->enterprise) {
>> +        create_virtio_devices(vms, pic);
>>
>> -    vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);
>> -    rom_set_fw(vms->fw_cfg);
>> +        vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);
>> +        rom_set_fw(vms->fw_cfg);
>> +    }
>>
>>      create_platform_bus(vms, pic);
>>
>> @@ -1828,6 +1956,7 @@ static void virt_3_0_instance_init(Object *obj)
>>
>>      vms->memmap = a15memmap;
>>      vms->irqmap = a15irqmap;
>> +    vms->enterprise = false;
>>  }
>>
>>  static void virt_machine_3_0_options(MachineClass *mc)
>> @@ -1960,3 +2089,65 @@ static void virt_machine_2_6_options(MachineClass *mc)
>>      vmc->no_pmu = true;
>>  }
>>  DEFINE_VIRT_MACHINE(2, 6)
>> +
>> +static void enterprise_instance_init(Object *obj)
>> +{
>> +    VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> +    vms->secure = true;
>> +    object_property_add_bool(obj, "secure", virt_get_secure,
>> +                             virt_set_secure, NULL);
>> +    object_property_set_description(obj, "secure",
>> +                                    "Set on/off to enable/disable the ARM "
>> +                                    "Security Extensions (TrustZone)",
>> +                                    NULL);
>> +
>> +    vms->virt = true;
>> +    object_property_add_bool(obj, "virtualization", virt_get_virt,
>> +                             virt_set_virt, NULL);
>> +    object_property_set_description(obj, "virtualization",
>> +                                    "Set on/off to enable/disable emulating a "
>> +                                    "guest CPU which implements the ARM "
>> +                                    "Virtualization Extensions",
>> +                                    NULL);
>> +
>> +    vms->highmem = true;
>> +
>> +    vms->gic_version = 3;
>> +    object_property_add_str(obj, "gic-version", virt_get_gic_version,
>> +                        virt_set_gic_version, NULL);
>> +    object_property_set_description(obj, "gic-version",
>> +                                    "Set GIC version. "
>> +                                    "Valid values are 2, 3, host, max", NULL);
>> +    vms->its = true;
>> +
>> +    vms->memmap = enterprise_memmap;
>> +    vms->irqmap = enterprise_irqmap;
>> +    vms->enterprise = true;
>> +}
>> +
>> +static void enterprise_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> +    mc->max_cpus = 246;
> Why 246?
>
This depends on the GIC redistributor space size.
I didn't change the size from previous a15memmap, each CPU core needs
64k memory size, and the total GIC redistributor can support 246 cores
at max, you can also see the comment in a15memmap source code.
(this only applies for GICv3, for the GICv2, max CPU number is much
less, and there is a problem of emulating MPIDR, I will send patch to
fix it if I have time )

>> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a57");
>> +    mc->block_default_type = IF_IDE;
>> +    mc->default_ram_size = 1 * GiB;
>> +    mc->default_cpus = 4;
>> +    mc->desc = "QEMU 'Enterprise' ARM Virtual Machine";
>> +}
>> +
>> +static const TypeInfo enterprise_info = {
>> +    .name          = MACHINE_TYPE_NAME("enterprise"),
>> +    .parent        = TYPE_VIRT_MACHINE,
>> +    .instance_init = enterprise_instance_init,
>> +    .class_init    = enterprise_class_init,
>> +};
>> +
>> +static void enterprise_machine_init(void)
>> +{
>> +    type_register_static(&enterprise_info);
>> +}
>> +
>> +type_init(enterprise_machine_init);
>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>> index 9a870cc..e65d478 100644
>> --- a/include/hw/arm/virt.h
>> +++ b/include/hw/arm/virt.h
>> @@ -78,6 +78,8 @@ enum {
>>      VIRT_GPIO,
>>      VIRT_SECURE_UART,
>>      VIRT_SECURE_MEM,
>> +    VIRT_AHCI,
>> +    VIRT_EHCI,
>>  };
>>
>>  typedef enum VirtIOMMUType {
>> @@ -124,6 +126,7 @@ typedef struct {
>>      uint32_t msi_phandle;
>>      uint32_t iommu_phandle;
>>      int psci_conduit;
>> +    bool enterprise;
>>  } VirtMachineState;
>>
>>  #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)
>>
>
> --
> Shannon
>

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Daniel P. Berrangé]

On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:

The 'enterprise' name is really awful - this is essentially a marketing
term completely devoid of any useful meaning.

You had previously called this "sbsa" which IIUC was related to a real
world hardware specification that it was based on. IOW, I think this old
name was preferrable to calling it "enterprise".

>  - Based on 'virt' machine type.
>  - Re-designed memory map.
>  - EL2 and EL3 are enabled by default.
>  - GIC version 3 by default.
>  - AHCI controller attached to system bus, and then CDROM and hard disc
>    can be added to it.
>  - EHCI controller attached to system bus, with USB mouse and key board
>    installed by default.
>  - E1000E ethernet card on PCIE bus.
>  - VGA display adaptor on PCIE bus.
>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>  - No virtio functions enabled, since this is to emulate real hardware.
>  - No paravirtualized fw_cfg device either.
> 
> Arm Trusted Firmware and UEFI porting to this are done accordingly.
> 
> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> ---
> Changes since v1:
>  - rebase on v3.0.0-rc0
>  - introduce another auxillary patch as 1/2, so this is 2/2
>  - rename 'sbsa' to 'enterprise'
>  - remove paravirualized fw_cfg
>  - set gic_vertion to 3 instead of 2
>  - edit commit message to describe purpose of this platform
> 
>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-
>  hw/arm/virt.c            | 199 ++++++++++++++++++++++++++++++++++++++++++++++-
>  include/hw/arm/virt.h    |   3 +
>  3 files changed, 255 insertions(+), 6 deletions(-)

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 25 July 2018 at 10:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>
> The 'enterprise' name is really awful - this is essentially a marketing
> term completely devoid of any useful meaning.
>
> You had previously called this "sbsa" which IIUC was related to a real
> world hardware specification that it was based on. IOW, I think this old
> name was preferrable to calling it "enterprise".
>

I couldn't agree more. However, IIUC this change was made at the
request of one of the reviewers, although I wasn't part of the
discussion at that point, so I'm not sure who it was.

Hongbo, could you please share a link to that discussion?

Thanks,
Ard.

>>  - Based on 'virt' machine type.
>>  - Re-designed memory map.
>>  - EL2 and EL3 are enabled by default.
>>  - GIC version 3 by default.
>>  - AHCI controller attached to system bus, and then CDROM and hard disc
>>    can be added to it.
>>  - EHCI controller attached to system bus, with USB mouse and key board
>>    installed by default.
>>  - E1000E ethernet card on PCIE bus.
>>  - VGA display adaptor on PCIE bus.
>>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>>  - No virtio functions enabled, since this is to emulate real hardware.
>>  - No paravirtualized fw_cfg device either.
>>
>> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>>
>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>> ---
>> Changes since v1:
>>  - rebase on v3.0.0-rc0
>>  - introduce another auxillary patch as 1/2, so this is 2/2
>>  - rename 'sbsa' to 'enterprise'
>>  - remove paravirualized fw_cfg
>>  - set gic_vertion to 3 instead of 2
>>  - edit commit message to describe purpose of this platform
>>
>>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-
>>  hw/arm/virt.c            | 199 ++++++++++++++++++++++++++++++++++++++++++++++-
>>  include/hw/arm/virt.h    |   3 +
>>  3 files changed, 255 insertions(+), 6 deletions(-)
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 17:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 25 July 2018 at 10:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>>
>> The 'enterprise' name is really awful - this is essentially a marketing
>> term completely devoid of any useful meaning.
>>
>> You had previously called this "sbsa" which IIUC was related to a real
>> world hardware specification that it was based on. IOW, I think this old
>> name was preferrable to calling it "enterprise".
>>
>
> I couldn't agree more. However, IIUC this change was made at the
> request of one of the reviewers, although I wasn't part of the
> discussion at that point, so I'm not sure who it was.
>
> Hongbo, could you please share a link to that discussion?
>
> Thanks,
> Ard.
>

V1 discussion here:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg545775.html


>>>  - Based on 'virt' machine type.
>>>  - Re-designed memory map.
>>>  - EL2 and EL3 are enabled by default.
>>>  - GIC version 3 by default.
>>>  - AHCI controller attached to system bus, and then CDROM and hard disc
>>>    can be added to it.
>>>  - EHCI controller attached to system bus, with USB mouse and key board
>>>    installed by default.
>>>  - E1000E ethernet card on PCIE bus.
>>>  - VGA display adaptor on PCIE bus.
>>>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>>>  - No virtio functions enabled, since this is to emulate real hardware.
>>>  - No paravirtualized fw_cfg device either.
>>>
>>> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>>>
>>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>>> ---
>>> Changes since v1:
>>>  - rebase on v3.0.0-rc0
>>>  - introduce another auxillary patch as 1/2, so this is 2/2
>>>  - rename 'sbsa' to 'enterprise'
>>>  - remove paravirualized fw_cfg
>>>  - set gic_vertion to 3 instead of 2
>>>  - edit commit message to describe purpose of this platform
>>>
>>>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-
>>>  hw/arm/virt.c            | 199 ++++++++++++++++++++++++++++++++++++++++++++++-
>>>  include/hw/arm/virt.h    |   3 +
>>>  3 files changed, 255 insertions(+), 6 deletions(-)
>>
>> Regards,
>> Daniel
>> --
>> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 25 July 2018 at 11:09, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> On 25 July 2018 at 17:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 25 July 2018 at 10:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>>>
>>> The 'enterprise' name is really awful - this is essentially a marketing
>>> term completely devoid of any useful meaning.
>>>
>>> You had previously called this "sbsa" which IIUC was related to a real
>>> world hardware specification that it was based on. IOW, I think this old
>>> name was preferrable to calling it "enterprise".
>>>
>>
>> I couldn't agree more. However, IIUC this change was made at the
>> request of one of the reviewers, although I wasn't part of the
>> discussion at that point, so I'm not sure who it was.
>>
>> Hongbo, could you please share a link to that discussion?
>>
>> Thanks,
>> Ard.
>>
>
> V1 discussion here:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg545775.html
>

So who asked for the sbsa -> enterprise change?

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 17:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 25 July 2018 at 11:09, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>> On 25 July 2018 at 17:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>> On 25 July 2018 at 10:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>>>>
>>>> The 'enterprise' name is really awful - this is essentially a marketing
>>>> term completely devoid of any useful meaning.
>>>>
>>>> You had previously called this "sbsa" which IIUC was related to a real
>>>> world hardware specification that it was based on. IOW, I think this old
>>>> name was preferrable to calling it "enterprise".
>>>>
>>>
>>> I couldn't agree more. However, IIUC this change was made at the
>>> request of one of the reviewers, although I wasn't part of the
>>> discussion at that point, so I'm not sure who it was.
>>>
>>> Hongbo, could you please share a link to that discussion?
>>>
>>> Thanks,
>>> Ard.
>>>
>>
>> V1 discussion here:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg545775.html
>>
>
> So who asked for the sbsa -> enterprise change?

Actually nobody, but it was argued that sbsa does not require ehci and
ahci etc, then we should find a name fitting for this platform better.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 25 July 2018 at 11:17, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> On 25 July 2018 at 17:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 25 July 2018 at 11:09, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>>> On 25 July 2018 at 17:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>>> On 25 July 2018 at 10:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>>> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>>>>>
>>>>> The 'enterprise' name is really awful - this is essentially a marketing
>>>>> term completely devoid of any useful meaning.
>>>>>
>>>>> You had previously called this "sbsa" which IIUC was related to a real
>>>>> world hardware specification that it was based on. IOW, I think this old
>>>>> name was preferrable to calling it "enterprise".
>>>>>
>>>>
>>>> I couldn't agree more. However, IIUC this change was made at the
>>>> request of one of the reviewers, although I wasn't part of the
>>>> discussion at that point, so I'm not sure who it was.
>>>>
>>>> Hongbo, could you please share a link to that discussion?
>>>>
>>>> Thanks,
>>>> Ard.
>>>>
>>>
>>> V1 discussion here:
>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg545775.html
>>>
>>
>> So who asked for the sbsa -> enterprise change?
>
> Actually nobody, but it was argued that sbsa does not require ehci and
> ahci etc, then we should find a name fitting for this platform better.

That doesn't make sense to me. The SBSA describes a minimal
configuration, it does not limit what peripherals may be attached to
the core system.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Jul 25, 2018 at 11:20:03AM +0200, Ard Biesheuvel wrote:
> On 25 July 2018 at 11:17, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> > On 25 July 2018 at 17:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> On 25 July 2018 at 11:09, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> >>> On 25 July 2018 at 17:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >>>> On 25 July 2018 at 10:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >>>>> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >>>>>
> >>>>> The 'enterprise' name is really awful - this is essentially a marketing
> >>>>> term completely devoid of any useful meaning.
> >>>>>
> >>>>> You had previously called this "sbsa" which IIUC was related to a real
> >>>>> world hardware specification that it was based on. IOW, I think this old
> >>>>> name was preferrable to calling it "enterprise".
> >>>>>
> >>>>
> >>>> I couldn't agree more. However, IIUC this change was made at the
> >>>> request of one of the reviewers, although I wasn't part of the
> >>>> discussion at that point, so I'm not sure who it was.
> >>>>
> >>>> Hongbo, could you please share a link to that discussion?
> >>>>
> >>>> Thanks,
> >>>> Ard.
> >>>>
> >>>
> >>> V1 discussion here:
> >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg545775.html
> >>>
> >>
> >> So who asked for the sbsa -> enterprise change?
> >
> > Actually nobody, but it was argued that sbsa does not require ehci and
> > ahci etc, then we should find a name fitting for this platform better.
> 
> That doesn't make sense to me. The SBSA describes a minimal
> configuration, it does not limit what peripherals may be attached to
> the core system.
>

Hi Ard,

I think that a machine model named 'sbsa' should provide all SBSA required
hardware, and nothing else, while providing a means to easily extend the
machine beyond that in any way the user likes. The user can easily add
devices with the command line and/or by using -readconfig to build a
"typical" machine. Note, it should even be possible to add, e.g. an ACHI
controller, to the memory map using the platform bus, if that's preferred
over PCIe.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 25 July 2018 at 11:40, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 11:20:03AM +0200, Ard Biesheuvel wrote:
>> On 25 July 2018 at 11:17, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>> > On 25 July 2018 at 17:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >> On 25 July 2018 at 11:09, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>> >>> On 25 July 2018 at 17:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >>>> On 25 July 2018 at 10:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> >>>>> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>> >>>>>
>> >>>>> The 'enterprise' name is really awful - this is essentially a marketing
>> >>>>> term completely devoid of any useful meaning.
>> >>>>>
>> >>>>> You had previously called this "sbsa" which IIUC was related to a real
>> >>>>> world hardware specification that it was based on. IOW, I think this old
>> >>>>> name was preferrable to calling it "enterprise".
>> >>>>>
>> >>>>
>> >>>> I couldn't agree more. However, IIUC this change was made at the
>> >>>> request of one of the reviewers, although I wasn't part of the
>> >>>> discussion at that point, so I'm not sure who it was.
>> >>>>
>> >>>> Hongbo, could you please share a link to that discussion?
>> >>>>
>> >>>> Thanks,
>> >>>> Ard.
>> >>>>
>> >>>
>> >>> V1 discussion here:
>> >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg545775.html
>> >>>
>> >>
>> >> So who asked for the sbsa -> enterprise change?
>> >
>> > Actually nobody, but it was argued that sbsa does not require ehci and
>> > ahci etc, then we should find a name fitting for this platform better.
>>
>> That doesn't make sense to me. The SBSA describes a minimal
>> configuration, it does not limit what peripherals may be attached to
>> the core system.
>>
>
> Hi Ard,
>
> I think that a machine model named 'sbsa' should provide all SBSA required
> hardware, and nothing else, while providing a means to easily extend the
> machine beyond that in any way the user likes. The user can easily add
> devices with the command line and/or by using -readconfig to build a
> "typical" machine. Note, it should even be possible to add, e.g. an ACHI
> controller, to the memory map using the platform bus, if that's preferred
> over PCIe.
>

The purpose of the SBSA machine is not to provide a minimal
configuration. It is intended to exercise all the moving parts one
might find in a server firmware/OS stack, including pieces that are
not usually found on x86 machines, such as DRAM starting above 4 GB
and SATA/USB controllers that are not PCIe based.

If we start layering the usual components on top, it is highly likely
that checking the EHCI box gives you a PCI based USB2 controller,
partially defeating the purpose of the exercise.

-- 
Ard.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Jul 25, 2018 at 11:47:39AM +0200, Ard Biesheuvel wrote:
> On 25 July 2018 at 11:40, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 11:20:03AM +0200, Ard Biesheuvel wrote:
> >> On 25 July 2018 at 11:17, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> >> > On 25 July 2018 at 17:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> >> On 25 July 2018 at 11:09, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> >> >>> On 25 July 2018 at 17:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> >>>> On 25 July 2018 at 10:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >> >>>>> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >> >>>>>
> >> >>>>> The 'enterprise' name is really awful - this is essentially a marketing
> >> >>>>> term completely devoid of any useful meaning.
> >> >>>>>
> >> >>>>> You had previously called this "sbsa" which IIUC was related to a real
> >> >>>>> world hardware specification that it was based on. IOW, I think this old
> >> >>>>> name was preferrable to calling it "enterprise".
> >> >>>>>
> >> >>>>
> >> >>>> I couldn't agree more. However, IIUC this change was made at the
> >> >>>> request of one of the reviewers, although I wasn't part of the
> >> >>>> discussion at that point, so I'm not sure who it was.
> >> >>>>
> >> >>>> Hongbo, could you please share a link to that discussion?
> >> >>>>
> >> >>>> Thanks,
> >> >>>> Ard.
> >> >>>>
> >> >>>
> >> >>> V1 discussion here:
> >> >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg545775.html
> >> >>>
> >> >>
> >> >> So who asked for the sbsa -> enterprise change?
> >> >
> >> > Actually nobody, but it was argued that sbsa does not require ehci and
> >> > ahci etc, then we should find a name fitting for this platform better.
> >>
> >> That doesn't make sense to me. The SBSA describes a minimal
> >> configuration, it does not limit what peripherals may be attached to
> >> the core system.
> >>
> >
> > Hi Ard,
> >
> > I think that a machine model named 'sbsa' should provide all SBSA required
> > hardware, and nothing else, while providing a means to easily extend the
> > machine beyond that in any way the user likes. The user can easily add
> > devices with the command line and/or by using -readconfig to build a
> > "typical" machine. Note, it should even be possible to add, e.g. an ACHI
> > controller, to the memory map using the platform bus, if that's preferred
> > over PCIe.
> >
> 
> The purpose of the SBSA machine is not to provide a minimal
> configuration. It is intended to exercise all the moving parts one
> might find in a server firmware/OS stack, including pieces that are
> not usually found on x86 machines, such as DRAM starting above 4 GB
> and SATA/USB controllers that are not PCIe based.

To me, this purpose actually requires that the base config be minimal.
How else can you have the flexibility to build a wide range of
configurations for testing?

> 
> If we start layering the usual components on top, it is highly likely
> that checking the EHCI box gives you a PCI based USB2 controller,
> partially defeating the purpose of the exercise.
>

As I said, it should be possible to put these controllers on the system
bus, even if they're specified on the command line. This would be done
using the platform-bus framework. Naturally, providing platform-bus
support for devices/controllers that need it will take some machine-done
notifier code, etc., so this machine model patch series will get more
complex.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 17:40, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 11:20:03AM +0200, Ard Biesheuvel wrote:
>> On 25 July 2018 at 11:17, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>> > On 25 July 2018 at 17:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >> On 25 July 2018 at 11:09, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>> >>> On 25 July 2018 at 17:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >>>> On 25 July 2018 at 10:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> >>>>> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>> >>>>>
>> >>>>> The 'enterprise' name is really awful - this is essentially a marketing
>> >>>>> term completely devoid of any useful meaning.
>> >>>>>
>> >>>>> You had previously called this "sbsa" which IIUC was related to a real
>> >>>>> world hardware specification that it was based on. IOW, I think this old
>> >>>>> name was preferrable to calling it "enterprise".
>> >>>>>
>> >>>>
>> >>>> I couldn't agree more. However, IIUC this change was made at the
>> >>>> request of one of the reviewers, although I wasn't part of the
>> >>>> discussion at that point, so I'm not sure who it was.
>> >>>>
>> >>>> Hongbo, could you please share a link to that discussion?
>> >>>>
>> >>>> Thanks,
>> >>>> Ard.
>> >>>>
>> >>>
>> >>> V1 discussion here:
>> >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg545775.html
>> >>>
>> >>
>> >> So who asked for the sbsa -> enterprise change?
>> >
>> > Actually nobody, but it was argued that sbsa does not require ehci and
>> > ahci etc, then we should find a name fitting for this platform better.
>>
>> That doesn't make sense to me. The SBSA describes a minimal
>> configuration, it does not limit what peripherals may be attached to
>> the core system.
>>
>
> Hi Ard,
>
> I think that a machine model named 'sbsa' should provide all SBSA required
> hardware, and nothing else, while providing a means to easily extend the
> machine beyond that in any way the user likes. The user can easily add
> devices with the command line and/or by using -readconfig to build a
> "typical" machine. Note, it should even be possible to add, e.g. an ACHI
> controller, to the memory map using the platform bus, if that's preferred
> over PCIe.
>
Even command line or readconfig can add AHCI on platform bus, I think
it can only create such devices, but no DT or ACPI nodes, which are
mandatory.

> Thanks,
> drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Jul 25, 2018 at 06:33:01PM +0800, Hongbo Zhang wrote:
> On 25 July 2018 at 17:40, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 11:20:03AM +0200, Ard Biesheuvel wrote:
> >> On 25 July 2018 at 11:17, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> >> > On 25 July 2018 at 17:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> >> On 25 July 2018 at 11:09, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> >> >>> On 25 July 2018 at 17:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> >>>> On 25 July 2018 at 10:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >> >>>>> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >> >>>>>
> >> >>>>> The 'enterprise' name is really awful - this is essentially a marketing
> >> >>>>> term completely devoid of any useful meaning.
> >> >>>>>
> >> >>>>> You had previously called this "sbsa" which IIUC was related to a real
> >> >>>>> world hardware specification that it was based on. IOW, I think this old
> >> >>>>> name was preferrable to calling it "enterprise".
> >> >>>>>
> >> >>>>
> >> >>>> I couldn't agree more. However, IIUC this change was made at the
> >> >>>> request of one of the reviewers, although I wasn't part of the
> >> >>>> discussion at that point, so I'm not sure who it was.
> >> >>>>
> >> >>>> Hongbo, could you please share a link to that discussion?
> >> >>>>
> >> >>>> Thanks,
> >> >>>> Ard.
> >> >>>>
> >> >>>
> >> >>> V1 discussion here:
> >> >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg545775.html
> >> >>>
> >> >>
> >> >> So who asked for the sbsa -> enterprise change?
> >> >
> >> > Actually nobody, but it was argued that sbsa does not require ehci and
> >> > ahci etc, then we should find a name fitting for this platform better.
> >>
> >> That doesn't make sense to me. The SBSA describes a minimal
> >> configuration, it does not limit what peripherals may be attached to
> >> the core system.
> >>
> >
> > Hi Ard,
> >
> > I think that a machine model named 'sbsa' should provide all SBSA required
> > hardware, and nothing else, while providing a means to easily extend the
> > machine beyond that in any way the user likes. The user can easily add
> > devices with the command line and/or by using -readconfig to build a
> > "typical" machine. Note, it should even be possible to add, e.g. an ACHI
> > controller, to the memory map using the platform bus, if that's preferred
> > over PCIe.
> >
> Even command line or readconfig can add AHCI on platform bus, I think
> it can only create such devices, but no DT or ACPI nodes, which are
> mandatory.

You have to write the code for that, similar to what this patch does,
but plus the machine-done notifier support to do it at platform-bus
attach time.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 16:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>
> The 'enterprise' name is really awful - this is essentially a marketing
> term completely devoid of any useful meaning.
>
> You had previously called this "sbsa" which IIUC was related to a real
> world hardware specification that it was based on. IOW, I think this old
> name was preferrable to calling it "enterprise".
>
Thanks for your comments.
Frankly, I myself prefer to 'sbsa' too, in fact, at the early stage of
developing, we called this 'enterprise', but later I changed it to
'sbsa' until I sent out v1 patch.

The work Arm TF and EDK2 porting to this platform needs this name to
be defined finally.

>>  - Based on 'virt' machine type.
>>  - Re-designed memory map.
>>  - EL2 and EL3 are enabled by default.
>>  - GIC version 3 by default.
>>  - AHCI controller attached to system bus, and then CDROM and hard disc
>>    can be added to it.
>>  - EHCI controller attached to system bus, with USB mouse and key board
>>    installed by default.
>>  - E1000E ethernet card on PCIE bus.
>>  - VGA display adaptor on PCIE bus.
>>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>>  - No virtio functions enabled, since this is to emulate real hardware.
>>  - No paravirtualized fw_cfg device either.
>>
>> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>>
>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>> ---
>> Changes since v1:
>>  - rebase on v3.0.0-rc0
>>  - introduce another auxillary patch as 1/2, so this is 2/2
>>  - rename 'sbsa' to 'enterprise'
>>  - remove paravirualized fw_cfg
>>  - set gic_vertion to 3 instead of 2
>>  - edit commit message to describe purpose of this platform
>>
>>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-
>>  hw/arm/virt.c            | 199 ++++++++++++++++++++++++++++++++++++++++++++++-
>>  include/hw/arm/virt.h    |   3 +
>>  3 files changed, 255 insertions(+), 6 deletions(-)
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Daniel P. Berrangé]

On Wed, Jul 25, 2018 at 05:05:06PM +0800, Hongbo Zhang wrote:
> On 25 July 2018 at 16:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >
> > The 'enterprise' name is really awful - this is essentially a marketing
> > term completely devoid of any useful meaning.
> >
> > You had previously called this "sbsa" which IIUC was related to a real
> > world hardware specification that it was based on. IOW, I think this old
> > name was preferrable to calling it "enterprise".
> >
> Thanks for your comments.
> Frankly, I myself prefer to 'sbsa' too, in fact, at the early stage of
> developing, we called this 'enterprise', but later I changed it to
> 'sbsa' until I sent out v1 patch.
> 
> The work Arm TF and EDK2 porting to this platform needs this name to
> be defined finally.

Why should EDK2 care what the QEMU machine type name is. The machine type
name is purely a QEMU internal tag and shouldn't be visible in the guest
ABI at all IIUC.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 17:18, Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 05:05:06PM +0800, Hongbo Zhang wrote:
>> On 25 July 2018 at 16:48, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>> >
>> > The 'enterprise' name is really awful - this is essentially a marketing
>> > term completely devoid of any useful meaning.
>> >
>> > You had previously called this "sbsa" which IIUC was related to a real
>> > world hardware specification that it was based on. IOW, I think this old
>> > name was preferrable to calling it "enterprise".
>> >
>> Thanks for your comments.
>> Frankly, I myself prefer to 'sbsa' too, in fact, at the early stage of
>> developing, we called this 'enterprise', but later I changed it to
>> 'sbsa' until I sent out v1 patch.
>>
>> The work Arm TF and EDK2 porting to this platform needs this name to
>> be defined finally.
>
> Why should EDK2 care what the QEMU machine type name is. The machine type
> name is purely a QEMU internal tag and shouldn't be visible in the guest
> ABI at all IIUC.
>
For example, in EDK2, ArmVirtPkg/ArmVirtQemuSBSA.dsc and
ArmVirtPkg/ArmVirtQemuSBSA.fdf are created for porting to the new
platform, similar situation for Arm TF, so the name is needed.

>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>  - Based on 'virt' machine type.
>  - Re-designed memory map.
>  - EL2 and EL3 are enabled by default.
>  - GIC version 3 by default.
>  - AHCI controller attached to system bus, and then CDROM and hard disc
>    can be added to it.
>  - EHCI controller attached to system bus, with USB mouse and key board
>    installed by default.
>  - E1000E ethernet card on PCIE bus.
>  - VGA display adaptor on PCIE bus.
>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>  - No virtio functions enabled, since this is to emulate real hardware.

In the last review it was pointed out that using virtio-pci should still
be "real" enough, so there's not much reason to avoid it. Well, unless
there's some concern as to what drivers are available in the firmware and
guest kernel. But that concern usually only applies to legacy firmwares
and kernels, and therefore shouldn't apply to AArch64.

>  - No paravirtualized fw_cfg device either.
> 
> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>

How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
see any sort of reserved ROM region in the patch for them.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>>  - Based on 'virt' machine type.
>>  - Re-designed memory map.
>>  - EL2 and EL3 are enabled by default.
>>  - GIC version 3 by default.
>>  - AHCI controller attached to system bus, and then CDROM and hard disc
>>    can be added to it.
>>  - EHCI controller attached to system bus, with USB mouse and key board
>>    installed by default.
>>  - E1000E ethernet card on PCIE bus.
>>  - VGA display adaptor on PCIE bus.
>>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>>  - No virtio functions enabled, since this is to emulate real hardware.
>
> In the last review it was pointed out that using virtio-pci should still
> be "real" enough, so there's not much reason to avoid it. Well, unless
> there's some concern as to what drivers are available in the firmware and
> guest kernel. But that concern usually only applies to legacy firmwares
> and kernels, and therefore shouldn't apply to AArch64.
>
In real physical arm hardware, *HCI are system memory mapped, not on PCIE.
we need a QEMU platform like that. We need firmware developed on this
QEMU platform can run on real hardware without change(or only a minor
change)
Concern is not only available firmwares, but more emphasis is on new
firmwares to be developed on this platform (target is developing
firmware for hardware, but using qemu if hardware is not available
temporarily), if virtio device used, then the newly developed firmware
must include virtio front-end codes, but it isn't needed while run on
real hardware at last.

>>  - No paravirtualized fw_cfg device either.
>>
>> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>>
>
> How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
> see any sort of reserved ROM region in the patch for them.
>
UEFI gets ACPI and kernel from network or mass storage, just like the
real hardware.
If we develop firmware depends on paravirtualized device like fw_cfg,
then we canot run such firmware on real hardware.

> Thanks,
> drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Dr. David Alan Gilbert]

* Hongbo Zhang (hongbo.zhang@linaro.org) wrote:
> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >>  - Based on 'virt' machine type.
> >>  - Re-designed memory map.
> >>  - EL2 and EL3 are enabled by default.
> >>  - GIC version 3 by default.
> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
> >>    can be added to it.
> >>  - EHCI controller attached to system bus, with USB mouse and key board
> >>    installed by default.
> >>  - E1000E ethernet card on PCIE bus.
> >>  - VGA display adaptor on PCIE bus.
> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
> >>  - No virtio functions enabled, since this is to emulate real hardware.
> >
> > In the last review it was pointed out that using virtio-pci should still
> > be "real" enough, so there's not much reason to avoid it. Well, unless
> > there's some concern as to what drivers are available in the firmware and
> > guest kernel. But that concern usually only applies to legacy firmwares
> > and kernels, and therefore shouldn't apply to AArch64.
> >
> In real physical arm hardware, *HCI are system memory mapped, not on PCIE.
> we need a QEMU platform like that. We need firmware developed on this
> QEMU platform can run on real hardware without change(or only a minor
> change)

How would you deal with most modern hardware now using XHCI rather than
EHCI ?

Dave

> Concern is not only available firmwares, but more emphasis is on new
> firmwares to be developed on this platform (target is developing
> firmware for hardware, but using qemu if hardware is not available
> temporarily), if virtio device used, then the newly developed firmware
> must include virtio front-end codes, but it isn't needed while run on
> real hardware at last.
> 
> >>  - No paravirtualized fw_cfg device either.
> >>
> >> Arm Trusted Firmware and UEFI porting to this are done accordingly.
> >>
> >
> > How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
> > see any sort of reserved ROM region in the patch for them.
> >
> UEFI gets ACPI and kernel from network or mass storage, just like the
> real hardware.
> If we develop firmware depends on paravirtualized device like fw_cfg,
> then we canot run such firmware on real hardware.
> 
> > Thanks,
> > drew
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 18:53, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Hongbo Zhang (hongbo.zhang@linaro.org) wrote:
>> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
>> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>> >>  - Based on 'virt' machine type.
>> >>  - Re-designed memory map.
>> >>  - EL2 and EL3 are enabled by default.
>> >>  - GIC version 3 by default.
>> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
>> >>    can be added to it.
>> >>  - EHCI controller attached to system bus, with USB mouse and key board
>> >>    installed by default.
>> >>  - E1000E ethernet card on PCIE bus.
>> >>  - VGA display adaptor on PCIE bus.
>> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>> >>  - No virtio functions enabled, since this is to emulate real hardware.
>> >
>> > In the last review it was pointed out that using virtio-pci should still
>> > be "real" enough, so there's not much reason to avoid it. Well, unless
>> > there's some concern as to what drivers are available in the firmware and
>> > guest kernel. But that concern usually only applies to legacy firmwares
>> > and kernels, and therefore shouldn't apply to AArch64.
>> >
>> In real physical arm hardware, *HCI are system memory mapped, not on PCIE.
>> we need a QEMU platform like that. We need firmware developed on this
>> QEMU platform can run on real hardware without change(or only a minor
>> change)
>
> How would you deal with most modern hardware now using XHCI rather than
> EHCI ?
>
Well, EHCI still works well on server, some new X86 servers have both
XHCI and EHCI, Qualcomm Centriq Arm server even has only EHCI, so
currently only the EHCI is added.
I had no strong reason for XHCI or EHCI, if newer is better or some
users have special requirement for XHCI, it can be added too.

> Dave
>
>> Concern is not only available firmwares, but more emphasis is on new
>> firmwares to be developed on this platform (target is developing
>> firmware for hardware, but using qemu if hardware is not available
>> temporarily), if virtio device used, then the newly developed firmware
>> must include virtio front-end codes, but it isn't needed while run on
>> real hardware at last.
>>
>> >>  - No paravirtualized fw_cfg device either.
>> >>
>> >> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>> >>
>> >
>> > How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
>> > see any sort of reserved ROM region in the patch for them.
>> >
>> UEFI gets ACPI and kernel from network or mass storage, just like the
>> real hardware.
>> If we develop firmware depends on paravirtualized device like fw_cfg,
>> then we canot run such firmware on real hardware.
>>
>> > Thanks,
>> > drew
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 26 July 2018 at 09:35, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> On 25 July 2018 at 18:53, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
>> * Hongbo Zhang (hongbo.zhang@linaro.org) wrote:
>>> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
>>> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>>> >>  - Based on 'virt' machine type.
>>> >>  - Re-designed memory map.
>>> >>  - EL2 and EL3 are enabled by default.
>>> >>  - GIC version 3 by default.
>>> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
>>> >>    can be added to it.
>>> >>  - EHCI controller attached to system bus, with USB mouse and key board
>>> >>    installed by default.
>>> >>  - E1000E ethernet card on PCIE bus.
>>> >>  - VGA display adaptor on PCIE bus.
>>> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>>> >>  - No virtio functions enabled, since this is to emulate real hardware.
>>> >
>>> > In the last review it was pointed out that using virtio-pci should still
>>> > be "real" enough, so there's not much reason to avoid it. Well, unless
>>> > there's some concern as to what drivers are available in the firmware and
>>> > guest kernel. But that concern usually only applies to legacy firmwares
>>> > and kernels, and therefore shouldn't apply to AArch64.
>>> >
>>> In real physical arm hardware, *HCI are system memory mapped, not on PCIE.
>>> we need a QEMU platform like that. We need firmware developed on this
>>> QEMU platform can run on real hardware without change(or only a minor
>>> change)
>>
>> How would you deal with most modern hardware now using XHCI rather than
>> EHCI ?
>>
> Well, EHCI still works well on server, some new X86 servers have both
> XHCI and EHCI, Qualcomm Centriq Arm server even has only EHCI, so
> currently only the EHCI is added.
> I had no strong reason for XHCI or EHCI, if newer is better or some
> users have special requirement for XHCI, it can be added too.
>

Does that really matter? As I pointed out before, we are not
interested in running the firmware on actual bare metal. What we do
care about is not having code in the reference firmware stack that was
added specifically to deal with the dynamic nature of our QEMU
platform.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 26 July 2018 at 08:44, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Does that really matter? As I pointed out before, we are not
> interested in running the firmware on actual bare metal. What we do
> care about is not having code in the reference firmware stack that was
> added specifically to deal with the dynamic nature of our QEMU
> platform.

I think we also care in the sense that what we put in the reference
platform is an implicit recommendation. So if we (collectively) think
that good Arm server platforms are likely to or should use XHCI,
we should definitely consider going with that, rather than going with
what happens to be being used on currently shipping hardware.
(We should also consider whether we want to implicitly recommend
that the USB controller should be memory-mapped rather than
hanging off a PCI controller.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Jul 25, 2018 at 06:22:17PM +0800, Hongbo Zhang wrote:
> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >>  - Based on 'virt' machine type.
> >>  - Re-designed memory map.
> >>  - EL2 and EL3 are enabled by default.
> >>  - GIC version 3 by default.
> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
> >>    can be added to it.
> >>  - EHCI controller attached to system bus, with USB mouse and key board
> >>    installed by default.
> >>  - E1000E ethernet card on PCIE bus.
> >>  - VGA display adaptor on PCIE bus.
> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
> >>  - No virtio functions enabled, since this is to emulate real hardware.
> >
> > In the last review it was pointed out that using virtio-pci should still
> > be "real" enough, so there's not much reason to avoid it. Well, unless
> > there's some concern as to what drivers are available in the firmware and
> > guest kernel. But that concern usually only applies to legacy firmwares
> > and kernels, and therefore shouldn't apply to AArch64.
> >
> In real physical arm hardware, *HCI are system memory mapped, not on PCIE.
> we need a QEMU platform like that. We need firmware developed on this
> QEMU platform can run on real hardware without change(or only a minor
> change)

virtio-pci has nothing to do with *HCI. You're adding an E1000e to the
PCIe bus instead of a virtio-pci nic. Why?

> Concern is not only available firmwares, but more emphasis is on new
> firmwares to be developed on this platform (target is developing
> firmware for hardware, but using qemu if hardware is not available
> temporarily), if virtio device used, then the newly developed firmware
> must include virtio front-end codes, but it isn't needed while run on
> real hardware at last.

Right. The new firmwares and kernels would need to include virtio-pci nic
and scsi drivers. Is that a problem? Anyway, this is all the more reason
not to hard code peripherals. If a particular peripheral is a problem
for a given firmware, then simply don't add it to the command line, add a
different one.

> 
> >>  - No paravirtualized fw_cfg device either.
> >>
> >> Arm Trusted Firmware and UEFI porting to this are done accordingly.
> >>
> >
> > How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
> > see any sort of reserved ROM region in the patch for them.
> >
> UEFI gets ACPI and kernel from network or mass storage, just like the
> real hardware.

Hmm. I thought for real hardware that the ACPI tables were built into
the firmware. So, assuming UEFI knows how to read ACPI tables from
some storage, then how do the QEMU generated ACPI tables get into that
storage?

> If we develop firmware depends on paravirtualized device like fw_cfg,
> then we canot run such firmware on real hardware.

Yes, fw-cfg is paravirt, so it's obvious why you'd prefer it not be there,
but, where real hardware builds its hardware descriptions into its
platform-specific firmware, or uses some platform-specific means of
extracting a description from some platform-specific place, QEMU
machine models use fw-cfg.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 19:26, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 06:22:17PM +0800, Hongbo Zhang wrote:
>> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
>> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>> >>  - Based on 'virt' machine type.
>> >>  - Re-designed memory map.
>> >>  - EL2 and EL3 are enabled by default.
>> >>  - GIC version 3 by default.
>> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
>> >>    can be added to it.
>> >>  - EHCI controller attached to system bus, with USB mouse and key board
>> >>    installed by default.
>> >>  - E1000E ethernet card on PCIE bus.
>> >>  - VGA display adaptor on PCIE bus.
>> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>> >>  - No virtio functions enabled, since this is to emulate real hardware.
>> >
>> > In the last review it was pointed out that using virtio-pci should still
>> > be "real" enough, so there's not much reason to avoid it. Well, unless
>> > there's some concern as to what drivers are available in the firmware and
>> > guest kernel. But that concern usually only applies to legacy firmwares
>> > and kernels, and therefore shouldn't apply to AArch64.
>> >
>> In real physical arm hardware, *HCI are system memory mapped, not on PCIE.
>> we need a QEMU platform like that. We need firmware developed on this
>> QEMU platform can run on real hardware without change(or only a minor
>> change)
>
> virtio-pci has nothing to do with *HCI. You're adding an E1000e to the
> PCIe bus instead of a virtio-pci nic. Why?
>
No virtio devices are need on this platform, so no virtio-pci either,
on the real Arm server hardware, a NIC is inserted into PCIE, and
E1000E is a typical one.

>> Concern is not only available firmwares, but more emphasis is on new
>> firmwares to be developed on this platform (target is developing
>> firmware for hardware, but using qemu if hardware is not available
>> temporarily), if virtio device used, then the newly developed firmware
>> must include virtio front-end codes, but it isn't needed while run on
>> real hardware at last.
>
> Right. The new firmwares and kernels would need to include virtio-pci nic
> and scsi drivers. Is that a problem? Anyway, this is all the more reason
> not to hard code peripherals. If a particular peripheral is a problem
> for a given firmware, then simply don't add it to the command line, add a
> different one.
>
Yes that is problem, for newly developed firmwares, extra efforts will
be wasted on frontend codes (or glue-layer, whatever we call it), we
want firmwares developed on this platform can run easily on real
hardware, without such change.
Requirement is: some Linaro members just want a qemu platform as true
as possible with real hardware, there should be no problem with such
requirement, problem is 'virt' machine cannot satisfy the requirement,
so a new one is needed.

>>
>> >>  - No paravirtualized fw_cfg device either.
>> >>
>> >> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>> >>
>> >
>> > How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
>> > see any sort of reserved ROM region in the patch for them.
>> >
>> UEFI gets ACPI and kernel from network or mass storage, just like the
>> real hardware.
>
> Hmm. I thought for real hardware that the ACPI tables were built into
> the firmware. So, assuming UEFI knows how to read ACPI tables from
> some storage, then how do the QEMU generated ACPI tables get into that
> storage?
>
I should say "mass storage and flash"
There was fw_cfg in v1 patch, it is removed in v2.
If no fw_cfg, just like real hardware, UEFI should include ACPI
support for this SBSA platform, and UEFI/ACPI is load via -pflash,
then the QEMU built-in ACPI isn't used.
But there are side effects too, command 'qemu -bios uefi -kernel'
won't work, I need extra time to evaluate this change.

>> If we develop firmware depends on paravirtualized device like fw_cfg,
>> then we canot run such firmware on real hardware.
>
> Yes, fw-cfg is paravirt, so it's obvious why you'd prefer it not be there,
> but, where real hardware builds its hardware descriptions into its
> platform-specific firmware, or uses some platform-specific means of
> extracting a description from some platform-specific place, QEMU
> machine models use fw-cfg.
>
> Thanks,
> drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Thu, Jul 26, 2018 at 05:22:14PM +0800, Hongbo Zhang wrote:
> On 25 July 2018 at 19:26, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 06:22:17PM +0800, Hongbo Zhang wrote:
> >> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
> >> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >> >>  - Based on 'virt' machine type.
> >> >>  - Re-designed memory map.
> >> >>  - EL2 and EL3 are enabled by default.
> >> >>  - GIC version 3 by default.
> >> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
> >> >>    can be added to it.
> >> >>  - EHCI controller attached to system bus, with USB mouse and key board
> >> >>    installed by default.
> >> >>  - E1000E ethernet card on PCIE bus.
> >> >>  - VGA display adaptor on PCIE bus.
> >> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
> >> >>  - No virtio functions enabled, since this is to emulate real hardware.
> >> >
> >> > In the last review it was pointed out that using virtio-pci should still
> >> > be "real" enough, so there's not much reason to avoid it. Well, unless
> >> > there's some concern as to what drivers are available in the firmware and
> >> > guest kernel. But that concern usually only applies to legacy firmwares
> >> > and kernels, and therefore shouldn't apply to AArch64.
> >> >
> >> In real physical arm hardware, *HCI are system memory mapped, not on PCIE.
> >> we need a QEMU platform like that. We need firmware developed on this
> >> QEMU platform can run on real hardware without change(or only a minor
> >> change)
> >
> > virtio-pci has nothing to do with *HCI. You're adding an E1000e to the
> > PCIe bus instead of a virtio-pci nic. Why?
> >
> No virtio devices are need on this platform, so no virtio-pci either,
> on the real Arm server hardware, a NIC is inserted into PCIE, and
> E1000E is a typical one.

It is possible to make a real piece of hardware that really goes in a PCIe
slot which knows how to talk VirtIO. The fact that an E1000e driver will
drive an E1000e QEMU model instead of a VirtIO driver driving a VirtIO
backend is, to me, pretty arbitrary. The only reason it should matter for
the guest firmware/kernel is whether or not the firmware/kernel will have
VirtIO drivers available. Do we know that? Is it documented somewhere
that the guest firmware/kernel is guaranteed to have E1000e drivers, but
VirtIO drivers are optional, or even forbidden? If so, where's that
document?

> 
> >> Concern is not only available firmwares, but more emphasis is on new
> >> firmwares to be developed on this platform (target is developing
> >> firmware for hardware, but using qemu if hardware is not available
> >> temporarily), if virtio device used, then the newly developed firmware
> >> must include virtio front-end codes, but it isn't needed while run on
> >> real hardware at last.
> >
> > Right. The new firmwares and kernels would need to include virtio-pci nic
> > and scsi drivers. Is that a problem? Anyway, this is all the more reason
> > not to hard code peripherals. If a particular peripheral is a problem
> > for a given firmware, then simply don't add it to the command line, add a
> > different one.
> >
> Yes that is problem, for newly developed firmwares, extra efforts will
> be wasted on frontend codes (or glue-layer, whatever we call it), we
> want firmwares developed on this platform can run easily on real
> hardware, without such change.
> Requirement is: some Linaro members just want a qemu platform as true
> as possible with real hardware, there should be no problem with such
> requirement, problem is 'virt' machine cannot satisfy the requirement,
> so a new one is needed.

It sounds like somebody knows what drivers are available and what
drivers aren't. If that's not already documented, then it should
be, and a pointer to it should be in this patch series.

> 
> >>
> >> >>  - No paravirtualized fw_cfg device either.
> >> >>
> >> >> Arm Trusted Firmware and UEFI porting to this are done accordingly.
> >> >>
> >> >
> >> > How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
> >> > see any sort of reserved ROM region in the patch for them.
> >> >
> >> UEFI gets ACPI and kernel from network or mass storage, just like the
> >> real hardware.
> >
> > Hmm. I thought for real hardware that the ACPI tables were built into
> > the firmware. So, assuming UEFI knows how to read ACPI tables from
> > some storage, then how do the QEMU generated ACPI tables get into that
> > storage?
> >
> I should say "mass storage and flash"
> There was fw_cfg in v1 patch, it is removed in v2.
> If no fw_cfg, just like real hardware, UEFI should include ACPI
> support for this SBSA platform, and UEFI/ACPI is load via -pflash,
> then the QEMU built-in ACPI isn't used.
> But there are side effects too, command 'qemu -bios uefi -kernel'
> won't work, I need extra time to evaluate this change.

Right. Neither ACPI nor '-bios ... -kernel ...' can work without fw-cfg.
This patch either needs to keep fw-cfg, or to remove the ACPI changes.
I can't see how an SBSA reference platform would be much use without ACPI
though.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 26 July 2018 at 12:28, Andrew Jones <drjones@redhat.com> wrote:
> On Thu, Jul 26, 2018 at 05:22:14PM +0800, Hongbo Zhang wrote:
>> On 25 July 2018 at 19:26, Andrew Jones <drjones@redhat.com> wrote:
>> > On Wed, Jul 25, 2018 at 06:22:17PM +0800, Hongbo Zhang wrote:
>> >> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
>> >> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>> >> >>  - Based on 'virt' machine type.
>> >> >>  - Re-designed memory map.
>> >> >>  - EL2 and EL3 are enabled by default.
>> >> >>  - GIC version 3 by default.
>> >> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
>> >> >>    can be added to it.
>> >> >>  - EHCI controller attached to system bus, with USB mouse and key board
>> >> >>    installed by default.
>> >> >>  - E1000E ethernet card on PCIE bus.
>> >> >>  - VGA display adaptor on PCIE bus.
>> >> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>> >> >>  - No virtio functions enabled, since this is to emulate real hardware.
>> >> >
>> >> > In the last review it was pointed out that using virtio-pci should still
>> >> > be "real" enough, so there's not much reason to avoid it. Well, unless
>> >> > there's some concern as to what drivers are available in the firmware and
>> >> > guest kernel. But that concern usually only applies to legacy firmwares
>> >> > and kernels, and therefore shouldn't apply to AArch64.
>> >> >
>> >> In real physical arm hardware, *HCI are system memory mapped, not on PCIE.
>> >> we need a QEMU platform like that. We need firmware developed on this
>> >> QEMU platform can run on real hardware without change(or only a minor
>> >> change)
>> >
>> > virtio-pci has nothing to do with *HCI. You're adding an E1000e to the
>> > PCIe bus instead of a virtio-pci nic. Why?
>> >
>> No virtio devices are need on this platform, so no virtio-pci either,
>> on the real Arm server hardware, a NIC is inserted into PCIE, and
>> E1000E is a typical one.
>
> It is possible to make a real piece of hardware that really goes in a PCIe
> slot which knows how to talk VirtIO. The fact that an E1000e driver will
> drive an E1000e QEMU model instead of a VirtIO driver driving a VirtIO
> backend is, to me, pretty arbitrary. The only reason it should matter for
> the guest firmware/kernel is whether or not the firmware/kernel will have
> VirtIO drivers available. Do we know that? Is it documented somewhere
> that the guest firmware/kernel is guaranteed to have E1000e drivers, but
> VirtIO drivers are optional, or even forbidden? If so, where's that
> document?
>

It is not pretty arbitrary. One is paravirtualization and one is not.

>>
>> >> Concern is not only available firmwares, but more emphasis is on new
>> >> firmwares to be developed on this platform (target is developing
>> >> firmware for hardware, but using qemu if hardware is not available
>> >> temporarily), if virtio device used, then the newly developed firmware
>> >> must include virtio front-end codes, but it isn't needed while run on
>> >> real hardware at last.
>> >
>> > Right. The new firmwares and kernels would need to include virtio-pci nic
>> > and scsi drivers. Is that a problem? Anyway, this is all the more reason
>> > not to hard code peripherals. If a particular peripheral is a problem
>> > for a given firmware, then simply don't add it to the command line, add a
>> > different one.
>> >
>> Yes that is problem, for newly developed firmwares, extra efforts will
>> be wasted on frontend codes (or glue-layer, whatever we call it), we
>> want firmwares developed on this platform can run easily on real
>> hardware, without such change.
>> Requirement is: some Linaro members just want a qemu platform as true
>> as possible with real hardware, there should be no problem with such
>> requirement, problem is 'virt' machine cannot satisfy the requirement,
>> so a new one is needed.
>
> It sounds like somebody knows what drivers are available and what
> drivers aren't. If that's not already documented, then it should
> be, and a pointer to it should be in this patch series.
>

Available where?

UEFI has drivers for ?HCI industry standard hardware. As for the
networking side, we should review whether E1000e is the most
appropriate or this, given the lack of open source drivers. However, I
do agree that discoverable hardware should not be hardcoded, and we
should even try to use the emulated option ROM to provide a UEFI
driver.

>>
>> >>
>> >> >>  - No paravirtualized fw_cfg device either.
>> >> >>
>> >> >> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>> >> >>
>> >> >
>> >> > How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
>> >> > see any sort of reserved ROM region in the patch for them.
>> >> >
>> >> UEFI gets ACPI and kernel from network or mass storage, just like the
>> >> real hardware.
>> >
>> > Hmm. I thought for real hardware that the ACPI tables were built into
>> > the firmware. So, assuming UEFI knows how to read ACPI tables from
>> > some storage, then how do the QEMU generated ACPI tables get into that
>> > storage?
>> >
>> I should say "mass storage and flash"
>> There was fw_cfg in v1 patch, it is removed in v2.
>> If no fw_cfg, just like real hardware, UEFI should include ACPI
>> support for this SBSA platform, and UEFI/ACPI is load via -pflash,
>> then the QEMU built-in ACPI isn't used.
>> But there are side effects too, command 'qemu -bios uefi -kernel'
>> won't work, I need extra time to evaluate this change.
>
> Right. Neither ACPI nor '-bios ... -kernel ...' can work without fw-cfg.
> This patch either needs to keep fw-cfg, or to remove the ACPI changes.
> I can't see how an SBSA reference platform would be much use without ACPI
> though.
>

Even if mach-virt's ACPI code depends on fw_cfg currently, there is no
reason whatsoever that this sbsa machine should not implement it like
real hardware does, i.e., hard coded tables.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Thu, Jul 26, 2018 at 12:35:08PM +0200, Ard Biesheuvel wrote:
> On 26 July 2018 at 12:28, Andrew Jones <drjones@redhat.com> wrote:
> > On Thu, Jul 26, 2018 at 05:22:14PM +0800, Hongbo Zhang wrote:
> >> On 25 July 2018 at 19:26, Andrew Jones <drjones@redhat.com> wrote:
> >> > On Wed, Jul 25, 2018 at 06:22:17PM +0800, Hongbo Zhang wrote:
> >> >> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
> >> >> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >> >> >>  - Based on 'virt' machine type.
> >> >> >>  - Re-designed memory map.
> >> >> >>  - EL2 and EL3 are enabled by default.
> >> >> >>  - GIC version 3 by default.
> >> >> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
> >> >> >>    can be added to it.
> >> >> >>  - EHCI controller attached to system bus, with USB mouse and key board
> >> >> >>    installed by default.
> >> >> >>  - E1000E ethernet card on PCIE bus.
> >> >> >>  - VGA display adaptor on PCIE bus.
> >> >> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
> >> >> >>  - No virtio functions enabled, since this is to emulate real hardware.
> >> >> >
> >> >> > In the last review it was pointed out that using virtio-pci should still
> >> >> > be "real" enough, so there's not much reason to avoid it. Well, unless
> >> >> > there's some concern as to what drivers are available in the firmware and
> >> >> > guest kernel. But that concern usually only applies to legacy firmwares
> >> >> > and kernels, and therefore shouldn't apply to AArch64.
> >> >> >
> >> >> In real physical arm hardware, *HCI are system memory mapped, not on PCIE.
> >> >> we need a QEMU platform like that. We need firmware developed on this
> >> >> QEMU platform can run on real hardware without change(or only a minor
> >> >> change)
> >> >
> >> > virtio-pci has nothing to do with *HCI. You're adding an E1000e to the
> >> > PCIe bus instead of a virtio-pci nic. Why?
> >> >
> >> No virtio devices are need on this platform, so no virtio-pci either,
> >> on the real Arm server hardware, a NIC is inserted into PCIE, and
> >> E1000E is a typical one.
> >
> > It is possible to make a real piece of hardware that really goes in a PCIe
> > slot which knows how to talk VirtIO. The fact that an E1000e driver will
> > drive an E1000e QEMU model instead of a VirtIO driver driving a VirtIO
> > backend is, to me, pretty arbitrary. The only reason it should matter for
> > the guest firmware/kernel is whether or not the firmware/kernel will have
> > VirtIO drivers available. Do we know that? Is it documented somewhere
> > that the guest firmware/kernel is guaranteed to have E1000e drivers, but
> > VirtIO drivers are optional, or even forbidden? If so, where's that
> > document?
> >
> 
> It is not pretty arbitrary. One is paravirtualization and one is not.

But the paravirtness is a driver detail, not a platform detail. The
virtio-pci device is just a PCIe device to the platform. Drive it or
not, drive it with knowledge that it's paravirt or not, the platform
doesn't care.

> 
> >>
> >> >> Concern is not only available firmwares, but more emphasis is on new
> >> >> firmwares to be developed on this platform (target is developing
> >> >> firmware for hardware, but using qemu if hardware is not available
> >> >> temporarily), if virtio device used, then the newly developed firmware
> >> >> must include virtio front-end codes, but it isn't needed while run on
> >> >> real hardware at last.
> >> >
> >> > Right. The new firmwares and kernels would need to include virtio-pci nic
> >> > and scsi drivers. Is that a problem? Anyway, this is all the more reason
> >> > not to hard code peripherals. If a particular peripheral is a problem
> >> > for a given firmware, then simply don't add it to the command line, add a
> >> > different one.
> >> >
> >> Yes that is problem, for newly developed firmwares, extra efforts will
> >> be wasted on frontend codes (or glue-layer, whatever we call it), we
> >> want firmwares developed on this platform can run easily on real
> >> hardware, without such change.
> >> Requirement is: some Linaro members just want a qemu platform as true
> >> as possible with real hardware, there should be no problem with such
> >> requirement, problem is 'virt' machine cannot satisfy the requirement,
> >> so a new one is needed.
> >
> > It sounds like somebody knows what drivers are available and what
> > drivers aren't. If that's not already documented, then it should
> > be, and a pointer to it should be in this patch series.
> >
> 
> Available where?

Available in UEFI, ARM-TF, and the target guest kernel. What software
stack is this machine model targeting? I get the impression people
know what they need, but knowing and specifying with a document are
two different things.

> 
> UEFI has drivers for ?HCI industry standard hardware. As for the
> networking side, we should review whether E1000e is the most
> appropriate or this, given the lack of open source drivers. However, I
> do agree that discoverable hardware should not be hardcoded, and we
> should even try to use the emulated option ROM to provide a UEFI
> driver.

Amen to that.

> 
> >>
> >> >>
> >> >> >>  - No paravirtualized fw_cfg device either.
> >> >> >>
> >> >> >> Arm Trusted Firmware and UEFI porting to this are done accordingly.
> >> >> >>
> >> >> >
> >> >> > How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
> >> >> > see any sort of reserved ROM region in the patch for them.
> >> >> >
> >> >> UEFI gets ACPI and kernel from network or mass storage, just like the
> >> >> real hardware.
> >> >
> >> > Hmm. I thought for real hardware that the ACPI tables were built into
> >> > the firmware. So, assuming UEFI knows how to read ACPI tables from
> >> > some storage, then how do the QEMU generated ACPI tables get into that
> >> > storage?
> >> >
> >> I should say "mass storage and flash"
> >> There was fw_cfg in v1 patch, it is removed in v2.
> >> If no fw_cfg, just like real hardware, UEFI should include ACPI
> >> support for this SBSA platform, and UEFI/ACPI is load via -pflash,
> >> then the QEMU built-in ACPI isn't used.
> >> But there are side effects too, command 'qemu -bios uefi -kernel'
> >> won't work, I need extra time to evaluate this change.
> >
> > Right. Neither ACPI nor '-bios ... -kernel ...' can work without fw-cfg.
> > This patch either needs to keep fw-cfg, or to remove the ACPI changes.
> > I can't see how an SBSA reference platform would be much use without ACPI
> > though.
> >
> 
> Even if mach-virt's ACPI code depends on fw_cfg currently, there is no
> reason whatsoever that this sbsa machine should not implement it like
> real hardware does, i.e., hard coded tables.
> 

I don't disagree, but there's no point in making QEMU ACPI generation
code changes that will never be consumed. This patch adds tables for
the hard coded ?HCI controllers to ACPI. We don't need those changes for
the virt machine and, without fw-cfg, you can't use them on the reference
machine.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 26 July 2018 at 13:11, Andrew Jones <drjones@redhat.com> wrote:
> On Thu, Jul 26, 2018 at 12:35:08PM +0200, Ard Biesheuvel wrote:
>> On 26 July 2018 at 12:28, Andrew Jones <drjones@redhat.com> wrote:
>> > On Thu, Jul 26, 2018 at 05:22:14PM +0800, Hongbo Zhang wrote:
>> >> On 25 July 2018 at 19:26, Andrew Jones <drjones@redhat.com> wrote:
>> >> > On Wed, Jul 25, 2018 at 06:22:17PM +0800, Hongbo Zhang wrote:
>> >> >> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
>> >> >> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>> >> >> >>  - Based on 'virt' machine type.
>> >> >> >>  - Re-designed memory map.
>> >> >> >>  - EL2 and EL3 are enabled by default.
>> >> >> >>  - GIC version 3 by default.
>> >> >> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
>> >> >> >>    can be added to it.
>> >> >> >>  - EHCI controller attached to system bus, with USB mouse and key board
>> >> >> >>    installed by default.
>> >> >> >>  - E1000E ethernet card on PCIE bus.
>> >> >> >>  - VGA display adaptor on PCIE bus.
>> >> >> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>> >> >> >>  - No virtio functions enabled, since this is to emulate real hardware.
>> >> >> >
>> >> >> > In the last review it was pointed out that using virtio-pci should still
>> >> >> > be "real" enough, so there's not much reason to avoid it. Well, unless
>> >> >> > there's some concern as to what drivers are available in the firmware and
>> >> >> > guest kernel. But that concern usually only applies to legacy firmwares
>> >> >> > and kernels, and therefore shouldn't apply to AArch64.
>> >> >> >
>> >> >> In real physical arm hardware, *HCI are system memory mapped, not on PCIE.
>> >> >> we need a QEMU platform like that. We need firmware developed on this
>> >> >> QEMU platform can run on real hardware without change(or only a minor
>> >> >> change)
>> >> >
>> >> > virtio-pci has nothing to do with *HCI. You're adding an E1000e to the
>> >> > PCIe bus instead of a virtio-pci nic. Why?
>> >> >
>> >> No virtio devices are need on this platform, so no virtio-pci either,
>> >> on the real Arm server hardware, a NIC is inserted into PCIE, and
>> >> E1000E is a typical one.
>> >
>> > It is possible to make a real piece of hardware that really goes in a PCIe
>> > slot which knows how to talk VirtIO. The fact that an E1000e driver will
>> > drive an E1000e QEMU model instead of a VirtIO driver driving a VirtIO
>> > backend is, to me, pretty arbitrary. The only reason it should matter for
>> > the guest firmware/kernel is whether or not the firmware/kernel will have
>> > VirtIO drivers available. Do we know that? Is it documented somewhere
>> > that the guest firmware/kernel is guaranteed to have E1000e drivers, but
>> > VirtIO drivers are optional, or even forbidden? If so, where's that
>> > document?
>> >
>>
>> It is not pretty arbitrary. One is paravirtualization and one is not.
>
> But the paravirtness is a driver detail, not a platform detail. The
> virtio-pci device is just a PCIe device to the platform. Drive it or
> not, drive it with knowledge that it's paravirt or not, the platform
> doesn't care.
>

That may be true. But we'll still end up with a UEFI build that has
OVMF virtio bus drivers and device drivers included, blurring the line
between emulation and virtualization.

>>
>> >>
>> >> >> Concern is not only available firmwares, but more emphasis is on new
>> >> >> firmwares to be developed on this platform (target is developing
>> >> >> firmware for hardware, but using qemu if hardware is not available
>> >> >> temporarily), if virtio device used, then the newly developed firmware
>> >> >> must include virtio front-end codes, but it isn't needed while run on
>> >> >> real hardware at last.
>> >> >
>> >> > Right. The new firmwares and kernels would need to include virtio-pci nic
>> >> > and scsi drivers. Is that a problem? Anyway, this is all the more reason
>> >> > not to hard code peripherals. If a particular peripheral is a problem
>> >> > for a given firmware, then simply don't add it to the command line, add a
>> >> > different one.
>> >> >
>> >> Yes that is problem, for newly developed firmwares, extra efforts will
>> >> be wasted on frontend codes (or glue-layer, whatever we call it), we
>> >> want firmwares developed on this platform can run easily on real
>> >> hardware, without such change.
>> >> Requirement is: some Linaro members just want a qemu platform as true
>> >> as possible with real hardware, there should be no problem with such
>> >> requirement, problem is 'virt' machine cannot satisfy the requirement,
>> >> so a new one is needed.
>> >
>> > It sounds like somebody knows what drivers are available and what
>> > drivers aren't. If that's not already documented, then it should
>> > be, and a pointer to it should be in this patch series.
>> >
>>
>> Available where?
>
> Available in UEFI, ARM-TF, and the target guest kernel. What software
> stack is this machine model targeting? I get the impression people
> know what they need, but knowing and specifying with a document are
> two different things.
>

Right.

>>
>> UEFI has drivers for ?HCI industry standard hardware. As for the
>> networking side, we should review whether E1000e is the most
>> appropriate or this, given the lack of open source drivers. However, I
>> do agree that discoverable hardware should not be hardcoded, and we
>> should even try to use the emulated option ROM to provide a UEFI
>> driver.
>
> Amen to that.
>
>>
>> >>
>> >> >>
>> >> >> >>  - No paravirtualized fw_cfg device either.
>> >> >> >>
>> >> >> >> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>> >> >> >>
>> >> >> >
>> >> >> > How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
>> >> >> > see any sort of reserved ROM region in the patch for them.
>> >> >> >
>> >> >> UEFI gets ACPI and kernel from network or mass storage, just like the
>> >> >> real hardware.
>> >> >
>> >> > Hmm. I thought for real hardware that the ACPI tables were built into
>> >> > the firmware. So, assuming UEFI knows how to read ACPI tables from
>> >> > some storage, then how do the QEMU generated ACPI tables get into that
>> >> > storage?
>> >> >
>> >> I should say "mass storage and flash"
>> >> There was fw_cfg in v1 patch, it is removed in v2.
>> >> If no fw_cfg, just like real hardware, UEFI should include ACPI
>> >> support for this SBSA platform, and UEFI/ACPI is load via -pflash,
>> >> then the QEMU built-in ACPI isn't used.
>> >> But there are side effects too, command 'qemu -bios uefi -kernel'
>> >> won't work, I need extra time to evaluate this change.
>> >
>> > Right. Neither ACPI nor '-bios ... -kernel ...' can work without fw-cfg.
>> > This patch either needs to keep fw-cfg, or to remove the ACPI changes.
>> > I can't see how an SBSA reference platform would be much use without ACPI
>> > though.
>> >
>>
>> Even if mach-virt's ACPI code depends on fw_cfg currently, there is no
>> reason whatsoever that this sbsa machine should not implement it like
>> real hardware does, i.e., hard coded tables.
>>
>
> I don't disagree, but there's no point in making QEMU ACPI generation
> code changes that will never be consumed. This patch adds tables for
> the hard coded ?HCI controllers to ACPI. We don't need those changes for
> the virt machine and, without fw-cfg, you can't use them on the reference
> machine.
>

Ah, indeed. I missed that bit.

We should not include any changes that modify the DT node or ACPI
table generation for mach-virt.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Thu, Jul 26, 2018 at 01:15:15PM +0200, Ard Biesheuvel wrote:
> That may be true. But we'll still end up with a UEFI build that has
> OVMF virtio bus drivers and device drivers included, blurring the line
> between emulation and virtualization.

The UEFI build can drop the virtio-mmio support, and all the virtio-mmio
device drivers. Those haven't been necessary since the virt machine got
a PCIe host bridge. You'd still need the virtio-pci device drivers, which
may or may not "taint" the reference code, depending on its requirements.

> > I don't disagree, but there's no point in making QEMU ACPI generation
> > code changes that will never be consumed. This patch adds tables for
> > the hard coded ?HCI controllers to ACPI. We don't need those changes for
> > the virt machine and, without fw-cfg, you can't use them on the reference
> > machine.
> >
> 
> Ah, indeed. I missed that bit.
> 
> We should not include any changes that modify the DT node or ACPI
> table generation for mach-virt.
>

The patch guards the generation. It'll only modify DT and ACPI for the
new machine type. But, while modifying the DT makes sense, as that
generated DT will get consumed, the ACPI tables have no way of being
consumed without fw-cfg. If the new machine type doesn't want fw-cfg,
then there's no need to touch any ACPI generation code at all in this
patch.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 26 July 2018 at 12:41, Andrew Jones <drjones@redhat.com> wrote:
> The patch guards the generation. It'll only modify DT and ACPI for the
> new machine type. But, while modifying the DT makes sense, as that
> generated DT will get consumed

...will it? Why would we want the firmware to read the
QEMU-generated DT? Real hardware doesn't work that way.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Thu, Jul 26, 2018 at 01:10:34PM +0100, Peter Maydell wrote:
> On 26 July 2018 at 12:41, Andrew Jones <drjones@redhat.com> wrote:
> > The patch guards the generation. It'll only modify DT and ACPI for the
> > new machine type. But, while modifying the DT makes sense, as that
> > generated DT will get consumed
> 
> ...will it? Why would we want the firmware to read the
> QEMU-generated DT? Real hardware doesn't work that way.
>

Good point. If the plan for this reference software is to always
prepare its own DTB and ACPI tables, then this patch shouldn't
touch the DT generation either. Of course a lot of the device
and fdt node creation is intertwined in mach-virt, so it's going
to create a DTB anyway.

(Unless major refactoring is done first...)

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 26 July 2018 at 13:35, Andrew Jones <drjones@redhat.com> wrote:
> On Thu, Jul 26, 2018 at 01:10:34PM +0100, Peter Maydell wrote:
>> On 26 July 2018 at 12:41, Andrew Jones <drjones@redhat.com> wrote:
>> > The patch guards the generation. It'll only modify DT and ACPI for the
>> > new machine type. But, while modifying the DT makes sense, as that
>> > generated DT will get consumed
>>
>> ...will it? Why would we want the firmware to read the
>> QEMU-generated DT? Real hardware doesn't work that way.
>>
>
> Good point. If the plan for this reference software is to always
> prepare its own DTB and ACPI tables, then this patch shouldn't
> touch the DT generation either. Of course a lot of the device
> and fdt node creation is intertwined in mach-virt, so it's going
> to create a DTB anyway.

I haven't yet looked at this patch so I might change my mind
once I've had time to look at the code, but my initial
thought is to prefer it to be in its own file rather than
trying to share code with the virt board. There's a lot
of stuff 'virt' needs that this doesn't (DT generation,
ACPI generation, switches to disable virtualization or
trustzone support, options to select GICv2, etc etc).

Q: is this new board model intended to be able to work
under KVM, or is that out of scope? (You wouldn't be able
to run guest EL3 firmware under KVM, certainly.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 26 July 2018 at 20:43, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 July 2018 at 13:35, Andrew Jones <drjones@redhat.com> wrote:
>> On Thu, Jul 26, 2018 at 01:10:34PM +0100, Peter Maydell wrote:
>>> On 26 July 2018 at 12:41, Andrew Jones <drjones@redhat.com> wrote:
>>> > The patch guards the generation. It'll only modify DT and ACPI for the
>>> > new machine type. But, while modifying the DT makes sense, as that
>>> > generated DT will get consumed
>>>
>>> ...will it? Why would we want the firmware to read the
>>> QEMU-generated DT? Real hardware doesn't work that way.
>>>
>>
>> Good point. If the plan for this reference software is to always
>> prepare its own DTB and ACPI tables, then this patch shouldn't
>> touch the DT generation either. Of course a lot of the device
>> and fdt node creation is intertwined in mach-virt, so it's going
>> to create a DTB anyway.
>
> I haven't yet looked at this patch so I might change my mind
> once I've had time to look at the code, but my initial
> thought is to prefer it to be in its own file rather than
> trying to share code with the virt board. There's a lot
> of stuff 'virt' needs that this doesn't (DT generation,
> ACPI generation, switches to disable virtualization or
> trustzone support, options to select GICv2, etc etc).
>
> Q: is this new board model intended to be able to work
> under KVM, or is that out of scope? (You wouldn't be able
> to run guest EL3 firmware under KVM, certainly.)
>
KVM is out of scope.

> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 26 July 2018 at 20:43, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 July 2018 at 13:35, Andrew Jones <drjones@redhat.com> wrote:
>> On Thu, Jul 26, 2018 at 01:10:34PM +0100, Peter Maydell wrote:
>>> On 26 July 2018 at 12:41, Andrew Jones <drjones@redhat.com> wrote:
>>> > The patch guards the generation. It'll only modify DT and ACPI for the
>>> > new machine type. But, while modifying the DT makes sense, as that
>>> > generated DT will get consumed
>>>
>>> ...will it? Why would we want the firmware to read the
>>> QEMU-generated DT? Real hardware doesn't work that way.
>>>
>>
>> Good point. If the plan for this reference software is to always
>> prepare its own DTB and ACPI tables, then this patch shouldn't
>> touch the DT generation either. Of course a lot of the device
>> and fdt node creation is intertwined in mach-virt, so it's going
>> to create a DTB anyway.
>
> I haven't yet looked at this patch so I might change my mind
> once I've had time to look at the code, but my initial
> thought is to prefer it to be in its own file rather than
> trying to share code with the virt board. There's a lot
> of stuff 'virt' needs that this doesn't (DT generation,
> ACPI generation, switches to disable virtualization or
> trustzone support, options to select GICv2, etc etc).
>
The 'sbsa' machine won't consume QEMU generated ACPI, so it won't
touch or add new ACPI tables.

UEFI relies on its ACPI to load OS, but currently it still needs DT
from QEMU to pass some info, one example is CPU number.

So, the 'sbsa' code implementation should be like this:
A separate file, no ACPI codes, a little necessary DT codes.

"Necessary DT codes" doesn't include so many peripheral devices nodes,
so the code overlap with virt won't be so much (contrary to sbsa.c
with all the DT codes), then no need to separate the common part to a
third file, and virt.c can be untouched or only a minor change with
few lines.

Any comments please?

> Q: is this new board model intended to be able to work
> under KVM, or is that out of scope? (You wouldn't be able
> to run guest EL3 firmware under KVM, certainly.)
>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 3 August 2018 at 10:21, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> The 'sbsa' machine won't consume QEMU generated ACPI, so it won't
> touch or add new ACPI tables.
>
> UEFI relies on its ACPI to load OS, but currently it still needs DT
> from QEMU to pass some info, one example is CPU number.
>
> So, the 'sbsa' code implementation should be like this:
> A separate file, no ACPI codes, a little necessary DT codes.
>
> "Necessary DT codes" doesn't include so many peripheral devices nodes,
> so the code overlap with virt won't be so much (contrary to sbsa.c
> with all the DT codes), then no need to separate the common part to a
> third file, and virt.c can be untouched or only a minor change with
> few lines.

Would the real hardware you are trying to be an example
for use DT for this? It seems a bit unlikely to me.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 3 August 2018 at 11:23, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 August 2018 at 10:21, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>> The 'sbsa' machine won't consume QEMU generated ACPI, so it won't
>> touch or add new ACPI tables.
>>
>> UEFI relies on its ACPI to load OS, but currently it still needs DT
>> from QEMU to pass some info, one example is CPU number.
>>
>> So, the 'sbsa' code implementation should be like this:
>> A separate file, no ACPI codes, a little necessary DT codes.
>>
>> "Necessary DT codes" doesn't include so many peripheral devices nodes,
>> so the code overlap with virt won't be so much (contrary to sbsa.c
>> with all the DT codes), then no need to separate the common part to a
>> third file, and virt.c can be untouched or only a minor change with
>> few lines.
>
> Would the real hardware you are trying to be an example
> for use DT for this? It seems a bit unlikely to me.
>

Yes, as a matter of fact. There is work underway both on the EDK2 and
the ARM-TF side to rely less on static descriptions, and more on DT to
instantiate drivers and ACPI tables at runtime rather than at build
time.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Fri, Aug 03, 2018 at 11:26:41AM +0200, Ard Biesheuvel wrote:
> On 3 August 2018 at 11:23, Peter Maydell <peter.maydell@linaro.org> wrote:
> > On 3 August 2018 at 10:21, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> >> The 'sbsa' machine won't consume QEMU generated ACPI, so it won't
> >> touch or add new ACPI tables.
> >>
> >> UEFI relies on its ACPI to load OS, but currently it still needs DT
> >> from QEMU to pass some info, one example is CPU number.
> >>
> >> So, the 'sbsa' code implementation should be like this:
> >> A separate file, no ACPI codes, a little necessary DT codes.
> >>
> >> "Necessary DT codes" doesn't include so many peripheral devices nodes,
> >> so the code overlap with virt won't be so much (contrary to sbsa.c
> >> with all the DT codes), then no need to separate the common part to a
> >> third file, and virt.c can be untouched or only a minor change with
> >> few lines.
> >
> > Would the real hardware you are trying to be an example
> > for use DT for this? It seems a bit unlikely to me.
> >
> 
> Yes, as a matter of fact. There is work underway both on the EDK2 and
> the ARM-TF side to rely less on static descriptions, and more on DT to
> instantiate drivers and ACPI tables at runtime rather than at build
> time.

Hi Ard,

(A bit off-topic, but related to your comment above.)

I started poking at teaching ArmVirtQemu how to get the base of RAM
from the DTB, where the DTB is passed to it through x1 (QEMU needed a
few new lines to ensure x1 had it in the firmware=y,-kernel=no case
too). I have the assembler written, but I got hung up on the integration
with edk2, because I don't understand the build files well enough yet to
swap in ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S for the ArmVirtQemu
platform. I would also need to clean up the assembler a bit before
posting. I'll be out-of-office for all next week, but if you send me
some edk2 pointers in the meantime, then I should be able to post an
RFC the following week.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Laszlo Ersek]

Hi Drew,

On 08/03/18 11:37, Andrew Jones wrote:
> On Fri, Aug 03, 2018 at 11:26:41AM +0200, Ard Biesheuvel wrote:
>> On 3 August 2018 at 11:23, Peter Maydell <peter.maydell@linaro.org>
>> wrote:
>>> On 3 August 2018 at 10:21, Hongbo Zhang <hongbo.zhang@linaro.org>
>>> wrote:
>>>> The 'sbsa' machine won't consume QEMU generated ACPI, so it won't
>>>> touch or add new ACPI tables.
>>>>
>>>> UEFI relies on its ACPI to load OS, but currently it still needs DT
>>>> from QEMU to pass some info, one example is CPU number.
>>>>
>>>> So, the 'sbsa' code implementation should be like this:
>>>> A separate file, no ACPI codes, a little necessary DT codes.
>>>>
>>>> "Necessary DT codes" doesn't include so many peripheral devices
>>>> nodes, so the code overlap with virt won't be so much (contrary to
>>>> sbsa.c with all the DT codes), then no need to separate the common
>>>> part to a third file, and virt.c can be untouched or only a minor
>>>> change with few lines.
>>>
>>> Would the real hardware you are trying to be an example
>>> for use DT for this? It seems a bit unlikely to me.
>>>
>>
>> Yes, as a matter of fact. There is work underway both on the EDK2 and
>> the ARM-TF side to rely less on static descriptions, and more on DT
>> to instantiate drivers and ACPI tables at runtime rather than at
>> build time.
>
> Hi Ard,
>
> (A bit off-topic, but related to your comment above.)
>
> I started poking at teaching ArmVirtQemu how to get the base of RAM
> from the DTB, where the DTB is passed to it through x1 (QEMU needed a
> few new lines to ensure x1 had it in the firmware=y,-kernel=no case
> too). I have the assembler written, but I got hung up on the
> integration with edk2, because I don't understand the build files well
> enough yet to swap in ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S for
> the ArmVirtQemu platform. I would also need to clean up the assembler
> a bit before posting. I'll be out-of-office for all next week, but if
> you send me some edk2 pointers in the meantime, then I should be able
> to post an RFC the following week.

"ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S" is part of the
"ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf" module.

This module is built as part of two platforms:

- ArmVirtPkg/ArmVirtQemuKernel
- ArmVirtPkg/ArmVirtXen

For each of those platforms, you'll find two files: DSC and FDF. DSC
("platform description") basically specifies what modules to build (and
with what settings), while FDF ("flash device file") specifies how to
lay out the flash device (i.e. how to include the EFI binaries built by
the DSC into a firmware image). The DSC file is the starting point. The
FDF file is referenced by the DSC (see [Defines] | FLASH_DEFINITION).

Now, based on the above, "ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S"
is not the file that should be modified. Under ArmVirtPkg, three
firmware platforms exist in total:

- ArmVirtPkg/ArmVirtQemu
- ArmVirtPkg/ArmVirtQemuKernel
- ArmVirtPkg/ArmVirtXen

The first platform (ArmVirtQemu) is what gets booted from pflash.

The second platform (ArmVirtQemuKernel) is booted with the "-kernel"
QEMU option, *without* pflash (i.e. it is launched like a "naked" Linux
kernel).

The third platform (ArmVirtXen) is for booting in a Xen (PV)HVM VM.

The second and third firmware platforms (ArmVirtQemuKernel and
ArmVirtXen) share the trait that the UEFI payload is not the very first
payload to run in the guest. They are both launched by an earlier stage,
and they are loaded immediately into r/w memory. This has a big effect
on their entry points, and it gives them a lot more freedom in the
earliest code.

Because these firmware platforms (ArmVirtQemuKernel and ArmVirtXen)
share this trait, they both use the

  ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf

SEC driver, which includes the assembly file you named. (You can see
"SEC" from MODULE_TYPE in the INF file.) The module says "relocatable"
in the name because, due to existing in r/w memory from the start, it is
at liberty to patch various things on the fly, for relocating itself.


The same does not apply to the ArmVirtQemu platform. It gets loaded into
pflash, as the very first payload to run. During the 1st (SEC) phase,
and until late into the 2nd (PEI) phase, code and global variables are
read-only. In other words, there's nothing we can patch at runtime.

Accordingly, ArmVirtQemu uses a different SEC driver; namely, it pulls
in:

  ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf

for which the first assembly-language file is

  ArmPlatformPkg/PrePeiCore/AArch64/PrePeiCoreEntryPoint.S

Initially, only the stack is writeable -- and the location of the stack
is also hard-coded: see "PcdCPUCoresStackBase" in "ArmVirtQemu.dsc".

(The differences between the two *kinds* of platforms are explained
somewhat at
<https://github.com/tianocore/tianocore.github.io/wiki/ArmPlatformPkg>.
Unfortunately, the most important diagram that explains the boot flow
differences between "1st stage payload" and "2nd stage payload", namely
"ARM-EDK2-Overview.png", seems to have been lost when the TianoCore wiki
was moved from sourceforge to github. Sigh.)

In my earlier email

  dynamic DRAM base for ArmVirtQemu
  http://mid.mail-archive.com/4cce2b8b-a411-bd5d-a06f-b0b80a5fb2f1@redhat.com

I investigated what it would take to adapt this SEC driver,
"ArmPlatformPkg/PrePeiCore" (and other drivers that depend on it, and
run from pflash) to a dynamic RAM base.

First of all, the driver would have to be cloned from ArmPlatformPkg to
ArmVirtPkg (becasue we shouldn't / can't modify ArmPlatformPkg for this
paravirt feature). Then all the changes should occur under ArmVirtPkg.

Those would begin with replacing the fixed PcdCPUCoresStackBase (i.e.,
the location of the stack, which is the only writeable thing we start
out with) with calculating (and passing around) a stack base derived
from the initial x0 value. (This is bullet (1) in my above email.)

IOW, the cornerstone of this feature is to replace all constants
(FixedPCDs and otherwise) related to the RAM Base with values
dynamically calculated from the initial x0 value. My earlier email
details this as well -- at the earliest, the results can only be passed
around on the (dynamically configured) stack itself; a bit later, PPIs
(= structures identified by particular GUIDs, located originally in
temporary SEC/PEI RAM) can be used too. But, there's nothing ArmVirtQemu
can "patch" the way ArmVirtQemuKernel and ArmVirtXen do.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Fri, Aug 03, 2018 at 03:44:21PM +0200, Laszlo Ersek wrote:
> Hi Drew,
> 
> On 08/03/18 11:37, Andrew Jones wrote:
> > On Fri, Aug 03, 2018 at 11:26:41AM +0200, Ard Biesheuvel wrote:
> >> On 3 August 2018 at 11:23, Peter Maydell <peter.maydell@linaro.org>
> >> wrote:
> >>> On 3 August 2018 at 10:21, Hongbo Zhang <hongbo.zhang@linaro.org>
> >>> wrote:
> >>>> The 'sbsa' machine won't consume QEMU generated ACPI, so it won't
> >>>> touch or add new ACPI tables.
> >>>>
> >>>> UEFI relies on its ACPI to load OS, but currently it still needs DT
> >>>> from QEMU to pass some info, one example is CPU number.
> >>>>
> >>>> So, the 'sbsa' code implementation should be like this:
> >>>> A separate file, no ACPI codes, a little necessary DT codes.
> >>>>
> >>>> "Necessary DT codes" doesn't include so many peripheral devices
> >>>> nodes, so the code overlap with virt won't be so much (contrary to
> >>>> sbsa.c with all the DT codes), then no need to separate the common
> >>>> part to a third file, and virt.c can be untouched or only a minor
> >>>> change with few lines.
> >>>
> >>> Would the real hardware you are trying to be an example
> >>> for use DT for this? It seems a bit unlikely to me.
> >>>
> >>
> >> Yes, as a matter of fact. There is work underway both on the EDK2 and
> >> the ARM-TF side to rely less on static descriptions, and more on DT
> >> to instantiate drivers and ACPI tables at runtime rather than at
> >> build time.
> >
> > Hi Ard,
> >
> > (A bit off-topic, but related to your comment above.)
> >
> > I started poking at teaching ArmVirtQemu how to get the base of RAM
> > from the DTB, where the DTB is passed to it through x1 (QEMU needed a
> > few new lines to ensure x1 had it in the firmware=y,-kernel=no case
> > too). I have the assembler written, but I got hung up on the
> > integration with edk2, because I don't understand the build files well
> > enough yet to swap in ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S for
> > the ArmVirtQemu platform. I would also need to clean up the assembler
> > a bit before posting. I'll be out-of-office for all next week, but if
> > you send me some edk2 pointers in the meantime, then I should be able
> > to post an RFC the following week.

Of course I meant x0 above. (I have no idea why I wrote x1.)

> 
> "ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S" is part of the
> "ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf" module.
> 
> This module is built as part of two platforms:
> 
> - ArmVirtPkg/ArmVirtQemuKernel
> - ArmVirtPkg/ArmVirtXen
> 
> For each of those platforms, you'll find two files: DSC and FDF. DSC
> ("platform description") basically specifies what modules to build (and
> with what settings), while FDF ("flash device file") specifies how to
> lay out the flash device (i.e. how to include the EFI binaries built by
> the DSC into a firmware image). The DSC file is the starting point. The
> FDF file is referenced by the DSC (see [Defines] | FLASH_DEFINITION).
> 
> Now, based on the above, "ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S"
> is not the file that should be modified. Under ArmVirtPkg, three
> firmware platforms exist in total:
> 
> - ArmVirtPkg/ArmVirtQemu
> - ArmVirtPkg/ArmVirtQemuKernel
> - ArmVirtPkg/ArmVirtXen
> 
> The first platform (ArmVirtQemu) is what gets booted from pflash.
> 
> The second platform (ArmVirtQemuKernel) is booted with the "-kernel"
> QEMU option, *without* pflash (i.e. it is launched like a "naked" Linux
> kernel).
> 
> The third platform (ArmVirtXen) is for booting in a Xen (PV)HVM VM.
> 
> The second and third firmware platforms (ArmVirtQemuKernel and
> ArmVirtXen) share the trait that the UEFI payload is not the very first
> payload to run in the guest. They are both launched by an earlier stage,
> and they are loaded immediately into r/w memory. This has a big effect
> on their entry points, and it gives them a lot more freedom in the
> earliest code.
> 
> Because these firmware platforms (ArmVirtQemuKernel and ArmVirtXen)
> share this trait, they both use the
> 
>   ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> 
> SEC driver, which includes the assembly file you named. (You can see
> "SEC" from MODULE_TYPE in the INF file.) The module says "relocatable"
> in the name because, due to existing in r/w memory from the start, it is
> at liberty to patch various things on the fly, for relocating itself.
> 
> 
> The same does not apply to the ArmVirtQemu platform. It gets loaded into
> pflash, as the very first payload to run. During the 1st (SEC) phase,
> and until late into the 2nd (PEI) phase, code and global variables are
> read-only. In other words, there's nothing we can patch at runtime.
> 
> Accordingly, ArmVirtQemu uses a different SEC driver; namely, it pulls
> in:
> 
>   ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> 
> for which the first assembly-language file is
> 
>   ArmPlatformPkg/PrePeiCore/AArch64/PrePeiCoreEntryPoint.S

As always thank you Laszlo for the detailed explanation! I'll digest
it more when I'm back in the office.

> 
> Initially, only the stack is writeable -- and the location of the stack
> is also hard-coded: see "PcdCPUCoresStackBase" in "ArmVirtQemu.dsc".
> 
> (The differences between the two *kinds* of platforms are explained
> somewhat at
> <https://github.com/tianocore/tianocore.github.io/wiki/ArmPlatformPkg>.
> Unfortunately, the most important diagram that explains the boot flow
> differences between "1st stage payload" and "2nd stage payload", namely
> "ARM-EDK2-Overview.png", seems to have been lost when the TianoCore wiki
> was moved from sourceforge to github. Sigh.)
> 
> In my earlier email
> 
>   dynamic DRAM base for ArmVirtQemu
>   http://mid.mail-archive.com/4cce2b8b-a411-bd5d-a06f-b0b80a5fb2f1@redhat.com
> 
> I investigated what it would take to adapt this SEC driver,
> "ArmPlatformPkg/PrePeiCore" (and other drivers that depend on it, and
> run from pflash) to a dynamic RAM base.
> 
> First of all, the driver would have to be cloned from ArmPlatformPkg to
> ArmVirtPkg (becasue we shouldn't / can't modify ArmPlatformPkg for this
> paravirt feature). Then all the changes should occur under ArmVirtPkg.
> 
> Those would begin with replacing the fixed PcdCPUCoresStackBase (i.e.,
> the location of the stack, which is the only writeable thing we start
> out with) with calculating (and passing around) a stack base derived
> from the initial x0 value. (This is bullet (1) in my above email.)

I'm actually attacking it without a stack. x0 will have the address of
the DTB, which may or may not also be the base address of main memory.
There's no need to code to the assumption it is in order to have a
stack. The code to parse the DT can all be done in assembler without
a stack.

> 
> IOW, the cornerstone of this feature is to replace all constants
> (FixedPCDs and otherwise) related to the RAM Base with values
> dynamically calculated from the initial x0 value. My earlier email
> details this as well -- at the earliest, the results can only be passed
> around on the (dynamically configured) stack itself; a bit later, PPIs
> (= structures identified by particular GUIDs, located originally in
> temporary SEC/PEI RAM) can be used too. But, there's nothing ArmVirtQemu
> can "patch" the way ArmVirtQemuKernel and ArmVirtXen do.

I need to digest this more, too, and to modify it to use the actual base
of memory found in the DT, rather then value of x0, which may or may
not be the same value.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Laszlo Ersek]

On 08/03/18 16:39, Andrew Jones wrote:
> On Fri, Aug 03, 2018 at 03:44:21PM +0200, Laszlo Ersek wrote:

>> In my earlier email
>>
>>   dynamic DRAM base for ArmVirtQemu
>>   http://mid.mail-archive.com/4cce2b8b-a411-bd5d-a06f-b0b80a5fb2f1@redhat.com
>>
>> I investigated what it would take to adapt this SEC driver,
>> "ArmPlatformPkg/PrePeiCore" (and other drivers that depend on it, and
>> run from pflash) to a dynamic RAM base.
>>
>> First of all, the driver would have to be cloned from ArmPlatformPkg to
>> ArmVirtPkg (becasue we shouldn't / can't modify ArmPlatformPkg for this
>> paravirt feature). Then all the changes should occur under ArmVirtPkg.
>>
>> Those would begin with replacing the fixed PcdCPUCoresStackBase (i.e.,
>> the location of the stack, which is the only writeable thing we start
>> out with) with calculating (and passing around) a stack base derived
>> from the initial x0 value. (This is bullet (1) in my above email.)
> 
> I'm actually attacking it without a stack. x0 will have the address of
> the DTB, which may or may not also be the base address of main memory.
> There's no need to code to the assumption it is in order to have a
> stack. The code to parse the DT can all be done in assembler without
> a stack.

That sounds very cunning :)

But, a stack will certainly be needed by the rest of the code, so even
if the DTB parsing itself needs no stack, later (but still early)
dependencies on PcdCPUCoresStackBase (and friends) will have to be replaced.

>> IOW, the cornerstone of this feature is to replace all constants
>> (FixedPCDs and otherwise) related to the RAM Base with values
>> dynamically calculated from the initial x0 value. My earlier email
>> details this as well -- at the earliest, the results can only be passed
>> around on the (dynamically configured) stack itself; a bit later, PPIs
>> (= structures identified by particular GUIDs, located originally in
>> temporary SEC/PEI RAM) can be used too. But, there's nothing ArmVirtQemu
>> can "patch" the way ArmVirtQemuKernel and ArmVirtXen do.
> 
> I need to digest this more, too, and to modify it to use the actual base
> of memory found in the DT, rather then value of x0, which may or may
> not be the same value.

Good point!

Laszlo

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 3 August 2018 at 10:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 3 August 2018 at 11:23, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Would the real hardware you are trying to be an example
>> for use DT for this? It seems a bit unlikely to me.
>>
>
> Yes, as a matter of fact. There is work underway both on the EDK2 and
> the ARM-TF side to rely less on static descriptions, and more on DT to
> instantiate drivers and ACPI tables at runtime rather than at build
> time.

Cool. (My motivation here is mostly to make sure that we
build a model in QEMU that makes the right design choices to
be an accurate reference platform, rather than doing things
simply because 'virt' happens to do them.)

Where would the hardware version of this put its DT ?
In a flash memory ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 3 August 2018 at 17:39, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 August 2018 at 10:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 3 August 2018 at 11:23, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> Would the real hardware you are trying to be an example
>>> for use DT for this? It seems a bit unlikely to me.
>>>
>>
>> Yes, as a matter of fact. There is work underway both on the EDK2 and
>> the ARM-TF side to rely less on static descriptions, and more on DT to
>> instantiate drivers and ACPI tables at runtime rather than at build
>> time.
>
> Cool. (My motivation here is mostly to make sure that we
> build a model in QEMU that makes the right design choices to
> be an accurate reference platform, rather than doing things
> simply because 'virt' happens to do them.)
>
> Where would the hardware version of this put its DT ?
> In a flash memory ?
>
I would mention that smbios relies on fw_cfg to be passed to UEFI too,
so without fw_cfg, a flash becomes the way for it.

> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>>  - Based on 'virt' machine type.
>>  - Re-designed memory map.
>>  - EL2 and EL3 are enabled by default.
>>  - GIC version 3 by default.
>>  - AHCI controller attached to system bus, and then CDROM and hard disc
>>    can be added to it.
>>  - EHCI controller attached to system bus, with USB mouse and key board
>>    installed by default.
>>  - E1000E ethernet card on PCIE bus.
>>  - VGA display adaptor on PCIE bus.
>>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>>  - No virtio functions enabled, since this is to emulate real hardware.
>
> In the last review it was pointed out that using virtio-pci should still
> be "real" enough, so there's not much reason to avoid it. Well, unless
> there's some concern as to what drivers are available in the firmware and
> guest kernel. But that concern usually only applies to legacy firmwares
> and kernels, and therefore shouldn't apply to AArch64.
>
For Armv7, there is one typical platform 'vexpress', but for Armv8, no
such typical one, the 'virt' is typically for running workloads, one
example is using it under OpenStack.
So a 'typical' one for Armv8 is needed for firmware and OS
development, similar like 'vexpress' for Armv7.

>>  - No paravirtualized fw_cfg device either.
>>
>> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>>
>
> How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
> see any sort of reserved ROM region in the patch for them.
>
> Thanks,
> drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Jul 25, 2018 at 06:46:59PM +0800, Hongbo Zhang wrote:
> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >>  - Based on 'virt' machine type.
> >>  - Re-designed memory map.
> >>  - EL2 and EL3 are enabled by default.
> >>  - GIC version 3 by default.
> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
> >>    can be added to it.
> >>  - EHCI controller attached to system bus, with USB mouse and key board
> >>    installed by default.
> >>  - E1000E ethernet card on PCIE bus.
> >>  - VGA display adaptor on PCIE bus.
> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
> >>  - No virtio functions enabled, since this is to emulate real hardware.
> >
> > In the last review it was pointed out that using virtio-pci should still
> > be "real" enough, so there's not much reason to avoid it. Well, unless
> > there's some concern as to what drivers are available in the firmware and
> > guest kernel. But that concern usually only applies to legacy firmwares
> > and kernels, and therefore shouldn't apply to AArch64.
> >
> For Armv7, there is one typical platform 'vexpress', but for Armv8, no

Wasn't the vexpress model designed for a specific machine? Namely for
Arm's simulator? Is the vexpress model really something typical among
all the Armv7 platforms?

> such typical one, the 'virt' is typically for running workloads, one
> example is using it under OpenStack.
> So a 'typical' one for Armv8 is needed for firmware and OS
> development, similar like 'vexpress' for Armv7.

What is a "typical" Armv8 machine? What will a typical Armv8 machine be in
two years?

Note, I'm not actually opposed to the current definition (because I don't
really have one myself). I'm just opposed to hard coding one.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 25 July 2018 at 12:44, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 06:46:59PM +0800, Hongbo Zhang wrote:
>> For Armv7, there is one typical platform 'vexpress', but for Armv8, no
>
> Wasn't the vexpress model designed for a specific machine?

Yes.

> Namely for
> Arm's simulator?

No.

> Is the vexpress model really something typical among
> all the Armv7 platforms?

No.

"Vexpress" is a model specifically of a development board
produced by Arm (the versatile express). It's useful if you
want to run code that runs on that devboard, but (as with
most of the devboards we model), it's not necessarily ideal,
because it has all the limitations of the real hardware it's
modelling (in this case the big ones are limited memory, no PCI).
The hardware it models is also quite old now (maybe 7 or 8 years)
and it's not really "typical" of anything. (In the primarily
embedded space where most v7 CPUs are there's not really anything
that could be described as "typical" anyway: everything is
different.)

For most people who just want to run Linux on an emulated v7 CPU,
I would recommend the "virt" board, for the same reasons I
recommend it for v8 cores.

>> such typical one, the 'virt' is typically for running workloads, one
>> example is using it under OpenStack.
>> So a 'typical' one for Armv8 is needed for firmware and OS
>> development, similar like 'vexpress' for Armv7.
>
> What is a "typical" Armv8 machine? What will a typical Armv8 machine be in
> two years?
>
> Note, I'm not actually opposed to the current definition (because I don't
> really have one myself). I'm just opposed to hard coding one.

AIUI the aim here is to provide an emulated platform that is
set up in the way that server-style armv8 machines are
recommended to be set up, so it can be used as a testbed and
demonstration for the firmware/OS software stack. The hope
is that following "best practices" results in a "typical"
machine :-)  But the word "typical" is probably not really
very helpful here...

I would expect that in the future we'd want this machine type
to evolve with the recommendations for how to build server
platform hardware, which might indeed be different in two years,
since it would be the development platform for writing/testing
the firmware/OS stack for that two-years-time hardware.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 25 July 2018 at 14:19, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 25 July 2018 at 12:44, Andrew Jones <drjones@redhat.com> wrote:
>> On Wed, Jul 25, 2018 at 06:46:59PM +0800, Hongbo Zhang wrote:
>>> For Armv7, there is one typical platform 'vexpress', but for Armv8, no
>>
>> Wasn't the vexpress model designed for a specific machine?
>
> Yes.
>
>> Namely for
>> Arm's simulator?
>
> No.
>
>> Is the vexpress model really something typical among
>> all the Armv7 platforms?
>
> No.
>
> "Vexpress" is a model specifically of a development board
> produced by Arm (the versatile express). It's useful if you
> want to run code that runs on that devboard, but (as with
> most of the devboards we model), it's not necessarily ideal,
> because it has all the limitations of the real hardware it's
> modelling (in this case the big ones are limited memory, no PCI).
> The hardware it models is also quite old now (maybe 7 or 8 years)
> and it's not really "typical" of anything. (In the primarily
> embedded space where most v7 CPUs are there's not really anything
> that could be described as "typical" anyway: everything is
> different.)
>
> For most people who just want to run Linux on an emulated v7 CPU,
> I would recommend the "virt" board, for the same reasons I
> recommend it for v8 cores.
>
>>> such typical one, the 'virt' is typically for running workloads, one
>>> example is using it under OpenStack.
>>> So a 'typical' one for Armv8 is needed for firmware and OS
>>> development, similar like 'vexpress' for Armv7.
>>
>> What is a "typical" Armv8 machine? What will a typical Armv8 machine be in
>> two years?
>>
>> Note, I'm not actually opposed to the current definition (because I don't
>> really have one myself). I'm just opposed to hard coding one.
>
> AIUI the aim here is to provide an emulated platform that is
> set up in the way that server-style armv8 machines are
> recommended to be set up, so it can be used as a testbed and
> demonstration for the firmware/OS software stack. The hope
> is that following "best practices" results in a "typical"
> machine :-)  But the word "typical" is probably not really
> very helpful here...
>
> I would expect that in the future we'd want this machine type
> to evolve with the recommendations for how to build server
> platform hardware, which might indeed be different in two years,
> since it would be the development platform for writing/testing
> the firmware/OS stack for that two-years-time hardware.
>

To prevent spending a disproportionate amount of time on inventing
ways to parameterize these 'models', which includes interfaces not
only between UEFI and QEMU but also between ARM-TF and QEMU (and
potentially between UEFI and ARM-TF as well), could we please consider
fixed configurations for the non-discoverable bits? If there is a new
and improved sbsa model in the future, we could call it sbsa-2.0, no?

On the firmware side, I would like to implement this using build time
configuration options only, with the exception of memory size and
other things that are expected to be runtime discoverable on the real
world counterparts of our model.

I should also note that Hongbo's remark about running such firmware on
real hardware as well is slightly off the mark: we want to *model*
real hardware rather than a guest machine abstraction, but we have no
intention to share firmware builds between the sbsa model and physical
hardware.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 25 July 2018 at 13:29, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> To prevent spending a disproportionate amount of time on inventing
> ways to parameterize these 'models', which includes interfaces not
> only between UEFI and QEMU but also between ARM-TF and QEMU (and
> potentially between UEFI and ARM-TF as well), could we please consider
> fixed configurations for the non-discoverable bits?

Yes, I had assumed that would be the way this would work. After
all for the hardware setup we're trying to be a "best practices/
reference" demonstration for, the firmware would consider all
the non-discoverable hardware to be basically fixed, right?

(Maybe we could have some sort of "board revision" register so the
firmware could figure out if it was looking at the equivalent
of board rev A/B/C and whether some of the fixed devices exist,
but I think we should only do that if it's the kind of thing
that we're expecting/recommending real h/w manufacturers do.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Jul 25, 2018 at 02:29:09PM +0200, Ard Biesheuvel wrote:
> To prevent spending a disproportionate amount of time on inventing
> ways to parameterize these 'models', which includes interfaces not
> only between UEFI and QEMU but also between ARM-TF and QEMU (and
> potentially between UEFI and ARM-TF as well), could we please consider

I guess a UEFI <=> ARM-TF interface will also need to work on real
hardware, and therefore need a QEMU model. If whatever that interface
needs can also satisfy the UEFI <=> QEMU and ARM-TF <=> interfaces,
then that's all we need - no need for fw-cfg.

> fixed configurations for the non-discoverable bits? If there is a new

Well, at least one thing must to be fixed, which is

 a) address/register where the address of the DTB will be stored
 b) address of the QEMU interface device, e.g. fw-cfg. fw-cfg or
    whatever can then be used to get the DTB.

And I guess everything the SBSA says should be fixed should be fixed.
Otherwise, what's non-discoverable?

> and improved sbsa model in the future, we could call it sbsa-2.0, no?

Of course the model can be versioned, allowing for any change, but
I don't think model versions were meant to be aliases for peripheral
selections. That's what scripts wrapped around qemu are for.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Daniel P. Berrangé]

On Wed, Jul 25, 2018 at 01:19:09PM +0100, Peter Maydell wrote:
> On 25 July 2018 at 12:44, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 06:46:59PM +0800, Hongbo Zhang wrote:
> >> For Armv7, there is one typical platform 'vexpress', but for Armv8, no
> >
> > Wasn't the vexpress model designed for a specific machine?
> 
> Yes.
> 
> > Namely for
> > Arm's simulator?
> 
> No.
> 
> > Is the vexpress model really something typical among
> > all the Armv7 platforms?
> 
> No.
> 
> "Vexpress" is a model specifically of a development board
> produced by Arm (the versatile express). It's useful if you
> want to run code that runs on that devboard, but (as with
> most of the devboards we model), it's not necessarily ideal,
> because it has all the limitations of the real hardware it's
> modelling (in this case the big ones are limited memory, no PCI).
> The hardware it models is also quite old now (maybe 7 or 8 years)
> and it's not really "typical" of anything. (In the primarily
> embedded space where most v7 CPUs are there's not really anything
> that could be described as "typical" anyway: everything is
> different.)
> 
> For most people who just want to run Linux on an emulated v7 CPU,
> I would recommend the "virt" board, for the same reasons I
> recommend it for v8 cores.
> 
> >> such typical one, the 'virt' is typically for running workloads, one
> >> example is using it under OpenStack.
> >> So a 'typical' one for Armv8 is needed for firmware and OS
> >> development, similar like 'vexpress' for Armv7.
> >
> > What is a "typical" Armv8 machine? What will a typical Armv8 machine be in
> > two years?
> >
> > Note, I'm not actually opposed to the current definition (because I don't
> > really have one myself). I'm just opposed to hard coding one.
> 
> AIUI the aim here is to provide an emulated platform that is
> set up in the way that server-style armv8 machines are
> recommended to be set up, so it can be used as a testbed and
> demonstration for the firmware/OS software stack. The hope
> is that following "best practices" results in a "typical"
> machine :-)  But the word "typical" is probably not really
> very helpful here...
> 
> I would expect that in the future we'd want this machine type
> to evolve with the recommendations for how to build server
> platform hardware, which might indeed be different in two years,
> since it would be the development platform for writing/testing
> the firmware/OS stack for that two-years-time hardware.

Would iut make any sense to call the machine  "refplatform"  or "refboard"
to indicate it is a generic reference platform, not specifically following
any particular real impl, albeit influence by the sbsa spec.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Jul 25, 2018 at 01:47:00PM +0100, Daniel P. Berrangé wrote:
> Would iut make any sense to call the machine  "refplatform"  or "refboard"
> to indicate it is a generic reference platform, not specifically following
> any particular real impl, albeit influence by the sbsa spec.
>

That would indeed stop me from whining about a machine model with an
'sbsa' name not strictly implementing a minimal SBSA machine :-)

drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 25 July 2018 at 14:59, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 01:47:00PM +0100, Daniel P. Berrangé wrote:
>> Would iut make any sense to call the machine  "refplatform"  or "refboard"
>> to indicate it is a generic reference platform, not specifically following
>> any particular real impl, albeit influence by the sbsa spec.
>>
>
> That would indeed stop me from whining about a machine model with an
> 'sbsa' name not strictly implementing a minimal SBSA machine :-)
>

I still don't get why only a minimal machine is worth considering,
given that a real-world minimal SBSA machine is not capable of doing
anything useful.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Jul 25, 2018 at 03:03:40PM +0200, Ard Biesheuvel wrote:
> On 25 July 2018 at 14:59, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 01:47:00PM +0100, Daniel P. Berrangé wrote:
> >> Would iut make any sense to call the machine  "refplatform"  or "refboard"
> >> to indicate it is a generic reference platform, not specifically following
> >> any particular real impl, albeit influence by the sbsa spec.
> >>
> >
> > That would indeed stop me from whining about a machine model with an
> > 'sbsa' name not strictly implementing a minimal SBSA machine :-)
> >
> 
> I still don't get why only a minimal machine is worth considering,
> given that a real-world minimal SBSA machine is not capable of doing
> anything useful.

One definition of an SBSA machine can be quite different than another.
If we only hard code the required [useless] base, but also provide a
readconfig for the rest, then we get a useful machine without loss of
flexibility.

That flexibility comes at the cost of platform-bus code (since we need
to add devices to the system bus) and a less concise command line. The
platform-bus code may indeed be too expensive for this purpose, but
we may need to see patches to be sure. I understand the desire to have
a shorter command line, but this is QEMU :)

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 25 July 2018 at 15:38, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 03:03:40PM +0200, Ard Biesheuvel wrote:
>> On 25 July 2018 at 14:59, Andrew Jones <drjones@redhat.com> wrote:
>> > On Wed, Jul 25, 2018 at 01:47:00PM +0100, Daniel P. Berrangé wrote:
>> >> Would iut make any sense to call the machine  "refplatform"  or "refboard"
>> >> to indicate it is a generic reference platform, not specifically following
>> >> any particular real impl, albeit influence by the sbsa spec.
>> >>
>> >
>> > That would indeed stop me from whining about a machine model with an
>> > 'sbsa' name not strictly implementing a minimal SBSA machine :-)
>> >
>>
>> I still don't get why only a minimal machine is worth considering,
>> given that a real-world minimal SBSA machine is not capable of doing
>> anything useful.
>
> One definition of an SBSA machine can be quite different than another.
> If we only hard code the required [useless] base, but also provide a
> readconfig for the rest, then we get a useful machine without loss of
> flexibility.
>
> That flexibility comes at the cost of platform-bus code (since we need
> to add devices to the system bus) and a less concise command line. The
> platform-bus code may indeed be too expensive for this purpose, but
> we may need to see patches to be sure. I understand the desire to have
> a shorter command line, but this is QEMU :)
>

My concern is not the QEMU side. It is the code that we will need to
add to UEFI and ARM-TF to deal with the dynamic nature of the
underlying platform. That code has no counterpart in real world
hardware, but will surely turn up in production firmware nonetheless
if we end up having to add that to our SBSA reference codebase.

Of course, we can keep QEMU dynamic, and hardcode the firmware bits
against a certain instantiation of the SBSA machine. Bonus points for
making that command line part of the firmware payload :-)

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Jul 25, 2018 at 03:46:01PM +0200, Ard Biesheuvel wrote:
> On 25 July 2018 at 15:38, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 03:03:40PM +0200, Ard Biesheuvel wrote:
> >> On 25 July 2018 at 14:59, Andrew Jones <drjones@redhat.com> wrote:
> >> > On Wed, Jul 25, 2018 at 01:47:00PM +0100, Daniel P. Berrangé wrote:
> >> >> Would iut make any sense to call the machine  "refplatform"  or "refboard"
> >> >> to indicate it is a generic reference platform, not specifically following
> >> >> any particular real impl, albeit influence by the sbsa spec.
> >> >>
> >> >
> >> > That would indeed stop me from whining about a machine model with an
> >> > 'sbsa' name not strictly implementing a minimal SBSA machine :-)
> >> >
> >>
> >> I still don't get why only a minimal machine is worth considering,
> >> given that a real-world minimal SBSA machine is not capable of doing
> >> anything useful.
> >
> > One definition of an SBSA machine can be quite different than another.
> > If we only hard code the required [useless] base, but also provide a
> > readconfig for the rest, then we get a useful machine without loss of
> > flexibility.
> >
> > That flexibility comes at the cost of platform-bus code (since we need
> > to add devices to the system bus) and a less concise command line. The
> > platform-bus code may indeed be too expensive for this purpose, but
> > we may need to see patches to be sure. I understand the desire to have
> > a shorter command line, but this is QEMU :)
> >
> 
> My concern is not the QEMU side. It is the code that we will need to
> add to UEFI and ARM-TF to deal with the dynamic nature of the
> underlying platform. That code has no counterpart in real world
> hardware, but will surely turn up in production firmware nonetheless
> if we end up having to add that to our SBSA reference codebase.

It sounds like there's already some sort of informal SBSA reference
instance specification that UEFI and ARM-TF intend to support. If
that instance specification were slightly more formal, i.e. documented
somewhere and given a more descriptive name (not just 'sbsa'), then
I think it would be a lot more palatable to hard code those specifications
directly into a QEMU machine model of the same name.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 22:08, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 03:46:01PM +0200, Ard Biesheuvel wrote:
>> On 25 July 2018 at 15:38, Andrew Jones <drjones@redhat.com> wrote:
>> > On Wed, Jul 25, 2018 at 03:03:40PM +0200, Ard Biesheuvel wrote:
>> >> On 25 July 2018 at 14:59, Andrew Jones <drjones@redhat.com> wrote:
>> >> > On Wed, Jul 25, 2018 at 01:47:00PM +0100, Daniel P. Berrangé wrote:
>> >> >> Would iut make any sense to call the machine  "refplatform"  or "refboard"
>> >> >> to indicate it is a generic reference platform, not specifically following
>> >> >> any particular real impl, albeit influence by the sbsa spec.
>> >> >>
>> >> >
>> >> > That would indeed stop me from whining about a machine model with an
>> >> > 'sbsa' name not strictly implementing a minimal SBSA machine :-)
>> >> >
>> >>
>> >> I still don't get why only a minimal machine is worth considering,
>> >> given that a real-world minimal SBSA machine is not capable of doing
>> >> anything useful.
>> >
>> > One definition of an SBSA machine can be quite different than another.
>> > If we only hard code the required [useless] base, but also provide a
>> > readconfig for the rest, then we get a useful machine without loss of
>> > flexibility.
>> >
>> > That flexibility comes at the cost of platform-bus code (since we need
>> > to add devices to the system bus) and a less concise command line. The
>> > platform-bus code may indeed be too expensive for this purpose, but
>> > we may need to see patches to be sure. I understand the desire to have
>> > a shorter command line, but this is QEMU :)
>> >
>>
>> My concern is not the QEMU side. It is the code that we will need to
>> add to UEFI and ARM-TF to deal with the dynamic nature of the
>> underlying platform. That code has no counterpart in real world
>> hardware, but will surely turn up in production firmware nonetheless
>> if we end up having to add that to our SBSA reference codebase.
>
> It sounds like there's already some sort of informal SBSA reference
> instance specification that UEFI and ARM-TF intend to support. If
> that instance specification were slightly more formal, i.e. documented
> somewhere and given a more descriptive name (not just 'sbsa'), then
> I think it would be a lot more palatable to hard code those specifications
> directly into a QEMU machine model of the same name.
>
Root cause is requirement. This platform and virt serve different use cases.

As to command line and readconfig, my opinion is they are suitable for
change slightly some base/typical platforms, not for change one to
another with big difference, otherwise we can even reduce some
platforms in qemu I guess.

> Thanks,
> drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Thu, Jul 26, 2018 at 05:46:23PM +0800, Hongbo Zhang wrote:
> On 25 July 2018 at 22:08, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 03:46:01PM +0200, Ard Biesheuvel wrote:
> >> On 25 July 2018 at 15:38, Andrew Jones <drjones@redhat.com> wrote:
> >> > On Wed, Jul 25, 2018 at 03:03:40PM +0200, Ard Biesheuvel wrote:
> >> >> On 25 July 2018 at 14:59, Andrew Jones <drjones@redhat.com> wrote:
> >> >> > On Wed, Jul 25, 2018 at 01:47:00PM +0100, Daniel P. Berrangé wrote:
> >> >> >> Would iut make any sense to call the machine  "refplatform"  or "refboard"
> >> >> >> to indicate it is a generic reference platform, not specifically following
> >> >> >> any particular real impl, albeit influence by the sbsa spec.
> >> >> >>
> >> >> >
> >> >> > That would indeed stop me from whining about a machine model with an
> >> >> > 'sbsa' name not strictly implementing a minimal SBSA machine :-)
> >> >> >
> >> >>
> >> >> I still don't get why only a minimal machine is worth considering,
> >> >> given that a real-world minimal SBSA machine is not capable of doing
> >> >> anything useful.
> >> >
> >> > One definition of an SBSA machine can be quite different than another.
> >> > If we only hard code the required [useless] base, but also provide a
> >> > readconfig for the rest, then we get a useful machine without loss of
> >> > flexibility.
> >> >
> >> > That flexibility comes at the cost of platform-bus code (since we need
> >> > to add devices to the system bus) and a less concise command line. The
> >> > platform-bus code may indeed be too expensive for this purpose, but
> >> > we may need to see patches to be sure. I understand the desire to have
> >> > a shorter command line, but this is QEMU :)
> >> >
> >>
> >> My concern is not the QEMU side. It is the code that we will need to
> >> add to UEFI and ARM-TF to deal with the dynamic nature of the
> >> underlying platform. That code has no counterpart in real world
> >> hardware, but will surely turn up in production firmware nonetheless
> >> if we end up having to add that to our SBSA reference codebase.
> >
> > It sounds like there's already some sort of informal SBSA reference
> > instance specification that UEFI and ARM-TF intend to support. If
> > that instance specification were slightly more formal, i.e. documented
> > somewhere and given a more descriptive name (not just 'sbsa'), then
> > I think it would be a lot more palatable to hard code those specifications
> > directly into a QEMU machine model of the same name.
> >
> Root cause is requirement. This platform and virt serve different use cases.

Never any disagreement there. I'm just disagreeing with how you're naming
one instance of an SBSA platform with the very generic name of 'sbsa'.

> 
> As to command line and readconfig, my opinion is they are suitable for
> change slightly some base/typical platforms, not for change one to

Yes, command line and readconfig are suitable for changing a base
platform. That's why your new machine model should be a *base* platform,
not a specific platform with hard coded peripheral selections.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 20:19, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 25 July 2018 at 12:44, Andrew Jones <drjones@redhat.com> wrote:
>> On Wed, Jul 25, 2018 at 06:46:59PM +0800, Hongbo Zhang wrote:
>>> For Armv7, there is one typical platform 'vexpress', but for Armv8, no
>>
>> Wasn't the vexpress model designed for a specific machine?
>
> Yes.
>
>> Namely for
>> Arm's simulator?
>
> No.
>
>> Is the vexpress model really something typical among
>> all the Armv7 platforms?
>
> No.
>
> "Vexpress" is a model specifically of a development board
> produced by Arm (the versatile express). It's useful if you
> want to run code that runs on that devboard, but (as with
> most of the devboards we model), it's not necessarily ideal,
> because it has all the limitations of the real hardware it's
> modelling (in this case the big ones are limited memory, no PCI).
> The hardware it models is also quite old now (maybe 7 or 8 years)
> and it's not really "typical" of anything. (In the primarily
> embedded space where most v7 CPUs are there's not really anything
> that could be described as "typical" anyway: everything is
> different.)
>
> For most people who just want to run Linux on an emulated v7 CPU,
> I would recommend the "virt" board, for the same reasons I
> recommend it for v8 cores.
>
>>> such typical one, the 'virt' is typically for running workloads, one
>>> example is using it under OpenStack.
>>> So a 'typical' one for Armv8 is needed for firmware and OS
>>> development, similar like 'vexpress' for Armv7.
>>
>> What is a "typical" Armv8 machine? What will a typical Armv8 machine be in
>> two years?
>>
>> Note, I'm not actually opposed to the current definition (because I don't
>> really have one myself). I'm just opposed to hard coding one.
>
> AIUI the aim here is to provide an emulated platform that is
> set up in the way that server-style armv8 machines are
> recommended to be set up, so it can be used as a testbed and
> demonstration for the firmware/OS software stack. The hope
> is that following "best practices" results in a "typical"
> machine :-)  But the word "typical" is probably not really
> very helpful here...
Yes the aim is truely like that.
Let's forget the word 'typical', a none-english speaker may has his
slightly different understanding according to his own language
culture...

>
> I would expect that in the future we'd want this machine type
> to evolve with the recommendations for how to build server
> platform hardware, which might indeed be different in two years,
> since it would be the development platform for writing/testing
> the firmware/OS stack for that two-years-time hardware.
>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 25 July 2018 at 19:44, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Jul 25, 2018 at 06:46:59PM +0800, Hongbo Zhang wrote:
>> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
>> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>> >>  - Based on 'virt' machine type.
>> >>  - Re-designed memory map.
>> >>  - EL2 and EL3 are enabled by default.
>> >>  - GIC version 3 by default.
>> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
>> >>    can be added to it.
>> >>  - EHCI controller attached to system bus, with USB mouse and key board
>> >>    installed by default.
>> >>  - E1000E ethernet card on PCIE bus.
>> >>  - VGA display adaptor on PCIE bus.
>> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>> >>  - No virtio functions enabled, since this is to emulate real hardware.
>> >
>> > In the last review it was pointed out that using virtio-pci should still
>> > be "real" enough, so there's not much reason to avoid it. Well, unless
>> > there's some concern as to what drivers are available in the firmware and
>> > guest kernel. But that concern usually only applies to legacy firmwares
>> > and kernels, and therefore shouldn't apply to AArch64.
>> >
>> For Armv7, there is one typical platform 'vexpress', but for Armv8, no
>
> Wasn't the vexpress model designed for a specific machine? Namely for
> Arm's simulator? Is the vexpress model really something typical among
> all the Armv7 platforms?
>
>> such typical one, the 'virt' is typically for running workloads, one
>> example is using it under OpenStack.
>> So a 'typical' one for Armv8 is needed for firmware and OS
>> development, similar like 'vexpress' for Armv7.
>
> What is a "typical" Armv8 machine? What will a typical Armv8 machine be in
> two years?
>
> Note, I'm not actually opposed to the current definition (because I don't
> really have one myself). I'm just opposed to hard coding one.
>
For x86, we have i440fx and q35 (although they are old), but for
Armv8, simple usage like "qemu -bios/-pflash -cdrom" to install an OS
and "qemu -bios/-pflash -hda" to launch is needed too.
Armv8 has no such ones like x86, but we need, and SBSA could be the one.

Hard coding, user may have different customs, It couldn't be better if
one platform satisfy requirement, if not satisfied we edit it with
command or readconfig slightly, if we always need to change a platform
to another one with huge difference, then why not maintain the other
one, otherwise we don't need to maintain so many platforms at all.

> Thanks,
> drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Thu, Jul 26, 2018 at 06:17:36PM +0800, Hongbo Zhang wrote:
> On 25 July 2018 at 19:44, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Jul 25, 2018 at 06:46:59PM +0800, Hongbo Zhang wrote:
> >> On 25 July 2018 at 17:54, Andrew Jones <drjones@redhat.com> wrote:
> >> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >> >>  - Based on 'virt' machine type.
> >> >>  - Re-designed memory map.
> >> >>  - EL2 and EL3 are enabled by default.
> >> >>  - GIC version 3 by default.
> >> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
> >> >>    can be added to it.
> >> >>  - EHCI controller attached to system bus, with USB mouse and key board
> >> >>    installed by default.
> >> >>  - E1000E ethernet card on PCIE bus.
> >> >>  - VGA display adaptor on PCIE bus.
> >> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
> >> >>  - No virtio functions enabled, since this is to emulate real hardware.
> >> >
> >> > In the last review it was pointed out that using virtio-pci should still
> >> > be "real" enough, so there's not much reason to avoid it. Well, unless
> >> > there's some concern as to what drivers are available in the firmware and
> >> > guest kernel. But that concern usually only applies to legacy firmwares
> >> > and kernels, and therefore shouldn't apply to AArch64.
> >> >
> >> For Armv7, there is one typical platform 'vexpress', but for Armv8, no
> >
> > Wasn't the vexpress model designed for a specific machine? Namely for
> > Arm's simulator? Is the vexpress model really something typical among
> > all the Armv7 platforms?
> >
> >> such typical one, the 'virt' is typically for running workloads, one
> >> example is using it under OpenStack.
> >> So a 'typical' one for Armv8 is needed for firmware and OS
> >> development, similar like 'vexpress' for Armv7.
> >
> > What is a "typical" Armv8 machine? What will a typical Armv8 machine be in
> > two years?
> >
> > Note, I'm not actually opposed to the current definition (because I don't
> > really have one myself). I'm just opposed to hard coding one.
> >
> For x86, we have i440fx and q35 (although they are old), but for
> Armv8, simple usage like "qemu -bios/-pflash -cdrom" to install an OS
> and "qemu -bios/-pflash -hda" to launch is needed too.
> Armv8 has no such ones like x86, but we need, and SBSA could be the one.

i440fx and q35 are specific machine types. 'sbsa' is a generic machine
type that conforms to the SBSA specification. If you weren't trying to
memory map AHCI and EHCI controllers, or if memory mapped AHCI and EHCI
controllers were mandated in the SBSA spec, then there wouldn't be
anything to discuss. But that's not the case. You're attempting to
hard code one instance of a generic class of SBSA machines, but then
just call it 'sbsa'.

> 
> Hard coding, user may have different customs, It couldn't be better if
> one platform satisfy requirement, if not satisfied we edit it with
> command or readconfig slightly, if we always need to change a platform
> to another one with huge difference, then why not maintain the other
> one, otherwise we don't need to maintain so many platforms at all.
>

I won't argue about creating a list of default devices for an 'sbsa'
machine that get plugged when '-nodefaults' isn't used, but if you add
them to the memory map then '-nodefaults' won't remove them, which isn't
acceptable for a machine type generically named 'sbsa'.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 26 July 2018 at 11:46, Andrew Jones <drjones@redhat.com> wrote:
> i440fx and q35 are specific machine types. 'sbsa' is a generic machine
> type that conforms to the SBSA specification. If you weren't trying to
> memory map AHCI and EHCI controllers, or if memory mapped AHCI and EHCI
> controllers were mandated in the SBSA spec, then there wouldn't be
> anything to discuss. But that's not the case. You're attempting to
> hard code one instance of a generic class of SBSA machines, but then
> just call it 'sbsa'.

Modelling a 'generic SBSA machine' is not the goal here (I'm
not sure that's even possible or useful), so let's just
stipulate that we'll call the machine type something else
and move on ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 26 July 2018 at 12:52, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 July 2018 at 11:46, Andrew Jones <drjones@redhat.com> wrote:
>> i440fx and q35 are specific machine types. 'sbsa' is a generic machine
>> type that conforms to the SBSA specification. If you weren't trying to
>> memory map AHCI and EHCI controllers, or if memory mapped AHCI and EHCI
>> controllers were mandated in the SBSA spec, then there wouldn't be
>> anything to discuss. But that's not the case. You're attempting to
>> hard code one instance of a generic class of SBSA machines, but then
>> just call it 'sbsa'.
>
> Modelling a 'generic SBSA machine' is not the goal here (I'm
> not sure that's even possible or useful), so let's just
> stipulate that we'll call the machine type something else
> and move on ?
>

Fine with me. Care to suggest a name? :-)

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Thu, Jul 26, 2018 at 12:56:22PM +0200, Ard Biesheuvel wrote:
> On 26 July 2018 at 12:52, Peter Maydell <peter.maydell@linaro.org> wrote:
> > On 26 July 2018 at 11:46, Andrew Jones <drjones@redhat.com> wrote:
> >> i440fx and q35 are specific machine types. 'sbsa' is a generic machine
> >> type that conforms to the SBSA specification. If you weren't trying to
> >> memory map AHCI and EHCI controllers, or if memory mapped AHCI and EHCI
> >> controllers were mandated in the SBSA spec, then there wouldn't be
> >> anything to discuss. But that's not the case. You're attempting to
> >> hard code one instance of a generic class of SBSA machines, but then
> >> just call it 'sbsa'.
> >
> > Modelling a 'generic SBSA machine' is not the goal here (I'm
> > not sure that's even possible or useful), so let's just
> > stipulate that we'll call the machine type something else
> > and move on ?
> >
> 
> Fine with me. Care to suggest a name? :-)
>

Also fine by me, but I'm not going to suggest the name. I fear the
backlash I'd receive after this mail thread!

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Laszlo Ersek]

On 07/26/18 13:13, Andrew Jones wrote:
> On Thu, Jul 26, 2018 at 12:56:22PM +0200, Ard Biesheuvel wrote:
>> On 26 July 2018 at 12:52, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 26 July 2018 at 11:46, Andrew Jones <drjones@redhat.com> wrote:
>>>> i440fx and q35 are specific machine types. 'sbsa' is a generic machine
>>>> type that conforms to the SBSA specification. If you weren't trying to
>>>> memory map AHCI and EHCI controllers, or if memory mapped AHCI and EHCI
>>>> controllers were mandated in the SBSA spec, then there wouldn't be
>>>> anything to discuss. But that's not the case. You're attempting to
>>>> hard code one instance of a generic class of SBSA machines, but then
>>>> just call it 'sbsa'.
>>>
>>> Modelling a 'generic SBSA machine' is not the goal here (I'm
>>> not sure that's even possible or useful), so let's just
>>> stipulate that we'll call the machine type something else
>>> and move on ?
>>>
>>
>> Fine with me. Care to suggest a name? :-)
>>
> 
> Also fine by me, but I'm not going to suggest the name. I fear the
> backlash I'd receive after this mail thread!
> 

I suggest "showcase2018". (Dead serious.)

Laszlo

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Daniel P. Berrangé]

On Thu, Jul 26, 2018 at 02:23:46PM +0200, Laszlo Ersek wrote:
> On 07/26/18 13:13, Andrew Jones wrote:
> > On Thu, Jul 26, 2018 at 12:56:22PM +0200, Ard Biesheuvel wrote:
> >> On 26 July 2018 at 12:52, Peter Maydell <peter.maydell@linaro.org> wrote:
> >>> On 26 July 2018 at 11:46, Andrew Jones <drjones@redhat.com> wrote:
> >>>> i440fx and q35 are specific machine types. 'sbsa' is a generic machine
> >>>> type that conforms to the SBSA specification. If you weren't trying to
> >>>> memory map AHCI and EHCI controllers, or if memory mapped AHCI and EHCI
> >>>> controllers were mandated in the SBSA spec, then there wouldn't be
> >>>> anything to discuss. But that's not the case. You're attempting to
> >>>> hard code one instance of a generic class of SBSA machines, but then
> >>>> just call it 'sbsa'.
> >>>
> >>> Modelling a 'generic SBSA machine' is not the goal here (I'm
> >>> not sure that's even possible or useful), so let's just
> >>> stipulate that we'll call the machine type something else
> >>> and move on ?
> >>>
> >>
> >> Fine with me. Care to suggest a name? :-)
> >>
> > 
> > Also fine by me, but I'm not going to suggest the name. I fear the
> > backlash I'd receive after this mail thread!
> > 
> 
> I suggest "showcase2018". (Dead serious.)

Further suggestions

   "refplatform"
   "sbsareference"
   "sbsademo"
   "phys"   (physical counterpart to "virt")


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 26 July 2018 at 20:49, Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Thu, Jul 26, 2018 at 02:23:46PM +0200, Laszlo Ersek wrote:
>> On 07/26/18 13:13, Andrew Jones wrote:
>> > On Thu, Jul 26, 2018 at 12:56:22PM +0200, Ard Biesheuvel wrote:
>> >> On 26 July 2018 at 12:52, Peter Maydell <peter.maydell@linaro.org> wrote:
>> >>> On 26 July 2018 at 11:46, Andrew Jones <drjones@redhat.com> wrote:
>> >>>> i440fx and q35 are specific machine types. 'sbsa' is a generic machine
>> >>>> type that conforms to the SBSA specification. If you weren't trying to
>> >>>> memory map AHCI and EHCI controllers, or if memory mapped AHCI and EHCI
>> >>>> controllers were mandated in the SBSA spec, then there wouldn't be
>> >>>> anything to discuss. But that's not the case. You're attempting to
>> >>>> hard code one instance of a generic class of SBSA machines, but then
>> >>>> just call it 'sbsa'.
>> >>>
>> >>> Modelling a 'generic SBSA machine' is not the goal here (I'm
>> >>> not sure that's even possible or useful), so let's just
>> >>> stipulate that we'll call the machine type something else
>> >>> and move on ?
>> >>>
>> >>
>> >> Fine with me. Care to suggest a name? :-)
>> >>
>> >
>> > Also fine by me, but I'm not going to suggest the name. I fear the
>> > backlash I'd receive after this mail thread!
>> >
>>
>> I suggest "showcase2018". (Dead serious.)
>
> Further suggestions
>
>    "refplatform"
>    "sbsareference"
>    "sbsademo"
>    "phys"   (physical counterpart to "virt")
>
I like "sbsareference", some people still like the letters "sbsa" in
the name, and the "reference" sounds good too.
the only drawback is the string seems a bit long, I like this format
"sbsa-reference" but it is longer, "sbsa-ref"?

>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Dr. David Alan Gilbert]

* Andrew Jones (drjones@redhat.com) wrote:
> On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >  - Based on 'virt' machine type.
> >  - Re-designed memory map.
> >  - EL2 and EL3 are enabled by default.
> >  - GIC version 3 by default.
> >  - AHCI controller attached to system bus, and then CDROM and hard disc
> >    can be added to it.
> >  - EHCI controller attached to system bus, with USB mouse and key board
> >    installed by default.
> >  - E1000E ethernet card on PCIE bus.
> >  - VGA display adaptor on PCIE bus.
> >  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
> >  - No virtio functions enabled, since this is to emulate real hardware.
> 
> In the last review it was pointed out that using virtio-pci should still
> be "real" enough, so there's not much reason to avoid it. Well, unless
> there's some concern as to what drivers are available in the firmware and
> guest kernel. But that concern usually only applies to legacy firmwares
> and kernels, and therefore shouldn't apply to AArch64.

I think the difference from last time is Ard's comments earlier in this
thread:

    The purpose of the SBSA machine is not to provide a minimal
    configuration. It is intended to exercise all the moving parts one
    might find in a server firmware/OS stack, including pieces that are
    not usually found on x86 machines, such as DRAM starting above 4 GB
    and SATA/USB controllers that are not PCIe based.

that suggests that the intent of this board is to provide everything
which a firmware writer might want to test;  that's quite different
from forming the basis of a virtualised machine for real use.

Dave


> >  - No paravirtualized fw_cfg device either.
> > 
> > Arm Trusted Firmware and UEFI porting to this are done accordingly.
> >
> 
> How will UEFI get the ACPI tables from QEMU without fw-cfg? I didn't
> see any sort of reserved ROM region in the patch for them.
> 
> Thanks,
> drew
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Jul 25, 2018 at 11:50:41AM +0100, Dr. David Alan Gilbert wrote:
> * Andrew Jones (drjones@redhat.com) wrote:
> > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> > >  - Based on 'virt' machine type.
> > >  - Re-designed memory map.
> > >  - EL2 and EL3 are enabled by default.
> > >  - GIC version 3 by default.
> > >  - AHCI controller attached to system bus, and then CDROM and hard disc
> > >    can be added to it.
> > >  - EHCI controller attached to system bus, with USB mouse and key board
> > >    installed by default.
> > >  - E1000E ethernet card on PCIE bus.
> > >  - VGA display adaptor on PCIE bus.
> > >  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
> > >  - No virtio functions enabled, since this is to emulate real hardware.
> > 
> > In the last review it was pointed out that using virtio-pci should still
> > be "real" enough, so there's not much reason to avoid it. Well, unless
> > there's some concern as to what drivers are available in the firmware and
> > guest kernel. But that concern usually only applies to legacy firmwares
> > and kernels, and therefore shouldn't apply to AArch64.
> 
> I think the difference from last time is Ard's comments earlier in this
> thread:
> 
>     The purpose of the SBSA machine is not to provide a minimal
>     configuration. It is intended to exercise all the moving parts one
>     might find in a server firmware/OS stack, including pieces that are
>     not usually found on x86 machines, such as DRAM starting above 4 GB
>     and SATA/USB controllers that are not PCIe based.
> 
> that suggests that the intent of this board is to provide everything
> which a firmware writer might want to test;  that's quite different
> from forming the basis of a virtualised machine for real use.
>

I think I understand the purpose, and I also don't believe anything I've
said is counter to it. Whether or not one drives a virtio-pci nic with a
virtio-pci-net driver or drives an E1000e, also on the PCIe bus, makes
little difference to the firmware, nor to the guest kernel - besides which
driver gets used. And, nothing stops somebody from not plugging the
virtio-pci nic (use -nodefaults) and then plugging the E1000e (-device)
instead. Machine models don't need to hard code these assumptions. For
this patch it'd probably be best if we just ensured there were no
default devices at all, rather than replace one with another.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Igor Mammedov]

On Wed, 25 Jul 2018 13:36:45 +0200
Andrew Jones <drjones@redhat.com> wrote:

> On Wed, Jul 25, 2018 at 11:50:41AM +0100, Dr. David Alan Gilbert wrote:
> > * Andrew Jones (drjones@redhat.com) wrote:  
> > > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> > > >  - Based on 'virt' machine type.
> > > >  - Re-designed memory map.
> > > >  - EL2 and EL3 are enabled by default.
> > > >  - GIC version 3 by default.
> > > >  - AHCI controller attached to system bus, and then CDROM and hard disc
> > > >    can be added to it.
> > > >  - EHCI controller attached to system bus, with USB mouse and key board
> > > >    installed by default.
> > > >  - E1000E ethernet card on PCIE bus.
> > > >  - VGA display adaptor on PCIE bus.
> > > >  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
> > > >  - No virtio functions enabled, since this is to emulate real hardware.  
> > > 
> > > In the last review it was pointed out that using virtio-pci should still
> > > be "real" enough, so there's not much reason to avoid it. Well, unless
> > > there's some concern as to what drivers are available in the firmware and
> > > guest kernel. But that concern usually only applies to legacy firmwares
> > > and kernels, and therefore shouldn't apply to AArch64.  
> > 
> > I think the difference from last time is Ard's comments earlier in this
> > thread:
> > 
> >     The purpose of the SBSA machine is not to provide a minimal
> >     configuration. It is intended to exercise all the moving parts one
> >     might find in a server firmware/OS stack, including pieces that are
> >     not usually found on x86 machines, such as DRAM starting above 4 GB
> >     and SATA/USB controllers that are not PCIe based.
> > 
> > that suggests that the intent of this board is to provide everything
> > which a firmware writer might want to test;  that's quite different
> > from forming the basis of a virtualised machine for real use.
> >  
> 
> I think I understand the purpose, and I also don't believe anything I've
> said is counter to it. Whether or not one drives a virtio-pci nic with a
> virtio-pci-net driver or drives an E1000e, also on the PCIe bus, makes
> little difference to the firmware, nor to the guest kernel - besides which
> driver gets used. And, nothing stops somebody from not plugging the
> virtio-pci nic (use -nodefaults) and then plugging the E1000e (-device)
> instead. Machine models don't need to hard code these assumptions. For
> this patch it'd probably be best if we just ensured there were no
> default devices at all, rather than replace one with another.

with that thinking it might be better to put this machine in completely
different file so not mess with 'production' virt machine code and
call it intentionally ambiguous: "[linaro|generic|dev|...]_armv8"
so it would be just another sbsa compliant board with  a bunch of
default devices needed for testing purposes if users/authors think
that it serves their purpose better.

> Thanks,
> drew
> 

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 26 July 2018 at 00:15, Igor Mammedov <imammedo@redhat.com> wrote:
> On Wed, 25 Jul 2018 13:36:45 +0200
> Andrew Jones <drjones@redhat.com> wrote:
>
>> On Wed, Jul 25, 2018 at 11:50:41AM +0100, Dr. David Alan Gilbert wrote:
>> > * Andrew Jones (drjones@redhat.com) wrote:
>> > > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
>> > > >  - Based on 'virt' machine type.
>> > > >  - Re-designed memory map.
>> > > >  - EL2 and EL3 are enabled by default.
>> > > >  - GIC version 3 by default.
>> > > >  - AHCI controller attached to system bus, and then CDROM and hard disc
>> > > >    can be added to it.
>> > > >  - EHCI controller attached to system bus, with USB mouse and key board
>> > > >    installed by default.
>> > > >  - E1000E ethernet card on PCIE bus.
>> > > >  - VGA display adaptor on PCIE bus.
>> > > >  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>> > > >  - No virtio functions enabled, since this is to emulate real hardware.
>> > >
>> > > In the last review it was pointed out that using virtio-pci should still
>> > > be "real" enough, so there's not much reason to avoid it. Well, unless
>> > > there's some concern as to what drivers are available in the firmware and
>> > > guest kernel. But that concern usually only applies to legacy firmwares
>> > > and kernels, and therefore shouldn't apply to AArch64.
>> >
>> > I think the difference from last time is Ard's comments earlier in this
>> > thread:
>> >
>> >     The purpose of the SBSA machine is not to provide a minimal
>> >     configuration. It is intended to exercise all the moving parts one
>> >     might find in a server firmware/OS stack, including pieces that are
>> >     not usually found on x86 machines, such as DRAM starting above 4 GB
>> >     and SATA/USB controllers that are not PCIe based.
>> >
>> > that suggests that the intent of this board is to provide everything
>> > which a firmware writer might want to test;  that's quite different
>> > from forming the basis of a virtualised machine for real use.
>> >
>>
>> I think I understand the purpose, and I also don't believe anything I've
>> said is counter to it. Whether or not one drives a virtio-pci nic with a
>> virtio-pci-net driver or drives an E1000e, also on the PCIe bus, makes
>> little difference to the firmware, nor to the guest kernel - besides which
>> driver gets used. And, nothing stops somebody from not plugging the
>> virtio-pci nic (use -nodefaults) and then plugging the E1000e (-device)
>> instead. Machine models don't need to hard code these assumptions. For
>> this patch it'd probably be best if we just ensured there were no
>> default devices at all, rather than replace one with another.
>
> with that thinking it might be better to put this machine in completely
> different file so not mess with 'production' virt machine code and
> call it intentionally ambiguous: "[linaro|generic|dev|...]_armv8"
> so it would be just another sbsa compliant board with  a bunch of
> default devices needed for testing purposes if users/authors think
> that it serves their purpose better.
>
Yes, before sending, I had to solutions: a separate file and share with virt.c.
For a internal release, I have a separate file sbsa.c
http://git.linaro.org/people/hongbo.zhang/qemu-enterprise.git/tree/hw/arm/sbsa.c?h=sbsa-v2.12

If separate file is used as above, there are much duplicate with
virt.c, such as both have create_pcie() etc, I need to move all the
common parts to a new file say virt-common.c, then the virt.c and
sbsa.c can only handle their specific parts, in such a way, the
previous still cannot be left untouched.

so I send out this solution for discuss, people mainly concern the
necessity of it till now, not the code and file format yet.
Thanks for you advice.

>> Thanks,
>> drew
>>
>

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Thu, Jul 26, 2018 at 05:55:54PM +0800, Hongbo Zhang wrote:
> On 26 July 2018 at 00:15, Igor Mammedov <imammedo@redhat.com> wrote:
> > On Wed, 25 Jul 2018 13:36:45 +0200
> > Andrew Jones <drjones@redhat.com> wrote:
> >
> >> On Wed, Jul 25, 2018 at 11:50:41AM +0100, Dr. David Alan Gilbert wrote:
> >> > * Andrew Jones (drjones@redhat.com) wrote:
> >> > > On Wed, Jul 25, 2018 at 01:30:52PM +0800, Hongbo Zhang 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 'Enterprise' with main features:
> >> > > >  - Based on 'virt' machine type.
> >> > > >  - Re-designed memory map.
> >> > > >  - EL2 and EL3 are enabled by default.
> >> > > >  - GIC version 3 by default.
> >> > > >  - AHCI controller attached to system bus, and then CDROM and hard disc
> >> > > >    can be added to it.
> >> > > >  - EHCI controller attached to system bus, with USB mouse and key board
> >> > > >    installed by default.
> >> > > >  - E1000E ethernet card on PCIE bus.
> >> > > >  - VGA display adaptor on PCIE bus.
> >> > > >  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
> >> > > >  - No virtio functions enabled, since this is to emulate real hardware.
> >> > >
> >> > > In the last review it was pointed out that using virtio-pci should still
> >> > > be "real" enough, so there's not much reason to avoid it. Well, unless
> >> > > there's some concern as to what drivers are available in the firmware and
> >> > > guest kernel. But that concern usually only applies to legacy firmwares
> >> > > and kernels, and therefore shouldn't apply to AArch64.
> >> >
> >> > I think the difference from last time is Ard's comments earlier in this
> >> > thread:
> >> >
> >> >     The purpose of the SBSA machine is not to provide a minimal
> >> >     configuration. It is intended to exercise all the moving parts one
> >> >     might find in a server firmware/OS stack, including pieces that are
> >> >     not usually found on x86 machines, such as DRAM starting above 4 GB
> >> >     and SATA/USB controllers that are not PCIe based.
> >> >
> >> > that suggests that the intent of this board is to provide everything
> >> > which a firmware writer might want to test;  that's quite different
> >> > from forming the basis of a virtualised machine for real use.
> >> >
> >>
> >> I think I understand the purpose, and I also don't believe anything I've
> >> said is counter to it. Whether or not one drives a virtio-pci nic with a
> >> virtio-pci-net driver or drives an E1000e, also on the PCIe bus, makes
> >> little difference to the firmware, nor to the guest kernel - besides which
> >> driver gets used. And, nothing stops somebody from not plugging the
> >> virtio-pci nic (use -nodefaults) and then plugging the E1000e (-device)
> >> instead. Machine models don't need to hard code these assumptions. For
> >> this patch it'd probably be best if we just ensured there were no
> >> default devices at all, rather than replace one with another.
> >
> > with that thinking it might be better to put this machine in completely
> > different file so not mess with 'production' virt machine code and
> > call it intentionally ambiguous: "[linaro|generic|dev|...]_armv8"
> > so it would be just another sbsa compliant board with  a bunch of
> > default devices needed for testing purposes if users/authors think
> > that it serves their purpose better.
> >
> Yes, before sending, I had to solutions: a separate file and share with virt.c.
> For a internal release, I have a separate file sbsa.c
> http://git.linaro.org/people/hongbo.zhang/qemu-enterprise.git/tree/hw/arm/sbsa.c?h=sbsa-v2.12
> 
> If separate file is used as above, there are much duplicate with
> virt.c, such as both have create_pcie() etc, I need to move all the
> common parts to a new file say virt-common.c, then the virt.c and
> sbsa.c can only handle their specific parts, in such a way, the
> previous still cannot be left untouched.
> 
> so I send out this solution for discuss, people mainly concern the
> necessity of it till now, not the code and file format yet.
> Thanks for you advice.
>

Let's try to figure out how the "sbsa" machine type memory map should look
first. If it isn't adding a bunch of new stuff then it may not matter that
it's in the same file. OTOH, separating virt machine class device and fdt
building functions from virt machine instance parameters would be a nice
cleanup anyway. Doing that cleanup first, and then simply adding a new
machine instance file, would probably be the cleanest approach.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 25 July 2018 at 06:30, 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 'Enterprise' with main features:
>  - Based on 'virt' machine type.
>  - Re-designed memory map.
>  - EL2 and EL3 are enabled by default.
>  - GIC version 3 by default.
>  - AHCI controller attached to system bus, and then CDROM and hard disc
>    can be added to it.
>  - EHCI controller attached to system bus, with USB mouse and key board
>    installed by default.
>  - E1000E ethernet card on PCIE bus.
>  - VGA display adaptor on PCIE bus.
>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>  - No virtio functions enabled, since this is to emulate real hardware.
>  - No paravirtualized fw_cfg device either.
>
> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>
> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> ---
> Changes since v1:
>  - rebase on v3.0.0-rc0
>  - introduce another auxillary patch as 1/2, so this is 2/2
>  - rename 'sbsa' to 'enterprise'
>  - remove paravirualized fw_cfg
>  - set gic_vertion to 3 instead of 2
>  - edit commit message to describe purpose of this platform

Hi; there's been a lot of discussion on this thread. I'm going
to assume that you don't need any further review commentary for
the moment, and will wait for a v3. If there are any specific
questions you want guidance on to produce that v3, let me know.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 17 August 2018 at 21:37, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 25 July 2018 at 06:30, 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 'Enterprise' with main features:
>>  - Based on 'virt' machine type.
>>  - Re-designed memory map.
>>  - EL2 and EL3 are enabled by default.
>>  - GIC version 3 by default.
>>  - AHCI controller attached to system bus, and then CDROM and hard disc
>>    can be added to it.
>>  - EHCI controller attached to system bus, with USB mouse and key board
>>    installed by default.
>>  - E1000E ethernet card on PCIE bus.
>>  - VGA display adaptor on PCIE bus.
>>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>>  - No virtio functions enabled, since this is to emulate real hardware.
>>  - No paravirtualized fw_cfg device either.
>>
>> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>>
>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>> ---
>> Changes since v1:
>>  - rebase on v3.0.0-rc0
>>  - introduce another auxillary patch as 1/2, so this is 2/2
>>  - rename 'sbsa' to 'enterprise'
>>  - remove paravirualized fw_cfg
>>  - set gic_vertion to 3 instead of 2
>>  - edit commit message to describe purpose of this platform
>
> Hi; there's been a lot of discussion on this thread. I'm going
> to assume that you don't need any further review commentary for
> the moment, and will wait for a v3. If there are any specific
> questions you want guidance on to produce that v3, let me know.
>
Thank you for this summary/remind.
Sorry for the late reply due to my vacation last week.

Yes, I am working on the v3, with main changes:
 - machine name "sbsa-ref" (good name?)
 - a separate file sbsa-ref.c
 - don't touch the acpi c file, acpi will be supplied by uefi
 - only necessary dt node, no other peripheral dt nodes

Before my sending, I still like comments on the above effort, it is
feasible/reasonable or not etc, in case I go wrong too much.

I also know that some reviewers may like to give comments when they
see the patch codes, so if you don't have comments now, you can do it
after seeing the patch.
Thanks all.

> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Aug 29, 2018 at 05:17:20PM +0800, Hongbo Zhang wrote:
> On 17 August 2018 at 21:37, Peter Maydell <peter.maydell@linaro.org> wrote:
> > On 25 July 2018 at 06:30, 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 'Enterprise' with main features:
> >>  - Based on 'virt' machine type.
> >>  - Re-designed memory map.
> >>  - EL2 and EL3 are enabled by default.
> >>  - GIC version 3 by default.
> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
> >>    can be added to it.
> >>  - EHCI controller attached to system bus, with USB mouse and key board
> >>    installed by default.
> >>  - E1000E ethernet card on PCIE bus.
> >>  - VGA display adaptor on PCIE bus.
> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
> >>  - No virtio functions enabled, since this is to emulate real hardware.
> >>  - No paravirtualized fw_cfg device either.
> >>
> >> Arm Trusted Firmware and UEFI porting to this are done accordingly.
> >>
> >> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> >> ---
> >> Changes since v1:
> >>  - rebase on v3.0.0-rc0
> >>  - introduce another auxillary patch as 1/2, so this is 2/2
> >>  - rename 'sbsa' to 'enterprise'
> >>  - remove paravirualized fw_cfg
> >>  - set gic_vertion to 3 instead of 2
> >>  - edit commit message to describe purpose of this platform
> >
> > Hi; there's been a lot of discussion on this thread. I'm going
> > to assume that you don't need any further review commentary for
> > the moment, and will wait for a v3. If there are any specific
> > questions you want guidance on to produce that v3, let me know.
> >
> Thank you for this summary/remind.
> Sorry for the late reply due to my vacation last week.
> 
> Yes, I am working on the v3, with main changes:
>  - machine name "sbsa-ref" (good name?)
>  - a separate file sbsa-ref.c
>  - don't touch the acpi c file, acpi will be supplied by uefi

I agree with the above three bullets.

>  - only necessary dt node, no other peripheral dt nodes

I'm not sure what DT nodes are necessary. After discussing the
ACPI tables, and that firmware would supply them, I thought we
also agreed firmware would supply the DTB.

Thanks,
drew

> 
> Before my sending, I still like comments on the above effort, it is
> feasible/reasonable or not etc, in case I go wrong too much.
> 
> I also know that some reviewers may like to give comments when they
> see the patch codes, so if you don't have comments now, you can do it
> after seeing the patch.
> Thanks all.
> 
> > thanks
> > -- PMM
> 

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 29 August 2018 at 21:42, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Aug 29, 2018 at 05:17:20PM +0800, Hongbo Zhang wrote:
>> On 17 August 2018 at 21:37, Peter Maydell <peter.maydell@linaro.org> wrote:
>> > On 25 July 2018 at 06:30, 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 'Enterprise' with main features:
>> >>  - Based on 'virt' machine type.
>> >>  - Re-designed memory map.
>> >>  - EL2 and EL3 are enabled by default.
>> >>  - GIC version 3 by default.
>> >>  - AHCI controller attached to system bus, and then CDROM and hard disc
>> >>    can be added to it.
>> >>  - EHCI controller attached to system bus, with USB mouse and key board
>> >>    installed by default.
>> >>  - E1000E ethernet card on PCIE bus.
>> >>  - VGA display adaptor on PCIE bus.
>> >>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>> >>  - No virtio functions enabled, since this is to emulate real hardware.
>> >>  - No paravirtualized fw_cfg device either.
>> >>
>> >> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>> >>
>> >> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>> >> ---
>> >> Changes since v1:
>> >>  - rebase on v3.0.0-rc0
>> >>  - introduce another auxillary patch as 1/2, so this is 2/2
>> >>  - rename 'sbsa' to 'enterprise'
>> >>  - remove paravirualized fw_cfg
>> >>  - set gic_vertion to 3 instead of 2
>> >>  - edit commit message to describe purpose of this platform
>> >
>> > Hi; there's been a lot of discussion on this thread. I'm going
>> > to assume that you don't need any further review commentary for
>> > the moment, and will wait for a v3. If there are any specific
>> > questions you want guidance on to produce that v3, let me know.
>> >
>> Thank you for this summary/remind.
>> Sorry for the late reply due to my vacation last week.
>>
>> Yes, I am working on the v3, with main changes:
>>  - machine name "sbsa-ref" (good name?)
>>  - a separate file sbsa-ref.c
>>  - don't touch the acpi c file, acpi will be supplied by uefi
>
> I agree with the above three bullets.
>
>>  - only necessary dt node, no other peripheral dt nodes
>
> I'm not sure what DT nodes are necessary. After discussing the
> ACPI tables, and that firmware would supply them, I thought we
> also agreed firmware would supply the DTB.
>
Yes, firmware supply ACPI to load OS.
But for DT, It is said that UEFI still needs some DT nodes, at least
for memory size. I am trying to figure out which DT nodes are
mandatory too, it is better we don't have any DT nodes left.

@Ard, @Leif, is there any possibility to remove all the DT nodes?
On real hardware, how does UEFI find the memory size and CPU number?

> Thanks,
> drew
>
>>
>> Before my sending, I still like comments on the above effort, it is
>> feasible/reasonable or not etc, in case I go wrong too much.
>>
>> I also know that some reviewers may like to give comments when they
>> see the patch codes, so if you don't have comments now, you can do it
>> after seeing the patch.
>> Thanks all.
>>
>> > thanks
>> > -- PMM
>>

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Leif Lindholm]

On Thu, Aug 30, 2018 at 03:07:29PM +0800, Hongbo Zhang wrote:
> >> Yes, I am working on the v3, with main changes:
> >>  - machine name "sbsa-ref" (good name?)
> >>  - a separate file sbsa-ref.c
> >>  - don't touch the acpi c file, acpi will be supplied by uefi
> >
> > I agree with the above three bullets.
> >
> >>  - only necessary dt node, no other peripheral dt nodes
> >
> > I'm not sure what DT nodes are necessary. After discussing the
> > ACPI tables, and that firmware would supply them, I thought we
> > also agreed firmware would supply the DTB.
>
> Yes, firmware supply ACPI to load OS.
> But for DT, It is said that UEFI still needs some DT nodes, at least
> for memory size. I am trying to figure out which DT nodes are
> mandatory too, it is better we don't have any DT nodes left.
> 
> @Ard, @Leif, is there any possibility to remove all the DT nodes?
> On real hardware, how does UEFI find the memory size and CPU number?

Usually by asking some form of SCP/PMU.
So until we have rock-solid multi-cpu-instance (making terms up here,
there may be a proper name already?) support upstream, we need to take
shortcuts.

Using DT in the same way as we do for mach-virt when booting the OS
with ACPI seems the most logical choice to me.

The best alternative I can think of would be extending fw_cfg, but
that would then be lost effort once (if) we get to the abovementioned
situation, whereas the DT route comes for free.

/
    Leif

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 30 August 2018 at 09:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Aug 30, 2018 at 03:07:29PM +0800, Hongbo Zhang wrote:
>> @Ard, @Leif, is there any possibility to remove all the DT nodes?
>> On real hardware, how does UEFI find the memory size and CPU number?
>
> Usually by asking some form of SCP/PMU.
> So until we have rock-solid multi-cpu-instance (making terms up here,
> there may be a proper name already?) support upstream, we need to take
> shortcuts.

I would expect that you'd want a "faked-on-the-QEMU end" SCP
communications endpoint to start with anyway. We have one of
those for vexpress, for instance.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Leif Lindholm]

On Thu, Aug 30, 2018 at 09:39:33AM +0100, Peter Maydell wrote:
> On 30 August 2018 at 09:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Thu, Aug 30, 2018 at 03:07:29PM +0800, Hongbo Zhang wrote:
> >> @Ard, @Leif, is there any possibility to remove all the DT nodes?
> >> On real hardware, how does UEFI find the memory size and CPU number?
> >
> > Usually by asking some form of SCP/PMU.
> > So until we have rock-solid multi-cpu-instance (making terms up here,
> > there may be a proper name already?) support upstream, we need to take
> > shortcuts.
> 
> I would expect that you'd want a "faked-on-the-QEMU end" SCP
> communications endpoint to start with anyway. We have one of
> those for vexpress, for instance.

Ah, I wasn't aware we did!

In that case - sure, that is probably the more sensible solution.
Does it emulate SCMI?

/
    Leif

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 30 August 2018 at 12:02, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Aug 30, 2018 at 09:39:33AM +0100, Peter Maydell wrote:
>> On 30 August 2018 at 09:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> > On Thu, Aug 30, 2018 at 03:07:29PM +0800, Hongbo Zhang wrote:
>> >> @Ard, @Leif, is there any possibility to remove all the DT nodes?
>> >> On real hardware, how does UEFI find the memory size and CPU number?
>> >
>> > Usually by asking some form of SCP/PMU.
>> > So until we have rock-solid multi-cpu-instance (making terms up here,
>> > there may be a proper name already?) support upstream, we need to take
>> > shortcuts.
>>
>> I would expect that you'd want a "faked-on-the-QEMU end" SCP
>> communications endpoint to start with anyway. We have one of
>> those for vexpress, for instance.
>
> Ah, I wasn't aware we did!
>
> In that case - sure, that is probably the more sensible solution.
> Does it emulate SCMI?
>

The purpose of the SBSA/SBBR qemu machine is prototyping the
interactions between rich firmware and the OS for things like RAS,
secure variable storage for UEFI or capsule update.

How exactly the firmware figures out how many CPUs and how much memory
we are running with is out of scope for this, and so I don't think
there is a need to build something from scratch here: DT will do just
fine, given that both EDK2 and ARM-TF have working DT code for these
things.

fw_cfg, on the other hand, is unsuitable for us. Generating ACPI
tables in a different way from actual bare metal is not a good idea,
given that things like RAS interact with ACPI as well, and fw_cfg is
also exposed to the OS in the default mach-virt configuration. So we
could tweak fw_cfg to be more suitable, but I think that does not
improve the usefulness of our prototype at all.

At some point, it would be nice if we could get UEFI to receive its
platform description from ARM-TF in a generic manner, and it would be
good for EDK2 and ARM-TF to align on this. So even if we decide that
SCMI is more suitable way to convey this information from QEMU to the
guest firmware, I would object to implementing the client side of that
in UEFI, given that it should be ARM-TF that provides this
information.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Leif Lindholm]

On Thu, Aug 30, 2018 at 03:29:02PM +0200, Ard Biesheuvel wrote:
> On 30 August 2018 at 12:02, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Thu, Aug 30, 2018 at 09:39:33AM +0100, Peter Maydell wrote:
> >> On 30 August 2018 at 09:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> > On Thu, Aug 30, 2018 at 03:07:29PM +0800, Hongbo Zhang wrote:
> >> >> @Ard, @Leif, is there any possibility to remove all the DT nodes?
> >> >> On real hardware, how does UEFI find the memory size and CPU number?
> >> >
> >> > Usually by asking some form of SCP/PMU.
> >> > So until we have rock-solid multi-cpu-instance (making terms up here,
> >> > there may be a proper name already?) support upstream, we need to take
> >> > shortcuts.
> >>
> >> I would expect that you'd want a "faked-on-the-QEMU end" SCP
> >> communications endpoint to start with anyway. We have one of
> >> those for vexpress, for instance.
> >
> > Ah, I wasn't aware we did!
> >
> > In that case - sure, that is probably the more sensible solution.
> > Does it emulate SCMI?
> 
> The purpose of the SBSA/SBBR qemu machine is prototyping the
> interactions between rich firmware and the OS for things like RAS,
> secure variable storage for UEFI or capsule update.

I think it would be useful to at some point have also an open-source
SCP implementation and tie it into this platform (if feasible). I do
not however think it is important, and certainly nothing we need to
worry about for now.

> How exactly the firmware figures out how many CPUs and how much memory
> we are running with is out of scope for this, and so I don't think
> there is a need to build something from scratch here: DT will do just
> fine, given that both EDK2 and ARM-TF have working DT code for these
> things.

Agreed.

> fw_cfg, on the other hand, is unsuitable for us. Generating ACPI
> tables in a different way from actual bare metal is not a good idea,
> given that things like RAS interact with ACPI as well, and fw_cfg is
> also exposed to the OS in the default mach-virt configuration. So we
> could tweak fw_cfg to be more suitable, but I think that does not
> improve the usefulness of our prototype at all.

Indeed.

> At some point, it would be nice if we could get UEFI to receive its
> platform description from ARM-TF in a generic manner, and it would be
> good for EDK2 and ARM-TF to align on this. So even if we decide that
> SCMI is more suitable way to convey this information from QEMU to the
> guest firmware, I would object to implementing the client side of that
> in UEFI, given that it should be ARM-TF that provides this
> information.

Yes, but the modeled SCP is relevant for the ARM-TF port for this
platform. A "faked-on-the-QEMU end" SCP that communicates via SCMI (or
if that's been superseded, I may have lost track) allows for sharing
generic arm-tf code for retrieving this information with most hardware
platforms.

And that _would_ be nicer than having the lowest layers of the EDK2
port looking more like hardware than mach-virt.

This is of course also something that could be addressed at a later
date - I'm not sure how generic arm-tf is in this regard today.

/
    Leif

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Peter Maydell]

On 30 August 2018 at 14:29, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> How exactly the firmware figures out how many CPUs and how much memory
> we are running with is out of scope for this, and so I don't think
> there is a need to build something from scratch here: DT will do just
> fine, given that both EDK2 and ARM-TF have working DT code for these
> things.
>
> fw_cfg, on the other hand, is unsuitable for us. Generating ACPI
> tables in a different way from actual bare metal is not a good idea,
> given that things like RAS interact with ACPI as well, and fw_cfg is
> also exposed to the OS in the default mach-virt configuration. So we
> could tweak fw_cfg to be more suitable, but I think that does not
> improve the usefulness of our prototype at all.

fw_cfg is an entirely generic transport for passing named
data blobs from QEMU to the other end. You don't have to
pass ACPI table fragments over it the way we do for
virt and the x86 PC platforms if you don't want that.
It's true that it's visible to the OS in the sense that it's
a physical device in the system, but so is a hardware connection
to an SCP.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Ard Biesheuvel]

On 30 August 2018 at 18:36, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 30 August 2018 at 14:29, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> How exactly the firmware figures out how many CPUs and how much memory
>> we are running with is out of scope for this, and so I don't think
>> there is a need to build something from scratch here: DT will do just
>> fine, given that both EDK2 and ARM-TF have working DT code for these
>> things.
>>
>> fw_cfg, on the other hand, is unsuitable for us. Generating ACPI
>> tables in a different way from actual bare metal is not a good idea,
>> given that things like RAS interact with ACPI as well, and fw_cfg is
>> also exposed to the OS in the default mach-virt configuration. So we
>> could tweak fw_cfg to be more suitable, but I think that does not
>> improve the usefulness of our prototype at all.
>
> fw_cfg is an entirely generic transport for passing named
> data blobs from QEMU to the other end. You don't have to
> pass ACPI table fragments over it the way we do for
> virt and the x86 PC platforms if you don't want that.

Sure. But the point is that fw_cfg does not really describe anything
that is useful to us. The information that we need in ARM-TF, and
-directly or indirectly- in UEFI is currently provided via the DT that
is passed in memory, and at the moment, I don't see a reason to change
that since it doesn't conflict with the goals we have for this
prototype. Relying on an emulated device which should never exist on
actual hardware does.

> It's true that it's visible to the OS in the sense that it's
> a physical device in the system, but so is a hardware connection
> to an SCP.
>

Such a connection would not be exposed to the OS on a arm64 server system.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 30 August 2018 at 21:29, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 30 August 2018 at 12:02, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> On Thu, Aug 30, 2018 at 09:39:33AM +0100, Peter Maydell wrote:
>>> On 30 August 2018 at 09:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>>> > On Thu, Aug 30, 2018 at 03:07:29PM +0800, Hongbo Zhang wrote:
>>> >> @Ard, @Leif, is there any possibility to remove all the DT nodes?
>>> >> On real hardware, how does UEFI find the memory size and CPU number?
>>> >
>>> > Usually by asking some form of SCP/PMU.
>>> > So until we have rock-solid multi-cpu-instance (making terms up here,
>>> > there may be a proper name already?) support upstream, we need to take
>>> > shortcuts.
>>>
>>> I would expect that you'd want a "faked-on-the-QEMU end" SCP
>>> communications endpoint to start with anyway. We have one of
>>> those for vexpress, for instance.
>>
>> Ah, I wasn't aware we did!
>>
>> In that case - sure, that is probably the more sensible solution.
>> Does it emulate SCMI?
>>
>
> The purpose of the SBSA/SBBR qemu machine is prototyping the
> interactions between rich firmware and the OS for things like RAS,
> secure variable storage for UEFI or capsule update.
>
> How exactly the firmware figures out how many CPUs and how much memory
> we are running with is out of scope for this, and so I don't think
> there is a need to build something from scratch here: DT will do just
> fine, given that both EDK2 and ARM-TF have working DT code for these
> things.

For firmware prototype development, CPU number and memory size may not
key points.
But from QEMU platform implementation point of view, they can not be
out of scope, they should be passed by command parameters instead,
because we don't know which numbers suit for users (although there are
default values).

>From the previous discussion, SBSA machine doesn't supply ACPI, as to
DT, implicit conclusion was same as ACPI: supplied by firmware.
That is why I had the question,  if QEMU doesn't supply DT, if memory
size and CPU number are passed by parameters dynamically, then how can
they be passed to guest firmware and OS, so I considered to keep as
least necessary DT nodes such as memory size, and also learned how
firmware does it on real hardware, to see if we can completely remove
all DT nodes.

You support to keep DT as virt, it is really fine with me, but this
triggers how my patch look like:
If no DT in SBSA machine, then the code had much more difference with
virt, so there is possibility to make it a simple separate file;
If keep same DT as virt, then there are more overlap with virt codes,
so I am not sure how the patch should like finally, I still prefer
separate file to keep virt untouched, but not sure if others would
suggest to extract common codes to a third file, it is up to the
maintainer and reviewers.

So, keeping DT same as virt is final choice, right?
And how the patch look like? or I send a v3 with separate file to see
for discuss?
(understand sometime people cannot / don't like to give comments
without seeing the patch)

>
> fw_cfg, on the other hand, is unsuitable for us. Generating ACPI
> tables in a different way from actual bare metal is not a good idea,
> given that things like RAS interact with ACPI as well, and fw_cfg is
> also exposed to the OS in the default mach-virt configuration. So we
> could tweak fw_cfg to be more suitable, but I think that does not
> improve the usefulness of our prototype at all.
>
> At some point, it would be nice if we could get UEFI to receive its
> platform description from ARM-TF in a generic manner, and it would be
> good for EDK2 and ARM-TF to align on this. So even if we decide that
> SCMI is more suitable way to convey this information from QEMU to the
> guest firmware, I would object to implementing the client side of that
> in UEFI, given that it should be ARM-TF that provides this
> information.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Fri, Aug 31, 2018 at 03:20:45PM +0800, Hongbo Zhang wrote:
> If no DT in SBSA machine, then the code had much more difference with
> virt, so there is possibility to make it a simple separate file;
> If keep same DT as virt, then there are more overlap with virt codes,
> so I am not sure how the patch should like finally, I still prefer
> separate file to keep virt untouched, but not sure if others would
> suggest to extract common codes to a third file, it is up to the
> maintainer and reviewers.
>

It sounds like you don't need many DT nodes, so a new file for
sbsa-ref, where you build whatever you want it to look like and
add the few DT nodes to inform firmware of a few parameters,
should be completely independent of mach-virt. I.e. I don't think
it even makes sense to refactor those few nodes in order to
share a tiny bit of DT node generation code.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 31 August 2018 at 16:42, Andrew Jones <drjones@redhat.com> wrote:
> On Fri, Aug 31, 2018 at 03:20:45PM +0800, Hongbo Zhang wrote:
>> If no DT in SBSA machine, then the code had much more difference with
>> virt, so there is possibility to make it a simple separate file;
>> If keep same DT as virt, then there are more overlap with virt codes,
>> so I am not sure how the patch should like finally, I still prefer
>> separate file to keep virt untouched, but not sure if others would
>> suggest to extract common codes to a third file, it is up to the
>> maintainer and reviewers.
>>
>
> It sounds like you don't need many DT nodes, so a new file for
> sbsa-ref, where you build whatever you want it to look like and
> add the few DT nodes to inform firmware of a few parameters,
> should be completely independent of mach-virt. I.e. I don't think
> it even makes sense to refactor those few nodes in order to
> share a tiny bit of DT node generation code.
>
Yes, this is what I intended to do as I mentioned.

> Thanks,
> drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 30 August 2018 at 21:29, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 30 August 2018 at 12:02, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> On Thu, Aug 30, 2018 at 09:39:33AM +0100, Peter Maydell wrote:
>>> On 30 August 2018 at 09:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>>> > On Thu, Aug 30, 2018 at 03:07:29PM +0800, Hongbo Zhang wrote:
>>> >> @Ard, @Leif, is there any possibility to remove all the DT nodes?
>>> >> On real hardware, how does UEFI find the memory size and CPU number?
>>> >
>>> > Usually by asking some form of SCP/PMU.
>>> > So until we have rock-solid multi-cpu-instance (making terms up here,
>>> > there may be a proper name already?) support upstream, we need to take
>>> > shortcuts.
>>>
>>> I would expect that you'd want a "faked-on-the-QEMU end" SCP
>>> communications endpoint to start with anyway. We have one of
>>> those for vexpress, for instance.
>>
>> Ah, I wasn't aware we did!
>>
>> In that case - sure, that is probably the more sensible solution.
>> Does it emulate SCMI?
>>
>
> The purpose of the SBSA/SBBR qemu machine is prototyping the
> interactions between rich firmware and the OS for things like RAS,
> secure variable storage for UEFI or capsule update.
>
> How exactly the firmware figures out how many CPUs and how much memory
> we are running with is out of scope for this, and so I don't think
> there is a need to build something from scratch here: DT will do just
> fine, given that both EDK2 and ARM-TF have working DT code for these
> things.

Ard, we need to clarify which DT nodes should be kept in SBSA-ref QEMU.
I've researched and tested, there are 3 kinds of DT nodes in the
current 'virt' machine:

a) CPU DT node, because it is dynamic according to input parameters,
and we don't want to get it by other way, so we agreed to keep it.
b) GIC, UART, Timer (and RTC?) nodes, these nodes are now used by
UEFI, if removed, UEFI needs a rework to not rely on them any longer.
c) All the other devices nodes, such as GPIO, PCIE etc, currently the
UEFI doesn't need them, so UEFI can boot with them removed.

How about b) and c)?
my idea is to keep a) only, remove b) and thus a UEFI modification
accordingly is to be done, c) can be removed freely.

So what is your consideration from firmware point of view?

>
> fw_cfg, on the other hand, is unsuitable for us. Generating ACPI
> tables in a different way from actual bare metal is not a good idea,
> given that things like RAS interact with ACPI as well, and fw_cfg is
> also exposed to the OS in the default mach-virt configuration. So we
> could tweak fw_cfg to be more suitable, but I think that does not
> improve the usefulness of our prototype at all.
>
> At some point, it would be nice if we could get UEFI to receive its
> platform description from ARM-TF in a generic manner, and it would be
> good for EDK2 and ARM-TF to align on this. So even if we decide that
> SCMI is more suitable way to convey this information from QEMU to the
> guest firmware, I would object to implementing the client side of that
> in UEFI, given that it should be ARM-TF that provides this
> information.

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Sep 05, 2018 at 06:08:57PM +0800, Hongbo Zhang wrote:
> On 30 August 2018 at 21:29, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 30 August 2018 at 12:02, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> On Thu, Aug 30, 2018 at 09:39:33AM +0100, Peter Maydell wrote:
> >>> On 30 August 2018 at 09:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >>> > On Thu, Aug 30, 2018 at 03:07:29PM +0800, Hongbo Zhang wrote:
> >>> >> @Ard, @Leif, is there any possibility to remove all the DT nodes?
> >>> >> On real hardware, how does UEFI find the memory size and CPU number?
> >>> >
> >>> > Usually by asking some form of SCP/PMU.
> >>> > So until we have rock-solid multi-cpu-instance (making terms up here,
> >>> > there may be a proper name already?) support upstream, we need to take
> >>> > shortcuts.
> >>>
> >>> I would expect that you'd want a "faked-on-the-QEMU end" SCP
> >>> communications endpoint to start with anyway. We have one of
> >>> those for vexpress, for instance.
> >>
> >> Ah, I wasn't aware we did!
> >>
> >> In that case - sure, that is probably the more sensible solution.
> >> Does it emulate SCMI?
> >>
> >
> > The purpose of the SBSA/SBBR qemu machine is prototyping the
> > interactions between rich firmware and the OS for things like RAS,
> > secure variable storage for UEFI or capsule update.
> >
> > How exactly the firmware figures out how many CPUs and how much memory
> > we are running with is out of scope for this, and so I don't think
> > there is a need to build something from scratch here: DT will do just
> > fine, given that both EDK2 and ARM-TF have working DT code for these
> > things.
> 
> Ard, we need to clarify which DT nodes should be kept in SBSA-ref QEMU.
> I've researched and tested, there are 3 kinds of DT nodes in the
> current 'virt' machine:
> 
> a) CPU DT node, because it is dynamic according to input parameters,
> and we don't want to get it by other way, so we agreed to keep it.

I have a patch series on the list[*] (which I need to refresh) that
would also add a generated cpu-map node. I think the SBSA-ref machine
may be interested in testing more complex cpu topologies, but that won't
be easy to do with ACPI boot guests unless the topology is hard coded.
How will sbsa-ref know when it should use a hard coded cpu topology vs.
a dynamic number of cpus in a flat topology?

[*] http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg01170.html

> b) GIC, UART, Timer (and RTC?) nodes, these nodes are now used by
> UEFI, if removed, UEFI needs a rework to not rely on them any longer.

Will sbsa-ref support GICv2? If so, then timer and pmu DT nodes will
need to have their irqflags fields modified depending on the number of
configured CPUs.

> c) All the other devices nodes, such as GPIO, PCIE etc, currently the
> UEFI doesn't need them, so UEFI can boot with them removed.
> 
> How about b) and c)?
> my idea is to keep a) only, remove b) and thus a UEFI modification
> accordingly is to be done, c) can be removed freely.

So UEFI would learn to augment static DTBs with QEMU generated
DT nodes and/or other auxiliary info from QEMU passed to it
through the "guest DT" (which is now looking more like a guest
firmware configuration description)? Is it worth it? Why not
just require the entire DTB to be provided by the user on the
QEMU command line and completely ignore QEMU parameters like
'-smp'?

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

On 5 September 2018 at 20:02, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Sep 05, 2018 at 06:08:57PM +0800, Hongbo Zhang wrote:
>> On 30 August 2018 at 21:29, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > On 30 August 2018 at 12:02, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> >> On Thu, Aug 30, 2018 at 09:39:33AM +0100, Peter Maydell wrote:
>> >>> On 30 August 2018 at 09:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> >>> > On Thu, Aug 30, 2018 at 03:07:29PM +0800, Hongbo Zhang wrote:
>> >>> >> @Ard, @Leif, is there any possibility to remove all the DT nodes?
>> >>> >> On real hardware, how does UEFI find the memory size and CPU number?
>> >>> >
>> >>> > Usually by asking some form of SCP/PMU.
>> >>> > So until we have rock-solid multi-cpu-instance (making terms up here,
>> >>> > there may be a proper name already?) support upstream, we need to take
>> >>> > shortcuts.
>> >>>
>> >>> I would expect that you'd want a "faked-on-the-QEMU end" SCP
>> >>> communications endpoint to start with anyway. We have one of
>> >>> those for vexpress, for instance.
>> >>
>> >> Ah, I wasn't aware we did!
>> >>
>> >> In that case - sure, that is probably the more sensible solution.
>> >> Does it emulate SCMI?
>> >>
>> >
>> > The purpose of the SBSA/SBBR qemu machine is prototyping the
>> > interactions between rich firmware and the OS for things like RAS,
>> > secure variable storage for UEFI or capsule update.
>> >
>> > How exactly the firmware figures out how many CPUs and how much memory
>> > we are running with is out of scope for this, and so I don't think
>> > there is a need to build something from scratch here: DT will do just
>> > fine, given that both EDK2 and ARM-TF have working DT code for these
>> > things.
>>
>> Ard, we need to clarify which DT nodes should be kept in SBSA-ref QEMU.
>> I've researched and tested, there are 3 kinds of DT nodes in the
>> current 'virt' machine:
>>
>> a) CPU DT node, because it is dynamic according to input parameters,
>> and we don't want to get it by other way, so we agreed to keep it.
>
> I have a patch series on the list[*] (which I need to refresh) that
> would also add a generated cpu-map node. I think the SBSA-ref machine
> may be interested in testing more complex cpu topologies, but that won't
> be easy to do with ACPI boot guests unless the topology is hard coded.
> How will sbsa-ref know when it should use a hard coded cpu topology vs.
> a dynamic number of cpus in a flat topology?
>
If I understand you question correctly: for the cpu topology, firmware
always get CPU DT from QEMU (which is generated dynamically form
command parameter), so it is always dynamic, not hard coded in this
case.
(and then firmware may translate this DT and insert it into a whole
ACPI to load OS, it is up to the firmware developer)

And I even considered, do we really need dynamic CPU nodes for such
platform? memory size has to be dynamic, but it isn't in DT.

> [*] http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg01170.html
>
>> b) GIC, UART, Timer (and RTC?) nodes, these nodes are now used by
>> UEFI, if removed, UEFI needs a rework to not rely on them any longer.
>
> Will sbsa-ref support GICv2? If so, then timer and pmu DT nodes will
> need to have their irqflags fields modified depending on the number of
> configured CPUs.
>
No, I removed the GICv2 in next patch.

>> c) All the other devices nodes, such as GPIO, PCIE etc, currently the
>> UEFI doesn't need them, so UEFI can boot with them removed.
>>
>> How about b) and c)?
>> my idea is to keep a) only, remove b) and thus a UEFI modification
>> accordingly is to be done, c) can be removed freely.
>
> So UEFI would learn to augment static DTBs with QEMU generated
> DT nodes and/or other auxiliary info from QEMU passed to it
> through the "guest DT" (which is now looking more like a guest
> firmware configuration description)? Is it worth it? Why not
> just require the entire DTB to be provided by the user on the
> QEMU command line and completely ignore QEMU parameters like
> '-smp'?
>
UEFI loads OS by ACPI, not DT for this machine.
But UEFI has to handle DT too, becuase there is DT passing info from
ARMTF to UEFI, and this dynamic CPU DT as well.
I think every thing UEFI gets from DT should translated into ACPI,
again,it is my understaning now, it is up to firmware developers.

"entire DTB provided by the user on the QEMU command line", you mean
firmware can use it to load OS? it is not fine, some featurs such as
RAS, may still need ACPI, so ACPI is to be used to load OS on this
sbsa-ref machine.

> Thanks,
> drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Andrew Jones]

On Wed, Sep 05, 2018 at 10:09:46PM +0800, Hongbo Zhang wrote:
> On 5 September 2018 at 20:02, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Sep 05, 2018 at 06:08:57PM +0800, Hongbo Zhang wrote:
> >> On 30 August 2018 at 21:29, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> > On 30 August 2018 at 12:02, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> >> On Thu, Aug 30, 2018 at 09:39:33AM +0100, Peter Maydell wrote:
> >> >>> On 30 August 2018 at 09:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> >>> > On Thu, Aug 30, 2018 at 03:07:29PM +0800, Hongbo Zhang wrote:
> >> >>> >> @Ard, @Leif, is there any possibility to remove all the DT nodes?
> >> >>> >> On real hardware, how does UEFI find the memory size and CPU number?
> >> >>> >
> >> >>> > Usually by asking some form of SCP/PMU.
> >> >>> > So until we have rock-solid multi-cpu-instance (making terms up here,
> >> >>> > there may be a proper name already?) support upstream, we need to take
> >> >>> > shortcuts.
> >> >>>
> >> >>> I would expect that you'd want a "faked-on-the-QEMU end" SCP
> >> >>> communications endpoint to start with anyway. We have one of
> >> >>> those for vexpress, for instance.
> >> >>
> >> >> Ah, I wasn't aware we did!
> >> >>
> >> >> In that case - sure, that is probably the more sensible solution.
> >> >> Does it emulate SCMI?
> >> >>
> >> >
> >> > The purpose of the SBSA/SBBR qemu machine is prototyping the
> >> > interactions between rich firmware and the OS for things like RAS,
> >> > secure variable storage for UEFI or capsule update.
> >> >
> >> > How exactly the firmware figures out how many CPUs and how much memory
> >> > we are running with is out of scope for this, and so I don't think
> >> > there is a need to build something from scratch here: DT will do just
> >> > fine, given that both EDK2 and ARM-TF have working DT code for these
> >> > things.
> >>
> >> Ard, we need to clarify which DT nodes should be kept in SBSA-ref QEMU.
> >> I've researched and tested, there are 3 kinds of DT nodes in the
> >> current 'virt' machine:
> >>
> >> a) CPU DT node, because it is dynamic according to input parameters,
> >> and we don't want to get it by other way, so we agreed to keep it.
> >
> > I have a patch series on the list[*] (which I need to refresh) that
> > would also add a generated cpu-map node. I think the SBSA-ref machine
> > may be interested in testing more complex cpu topologies, but that won't
> > be easy to do with ACPI boot guests unless the topology is hard coded.
> > How will sbsa-ref know when it should use a hard coded cpu topology vs.
> > a dynamic number of cpus in a flat topology?
> >
> If I understand you question correctly: for the cpu topology, firmware
> always get CPU DT from QEMU (which is generated dynamically form
> command parameter), so it is always dynamic, not hard coded in this
> case.
> (and then firmware may translate this DT and insert it into a whole
> ACPI to load OS, it is up to the firmware developer)

I doubt firmware developers will want to generate ACPI tables. Maybe
multiple tables can be pre-generated and then selected by firmware
using a parameter passed to it from QEMU through the DT though.

> 
> And I even considered, do we really need dynamic CPU nodes for such
> platform? 

That was my question. If you do, then I think things get complicated
quick.

> memory size has to be dynamic, but it isn't in DT.

Memory size is in the DT. Making it dynamic may also be complicated
though unless you only want one chunk of contiguous memory. If you
want to represent SBSA machines that have multiple NUMA nodes and/or
gaps in the physical address space, then your ACPI tables will not
be patched easily. I.e. you'll want static ACPI tables regarding
different memory configurations too.

> 
> > [*] http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg01170.html
> >
> >> b) GIC, UART, Timer (and RTC?) nodes, these nodes are now used by
> >> UEFI, if removed, UEFI needs a rework to not rely on them any longer.
> >
> > Will sbsa-ref support GICv2? If so, then timer and pmu DT nodes will
> > need to have their irqflags fields modified depending on the number of
> > configured CPUs.
> >
> No, I removed the GICv2 in next patch.
> 
> >> c) All the other devices nodes, such as GPIO, PCIE etc, currently the
> >> UEFI doesn't need them, so UEFI can boot with them removed.
> >>
> >> How about b) and c)?
> >> my idea is to keep a) only, remove b) and thus a UEFI modification
> >> accordingly is to be done, c) can be removed freely.
> >
> > So UEFI would learn to augment static DTBs with QEMU generated
> > DT nodes and/or other auxiliary info from QEMU passed to it
> > through the "guest DT" (which is now looking more like a guest
> > firmware configuration description)? Is it worth it? Why not
> > just require the entire DTB to be provided by the user on the
> > QEMU command line and completely ignore QEMU parameters like
> > '-smp'?
> >
> UEFI loads OS by ACPI, not DT for this machine.
> But UEFI has to handle DT too, becuase there is DT passing info from
> ARMTF to UEFI, and this dynamic CPU DT as well.
> I think every thing UEFI gets from DT should translated into ACPI,
> again,it is my understaning now, it is up to firmware developers.
> 
> "entire DTB provided by the user on the QEMU command line", you mean
> firmware can use it to load OS? it is not fine, some featurs such as
> RAS, may still need ACPI, so ACPI is to be used to load OS on this
> sbsa-ref machine.

If booting the guest with ACPI is the plan, but using QEMU's ACPI
generation is not the plan. Then either firmware will have to reinvent
that generation (unlikely) or multiple ACPI tables representing all
the desired configs will need to be pre-generated and made selectable
in some way. Either by choosing from multiple UEFI builds where each
build has one set of tables, or by having all the tables in one UEFI
build and then adding some UEFI code to select them based on parameters
it can get from QEMU through DT.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm: Add Arm Enterprise machine type

[Hongbo Zhang]

Well, thank you all for the discussions, we achieved some agreement,
and there are still things for further review.
The v3 patch has been sent out, we can move there and review/discuss
the updated patch now.

Thanks.

On 5 September 2018 at 23:00, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Sep 05, 2018 at 10:09:46PM +0800, Hongbo Zhang wrote:
>> On 5 September 2018 at 20:02, Andrew Jones <drjones@redhat.com> wrote:
>> > On Wed, Sep 05, 2018 at 06:08:57PM +0800, Hongbo Zhang wrote:
>> >> On 30 August 2018 at 21:29, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >> > On 30 August 2018 at 12:02, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> >> >> On Thu, Aug 30, 2018 at 09:39:33AM +0100, Peter Maydell wrote:
>> >> >>> On 30 August 2018 at 09:31, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> >> >>> > On Thu, Aug 30, 2018 at 03:07:29PM +0800, Hongbo Zhang wrote:
>> >> >>> >> @Ard, @Leif, is there any possibility to remove all the DT nodes?
>> >> >>> >> On real hardware, how does UEFI find the memory size and CPU number?
>> >> >>> >
>> >> >>> > Usually by asking some form of SCP/PMU.
>> >> >>> > So until we have rock-solid multi-cpu-instance (making terms up here,
>> >> >>> > there may be a proper name already?) support upstream, we need to take
>> >> >>> > shortcuts.
>> >> >>>
>> >> >>> I would expect that you'd want a "faked-on-the-QEMU end" SCP
>> >> >>> communications endpoint to start with anyway. We have one of
>> >> >>> those for vexpress, for instance.
>> >> >>
>> >> >> Ah, I wasn't aware we did!
>> >> >>
>> >> >> In that case - sure, that is probably the more sensible solution.
>> >> >> Does it emulate SCMI?
>> >> >>
>> >> >
>> >> > The purpose of the SBSA/SBBR qemu machine is prototyping the
>> >> > interactions between rich firmware and the OS for things like RAS,
>> >> > secure variable storage for UEFI or capsule update.
>> >> >
>> >> > How exactly the firmware figures out how many CPUs and how much memory
>> >> > we are running with is out of scope for this, and so I don't think
>> >> > there is a need to build something from scratch here: DT will do just
>> >> > fine, given that both EDK2 and ARM-TF have working DT code for these
>> >> > things.
>> >>
>> >> Ard, we need to clarify which DT nodes should be kept in SBSA-ref QEMU.
>> >> I've researched and tested, there are 3 kinds of DT nodes in the
>> >> current 'virt' machine:
>> >>
>> >> a) CPU DT node, because it is dynamic according to input parameters,
>> >> and we don't want to get it by other way, so we agreed to keep it.
>> >
>> > I have a patch series on the list[*] (which I need to refresh) that
>> > would also add a generated cpu-map node. I think the SBSA-ref machine
>> > may be interested in testing more complex cpu topologies, but that won't
>> > be easy to do with ACPI boot guests unless the topology is hard coded.
>> > How will sbsa-ref know when it should use a hard coded cpu topology vs.
>> > a dynamic number of cpus in a flat topology?
>> >
>> If I understand you question correctly: for the cpu topology, firmware
>> always get CPU DT from QEMU (which is generated dynamically form
>> command parameter), so it is always dynamic, not hard coded in this
>> case.
>> (and then firmware may translate this DT and insert it into a whole
>> ACPI to load OS, it is up to the firmware developer)
>
> I doubt firmware developers will want to generate ACPI tables. Maybe
> multiple tables can be pre-generated and then selected by firmware
> using a parameter passed to it from QEMU through the DT though.
>
>>
>> And I even considered, do we really need dynamic CPU nodes for such
>> platform?
>
> That was my question. If you do, then I think things get complicated
> quick.
>
>> memory size has to be dynamic, but it isn't in DT.
>
> Memory size is in the DT. Making it dynamic may also be complicated
> though unless you only want one chunk of contiguous memory. If you
> want to represent SBSA machines that have multiple NUMA nodes and/or
> gaps in the physical address space, then your ACPI tables will not
> be patched easily. I.e. you'll want static ACPI tables regarding
> different memory configurations too.
>
>>
>> > [*] http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg01170.html
>> >
>> >> b) GIC, UART, Timer (and RTC?) nodes, these nodes are now used by
>> >> UEFI, if removed, UEFI needs a rework to not rely on them any longer.
>> >
>> > Will sbsa-ref support GICv2? If so, then timer and pmu DT nodes will
>> > need to have their irqflags fields modified depending on the number of
>> > configured CPUs.
>> >
>> No, I removed the GICv2 in next patch.
>>
>> >> c) All the other devices nodes, such as GPIO, PCIE etc, currently the
>> >> UEFI doesn't need them, so UEFI can boot with them removed.
>> >>
>> >> How about b) and c)?
>> >> my idea is to keep a) only, remove b) and thus a UEFI modification
>> >> accordingly is to be done, c) can be removed freely.
>> >
>> > So UEFI would learn to augment static DTBs with QEMU generated
>> > DT nodes and/or other auxiliary info from QEMU passed to it
>> > through the "guest DT" (which is now looking more like a guest
>> > firmware configuration description)? Is it worth it? Why not
>> > just require the entire DTB to be provided by the user on the
>> > QEMU command line and completely ignore QEMU parameters like
>> > '-smp'?
>> >
>> UEFI loads OS by ACPI, not DT for this machine.
>> But UEFI has to handle DT too, becuase there is DT passing info from
>> ARMTF to UEFI, and this dynamic CPU DT as well.
>> I think every thing UEFI gets from DT should translated into ACPI,
>> again,it is my understaning now, it is up to firmware developers.
>>
>> "entire DTB provided by the user on the QEMU command line", you mean
>> firmware can use it to load OS? it is not fine, some featurs such as
>> RAS, may still need ACPI, so ACPI is to be used to load OS on this
>> sbsa-ref machine.
>
> If booting the guest with ACPI is the plan, but using QEMU's ACPI
> generation is not the plan. Then either firmware will have to reinvent
> that generation (unlikely) or multiple ACPI tables representing all
> the desired configs will need to be pre-generated and made selectable
> in some way. Either by choosing from multiple UEFI builds where each
> build has one set of tables, or by having all the tables in one UEFI
> build and then adding some UEFI code to select them based on parameters
> it can get from QEMU through DT.
>
> Thanks,
> drew

Re: [Qemu-devel] [PATCH v2 1/2] hw/arm: check fw_cfg return value before using it

[Peter Maydell]

On 25 July 2018 at 06:30, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> The fw_cfg value returned from fw_cfg_find() may be NULL, so check it
> before using.
>
> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

Hi -- this patch series seems to be missing its cover letter.
Please could you include a cover letter for any patchset
with more than one patch -- some of our automated patch
email handling tools rely on it.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/2] hw/arm: check fw_cfg return value before using it

[Hongbo Zhang]

On 25 July 2018 at 17:00, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 25 July 2018 at 06:30, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>> The fw_cfg value returned from fw_cfg_find() may be NULL, so check it
>> before using.
>>
>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>
> Hi -- this patch series seems to be missing its cover letter.
> Please could you include a cover letter for any patchset
> with more than one patch -- some of our automated patch
> email handling tools rely on it.
>
Get it.
2/2 needs a v3 at least, so will remember to include the cover letter
in v3 then.

Thanks.

> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v2 1/2] hw/arm: check fw_cfg return value before using it

[Peter Maydell]

On 25 July 2018 at 06:30, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
> The fw_cfg value returned from fw_cfg_find() may be NULL, so check it
> before using.
>
> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> ---
>  hw/arm/boot.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index e09201c..43b217f 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -930,6 +930,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>      hwaddr entry;
>      static const ARMInsnFixup *primary_loader;
>      AddressSpace *as = arm_boot_address_space(cpu, info);
> +    FWCfgState *fw_cfg;
>
>      /* CPU objects (unlike devices) are not automatically reset on system
>       * reset, so we must always register a handler to do so. If we're
> @@ -960,11 +961,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>              info->dtb_start = info->loader_start;
>          }
>
> -        if (info->kernel_filename) {
> -            FWCfgState *fw_cfg;
> +        fw_cfg = fw_cfg_find();
> +        if (info->kernel_filename && fw_cfg) {
>              bool try_decompressing_kernel;

This can only happen if:
 * the user provided a firmware blob
 * the user provided a -kernel option
 * the board does not have a fw_cfg so we can't pass the kernel to
   the firmware blob
right?

I think in this situation we should exit with a helpful error
message to the user (telling them that this board model does
not support using the -kernel option when a guest firmware image
is being booted), rather than silently ignoring the option.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/2] hw/arm: check fw_cfg return value before using it

[Hongbo Zhang]

On 31 July 2018 at 02:07, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 25 July 2018 at 06:30, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>> The fw_cfg value returned from fw_cfg_find() may be NULL, so check it
>> before using.
>>
>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>> ---
>>  hw/arm/boot.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index e09201c..43b217f 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -930,6 +930,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>>      hwaddr entry;
>>      static const ARMInsnFixup *primary_loader;
>>      AddressSpace *as = arm_boot_address_space(cpu, info);
>> +    FWCfgState *fw_cfg;
>>
>>      /* CPU objects (unlike devices) are not automatically reset on system
>>       * reset, so we must always register a handler to do so. If we're
>> @@ -960,11 +961,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>>              info->dtb_start = info->loader_start;
>>          }
>>
>> -        if (info->kernel_filename) {
>> -            FWCfgState *fw_cfg;
>> +        fw_cfg = fw_cfg_find();
>> +        if (info->kernel_filename && fw_cfg) {
>>              bool try_decompressing_kernel;
>
> This can only happen if:
>  * the user provided a firmware blob
>  * the user provided a -kernel option
>  * the board does not have a fw_cfg so we can't pass the kernel to
>    the firmware blob
> right?
>
Yes.

> I think in this situation we should exit with a helpful error
> message to the user (telling them that this board model does
> not support using the -kernel option when a guest firmware image
> is being booted), rather than silently ignoring the option.
>
The suggestion sounds better.
Thanks.

> thanks
> -- PMM