qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v1 1/1] xlnx-ep108: Add support for high DDR mem


From: Alistair Francis
Subject: Re: [Qemu-devel] [PATCH v1 1/1] xlnx-ep108: Add support for high DDR memory regions
Date: Wed, 16 Dec 2015 11:08:01 -0800

On Tue, Nov 24, 2015 at 10:33 PM, Peter Crosthwaite
<address@hidden> wrote:
> On Mon, Nov 23, 2015 at 9:00 PM, Alistair Francis
> <address@hidden> 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 <address@hidden>
>> ---
>> 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;
>>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]