Re: [PATCH v5 9/9] Documentation: virtual: kvm: Support vcpu preempted check

[rkrc...@redhat.com]

2016-10-24 16:42+0200, Paolo Bonzini:
> On 21/10/2016 20:39, rkrc...@redhat.com wrote:
>> 2016-10-21 11:27+, David Laight:
>>> From: Pan Xinhui
 Sent: 20 October 2016 22:28
 Commit ("x86, kvm: support vcpu preempted check") add one field "__u8
 preempted" into struct kvm_steal_time. This field tells if one vcpu is
 running or not.

 It is zero if 1) some old KVM deos not support this filed. 2) the vcpu is
 preempted. Other values means the vcpu has been preempted.

 Signed-off-by: Pan Xinhui 
 ---
  Documentation/virtual/kvm/msr.txt | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

 diff --git a/Documentation/virtual/kvm/msr.txt 
 b/Documentation/virtual/kvm/msr.txt
 index 2a71c8f..3376f13 100644
 --- a/Documentation/virtual/kvm/msr.txt
 +++ b/Documentation/virtual/kvm/msr.txt
 @@ -208,7 +208,8 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
__u64 steal;
__u32 version;
__u32 flags;
 -  __u32 pad[12];
 +  __u8  preempted;
 +  __u32 pad[11];
}
>>>
>>> I think I'd be explicit about the 3 pad bytes you've left.
>> 
>> Seconded.
>> 
>> With that change are all KVM bits
>> 
>> Acked-by: Radim Krčmář 
> 
> Saw this after replying to the previous message.  If you need to post v6
> of the full series, it would be nice if you removed the
> kvm_read_guest_cached.  But anyway it wasn't my intention to override Radim.

The patch was acceptable to me even now, so I definitely wouldn't mind
if it were even nicer. :)

Re: [PATCH v5 9/9] Documentation: virtual: kvm: Support vcpu preempted check

[Paolo Bonzini]

On 21/10/2016 20:39, rkrc...@redhat.com wrote:
> 2016-10-21 11:27+, David Laight:
>> From: Pan Xinhui
>>> Sent: 20 October 2016 22:28
>>> Commit ("x86, kvm: support vcpu preempted check") add one field "__u8
>>> preempted" into struct kvm_steal_time. This field tells if one vcpu is
>>> running or not.
>>>
>>> It is zero if 1) some old KVM deos not support this filed. 2) the vcpu is
>>> preempted. Other values means the vcpu has been preempted.
>>>
>>> Signed-off-by: Pan Xinhui 
>>> ---
>>>  Documentation/virtual/kvm/msr.txt | 8 +++-
>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/virtual/kvm/msr.txt 
>>> b/Documentation/virtual/kvm/msr.txt
>>> index 2a71c8f..3376f13 100644
>>> --- a/Documentation/virtual/kvm/msr.txt
>>> +++ b/Documentation/virtual/kvm/msr.txt
>>> @@ -208,7 +208,8 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
>>> __u64 steal;
>>> __u32 version;
>>> __u32 flags;
>>> -   __u32 pad[12];
>>> +   __u8  preempted;
>>> +   __u32 pad[11];
>>> }
>>
>> I think I'd be explicit about the 3 pad bytes you've left.
> 
> Seconded.
> 
> With that change are all KVM bits
> 
> Acked-by: Radim Krčmář 

Saw this after replying to the previous message.  If you need to post v6
of the full series, it would be nice if you removed the
kvm_read_guest_cached.  But anyway it wasn't my intention to override Radim.

Paolo

Re: [PATCH v5 9/9] Documentation: virtual: kvm: Support vcpu preempted check

[xinhui]

This is new version for [PATCH v6 9/9] Documentation: virtual: kvm: Support 
vcpu preempted check
change:
an explicit pad[3] after __u8 preempted.
a typo fix in the commit log.

From defac64d7c6a50d5f18ef64a7c776af3e21e8b68 Mon Sep 17 00:00:00 2001
From: Pan Xinhui 
Date: Thu, 20 Oct 2016 09:33:36 -0400
Subject: [PATCH v6 9/9] Documentation: virtual: kvm: Support vcpu preempted 
check

