On Mon, Dec 16, 2019 at 03:06:57PM +0000, Peter Maydell wrote: > On Thu, 12 Dec 2019 at 17:33, Andrew Jones <drjo...@redhat.com> 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 <drjo...@redhat.com> > > --- > > 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
