Re: [PATCH v1] vmx: set 'SN' bit for the runstate transition from blocked to runnable

Re: [PATCH v1] vmx: set 'SN' bit for the runstate transition from blocked to runnable

[Chao Gao]

On Thu, Apr 13, 2017 at 02:20:23AM +0000, Xuquan (Quan Xu) wrote:
>From 946e7589e5a875574c7567a91943d47c38218a6f Mon Sep 17 00:00:00 2001
>From: Quan Xu <xuquan8@huawei.com>
>Date: Fri, 14 Apr 2017 02:11:30 +0800
>Subject: [PATCH v1] vmx: set 'SN' bit for the runstate transition from blocked
> to runnable
>
>set 'SN' bit of posted-interrupt descriptor when the runstate transition
>is from blocked to runnable (SN bit of posted-interrupt descriptor is still
>set to a blocked vCPU), otherwise PI notification event may be sent to a

Currently, we clear 'SN' bit for a blocked vCPU in order to receive a wakeup
interrupt to wake up the blocked vCPU. Also note that wakeup interrupt is not
a special vector for CPU.

>non-running vCPU.
>
>Signed-off-by: Quan Xu <xuquan8@huawei.com>
>---
> xen/arch/x86/hvm/vmx/vmx.c | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>index 9c5a388..956f104 100644
>--- a/xen/arch/x86/hvm/vmx/vmx.c
>+++ b/xen/arch/x86/hvm/vmx/vmx.c
>@@ -142,9 +142,6 @@ static void vmx_pi_switch_from(struct vcpu *v)
> {
>     struct pi_desc *pi_desc = &v->arch.hvm_vmx.pi_desc;
>
>-    if ( test_bit(_VPF_blocked, &v->pause_flags) )
>-        return;
>-

I wander whether vmx_pi_switch_from() will be called in the transition
blocked -> runnable.
In my mind, the function is called during context switch and consumes
the vcpu that is to be blocked or runnable. I think this if() statement
is to distinguish 'block' case and doesn't set 'SN' for this case.

Thanks
Chao

>     pi_set_sn(pi_desc);
> }
>
>--
>1.8.3.1


>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xen.org
>https://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v1] vmx: set 'SN' bit for the runstate transition from blocked to runnable

[Xuquan (Quan Xu)]

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

>From 946e7589e5a875574c7567a91943d47c38218a6f Mon Sep 17 00:00:00 2001
From: Quan Xu <xuquan8@huawei.com>
Date: Fri, 14 Apr 2017 02:11:30 +0800
Subject: [PATCH v1] vmx: set 'SN' bit for the runstate transition from blocked
 to runnable

set 'SN' bit of posted-interrupt descriptor when the runstate transition
is from blocked to runnable (SN bit of posted-interrupt descriptor is still
set to a blocked vCPU), otherwise PI notification event may be sent to a
non-running vCPU.

Signed-off-by: Quan Xu <xuquan8@huawei.com>
---
 xen/arch/x86/hvm/vmx/vmx.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 9c5a388..956f104 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -142,9 +142,6 @@ static void vmx_pi_switch_from(struct vcpu *v)
 {
     struct pi_desc *pi_desc = &v->arch.hvm_vmx.pi_desc;

-    if ( test_bit(_VPF_blocked, &v->pause_flags) )
-        return;
-
     pi_set_sn(pi_desc);
 }

--
1.8.3.1