Commit ("x86, kvm: support vcpu preempted check") add one field "__u8
preempted" into struct kvm_steal_time. This field tells if one vcpu is
running or not.

It is zero if 1) some old KVM deos not support this filed. 2) the vcpu is
not preempted. Other values mean the vcpu has been preempted.

Signed-off-by: Pan Xinhui 
Acked-by: Radim Krčmář 
---
 Documentation/virtual/kvm/msr.txt | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/msr.txt 
b/Documentation/virtual/kvm/msr.txt
index 2a71c8f..ab2ab76 100644
--- a/Documentation/virtual/kvm/msr.txt
+++ b/Documentation/virtual/kvm/msr.txt
@@ -208,7 +208,9 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
__u64 steal;
__u32 version;
__u32 flags;
-   __u32 pad[12];
+   __u8  preempted;
+   __u8  u8_pad[3];
+   __u32 pad[11];
}
 
 	whose data will be filled in by the hypervisor periodically. Only one

@@ -232,6 +234,11 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
nanoseconds. Time during which the vcpu is idle, will not be
reported as steal time.
 
+		preempted: indicate the VCPU who owns this struct is running or

+   not. Non-zero values mean the VCPU has been preempted. Zero
+   means the VCPU is not preempted. NOTE, it is always zero if the
+   the hypervisor doesn't support this field.
+
 MSR_KVM_EOI_EN: 0x4b564d04
data: Bit 0 is 1 when PV end of interrupt is enabled on the vcpu; 0
when disabled.  Bit 1 is reserved and must be zero.  When PV end of
--
2.4.11

Re: [PATCH v5 9/9] Documentation: virtual: kvm: Support vcpu preempted check

[xinhui]

On 2016年10月22日 02:39, rkrc...@redhat.com wrote:

2016-10-21 11:27+, David Laight:

From: Pan Xinhui

Sent: 20 October 2016 22:28
Commit ("x86, kvm: support vcpu preempted check") add one field "__u8
preempted" into struct kvm_steal_time. This field tells if one vcpu is
running or not.

It is zero if 1) some old KVM deos not support this filed. 2) the vcpu is
preempted. Other values means the vcpu has been preempted.

Signed-off-by: Pan Xinhui 
---
  Documentation/virtual/kvm/msr.txt | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/msr.txt 
b/Documentation/virtual/kvm/msr.txt
index 2a71c8f..3376f13 100644
--- a/Documentation/virtual/kvm/msr.txt
+++ b/Documentation/virtual/kvm/msr.txt
@@ -208,7 +208,8 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
__u64 steal;
__u32 version;
__u32 flags;
-   __u32 pad[12];
+   __u8  preempted;
+   __u32 pad[11];
}


I think I'd be explicit about the 3 pad bytes you've left.


Seconded.

With that change are all KVM bits


like below?
__u8 preempted;
__u8 kvm_pad[3];



Acked-by: Radim Krčmář 


thanks!

Re: [PATCH v5 9/9] Documentation: virtual: kvm: Support vcpu preempted check

[xinhui]

On 2016年10月21日 19:27, David Laight wrote:

From: Pan Xinhui

Sent: 20 October 2016 22:28
Commit ("x86, kvm: support vcpu preempted check") add one field "__u8
preempted" into struct kvm_steal_time. This field tells if one vcpu is
running or not.

It is zero if 1) some old KVM deos not support this filed. 2) the vcpu is
preempted. Other values means the vcpu has been preempted.

Signed-off-by: Pan Xinhui 
---
  Documentation/virtual/kvm/msr.txt | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/msr.txt 
b/Documentation/virtual/kvm/msr.txt
index 2a71c8f..3376f13 100644
--- a/Documentation/virtual/kvm/msr.txt
+++ b/Documentation/virtual/kvm/msr.txt
@@ -208,7 +208,8 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
__u64 steal;
__u32 version;
__u32 flags;
-   __u32 pad[12];
+   __u8  preempted;
+   __u32 pad[11];
}


I think I'd be explicit about the 3 pad bytes you've left.


yes,I  will do it in next version.

thanks


