Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AArch64: Rework secure states (NS vs S) discussion #32492

Closed
carlocaione opened this issue Feb 19, 2021 · 9 comments
Closed

AArch64: Rework secure states (NS vs S) discussion #32492

carlocaione opened this issue Feb 19, 2021 · 9 comments
Assignees
Labels
area: Trusted Execution Trusted Execution Enhancement Changes/Updates/Additions to existing features Feature Request A request for a new feature RFC Request For Comments: want input from the community

Comments

@carlocaione
Copy link
Collaborator

The current situation for the NS vs S story in Zephyr for AArch64 is very confusing (at lest to me) and I feel the urgency to fix it ASAP because the port is quickly growing in size and complexity.

This is the current status:

  1. When Zephyr is started from EL3 the SCR_EL3.NS is always set to 0.
    This is currently done on master:

    mov x0, xzr
    orr x0, x0, #(SCR_EL3_RW)
    msr scr_el3, x0

    This means that we drop into EL1.S without going through EL2.
    The same is done in the reset rework at aarch64: Refactor cpu.h and move reset routine to C #32394 but we go through EL2.S if this is implemented, see:
    https:/zephyrproject-rtos/zephyr/blob/6d7859bbce51a7cc428b616d5d20bf300ab8c558/arch/arm/core/aarch64/reset.c#L133

  2. We have in place a symbol CONFIG_ARMV8_A_NS that is doing basically three things: (a) setting up the GIC to deal with NS, (b) changing the MMU permission bit (b) forcing QEMU to start emulating from EL1. You usually want to use this symbol on real hardware when you have a secure monitor running already because the Zephyr entry point is usually done at EL1.


The point is that CONFIG_ARMV8_A_NS is not acting at all on the SCR_EL3.NS bit, that means that we are not actually changing the security state, we are only making the GIC playing nicely when Zephyr is entered at EL1.NS.

The first obvious change is to toggle SCR_EL3.NS in accordance with CONFIG_ARMV8_A_NS. But I want to discuss some other open points also because I'm not familiar with the nuances of NS vs S:

  1. Should CONFIG_ARMV8_A_NS also force QEMU to start from EL1 (as is today) or should we introduce some other symbol for that?
  2. What's the impact of this change on SMP? I'm referring to comments ARM64 SMP support #30676 (comment) and ARM64 SMP support #30676 (comment)
  3. Is there anything else to consider when using NS vs S besides the GIC configuration and MMU that I'm missing?
  4. Is QEMU going to play nicely for both NS and S?

Anyway before implementing any change I'm going to wait for #32394 to be merged.

@carlocaione carlocaione added the RFC Request For Comments: want input from the community label Feb 19, 2021
@carlocaione
Copy link
Collaborator Author

Tagging the relevant people: @npitre @jharris-intel @MrVan @ioannisg @stephanosio @Abhishek-brcm @sandeepbrcm

@carlocaione carlocaione added area: ARM_64 Enhancement Changes/Updates/Additions to existing features Feature Request A request for a new feature labels Feb 19, 2021
@carlocaione carlocaione self-assigned this Feb 19, 2021
@jharris-intel
Copy link
Contributor

For reference, with QEMU, there's actually more than two different possibilities there.

virtualization=on,secure=on -> QEMU has EL3.S / EL2.NS / EL1.NS / EL1.S / EL0.S / EL0.NS
virtualization=off,secure=on -> QEMU has EL3.S / EL1.NS / EL1.S / EL0.S / EL0.NS
virtualization=on,secure=off -> QEMU has EL2.NS / EL1.NS / EL0.NS
virtualization=off,secure=off -> QEMU has EL1.NS / EL0.NS (...ish. It's actually impdef as to if the single state here is NS or S, but it doesn't really matter anyway)

virtualization and secure in QEMU translate directly to the corresponding ARMv8 features.


EL3 is always secure, EL2 is always non-secure, so the real direct changes in the above are roughly:

  1. "Does EL3 exist"
  2. "Does EL2 exist"
  3. "Are we running EL1 in secure or nonsecure state"
  4. "Are we running EL0 in secure or nonsecure state"

...with the added wrinkle that switches between secure/nonsecure can only happen when going through EL3. (I wish ARM allowed a secure EL1 to directly interact with a nonsecure EL0...)

@jharris-intel
Copy link
Contributor

jharris-intel commented Feb 19, 2021

As for the impact of this and your questions...

Is there anything else to consider when using NS vs S besides the GIC configuration and MMU that I'm missing?

  1. GIC configuration, as you mention. Especially if you want FIQ support later. (Also, some things like timers have different interrupt ids for secure versus nonsecure, often.)
  2. MMU configuration, as you mention.
  3. Some processors support exporting secure/nonsecure state on the memory bus (e.g. for a defense-in-depth hardware measure to prevent someone who took over NS mode from accessing a security-critical region of memory).
  4. Many system registers are accessible or writable from secure mode only, or have banked copies for secure / nonsecure mode.
  5. Many system configuration bits are banked for controlling secure/nonsecure mode.

