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

[Peter Crosthwaite]

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?

Regards,
Peter

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