David

Re: [PATCH v5 9/9] Documentation: virtual: kvm: Support vcpu preempted check

[rkrc...@redhat.com]

2016-10-21 11:27+, David Laight:
> From: Pan Xinhui
>> Sent: 20 October 2016 22:28
>> Commit ("x86, kvm: support vcpu preempted check") add one field "__u8
>> preempted" into struct kvm_steal_time. This field tells if one vcpu is
>> running or not.
>> 
>> It is zero if 1) some old KVM deos not support this filed. 2) the vcpu is
>> preempted. Other values means the vcpu has been preempted.
>> 
>> Signed-off-by: Pan Xinhui 
>> ---
>>  Documentation/virtual/kvm/msr.txt | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/virtual/kvm/msr.txt 
>> b/Documentation/virtual/kvm/msr.txt
>> index 2a71c8f..3376f13 100644
>> --- a/Documentation/virtual/kvm/msr.txt
>> +++ b/Documentation/virtual/kvm/msr.txt
>> @@ -208,7 +208,8 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
>>  __u64 steal;
>>  __u32 version;
>>  __u32 flags;
>> -__u32 pad[12];
>> +__u8  preempted;
>> +__u32 pad[11];
>>  }
> 
> I think I'd be explicit about the 3 pad bytes you've left.

Seconded.

With that change are all KVM bits

Acked-by: Radim Krčmář 

RE: [PATCH v5 9/9] Documentation: virtual: kvm: Support vcpu preempted check

[David Laight]

From: Pan Xinhui
> Sent: 20 October 2016 22:28
> Commit ("x86, kvm: support vcpu preempted check") add one field "__u8
> preempted" into struct kvm_steal_time. This field tells if one vcpu is
> running or not.
> 
> It is zero if 1) some old KVM deos not support this filed. 2) the vcpu is
> preempted. Other values means the vcpu has been preempted.
> 
> Signed-off-by: Pan Xinhui 
> ---
>  Documentation/virtual/kvm/msr.txt | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/virtual/kvm/msr.txt 
> b/Documentation/virtual/kvm/msr.txt
> index 2a71c8f..3376f13 100644
> --- a/Documentation/virtual/kvm/msr.txt
> +++ b/Documentation/virtual/kvm/msr.txt
> @@ -208,7 +208,8 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
>   __u64 steal;
>   __u32 version;
>   __u32 flags;
> - __u32 pad[12];
> + __u8  preempted;
> + __u32 pad[11];
>   }

I think I'd be explicit about the 3 pad bytes you've left.

David

Re: [PATCH v5 9/9] Documentation: virtual: kvm: Support vcpu preempted check

[Pan Xinhui]

在 2016/10/21 09:23, Boqun Feng 写道:

On Thu, Oct 20, 2016 at 05:27:54PM -0400, Pan Xinhui wrote:

Commit ("x86, kvm: support vcpu preempted check") add one field "__u8
preempted" into struct kvm_steal_time. This field tells if one vcpu is
running or not.

It is zero if 1) some old KVM deos not support this filed. 2) the vcpu is
preempted. Other values means the vcpu has been preempted.

  ^
s/preempted/not preempted


yes. the less of *not* definitely sould be avoided..

And better to fix other typos in the commit log ;-)
Maybe you can try aspell? That works for me.


I will try it. :)


Regards,
Boqun



Signed-off-by: Pan Xinhui 
---
 Documentation/virtual/kvm/msr.txt | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/msr.txt 
b/Documentation/virtual/kvm/msr.txt
index 2a71c8f..3376f13 100644
--- a/Documentation/virtual/kvm/msr.txt
+++ b/Documentation/virtual/kvm/msr.txt
@@ -208,7 +208,8 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
__u64 steal;
__u32 version;
__u32 flags;
-   __u32 pad[12];
+   __u8  preempted;
+   __u32 pad[11];
}

whose data will be filled in by the hypervisor periodically. Only one
@@ -232,6 +233,11 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
nanoseconds. Time during which the vcpu is idle, will not be
reported as steal time.

