On Tue, Jun 30, 2015 at 12:42 PM, Peter Maydell
<peter.mayd...@linaro.org> wrote:
> On 30 June 2015 at 20:01, Peter Crosthwaite
> <peter.crosthwa...@xilinx.com> wrote:
>> On Tue, Jun 30, 2015 at 6:07 AM, Peter Maydell <peter.mayd...@linaro.org>
>> wrote:
>>> If our builtin kernel bootloader is directly booting a kernel in
>>> the NonSecure world, then we must configure the GIC to put all
>>> the IRQs into the NonSecure group. (By default all interrupts
>>> are configured to be Secure on reset, which means that a NonSecure
>>> guest kernel cannot use any of them.) This job would usually
>>> be done by the Secure boot firmware, but our builtin bootloader
>>> is doing the job of firmware.
>>>
>>> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
>>> ---
>>> hw/arm/boot.c | 39 +++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 39 insertions(+)
>>>
>>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>>> index 1e7fd28..3974499 100644
>>> --- a/hw/arm/boot.c
>>> +++ b/hw/arm/boot.c
>>> @@ -13,6 +13,7 @@
>>> #include "sysemu/sysemu.h"
>>> #include "hw/boards.h"
>>> #include "hw/loader.h"
>>> +#include "hw/intc/arm_gic_common.h"
>>> #include "elf.h"
>>> #include "sysemu/device_tree.h"
>>> #include "qemu/config-file.h"
>>> @@ -557,6 +558,33 @@ static void load_image_to_fw_cfg(FWCfgState *fw_cfg,
>>> uint16_t size_key,
>>> fw_cfg_add_bytes(fw_cfg, data_key, data, size);
>>> }
>>>
>>> +static int find_gics(Object *obj, void *opaque)
>>> +{
>>> + GICState *gic = (GICState *)object_dynamic_cast(obj,
>>> TYPE_ARM_GIC_COMMON);
>>> + bool has_sec_extns;
>>> +
>>> + if (!gic) {
>>> + /* Might be a container, traverse it for children */
>>> + return object_child_foreach(obj, find_gics, opaque);
>>> + }
>>
>> This need to traverse the whole tree has come up more than once for
>> me, so I think this should be a core feature of QOM.
>
> I did think about that, yeah. I guess something like
> object_descendants_foreach(), same signature as
> object_child_foreach(), same semantics except it also
> iterates through the whole tree (and calls the callback
> for the nodes-with-children as well as the leaves) ?
>
>>> +
>>> + has_sec_extns = object_property_get_bool(obj,
>>> "has-security-extensions",
>>> + &error_abort);
>>> + if (has_sec_extns) {
>>> + object_property_set_bool(obj, true, "irqs-reset-nonsecure",
>>> + &error_abort);
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +static void reconfigure_gics_nonsecure(void)
>>> +{
>>> + /* Find every GIC in the system and tell it to reconfigure
>>> + * itself with interrupts as NonSecure.
>>> + */
>>> + object_child_foreach(qdev_get_machine(), find_gics, NULL);
>>> +}
>>> +
>>> static void arm_load_kernel_notify(Notifier *notifier, void *data)
>>> {
>>> CPUState *cs;
>>> @@ -767,6 +795,17 @@ static void arm_load_kernel_notify(Notifier *notifier,
>>> void *data)
>>> }
>>> info->is_linux = is_linux;
>>>
>>> + if (is_linux && arm_feature(&cpu->env, ARM_FEATURE_EL3) &&
>>
>> Do we need to conditional on ARM_FEATURE_EL3 or can we make GIC logic
>> independent of the primary CPU?
>
> The point here is that "do we need to do this" is exactly
> dependent on what we're doing with the CPU. Only if we
> want to put the guest into NS do we do this, and the
> condition for "are we going to put the guest into NS"
> is "is this a Linux boot on a CPU with EL3 but where
> the board says don't boot in S". It matches what the
> existing logic does for when it sets the SCR_NS bit in
> do_cpu_reset() in this file.
>
Then maybe this belongs on the lowest common denominator for GIC and
CPU - the SoC level. SoCs can register a linux_init function that
checks the CPU and GIC NS support and does the switchup. Can make it a
common function that multiple socs/machines can call (virt, vexpress
and zynq-mp so far I think). For cases where the linux_init in
genuinely self contained (like my zynq SLCR case), the device can
register it.
>>> + !info->secure_boot) {
>>> + /* We're directly booting a kernel into NonSecure. If the system
>>> + * has a GIC which implements the security extensions then we must
>>> + * configure it to have all the interrupts be NonSecure (this is
>>> + * a job that is done by the Secure boot firmware, and boot.c is
>>> + * a minimalist firmware-and-boot-loader equivalent).
>>> + */
>>
>> So I actually had my own patches for this one that went in a different
>> direction. The reason is, there are also other devs out there which
>> need post-firmware state setup. The one I an thinking of mainly is the
>> Zynq SLCR setup for non-firmware boots, I.E. the Linux kernel expects
>> firmware to setup devices to some sort of initialized state (mainly
>> turning clocks on). I think this GIC setup falls in the same category.
>> The third use case is the GIC_CPU_IF stuff currently managed by
>> machine code in boot.c that could be outsourced to GIC (in either a
>> similar way to what is done here, or more fully outsourced using my
>> new API).
>
> I'm a bit torn here -- I don't want to make it *too* easy for
> people to add linux-booting specific code to random devices,
> as this will lead to the bootloader code having its tentacles
> everywhere within the tree...
>
Maybe the compromise to to restrict this API to SoCs and machines and
I can handle my SLCR case with a single "post-firmware" boolean
property?
Regards,
Peter
> thanks
> -- PMM
>
