Messages in this thread | | | Date | Tue, 1 Dec 2020 14:57:20 +0000 | From | Mark Rutland <> | Subject | Re: [PATCH v3 20/23] kvm: arm64: Intercept host's CPU_SUSPEND PSCI SMCs |
| |
On Thu, Nov 26, 2020 at 03:54:18PM +0000, David Brazdil wrote: > Add a handler of CPU_SUSPEND host PSCI SMCs. The SMC can either enter > a sleep state indistinguishable from a WFI or a deeper sleep state that > behaves like a CPU_OFF+CPU_ON except that the core is still considered > online when asleep. > > The handler saves r0,pc of the host and makes the same call to EL3 with > the hyp CPU entry point. It either returns back to the handler and then > back to the host, or wakes up into the entry point and initializes EL2 > state before dropping back to EL1.
For those CPU_SUSPEND calls which lose context, is there no EL2 state that you need to save/restore, or is that all saved elsewhere already?
The usual suspects are PMU, debug, and timers, so maybe not. It'd be nice to have a statement in the commit message if we're certain there's no state that we need to save.
> A core can only suspend itself but other cores can concurrently invoke > CPU_ON with this core as target. To avoid racing them for the same > boot args struct, CPU_SUSPEND uses a different struct instance and entry > point. Each entry point selects the corresponding struct to restore host > boot args from. This avoids the need for locking in CPU_SUSPEND.
I found this a bit confusing since the first sentence can be read to mean that CPU_ON is expected to compose with CPU_SUSPEND, whereas what this is actually saying is the implementation ensures they don't interact. How about:
| CPU_ON and CPU_SUSPEND are both implemented using struct cpu_boot_args | to store the state upon powerup, with each CPU having separate structs | for CPU_ON and CPU_SUSPEND so that CPU_SUSPEND can operate locklessly | and so that a CPU_ON xall targetting a CPU cannot interfere with a | concurrent CPU_SUSPEND call on that CPU.
The patch itself looks fine to me.
Thanks, Mark.
| |