Re: [Qemu-devel] [PATCH v4 1/1] xlnx-zynqmp: Add support for high DDR memory regions

[Alistair Francis]

On Tue, Jan 12, 2016 at 2:15 PM, Peter Crosthwaite
<address@hidden> wrote:
> On Tue, Jan 12, 2016 at 2:07 PM, Alistair Francis
> <address@hidden> wrote:
>> On Tue, Jan 12, 2016 at 2:00 PM, Peter Crosthwaite
>> <address@hidden> wrote:
>>> On Tue, Jan 12, 2016 at 1:59 PM, Alistair Francis
>>> <address@hidden> wrote:
>>>> On Tue, Jan 12, 2016 at 1:01 AM, Peter Maydell <address@hidden> wrote:
>>>>> On 12 January 2016 at 00:24, Alistair Francis
>>>>> <address@hidden> wrote:
>>>>>> On Mon, Jan 11, 2016 at 8:04 AM, Peter Maydell <address@hidden> wrote:
>>>>>>> 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?
>>>>>
>>>>> I think you should do the same thing the other boards do anyway.
>>>>
>>>> Ok, I have changed it to exit instead of resize.
>>>>
>>>>>
>>>>>>> (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?
>>>>>
>>>>> %llx isn't right for uint64_t either :-)
>>>>
>>>> I still have a %llx for the macro as it isn't a ram_addr_t.
>>>>
>>>
>>> Should be PRIx64 though.
>>
>> Then I get a compiler warning as the macro is a long long unsigned int.
>>
>
> Ahh, the llx is for printing the constant, which hasn't really changed
> from original code anyway (other than a rename). I think the
> RAM_ADDR_FMT  goes to uint64_t and the llx stays?

Yep, that sounds good to me.

Thanks,

Alistair

>
> Regards,
> Peter
>
>> Thanks,
>>
>> Alistair
>>
>>>
>>> Regards,
>>> Peter
>>>
>>>> Re sending now.
>>>>
>>>> Thanks,
>>>>
>>>> Alistair
>>>>
>>>>>
>>>>> thanks
>>>>> -- PMM
>>>>>
>>>
>