[RFC,v0,4/4] arm: boot: Add support for Linux specific boot devs

If booting Linux, call the Linux specific init routine for all devs
that support it.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

---
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(+)

On 30 June 2015 at 20:21, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> If booting Linux, call the Linux specific init routine for all devs
> that support it.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> ---
> 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)?

-- PMM

On Tue, Jun 30, 2015 at 12:45 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 30 June 2015 at 20:21, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> If booting Linux, call the Linux specific init routine for all devs
>> that support it.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>> ---
>> 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
>