Is QEMU going to play nicely for both NS and S?

I think so, see my previous comment. You can get QEMU to start in all of the supported configurations, and between that and maybe writing a thin secure monitor for testing purposes you should be good.

Should CONFIG_ARMV8_A_NS also force QEMU to start from EL1 (as is today) or should we introduce some other symbol for that?

I think this should be more "we should introduce CPU flags for if the core (from our perspective) supports security / virtualization, on top of the existing CONFIG_ARMV8_A_NS / CONFIG_ARMV8_A_S".

So roughly:

Core supports the security extensions? Great, then we can choose between CONFIG_ARMV8_A_NS and CONFIG_ARMV8_A_S.
Core does not support the security extensions? Enforce CONFIG_ARMV8_A_NS.
CONFIG_ARMV8_A_N?S does not directly affect QEMU at all. It's purely a software/Zephyr flag of "should we run in secure or nonsecure mode".

(With a wrinkle here that it's not so much "core supports" as "we have control over". You could have a core with EL3, running a secure monitor outside our control, where Zephyr is only in control of EL1 and below...)

There is one other long-tail "nice-to-have", which is to write a quick secure monitor / hypervisor, to allow testing of such cases, but honestly I don't think it's worth it.

What's the impact of this change on SMP? I'm referring to comments #30676 (comment) and #30676 (comment)

Roughly speaking:

There is a (...mostly) standard power-management protocol for ARM cores. The defined protocol is to do SMCs, essentially. That is, it assumes that there is code on the board that receives this SMC and does the actual power management. This of course requires that either a) EL3 is not in Zephyr's control (because otherwise the SMC would go straight back to Zephyr!), or b) EL2 is not in Zephyr's control, and EL2 has set HCR_EL2.TSC to 1 to trap SMCs to EL2.

Annoyingly, PSCI is entirely defined for the non-secure world, and e.g. is designed to boot CPU cores into non-secure state. What this means is roughly that PSCI (the standard) only works if Zephyr is running in non-secure state, or if there's some platform-defined manner to switch between secure and non-secure state.

My suggestion for SMP CPU management here is to just define a standard interface for starting a core / stopping a core / suspending a core (optionally) / resuming a core, with the interface for AArch64 requiring that the core starts in the appropriate mode (the same EL and security state as Zephyr starts up in).

Then the standard PSCI interface can be an implementation of this interface, gated behind CONFIG_ARMV8_A_NS && ((CONFIG_CPU_HAS_SECURITY_EXTENSIONS && !CONFIG_ZEPHYR_CONTROLLED_EL3) || (CONFIG_CPU_HAS_VIRTUALIZATION_EXTENSIONS && !CONFIG_ZEPHYR_CONTROLLED_EL2)) (terrible names I know...)

@jharris-intel
Copy link
Contributor

As an aside, regardless of whichever PSCI decision is made, can we comply with the Linux kernel's devicetree spec for PSCI? https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/psci.txt

@carlocaione
Copy link
Collaborator Author

EL3 is always secure, EL2 is always non-secure, so the real direct changes in the above are roughly:

At least until you want to support ARMv8.4-SecEL2.

Core supports the security extensions? Great, then we can choose between CONFIG_ARMV8_A_NS and CONFIG_ARMV8_A_S.
Core does not support the security extensions? Enforce CONFIG_ARMV8_A_NS.

Ok, something like CONFIG_HAS_SEC_EXT or similar should do the job.

CONFIG_ARMV8_A_N?S does not directly affect QEMU at all. It's purely a software/Zephyr flag of "should we run in secure or nonsecure mode".

Agree. It is really weird to me that currently we force QEMU to start from EL1 when in NS mode.