+   preempted: indicate the VCPU who owns this struct is running or
+   not. Non-zero values mean the VCPU has been preempted. Zero
+   means the VCPU is not preempted. NOTE, it is always zero if the
+   the hypervisor doesn't support this field.
+
 MSR_KVM_EOI_EN: 0x4b564d04
data: Bit 0 is 1 when PV end of interrupt is enabled on the vcpu; 0
when disabled.  Bit 1 is reserved and must be zero.  When PV end of
--
2.4.11

Re: [PATCH v5 9/9] Documentation: virtual: kvm: Support vcpu preempted check

[Boqun Feng]

On Thu, Oct 20, 2016 at 05:27:54PM -0400, Pan Xinhui wrote:
> Commit ("x86, kvm: support vcpu preempted check") add one field "__u8
> preempted" into struct kvm_steal_time. This field tells if one vcpu is
> running or not.
> 
> It is zero if 1) some old KVM deos not support this filed. 2) the vcpu is
> preempted. Other values means the vcpu has been preempted.
  ^
s/preempted/not preempted

And better to fix other typos in the commit log ;-)
Maybe you can try aspell? That works for me.

Regards,
Boqun

> 
> Signed-off-by: Pan Xinhui 
> ---
>  Documentation/virtual/kvm/msr.txt | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/virtual/kvm/msr.txt 
> b/Documentation/virtual/kvm/msr.txt
> index 2a71c8f..3376f13 100644
> --- a/Documentation/virtual/kvm/msr.txt
> +++ b/Documentation/virtual/kvm/msr.txt
> @@ -208,7 +208,8 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
>   __u64 steal;
>   __u32 version;
>   __u32 flags;
> - __u32 pad[12];
> + __u8  preempted;
> + __u32 pad[11];
>   }
>  
>   whose data will be filled in by the hypervisor periodically. Only one
> @@ -232,6 +233,11 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
>   nanoseconds. Time during which the vcpu is idle, will not be
>   reported as steal time.
>  
> + preempted: indicate the VCPU who owns this struct is running or
> + not. Non-zero values mean the VCPU has been preempted. Zero
> + means the VCPU is not preempted. NOTE, it is always zero if the
> + the hypervisor doesn't support this field.
> +
>  MSR_KVM_EOI_EN: 0x4b564d04
>   data: Bit 0 is 1 when PV end of interrupt is enabled on the vcpu; 0
>   when disabled.  Bit 1 is reserved and must be zero.  When PV end of
> -- 
> 2.4.11
> 


signature.asc
Description: PGP signature

[PATCH v5 9/9] Documentation: virtual: kvm: Support vcpu preempted check

[Pan Xinhui]

Commit ("x86, kvm: support vcpu preempted check") add one field "__u8
preempted" into struct kvm_steal_time. This field tells if one vcpu is
running or not.

It is zero if 1) some old KVM deos not support this filed. 2) the vcpu is
preempted. Other values means the vcpu has been preempted.

Signed-off-by: Pan Xinhui 
---
 Documentation/virtual/kvm/msr.txt | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/msr.txt 
b/Documentation/virtual/kvm/msr.txt
index 2a71c8f..3376f13 100644
--- a/Documentation/virtual/kvm/msr.txt
+++ b/Documentation/virtual/kvm/msr.txt
@@ -208,7 +208,8 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
__u64 steal;
__u32 version;
__u32 flags;
-   __u32 pad[12];
+   __u8  preempted;
+   __u32 pad[11];
}
 
whose data will be filled in by the hypervisor periodically. Only one
@@ -232,6 +233,11 @@ MSR_KVM_STEAL_TIME: 0x4b564d03
nanoseconds. Time during which the vcpu is idle, will not be
reported as steal time.
 
+   preempted: indicate the VCPU who owns this struct is running or
+   not. Non-zero values mean the VCPU has been preempted. Zero
+   means the VCPU is not preempted. NOTE, it is always zero if the
+   the hypervisor doesn't support this field.
+
 MSR_KVM_EOI_EN: 0x4b564d04
data: Bit 0 is 1 when PV end of interrupt is enabled on the vcpu; 0
when disabled.  Bit 1 is reserved and must be zero.  When PV end of
-- 
2.4.11