On Mon, 2024-03-18 at 18:07 +0000, Marc Zyngier wrote: > On Mon, 18 Mar 2024 17:54:06 +0000, > David Woodhouse <dwmw2@xxxxxxxxxxxxx> wrote: > > > > [1 <text/plain; UTF-8 (quoted-printable)>] > > On Mon, 2024-03-18 at 17:29 +0000, Marc Zyngier wrote: > > > > > > Again, I really oppose this way of doing things. We already have an > > > infrastructure for selecting PSCI levels. You may not like it, but it > > > exists, and I'm not going entertain supporting yet another bike-shed > > > model. Adding an orthogonal cap for a feature that is specific to a > > > new PSCI version is just awful. > > > > Huh? This isn't a "new bike-shed model". This is a straight copy of > > what we *already* have for SYSTEM_RESET2. > > There is no KVM capability for SYSTEM_RESET2. It is directly > advertised to the guest when PSCI 1.1 is supported. Apologies, I got that wrong. It's SYSTEM_SUSPEND and the corresponding KVM_CAP_ARM_SYSTEM_SUSPEND that I was thinking of. Not SYSTEM_RESET2.I mixed those up. > > If I were bike-shedding, I wouldn't do separate caps for them; I'd have > > done it as a *bitmask* of the optional PSCI calls that should be > > enabled. > > > > The *mandatory* ones should obviously come from the PSCI version alone, > > but I can't see how that makes sense for the optional ones... > > The guest is in a position to probe for what is supported or not with > the PSCI_FEATURES call. Why would you add anything else? Because we don't want to silently *change* what's advertised to the guest with the VMM explicitly opting in. > > > Please make PSCI 1.3 the only version of PSCI supporting suspend in a > > > non-optional way, and be done with it. > > > > SYSTEM_OFF2 is an *optional* feature in PSCI v1.3. As are > > CLEAR_INV_MEMREGION and CLEAR_INV_MEMREGION_ATTRIBUTES. > > > > Are you suggesting that enabling v1.3 should automatically enable *all* > > of the optional features that were defined in that version (and > > previous versions) of the spec? > > No. We have everything we need to incrementally *add* features. So you > can perfectly implement PSCI 1.3 with only SYSTEM_OFF2, and only later > on add the rest, if ever. OK. It's still awful, but I suppose can live with that since existing VMMs will just see the same KVM_SYSTEM_EVENT_SHUTDOWN as before, and hopefully just won't understand the flag (and won't notice) the extra flag which says it's a hibernate. A VMM might *perhaps* check for flags it doesn't understand and complain about them, which is why we shouldn't really do that. But where PSCI is concerned it seems we've left best practice behind a long time ago, so I'll let it go.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
