On Mon, Jan 11, 2016 at 8:04 AM, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 5 January 2016 at 22:05, Alistair Francis > <alistair.fran...@xilinx.com> wrote: >> The Xilinx ZynqMP SoC and EP108 board supports three memory regions: >> - A 2GB region starting at 0 >> - A 32GB region starting at 32GB >> - A 256GB region starting at 768GB >> >> This patch adds support for the first two memory regions, which is >> automatically created based on the size specified by the QEMU memory >> command line argument. >> >> On hardware the physical memory region is one continuous region, it is then >> mapped into the three different regions by the DDRC. As we don't model the >> DDRC this is done at startup by QEMU. The board creates the memory region and >> then passes that memory region to the SoC. The SoC then maps the memory >> regions. >> >> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com> >> Reviewed-by: Peter Crosthwaite <crosthwaite.pe...@gmail.com> >> --- >> V4: >> - Small fixes >> - Localisation of ram_size >> V3: >> - Assert on the RAM sizes >> - Remove ram_size property >> - General fixes >> V2: >> - Create one continuous memory region and pass it to the SoC > >> @@ -35,20 +32,12 @@ static void xlnx_ep108_init(MachineState *machine) >> XlnxEP108 *s = g_new0(XlnxEP108, 1); >> Error *err = NULL; >> >> - object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP); >> - object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc), >> - &error_abort); >> - >> - object_property_set_bool(OBJECT(&s->soc), true, "realized", &err); >> - if (err) { >> - error_report("%s", error_get_pretty(err)); >> - exit(1); >> - } >> - >> - if (machine->ram_size > EP108_MAX_RAM_SIZE) { >> + /* Create the memory region to pass to the SoC */ >> + if (machine->ram_size > XLNX_ZYNQMP_MAX_RAM_SIZE) { > > Unfortunately this doesn't build on 32-bit hosts: > > /home/petmay01/qemu/hw/arm/xlnx-ep108.c: In function 'xlnx_ep108_init': > /home/petmay01/qemu/hw/arm/xlnx-ep108.c:36:16: error: comparison is > always false due to limited range of data type [-Werror=type-limits] > if (machine->ram_size > XLNX_ZYNQMP_MAX_RAM_SIZE) { > ^ > /home/petmay01/qemu/hw/arm/xlnx-ep108.c:40:9: error: large integer > implicitly truncated to unsigned type [-Werror=overflow] > machine->ram_size = XLNX_ZYNQMP_MAX_RAM_SIZE; > ^ > cc1: all warnings being treated as errors > > so I'm going to drop it from the target-arm pullreq. Please could > you fix the compile issue and resend?
I think I have fixed them. Unfortunately I don't have access to a 32-bit machine to test. > > There are a couple of problems you're running into: > > (1) machine->ram_size is a ram_addr_t so might be 32 bit; you > can do what virt.c does to avoid the warning and use a local > uin64_t variable for the comparison Ok, I now create a uint64_t variable to store the value. > > (2) complaint about reassigning back to ram_size. this is spurious > but you can avoid it by making this board behave the same way as > virt.c, vexpress.c etc do if presented with an unsupported > ram_size -- you should fail, rather than truncating and continuing. If I'm using a 64-bit variable to store the value won't this no longer be a problem? > > (3) %llx is not the correct format string for a ram_addr_t: > use RAM_ADDR_FMT. (This isn't making the compiler complain, > but I noticed it looking at the code.) Again, isn't this fixed by changing to a variable? Thanks, Alistair > > thanks > -- PMM >