(With a wrinkle here that it's not so much "core supports" as "we have control over". You could have a core with EL3, running a secure monitor outside our control, where Zephyr is only in control of EL1 and below...)

Yes, this is one of the reason why I decided to rewrite the boot code in C in #32394, so that we can easily implement these kind of checks more easily. In general I want to avoid creating too many symbols to be defined at compile time and doing as much as possible a "smart detection" of what's implemented on the core at run-time.

Annoyingly, PSCI is entirely defined for the non-secure world, and e.g. is designed to boot CPU cores into non-secure state. What this means is roughly that PSCI (the standard) only works if Zephyr is running in non-secure state, or if there's some platform-defined manner to switch between secure and non-secure state.

My suggestion for SMP CPU management here is to just define a standard interface for starting a core / stopping a core / suspending a core (optionally) / resuming a core, with the interface for AArch64 requiring that the core starts in the appropriate mode (the same EL and security state as Zephyr starts up in).

The only interface currently available goes through PSCI

__subsystem struct psci_driver_api {
psci_get_version_f get_version;
psci_cpu_off_f cpu_off;
psci_cpu_on_f cpu_on;
psci_affinity_info_f affinity_info;
};

As an aside, regardless of whichever PSCI decision is made, can we comply with the Linux kernel's devicetree spec for PSCI? https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/psci.txt

We comply already with PSCI v1.0 where the only thing you specify in the DT is the conduit. Anything missing?

@jharris-intel
Copy link
Contributor

ARMv8.4-SecEL2

Oh interesting. I must admit I'm unfamiliar with the ARMv8.x extensions for the most part.

The only interface currently available goes through PSCI

I mean, not quite? The interface is obviously designed as a wrapper around PSCI, and the current sole in-tree implementation does so, but it's mostly generic enough that you can write a backend driver implementation that doesn't actually use PSCI (and indeed I did so locally.)

(Or to put it another way, I'd argue that the current PSCI interface is misnamed and really should be a cpu power control interface, with the current driver being a PSCI-based implementation of said interface. Similar to how there's a serial interface, and uart drivers for said serial interface...)

We comply already with PSCI v1.0 where the only thing you specify in the DT is the conduit. Anything missing?

We don't support old PSCI versions - which honestly is likely fine and fairly easily extendable in the driver implementation if/when someone wants it.

The main part there is documenting that "hey, we'd like to match the Linux kernel here".

In general I want to avoid creating too many symbols to be defined at compile time and doing as much as possible a "smart detection" of what's implemented on the core at run-time.

+1 from me for this for the most part. The one thing I'll note is that if you've got a platform with a very minimal core (that e.g. have nothing above EL1) it's likely to be coresize-constrained also. But really, cross that bridge if/when we come to it...

@sandeepbrcm
Copy link
Collaborator

sandeepbrcm commented Feb 21, 2021

Supporting All permutation to enter Zephyr is going to increase complexity. I would suggest to defer such support without practical motivation unless you have time to support them all :)
The following two are currently valid choices AFAIK

1-Zephyr starts @ EL3 and drops to secure El1 and runs in secure state. TZ neutral system
They can implement their own 'plat_cpu_on()' to power up the secondary cores for example.

2-Zephyr starts @ NS EL1 (or NS EL2) and continues in NS state and obviously in this case it can be expected (mandated) to have a EL3 runtime to provide the secure services like PSCI. It is unlikely that SoC will have core control sequences available to NS state. Zephyr can leave it to platform choice though.

I feel the PSCI usage issue can be easily sorted with a thin wrapper for cpu ops.
for ex: In case of (1) plat_cpu_on() will implement plat specific cpu power up sequences.
In case of (1) it can translate to psci call.

@carlocaione
Copy link
Collaborator Author

Supporting All permutation to enter Zephyr is going to increase complexity. I would suggest to defer such support without practical motivation unless you have time to support them all :)
The following two are currently valid choices AFAIK

1-Zephyr starts @ EL3 and drops to secure El1 and runs in secure state. TZ neutral system
They can implement their own 'plat_cpu_on()' to power up the secondary cores for example.

2-Zephyr starts @ NS EL1 (or NS EL2) and continues in NS state and obviously in this case it can be expected (mandated) to have a EL3 runtime to provide the secure services like PSCI. It is unlikely that SoC will have core control sequences available to NS state. Zephyr can leave it to platform choice though.

But this is what we already have in place. Case (1) is the case !ARMV8_A_NS, case (2) is ARMV8_A_NS. What's missing is an agnostic cpu ops interface.

I feel the PSCI usage issue can be easily sorted with a thin wrapper for cpu ops.
for ex: In case of (1) plat_cpu_on() will implement plat specific cpu power up sequences.
In case of (1) it can translate to psci call.

Ok, let's take this step-by-step then. I'll try to prepare a patch to migrate the CPU ops interface / subsystem from PSCI to a more generic interface and then we can proceed from there.

@carlescufi carlescufi added the area: Trusted Execution Trusted Execution label Feb 23, 2021
@carlocaione
Copy link
Collaborator Author

Ok, let's take this step-by-step then. I'll try to prepare a patch to migrate the CPU ops interface / subsystem from PSCI to a more generic interface and then we can proceed from there.

here #32673

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Trusted Execution Trusted Execution Enhancement Changes/Updates/Additions to existing features Feature Request A request for a new feature RFC Request For Comments: want input from the community
Projects
None yet
Development

No branches or pull requests

4 participants