diff mbox series

hw/arm: Add SBSA machine type

Message ID 1530094388-607-1-git-send-email-hongbo.zhang@linaro.org
State New
Headers show
Series hw/arm: Add SBSA machine type | expand

Commit Message

Hongbo Zhang June 27, 2018, 10:13 a.m. UTC
This patch introduces a new Arm machine type 'SBSA' with features:
 - Based on legacy 'virt' machine type.
 - Newly designed memory map.
 - EL2 and EL3 are enabled 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.
 - E1000 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.

This is the prototype, more features can be added in futrue.

Purpose of this is to have a standard QEMU platform for Arm firmware
developement etc. where the legacy machines cannot meets requirements.

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

Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

---
 hw/arm/virt-acpi-build.c |  59 +++++++++++++-
 hw/arm/virt.c            | 196 ++++++++++++++++++++++++++++++++++++++++++++++-
 include/hw/arm/virt.h    |   3 +
 3 files changed, 254 insertions(+), 4 deletions(-)

-- 
2.7.4

Comments

Dr. David Alan Gilbert June 27, 2018, 12:31 p.m. UTC | #1
* Hongbo Zhang (hongbo.zhang@linaro.org) wrote:
> This patch introduces a new Arm machine type 'SBSA' with features:

>  - Based on legacy 'virt' machine type.

>  - Newly designed memory map.

>  - EL2 and EL3 are enabled 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.

>  - E1000 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.


I'm a bit confused; do you have real modern ARM hardware that has an
e1000 on it?  If I understand correctly, e1000 is the old PCI version
and the e1000e is at least the more modern PCIe version which makes
more sense if you're building on PCIe.  
However, if you:
  a) Don't have real hardware with the e1000 on
  b) Do have PCIe

then to my mind it makes sense to use virtio-net-pci rather than
an e1000e.

Dave

> This is the prototype, more features can be added in futrue.

> 

> Purpose of this is to have a standard QEMU platform for Arm firmware

> developement etc. where the legacy machines cannot meets requirements.

> 

> Arm Trusted Firmware and UEFI porting to this are done seperately.

> 

> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

> ---

>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-

>  hw/arm/virt.c            | 196 ++++++++++++++++++++++++++++++++++++++++++++++-

>  include/hw/arm/virt.h    |   3 +

>  3 files changed, 254 insertions(+), 4 deletions(-)

> 

> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c

> index 6ea47e2..60af414 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->sbsa) {

> +        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->sbsa) {

> +        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 742f68a..491f23b 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/cutils.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 sbsa_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 sbsa_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"),

> @@ -696,6 +743,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)

>  {

> @@ -1107,13 +1220,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->sbsa) {

> +                    nd->model = g_strdup("e1000");

> +                } else {

> +                    nd->model = g_strdup("virtio");

> +                }

>              }

>  

>              pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);

>          }

>      }

>  

> +    if (vms->sbsa) {

> +        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,

> @@ -1502,11 +1623,18 @@ static void machvirt_init(MachineState *machine)

>  

>      create_gpio(vms, pic);

>  

> +    if (vms->sbsa) {

> +        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->sbsa) {

> +        create_virtio_devices(vms, pic);

> +    }

>  

>      vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);

>      rom_set_fw(vms->fw_cfg);

> @@ -1818,6 +1946,7 @@ static void virt_3_0_instance_init(Object *obj)

>  

>      vms->memmap = a15memmap;

>      vms->irqmap = a15irqmap;

> +    vms->sbsa = false;

>  }

>  

>  static void virt_machine_3_0_options(MachineClass *mc)

> @@ -1950,3 +2079,66 @@ static void virt_machine_2_6_options(MachineClass *mc)

>      vmc->no_pmu = true;

>  }

>  DEFINE_VIRT_MACHINE(2, 6)

> +

> +static void sbsa_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;

> +

> +    /* This can be changed to GICv3 when firmware is ready*/

> +    vms->gic_version = 2;

> +    object_property_add_str(obj, "gic-version", virt_get_gic_version,

> +                        virt_set_gic_version, NULL);

> +    object_property_set_description(obj, "gic-version",

> +                                    "Set GIC version. "

> +                                    "Valid values are 2, 3, host, max", NULL);

> +    vms->its = true;

> +

> +    vms->memmap = sbsa_memmap;

> +    vms->irqmap = sbsa_irqmap;

> +    vms->sbsa = true;

> +}

> +

> +static void sbsa_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 * G_BYTE;

> +    mc->default_cpus = 4;

> +    mc->desc = "QEMU 'SBSA' ARM Virtual Machine";

> +}

> +

> +static const TypeInfo sbsa_info = {

> +    .name          = MACHINE_TYPE_NAME("sbsa"),

> +    .parent        = TYPE_VIRT_MACHINE,

> +    .instance_init = sbsa_instance_init,

> +    .class_init    = sbsa_class_init,

> +};

> +

> +static void sbsa_machine_init(void)

> +{

> +    type_register_static(&sbsa_info);

> +}

> +

> +type_init(sbsa_machine_init);

> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

> index 9a870cc..619473e 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 sbsa;

>  } VirtMachineState;

>  

>  #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)

> -- 

> 2.7.4

> 

> 

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Igor Mammedov June 27, 2018, 2:56 p.m. UTC | #2
On Wed, 27 Jun 2018 18:13:08 +0800
Hongbo Zhang <hongbo.zhang@linaro.org> wrote:

> This patch introduces a new Arm machine type 'SBSA' with features:

>  - Based on legacy 'virt' machine type.

>  - Newly designed memory map.

>  - EL2 and EL3 are enabled 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.

>  - E1000 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.

I'd drop all default (convenience) devices unless they are specified in SBSA,
and make management explicitly provide needed devices on CLI.

> 

> This is the prototype, more features can be added in futrue.

> 

> Purpose of this is to have a standard QEMU platform for Arm firmware

> developement etc. where the legacy machines cannot meets requirements.

> 

> Arm Trusted Firmware and UEFI porting to this are done seperately.

> 

> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

> ---

[...]

> @@ -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 sbsa_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 },

does spec require support for 32-bit guest or is it only 64bit,
if the later I'd put it somewhere high where we can increase region
to terrabytes.

another idea that was floating around (considering we don't care about legacy)
is to use flexible base address and tell firmware[*] via register where it's.
Then it would be able to support 32 guest with small amount of RAM
and 64 bit guests with huge amount of RAM using a single memory range.
(to keep memory management simple). It's also future friendly, as in case
if we need to move it we could do easily by changing base address for new machine
type only and firmware would automatically pick it up from register
(without all compat nightmare we have with 2 regions in pc/q35 machines).

[*] When I've talked with Laszlo about it he said it was not easy to do in uefi
but still possible.

same applies to GIG regions/mmio/ecam where we are twisting our hands when trying to
increase number of things.

[...]
Hongbo Zhang June 28, 2018, 4:19 a.m. UTC | #3
On 27 June 2018 at 20:31, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Hongbo Zhang (hongbo.zhang@linaro.org) wrote:

>> This patch introduces a new Arm machine type 'SBSA' with features:

>>  - Based on legacy 'virt' machine type.

>>  - Newly designed memory map.

>>  - EL2 and EL3 are enabled 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.

>>  - E1000 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.

>

> I'm a bit confused; do you have real modern ARM hardware that has an

> e1000 on it?  If I understand correctly, e1000 is the old PCI version

> and the e1000e is at least the more modern PCIe version which makes

> more sense if you're building on PCIe.

> However, if you:

>   a) Don't have real hardware with the e1000 on

>   b) Do have PCIe

>

Thanks for review.

Yes, e1000e is used on modern hardware PCIe, using e1000e can reflect
the real world.
I like e1000e too, but I checked q35, it still uses an e1000 attached
to pcie bus.
Another fact is, when compile the arm64 kernel by defconfig, the e1000
isn't selected by default, but e1000e is.
That is why I still chose e1000 here.
(Maybe we can change the kernel defconfig to e1000e, then we chose
e1000e here too)

> then to my mind it makes sense to use virtio-net-pci rather than

> an e1000e.

>

Well, this is to emulate a 'true' hardware for firmware etc
development, primary purpose it to emulate hardware functions, not for
performance, so virtio is disabled, some firmware, boot loaders etc
may not work with virtio either, if people want virtio, they can use
the legacy 'virt' machine.

> Dave

>

>> This is the prototype, more features can be added in futrue.

>>

>> Purpose of this is to have a standard QEMU platform for Arm firmware

>> developement etc. where the legacy machines cannot meets requirements.

>>

>> Arm Trusted Firmware and UEFI porting to this are done seperately.

>>

>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

>> ---

>>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-

>>  hw/arm/virt.c            | 196 ++++++++++++++++++++++++++++++++++++++++++++++-

>>  include/hw/arm/virt.h    |   3 +

>>  3 files changed, 254 insertions(+), 4 deletions(-)

>>

>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c

>> index 6ea47e2..60af414 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->sbsa) {

>> +        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->sbsa) {

>> +        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 742f68a..491f23b 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/cutils.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 sbsa_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 sbsa_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"),

>> @@ -696,6 +743,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)

>>  {

>> @@ -1107,13 +1220,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->sbsa) {

>> +                    nd->model = g_strdup("e1000");

>> +                } else {

>> +                    nd->model = g_strdup("virtio");

>> +                }

>>              }

>>

>>              pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);

>>          }

>>      }

>>

>> +    if (vms->sbsa) {

>> +        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,

>> @@ -1502,11 +1623,18 @@ static void machvirt_init(MachineState *machine)

>>

>>      create_gpio(vms, pic);

>>

>> +    if (vms->sbsa) {

>> +        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->sbsa) {

>> +        create_virtio_devices(vms, pic);

>> +    }

>>

>>      vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);

>>      rom_set_fw(vms->fw_cfg);

>> @@ -1818,6 +1946,7 @@ static void virt_3_0_instance_init(Object *obj)

>>

>>      vms->memmap = a15memmap;

>>      vms->irqmap = a15irqmap;

>> +    vms->sbsa = false;

>>  }

>>

>>  static void virt_machine_3_0_options(MachineClass *mc)

>> @@ -1950,3 +2079,66 @@ static void virt_machine_2_6_options(MachineClass *mc)

>>      vmc->no_pmu = true;

>>  }

>>  DEFINE_VIRT_MACHINE(2, 6)

>> +

>> +static void sbsa_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;

>> +

>> +    /* This can be changed to GICv3 when firmware is ready*/

>> +    vms->gic_version = 2;

>> +    object_property_add_str(obj, "gic-version", virt_get_gic_version,

>> +                        virt_set_gic_version, NULL);

>> +    object_property_set_description(obj, "gic-version",

>> +                                    "Set GIC version. "

>> +                                    "Valid values are 2, 3, host, max", NULL);

>> +    vms->its = true;

>> +

>> +    vms->memmap = sbsa_memmap;

>> +    vms->irqmap = sbsa_irqmap;

>> +    vms->sbsa = true;

>> +}

>> +

>> +static void sbsa_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 * G_BYTE;

>> +    mc->default_cpus = 4;

>> +    mc->desc = "QEMU 'SBSA' ARM Virtual Machine";

>> +}

>> +

>> +static const TypeInfo sbsa_info = {

>> +    .name          = MACHINE_TYPE_NAME("sbsa"),

>> +    .parent        = TYPE_VIRT_MACHINE,

>> +    .instance_init = sbsa_instance_init,

>> +    .class_init    = sbsa_class_init,

>> +};

>> +

>> +static void sbsa_machine_init(void)

>> +{

>> +    type_register_static(&sbsa_info);

>> +}

>> +

>> +type_init(sbsa_machine_init);

>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

>> index 9a870cc..619473e 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 sbsa;

>>  } VirtMachineState;

>>

>>  #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)

>> --

>> 2.7.4

>>

>>

> --

> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Hongbo Zhang June 28, 2018, 8:11 a.m. UTC | #4
On 27 June 2018 at 22:56, Igor Mammedov <imammedo@redhat.com> wrote:
> On Wed, 27 Jun 2018 18:13:08 +0800

> Hongbo Zhang <hongbo.zhang@linaro.org> wrote:

>

>> This patch introduces a new Arm machine type 'SBSA' with features:

>>  - Based on legacy 'virt' machine type.

>>  - Newly designed memory map.

>>  - EL2 and EL3 are enabled 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.

>>  - E1000 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.

> I'd drop all default (convenience) devices unless they are specified in SBSA,

> and make management explicitly provide needed devices on CLI.

>

Thanks for review.

I mentioned these default values because they are different from the
'virt' machine from which this sbsa machine derives. (sbsa has uart
too, it is same with 'virt' PL011).
Qemu implementation should has such default values, if the sbsa
machine does not offer them, they fallback to the parent machine's.
The parent 'virt' machine's default cpu type is cortext-a15, that is
not proper for a armv8 server, and its default memory is 128M, that is
not enough to load uefi, even 512M cannot either, so I set the sbsa
defualt minimal memory to 1G.

The core numbers, is a new default. Server is smp in common sense,
single core isn't like a server, so some even want to set to max
capability as default, but this isn't good either I think, because
gicv3 memory space in Qemu can even support more than 200 cores, that
is too much.
(gicv2 currently support 8 cores, but there are issues to boot smp for
both gicv2 and gicv3 now, it should be another thread)

>>

>> This is the prototype, more features can be added in futrue.

>>

>> Purpose of this is to have a standard QEMU platform for Arm firmware

>> developement etc. where the legacy machines cannot meets requirements.

>>

>> Arm Trusted Firmware and UEFI porting to this are done seperately.

>>

>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

>> ---

> [...]

>

>> @@ -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 sbsa_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 },

> does spec require support for 32-bit guest or is it only 64bit,

> if the later I'd put it somewhere high where we can increase region

> to terrabytes.

>

Spec only requires it to be compliant with armv8.
Designed purpose of this is to run 64bit guests, because in practice
an arm server is usually 64bit.

> another idea that was floating around (considering we don't care about legacy)

> is to use flexible base address and tell firmware[*] via register where it's.

> Then it would be able to support 32 guest with small amount of RAM

> and 64 bit guests with huge amount of RAM using a single memory range.

> (to keep memory management simple). It's also future friendly, as in case

> if we need to move it we could do easily by changing base address for new machine

> type only and firmware would automatically pick it up from register

> (without all compat nightmare we have with 2 regions in pc/q35 machines).

>

> [*] When I've talked with Laszlo about it he said it was not easy to do in uefi

> but still possible.

Sounds not easy.
I think the first step is to get this merged, and then we can add more
features if necessary.
I've already seen some points in trusted firmware and uefi need to be fixed.

>

> same applies to GIG regions/mmio/ecam where we are twisting our hands when trying to

> increase number of things.

>

> [...]

>
Dr. David Alan Gilbert June 28, 2018, 8:12 a.m. UTC | #5
* Hongbo Zhang (hongbo.zhang@linaro.org) wrote:
> On 27 June 2018 at 20:31, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:

> > * Hongbo Zhang (hongbo.zhang@linaro.org) wrote:

> >> This patch introduces a new Arm machine type 'SBSA' with features:

> >>  - Based on legacy 'virt' machine type.

> >>  - Newly designed memory map.

> >>  - EL2 and EL3 are enabled 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.

> >>  - E1000 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.

> >

> > I'm a bit confused; do you have real modern ARM hardware that has an

> > e1000 on it?  If I understand correctly, e1000 is the old PCI version

> > and the e1000e is at least the more modern PCIe version which makes

> > more sense if you're building on PCIe.

> > However, if you:

> >   a) Don't have real hardware with the e1000 on

> >   b) Do have PCIe

> >

> Thanks for review.

> 

> Yes, e1000e is used on modern hardware PCIe, using e1000e can reflect

> the real world.

> I like e1000e too, but I checked q35, it still uses an e1000 attached

> to pcie bus.


I think we think of that choice on q35 as a mistake / or we didn't
have e1000e at the time.  If you used an e1000e then people could
actually buy an e1000e card to plug into the test system to compare.

> Another fact is, when compile the arm64 kernel by defconfig, the e1000

> isn't selected by default, but e1000e is.

> That is why I still chose e1000 here.

> (Maybe we can change the kernel defconfig to e1000e, then we chose

> e1000e here too)

> 

> > then to my mind it makes sense to use virtio-net-pci rather than

> > an e1000e.

> >

> Well, this is to emulate a 'true' hardware for firmware etc

> development, primary purpose it to emulate hardware functions, not for

> performance, so virtio is disabled, some firmware, boot loaders etc

> may not work with virtio either, if people want virtio, they can use

> the legacy 'virt' machine.


The virtio-mmio that the 'virt' machine has is quite odd; however
virtio-pci devices are much less special.

Dave

> > Dave

> >

> >> This is the prototype, more features can be added in futrue.

> >>

> >> Purpose of this is to have a standard QEMU platform for Arm firmware

> >> developement etc. where the legacy machines cannot meets requirements.

> >>

> >> Arm Trusted Firmware and UEFI porting to this are done seperately.

> >>

> >> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

> >> ---

> >>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-

> >>  hw/arm/virt.c            | 196 ++++++++++++++++++++++++++++++++++++++++++++++-

> >>  include/hw/arm/virt.h    |   3 +

> >>  3 files changed, 254 insertions(+), 4 deletions(-)

> >>

> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c

> >> index 6ea47e2..60af414 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->sbsa) {

> >> +        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->sbsa) {

> >> +        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 742f68a..491f23b 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/cutils.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 sbsa_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 sbsa_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"),

> >> @@ -696,6 +743,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)

> >>  {

> >> @@ -1107,13 +1220,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->sbsa) {

> >> +                    nd->model = g_strdup("e1000");

> >> +                } else {

> >> +                    nd->model = g_strdup("virtio");

> >> +                }

> >>              }

> >>

> >>              pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);

> >>          }

> >>      }

> >>

> >> +    if (vms->sbsa) {

> >> +        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,

> >> @@ -1502,11 +1623,18 @@ static void machvirt_init(MachineState *machine)

> >>

> >>      create_gpio(vms, pic);

> >>

> >> +    if (vms->sbsa) {

> >> +        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->sbsa) {

> >> +        create_virtio_devices(vms, pic);

> >> +    }

> >>

> >>      vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);

> >>      rom_set_fw(vms->fw_cfg);

> >> @@ -1818,6 +1946,7 @@ static void virt_3_0_instance_init(Object *obj)

> >>

> >>      vms->memmap = a15memmap;

> >>      vms->irqmap = a15irqmap;

> >> +    vms->sbsa = false;

> >>  }

> >>

> >>  static void virt_machine_3_0_options(MachineClass *mc)

> >> @@ -1950,3 +2079,66 @@ static void virt_machine_2_6_options(MachineClass *mc)

> >>      vmc->no_pmu = true;

> >>  }

> >>  DEFINE_VIRT_MACHINE(2, 6)

> >> +

> >> +static void sbsa_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;

> >> +

> >> +    /* This can be changed to GICv3 when firmware is ready*/

> >> +    vms->gic_version = 2;

> >> +    object_property_add_str(obj, "gic-version", virt_get_gic_version,

> >> +                        virt_set_gic_version, NULL);

> >> +    object_property_set_description(obj, "gic-version",

> >> +                                    "Set GIC version. "

> >> +                                    "Valid values are 2, 3, host, max", NULL);

> >> +    vms->its = true;

> >> +

> >> +    vms->memmap = sbsa_memmap;

> >> +    vms->irqmap = sbsa_irqmap;

> >> +    vms->sbsa = true;

> >> +}

> >> +

> >> +static void sbsa_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 * G_BYTE;

> >> +    mc->default_cpus = 4;

> >> +    mc->desc = "QEMU 'SBSA' ARM Virtual Machine";

> >> +}

> >> +

> >> +static const TypeInfo sbsa_info = {

> >> +    .name          = MACHINE_TYPE_NAME("sbsa"),

> >> +    .parent        = TYPE_VIRT_MACHINE,

> >> +    .instance_init = sbsa_instance_init,

> >> +    .class_init    = sbsa_class_init,

> >> +};

> >> +

> >> +static void sbsa_machine_init(void)

> >> +{

> >> +    type_register_static(&sbsa_info);

> >> +}

> >> +

> >> +type_init(sbsa_machine_init);

> >> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

> >> index 9a870cc..619473e 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 sbsa;

> >>  } VirtMachineState;

> >>

> >>  #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)

> >> --

> >> 2.7.4

> >>

> >>

> > --

> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Andrew Jones June 28, 2018, 9:04 a.m. UTC | #6
On Thu, Jun 28, 2018 at 04:11:56PM +0800, Hongbo Zhang wrote:
> On 27 June 2018 at 22:56, Igor Mammedov <imammedo@redhat.com> wrote:

> > On Wed, 27 Jun 2018 18:13:08 +0800

> > Hongbo Zhang <hongbo.zhang@linaro.org> wrote:

> >

> >> This patch introduces a new Arm machine type 'SBSA' with features:

> >>  - Based on legacy 'virt' machine type.

> >>  - Newly designed memory map.

> >>  - EL2 and EL3 are enabled 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.

> >>  - E1000 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.

> > I'd drop all default (convenience) devices unless they are specified in SBSA,

> > and make management explicitly provide needed devices on CLI.

> >

> Thanks for review.

> 

> I mentioned these default values because they are different from the

> 'virt' machine from which this sbsa machine derives. (sbsa has uart

> too, it is same with 'virt' PL011).

> Qemu implementation should has such default values, if the sbsa

> machine does not offer them, they fallback to the parent machine's.


QEMU's default devices is one of the problems with QEMU. Any serious
use of QEMU must be done with '-nodefaults' and then explicit devices.
To avoid requiring long command lines QEMU supports '-readconfig'. We
already have two mach-virt configs that can be used as a basis for a
new SBSA config. See docs/config/mach-virt-graphical.cfg and
docs/config/mach-virt-serial.cfg.

> The parent 'virt' machine's default cpu type is cortext-a15, that is

> not proper for a armv8 server, and its default memory is 128M, that is

> not enough to load uefi, even 512M cannot either, so I set the sbsa

> defualt minimal memory to 1G.


config files support setting a memory size, but unfortunately not the
CPU type. I'm not sure why it doesn't. Maybe readconfig can be taught
to do so?

> 

> The core numbers, is a new default. Server is smp in common sense,

> single core isn't like a server, so some even want to set to max

> capability as default, but this isn't good either I think, because

> gicv3 memory space in Qemu can even support more than 200 cores, that

> is too much.


Core numbers can be managed with config
[smp-opts]
  cpus = "4"

> (gicv2 currently support 8 cores, but there are issues to boot smp for

> both gicv2 and gicv3 now, it should be another thread)

> 

> >>

> >> This is the prototype, more features can be added in futrue.

> >>

> >> Purpose of this is to have a standard QEMU platform for Arm firmware

> >> developement etc. where the legacy machines cannot meets requirements.

> >>

> >> Arm Trusted Firmware and UEFI porting to this are done seperately.

> >>

> >> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

> >> ---

> > [...]

> >

> >> @@ -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 sbsa_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 },

> > does spec require support for 32-bit guest or is it only 64bit,

> > if the later I'd put it somewhere high where we can increase region

> > to terrabytes.

> >

> Spec only requires it to be compliant with armv8.

> Designed purpose of this is to run 64bit guests, because in practice

> an arm server is usually 64bit.


I agree with Igor that if we're going to add a new machine type that is
only for 64bit, then we shouldn't base our memory map on mach-virt's,
but rather create a new one taking advantage of a much larger address
space. Firmware would need to start taking the base of RAM from the
DTB, and the address of the DTB from x0, but it probably should anyway.

> 

> > another idea that was floating around (considering we don't care about legacy)

> > is to use flexible base address and tell firmware[*] via register where it's.

> > Then it would be able to support 32 guest with small amount of RAM

> > and 64 bit guests with huge amount of RAM using a single memory range.

> > (to keep memory management simple). It's also future friendly, as in case

> > if we need to move it we could do easily by changing base address for new machine

> > type only and firmware would automatically pick it up from register

> > (without all compat nightmare we have with 2 regions in pc/q35 machines).

> >

> > [*] When I've talked with Laszlo about it he said it was not easy to do in uefi

> > but still possible.

> Sounds not easy.

> I think the first step is to get this merged, and then we can add more

> features if necessary.

> I've already seen some points in trusted firmware and uefi need to be fixed.


It sounds to me like the current mach-virt machine type, with an SBSA
readconfig file would be sufficient for now, and that we should only
merge a new machine type when firmware supports a dynamic RAM base,
allowing the new machine type to have quite a different memory map.

> 

> >

> > same applies to GIG regions/mmio/ecam where we are twisting our hands when trying to

> > increase number of things.

> >

> > [...]

> >

>


Thanks,
drew
Alex Bennée June 28, 2018, 9:28 a.m. UTC | #7
Hongbo Zhang <hongbo.zhang@linaro.org> writes:

> On 27 June 2018 at 20:31, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:

>> * Hongbo Zhang (hongbo.zhang@linaro.org) wrote:

<snip>
> (Maybe we can change the kernel defconfig to e1000e, then we chose

> e1000e here too)

>

>> then to my mind it makes sense to use virtio-net-pci rather than

>> an e1000e.

>>

> Well, this is to emulate a 'true' hardware for firmware etc

> development, primary purpose it to emulate hardware functions, not for

> performance, so virtio is disabled, some firmware, boot loaders etc

> may not work with virtio either, if people want virtio, they can use

> the legacy 'virt' machine.


I don't think pci based virtio is any different from "real" hardware in
that sense. The bus supports device enumeration and detection via a
standard mechanism, the actual endpoint is merely a driver detail.

However I think we should be aiming for an absolute minimum that you
then configure on top off. I must admit I end up --nodefaults in my
command line more than I would like.

>

>> Dave

>>

>>> This is the prototype, more features can be added in futrue.

>>>

>>> Purpose of this is to have a standard QEMU platform for Arm firmware

>>> developement etc. where the legacy machines cannot meets requirements.

>>>

>>> Arm Trusted Firmware and UEFI porting to this are done seperately.

>>>

>>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

>>> ---

>>>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-

>>>  hw/arm/virt.c            | 196 ++++++++++++++++++++++++++++++++++++++++++++++-

>>>  include/hw/arm/virt.h    |   3 +

>>>  3 files changed, 254 insertions(+), 4 deletions(-)

>>>

>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c

>>> index 6ea47e2..60af414 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->sbsa) {

>>> +        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->sbsa) {

>>> +        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 742f68a..491f23b 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/cutils.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 sbsa_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 sbsa_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"),

>>> @@ -696,6 +743,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)

>>>  {

>>> @@ -1107,13 +1220,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->sbsa) {

>>> +                    nd->model = g_strdup("e1000");

>>> +                } else {

>>> +                    nd->model = g_strdup("virtio");

>>> +                }

>>>              }

>>>

>>>              pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);

>>>          }

>>>      }

>>>

>>> +    if (vms->sbsa) {

>>> +        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,

>>> @@ -1502,11 +1623,18 @@ static void machvirt_init(MachineState *machine)

>>>

>>>      create_gpio(vms, pic);

>>>

>>> +    if (vms->sbsa) {

>>> +        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->sbsa) {

>>> +        create_virtio_devices(vms, pic);

>>> +    }

>>>

>>>      vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);

>>>      rom_set_fw(vms->fw_cfg);

>>> @@ -1818,6 +1946,7 @@ static void virt_3_0_instance_init(Object *obj)

>>>

>>>      vms->memmap = a15memmap;

>>>      vms->irqmap = a15irqmap;

>>> +    vms->sbsa = false;

>>>  }

>>>

>>>  static void virt_machine_3_0_options(MachineClass *mc)

>>> @@ -1950,3 +2079,66 @@ static void virt_machine_2_6_options(MachineClass *mc)

>>>      vmc->no_pmu = true;

>>>  }

>>>  DEFINE_VIRT_MACHINE(2, 6)

>>> +

>>> +static void sbsa_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;

>>> +

>>> +    /* This can be changed to GICv3 when firmware is ready*/

>>> +    vms->gic_version = 2;

>>> +    object_property_add_str(obj, "gic-version", virt_get_gic_version,

>>> +                        virt_set_gic_version, NULL);

>>> +    object_property_set_description(obj, "gic-version",

>>> +                                    "Set GIC version. "

>>> +                                    "Valid values are 2, 3, host, max", NULL);

>>> +    vms->its = true;

>>> +

>>> +    vms->memmap = sbsa_memmap;

>>> +    vms->irqmap = sbsa_irqmap;

>>> +    vms->sbsa = true;

>>> +}

>>> +

>>> +static void sbsa_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 * G_BYTE;

>>> +    mc->default_cpus = 4;

>>> +    mc->desc = "QEMU 'SBSA' ARM Virtual Machine";

>>> +}

>>> +

>>> +static const TypeInfo sbsa_info = {

>>> +    .name          = MACHINE_TYPE_NAME("sbsa"),

>>> +    .parent        = TYPE_VIRT_MACHINE,

>>> +    .instance_init = sbsa_instance_init,

>>> +    .class_init    = sbsa_class_init,

>>> +};

>>> +

>>> +static void sbsa_machine_init(void)

>>> +{

>>> +    type_register_static(&sbsa_info);

>>> +}

>>> +

>>> +type_init(sbsa_machine_init);

>>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

>>> index 9a870cc..619473e 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 sbsa;

>>>  } VirtMachineState;

>>>

>>>  #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)

>>> --

>>> 2.7.4

>>>

>>>

>> --

>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



--
Alex Bennée
Hongbo Zhang June 28, 2018, 9:30 a.m. UTC | #8
On 28 June 2018 at 16:12, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> * Hongbo Zhang (hongbo.zhang@linaro.org) wrote:

>> On 27 June 2018 at 20:31, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:

>> > * Hongbo Zhang (hongbo.zhang@linaro.org) wrote:

>> >> This patch introduces a new Arm machine type 'SBSA' with features:

>> >>  - Based on legacy 'virt' machine type.

>> >>  - Newly designed memory map.

>> >>  - EL2 and EL3 are enabled 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.

>> >>  - E1000 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.

>> >

>> > I'm a bit confused; do you have real modern ARM hardware that has an

>> > e1000 on it?  If I understand correctly, e1000 is the old PCI version

>> > and the e1000e is at least the more modern PCIe version which makes

>> > more sense if you're building on PCIe.

>> > However, if you:

>> >   a) Don't have real hardware with the e1000 on

>> >   b) Do have PCIe

>> >

>> Thanks for review.

>>

>> Yes, e1000e is used on modern hardware PCIe, using e1000e can reflect

>> the real world.

>> I like e1000e too, but I checked q35, it still uses an e1000 attached

>> to pcie bus.

>

> I think we think of that choice on q35 as a mistake / or we didn't

> have e1000e at the time.  If you used an e1000e then people could

> actually buy an e1000e card to plug into the test system to compare.

>

Yes q35 is old, I just wanted some reference at the beginning.
I re-checked latest kernel just now, arm64 defconfig has already added
e1000e support, so considering use e1000e too in next iteration.

Thanks.

>> Another fact is, when compile the arm64 kernel by defconfig, the e1000

>> isn't selected by default, but e1000e is.

>> That is why I still chose e1000 here.

>> (Maybe we can change the kernel defconfig to e1000e, then we chose

>> e1000e here too)

>>

>> > then to my mind it makes sense to use virtio-net-pci rather than

>> > an e1000e.

>> >

>> Well, this is to emulate a 'true' hardware for firmware etc

>> development, primary purpose it to emulate hardware functions, not for

>> performance, so virtio is disabled, some firmware, boot loaders etc

>> may not work with virtio either, if people want virtio, they can use

>> the legacy 'virt' machine.

>

> The virtio-mmio that the 'virt' machine has is quite odd; however

> virtio-pci devices are much less special.

>

> Dave

>

>> > Dave

>> >

>> >> This is the prototype, more features can be added in futrue.

>> >>

>> >> Purpose of this is to have a standard QEMU platform for Arm firmware

>> >> developement etc. where the legacy machines cannot meets requirements.

>> >>

>> >> Arm Trusted Firmware and UEFI porting to this are done seperately.

>> >>

>> >> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

>> >> ---

>> >>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-

>> >>  hw/arm/virt.c            | 196 ++++++++++++++++++++++++++++++++++++++++++++++-

>> >>  include/hw/arm/virt.h    |   3 +

>> >>  3 files changed, 254 insertions(+), 4 deletions(-)

>> >>

>> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c

>> >> index 6ea47e2..60af414 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->sbsa) {

>> >> +        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->sbsa) {

>> >> +        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 742f68a..491f23b 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/cutils.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 sbsa_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 sbsa_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"),

>> >> @@ -696,6 +743,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)

>> >>  {

>> >> @@ -1107,13 +1220,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->sbsa) {

>> >> +                    nd->model = g_strdup("e1000");

>> >> +                } else {

>> >> +                    nd->model = g_strdup("virtio");

>> >> +                }

>> >>              }

>> >>

>> >>              pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);

>> >>          }

>> >>      }

>> >>

>> >> +    if (vms->sbsa) {

>> >> +        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,

>> >> @@ -1502,11 +1623,18 @@ static void machvirt_init(MachineState *machine)

>> >>

>> >>      create_gpio(vms, pic);

>> >>

>> >> +    if (vms->sbsa) {

>> >> +        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->sbsa) {

>> >> +        create_virtio_devices(vms, pic);

>> >> +    }

>> >>

>> >>      vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);

>> >>      rom_set_fw(vms->fw_cfg);

>> >> @@ -1818,6 +1946,7 @@ static void virt_3_0_instance_init(Object *obj)

>> >>

>> >>      vms->memmap = a15memmap;

>> >>      vms->irqmap = a15irqmap;

>> >> +    vms->sbsa = false;

>> >>  }

>> >>

>> >>  static void virt_machine_3_0_options(MachineClass *mc)

>> >> @@ -1950,3 +2079,66 @@ static void virt_machine_2_6_options(MachineClass *mc)

>> >>      vmc->no_pmu = true;

>> >>  }

>> >>  DEFINE_VIRT_MACHINE(2, 6)

>> >> +

>> >> +static void sbsa_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;

>> >> +

>> >> +    /* This can be changed to GICv3 when firmware is ready*/

>> >> +    vms->gic_version = 2;

>> >> +    object_property_add_str(obj, "gic-version", virt_get_gic_version,

>> >> +                        virt_set_gic_version, NULL);

>> >> +    object_property_set_description(obj, "gic-version",

>> >> +                                    "Set GIC version. "

>> >> +                                    "Valid values are 2, 3, host, max", NULL);

>> >> +    vms->its = true;

>> >> +

>> >> +    vms->memmap = sbsa_memmap;

>> >> +    vms->irqmap = sbsa_irqmap;

>> >> +    vms->sbsa = true;

>> >> +}

>> >> +

>> >> +static void sbsa_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 * G_BYTE;

>> >> +    mc->default_cpus = 4;

>> >> +    mc->desc = "QEMU 'SBSA' ARM Virtual Machine";

>> >> +}

>> >> +

>> >> +static const TypeInfo sbsa_info = {

>> >> +    .name          = MACHINE_TYPE_NAME("sbsa"),

>> >> +    .parent        = TYPE_VIRT_MACHINE,

>> >> +    .instance_init = sbsa_instance_init,

>> >> +    .class_init    = sbsa_class_init,

>> >> +};

>> >> +

>> >> +static void sbsa_machine_init(void)

>> >> +{

>> >> +    type_register_static(&sbsa_info);

>> >> +}

>> >> +

>> >> +type_init(sbsa_machine_init);

>> >> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

>> >> index 9a870cc..619473e 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 sbsa;

>> >>  } VirtMachineState;

>> >>

>> >>  #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)

>> >> --

>> >> 2.7.4

>> >>

>> >>

>> > --

>> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

> --

> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Laszlo Ersek June 28, 2018, 9:31 a.m. UTC | #9
On 06/27/18 12:13, Hongbo Zhang wrote:
> This patch introduces a new Arm machine type 'SBSA' with features:

>  - Based on legacy 'virt' machine type.


My understanding is that this new machine type serves a different use
case than the "virt" machine type; i.e. it is not primarily meant to run
on KVM and execute virtualization workloads. Instead, it is meant to
provide an environment as faithful as possible to physical hardware, for
supporting firmware and OS development for physical ARM64 machines.

In other words, this machine type is not a "goal" for running production
workloads (on KVM); instead it is a development and testbed "means",
where the end-goal is writing OS and firmware code for physical machines
that conform to the SBSA spec. Thus, the machine type is similar to e.g.
the "vexpress" machine types, except the emulated hardware platform is
"SBSA".

Can you please confirm that?

If that's the case, then please remove the word "legacy" from the commit
message (multiple instances). This machine type does not obsolete or
replace "virt", for "virt"'s stated use case; this machine type is for
an independent use case.

One consequence of this would be that performance isn't as important.

Another consequence is that paravirt devices are less desirable.

I have another question:

>  - Newly designed memory map.

>  - EL2 and EL3 are enabled 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.

>  - E1000 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.

> 

> This is the prototype, more features can be added in futrue.

> 

> Purpose of this is to have a standard QEMU platform for Arm firmware

> developement etc. where the legacy machines cannot meets requirements.

> 

> Arm Trusted Firmware and UEFI porting to this are done seperately.

> 

> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

> ---

>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-

>  hw/arm/virt.c            | 196 ++++++++++++++++++++++++++++++++++++++++++++++-

>  include/hw/arm/virt.h    |   3 +

>  3 files changed, 254 insertions(+), 4 deletions(-)

> 

> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c

> index 6ea47e2..60af414 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->sbsa) {

> +        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->sbsa) {

> +        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 742f68a..491f23b 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/cutils.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 sbsa_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 },


It's hard for any device to be *more* paravirtual than fw_cfg is --
given that (to my understanding) paravirtualization is an explicit
non-goal for this machine type, what do you need fw_cfg for? Is it an
oversight in the memory map, or is it an exception -- a limited paravirt
info channel -- because you need *some* way for passing information from
QEMU to a small subset of guest code?

Thanks
Laszlo

> +    [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 sbsa_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"),

> @@ -696,6 +743,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)

>  {

> @@ -1107,13 +1220,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->sbsa) {

> +                    nd->model = g_strdup("e1000");

> +                } else {

> +                    nd->model = g_strdup("virtio");

> +                }

>              }

>  

>              pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);

>          }

>      }

>  

> +    if (vms->sbsa) {

> +        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,

> @@ -1502,11 +1623,18 @@ static void machvirt_init(MachineState *machine)

>  

>      create_gpio(vms, pic);

>  

> +    if (vms->sbsa) {

> +        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->sbsa) {

> +        create_virtio_devices(vms, pic);

> +    }

>  

>      vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);

>      rom_set_fw(vms->fw_cfg);

> @@ -1818,6 +1946,7 @@ static void virt_3_0_instance_init(Object *obj)

>  

>      vms->memmap = a15memmap;

>      vms->irqmap = a15irqmap;

> +    vms->sbsa = false;

>  }

>  

>  static void virt_machine_3_0_options(MachineClass *mc)

> @@ -1950,3 +2079,66 @@ static void virt_machine_2_6_options(MachineClass *mc)

>      vmc->no_pmu = true;

>  }

>  DEFINE_VIRT_MACHINE(2, 6)

> +

> +static void sbsa_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;

> +

> +    /* This can be changed to GICv3 when firmware is ready*/

> +    vms->gic_version = 2;

> +    object_property_add_str(obj, "gic-version", virt_get_gic_version,

> +                        virt_set_gic_version, NULL);

> +    object_property_set_description(obj, "gic-version",

> +                                    "Set GIC version. "

> +                                    "Valid values are 2, 3, host, max", NULL);

> +    vms->its = true;

> +

> +    vms->memmap = sbsa_memmap;

> +    vms->irqmap = sbsa_irqmap;

> +    vms->sbsa = true;

> +}

> +

> +static void sbsa_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 * G_BYTE;

> +    mc->default_cpus = 4;

> +    mc->desc = "QEMU 'SBSA' ARM Virtual Machine";

> +}

> +

> +static const TypeInfo sbsa_info = {

> +    .name          = MACHINE_TYPE_NAME("sbsa"),

> +    .parent        = TYPE_VIRT_MACHINE,

> +    .instance_init = sbsa_instance_init,

> +    .class_init    = sbsa_class_init,

> +};

> +

> +static void sbsa_machine_init(void)

> +{

> +    type_register_static(&sbsa_info);

> +}

> +

> +type_init(sbsa_machine_init);

> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

> index 9a870cc..619473e 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 sbsa;

>  } VirtMachineState;

>  

>  #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)

>
Hongbo Zhang June 28, 2018, 10:13 a.m. UTC | #10
On 28 June 2018 at 17:04, Andrew Jones <drjones@redhat.com> wrote:
> On Thu, Jun 28, 2018 at 04:11:56PM +0800, Hongbo Zhang wrote:

>> On 27 June 2018 at 22:56, Igor Mammedov <imammedo@redhat.com> wrote:

>> > On Wed, 27 Jun 2018 18:13:08 +0800

>> > Hongbo Zhang <hongbo.zhang@linaro.org> wrote:

>> >

>> >> This patch introduces a new Arm machine type 'SBSA' with features:

>> >>  - Based on legacy 'virt' machine type.

>> >>  - Newly designed memory map.

>> >>  - EL2 and EL3 are enabled 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.

>> >>  - E1000 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.

>> > I'd drop all default (convenience) devices unless they are specified in SBSA,

>> > and make management explicitly provide needed devices on CLI.

>> >

>> Thanks for review.

>>

>> I mentioned these default values because they are different from the

>> 'virt' machine from which this sbsa machine derives. (sbsa has uart

>> too, it is same with 'virt' PL011).

>> Qemu implementation should has such default values, if the sbsa

>> machine does not offer them, they fallback to the parent machine's.

>

> QEMU's default devices is one of the problems with QEMU. Any serious

> use of QEMU must be done with '-nodefaults' and then explicit devices.

> To avoid requiring long command lines QEMU supports '-readconfig'. We

> already have two mach-virt configs that can be used as a basis for a

> new SBSA config. See docs/config/mach-virt-graphical.cfg and

> docs/config/mach-virt-serial.cfg.

>

When Linaro members discussed the target, some of them want this can
be used easily and simply "qemu -bios xxx -hda yyy", just like the
typical x86 machines, that is another reason I set default values.

>> The parent 'virt' machine's default cpu type is cortext-a15, that is

>> not proper for a armv8 server, and its default memory is 128M, that is

>> not enough to load uefi, even 512M cannot either, so I set the sbsa

>> defualt minimal memory to 1G.

>

> config files support setting a memory size, but unfortunately not the

> CPU type. I'm not sure why it doesn't. Maybe readconfig can be taught

> to do so?

>

>>

>> The core numbers, is a new default. Server is smp in common sense,

>> single core isn't like a server, so some even want to set to max

>> capability as default, but this isn't good either I think, because

>> gicv3 memory space in Qemu can even support more than 200 cores, that

>> is too much.

>

> Core numbers can be managed with config

> [smp-opts]

>   cpus = "4"

>

>> (gicv2 currently support 8 cores, but there are issues to boot smp for

>> both gicv2 and gicv3 now, it should be another thread)

>>

>> >>

>> >> This is the prototype, more features can be added in futrue.

>> >>

>> >> Purpose of this is to have a standard QEMU platform for Arm firmware

>> >> developement etc. where the legacy machines cannot meets requirements.

>> >>

>> >> Arm Trusted Firmware and UEFI porting to this are done seperately.

>> >>

>> >> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

>> >> ---

>> > [...]

>> >

>> >> @@ -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 sbsa_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 },

>> > does spec require support for 32-bit guest or is it only 64bit,

>> > if the later I'd put it somewhere high where we can increase region

>> > to terrabytes.

>> >

>> Spec only requires it to be compliant with armv8.

>> Designed purpose of this is to run 64bit guests, because in practice

>> an arm server is usually 64bit.

>

> I agree with Igor that if we're going to add a new machine type that is

> only for 64bit, then we shouldn't base our memory map on mach-virt's,

> but rather create a new one taking advantage of a much larger address

> space. Firmware would need to start taking the base of RAM from the

> DTB, and the address of the DTB from x0, but it probably should anyway.

>

>>

>> > another idea that was floating around (considering we don't care about legacy)

>> > is to use flexible base address and tell firmware[*] via register where it's.

>> > Then it would be able to support 32 guest with small amount of RAM

>> > and 64 bit guests with huge amount of RAM using a single memory range.

>> > (to keep memory management simple). It's also future friendly, as in case

>> > if we need to move it we could do easily by changing base address for new machine

>> > type only and firmware would automatically pick it up from register

>> > (without all compat nightmare we have with 2 regions in pc/q35 machines).

>> >

>> > [*] When I've talked with Laszlo about it he said it was not easy to do in uefi

>> > but still possible.

>> Sounds not easy.

>> I think the first step is to get this merged, and then we can add more

>> features if necessary.

>> I've already seen some points in trusted firmware and uefi need to be fixed.

>

> It sounds to me like the current mach-virt machine type, with an SBSA

> readconfig file would be sufficient for now, and that we should only

> merge a new machine type when firmware supports a dynamic RAM base,

> allowing the new machine type to have quite a different memory map.

>

Had a quick look at readconfig, it seems it cannot satisfy the SBSA design.

The SBSA has EHCI and AHCI on the top level, eg so called system bus
in the Arm system, that is the case in real hardware.
If we use readconfig, they should appear at PCIE bus, that is not the
case we want.
And when people start to use this, I think more features will be added in.

"firmware supports a dynamic RAM base", -- this need to be done, but
other porting on this platform have been done, initial idea is to have
a SBSA prototype first, and firmware later, and then further usage,
such as CCIX development, can be done on this platform at last.

>>

>> >

>> > same applies to GIG regions/mmio/ecam where we are twisting our hands when trying to

>> > increase number of things.

>> >

>> > [...]

>> >

>>

>

> Thanks,

> drew
Daniel P. Berrangé June 28, 2018, 10:22 a.m. UTC | #11
On Wed, Jun 27, 2018 at 01:31:17PM +0100, Dr. David Alan Gilbert wrote:
> * Hongbo Zhang (hongbo.zhang@linaro.org) wrote:

> > This patch introduces a new Arm machine type 'SBSA' with features:

> >  - Based on legacy 'virt' machine type.


The 'virt' machine type is absolutely *not* legacy. It is the preferred
modern, legacy-free machine type when running virtual machines. Since
aarch64 is greenfield, there is no compelling reason to emulate a
specific physical machine for VMs, hence the 'virt' machine type.

> >  - Newly designed memory map.

> >  - EL2 and EL3 are enabled 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.

> >  - E1000 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.

> 

> I'm a bit confused; do you have real modern ARM hardware that has an

> e1000 on it?  If I understand correctly, e1000 is the old PCI version

> and the e1000e is at least the more modern PCIe version which makes

> more sense if you're building on PCIe.


Indeed, it makes little sense to default to e1000 if the goal is to make
it legacy-free

> However, if you:

>   a) Don't have real hardware with the e1000 on

>   b) Do have PCIe

> 

> then to my mind it makes sense to use virtio-net-pci rather than

> an e1000e.


If it does down the virtio-* route though, I fail to see any point in
adding the new machine type at all -  'virt' machine type is intended
to be used for guests where everything is paravirtualized, ignoring
physical hardware constraints.

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 :|
Hongbo Zhang June 28, 2018, 10:23 a.m. UTC | #12
On 28 June 2018 at 17:28, Alex Bennée <alex.bennee@linaro.org> wrote:
>

> Hongbo Zhang <hongbo.zhang@linaro.org> writes:

>

>> On 27 June 2018 at 20:31, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:

>>> * Hongbo Zhang (hongbo.zhang@linaro.org) wrote:

> <snip>

>> (Maybe we can change the kernel defconfig to e1000e, then we chose

>> e1000e here too)

>>

>>> then to my mind it makes sense to use virtio-net-pci rather than

>>> an e1000e.

>>>

>> Well, this is to emulate a 'true' hardware for firmware etc

>> development, primary purpose it to emulate hardware functions, not for

>> performance, so virtio is disabled, some firmware, boot loaders etc

>> may not work with virtio either, if people want virtio, they can use

>> the legacy 'virt' machine.

>

> I don't think pci based virtio is any different from "real" hardware in

> that sense. The bus supports device enumeration and detection via a

> standard mechanism, the actual endpoint is merely a driver detail.

>

Difference is from the perspective of hardware structure, in real Arm
hardware, AHCI and EHCI are system memory mapped, not PCIE devices.

> However I think we should be aiming for an absolute minimum that you

> then configure on top off. I must admit I end up --nodefaults in my

> command line more than I would like.

>

>>

>>> Dave

>>>

>>>> This is the prototype, more features can be added in futrue.

>>>>

>>>> Purpose of this is to have a standard QEMU platform for Arm firmware

>>>> developement etc. where the legacy machines cannot meets requirements.

>>>>

>>>> Arm Trusted Firmware and UEFI porting to this are done seperately.

>>>>

>>>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

>>>> ---

>>>>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-

>>>>  hw/arm/virt.c            | 196 ++++++++++++++++++++++++++++++++++++++++++++++-

>>>>  include/hw/arm/virt.h    |   3 +

>>>>  3 files changed, 254 insertions(+), 4 deletions(-)

>>>>

>>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c

>>>> index 6ea47e2..60af414 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->sbsa) {

>>>> +        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->sbsa) {

>>>> +        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 742f68a..491f23b 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/cutils.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 sbsa_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 sbsa_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"),

>>>> @@ -696,6 +743,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)

>>>>  {

>>>> @@ -1107,13 +1220,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->sbsa) {

>>>> +                    nd->model = g_strdup("e1000");

>>>> +                } else {

>>>> +                    nd->model = g_strdup("virtio");

>>>> +                }

>>>>              }

>>>>

>>>>              pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);

>>>>          }

>>>>      }

>>>>

>>>> +    if (vms->sbsa) {

>>>> +        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,

>>>> @@ -1502,11 +1623,18 @@ static void machvirt_init(MachineState *machine)

>>>>

>>>>      create_gpio(vms, pic);

>>>>

>>>> +    if (vms->sbsa) {

>>>> +        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->sbsa) {

>>>> +        create_virtio_devices(vms, pic);

>>>> +    }

>>>>

>>>>      vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);

>>>>      rom_set_fw(vms->fw_cfg);

>>>> @@ -1818,6 +1946,7 @@ static void virt_3_0_instance_init(Object *obj)

>>>>

>>>>      vms->memmap = a15memmap;

>>>>      vms->irqmap = a15irqmap;

>>>> +    vms->sbsa = false;

>>>>  }

>>>>

>>>>  static void virt_machine_3_0_options(MachineClass *mc)

>>>> @@ -1950,3 +2079,66 @@ static void virt_machine_2_6_options(MachineClass *mc)

>>>>      vmc->no_pmu = true;

>>>>  }

>>>>  DEFINE_VIRT_MACHINE(2, 6)

>>>> +

>>>> +static void sbsa_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;

>>>> +

>>>> +    /* This can be changed to GICv3 when firmware is ready*/

>>>> +    vms->gic_version = 2;

>>>> +    object_property_add_str(obj, "gic-version", virt_get_gic_version,

>>>> +                        virt_set_gic_version, NULL);

>>>> +    object_property_set_description(obj, "gic-version",

>>>> +                                    "Set GIC version. "

>>>> +                                    "Valid values are 2, 3, host, max", NULL);

>>>> +    vms->its = true;

>>>> +

>>>> +    vms->memmap = sbsa_memmap;

>>>> +    vms->irqmap = sbsa_irqmap;

>>>> +    vms->sbsa = true;

>>>> +}

>>>> +

>>>> +static void sbsa_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 * G_BYTE;

>>>> +    mc->default_cpus = 4;

>>>> +    mc->desc = "QEMU 'SBSA' ARM Virtual Machine";

>>>> +}

>>>> +

>>>> +static const TypeInfo sbsa_info = {

>>>> +    .name          = MACHINE_TYPE_NAME("sbsa"),

>>>> +    .parent        = TYPE_VIRT_MACHINE,

>>>> +    .instance_init = sbsa_instance_init,

>>>> +    .class_init    = sbsa_class_init,

>>>> +};

>>>> +

>>>> +static void sbsa_machine_init(void)

>>>> +{

>>>> +    type_register_static(&sbsa_info);

>>>> +}

>>>> +

>>>> +type_init(sbsa_machine_init);

>>>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

>>>> index 9a870cc..619473e 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 sbsa;

>>>>  } VirtMachineState;

>>>>

>>>>  #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)

>>>> --

>>>> 2.7.4

>>>>

>>>>

>>> --

>>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

>

>

> --

> Alex Bennée
Hongbo Zhang June 28, 2018, 10:38 a.m. UTC | #13
On 28 June 2018 at 17:31, Laszlo Ersek <lersek@redhat.com> wrote:
> On 06/27/18 12:13, Hongbo Zhang wrote:

>> This patch introduces a new Arm machine type 'SBSA' with features:

>>  - Based on legacy 'virt' machine type.

>

> My understanding is that this new machine type serves a different use

> case than the "virt" machine type; i.e. it is not primarily meant to run

> on KVM and execute virtualization workloads. Instead, it is meant to

> provide an environment as faithful as possible to physical hardware, for

> supporting firmware and OS development for physical ARM64 machines.

>

> In other words, this machine type is not a "goal" for running production

> workloads (on KVM); instead it is a development and testbed "means",

> where the end-goal is writing OS and firmware code for physical machines

> that conform to the SBSA spec. Thus, the machine type is similar to e.g.

> the "vexpress" machine types, except the emulated hardware platform is

> "SBSA".

>

> Can you please confirm that?

>

Yes, yes, yes.
You describe much better than me, thanks.

> If that's the case, then please remove the word "legacy" from the commit

> message (multiple instances). This machine type does not obsolete or

> replace "virt", for "virt"'s stated use case; this machine type is for

> an independent use case.

>

Good suggestion.
I didn't want make 'virt' obsolete at all, English isn't my mother
language, this should be my misunderstanding of term 'legacy' some
how, I just meant SBSA based on 'virt' :)

> One consequence of this would be that performance isn't as important.

>

> Another consequence is that paravirt devices are less desirable.

>

> I have another question:

>

>>  - Newly designed memory map.

>>  - EL2 and EL3 are enabled 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.

>>  - E1000 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.

>>

>> This is the prototype, more features can be added in futrue.

>>

>> Purpose of this is to have a standard QEMU platform for Arm firmware

>> developement etc. where the legacy machines cannot meets requirements.

>>

>> Arm Trusted Firmware and UEFI porting to this are done seperately.

>>

>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

>> ---

>>  hw/arm/virt-acpi-build.c |  59 +++++++++++++-

>>  hw/arm/virt.c            | 196 ++++++++++++++++++++++++++++++++++++++++++++++-

>>  include/hw/arm/virt.h    |   3 +

>>  3 files changed, 254 insertions(+), 4 deletions(-)

>>

>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c

>> index 6ea47e2..60af414 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->sbsa) {

>> +        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->sbsa) {

>> +        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 742f68a..491f23b 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/cutils.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 sbsa_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 },

>

> It's hard for any device to be *more* paravirtual than fw_cfg is --

> given that (to my understanding) paravirtualization is an explicit

> non-goal for this machine type, what do you need fw_cfg for? Is it an

> oversight in the memory map, or is it an exception -- a limited paravirt

> info channel -- because you need *some* way for passing information from

> QEMU to a small subset of guest code?

>

Well, when design discussed, fw_cfg wasn't mentioned at all, so I
didn't pay much attention to it.
Will double check, I think this should be removed.

Thanks for review.

> Thanks

> Laszlo

>

>> +    [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 sbsa_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"),

>> @@ -696,6 +743,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)

>>  {

>> @@ -1107,13 +1220,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->sbsa) {

>> +                    nd->model = g_strdup("e1000");

>> +                } else {

>> +                    nd->model = g_strdup("virtio");

>> +                }

>>              }

>>

>>              pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);

>>          }

>>      }

>>

>> +    if (vms->sbsa) {

>> +        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,

>> @@ -1502,11 +1623,18 @@ static void machvirt_init(MachineState *machine)

>>

>>      create_gpio(vms, pic);

>>

>> +    if (vms->sbsa) {

>> +        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->sbsa) {

>> +        create_virtio_devices(vms, pic);

>> +    }

>>

>>      vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);

>>      rom_set_fw(vms->fw_cfg);

>> @@ -1818,6 +1946,7 @@ static void virt_3_0_instance_init(Object *obj)

>>

>>      vms->memmap = a15memmap;

>>      vms->irqmap = a15irqmap;

>> +    vms->sbsa = false;

>>  }

>>

>>  static void virt_machine_3_0_options(MachineClass *mc)

>> @@ -1950,3 +2079,66 @@ static void virt_machine_2_6_options(MachineClass *mc)

>>      vmc->no_pmu = true;

>>  }

>>  DEFINE_VIRT_MACHINE(2, 6)

>> +

>> +static void sbsa_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;

>> +

>> +    /* This can be changed to GICv3 when firmware is ready*/

>> +    vms->gic_version = 2;

>> +    object_property_add_str(obj, "gic-version", virt_get_gic_version,

>> +                        virt_set_gic_version, NULL);

>> +    object_property_set_description(obj, "gic-version",

>> +                                    "Set GIC version. "

>> +                                    "Valid values are 2, 3, host, max", NULL);

>> +    vms->its = true;

>> +

>> +    vms->memmap = sbsa_memmap;

>> +    vms->irqmap = sbsa_irqmap;

>> +    vms->sbsa = true;

>> +}

>> +

>> +static void sbsa_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 * G_BYTE;

>> +    mc->default_cpus = 4;

>> +    mc->desc = "QEMU 'SBSA' ARM Virtual Machine";

>> +}

>> +

>> +static const TypeInfo sbsa_info = {

>> +    .name          = MACHINE_TYPE_NAME("sbsa"),

>> +    .parent        = TYPE_VIRT_MACHINE,

>> +    .instance_init = sbsa_instance_init,

>> +    .class_init    = sbsa_class_init,

>> +};

