| Message ID | 20190109152430.32359-1-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Allow to switch from MON->HYP on AArch32 | expand |
Alexander Graf <agraf@suse.de> writes: > In U-boot, we switch from S-SVC -> MON -> HYP when we want to enter > HYP mode. This dance seems to work ok (hence it's there in the code > base), but breaks with current QEMU. What EL is MON in this case? EL3? In which case I'm confused by the terminology as the ARM ARM states: The principles of the ARMv8-A security model are: • If the implementation includes EL3, then it has two Security states, Secure and Non-secure, and: — EL3 exists only in Secure state. — A change from Non-secure state to Secure state can only occur on taking an exception to EL3. — A change from Secure state to Non-secure state can only occur on an exception return from EL3. We don't currently implement ARMv8.4-SecEL2 but that adds secure EL2 but as you can only switch security state in in/out of EL3 you have to be go to the secure monitor before you enter it. > The reason seems to be that we try to see whether we are entering > HYP mode from the NS side. However, MON is always considered secure, > so we always fall into an error case when going MON->HYP and never > manage to actually do the switch. > > Fix this by not using a different helper function that does not bork > when we're in MON state, as that switch is supposed to work. > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > target/arm/helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index f00c141ef9..9bf8fbd8f9 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -6297,7 +6297,7 @@ static int bad_mode_switch(CPUARMState *env, int mode, CPSRWriteType write_type) > return 0; > case ARM_CPU_MODE_HYP: > return !arm_feature(env, ARM_FEATURE_EL2) > - || arm_current_el(env) < 2 || arm_is_secure(env); > + || arm_current_el(env) < 2 || > arm_is_secure_below_el3(env); This seems to violate the rule that you can enter a NS state from anything other than EL3. > case ARM_CPU_MODE_MON: > return arm_current_el(env) < 3; > default: -- Alex Bennée
On 01/09/2019 05:10 PM, Alex Bennée wrote: > Alexander Graf <agraf@suse.de> writes: > >> In U-boot, we switch from S-SVC -> MON -> HYP when we want to enter >> HYP mode. This dance seems to work ok (hence it's there in the code >> base), but breaks with current QEMU. > What EL is MON in this case? EL3? In which case I'm confused by the > terminology as the ARM ARM states: > > The principles of the ARMv8-A security model are: > • If the implementation includes EL3, then it has two Security states, Secure and Non-secure, and: > — EL3 exists only in Secure state. > — A change from Non-secure state to Secure state can only occur on taking an exception to EL3. > — A change from Secure state to Non-secure state can only occur on an exception return from EL3. > > We don't currently implement ARMv8.4-SecEL2 but that adds secure EL2 but > as you can only switch security state in in/out of EL3 you have to be go > to the secure monitor before you enter it. Right - and exactly that switch seems to be prohibited here. > >> The reason seems to be that we try to see whether we are entering >> HYP mode from the NS side. However, MON is always considered secure, >> so we always fall into an error case when going MON->HYP and never >> manage to actually do the switch. >> >> Fix this by not using a different helper function that does not bork >> when we're in MON state, as that switch is supposed to work. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> target/arm/helper.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/target/arm/helper.c b/target/arm/helper.c >> index f00c141ef9..9bf8fbd8f9 100644 >> --- a/target/arm/helper.c >> +++ b/target/arm/helper.c >> @@ -6297,7 +6297,7 @@ static int bad_mode_switch(CPUARMState *env, int mode, CPSRWriteType write_type) >> return 0; >> case ARM_CPU_MODE_HYP: >> return !arm_feature(env, ARM_FEATURE_EL2) >> - || arm_current_el(env) < 2 || arm_is_secure(env); >> + || arm_current_el(env) < 2 || >> arm_is_secure_below_el3(env); > This seems to violate the rule that you can enter a NS state from > anything other than EL3. How so? This statement basically says "Only return to HYP from NS-HYP or EL3" now. Before it said "Only return to HYP from HYP". Alex > >> case ARM_CPU_MODE_MON: >> return arm_current_el(env) < 3; >> default: > > -- > Alex Bennée >
On Wed, 9 Jan 2019 at 16:11, Alex Bennée <alex.bennee@linaro.org> wrote: > > > Alexander Graf <agraf@suse.de> writes: > > > In U-boot, we switch from S-SVC -> MON -> HYP when we want to enter > > HYP mode. This dance seems to work ok (hence it's there in the code > > base), but breaks with current QEMU. > > What EL is MON in this case? EL3? Mon is an AArch32 term and EL is really AArch64, but in practice Mon is always (Secure) EL3. Note that if EL3 is AArch32 then S-SVC is *also* EL3 ! thanks -- PMM
On Wed, 9 Jan 2019 at 15:26, Alexander Graf <agraf@suse.de> wrote: > > In U-boot, we switch from S-SVC -> MON -> HYP when we want to enter > HYP mode. This dance seems to work ok (hence it's there in the code > base), but breaks with current QEMU. > > The reason seems to be that we try to see whether we are entering > HYP mode from the NS side. However, MON is always considered secure, > so we always fall into an error case when going MON->HYP and never > manage to actually do the switch. > > Fix this by not using a different helper function that does not bork > when we're in MON state, as that switch is supposed to work. > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > target/arm/helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index f00c141ef9..9bf8fbd8f9 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -6297,7 +6297,7 @@ static int bad_mode_switch(CPUARMState *env, int mode, CPSRWriteType write_type) > return 0; > case ARM_CPU_MODE_HYP: > return !arm_feature(env, ARM_FEATURE_EL2) > - || arm_current_el(env) < 2 || arm_is_secure(env); > + || arm_current_el(env) < 2 || arm_is_secure_below_el3(env); > case ARM_CPU_MODE_MON: > return arm_current_el(env) < 3; > default: > -- This took me a while to figure out, because the bad_mode_switch() code is called for doing checks in two situations: (1) changes to mode by instructions writing to CPSR.M (ie not exception take/return) -- this corresponds to the Armv8 Arm ARM pseudocode AArch32.WriteModeByInstr, and using this to enter or leave Hyp mode is forbidden in v8 and UNPREDICTABLE in v7 (2) changes to mode by exception return Changing to or from Hyp by writing CPSR.M is forbidden, but we catch that already at the top of the bad_mode_switch() function. So what we need to consider here is the exception-return case. An exception return from Mon to Hyp with the NS bit 0 is OK. So this change is the right thing. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On Wed, 9 Jan 2019 at 16:52, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Wed, 9 Jan 2019 at 15:26, Alexander Graf <agraf@suse.de> wrote: > > > > In U-boot, we switch from S-SVC -> MON -> HYP when we want to enter > > HYP mode. This dance seems to work ok (hence it's there in the code > > base), but breaks with current QEMU. PS: it would be helpful if the commit message said how u-boot is trying to go from Mon to Hyp -- some ways to try to do this are OK, and some are not, so whether it's supposed to work or not depends on what u-boot is actually doing... thanks -- PMM
On 01/09/2019 05:59 PM, Peter Maydell wrote: > On Wed, 9 Jan 2019 at 16:52, Peter Maydell <peter.maydell@linaro.org> wrote: >> On Wed, 9 Jan 2019 at 15:26, Alexander Graf <agraf@suse.de> wrote: >>> In U-boot, we switch from S-SVC -> MON -> HYP when we want to enter >>> HYP mode. This dance seems to work ok (hence it's there in the code >>> base), but breaks with current QEMU. > PS: it would be helpful if the commit message said how u-boot > is trying to go from Mon to Hyp -- some ways to try to do > this are OK, and some are not, so whether it's supposed to > work or not depends on what u-boot is actually doing... I don't fully understand all of it to be honest :). But the code is here: http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/nonsec_virt.S What I managed to understand so far is that it goes to MON using the smc #0 call and then changes SPSR so that on return (movs pc) the mode will be different. But please double check whether this is supposed to work - I don't find mode switching on ARM terribly intuitive ;). Alex
On Wed, 9 Jan 2019 at 17:14, Alexander Graf <agraf@suse.de> wrote: > > On 01/09/2019 05:59 PM, Peter Maydell wrote: > > On Wed, 9 Jan 2019 at 16:52, Peter Maydell <peter.maydell@linaro.org> wrote: > >> On Wed, 9 Jan 2019 at 15:26, Alexander Graf <agraf@suse.de> wrote: > >>> In U-boot, we switch from S-SVC -> MON -> HYP when we want to enter > >>> HYP mode. This dance seems to work ok (hence it's there in the code > >>> base), but breaks with current QEMU. > > PS: it would be helpful if the commit message said how u-boot > > is trying to go from Mon to Hyp -- some ways to try to do > > this are OK, and some are not, so whether it's supposed to > > work or not depends on what u-boot is actually doing... > > I don't fully understand all of it to be honest :). But the code is here: > > http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/nonsec_virt.S > > What I managed to understand so far is that it goes to MON using the smc > #0 call and then changes SPSR so that on return (movs pc) the mode will > be different. Thanks -- yes, that's an exception return so it's the expected way to go from Mon to Hyp. -- PMM
On 10.01.19 00:08, Peter Maydell wrote: > On Wed, 9 Jan 2019 at 17:14, Alexander Graf <agraf@suse.de> wrote: >> >> On 01/09/2019 05:59 PM, Peter Maydell wrote: >>> On Wed, 9 Jan 2019 at 16:52, Peter Maydell <peter.maydell@linaro.org> wrote: >>>> On Wed, 9 Jan 2019 at 15:26, Alexander Graf <agraf@suse.de> wrote: >>>>> In U-boot, we switch from S-SVC -> MON -> HYP when we want to enter >>>>> HYP mode. This dance seems to work ok (hence it's there in the code >>>>> base), but breaks with current QEMU. >>> PS: it would be helpful if the commit message said how u-boot >>> is trying to go from Mon to Hyp -- some ways to try to do >>> this are OK, and some are not, so whether it's supposed to >>> work or not depends on what u-boot is actually doing... >> >> I don't fully understand all of it to be honest :). But the code is here: >> >> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/nonsec_virt.S >> >> What I managed to understand so far is that it goes to MON using the smc >> #0 call and then changes SPSR so that on return (movs pc) the mode will >> be different. > > Thanks -- yes, that's an exception return so it's the > expected way to go from Mon to Hyp. That was my understanding, yes. Do you still want me to change the commit message to mention that or will you just do it when applying? Thanks, Alex
On Thu, 10 Jan 2019 at 06:43, Alexander Graf <agraf@suse.de> wrote: > > > > On 10.01.19 00:08, Peter Maydell wrote: > > On Wed, 9 Jan 2019 at 17:14, Alexander Graf <agraf@suse.de> wrote: > >> > >> On 01/09/2019 05:59 PM, Peter Maydell wrote: > >>> On Wed, 9 Jan 2019 at 16:52, Peter Maydell <peter.maydell@linaro.org> wrote: > >>>> On Wed, 9 Jan 2019 at 15:26, Alexander Graf <agraf@suse.de> wrote: > >>>>> In U-boot, we switch from S-SVC -> MON -> HYP when we want to enter > >>>>> HYP mode. This dance seems to work ok (hence it's there in the code > >>>>> base), but breaks with current QEMU. > >>> PS: it would be helpful if the commit message said how u-boot > >>> is trying to go from Mon to Hyp -- some ways to try to do > >>> this are OK, and some are not, so whether it's supposed to > >>> work or not depends on what u-boot is actually doing... > >> > >> I don't fully understand all of it to be honest :). But the code is here: > >> > >> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/nonsec_virt.S > >> > >> What I managed to understand so far is that it goes to MON using the smc > >> #0 call and then changes SPSR so that on return (movs pc) the mode will > >> be different. > > > > Thanks -- yes, that's an exception return so it's the > > expected way to go from Mon to Hyp. > > That was my understanding, yes. Do you still want me to change the > commit message to mention that or will you just do it when applying? I'll add a note when I apply it; thanks. -- PMM
On Thu, 10 Jan 2019 at 10:34, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 10 Jan 2019 at 06:43, Alexander Graf <agraf@suse.de> wrote: > > > > > > > > On 10.01.19 00:08, Peter Maydell wrote: > > > On Wed, 9 Jan 2019 at 17:14, Alexander Graf <agraf@suse.de> wrote: > > >> > > >> On 01/09/2019 05:59 PM, Peter Maydell wrote: > > >>> On Wed, 9 Jan 2019 at 16:52, Peter Maydell <peter.maydell@linaro.org> wrote: > > >>>> On Wed, 9 Jan 2019 at 15:26, Alexander Graf <agraf@suse.de> wrote: > > >>>>> In U-boot, we switch from S-SVC -> MON -> HYP when we want to enter > > >>>>> HYP mode. This dance seems to work ok (hence it's there in the code > > >>>>> base), but breaks with current QEMU. > > >>> PS: it would be helpful if the commit message said how u-boot > > >>> is trying to go from Mon to Hyp -- some ways to try to do > > >>> this are OK, and some are not, so whether it's supposed to > > >>> work or not depends on what u-boot is actually doing... > > >> > > >> I don't fully understand all of it to be honest :). But the code is here: > > >> > > >> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/nonsec_virt.S > > >> > > >> What I managed to understand so far is that it goes to MON using the smc > > >> #0 call and then changes SPSR so that on return (movs pc) the mode will > > >> be different. > > > > > > Thanks -- yes, that's an exception return so it's the > > > expected way to go from Mon to Hyp. > > > > That was my understanding, yes. Do you still want me to change the > > commit message to mention that or will you just do it when applying? > > I'll add a note when I apply it; thanks. Now applied to target-arm.next with a rewritten commit message; thanks. -- PMM
diff --git a/target/arm/helper.c b/target/arm/helper.c index f00c141ef9..9bf8fbd8f9 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6297,7 +6297,7 @@ static int bad_mode_switch(CPUARMState *env, int mode, CPSRWriteType write_type) return 0; case ARM_CPU_MODE_HYP: return !arm_feature(env, ARM_FEATURE_EL2) - || arm_current_el(env) < 2 || arm_is_secure(env); + || arm_current_el(env) < 2 || arm_is_secure_below_el3(env); case ARM_CPU_MODE_MON: return arm_current_el(env) < 3; default:
In U-boot, we switch from S-SVC -> MON -> HYP when we want to enter HYP mode. This dance seems to work ok (hence it's there in the code base), but breaks with current QEMU. The reason seems to be that we try to see whether we are entering HYP mode from the NS side. However, MON is always considered secure, so we always fall into an error case when going MON->HYP and never manage to actually do the switch. Fix this by not using a different helper function that does not bork when we're in MON state, as that switch is supposed to work. Signed-off-by: Alexander Graf <agraf@suse.de> --- target/arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)