Message-ID: <87h7svm0o5.wl-maz@kernel.org>
Date:   Sat, 22 Aug 2020 11:31:54 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Steven Price <steven.price@....com>
Cc:     Keqian Zhu <zhukeqian1@...wei.com>, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        kvm@...r.kernel.org, Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        James Morse <james.morse@....com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        wanghaibin.wang@...wei.com
Subject: Re: [RFC PATCH 0/5] KVM: arm64: Add pvtime LPT support

Hi Steven,

On Wed, 19 Aug 2020 09:54:40 +0100,
Steven Price <steven.price@....com> wrote:
> 
> On 18/08/2020 15:41, Marc Zyngier wrote:
> > On 2020-08-17 09:41, Keqian Zhu wrote:
> >> Hi all,
> >> 
> >> This patch series picks up the LPT pvtime feature originally developed
> >> by Steven Price: https://patchwork.kernel.org/cover/10726499/
> >> 
> >> Backgroud:
> >> 
> >> There is demand for cross-platform migration, which means we have to
> >> solve different CPU features and arch counter frequency between hosts.
> >> This patch series can solve the latter problem.
> >> 
> >> About LPT:
> >> 
> >> This implements support for Live Physical Time (LPT) which provides the
> >> guest with a method to derive a stable counter of time during which the
> >> guest is executing even when the guest is being migrated between hosts
> >> with different physical counter frequencies.
> >> 
> >> Changes on Steven Price's work:
> >> 1. LPT structure: use symmatical semantics of scale multiplier, and use
> >>    fraction bits instead of "shift" to make everything clear.
> >> 2. Structure allocation: host kernel does not allocates the LPT
> >> structure,
> >>    instead it is allocated by userspace through VM attributes. The
> >> save/restore
> >>    functionality can be removed.
> >> 3. Since LPT structure just need update once for each guest run,
> >> add a flag to
> >>    indicate the update status. This has two benifits: 1) avoid
> >> multiple update
> >>    by each vCPUs. 2) If the update flag is not set, then return NOT
> >> SUPPORT for
> >>    coressponding guest HVC call.
> >> 4. Add VM device attributes interface for userspace configuration.
> >> 5. Add a base LPT read/write layer to reduce code.
> >> 6. Support ptimer scaling.
> >> 7. Support timer event stream translation.
> >> 
> >> Things need concern:
> >> 1. https://developer.arm.com/docs/den0057/a needs update.
> > 
> > LPT was explicitly removed from the spec because it doesn't really
> > solve the problem, specially for the firmware: EFI knows
> > nothing about this, for example. How is it going to work?
> > Also, nobody was ever able to explain how this would work for
> > nested virt.
> > 
> > ARMv8.4 and ARMv8.6 have the feature set that is required to solve
> > this problem without adding more PV to the kernel.
> 
> Hi Marc,
> 
> These are good points, however we do still have the situation that
> CPUs that don't have ARMv8.4/8.6 clearly cannot implement this. I
> presume the use-case Keqian is looking at predates the necessary
> support in the CPU - Keqian if you can provide more details on the
> architecture(s) involved that would be helpful.

My take on this is that it is a fictional use case. In my experience,
migration happens across *identical* systems, and *any* difference
visible to guests will cause things to go wrong. Errata management
gets in the way, as usual (name *one* integration that isn't broken
one way or another!).

Allowing migration across heterogeneous hosts requires a solution to
the errata management problem, which everyone (including me) has
decided to ignore so far (and I claim that not having a constant timer
frequency exposed to guests is an architecture bug).

> Nested virt is indeed more of an issue - we did have some ideas around
> using SDEI that never made it to the spec.

SDEI? Sigh... Why would SDEI be useful for NV and not for !NV?

> However I would argue that the most pragmatic approach would be to
> not support the combination of nested virt and LPT. Hopefully that
> can wait until the counter scaling support is available and not
> require PV.

And have yet another set of band aids that paper over the fact that we
can't get a consistent story on virtualization? No, thank you.

NV is (IMHO) much more important than LPT as it has a chance of
getting used. LPT is just another tick box, and the fact that ARM is
ready to ignore sideline a decent portion of the architecture is a
clear sign that it hasn't been thought out.

> We are discussing (re-)releasing the spec with the LPT parts added. If
> you have fundamental objections then please me know.

I do, see above. I'm stating that the use case doesn't really exist
given the state of the available HW and the fragmentation of the
architecture, and that ignoring the most important innovation in the
virtualization architecture since ARMv7 is at best short-sighted.

Time scaling is just an instance of the errata management problem, and
that is the issue that needs solving. Papering over part of the
problem is not helping.

	M.

-- 
Without deviation from the norm, progress is not possible.

Powered by blists - more mailing lists