>> +

>> +static void sbsa_machine_init(void)

>> +{

>> +    type_register_static(&sbsa_info);

>> +}

>> +

>> +type_init(sbsa_machine_init);

>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h

>> index 9a870cc..619473e 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 sbsa;

>>  } VirtMachineState;

>>

>>  #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)

>>

>
Hongbo Zhang June 28, 2018, 10:47 a.m. UTC | #14
On 28 June 2018 at 18:22, Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Wed, Jun 27, 2018 at 01:31:17PM +0100, Dr. David Alan Gilbert wrote:

>> * Hongbo Zhang (hongbo.zhang@linaro.org) wrote:

>> > This patch introduces a new Arm machine type 'SBSA' with features:

>> >  - Based on legacy 'virt' machine type.

>

> The 'virt' machine type is absolutely *not* legacy. It is the preferred

> modern, legacy-free machine type when running virtual machines. Since

> aarch64 is greenfield, there is no compelling reason to emulate a

> specific physical machine for VMs, hence the 'virt' machine type.

>

>> >  - Newly designed memory map.

>> >  - EL2 and EL3 are enabled 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.

>> >  - E1000 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.

>>

>> I'm a bit confused; do you have real modern ARM hardware that has an

>> e1000 on it?  If I understand correctly, e1000 is the old PCI version

>> and the e1000e is at least the more modern PCIe version which makes

>> more sense if you're building on PCIe.

>

> Indeed, it makes little sense to default to e1000 if the goal is to make

> it legacy-free

>

>> However, if you:

>>   a) Don't have real hardware with the e1000 on

>>   b) Do have PCIe

>>

>> then to my mind it makes sense to use virtio-net-pci rather than

>> an e1000e.

>

> If it does down the virtio-* route though, I fail to see any point in

> adding the new machine type at all -  'virt' machine type is intended

> to be used for guests where everything is paravirtualized, ignoring

> physical hardware constraints.

>

Your comment comes earlier than I replied to Laszlo Ersek:

I said "yes yes yes" to him due to he describes the purpose so well,
even better than me.

And the I had some misunderstanding of term 'legacy' too, as I
explained in that reply.

> 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 :|
Andrew Jones June 28, 2018, 11:36 a.m. UTC | #15
On Thu, Jun 28, 2018 at 06:13:28PM +0800, Hongbo Zhang wrote:
> On 28 June 2018 at 17:04, Andrew Jones <drjones@redhat.com> wrote:

> > On Thu, Jun 28, 2018 at 04:11:56PM +0800, Hongbo Zhang wrote:

> >> On 27 June 2018 at 22:56, Igor Mammedov <imammedo@redhat.com> wrote:

> >> > On Wed, 27 Jun 2018 18:13:08 +0800

> >> > Hongbo Zhang <hongbo.zhang@linaro.org> wrote:

> >> >

> >> >> This patch introduces a new Arm machine type 'SBSA' with features:

> >> >>  - Based on legacy 'virt' machine type.

> >> >>  - Newly designed memory map.

> >> >>  - EL2 and EL3 are enabled 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.

> >> >>  - E1000 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.

> >> > I'd drop all default (convenience) devices unless they are specified in SBSA,

> >> > and make management explicitly provide needed devices on CLI.

> >> >

> >> Thanks for review.

> >>

> >> I mentioned these default values because they are different from the

> >> 'virt' machine from which this sbsa machine derives. (sbsa has uart

> >> too, it is same with 'virt' PL011).

> >> Qemu implementation should has such default values, if the sbsa

> >> machine does not offer them, they fallback to the parent machine's.

> >

> > QEMU's default devices is one of the problems with QEMU. Any serious

> > use of QEMU must be done with '-nodefaults' and then explicit devices.

> > To avoid requiring long command lines QEMU supports '-readconfig'. We

> > already have two mach-virt configs that can be used as a basis for a

> > new SBSA config. See docs/config/mach-virt-graphical.cfg and

> > docs/config/mach-virt-serial.cfg.

> >

> When Linaro members discussed the target, some of them want this can

> be used easily and simply "qemu -bios xxx -hda yyy", just like the

> typical x86 machines, that is another reason I set default values.

> 

> >> The parent 'virt' machine's default cpu type is cortext-a15, that is

> >> not proper for a armv8 server, and its default memory is 128M, that is

> >> not enough to load uefi, even 512M cannot either, so I set the sbsa

> >> defualt minimal memory to 1G.

> >

> > config files support setting a memory size, but unfortunately not the

> > CPU type. I'm not sure why it doesn't. Maybe readconfig can be taught

> > to do so?

> >

> >>

> >> The core numbers, is a new default. Server is smp in common sense,

> >> single core isn't like a server, so some even want to set to max

> >> capability as default, but this isn't good either I think, because

> >> gicv3 memory space in Qemu can even support more than 200 cores, that

> >> is too much.

> >

> > Core numbers can be managed with config

> > [smp-opts]

> >   cpus = "4"

> >

> >> (gicv2 currently support 8 cores, but there are issues to boot smp for

> >> both gicv2 and gicv3 now, it should be another thread)

> >>

> >> >>

> >> >> This is the prototype, more features can be added in futrue.

> >> >>

> >> >> Purpose of this is to have a standard QEMU platform for Arm firmware

> >> >> developement etc. where the legacy machines cannot meets requirements.

> >> >>

> >> >> Arm Trusted Firmware and UEFI porting to this are done seperately.

> >> >>

> >> >> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

> >> >> ---

> >> > [...]

> >> >

> >> >> @@ -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 sbsa_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 },

> >> > does spec require support for 32-bit guest or is it only 64bit,

> >> > if the later I'd put it somewhere high where we can increase region

> >> > to terrabytes.

> >> >

> >> Spec only requires it to be compliant with armv8.

> >> Designed purpose of this is to run 64bit guests, because in practice

> >> an arm server is usually 64bit.

> >

> > I agree with Igor that if we're going to add a new machine type that is

> > only for 64bit, then we shouldn't base our memory map on mach-virt's,

> > but rather create a new one taking advantage of a much larger address

> > space. Firmware would need to start taking the base of RAM from the

> > DTB, and the address of the DTB from x0, but it probably should anyway.

> >

> >>

> >> > another idea that was floating around (considering we don't care about legacy)

> >> > is to use flexible base address and tell firmware[*] via register where it's.

> >> > Then it would be able to support 32 guest with small amount of RAM

> >> > and 64 bit guests with huge amount of RAM using a single memory range.

> >> > (to keep memory management simple). It's also future friendly, as in case

> >> > if we need to move it we could do easily by changing base address for new machine

> >> > type only and firmware would automatically pick it up from register

> >> > (without all compat nightmare we have with 2 regions in pc/q35 machines).

> >> >

> >> > [*] When I've talked with Laszlo about it he said it was not easy to do in uefi

> >> > but still possible.

> >> Sounds not easy.

> >> I think the first step is to get this merged, and then we can add more

> >> features if necessary.

> >> I've already seen some points in trusted firmware and uefi need to be fixed.

> >

> > It sounds to me like the current mach-virt machine type, with an SBSA

> > readconfig file would be sufficient for now, and that we should only

> > merge a new machine type when firmware supports a dynamic RAM base,

> > allowing the new machine type to have quite a different memory map.

> >

> Had a quick look at readconfig, it seems it cannot satisfy the SBSA design.

> 

> The SBSA has EHCI and AHCI on the top level, eg so called system bus

> in the Arm system, that is the case in real hardware.

> If we use readconfig, they should appear at PCIE bus, that is not the

> case we want.


SBSA doesn't even require EHCI or AHCI, afaict, so the decision to have
them on the system bus must have a different rationale. Can you please
share it? Also, when making that decision were other operating systems
(Windows) considered?

Anyway, I believe with some work you should be able to modify the platform
bus code to allow them to be specified on the command line and still show
up on the system bus.

> And when people start to use this, I think more features will be added in.

> 

> "firmware supports a dynamic RAM base", -- this need to be done, but

> other porting on this platform have been done, initial idea is to have

> a SBSA prototype first, and firmware later, and then further usage,

> such as CCIX development, can be done on this platform at last.


The firmware has no reason to change if the base of RAM matches mach-virt.
To me, it seems this machine model and the firmware change should be done
together.

Thanks,
drew

> 

> >>

> >> >

> >> > same applies to GIG regions/mmio/ecam where we are twisting our hands when trying to

> >> > increase number of things.

> >> >

> >> > [...]

> >> >

> >>

> >

> > Thanks,

> > drew

>
Hongbo Zhang June 29, 2018, 3:17 a.m. UTC | #16
On 28 June 2018 at 19:36, Andrew Jones <drjones@redhat.com> wrote:
> On Thu, Jun 28, 2018 at 06:13:28PM +0800, Hongbo Zhang wrote:

>> On 28 June 2018 at 17:04, Andrew Jones <drjones@redhat.com> wrote:

>> > On Thu, Jun 28, 2018 at 04:11:56PM +0800, Hongbo Zhang wrote:

>> >> On 27 June 2018 at 22:56, Igor Mammedov <imammedo@redhat.com> wrote:

>> >> > On Wed, 27 Jun 2018 18:13:08 +0800

>> >> > Hongbo Zhang <hongbo.zhang@linaro.org> wrote:

>> >> >

>> >> >> This patch introduces a new Arm machine type 'SBSA' with features:

>> >> >>  - Based on legacy 'virt' machine type.

>> >> >>  - Newly designed memory map.

>> >> >>  - EL2 and EL3 are enabled 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.

>> >> >>  - E1000 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.

>> >> > I'd drop all default (convenience) devices unless they are specified in SBSA,

>> >> > and make management explicitly provide needed devices on CLI.

>> >> >

>> >> Thanks for review.

>> >>

>> >> I mentioned these default values because they are different from the

>> >> 'virt' machine from which this sbsa machine derives. (sbsa has uart

>> >> too, it is same with 'virt' PL011).

>> >> Qemu implementation should has such default values, if the sbsa

>> >> machine does not offer them, they fallback to the parent machine's.

>> >

>> > QEMU's default devices is one of the problems with QEMU. Any serious

>> > use of QEMU must be done with '-nodefaults' and then explicit devices.

>> > To avoid requiring long command lines QEMU supports '-readconfig'. We

>> > already have two mach-virt configs that can be used as a basis for a

>> > new SBSA config. See docs/config/mach-virt-graphical.cfg and

>> > docs/config/mach-virt-serial.cfg.

>> >

>> When Linaro members discussed the target, some of them want this can

>> be used easily and simply "qemu -bios xxx -hda yyy", just like the

>> typical x86 machines, that is another reason I set default values.

>>

>> >> The parent 'virt' machine's default cpu type is cortext-a15, that is

>> >> not proper for a armv8 server, and its default memory is 128M, that is

>> >> not enough to load uefi, even 512M cannot either, so I set the sbsa

>> >> defualt minimal memory to 1G.

>> >

>> > config files support setting a memory size, but unfortunately not the

>> > CPU type. I'm not sure why it doesn't. Maybe readconfig can be taught

>> > to do so?

>> >

>> >>

>> >> The core numbers, is a new default. Server is smp in common sense,

>> >> single core isn't like a server, so some even want to set to max

>> >> capability as default, but this isn't good either I think, because

>> >> gicv3 memory space in Qemu can even support more than 200 cores, that

>> >> is too much.

>> >

>> > Core numbers can be managed with config

>> > [smp-opts]

>> >   cpus = "4"

>> >

>> >> (gicv2 currently support 8 cores, but there are issues to boot smp for

>> >> both gicv2 and gicv3 now, it should be another thread)

>> >>

>> >> >>

>> >> >> This is the prototype, more features can be added in futrue.

>> >> >>

>> >> >> Purpose of this is to have a standard QEMU platform for Arm firmware

>> >> >> developement etc. where the legacy machines cannot meets requirements.

>> >> >>

>> >> >> Arm Trusted Firmware and UEFI porting to this are done seperately.

>> >> >>

>> >> >> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>

>> >> >> ---

>> >> > [...]

>> >> >

>> >> >> @@ -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 sbsa_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 },

