Re: [PATCH 08/16] KVM: arm64: timers: Allow userspace to set the counter offsets

[Marc Zyngier <maz@kernel.org>]

On Wed, 22 Feb 2023 16:34:32 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> On Wed, Feb 22, 2023 at 11:56:53AM +0000, Marc Zyngier wrote:
> 
> > > Chewing on this a bit more, I don't think userspace has any business
> > > messing with virtual and physical time independently, especially when
> > > nested virtualization comes into play.
> > 
> > Well, NV already ignores the virtual offset completely (see how the
> > virtual timer gets its offset reassigned at reset time).
> 
> I'll need to have a look at that, but if we need to ignore user input on
> the shiny new interface for NV then I really do wonder if it is the
> right fit.

The thing is, I'm not keen making the interface modal. Modes suck and
lead to invasive userspace changes. I'd rather ignore irrelevant
parameters than change the way userspace works. And you don't *have*
to provide the virtual offset with NV.

> > I previously toyed with this idea, and I really like it. However, the
> > problem with this is that it breaks the current behaviour of having
> > two different values for CNTVCT and CNTPCT in the guest, and CNTPCT
> > representing the counter value on the host.
> > 
> > Such a VM cannot be migrated *today*, but not everybody cares about
> > migration. My "dual offset" approach allows the current behaviour to
> > persist, and such a VM to be migrated. The luser even gets the choice
> > of preserving counter continuity in the guest or to stay without a
> > physical offset and reflect the host's counter.
> > 
> > Is it a good behaviour? Of course not. Does anyone depend on it? I
> > have no idea, but odds are that someone does. Can we break their toys?
> > The jury is still out.
> 
> Well, I think the new interface presents an opportunity to change the
> rules around counter migration, and the illusion of two distinct offsets
> for physical / virtual counters will need to be broken soon enough for
> NV.

Broken in the kernel, sure. Do we need to involve userspace in what
was an initial mis-design of the API? Changing these rules mean
changing the way userspace works, and I'm not keen on going there.

> Do we need to bend over backwards for a theoretical use case with
> the new UAPI? If anyone depends on the existing behavior then they can
> continue to use the old UAPI to partially migrate the guest counters.

I don't buy the old/new thing. My take is that these things should be
cumulative if there isn't a hard reason to break the existing API.

> My previous suggestion of tying the physical and virtual counters
> together at VM creation would definitely break such a use case, though,
> so we'd be at the point of requiring explicit opt-in from userspace.

I'm trying to find a middle ground, so bear with me. Here's the
situation as I see it:

(1) a VM that is migrating today can only set the virtual offset and
    doesn't affect the physical counter. This behaviour must be
    preserved in we cannot prove that nobody relies on it.

(2) setting the physical offset could be done by two means:

    (a) writing the counter register (like we do for CNTVCT)
    (b) providing an offset via a side channel

I think (1) must stay forever, just like we still support the old
GICv2 implicit initialisation.

(2a) is also desirable as it requires no extra work on the VMM side.
Just restore the damn thing, and nothing changes (we're finally able
to migrate the physical timer). (2b) really is icing on the cake.

The question is whether we can come up with an API offering (2b) that
still allows (1) and (2a). I'd be happy with a new API that, when
used, resets both offsets to the same value, matching your pretty
picture. But the dual offsetting still has to exist internally.

When it comes to NV, it uses either the physical offset that has been
provided by writing CNTPCT, or the one that has been written via the
new API. Under the hood, this is the same piece of data, of course.

The only meaningful difference with my initial proposal is that there
is no new virtual offset API. It is either register writes that obey
the same rules as before, or a single offset setting.

> > And I think that's the point where we differ. I can completely imagine
> > some in-VM code using the physical counter to export some timestamping
> > to the host (for tracing purposes, amongst other things).
> 
> So in this case the guest and userspace would already be in cahoots, so
> userspace could choose to not use UAPI. Hell, if userspace did
> absolutely nothing then it all continues to work.

Userspace, yes. Not necessarily the VMM. Let's say my guest spits a
bunch of timestamped traces over a network connection, which I then
correlate with host traces (all similarities with a shipping product
are completely fortuitous...). The VMM isn't involved at all here.

> Oh, we're definitely on the hook for any existing misuse of observable
> KVM behavior. I just think if we're at the point of adding new UAPI we
> may as well lay down some new rules with userspace to avoid surprises.
> 
> OTOH, ignoring the virtual offset for NV is another way out of the mess,
> but it just bothers me we're about to ignore input on a brand new
> UAPI...

I think my single-offset suggestion should answer your questioning.

Thoughts?

	M.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel