Hi Peter,
On Wed, Jul 17, 2013 at 12:43 AM, Peter Maydell
<peter.maydell@xxxxxxxxxx> wrote:
> On 16 July 2013 15:31, Peter Crosthwaite <peter.crosthwaite@xxxxxxxxxx> wrote:
>> Hi Peter,
>>
>> On Tue, Jul 16, 2013 at 10:25 PM, Peter Maydell
>> <peter.maydell@xxxxxxxxxx> wrote:
>>> Replace the opencoded assembly of the reg property array for the
>>> /memory node with a call to qemu_devtree_setprop_sized_cells().
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@xxxxxxxxxx>
>>> ---
>>> hw/arm/boot.c | 29 ++++++++---------------------
>>> 1 file changed, 8 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>>> index a2e4032..1780316 100644
>>> --- a/hw/arm/boot.c
>>> +++ b/hw/arm/boot.c
>>> @@ -227,12 +227,10 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
>>>
>>> static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
>>> {
>>> - uint32_t *mem_reg_property;
>>> - uint32_t mem_reg_propsize;
>>> void *fdt = NULL;
>>> char *filename;
>>> int size, rc;
>>> - uint32_t acells, scells, hival;
>>> + uint32_t acells, scells;
>>>
>>> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
>>> if (!filename) {
>>> @@ -255,29 +253,18 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
>>> goto fail;
>>> }
>>>
>>> - mem_reg_propsize = acells + scells;
>>> - mem_reg_property = g_new0(uint32_t, mem_reg_propsize);
>>> - mem_reg_property[acells - 1] = cpu_to_be32(binfo->loader_start);
>>> - hival = cpu_to_be32(binfo->loader_start >> 32);
>>> - if (acells > 1) {
>>> - mem_reg_property[acells - 2] = hival;
>>> - } else if (hival != 0) {
>>> - fprintf(stderr, "qemu: dtb file not compatible with "
>>> - "RAM start address > 4GB\n");
>>> - goto fail;
>>> - }
>>
>> So it confused me for a while as to why this check is deleted (and not
>> converted), but I'm guessing it is because binfo->loader_start is a
>> hwaddr which is probably 32 bit? Which I guess would cause a check
>> equivalent to the one below to werror. Is it possible in an arm build
>> for hwaddr to be 64 bit and if so should this check be converted?
>
> It's because the "ram start address won't fit" is basically a
> bug in the implementation of a QEMU machine -- it will basically
> only fire for a board which put its RAM all beyond the 4GB
> boundary (vanishingly unlikely as it would be a really stupid
> bit of h/w design) *and* where the user had a DTB with a one-cell
> address size field. So I'm happy to have that fail via the
> call to set_sized_cells() failing, without calling it out as
> a specific error message.
>
Makes sense. If you respin maybe its worth a mention in commit message
as its more that just a conversion of existing behaviour? But
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xxxxxxxxxx>
Regards,
Peter
> (hwaddr are always 64 bits for everybody now.)
>
> -- PMM
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