[-- Attachment #2: 0001-vmx-set-SN-bit-for-the-runstate-transition-from-bloc.patch --]
[-- Type: application/octet-stream, Size: 1003 bytes --]

From 946e7589e5a875574c7567a91943d47c38218a6f Mon Sep 17 00:00:00 2001
From: Quan Xu <xuquan8@huawei.com>
Date: Fri, 14 Apr 2017 02:11:30 +0800
Subject: [PATCH v1] vmx: set 'SN' bit for the runstate transition from blocked
 to runnable

set 'SN' bit of posted-interrupt descriptor when the runstate transition
is from blocked to runnable (SN bit of posted-interrupt descriptor is still
set to a blocked vCPU), otherwise PI notification event may be sent to a
non-running vCPU.

Signed-off-by: Quan Xu <xuquan8@huawei.com>
---
 xen/arch/x86/hvm/vmx/vmx.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 9c5a388..956f104 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -142,9 +142,6 @@ static void vmx_pi_switch_from(struct vcpu *v)
 {
     struct pi_desc *pi_desc = &v->arch.hvm_vmx.pi_desc;
 
-    if ( test_bit(_VPF_blocked, &v->pause_flags) )
-        return;
-
     pi_set_sn(pi_desc);
 }
 
-- 
1.8.3.1


[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v1] vmx: set 'SN' bit for the runstate transition from blocked to runnable

[Xuquan (Quan Xu)]

On April 13, 2017 3:35 AM, Chao Gao wrote:
>On Thu, Apr 13, 2017 at 02:20:23AM +0000, Xuquan (Quan Xu) wrote:
>>From 946e7589e5a875574c7567a91943d47c38218a6f Mon Sep 17
>00:00:00 2001
>>From: Quan Xu <xuquan8@huawei.com>
>>Date: Fri, 14 Apr 2017 02:11:30 +0800
>>Subject: [PATCH v1] vmx: set 'SN' bit for the runstate transition from
>>blocked  to runnable
>>
>>set 'SN' bit of posted-interrupt descriptor when the runstate
>>transition is from blocked to runnable (SN bit of posted-interrupt
>>descriptor is still set to a blocked vCPU), otherwise PI notification
>>event may be sent to a
>
>Currently, we clear 'SN' bit for a blocked vCPU in order to receive a wakeup
>interrupt to wake up the blocked vCPU. Also note that wakeup interrupt is not
>a special vector for CPU.
>
Sorry, a typo 's/set/clear/'.... 
You are right. It is clear. that's why I want to set it.


>>non-running vCPU.
>>
>>Signed-off-by: Quan Xu <xuquan8@huawei.com>
>>---
>> xen/arch/x86/hvm/vmx/vmx.c | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>>diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>>index 9c5a388..956f104 100644
>>--- a/xen/arch/x86/hvm/vmx/vmx.c
>>+++ b/xen/arch/x86/hvm/vmx/vmx.c
>>@@ -142,9 +142,6 @@ static void vmx_pi_switch_from(struct vcpu *v)  {
>>     struct pi_desc *pi_desc = &v->arch.hvm_vmx.pi_desc;
>>
>>-    if ( test_bit(_VPF_blocked, &v->pause_flags) )
>>-        return;
>>-
>
>I wander whether vmx_pi_switch_from() will be called in the transition
>blocked -> runnable.
>In my mind, the function is called during context switch and consumes the
>vcpu that is to be blocked or runnable. 

If you are right, my patch is wrong.. 

Chao, thank you. I will check it again..

Quan

>I think this if() statement is to
>distinguish 'block' case and doesn't set 'SN' for this case.
>





>Thanks
>Chao
>
>>     pi_set_sn(pi_desc);
>> }
>>
>>--
>>1.8.3.1
>
>
>>_______________________________________________
>>Xen-devel mailing list
>>Xen-devel@lists.xen.org
>>https://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v1] vmx: set 'SN' bit for the runstate transition from blocked to runnable

[Xuquan (Quan Xu)]

__sorry for the noise, please ignore this patch__

On April 13, 2017 11:45 AM, Quan Xu wrote:
>On April 13, 2017 3:35 AM, Chao Gao wrote:
>>On Thu, Apr 13, 2017 at 02:20:23AM +0000, Xuquan (Quan Xu) wrote:
>>>From 946e7589e5a875574c7567a91943d47c38218a6f Mon Sep 17
>>00:00:00 2001
>>>From: Quan Xu <xuquan8@huawei.com>
>>>Date: Fri, 14 Apr 2017 02:11:30 +0800
>>>Subject: [PATCH v1] vmx: set 'SN' bit for the runstate transition from
>>>blocked  to runnable
>>>
>>>set 'SN' bit of posted-interrupt descriptor when the runstate
>>>transition is from blocked to runnable (SN bit of posted-interrupt
>>>descriptor is still set to a blocked vCPU), otherwise PI notification
>>>event may be sent to a
>>
>>Currently, we clear 'SN' bit for a blocked vCPU in order to receive a
>>wakeup interrupt to wake up the blocked vCPU. Also note that wakeup
>>interrupt is not a special vector for CPU.
>>
>Sorry, a typo 's/set/clear/'....
>You are right. It is clear. that's why I want to set it.
>
>
>>>non-running vCPU.
>>>
>>>Signed-off-by: Quan Xu <xuquan8@huawei.com>
>>>---
>>> xen/arch/x86/hvm/vmx/vmx.c | 3 ---
>>> 1 file changed, 3 deletions(-)
>>>
>>>diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>>>index 9c5a388..956f104 100644
>>>--- a/xen/arch/x86/hvm/vmx/vmx.c
>>>+++ b/xen/arch/x86/hvm/vmx/vmx.c
>>>@@ -142,9 +142,6 @@ static void vmx_pi_switch_from(struct vcpu *v)  {
>>>     struct pi_desc *pi_desc = &v->arch.hvm_vmx.pi_desc;
>>>
>>>-    if ( test_bit(_VPF_blocked, &v->pause_flags) )
>>>-        return;
>>>-
>>
>>I wander whether vmx_pi_switch_from() will be called in the transition
>>blocked -> runnable.
>>In my mind, the function is called during context switch and consumes
>>the vcpu that is to be blocked or runnable.
>
>If you are right, my patch is wrong..
>
>Chao, thank you. I will check it again..

Chao, checked the code, I think you are right..
For example, HLT:

vmx_vmexit_handler(... case EXIT_REASON_HLT ... )
  |
  hvm_hlt()
   |
   do_sched_op(... SCHEDOP_block ...)
     |
     vcpu_block_enable_events()
       |
       vcpu_block()
         |
         arch_vcpu_block()
         raise_softirq(SCHEDULE_SOFTIRQ)



if the vcpu is blocked, the vcpu is de-scheduled..  sorry for the noise, please ignore this patch ..

Quan

>
>Quan
>
>>I think this if() statement is to
>>distinguish 'block' case and doesn't set 'SN' for this case.
>>
>
>
>
>
>
>>Thanks
>>Chao
>>
>>>     pi_set_sn(pi_desc);
>>> }
>>>
>>>--
>>>1.8.3.1
>>
>>
>>>_______________________________________________
>>>Xen-devel mailing list
>>>Xen-devel@lists.xen.org
>>>https://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel