Re: [Qemu-devel] [RFC v0 4/4] arm: boot: Add support for Linux specific boot devs

[Peter Crosthwaite]

On Tue, Jun 30, 2015 at 12:45 PM, Peter Maydell
<address@hidden> wrote:
> On 30 June 2015 at 20:21, Peter Crosthwaite
> <address@hidden> wrote:
>> If booting Linux, call the Linux specific init routine for all devs
>> that support it.
>>
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>>
>> ---
>> Doesn't solve the problem of conditional setup, e.g. GIC needs to only
>> do NS setup on NS boot. I think this should be solved by passing the
>> boot_info to the GIC as opaque data.
>> ---
>>  hw/arm/boot.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index 1e7fd28..2cf0dcb 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -18,6 +18,8 @@
>>  #include "qemu/config-file.h"
>>  #include "exec/address-spaces.h"
>>
>> +#include "hw/guest/linux.h"
>> +
>>  /* Kernel boot protocol is specified in the kernel docs
>>   * Documentation/arm/Booting and Documentation/arm64/booting.txt
>>   * They have different preferred image load offsets from system RAM base.
>> @@ -442,6 +444,19 @@ fail:
>>      return -1;
>>  }
>>
>> +static int do_linux_dev_init(Object *obj, void *opaue)
>> +{
>> +    if (object_dynamic_cast(obj, TYPE_LINUX_DEVICE)) {
>> +        LinuxDevice *ld = LINUX_DEVICE(obj);
>> +        LinuxDeviceClass *ldc = LINUX_DEVICE_GET_CLASS(obj);
>> +
>> +        if (ldc->linux_init) {
>> +            ldc->linux_init(ld);
>> +        }
>> +    }
>> +    return 0;
>> +}
>> +
>>  static void do_cpu_reset(void *opaque)
>>  {
>>      ARMCPU *cpu = opaque;
>> @@ -504,8 +519,11 @@ static void do_cpu_reset(void *opaque)
>>              } else {
>>                  info->secondary_cpu_reset_hook(cpu, info);
>>              }
>> +            object_child_foreach_recursive(object_get_root(),
>> +                                           do_linux_dev_init, NULL);
>>          }
>>      }
>> +
>
> This will call the device hooks once per CPU in the system,
> which doesn't seem right... Also, isn't this reintroducing
> a reset-order dependency, where we now rely on the CPU
> reset happening after any devices that implement this
> hook (otherwise the device reset will override any changes
> made by the linux_init hook)?
>

Yes. Although your patches may provide to solution, as in your patch,
as GIC just captures a flag in your linux_init equivalent. This means
this iterator is misplaced and should happen at _notify rather than
reset. That could be the correct API semantic - must be called before
reset.

Regards,
Peter

> -- PMM
>