[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH v1 11/12] hw/arm: versal: Add a model of Xilinx Ve
From: |
Edgar E. Iglesias |
Subject: |
Re: [Qemu-arm] [PATCH v1 11/12] hw/arm: versal: Add a model of Xilinx Versal SoC |
Date: |
Tue, 9 Oct 2018 00:25:41 +0200 |
User-agent: |
Mutt/1.9.4 (2018-02-28) |
On Mon, Oct 08, 2018 at 02:19:09PM +0100, Peter Maydell wrote:
> On 3 October 2018 at 16:07, Edgar E. Iglesias <address@hidden> wrote:
> > From: "Edgar E. Iglesias" <address@hidden>
> >
> > Add a model of Xilinx Versal SoC.
> >
> > Signed-off-by: Edgar E. Iglesias <address@hidden>
> > ---
> > default-configs/aarch64-softmmu.mak | 1 +
> > hw/arm/Makefile.objs | 1 +
> > hw/arm/xlnx-versal.c | 339
> > ++++++++++++++++++++++++++++++++++++
> > include/hw/arm/xlnx-versal.h | 122 +++++++++++++
> > 4 files changed, 463 insertions(+)
> > create mode 100644 hw/arm/xlnx-versal.c
> > create mode 100644 include/hw/arm/xlnx-versal.h
> >
>
>
> > +#define XLNX_VERSAL_ACPU_TYPE "cortex-a72" "-" TYPE_ARM_CPU
>
> ARM_CPU_TYPE_NAME("cortex-a72") is preferable to hand-assembling
> the type name like this.
Fixed for v2.
>
> > +#define GEM_REVISION 0x40070106
> > +
>
> > + for (i = 0; i < nr_apu_cpus; i++) {
> > + DeviceState *cpudev = DEVICE(s->fpd.apu.cpu[i]);
> > + int ppibase = XLNX_VERSAL_NR_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
> > + qemu_irq maint_irq;
> > + int ti;
> > + /* Mapping from the output timer irq lines from the CPU to the
> > + * GIC PPI inputs we use for the virt board.
> > + */
>
> This isn't the virt board :-) -- cut-n-pasted comment ?
Fixed for v2.
>
> > + const int timer_irq[] = {
> > + [GTIMER_PHYS] = VERSAL_TIMER_NS_EL1_IRQ,
> > + [GTIMER_VIRT] = VERSAL_TIMER_VIRT_IRQ,
> > + [GTIMER_HYP] = VERSAL_TIMER_NS_EL2_IRQ,
> > + [GTIMER_SEC] = VERSAL_TIMER_S_EL1_IRQ,
> > + };
> > +
> > + for (ti = 0; ti < ARRAY_SIZE(timer_irq); ti++) {
> > + qdev_connect_gpio_out(cpudev, ti,
> > + qdev_get_gpio_in(gicdev,
> > + ppibase +
> > timer_irq[ti]));
> > + }
> > + maint_irq = qdev_get_gpio_in(gicdev,
> > + ppibase + VERSAL_GIC_MAINT_IRQ);
> > + qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt",
> > + 0, maint_irq);
> > + sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev,
> > ARM_CPU_IRQ));
> > + sysbus_connect_irq(gicbusdev, i + nr_apu_cpus,
> > + qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> > + sysbus_connect_irq(gicbusdev, i + 2 * nr_apu_cpus,
> > + qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
> > + sysbus_connect_irq(gicbusdev, i + 3 * nr_apu_cpus,
> > + qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
> > + }
> > +
> > + for (i = 0; i < XLNX_VERSAL_NR_IRQS; i++) {
> > + pic[i] = qdev_get_gpio_in(gicdev, i);
> > + }
>
> > +/* This takes the board allocated linear DDR memory and creates aliases
> > + * for each split DDR range/apperture on the Versal address map.
>
> "aperture"
Fixed
>
>
>
> > +static void versal_realize(DeviceState *dev, Error **errp)
> > +{
> > + Versal *s = XLNX_VERSAL(dev);
> > + qemu_irq pic[XLNX_VERSAL_NR_IRQS];
> > +
> > + versal_create_apu_cpus(s, errp);
> > + versal_create_apu_gic(s, pic, errp);
> > + versal_create_uarts(s, pic);
> > + versal_create_gems(s, pic);
> > + versal_map_ddr(s);
> > + versal_unimp(s);
> > +
> > + /* Create the OCM. */
> > + memory_region_init_ram(&s->lpd.mr_ocm, OBJECT(s), "ocm",
> > + MM_OCM_SIZE, &error_fatal);
>
> What's an OCM? Is it really memory, or is this a stub for something?
I've changed the comment to spell out that it's an On Chip Memory.
>
> > +
> > + memory_region_add_subregion_overlap(&s->mr_ps, MM_OCM, &s->lpd.mr_ocm,
> > 0);
> > + memory_region_add_subregion_overlap(&s->fpd.apu.mr, 0, &s->mr_ps, 0);
> > +}
> > +
> > +static void versal_init(Object *obj)
> > +{
> > + Versal *s = XLNX_VERSAL(obj);
> > +
> > + memory_region_init(&s->fpd.apu.mr, obj, "mr-apu", UINT64_MAX);
> > + memory_region_init(&s->mr_ps, obj, "mr-ps-switch", UINT64_MAX);
> > +}
> > +
> > +static const VMStateDescription versal_vmstate = {
> > + .name = "xlnx-ve",
> > + .version_id = 1,
> > + .minimum_version_id = 1,
> > + .fields = (VMStateField[]) {
> > + /* FIXME. */
>
> Stray FIXME comment -- I think the answer may be "the
> SoC object has no state of its own so needs neither a
> vmsd nor a reset method" ? (If so and if you drop them,
> do put a comment in the class init about why they're not
> provided. Some day we may have a mechanism for a device
> to explicitly say "I need no vmstate" so we can assert if
> none is provided.)
Thanks, I'll do that.
>
> > + VMSTATE_END_OF_LIST()
> > + }
> > +};
> > +
> > +static Property versal_properties[] = {
> > + DEFINE_PROP_LINK("ddr", Versal, cfg.mr_ddr, TYPE_MEMORY_REGION,
> > + MemoryRegion *),
> > + DEFINE_PROP_UINT32("psci-conduit", Versal, cfg.psci_conduit, 0),
> > + DEFINE_PROP_END_OF_LIST()
> > +};
> > +
> > +static void versal_reset(DeviceState *dev)
> > +{
> > +}
> > +
> > +static void versal_class_init(ObjectClass *klass, void *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > + dc->realize = versal_realize;
> > + dc->vmsd = &versal_vmstate;
> > + dc->props = versal_properties;
> > + dc->reset = versal_reset;
> > +}
>
> Looks good otherwise.
Thanks,
Edgar
- [Qemu-arm] [PATCH v1 10/12] target/arm: Add the Cortex-A72, (continued)
- [Qemu-arm] [PATCH v1 10/12] target/arm: Add the Cortex-A72, Edgar E. Iglesias, 2018/10/03
- Re: [Qemu-arm] [PATCH v1 10/12] target/arm: Add the Cortex-A72, Peter Maydell, 2018/10/08
- Re: [Qemu-arm] [PATCH v1 10/12] target/arm: Add the Cortex-A72, Edgar E. Iglesias, 2018/10/08
- Re: [Qemu-arm] [PATCH v1 10/12] target/arm: Add the Cortex-A72, Peter Maydell, 2018/10/09
- Re: [Qemu-arm] [PATCH v1 10/12] target/arm: Add the Cortex-A72, Edgar E. Iglesias, 2018/10/09
- Re: [Qemu-arm] [Qemu-devel] [PATCH v1 10/12] target/arm: Add the Cortex-A72, Laurent Desnogues, 2018/10/09
- Re: [Qemu-arm] [Qemu-devel] [PATCH v1 10/12] target/arm: Add the Cortex-A72, Edgar E. Iglesias, 2018/10/09
[Qemu-arm] [PATCH v1 11/12] hw/arm: versal: Add a model of Xilinx Versal SoC, Edgar E. Iglesias, 2018/10/03
[Qemu-arm] [PATCH v1 12/12] hw/arm: versal: Add a virtual Xilinx Versal board, Edgar E. Iglesias, 2018/10/03
Re: [Qemu-arm] [PATCH v1 00/12] arm: Add first models of Xilinx Versal SoC, Peter Maydell, 2018/10/08