Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

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

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

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

> Peter, Ard, do we need more considerations here?
> 
>> So you are creating 2 identical flashes o

Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

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

> I wanted to ask "does it has to be CFI01?" because this device model is
> in bad shape, but I guess I answered myself looking at the EDK2 platform
> code:
>
> - P30_CFI_ADDR_VENDOR_ID is not used
> - NorFlashDxe::NorFlashReadCfiData() is not implemented
> - All co

Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

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

Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

2019-06-03 Thread Philippe Mathieu-Daudé
Hi Hongbo, Ard.

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

Please use TYPE_PFLASH_CFI01 instead of "cfi.pflash01".

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

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

> +SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +const uint64_t sectorlength = 256 * 1024;
> +
> +if (dinfo) {
> +qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
> +&error_abort);
> +}
> +
> +qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
> +  

Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

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

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



Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

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

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

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

thanks
-- PMM



Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

2019-05-08 Thread Radoslaw Biernacki
On Wed, 8 May 2019 at 13:30, Hongbo Zhang  wrote:

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

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


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

Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

2019-05-08 Thread Markus Armbruster
Peter Maydell  writes:

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

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

Longer answer:

The old way to configure block backends is -drive.

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

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

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

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

Questions?



Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

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

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

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

> thanks
> -- PMM



Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

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

Some fairly minor comments on this one.

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

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

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

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

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

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

thanks
-- PMM



[Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

2019-04-17 Thread Hongbo Zhang
Following the previous patch, this patch adds peripheral devices to the
newly introduced SBSA-ref machine.

Signed-off-by: Hongbo Zhang 
---
 hw/arm/sbsa-ref.c | 451 ++
 1 file changed, 451 insertions(+)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 652ec13..3fb0027 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -21,6 +21,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/units.h"
+#include "sysemu/device_tree.h"
 #include "sysemu/numa.h"
 #include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
@@ -28,11 +29,28 @@
 #include "kvm_arm.h"
 #include "hw/arm/arm.h"
 #include "hw/boards.h"
+#include "hw/ide/internal.h"
+#include "hw/ide/ahci_internal.h"
 #include "hw/intc/arm_gicv3_common.h"
+#include "hw/loader.h"
+#include "hw/pci-host/gpex.h"
+#include "hw/usb.h"
+#include "net/net.h"
 
 #define RAMLIMIT_GB 8192
 #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
 
+#define NUM_IRQS256
+#define NUM_SMMU_IRQS   4
+#define NUM_SATA_PORTS  6
+
+#define VIRTUAL_PMU_IRQ7
+#define ARCH_GIC_MAINT_IRQ 9
+#define ARCH_TIMER_VIRT_IRQ11
+#define ARCH_TIMER_S_EL1_IRQ   13
+#define ARCH_TIMER_NS_EL1_IRQ  14
+#define ARCH_TIMER_NS_EL2_IRQ  10
+
 enum {
 SBSA_FLASH,
 SBSA_MEM,
@@ -115,6 +133,415 @@ static const int sbsa_ref_irqmap[] = {
 [SBSA_EHCI] = 11,
 };
 
+/*
+ * Firmware on this machine only uses ACPI table to load OS, these limited
+ * device tree nodes are just to let firmware know the info which varies from
+ * command line parameters, so it is not necessary to be fully compatible
+ * with the kernel CPU and NUMA binding rules.
+ */
+static void create_fdt(SBSAMachineState *vms)
+{
+void *fdt = create_device_tree(&vms->fdt_size);
+const MachineState *ms = MACHINE(vms);
+int cpu;
+
+if (!fdt) {
+error_report("create_device_tree() failed");
+exit(1);
+}
+
+vms->fdt = fdt;
+
+qemu_fdt_setprop_string(fdt, "/", "compatible", "linux,sbsa-ref");
+qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x2);
+qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2);
+
+if (have_numa_distance) {
+int size = nb_numa_nodes * nb_numa_nodes * 3 * sizeof(uint32_t);
+uint32_t *matrix = g_malloc0(size);
+int idx, i, j;
+
+for (i = 0; i < nb_numa_nodes; i++) {
+for (j = 0; j < nb_numa_nodes; j++) {
+idx = (i * nb_numa_nodes + j) * 3;
+matrix[idx + 0] = cpu_to_be32(i);
+matrix[idx + 1] = cpu_to_be32(j);
+matrix[idx + 2] = cpu_to_be32(numa_info[i].distance[j]);
+}
+}
+
+qemu_fdt_add_subnode(fdt, "/distance-map");
+qemu_fdt_setprop(fdt, "/distance-map", "distance-matrix",
+ matrix, size);
+g_free(matrix);
+}
+
+qemu_fdt_add_subnode(vms->fdt, "/cpus");
+
+for (cpu = vms->smp_cpus - 1; cpu >= 0; cpu--) {
+char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
+ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));
+CPUState *cs = CPU(armcpu);
+
+qemu_fdt_add_subnode(vms->fdt, nodename);
+
+if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) {
+qemu_fdt_setprop_cell(vms->fdt, nodename, "numa-node-id",
+ms->possible_cpus->cpus[cs->cpu_index].props.node_id);
+}
+
+g_free(nodename);
+}
+}
+
+static void create_one_flash(const char *name, hwaddr flashbase,
+ hwaddr flashsize, const char *file,
+ MemoryRegion *sysmem)
+{
+/*
+ * Create and map a single flash device. We use the same
+ * parameters as the flash devices on the Versatile Express board.
+ */
+DriveInfo *dinfo = drive_get_next(IF_PFLASH);
+DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+const uint64_t sectorlength = 256 * 1024;
+
+if (dinfo) {
+qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
+&error_abort);
+}
+
+qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
+qdev_prop_set_uint64(dev, "sector-length", sectorlength);
+qdev_prop_set_uint8(dev, "width", 4);
+qdev_prop_set_uint8(dev, "device-width", 2);
+qdev_prop_set_bit(dev, "big-endian", false);
+qdev_prop_set_uint16(dev, "id0", 0x89);
+qdev_prop_set_uint16(dev, "id1", 0x18);
+qdev_prop_set_uint16(dev, "id2", 0x00);
+qdev_prop_set_uint16(dev, "id3", 0x00);
+qdev_prop_set_string(dev, "name", name);
+qdev_init_nofail(dev);
+
+memory_region_add_subregion(sysmem, flashbase,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 
0));
+
+if (file) {
+char *fn;
+int image_size;
+
+if (drive_get(IF_PFLASH, 0, 0)) {
+error_report("The contents of the first flash device may