| Message ID | 1399997768-32014-2-git-send-email-aggelerf@ethz.ch |
|---|---|
| State | New |
| Headers | show |
On 13 May 2014 17:15, Fabian Aggeler <aggelerf@ethz.ch> wrote: > From: Svetlana Fedoseeva <s.fedoseeva@samsung.com> > > Define Security Extensions CPU feature. Set that feature for relevant CPUs. This is potentially tricky because it means that CPUs which we were previously implementing without TZ now boot up with TZ and in Secure mode; I think this is probably the right thing but we'll need to check that we don't break guests which were really expecting to run in NS. (Also I'm not sure what the semantics of -kernel should be for TZ-supporting CPUs -- boot the kernel in S or NS ?) thanks -- PMM
On 13 May 2014 17:15, Fabian Aggeler <aggelerf@ethz.ch> wrote: > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -631,6 +631,7 @@ enum arm_features { > ARM_FEATURE_CBAR, /* has cp15 CBAR */ > ARM_FEATURE_CRC, /* ARMv8 CRC instructions */ > ARM_FEATURE_CBAR_RO, /* has cp15 CBAR and it is read-only */ > + ARM_FEATURE_SECURITY_EXTENSIONS, /* has Security Extensions */ > }; Also this feature name is pretty long and unwieldy; something shorter would be nice. I think we should probably go with ARM_FEATURE_EL2/ARM_FEATURE_EL3 as per Edgar's patchset. thanks -- PMM
Hi Peter, On 05/21/2014 10:46 AM, Peter Maydell wrote: > On 13 May 2014 17:15, Fabian Aggeler <aggelerf@ethz.ch> wrote: >> From: Svetlana Fedoseeva <s.fedoseeva@samsung.com> >> >> Define Security Extensions CPU feature. Set that feature for relevant CPUs. > > This is potentially tricky because it means that CPUs which we were > previously implementing without TZ now boot up with TZ and in > Secure mode; I think this is probably the right thing but we'll need > to check that we don't break guests which were really expecting > to run in NS. > > (Also I'm not sure what the semantics of -kernel should be for > TZ-supporting CPUs -- boot the kernel in S or NS ?) While Linux works in secure mode, non-secure hypervisor mode is required for KVM to work in the guest. "[Entry] in HYP mode ... is the recommended boot method ...." http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm/Booting#n183 Christopher
On 21.05.2014 20:14, Christopher Covington wrote: > Hi Peter, > > On 05/21/2014 10:46 AM, Peter Maydell wrote: >> On 13 May 2014 17:15, Fabian Aggeler <aggelerf@ethz.ch> wrote: >>> From: Svetlana Fedoseeva <s.fedoseeva@samsung.com> >>> >>> Define Security Extensions CPU feature. Set that feature for relevant CPUs. >> This is potentially tricky because it means that CPUs which we were >> previously implementing without TZ now boot up with TZ and in >> Secure mode; I think this is probably the right thing but we'll need >> to check that we don't break guests which were really expecting >> to run in NS. >> >> (Also I'm not sure what the semantics of -kernel should be for >> TZ-supporting CPUs -- boot the kernel in S or NS ?) > While Linux works in secure mode, non-secure hypervisor mode is required for > KVM to work in the guest. > > "[Entry] in HYP mode ... is the recommended boot method ...." > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm/Booting#n183 > > Christopher > AFAIK, in real hardware this switch to non-secure state is actually done by bootloader. Why don't implement this in Qemu bootloader stub so far? Regards, Sergey Fedorov.
On 21 May 2014 17:33, Sergey Fedorov <serge.fdrv@gmail.com> wrote: > On 21.05.2014 20:14, Christopher Covington wrote: >> On 05/21/2014 10:46 AM, Peter Maydell wrote: >>> (Also I'm not sure what the semantics of -kernel should be for >>> TZ-supporting CPUs -- boot the kernel in S or NS ?) >> While Linux works in secure mode, non-secure hypervisor mode is required for >> KVM to work in the guest. >> >> "[Entry] in HYP mode ... is the recommended boot method ...." >> >> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm/Booting#n183 Sure, but we don't implement HYP yet :-) > AFAIK, in real hardware this switch to non-secure state is actually done > by bootloader. Why don't implement this in Qemu bootloader stub so far? If we want to have -kernel boot in NS then yes, the bootloader stub is the place that code should go. The difficulty with -kernel being NS is that some guest kernels for some boards may be assuming that they will run in secure state and can directly access h/w or registers which aren't accessible from NS. I guess we need to implement it and then see if any of our guest images stop working... thanks -- PMM
On 21.05.2014 20:41, Peter Maydell wrote: > If we want to have -kernel boot in NS then yes, the bootloader > stub is the place that code should go. > > The difficulty with -kernel being NS is that some guest kernels > for some boards may be assuming that they will run in secure state > and can directly access h/w or registers which aren't accessible > from NS. I guess we need to implement it and then see if any of our > guest images stop working... Then maybe we can extend arm_boot_info structure to deal with this. Regards, Sergey Fedorov.
On 21 May 2014, at 16:51, Peter Maydell <peter.maydell@linaro.org> wrote: > On 13 May 2014 17:15, Fabian Aggeler <aggelerf@ethz.ch> wrote: >> --- a/target-arm/cpu.h >> +++ b/target-arm/cpu.h >> @@ -631,6 +631,7 @@ enum arm_features { >> ARM_FEATURE_CBAR, /* has cp15 CBAR */ >> ARM_FEATURE_CRC, /* ARMv8 CRC instructions */ >> ARM_FEATURE_CBAR_RO, /* has cp15 CBAR and it is read-only */ >> + ARM_FEATURE_SECURITY_EXTENSIONS, /* has Security Extensions */ >> }; > > Also this feature name is pretty long and unwieldy; something > shorter would be nice. I think we should probably go with > ARM_FEATURE_EL2/ARM_FEATURE_EL3 as per Edgar's > patchset. > > thanks > -- PMM Makes sense, I will use Edgar’s ARM_FEATURE_EL3 in the next version. Fabian
diff --git a/target-arm/cpu.c b/target-arm/cpu.c index c0ddc3e..b0d4a9b 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -527,6 +527,7 @@ static void arm1176_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS); set_feature(&cpu->env, ARM_FEATURE_CACHE_DIRTY_REG); set_feature(&cpu->env, ARM_FEATURE_CACHE_BLOCK_OPS); + set_feature(&cpu->env, ARM_FEATURE_SECURITY_EXTENSIONS); cpu->midr = 0x410fb767; cpu->reset_fpsid = 0x410120b5; cpu->mvfr0 = 0x11111111; @@ -613,6 +614,7 @@ static void cortex_a8_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_NEON); set_feature(&cpu->env, ARM_FEATURE_THUMB2EE); set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS); + set_feature(&cpu->env, ARM_FEATURE_SECURITY_EXTENSIONS); cpu->midr = 0x410fc080; cpu->reset_fpsid = 0x410330c0; cpu->mvfr0 = 0x11110222; @@ -679,6 +681,7 @@ static void cortex_a9_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_VFP_FP16); set_feature(&cpu->env, ARM_FEATURE_NEON); set_feature(&cpu->env, ARM_FEATURE_THUMB2EE); + set_feature(&cpu->env, ARM_FEATURE_SECURITY_EXTENSIONS); /* Note that A9 supports the MP extensions even for * A9UP and single-core A9MP (which are both different * and valid configurations; we don't model A9UP). @@ -746,6 +749,7 @@ static void cortex_a15_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS); set_feature(&cpu->env, ARM_FEATURE_CBAR_RO); set_feature(&cpu->env, ARM_FEATURE_LPAE); + set_feature(&cpu->env, ARM_FEATURE_SECURITY_EXTENSIONS); cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15; cpu->midr = 0x412fc0f1; cpu->reset_fpsid = 0x410430f0; diff --git a/target-arm/cpu.h b/target-arm/cpu.h index c83f249..5eba825 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -631,6 +631,7 @@ enum arm_features { ARM_FEATURE_CBAR, /* has cp15 CBAR */ ARM_FEATURE_CRC, /* ARMv8 CRC instructions */ ARM_FEATURE_CBAR_RO, /* has cp15 CBAR and it is read-only */ + ARM_FEATURE_SECURITY_EXTENSIONS, /* has Security Extensions */ }; static inline int arm_feature(CPUARMState *env, int feature)