| Message ID | bd4f60aa2f8ad0c5b739643459cd50b3bbe14d9a.1446428067.git.crosthwaite.peter@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 3 November 2015 at 04:30, Peter Crosthwaite <crosthwaitepeter@gmail.com> 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 <crosthwaite.peter@gmail.com> > --- > 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. */ > + 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). > + > /* 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
On Fri, Nov 6, 2015 at 6:45 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > > On 3 November 2015 at 04:30, Peter Crosthwaite > <crosthwaitepeter@gmail.com> 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 <crosthwaite.peter@gmail.com> > > --- > > 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
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 + */ + assert(!info->secure_board_setup || tcg_enabled()); + /* 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; }; /**
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 <crosthwaite.peter@gmail.com> --- 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(-)