From: Peter Maydell <peter.maydell@linaro.org> To: Hongbo Zhang <hongbo.zhang@linaro.org> Cc: QEMU Developers <qemu-devel@nongnu.org>, qemu-arm <qemu-arm@nongnu.org>, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Leif Lindholm <leif.lindholm@linaro.org>, Radoslaw Biernacki <radoslaw.biernacki@linaro.org>, Markus Armbruster <armbru@redhat.com> Subject: Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part Date: Tue, 30 Apr 2019 15:16:59 +0100 [thread overview] Message-ID: <CAFEAcA-poCxPqPtfhx4mUJ5pcOjn1Hz-WNxEt29f=JgpFMi4Kg@mail.gmail.com> (raw) In-Reply-To: <1555560291-3415-3-git-send-email-hongbo.zhang@linaro.org> On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote: > > Following the previous patch, this patch adds peripheral devices to the > newly introduced SBSA-ref machine. > > Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org> > --- > hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 451 insertions(+) Some fairly minor comments on this one. > +static void create_flash(const SBSAMachineState *vms, > + MemoryRegion *sysmem, > + MemoryRegion *secure_sysmem) > +{ > + /* > + * Create one secure and nonsecure flash devices to fill SBSA_FLASH > + * space in the memmap, file passed via -bios goes in the first one. > + */ > + hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2; > + hwaddr flashbase = vms->memmap[SBSA_FLASH].base; > + > + create_one_flash("sbsa-ref.flash0", flashbase, flashsize, > + bios_name, secure_sysmem); > + create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize, > + NULL, sysmem); > +} I think Markus might have an opinion on the best way to create flash devices on a new board model. Is "just create two flash devices the way the virt board does" the right thing? > +static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic) > +{ > + hwaddr base = vms->memmap[SBSA_AHCI].base; > + int irq = vms->irqmap[SBSA_AHCI]; > + DeviceState *dev; > + DriveInfo *hd[NUM_SATA_PORTS]; > + SysbusAHCIState *sysahci; > + AHCIState *ahci; > + int i; > + > + dev = qdev_create(NULL, "sysbus-ahci"); > + qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS); > + qdev_init_nofail(dev); > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); > + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]); > + > + sysahci = SYSBUS_AHCI(dev); > + ahci = &sysahci->ahci; > + ide_drive_get(hd, ARRAY_SIZE(hd)); > + for (i = 0; i < ahci->ports; i++) { > + if (hd[i] == NULL) { > + continue; > + } > + ide_create_drive(&ahci->dev[i].port, 0, hd[i]); > + } > +} > + > +static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic) > +{ > + hwaddr base = vms->memmap[SBSA_EHCI].base; > + int irq = vms->irqmap[SBSA_EHCI]; > + USBBus *usb_bus; > + > + sysbus_create_simple("platform-ehci-usb", base, pic[irq]); > + > + usb_bus = usb_bus_find(-1); > + usb_create_simple(usb_bus, "usb-kbd"); > + usb_create_simple(usb_bus, "usb-mouse"); I don't think we should automatically create the usb keyboard and mouse devices. The user can do it on the command line if they want them. > static void sbsa_ref_init(MachineState *machine) > { > SBSAMachineState *vms = SBSA_MACHINE(machine); > @@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine) > bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0); > const CPUArchIdList *possible_cpus; > int n, sbsa_max_cpus; > + qemu_irq pic[NUM_IRQS]; > > if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) { > error_report("sbsa-ref: CPU type other than the built-in " > @@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine) > machine->ram_size); > memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram); > > + create_fdt(vms); > + > + create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem); > + > + create_secure_ram(vms, secure_sysmem); > + > + create_gic(vms, pic); > + > + create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0)); > + create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1)); > + create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2)); What's the third UART for (ie what is the name intended to mean)? Should we have more than one non-secure UART? thanks -- PMM
WARNING: multiple messages have this Message-ID (diff)
From: Peter Maydell <peter.maydell@linaro.org> To: Hongbo Zhang <hongbo.zhang@linaro.org> Cc: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>, Ard Biesheuvel <ard.biesheuvel@linaro.org>, Markus Armbruster <armbru@redhat.com>, Leif Lindholm <leif.lindholm@linaro.org>, QEMU Developers <qemu-devel@nongnu.org>, qemu-arm <qemu-arm@nongnu.org> Subject: Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part Date: Tue, 30 Apr 2019 15:16:59 +0100 [thread overview] Message-ID: <CAFEAcA-poCxPqPtfhx4mUJ5pcOjn1Hz-WNxEt29f=JgpFMi4Kg@mail.gmail.com> (raw) Message-ID: <20190430141659.DLcRDOlhGFm5T6HEQFiDea-Rv9ItglglPx2JEaXyt-8@z> (raw) In-Reply-To: <1555560291-3415-3-git-send-email-hongbo.zhang@linaro.org> On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang <hongbo.zhang@linaro.org> wrote: > > Following the previous patch, this patch adds peripheral devices to the > newly introduced SBSA-ref machine. > > Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org> > --- > hw/arm/sbsa-ref.c | 451 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 451 insertions(+) Some fairly minor comments on this one. > +static void create_flash(const SBSAMachineState *vms, > + MemoryRegion *sysmem, > + MemoryRegion *secure_sysmem) > +{ > + /* > + * Create one secure and nonsecure flash devices to fill SBSA_FLASH > + * space in the memmap, file passed via -bios goes in the first one. > + */ > + hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2; > + hwaddr flashbase = vms->memmap[SBSA_FLASH].base; > + > + create_one_flash("sbsa-ref.flash0", flashbase, flashsize, > + bios_name, secure_sysmem); > + create_one_flash("sbsa-ref.flash1", flashbase + flashsize, flashsize, > + NULL, sysmem); > +} I think Markus might have an opinion on the best way to create flash devices on a new board model. Is "just create two flash devices the way the virt board does" the right thing? > +static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic) > +{ > + hwaddr base = vms->memmap[SBSA_AHCI].base; > + int irq = vms->irqmap[SBSA_AHCI]; > + DeviceState *dev; > + DriveInfo *hd[NUM_SATA_PORTS]; > + SysbusAHCIState *sysahci; > + AHCIState *ahci; > + int i; > + > + dev = qdev_create(NULL, "sysbus-ahci"); > + qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS); > + qdev_init_nofail(dev); > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); > + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]); > + > + sysahci = SYSBUS_AHCI(dev); > + ahci = &sysahci->ahci; > + ide_drive_get(hd, ARRAY_SIZE(hd)); > + for (i = 0; i < ahci->ports; i++) { > + if (hd[i] == NULL) { > + continue; > + } > + ide_create_drive(&ahci->dev[i].port, 0, hd[i]); > + } > +} > + > +static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic) > +{ > + hwaddr base = vms->memmap[SBSA_EHCI].base; > + int irq = vms->irqmap[SBSA_EHCI]; > + USBBus *usb_bus; > + > + sysbus_create_simple("platform-ehci-usb", base, pic[irq]); > + > + usb_bus = usb_bus_find(-1); > + usb_create_simple(usb_bus, "usb-kbd"); > + usb_create_simple(usb_bus, "usb-mouse"); I don't think we should automatically create the usb keyboard and mouse devices. The user can do it on the command line if they want them. > static void sbsa_ref_init(MachineState *machine) > { > SBSAMachineState *vms = SBSA_MACHINE(machine); > @@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine) > bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0); > const CPUArchIdList *possible_cpus; > int n, sbsa_max_cpus; > + qemu_irq pic[NUM_IRQS]; > > if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) { > error_report("sbsa-ref: CPU type other than the built-in " > @@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine) > machine->ram_size); > memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base, ram); > > + create_fdt(vms); > + > + create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem); > + > + create_secure_ram(vms, secure_sysmem); > + > + create_gic(vms, pic); > + > + create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0)); > + create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1)); > + create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2)); What's the third UART for (ie what is the name intended to mean)? Should we have more than one non-secure UART? thanks -- PMM
next prev parent reply other threads:[~2019-04-30 14:17 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-18 4:04 [Qemu-devel] [PATCH v7 0/2] Add Arm SBSA Reference Machine Hongbo Zhang 2019-04-18 4:04 ` Hongbo Zhang 2019-04-18 4:04 ` [Qemu-devel] [PATCH v7 1/2] hw/arm: Add arm SBSA reference machine, skeleton part Hongbo Zhang 2019-04-18 4:04 ` Hongbo Zhang 2019-04-30 14:03 ` Peter Maydell 2019-04-30 14:03 ` Peter Maydell 2019-05-08 11:22 ` Hongbo Zhang 2019-05-09 14:27 ` Igor Mammedov 2019-05-09 14:40 ` Peter Maydell 2019-04-18 4:04 ` [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part Hongbo Zhang 2019-04-18 4:04 ` Hongbo Zhang 2019-04-30 14:16 ` Peter Maydell [this message] 2019-04-30 14:16 ` Peter Maydell 2019-05-08 11:30 ` Hongbo Zhang 2019-05-08 17:48 ` Radoslaw Biernacki 2019-05-09 8:46 ` Peter Maydell 2019-05-08 13:59 ` Markus Armbruster 2019-06-02 3:16 ` Hongbo Zhang 2019-06-03 10:54 ` Philippe Mathieu-Daudé 2019-06-16 11:41 ` Hongbo Zhang 2019-06-17 11:08 ` Philippe Mathieu-Daudé 2019-06-17 2:44 ` Hongbo Zhang 2019-04-30 14:18 ` [Qemu-devel] [PATCH v7 0/2] Add Arm SBSA Reference Machine Peter Maydell 2019-04-30 14:18 ` Peter Maydell
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAFEAcA-poCxPqPtfhx4mUJ5pcOjn1Hz-WNxEt29f=JgpFMi4Kg@mail.gmail.com' \ --to=peter.maydell@linaro.org \ --cc=ard.biesheuvel@linaro.org \ --cc=armbru@redhat.com \ --cc=hongbo.zhang@linaro.org \ --cc=leif.lindholm@linaro.org \ --cc=qemu-arm@nongnu.org \ --cc=qemu-devel@nongnu.org \ --cc=radoslaw.biernacki@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.