Re: [Qemu-devel] [PATCH v3 2/8] arm/boot: Use qemu_devtree_setprop_sized_cells()

[Peter Maydell]

On 16 July 2013 15:31, Peter Crosthwaite <address@hidden> wrote:
> Hi Peter,
>
> On Tue, Jul 16, 2013 at 10:25 PM, Peter Maydell
> <address@hidden> 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 <address@hidden>
>> ---
>>  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.

(hwaddr are always 64 bits for everybody now.)

-- PMM