On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zh...@linaro.org> wrote:
>
> Following the previous patch, this patch adds peripheral devices to the
> newly introduced SBSA-ref machine.
>
> Signed-off-by: Hongbo Zhang <hongbo.zh...@linaro.org>
> ---
> hw/arm/sbsa-ref.c | 451
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 451 insertions(+)
Some fairly minor comments on this one.
> +static void create_flash(const SBSAMachineState *vms,
> + MemoryRegion *sysmem,
> + MemoryRegion *secure_sysmem)
> +{
> + /*
> + * Create one secure and nonsecure flash devices to fill SBSA_FLASH
> + * space in the memmap, file passed via -bios goes in the first one.
> + */
> + hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
> + hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
> +
> + create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
> + bios_name, secure_sysmem);
> + create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize,
> + NULL, sysmem);
> +}
I think Markus might have an opinion on the best way to create
flash devices on a new board model. Is "just create two flash
devices the way the virt board does" the right thing?
> +static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic)
> +{
> + hwaddr base = vms->memmap[SBSA_AHCI].base;
> + int irq = vms->irqmap[SBSA_AHCI];
> + DeviceState *dev;
> + DriveInfo *hd[NUM_SATA_PORTS];
> + SysbusAHCIState *sysahci;
> + AHCIState *ahci;
> + int i;
> +
> + dev = qdev_create(NULL, "sysbus-ahci");
> + qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS);
> + qdev_init_nofail(dev);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> +
> + sysahci = SYSBUS_AHCI(dev);
> + ahci = &sysahci->ahci;
> + ide_drive_get(hd, ARRAY_SIZE(hd));
> + for (i = 0; i < ahci->ports; i++) {
> + if (hd[i] == NULL) {
> + continue;
> + }
> + ide_create_drive(&ahci->dev[i].port, 0, hd[i]);
> + }
> +}
> +
> +static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic)
> +{
> + hwaddr base = vms->memmap[SBSA_EHCI].base;
> + int irq = vms->irqmap[SBSA_EHCI];
> + USBBus *usb_bus;
> +
> + sysbus_create_simple("platform-ehci-usb", base, pic[irq]);
> +
> + usb_bus = usb_bus_find(-1);
> + usb_create_simple(usb_bus, "usb-kbd");
> + usb_create_simple(usb_bus, "usb-mouse");
I don't think we should automatically create the usb keyboard
and mouse devices. The user can do it on the command line if they
want them.
> static void sbsa_ref_init(MachineState *machine)
> {
> SBSAMachineState *vms = SBSA_MACHINE(machine);
> @@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine)
> bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> const CPUArchIdList *possible_cpus;
> int n, sbsa_max_cpus;
> + qemu_irq pic[NUM_IRQS];
>
> if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
> error_report("sbsa-ref: CPU type other than the built-in "
> @@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine)
> machine->ram_size);
> memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram);
>
> + create_fdt(vms);
> +
> + create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
> +
> + create_secure_ram(vms, secure_sysmem);
> +
> + create_gic(vms, pic);
> +
> + create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0));
> + create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1));
> + create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2));
What's the third UART for (ie what is the name intended to mean)?
Should we have more than one non-secure UART?
thanks
-- PMM
