[PATCH ] Documentation/kvm: Update cpuid documentation for steal time and pv eoi

[PATCH ] Documentation/kvm: Update cpuid documentation for steal time and pv eoi

[Raghavendra K T]

Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
---
 While adding documentation for pvspinlock, I found that these two should
 be updated. I have based this on top of pvspinlock kvm host patchset (V12)

 Documentation/virtual/kvm/cpuid.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
index 22ff659..15a5ac20 100644
--- a/Documentation/virtual/kvm/cpuid.txt
+++ b/Documentation/virtual/kvm/cpuid.txt
@@ -43,6 +43,15 @@ KVM_FEATURE_CLOCKSOURCE2           ||     3 || kvmclock available at msrs
 KVM_FEATURE_ASYNC_PF               ||     4 || async pf can be enabled by
                                    ||       || writing to msr 0x4b564d02
 ------------------------------------------------------------------------------
+KVM_FEATURE_STEAL_TIME             ||     5 || guest accounts fine granularity
+                                   ||       || task steal time. enabled when
+                                   ||       || shedstat or task delay accounting
+                                   ||       || is supported by the host.
+------------------------------------------------------------------------------
+KVM_FEATURE_PV_EOI                 ||     6 || overrides the generic EOI
+                                   ||       || implementation with an optimized
+                                   ||       || version.
+------------------------------------------------------------------------------
 KVM_FEATURE_PV_UNHALT              ||     7 || guest checks this feature bit
                                    ||       || before enabling paravirtualized
                                    ||       || spinlock support.
-- 
1.7.11.7

Re: [PATCH ] Documentation/kvm: Update cpuid documentation for steal time and pv eoi

[Michael S. Tsirkin]

On Fri, Aug 23, 2013 at 05:34:47PM +0530, Raghavendra K T wrote:
> Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
> ---
>  While adding documentation for pvspinlock, I found that these two should
>  be updated. I have based this on top of pvspinlock kvm host patchset (V12)

I would change the description to merely say what the CPUID bits
mean, and what they mean is exactly that an MSR is valid.
Use KVM_FEATURE_ASYNC_PF as a template.


>  Documentation/virtual/kvm/cpuid.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
> index 22ff659..15a5ac20 100644
> --- a/Documentation/virtual/kvm/cpuid.txt
> +++ b/Documentation/virtual/kvm/cpuid.txt
> @@ -43,6 +43,15 @@ KVM_FEATURE_CLOCKSOURCE2           ||     3 || kvmclock available at msrs
>  KVM_FEATURE_ASYNC_PF               ||     4 || async pf can be enabled by
>                                     ||       || writing to msr 0x4b564d02
>  ------------------------------------------------------------------------------
> +KVM_FEATURE_STEAL_TIME             ||     5 || guest accounts fine granularity
> +                                   ||       || task steal time.

I'm not sure what this phrase means.
Steal time is a host feature, not a guest feature:
IIUC if this bit is set, the hypervisor can pass the guest information
about how much time was spent running other processes outside the VM.

> enabled when
> +                                   ||       || shedstat or task delay accounting
> +                                   ||       || is supported by the host.

I think it's enabled by guest, not by host.


> +------------------------------------------------------------------------------
> +KVM_FEATURE_PV_EOI                 ||     6 || overrides the generic EOI
> +                                   ||       || implementation with an optimized
> +                                   ||       || version.

More exactly "with a paravirtualized version".

> +------------------------------------------------------------------------------
>  KVM_FEATURE_PV_UNHALT              ||     7 || guest checks this feature bit
>                                     ||       || before enabling paravirtualized
>                                     ||       || spinlock support.
> -- 
> 1.7.11.7

Re: [PATCH ] Documentation/kvm: Update cpuid documentation for steal time and pv eoi

[Raghavendra K T]

On 08/26/2013 12:37 PM, Michael S. Tsirkin wrote:
> I would change the description to merely say what the CPUID bits
> mean, and what they mean is exactly that an MSR is valid.
> Use KVM_FEATURE_ASYNC_PF as a template.

Thank you for the review.
Changing the doc accordingly by adding msr info. Please refer below.

>> +KVM_FEATURE_STEAL_TIME             ||     5 || guest accounts fine granularity
>> +                                   ||       || task steal time.
>
> I'm not sure what this phrase means.
> Steal time is a host feature, not a guest feature:
> IIUC if this bit is set, the hypervisor can pass the guest information
> about how much time was spent running other processes outside the VM.

Okay. I guess I need some help here.

I took this from the PARAVIRT_TIME_ACCOUNTING config help. also I saw
that guest is  actually returning the steal time in kvm_steal_clock().

>
>> enabled when
>> +                                   ||       || shedstat or task delay accounting
>> +                                   ||       || is supported by the host.
>
> I think it's enabled by guest, not by host.

true. My understanding was, Guest enables it when host has schedstat or
task delay accounting  on.

I referred to this hunk in kvm/cpuid.c

if (sched_info_on())
    entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
and sched_info_on() is true when schedstat or task delay accounting is
on.

Does this look good?

"Enabled by writing to msr 0x4b564d03. The feature is
enabled by guest when host has schedstat or task delay accounting
support."

>> +KVM_FEATURE_PV_EOI                 ||     6 || overrides the generic EOI
>> +                                   ||       || implementation with an optimized
>> +                                   ||       || version.
>
> More exactly "with a paravirtualized version".

Okay. So how does this sound?

"overrides the generic EOI implementation with a paravirtualized
version. This feature is enabled by writing to msr  0x4b564d04."