[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH for-2.5 v3 1/3] arm: boot: Add secure_board_setup
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-arm] [PATCH for-2.5 v3 1/3] arm: boot: Add secure_board_setup flag |
Date: |
Sun, 8 Nov 2015 09:59:06 -0800 |
On Fri, Nov 6, 2015 at 6:45 AM, Peter Maydell <address@hidden> wrote:
>
> On 3 November 2015 at 04:30, Peter Crosthwaite
> <address@hidden> wrote:
> > Add a flag that when set, will cause the primary CPU to start in secure
> > mode, even if the overall boot is non-secure. This is useful for when
> > there is a board-setup blob that needs to run from secure mode, but
> > device and secondary CPU init should still be done as-normal for a non-
> > secure boot.
> >
> > Signed-off-by: Peter Crosthwaite <address@hidden>
> > ---
> > changed since v2:
> > Assert if running KVM and board_setup_secure is set
> >
> > hw/arm/boot.c | 8 +++++++-
> > include/hw/arm/arm.h | 6 ++++++
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> > index b0879a5..f671454 100644
> > --- a/hw/arm/boot.c
> > +++ b/hw/arm/boot.c
> > @@ -495,7 +495,8 @@ static void do_cpu_reset(void *opaque)
> > }
> >
> > /* Set to non-secure if not a secure boot */
> > - if (!info->secure_boot) {
> > + if (!info->secure_boot &&
> > + (cs != first_cpu || !info->secure_board_setup)) {
> > /* Linux expects non-secure state */
> > env->cp15.scr_el3 |= SCR_NS;
> > }
> > @@ -598,6 +599,11 @@ static void arm_load_kernel_notify(Notifier *notifier,
> > void *data)
> > struct arm_boot_info *info =
> > container_of(n, struct arm_boot_info, load_kernel_notifier);
> >
> > + /* It is the boards job to make sure secure_board_setup is actually
> > + * possible
> > + */
>
> I think we could improve this comment a bit:
> /* The board code is not supposed to set secure_board_setup unless
> * running its code in secure mode is actually possible, and KVM
> * doesn't support secure.
> */
>
Taken verbatim. Thanks.
> > + assert(!info->secure_board_setup || tcg_enabled());
>
> This assertion causes us to fail "make check", because there's
> a test target which starts every board with the "qtest" accelerator,
> which is not TCG. You need !kvm_enabled() instead (and
> to include sysemu/kvm.h).
>
Fixed. Changed to De Morgan && equivalent with the two !s for easier reading:
assert(!(info->secure_board_setup && kvm_enabled()));
Regards,
Peter
> > +
> > /* Load the kernel. */
> > if (!info->kernel_filename || info->firmware_loaded) {
> >
> > diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
> > index 9217b70..60dc919 100644
> > --- a/include/hw/arm/arm.h
> > +++ b/include/hw/arm/arm.h
> > @@ -97,6 +97,12 @@ struct arm_boot_info {
> > hwaddr board_setup_addr;
> > void (*write_board_setup)(ARMCPU *cpu,
> > const struct arm_boot_info *info);
> > +
> > + /* If set, the board specific loader/setup blob will be run from secure
> > + * mode, regardless of secure_boot. The blob becomes responsible for
> > + * changing to non-secure state if implementing a non-secure boot
> > + */
> > + bool secure_board_setup;
> > };
>
> thanks
> -- PMM