| Message ID | 3e026b514473978a9a68cf272a24a52c30bed437.1427108387.git.peter.crosthwaite@xilinx.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Mar 23, 2015 at 9:05 PM, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > With quad Cortex-A53 CPUs. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > changed since v2: > Added [*] to cpu child property name. > changed since v1: > Add &error_abort to CPU child adder call. > > default-configs/aarch64-softmmu.mak | 2 +- > hw/arm/Makefile.objs | 1 + > hw/arm/xlnx-zynqmp.c | 72 +++++++++++++++++++++++++++++++++++++ > include/hw/arm/xlnx-zynqmp.h | 21 +++++++++++ > 4 files changed, 95 insertions(+), 1 deletion(-) > create mode 100644 hw/arm/xlnx-zynqmp.c > create mode 100644 include/hw/arm/xlnx-zynqmp.h > > diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak > index 6d3b5c7..96dd994 100644 > --- a/default-configs/aarch64-softmmu.mak > +++ b/default-configs/aarch64-softmmu.mak > @@ -3,4 +3,4 @@ > # We support all the 32 bit boards so need all their config > include arm-softmmu.mak > > -# Currently no 64-bit specific config requirements > +CONFIG_XLNX_ZYNQMP=y > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs > index 2577f68..d7cd5f4 100644 > --- a/hw/arm/Makefile.objs > +++ b/hw/arm/Makefile.objs > @@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o > obj-y += omap1.o omap2.o strongarm.o > obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o > obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o > +obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o > diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c > new file mode 100644 > index 0000000..41c207a > --- /dev/null > +++ b/hw/arm/xlnx-zynqmp.c > @@ -0,0 +1,72 @@ > +/* > + * Xilinx Zynq MPSoC emulation > + * > + * Copyright (C) 2015 Xilinx Inc > + * Written by Peter Crosthwaite <peter.crosthwaite@xilinx.com> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > + * for more details. > + */ > + > +#include "hw/arm/xlnx-zynqmp.h" > + > +static void xlnx_zynqmp_init(Object *obj) > +{ > + XlnxZynqMPState *s = XLNX_ZYNQMP(obj); > + int i; > + > + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { > + object_initialize(&s->cpu[i], sizeof(s->cpu[i]), > + "cortex-a53-" TYPE_ARM_CPU); > + object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]), > + &error_abort); > + } > +} > + > +#define ERR_PROP_CHECK_RETURN(err, errp) do { \ > + if (err) { \ > + error_propagate((errp), (err)); \ > + return; \ > + } \ > +} while (0) > + > +static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) > +{ > + XlnxZynqMPState *s = XLNX_ZYNQMP(dev); > + uint8_t i; > + Error *err = NULL; > + > + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { > + object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err); > + ERR_PROP_CHECK_RETURN(err, errp); > + } > +} > + > +static void xlnx_zynqmp_class_init(ObjectClass *oc, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(oc); > + > + dc->realize = xlnx_zynqmp_realize; > +} > + > +static const TypeInfo xlnx_zynqmp_type_info = { > + .name = TYPE_XLNX_ZYNQMP, > + .parent = TYPE_DEVICE, > + .instance_size = sizeof(XlnxZynqMPState), > + .instance_init = xlnx_zynqmp_init, > + .class_init = xlnx_zynqmp_class_init, > +}; > + > +static void xlnx_zynqmp_register_types(void) > +{ > + type_register_static(&xlnx_zynqmp_type_info); > +} > + > +type_init(xlnx_zynqmp_register_types) > diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h > new file mode 100644 > index 0000000..d6b3b92 > --- /dev/null > +++ b/include/hw/arm/xlnx-zynqmp.h > @@ -0,0 +1,21 @@ > +#ifndef XLNX_ZYNQMP_H_ > + > +#include "qemu-common.h" > +#include "hw/arm/arm.h" > + > +#define TYPE_XLNX_ZYNQMP "xlnx,zynqmp" > +#define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \ > + TYPE_XLNX_ZYNQMP) > + > +#define XLNX_ZYNQMP_NUM_CPUS 4 > + > +typedef struct XlnxZynqMPState { > + /*< private >*/ > + DeviceState parent_obj; > + /*< public >*/ I have the same nit pick here as the others, I would just space it out differently. That's not important though. Reviewed-by: Alistair Francis <alistair.francis@xilinx.com> Thanks, Alistair > + > + ARMCPU cpu[XLNX_ZYNQMP_NUM_CPUS]; > +} XlnxZynqMPState; > + > +#define XLNX_ZYNQMP_H_ > +#endif > -- > 2.3.1.2.g90df61e.dirty > >
On 23 March 2015 at 11:05, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > With quad Cortex-A53 CPUs. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > changed since v2: > Added [*] to cpu child property name. > changed since v1: > Add &error_abort to CPU child adder call. > > default-configs/aarch64-softmmu.mak | 2 +- > hw/arm/Makefile.objs | 1 + > hw/arm/xlnx-zynqmp.c | 72 +++++++++++++++++++++++++++++++++++++ > include/hw/arm/xlnx-zynqmp.h | 21 +++++++++++ > 4 files changed, 95 insertions(+), 1 deletion(-) > create mode 100644 hw/arm/xlnx-zynqmp.c > create mode 100644 include/hw/arm/xlnx-zynqmp.h > > diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak > index 6d3b5c7..96dd994 100644 > --- a/default-configs/aarch64-softmmu.mak > +++ b/default-configs/aarch64-softmmu.mak > @@ -3,4 +3,4 @@ > # We support all the 32 bit boards so need all their config > include arm-softmmu.mak > > -# Currently no 64-bit specific config requirements > +CONFIG_XLNX_ZYNQMP=y > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs > index 2577f68..d7cd5f4 100644 > --- a/hw/arm/Makefile.objs > +++ b/hw/arm/Makefile.objs > @@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o > obj-y += omap1.o omap2.o strongarm.o > obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o > obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o > +obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o Can this be a common-obj- ? > diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c > new file mode 100644 > index 0000000..41c207a > --- /dev/null > +++ b/hw/arm/xlnx-zynqmp.c > @@ -0,0 +1,72 @@ > +/* > + * Xilinx Zynq MPSoC emulation > + * > + * Copyright (C) 2015 Xilinx Inc > + * Written by Peter Crosthwaite <peter.crosthwaite@xilinx.com> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > + * for more details. > + */ > + > +#include "hw/arm/xlnx-zynqmp.h" > + > +static void xlnx_zynqmp_init(Object *obj) We don't abbreviate 'Xilinx' in existing type and function names, so it's a bit inconsistent to do so here. > +{ > + XlnxZynqMPState *s = XLNX_ZYNQMP(obj); > + int i; > + > + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { > + object_initialize(&s->cpu[i], sizeof(s->cpu[i]), > + "cortex-a53-" TYPE_ARM_CPU); > + object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]), > + &error_abort); > + } > +} > + > +#define ERR_PROP_CHECK_RETURN(err, errp) do { \ > + if (err) { \ > + error_propagate((errp), (err)); \ > + return; \ > + } \ > +} while (0) I don't think our QOM error handling is so ugly as to justify hiding it behind a macro (particularly not one with hidden control flow). > +static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) > +{ > + XlnxZynqMPState *s = XLNX_ZYNQMP(dev); > + uint8_t i; > + Error *err = NULL; > + > + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { > + object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err); > + ERR_PROP_CHECK_RETURN(err, errp); > + } > +} > + > +static void xlnx_zynqmp_class_init(ObjectClass *oc, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(oc); > + > + dc->realize = xlnx_zynqmp_realize; > +} > + > +static const TypeInfo xlnx_zynqmp_type_info = { > + .name = TYPE_XLNX_ZYNQMP, > + .parent = TYPE_DEVICE, > + .instance_size = sizeof(XlnxZynqMPState), > + .instance_init = xlnx_zynqmp_init, > + .class_init = xlnx_zynqmp_class_init, > +}; > + > +static void xlnx_zynqmp_register_types(void) > +{ > + type_register_static(&xlnx_zynqmp_type_info); > +} > + > +type_init(xlnx_zynqmp_register_types) > diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h > new file mode 100644 > index 0000000..d6b3b92 > --- /dev/null > +++ b/include/hw/arm/xlnx-zynqmp.h > @@ -0,0 +1,21 @@ This file needs a copyright-and-license comment. > +#ifndef XLNX_ZYNQMP_H_ No trailing underscore, please. > + > +#include "qemu-common.h" > +#include "hw/arm/arm.h" > + > +#define TYPE_XLNX_ZYNQMP "xlnx,zynqmp" > +#define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \ > + TYPE_XLNX_ZYNQMP) > + > +#define XLNX_ZYNQMP_NUM_CPUS 4 > + > +typedef struct XlnxZynqMPState { > + /*< private >*/ > + DeviceState parent_obj; > + /*< public >*/ > + > + ARMCPU cpu[XLNX_ZYNQMP_NUM_CPUS]; > +} XlnxZynqMPState; > + > +#define XLNX_ZYNQMP_H_ > +#endif > -- > 2.3.1.2.g90df61e.dirty > thanks -- PMM
On 23 March 2015 at 11:05, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > +static void xlnx_zynqmp_init(Object *obj) > +{ > + XlnxZynqMPState *s = XLNX_ZYNQMP(obj); > + int i; > + > + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { > + object_initialize(&s->cpu[i], sizeof(s->cpu[i]), > + "cortex-a53-" TYPE_ARM_CPU); > + object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]), > + &error_abort); > + } > +} > +static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) > +{ > + XlnxZynqMPState *s = XLNX_ZYNQMP(dev); > + uint8_t i; > + Error *err = NULL; > + > + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { > + object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err); > + ERR_PROP_CHECK_RETURN(err, errp); > + } > +} This seems to be a touch short on CPU property setting. I would expect you want most or all of: * has_el3 * psci-conduit * start-powered-off for the secondaries * reset-cbar (Some of those may be new in master since this series was posted.) thanks -- PMM
On Thu, Apr 23, 2015 at 10:42 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 23 March 2015 at 11:05, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> With quad Cortex-A53 CPUs. >> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> --- >> changed since v2: >> Added [*] to cpu child property name. >> changed since v1: >> Add &error_abort to CPU child adder call. >> >> default-configs/aarch64-softmmu.mak | 2 +- >> hw/arm/Makefile.objs | 1 + >> hw/arm/xlnx-zynqmp.c | 72 +++++++++++++++++++++++++++++++++++++ >> include/hw/arm/xlnx-zynqmp.h | 21 +++++++++++ >> 4 files changed, 95 insertions(+), 1 deletion(-) >> create mode 100644 hw/arm/xlnx-zynqmp.c >> create mode 100644 include/hw/arm/xlnx-zynqmp.h >> >> diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak >> index 6d3b5c7..96dd994 100644 >> --- a/default-configs/aarch64-softmmu.mak >> +++ b/default-configs/aarch64-softmmu.mak >> @@ -3,4 +3,4 @@ >> # We support all the 32 bit boards so need all their config >> include arm-softmmu.mak >> >> -# Currently no 64-bit specific config requirements >> +CONFIG_XLNX_ZYNQMP=y >> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs >> index 2577f68..d7cd5f4 100644 >> --- a/hw/arm/Makefile.objs >> +++ b/hw/arm/Makefile.objs >> @@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o >> obj-y += omap1.o omap2.o strongarm.o >> obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o >> obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o >> +obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o > > Can this be a common-obj- ? > Seems to build fine. I'll make this change. It's inconsistent with surrounding code so I guess this is a new policy? >> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c >> new file mode 100644 >> index 0000000..41c207a >> --- /dev/null >> +++ b/hw/arm/xlnx-zynqmp.c >> @@ -0,0 +1,72 @@ >> +/* >> + * Xilinx Zynq MPSoC emulation >> + * >> + * Copyright (C) 2015 Xilinx Inc >> + * Written by Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License as published by the >> + * Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, but WITHOUT >> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License >> + * for more details. >> + */ >> + >> +#include "hw/arm/xlnx-zynqmp.h" >> + >> +static void xlnx_zynqmp_init(Object *obj) > > We don't abbreviate 'Xilinx' in existing type and function > names, so it's a bit inconsistent to do so here. > I want to fix this policy going forward as I am matching the device tree vendor code and the 2 less chars does save on a few 80 char wraps (particularly on lines with more than one "xilinx"). The existing code in tree is already self inconsistent between C variable names and literal string in this matter, e,g: hw/char/xilinx_uartlite.c:49:#define TYPE_XILINX_UARTLITE "xlnx.xps-uartlite" hw/char/xilinx_uartlite.c:221: "xlnx.xps-uartlite", R_MAX * 4); hw/dma/xilinx_axidma.c:36:#define TYPE_XILINX_AXI_DMA "xlnx.axi-dma" hw/dma/xilinx_axidma.c:601: "xlnx.axi-dma", R_MAX * 4 * 2); hw/intc/xilinx_intc.c:40:#define TYPE_XILINX_INTC "xlnx.xps-intc" hw/intc/xilinx_intc.c:171: memory_region_init_io(&p->mmio, obj, &pic_ops, p, "xlnx.xps-intc", With a mix of XILINX and "xlnx". I think "xlnx' is universally correct going forward. >> +{ >> + XlnxZynqMPState *s = XLNX_ZYNQMP(obj); >> + int i; >> + >> + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { >> + object_initialize(&s->cpu[i], sizeof(s->cpu[i]), >> + "cortex-a53-" TYPE_ARM_CPU); >> + object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]), >> + &error_abort); >> + } >> +} >> + >> +#define ERR_PROP_CHECK_RETURN(err, errp) do { \ >> + if (err) { \ >> + error_propagate((errp), (err)); \ >> + return; \ >> + } \ >> +} while (0) > > I don't think our QOM error handling is so ugly as to justify > hiding it behind a macro (particularly not one with hidden > control flow). > Fixing. This is discussed on list so I guess I can join that discussion. It is a little verbose having every 1 line API call have 4 lines or error handling. >> +static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) >> +{ >> + XlnxZynqMPState *s = XLNX_ZYNQMP(dev); >> + uint8_t i; >> + Error *err = NULL; >> + >> + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { >> + object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err); >> + ERR_PROP_CHECK_RETURN(err, errp); >> + } >> +} >> + >> +static void xlnx_zynqmp_class_init(ObjectClass *oc, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(oc); >> + >> + dc->realize = xlnx_zynqmp_realize; >> +} >> + >> +static const TypeInfo xlnx_zynqmp_type_info = { >> + .name = TYPE_XLNX_ZYNQMP, >> + .parent = TYPE_DEVICE, >> + .instance_size = sizeof(XlnxZynqMPState), >> + .instance_init = xlnx_zynqmp_init, >> + .class_init = xlnx_zynqmp_class_init, >> +}; >> + >> +static void xlnx_zynqmp_register_types(void) >> +{ >> + type_register_static(&xlnx_zynqmp_type_info); >> +} >> + >> +type_init(xlnx_zynqmp_register_types) >> diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h >> new file mode 100644 >> index 0000000..d6b3b92 >> --- /dev/null >> +++ b/include/hw/arm/xlnx-zynqmp.h >> @@ -0,0 +1,21 @@ > > This file needs a copyright-and-license comment. > Fixed. >> +#ifndef XLNX_ZYNQMP_H_ > > No trailing underscore, please. > Fixed. Regards, Peter >> + >> +#include "qemu-common.h" >> +#include "hw/arm/arm.h" >> + >> +#define TYPE_XLNX_ZYNQMP "xlnx,zynqmp" >> +#define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \ >> + TYPE_XLNX_ZYNQMP) >> + >> +#define XLNX_ZYNQMP_NUM_CPUS 4 >> + >> +typedef struct XlnxZynqMPState { >> + /*< private >*/ >> + DeviceState parent_obj; >> + /*< public >*/ >> + >> + ARMCPU cpu[XLNX_ZYNQMP_NUM_CPUS]; >> +} XlnxZynqMPState; >> + >> +#define XLNX_ZYNQMP_H_ >> +#endif >> -- >> 2.3.1.2.g90df61e.dirty >> > > thanks > -- PMM >
On Thu, Apr 23, 2015 at 10:47 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 23 March 2015 at 11:05, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> +static void xlnx_zynqmp_init(Object *obj) >> +{ >> + XlnxZynqMPState *s = XLNX_ZYNQMP(obj); >> + int i; >> + >> + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { >> + object_initialize(&s->cpu[i], sizeof(s->cpu[i]), >> + "cortex-a53-" TYPE_ARM_CPU); >> + object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]), >> + &error_abort); >> + } >> +} > >> +static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) >> +{ >> + XlnxZynqMPState *s = XLNX_ZYNQMP(dev); >> + uint8_t i; >> + Error *err = NULL; >> + >> + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { >> + object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err); >> + ERR_PROP_CHECK_RETURN(err, errp); >> + } >> +} > > This seems to be a touch short on CPU property setting. > I would expect you want most or all of: > * has_el3 has_el3 defines to true by default and the SoC does support EL3. DEFINE_PROP_BOOL("has_el3", ARMCPU, has_el3, true); > * psci-conduit > * start-powered-off for the secondaries Squashed in P16. > * reset-cbar Adding to GIC patch. Regards, Peter > > (Some of those may be new in master since this series was posted.) > > thanks > -- PMM >
On 23 April 2015 at 20:21, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Thu, Apr 23, 2015 at 10:42 AM, Peter Maydell > <peter.maydell@linaro.org> wrote: >> On 23 March 2015 at 11:05, Peter Crosthwaite >> <peter.crosthwaite@xilinx.com> wrote: >>> --- a/hw/arm/Makefile.objs >>> +++ b/hw/arm/Makefile.objs >>> @@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o >>> obj-y += omap1.o omap2.o strongarm.o >>> obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o >>> obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o >>> +obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o >> >> Can this be a common-obj- ? >> > > Seems to build fine. I'll make this change. It's inconsistent with > surrounding code so I guess this is a new policy? Historically this makefile's objects were only built for ARM targets anyway, so it didn't make much difference. Now we have aarch64 it avoids building a .o twice, so it helps to reduce build time where we have source files which don't actually care about which target CPU they're built for. In other makefiles we make more effort to keep things in common-obj. (The usual sticking point is use of functions like "load/store to memory in target CPU endianness" or caring about target CPU page size.) If the topic hadn't happened to come to my attention elsewhere already this week I'd probably not have noticed it in this patch :-) >> We don't abbreviate 'Xilinx' in existing type and function >> names, so it's a bit inconsistent to do so here. >> > > I want to fix this policy going forward as I am matching the device > tree vendor code and the 2 less chars does save on a few 80 char wraps > (particularly on lines with more than one "xilinx"). The existing code > in tree is already self inconsistent between C variable names and > literal string in this matter, e,g: > > hw/char/xilinx_uartlite.c:49:#define TYPE_XILINX_UARTLITE "xlnx.xps-uartlite" > hw/char/xilinx_uartlite.c:221: > "xlnx.xps-uartlite", R_MAX * 4); > hw/dma/xilinx_axidma.c:36:#define TYPE_XILINX_AXI_DMA "xlnx.axi-dma" > hw/dma/xilinx_axidma.c:601: "xlnx.axi-dma", > R_MAX * 4 * 2); > hw/intc/xilinx_intc.c:40:#define TYPE_XILINX_INTC "xlnx.xps-intc" > hw/intc/xilinx_intc.c:171: memory_region_init_io(&p->mmio, obj, > &pic_ops, p, "xlnx.xps-intc", > > With a mix of XILINX and "xlnx". I think "xlnx' is universally correct > going forward. OK. >> I don't think our QOM error handling is so ugly as to justify >> hiding it behind a macro (particularly not one with hidden >> control flow). >> > > Fixing. This is discussed on list so I guess I can join that > discussion. It is a little verbose having every 1 line API call have 4 > lines or error handling. Mmm. However if we want to provide a better approach to this I think it needs to be provided in common code so we can use it consistently across the codebase, rather than as a local macro hack. -- PMM
On 23 April 2015 at 22:38, Peter Maydell <peter.maydell@linaro.org> wrote: > On 23 April 2015 at 20:21, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> On Thu, Apr 23, 2015 at 10:42 AM, Peter Maydell >> <peter.maydell@linaro.org> wrote: >>> On 23 March 2015 at 11:05, Peter Crosthwaite >>> <peter.crosthwaite@xilinx.com> wrote: >>>> --- a/hw/arm/Makefile.objs >>>> +++ b/hw/arm/Makefile.objs >>>> @@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o >>>> obj-y += omap1.o omap2.o strongarm.o >>>> obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o >>>> obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o >>>> +obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o >>> >>> Can this be a common-obj- ? >>> >> >> Seems to build fine. I'll make this change. It's inconsistent with >> surrounding code so I guess this is a new policy? > > Historically this makefile's objects were only built for > ARM targets anyway, so it didn't make much difference. > Now we have aarch64 it avoids building a .o twice I looked a bit more closely at this, and hw/arm is treated specially (the hw/$(TARGET_BASE_ARCH) directories are pulled in directly by Makefile.target and only for obj-y), so for now new object files in this directory should just be in obj-. I might see if we can add support for common-obj-y, but that's a separate patch. thanks -- PMM
On Fri, Apr 24, 2015 at 8:26 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 23 April 2015 at 22:38, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 23 April 2015 at 20:21, Peter Crosthwaite >> <peter.crosthwaite@xilinx.com> wrote: >>> On Thu, Apr 23, 2015 at 10:42 AM, Peter Maydell >>> <peter.maydell@linaro.org> wrote: >>>> On 23 March 2015 at 11:05, Peter Crosthwaite >>>> <peter.crosthwaite@xilinx.com> wrote: >>>>> --- a/hw/arm/Makefile.objs >>>>> +++ b/hw/arm/Makefile.objs >>>>> @@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o >>>>> obj-y += omap1.o omap2.o strongarm.o >>>>> obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o >>>>> obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o >>>>> +obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o >>>> >>>> Can this be a common-obj- ? >>>> >>> >>> Seems to build fine. I'll make this change. It's inconsistent with >>> surrounding code so I guess this is a new policy? >> >> Historically this makefile's objects were only built for >> ARM targets anyway, so it didn't make much difference. >> Now we have aarch64 it avoids building a .o twice > > I looked a bit more closely at this, and hw/arm is treated > specially (the hw/$(TARGET_BASE_ARCH) directories are > pulled in directly by Makefile.target and only for obj-y), Yes, I discovered the same. > so for now new object files in this directory should just be > in obj-. I might see if we can add support for common-obj-y, > but that's a separate patch. > Will keep as-is. Regards, Peter > thanks > -- PMM >
diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak index 6d3b5c7..96dd994 100644 --- a/default-configs/aarch64-softmmu.mak +++ b/default-configs/aarch64-softmmu.mak @@ -3,4 +3,4 @@ # We support all the 32 bit boards so need all their config include arm-softmmu.mak -# Currently no 64-bit specific config requirements +CONFIG_XLNX_ZYNQMP=y diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs index 2577f68..d7cd5f4 100644 --- a/hw/arm/Makefile.objs +++ b/hw/arm/Makefile.objs @@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o obj-y += omap1.o omap2.o strongarm.o obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o +obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c new file mode 100644 index 0000000..41c207a --- /dev/null +++ b/hw/arm/xlnx-zynqmp.c @@ -0,0 +1,72 @@ +/* + * Xilinx Zynq MPSoC emulation + * + * Copyright (C) 2015 Xilinx Inc + * Written by Peter Crosthwaite <peter.crosthwaite@xilinx.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include "hw/arm/xlnx-zynqmp.h" + +static void xlnx_zynqmp_init(Object *obj) +{ + XlnxZynqMPState *s = XLNX_ZYNQMP(obj); + int i; + + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { + object_initialize(&s->cpu[i], sizeof(s->cpu[i]), + "cortex-a53-" TYPE_ARM_CPU); + object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]), + &error_abort); + } +} + +#define ERR_PROP_CHECK_RETURN(err, errp) do { \ + if (err) { \ + error_propagate((errp), (err)); \ + return; \ + } \ +} while (0) + +static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) +{ + XlnxZynqMPState *s = XLNX_ZYNQMP(dev); + uint8_t i; + Error *err = NULL; + + for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) { + object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err); + ERR_PROP_CHECK_RETURN(err, errp); + } +} + +static void xlnx_zynqmp_class_init(ObjectClass *oc, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(oc); + + dc->realize = xlnx_zynqmp_realize; +} + +static const TypeInfo xlnx_zynqmp_type_info = { + .name = TYPE_XLNX_ZYNQMP, + .parent = TYPE_DEVICE, + .instance_size = sizeof(XlnxZynqMPState), + .instance_init = xlnx_zynqmp_init, + .class_init = xlnx_zynqmp_class_init, +}; + +static void xlnx_zynqmp_register_types(void) +{ + type_register_static(&xlnx_zynqmp_type_info); +} + +type_init(xlnx_zynqmp_register_types) diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h new file mode 100644 index 0000000..d6b3b92 --- /dev/null +++ b/include/hw/arm/xlnx-zynqmp.h @@ -0,0 +1,21 @@ +#ifndef XLNX_ZYNQMP_H_ + +#include "qemu-common.h" +#include "hw/arm/arm.h" + +#define TYPE_XLNX_ZYNQMP "xlnx,zynqmp" +#define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \ + TYPE_XLNX_ZYNQMP) + +#define XLNX_ZYNQMP_NUM_CPUS 4 + +typedef struct XlnxZynqMPState { + /*< private >*/ + DeviceState parent_obj; + /*< public >*/ + + ARMCPU cpu[XLNX_ZYNQMP_NUM_CPUS]; +} XlnxZynqMPState; + +#define XLNX_ZYNQMP_H_ +#endif
With quad Cortex-A53 CPUs. Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- changed since v2: Added [*] to cpu child property name. changed since v1: Add &error_abort to CPU child adder call. default-configs/aarch64-softmmu.mak | 2 +- hw/arm/Makefile.objs | 1 + hw/arm/xlnx-zynqmp.c | 72 +++++++++++++++++++++++++++++++++++++ include/hw/arm/xlnx-zynqmp.h | 21 +++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 hw/arm/xlnx-zynqmp.c create mode 100644 include/hw/arm/xlnx-zynqmp.h