linux-kernel - Re: [PATCH v3 20/23] kvm: arm64: Intercept host's CPU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201201145720.GB86881@C02TD0UTHF1T.local>
Date:   Tue, 1 Dec 2020 14:57:20 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     David Brazdil <dbrazdil@...gle.com>
Cc:     kvmarm@...ts.cs.columbia.edu, Jonathan Corbet <corbet@....net>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, Marc Zyngier <maz@...nel.org>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Dennis Zhou <dennis@...nel.org>, Tejun Heo <tj@...nel.org>,
        Christoph Lameter <cl@...ux.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Sudeep Holla <sudeep.holla@....com>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        kernel-team@...roid.com
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.