Re: [Qemu-devel] [PATCH target-arm v4 14/16] arm: xilinx-ep108: Add external RAM

[Peter Crosthwaite <peter.crosthwaite@xilinx.com>]

On Thu, Apr 23, 2015 at 11:12 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 23 March 2015 at 11:05, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> Zynq MPSoC supports external DDR RAM. Add a RAM at 0 to the model.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
>> changed since v1:
>> Add ram size clamps and warnings
>>
>>  hw/arm/xlnx-ep108.c | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
>> index 81704bb..6e89456 100644
>> --- a/hw/arm/xlnx-ep108.c
>> +++ b/hw/arm/xlnx-ep108.c
>> @@ -18,11 +18,16 @@
>>  #include "hw/arm/xlnx-zynqmp.h"
>>  #include "hw/boards.h"
>>  #include "qemu/error-report.h"
>> +#include "exec/address-spaces.h"
>>
>>  typedef struct XlnxEP108 {
>>      XlnxZynqMPState soc;
>> +    MemoryRegion ddr_ram;
>>  } XlnxEP108;
>>
>> +/* Max 2GB RAM */
>> +#define EP108_MAX_RAM_SIZE 0x80000000ull
>> +
>>  static void xlnx_ep108_init(MachineState *machine)
>>  {
>>      XlnxEP108 *s = g_new0(XlnxEP108, 1);
>> @@ -37,6 +42,22 @@ static void xlnx_ep108_init(MachineState *machine)
>>          error_report("%s", error_get_pretty(err));
>>          exit(1);
>>      }
>> +
>> +    if (machine->ram_size > EP108_MAX_RAM_SIZE) {
>> +        error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, "
>> +                     "reduced to %llx", machine->ram_size, EP108_MAX_RAM_SIZE);
>> +        machine->ram_size = EP108_MAX_RAM_SIZE;
>> +    }
>> +
>> +    if (machine->ram_size <= 0x08000000) {
>> +        error_report("WARNING: RAM size " RAM_ADDR_FMT " is small for EP108\n",
>> +                     machine->ram_size);
>
> error_report() strings don't need the trailing \n.
>

Actually error_report is too noisy for this, so changing to qemu_log.
As-is, this comes up in make check.

>> +    }
>> +
>> +    memory_region_init_ram(&s->ddr_ram, NULL, "ddr-ram", machine->ram_size,
>> +                           &error_abort);
>> +    vmstate_register_ram_global(&s->ddr_ram);
>
> This should probably be using memory_region_allocate_system_memory().
>

Fixed.

Regards,
Peter

>> +    memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
>>  }
>>
>>  static QEMUMachine xlnx_ep108_machine = {
>
> -- PMM
>