>> >> > does spec require support for 32-bit guest or is it only 64bit,

>> >> > if the later I'd put it somewhere high where we can increase region

>> >> > to terrabytes.

>> >> >

>> >> Spec only requires it to be compliant with armv8.

>> >> Designed purpose of this is to run 64bit guests, because in practice

>> >> an arm server is usually 64bit.

>> >

>> > I agree with Igor that if we're going to add a new machine type that is

>> > only for 64bit, then we shouldn't base our memory map on mach-virt's,

>> > but rather create a new one taking advantage of a much larger address

>> > space. Firmware would need to start taking the base of RAM from the

>> > DTB, and the address of the DTB from x0, but it probably should anyway.

>> >

>> >>

>> >> > another idea that was floating around (considering we don't care about legacy)

>> >> > is to use flexible base address and tell firmware[*] via register where it's.

>> >> > Then it would be able to support 32 guest with small amount of RAM

>> >> > and 64 bit guests with huge amount of RAM using a single memory range.

>> >> > (to keep memory management simple). It's also future friendly, as in case

>> >> > if we need to move it we could do easily by changing base address for new machine

>> >> > type only and firmware would automatically pick it up from register

>> >> > (without all compat nightmare we have with 2 regions in pc/q35 machines).

>> >> >

>> >> > [*] When I've talked with Laszlo about it he said it was not easy to do in uefi

>> >> > but still possible.

>> >> Sounds not easy.

>> >> I think the first step is to get this merged, and then we can add more

>> >> features if necessary.

>> >> I've already seen some points in trusted firmware and uefi need to be fixed.

>> >

>> > It sounds to me like the current mach-virt machine type, with an SBSA

>> > readconfig file would be sufficient for now, and that we should only

>> > merge a new machine type when firmware supports a dynamic RAM base,

>> > allowing the new machine type to have quite a different memory map.

>> >

>> Had a quick look at readconfig, it seems it cannot satisfy the SBSA design.

>>

>> The SBSA has EHCI and AHCI on the top level, eg so called system bus

>> in the Arm system, that is the case in real hardware.

>> If we use readconfig, they should appear at PCIE bus, that is not the

>> case we want.

>

> SBSA doesn't even require EHCI or AHCI, afaict, so the decision to have

> them on the system bus must have a different rationale. Can you please

> share it? Also, when making that decision were other operating systems

> (Windows) considered?

>

Well, during the development stage, two names were used: Enterprise
and SBSA. I was hesitating to decide which one is better.
Now I know the Enterprise is better, because what we want is a
standard armv8 platform as a base of firmware and software
development. For armv7, there is vexpress acts as such role, but for
armv8, there is no such platform, 'virt' is really a virtual one, no
real hard ware is like that, we want a qemu platform similar as real
armv8 hardware, we want fw/sw developed on this platform can be easily
run on real hardware when it is ready, we want can be used as simple
as typical x86 "qemu -bios -hda" if no specific requirements needed.
But when term SBSA is used here, people may pay much attention to
details of SBSA spec itself, that is not we want, we may want some
part of it, but not fully compliant with it. SBSA spec described much
requirements on CPU cores, and the armv8 emulation in Qemu is already
there, my Enterprise/SBSA platform doesn't touch it at all.
So I am considering giving up name SBSA, reuse name Enterprise.

As to OS, Windows isn't considered yet. I've tested Debian official arm release.

> Anyway, I believe with some work you should be able to modify the platform

> bus code to allow them to be specified on the command line and still show

> up on the system bus.

>

>> And when people start to use this, I think more features will be added in.

>>

>> "firmware supports a dynamic RAM base", -- this need to be done, but

>> other porting on this platform have been done, initial idea is to have

>> a SBSA prototype first, and firmware later, and then further usage,

>> such as CCIX development, can be done on this platform at last.

>

> The firmware has no reason to change if the base of RAM matches mach-virt.

> To me, it seems this machine model and the firmware change should be done

> together.

>

Oops, it was my misunderstand, I though "firmware supports a dynamic
RAM base" is new feature and requirement, it is not, we've already use
it.

Firmware need some changes, for new peripherals, new memory map, and
some SBSA requirement for example GIVv3(after level2)
Firmware is ready, will be upstreamed soon:
https://git.linaro.org/people/hongbo.zhang/atf-sbsa.git/log/?h=sbsa_gicv3
https://git.linaro.org/people/radoslaw.biernacki/atf.git/log/?h=sbsa12
https://git.linaro.org/people/radoslaw.biernacki/edk2.git/


> Thanks,

> drew

>

>>

>> >>

>> >> >

>> >> > same applies to GIG regions/mmio/ecam where we are twisting our hands when trying to

>> >> > increase number of things.

>> >> >

>> >> > [...]

>> >> >

>> >>

>> >

>> > Thanks,

>> > drew

>>
diff mbox series

Patch

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 6ea47e2..60af414 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->sbsa) {
+        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->sbsa) {
+        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 742f68a..491f23b 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/cutils.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 sbsa_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 sbsa_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"),
@@ -696,6 +743,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)
 {
@@ -1107,13 +1220,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->sbsa) {
+                    nd->model = g_strdup("e1000");
+                } else {
+                    nd->model = g_strdup("virtio");
+                }
             }
 
             pci_nic_init_nofail(nd, pci->bus, nd->model, NULL);
         }
     }
 
+    if (vms->sbsa) {
+        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,
@@ -1502,11 +1623,18 @@  static void machvirt_init(MachineState *machine)
 
     create_gpio(vms, pic);
 
+    if (vms->sbsa) {
+        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->sbsa) {
+        create_virtio_devices(vms, pic);
+    }
 
     vms->fw_cfg = create_fw_cfg(vms, &address_space_memory);
     rom_set_fw(vms->fw_cfg);
@@ -1818,6 +1946,7 @@  static void virt_3_0_instance_init(Object *obj)
 
     vms->memmap = a15memmap;
     vms->irqmap = a15irqmap;
+    vms->sbsa = false;
 }
 
 static void virt_machine_3_0_options(MachineClass *mc)
@@ -1950,3 +2079,66 @@  static void virt_machine_2_6_options(MachineClass *mc)
     vmc->no_pmu = true;
 }
 DEFINE_VIRT_MACHINE(2, 6)
+
+static void sbsa_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;
+
+    /* This can be changed to GICv3 when firmware is ready*/
+    vms->gic_version = 2;
+    object_property_add_str(obj, "gic-version", virt_get_gic_version,
+                        virt_set_gic_version, NULL);
+    object_property_set_description(obj, "gic-version",
+                                    "Set GIC version. "
+                                    "Valid values are 2, 3, host, max", NULL);
+    vms->its = true;
+
+    vms->memmap = sbsa_memmap;
+    vms->irqmap = sbsa_irqmap;
+    vms->sbsa = true;
+}
+
+static void sbsa_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 * G_BYTE;
+    mc->default_cpus = 4;
+    mc->desc = "QEMU 'SBSA' ARM Virtual Machine";
+}
+
+static const TypeInfo sbsa_info = {
+    .name          = MACHINE_TYPE_NAME("sbsa"),
+    .parent        = TYPE_VIRT_MACHINE,
+    .instance_init = sbsa_instance_init,
+    .class_init    = sbsa_class_init,
+};
+
+static void sbsa_machine_init(void)
+{
+    type_register_static(&sbsa_info);
+}
+
+type_init(sbsa_machine_init);
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 9a870cc..619473e 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 sbsa;
 } VirtMachineState;
 
 #define VIRT_ECAM_ID(high) (high ? VIRT_PCIE_ECAM_HIGH : VIRT_PCIE_ECAM)