On Tue, Nov 24, 2015 at 10:33 PM, Peter Crosthwaite <crosthwaitepe...@gmail.com> wrote: > On Mon, Nov 23, 2015 at 9:00 PM, Alistair Francis > <alistair.fran...@xilinx.com> wrote: >> The Xilinx EP108 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 middle memory region, which is >> automatically created based on the size specified by the QEMU memory >> command line argument. >> > > Is that the hardware reset value or is it usually uninitialised?
Sorry for the long delay, I was on holidays. Is what uninitialised? > Although I'm guessing it is possible that in real HW even the low > region could be uninitialised (unmapped) too. > >> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com> >> --- >> Also, the Xilinx ZynqMP TRM has been released, if anyone is interested >> it is avalibale at: >> http://www.xilinx.com/products/silicon-devices/soc/zynq-ultrascale-mpsoc.html?resultsTablePreSelect=documenttype:User%20Guides#documentation >> > > I couldn't find much here on how runtime programmable this is (is > there more than on pg282?). Are the mappings software controllable? I can't find much information about this either. I can't image it being run time configurable, and even if it is that would be the DDRC, which we don't model. I also don't have any software cases that changes this during run time. So for the time being I think it should just be configured at boot. We can look into changing that if we ever add the DDRC. > >> hw/arm/xlnx-ep108.c | 47 +++++++++++++++++++++++++++++++++++------------ >> 1 file changed, 35 insertions(+), 12 deletions(-) >> >> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c >> index 2899698..8c59d6d 100644 >> --- a/hw/arm/xlnx-ep108.c >> +++ b/hw/arm/xlnx-ep108.c >> @@ -22,17 +22,22 @@ >> >> typedef struct XlnxEP108 { >> XlnxZynqMPState soc; >> - MemoryRegion ddr_ram; >> + MemoryRegion ddr_ram_low; >> + MemoryRegion ddr_ram_high; > > Looking at pg282 of the doc, the external RAM (as known to boards) is > still only a singleton. So the RAM as instantiated here should still > only be the one (corresponding to a single DIMM slot?). The mapping of > that single RAM to multiple windows is a SoC (or DDRC) implemented > feature. So instead, the machine should create the RAM but not attach > it to the system memory. The RAM is then handed over to the SoC (MRs > are QOM objects so it can be a QOM link) which can then create aliases > into the single RAM for the windows. Those aliases are then in turn > added to the system memory map by the SoC. This avoid having to dup > this code when more boards happen and also prepares support for > runtime remapping of the memory locations (assuming that is > possible?). Ok, I think I understand what you are thinking. I'll send a patch today. > > Ideally this would all be managed by a DDRC peripheral, but just > getting it on the SoC level would be a good step. > >> } XlnxEP108; >> >> -/* Max 2GB RAM */ >> -#define EP108_MAX_RAM_SIZE 0x80000000ull >> +/* Maximum size of the low memory region */ >> +#define EP108_MAX_LOW_RAM_SIZE 0x80000000ull >> +/* Total maximum size of the EP108 memory */ >> +#define EP108_MAX_RAM_SIZE 0x880000000ull > > This feels odd. I think both LOW and HIGH should have MAX defined and > the overall MAX is addition of the two. Doesn't really matter to me, I'll swap it to your way. Thanks, Alistair > > Regards, > Peter > >> +#define EP108_HIGH_RAM_START 0x800000000ull >> >> static struct arm_boot_info xlnx_ep108_binfo; >> >> static void xlnx_ep108_init(MachineState *machine) >> { >> XlnxEP108 *s = g_new0(XlnxEP108, 1); >> + ram_addr_t ddr_low_size, ddr_high_size; >> Error *err = NULL; >> >