[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v2 5/5] target/arm/cpu: Add the kvm-no-adjvtime CPU prope
From: |
Andrew Jones |
Subject: |
Re: [RFC PATCH v2 5/5] target/arm/cpu: Add the kvm-no-adjvtime CPU property |
Date: |
Mon, 16 Dec 2019 17:52:49 +0100 |
On Mon, Dec 16, 2019 at 03:06:57PM +0000, Peter Maydell wrote:
> On Thu, 12 Dec 2019 at 17:33, Andrew Jones <address@hidden> wrote:
> >
> > kvm-no-adjvtime is a KVM specific CPU property and a first of its kind.
> > To accommodate it we also add kvm_arm_add_vcpu_properties() and a
> > KVM specific CPU properties description to the CPU features document.
> >
> > Signed-off-by: Andrew Jones <address@hidden>
> > ---
> > docs/arm-cpu-features.rst | 31 ++++++++++++++++++++++++++++++-
> > hw/arm/virt.c | 8 ++++++++
> > include/hw/arm/virt.h | 1 +
> > target/arm/cpu.c | 2 ++
> > target/arm/cpu64.c | 1 +
> > target/arm/kvm.c | 28 ++++++++++++++++++++++++++++
> > target/arm/kvm_arm.h | 11 +++++++++++
> > target/arm/monitor.c | 1 +
> > tests/arm-cpu-features.c | 4 ++++
> > 9 files changed, 86 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/arm-cpu-features.rst b/docs/arm-cpu-features.rst
> > index 1b367e22e16e..641ec9cb8f4a 100644
> > --- a/docs/arm-cpu-features.rst
> > +++ b/docs/arm-cpu-features.rst
> > @@ -31,7 +31,9 @@ supporting the feature or only supporting the feature
> > under certain
> > configurations. For example, the `aarch64` CPU feature, which, when
> > disabled, enables the optional AArch32 CPU feature, is only supported
> > when using the KVM accelerator and when running on a host CPU type that
> > -supports the feature.
> > +supports the feature. While `aarch64` currently only works with KVM,
> > +it could work with TCG. CPU features that are specific to KVM are
> > +prefixed with "kvm-" and are described in "KVM VCPU Features".
> >
> > CPU Feature Probing
> > ===================
> > @@ -171,6 +173,33 @@ disabling many SVE vector lengths would be quite
> > verbose, the `sve<N>` CPU
> > properties have special semantics (see "SVE CPU Property Parsing
> > Semantics").
> >
> > +KVM VCPU Features
> > +=================
> > +
> > +KVM VCPU features are CPU features that are specific to KVM, such as
> > +paravirt features or features that enable CPU virtualization extensions.
> > +The features' CPU properties are only available when KVM is enabled and
> > +are named with the prefix "kvm-". KVM VCPU features may be probed,
> > +enabled, and disabled in the same way as other CPU features. Below is the
> > +list of KVM VCPU features and their descriptions.
> > +
> > + kvm-no-adjvtime When disabled, each time the VM transitions
> > + back to running state from the paused state the
> > + VCPU's vitual counter is updated to ensure the
>
> "virtual"
>
> > + stopped time is not counted. This avoids time
> > + jumps surprising guest OSes and applications,
> > + as long as they use the virtual counter for
> > + timekeeping, but has the side effect of the
> > + virtual and physical counters diverging. All
> > + timekeeping based on the virtual counter will
> > + appear to lag behind any timekeeping that does
> > + not subtract VM stopped time. The guest may
> > + resynchronize its virtual counter with other
> > + time sources as needed. Enabling this KVM VCPU
> > + feature provides the legacy behavior, which is
> > + to also count stopped time with the virtual
> > + counter.
>
> This phrasing reads a bit confusingly to me. What I would usually expect
> is that you get
> name-of-option Description of what the option does.
>
> But here we have
> name-of-option Long description of the default behaviour,
> taking many lines and several sentences.
> Brief note at the end that enabling this
> feature gives the opposite effect.
>
> Especially since the default-behaviour description isn't prefaced
> with "By default" or similar, it's quite easy to start reading the
> text assuming it's defining what the option is going to do, only
> to get to the end and realise that it's defining what the option
> is *not* going to do...
I'll take another stab at this, but my feeling is that a '-no-' option
should be one that just turns off the default behavior, which is why I
wrote a long description of the default behavior. If you'd prefer the
description to be more terse, then I can certainly delete a bunch of
the text, but then I fear what this option disables wouldn't be clear
enough.
>
> Incidentally, if I understand things correctly, for TCG the
> behaviour is (and has always been) that VM-stopped time is
> not counted, because we run the emulated versions of these counters
> off QEMU_CLOCK_VIRTUAL. So having the KVM default be the same as
> the TCG default is nicely consistent.
Thanks,
drew
[RFC PATCH v2 4/5] tests/arm-cpu-features: Check feature default values, Andrew Jones, 2019/12/12
Re: [RFC PATCH v2 0/5] target/arm/kvm: Adjust virtual time, Peter Maydell, 2019/12/16