* [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
@ 2015-05-06 1:47 Ouyang Zhaowei (Charles)
2015-05-28 8:34 ` Ouyang Zhaowei (Charles)
2015-05-28 11:07 ` David Vrabel
0 siblings, 2 replies; 9+ messages in thread
From: Ouyang Zhaowei (Charles) @ 2015-05-06 1:47 UTC (permalink / raw)
To: David Vrabel, Boris Ostrovsky, Konrad Rzeszutek Wilk
Cc: linux-kernel, Dingweiping, Yanqiangjun, jinjian
vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
while on canceled suspend, the call to xen_hvm_init_shared_info() will make the guest think that vcpu_info
is in the shared page. This will cause non-boot cpus keep checking a wrong vcpu_info address and can not handle
irqs any more after suspend canceled, for example after we do "xl save -c/restore", PVOPS VM can not be resumed
as the cpus can not finish their irqs.
Signed-off-by: Charles Ouyang <ouyangzhaowei@huawei.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
---
arch/x86/xen/suspend.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
index d949769..b2bed45 100644
--- a/arch/x86/xen/suspend.c
+++ b/arch/x86/xen/suspend.c
@@ -32,7 +32,8 @@ static void xen_hvm_post_suspend(int suspend_cancelled)
{
#ifdef CONFIG_XEN_PVHVM
int cpu;
- xen_hvm_init_shared_info();
+ if (!suspend_cancelled)
+ xen_hvm_init_shared_info();
xen_callback_vector();
xen_unplug_emulated_devices();
if (xen_feature(XENFEAT_hvm_safe_pvclock)) {
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
2015-05-06 1:47 [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu Ouyang Zhaowei (Charles)
@ 2015-05-28 8:34 ` Ouyang Zhaowei (Charles)
2015-05-28 11:07 ` David Vrabel
1 sibling, 0 replies; 9+ messages in thread
From: Ouyang Zhaowei (Charles) @ 2015-05-28 8:34 UTC (permalink / raw)
To: David Vrabel, Boris Ostrovsky, Konrad Rzeszutek Wilk
Cc: linux-kernel, Dingweiping, Yanqiangjun, jinjian
Hi all,
It's been a long time, I don't know whether this patch is OK or not. Or does it somewhere need to be improved a little bit?
thanks
On 2015.5.6 9:47, Ouyang Zhaowei (Charles) wrote:
> vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
>
> The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
> while on canceled suspend, the call to xen_hvm_init_shared_info() will make the guest think that vcpu_info
> is in the shared page. This will cause non-boot cpus keep checking a wrong vcpu_info address and can not handle
> irqs any more after suspend canceled, for example after we do "xl save -c/restore", PVOPS VM can not be resumed
> as the cpus can not finish their irqs.
>
>
> Signed-off-by: Charles Ouyang <ouyangzhaowei@huawei.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>
> ---
> arch/x86/xen/suspend.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
> index d949769..b2bed45 100644
> --- a/arch/x86/xen/suspend.c
> +++ b/arch/x86/xen/suspend.c
> @@ -32,7 +32,8 @@ static void xen_hvm_post_suspend(int suspend_cancelled)
> {
> #ifdef CONFIG_XEN_PVHVM
> int cpu;
> - xen_hvm_init_shared_info();
> + if (!suspend_cancelled)
> + xen_hvm_init_shared_info();
> xen_callback_vector();
> xen_unplug_emulated_devices();
> if (xen_feature(XENFEAT_hvm_safe_pvclock)) {
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
2015-05-06 1:47 [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu Ouyang Zhaowei (Charles)
2015-05-28 8:34 ` Ouyang Zhaowei (Charles)
@ 2015-05-28 11:07 ` David Vrabel
2015-05-28 11:38 ` Ouyang Zhaowei (Charles)
1 sibling, 1 reply; 9+ messages in thread
From: David Vrabel @ 2015-05-28 11:07 UTC (permalink / raw)
To: Ouyang Zhaowei (Charles), Boris Ostrovsky, Konrad Rzeszutek Wilk
Cc: linux-kernel, Dingweiping, Yanqiangjun, jinjian
On 06/05/15 02:47, Ouyang Zhaowei (Charles) wrote:
> vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
>
> The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
> while on canceled suspend, the call to xen_hvm_init_shared_info() will make the guest think that vcpu_info
> is in the shared page. This will cause non-boot cpus keep checking a wrong vcpu_info address and can not handle
> irqs any more after suspend canceled, for example after we do "xl save -c/restore", PVOPS VM can not be resumed
> as the cpus can not finish their irqs.
>
>
> Signed-off-by: Charles Ouyang <ouyangzhaowei@huawei.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>
1. I don't think I offered a reviewed-by tag for this patch
2. You did not Cc the xen-devel mailing list on this patch.
3. The patch is white-space damaged.
4. The commit message has lines longer than ~72 chars.
But I've applied this to for-linus-4.1b anyway, thanks.
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
2015-05-28 11:07 ` David Vrabel
@ 2015-05-28 11:38 ` Ouyang Zhaowei (Charles)
0 siblings, 0 replies; 9+ messages in thread
From: Ouyang Zhaowei (Charles) @ 2015-05-28 11:38 UTC (permalink / raw)
To: David Vrabel, Boris Ostrovsky, Konrad Rzeszutek Wilk
Cc: linux-kernel, Dingweiping, Yanqiangjun, jinjian
On 2015.5.28 19:07, David Vrabel wrote:
> On 06/05/15 02:47, Ouyang Zhaowei (Charles) wrote:
>> vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
>>
>> The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
>> while on canceled suspend, the call to xen_hvm_init_shared_info() will make the guest think that vcpu_info
>> is in the shared page. This will cause non-boot cpus keep checking a wrong vcpu_info address and can not handle
>> irqs any more after suspend canceled, for example after we do "xl save -c/restore", PVOPS VM can not be resumed
>> as the cpus can not finish their irqs.
>>
>>
>> Signed-off-by: Charles Ouyang <ouyangzhaowei@huawei.com>
>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Reviewed-by: David Vrabel <david.vrabel@citrix.com>
>
> 1. I don't think I offered a reviewed-by tag for this patch
> 2. You did not Cc the xen-devel mailing list on this patch.
> 3. The patch is white-space damaged.
> 4. The commit message has lines longer than ~72 chars.
Sorry about that, this is my first time to send patch, and I didn't notice that it was white-space damaged.
I'll modify the patch and commit message as you applied and Cc Xen-devel mailing list.
Thanks
Ouyang Zhaowei
>
> But I've applied this to for-linus-4.1b anyway, thanks.
>
> David
>
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
2015-05-05 17:21 ` David Vrabel
@ 2015-05-06 1:41 ` Ouyang Zhaowei (Charles)
0 siblings, 0 replies; 9+ messages in thread
From: Ouyang Zhaowei (Charles) @ 2015-05-06 1:41 UTC (permalink / raw)
To: David Vrabel, Boris Ostrovsky, Konrad Rzeszutek Wilk
Cc: linux-kernel, Dingweiping, Yanqiangjun, jinjian
On 2015.5.6 1:21, David Vrabel wrote:
> On 05/05/15 14:19, Ouyang Zhaowei (Charles) wrote:
>>
>>
>> On 2015.5.5 17:09, David Vrabel wrote:
>>> On 04/05/15 03:56, Ouyang Zhaowei (Charles) wrote:
>>>> The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
>>>> while on canceled suspend, the call to xen_hvm_init_shared_info() will now make the guest think that vcpu_info
>>>> is in the shared page, so we do not call xen_hvm_init_shared_info() on suspend canceled.
>>>
>>> I don't understand what the impact of this fix is. Can you include this
>>> in the commit message?
>>>
>>> David
>>
>> Sure, please let me know if there is anything not clear enough.
>>
>> vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
>>
>> The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
>> while on canceled suspend, the call to xen_hvm_init_shared_info() will make the guest think that vcpu_info
>> is in the shared page. This will cause non-boot cpus keep checking a wrong vcpu_info address and can not handle
>> irqs any more after suspend canceled, for example after we do "xl save -c/restore", PVOPS VM can not be resumed
>> as the cpus can not finish their irqs.
>
> Ok, so it's only cancelled suspends that are affected. Can you make the
> clear in the commit and resubmit. This looks like something that should
> be tagged for stable as well.
>
> David
OK, thanks
>
>>
>>>
>>>> Signed-off-by: Charles Ouyang <ouyangzhaowei@huawei.com>
>>>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>>> ---
>>>> arch/x86/xen/suspend.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
>>>> index d949769..b2bed45 100644
>>>> --- a/arch/x86/xen/suspend.c
>>>> +++ b/arch/x86/xen/suspend.c
>>>> @@ -32,7 +32,8 @@ static void xen_hvm_post_suspend(int suspend_cancelled)
>>>> {
>>>> #ifdef CONFIG_XEN_PVHVM
>>>> int cpu;
>>>> - xen_hvm_init_shared_info();
>>>> + if (!suspend_cancelled)
>>>> + xen_hvm_init_shared_info();
>>>> xen_callback_vector();
>>>> xen_unplug_emulated_devices();
>>>> if (xen_feature(XENFEAT_hvm_safe_pvclock)) {
>>>>
>>>
>>>
>>> .
>>>
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
2015-05-05 13:19 ` Ouyang Zhaowei (Charles)
@ 2015-05-05 17:21 ` David Vrabel
2015-05-06 1:41 ` Ouyang Zhaowei (Charles)
0 siblings, 1 reply; 9+ messages in thread
From: David Vrabel @ 2015-05-05 17:21 UTC (permalink / raw)
To: Ouyang Zhaowei (Charles), Boris Ostrovsky, Konrad Rzeszutek Wilk
Cc: linux-kernel, Dingweiping, Yanqiangjun, jinjian
On 05/05/15 14:19, Ouyang Zhaowei (Charles) wrote:
>
>
> On 2015.5.5 17:09, David Vrabel wrote:
>> On 04/05/15 03:56, Ouyang Zhaowei (Charles) wrote:
>>> The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
>>> while on canceled suspend, the call to xen_hvm_init_shared_info() will now make the guest think that vcpu_info
>>> is in the shared page, so we do not call xen_hvm_init_shared_info() on suspend canceled.
>>
>> I don't understand what the impact of this fix is. Can you include this
>> in the commit message?
>>
>> David
>
> Sure, please let me know if there is anything not clear enough.
>
> vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
>
> The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
> while on canceled suspend, the call to xen_hvm_init_shared_info() will make the guest think that vcpu_info
> is in the shared page. This will cause non-boot cpus keep checking a wrong vcpu_info address and can not handle
> irqs any more after suspend canceled, for example after we do "xl save -c/restore", PVOPS VM can not be resumed
> as the cpus can not finish their irqs.
Ok, so it's only cancelled suspends that are affected. Can you make the
clear in the commit and resubmit. This looks like something that should
be tagged for stable as well.
David
>
>>
>>> Signed-off-by: Charles Ouyang <ouyangzhaowei@huawei.com>
>>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>> ---
>>> arch/x86/xen/suspend.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
>>> index d949769..b2bed45 100644
>>> --- a/arch/x86/xen/suspend.c
>>> +++ b/arch/x86/xen/suspend.c
>>> @@ -32,7 +32,8 @@ static void xen_hvm_post_suspend(int suspend_cancelled)
>>> {
>>> #ifdef CONFIG_XEN_PVHVM
>>> int cpu;
>>> - xen_hvm_init_shared_info();
>>> + if (!suspend_cancelled)
>>> + xen_hvm_init_shared_info();
>>> xen_callback_vector();
>>> xen_unplug_emulated_devices();
>>> if (xen_feature(XENFEAT_hvm_safe_pvclock)) {
>>>
>>
>>
>> .
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
2015-05-05 9:09 ` David Vrabel
@ 2015-05-05 13:19 ` Ouyang Zhaowei (Charles)
2015-05-05 17:21 ` David Vrabel
0 siblings, 1 reply; 9+ messages in thread
From: Ouyang Zhaowei (Charles) @ 2015-05-05 13:19 UTC (permalink / raw)
To: David Vrabel, Boris Ostrovsky, Konrad Rzeszutek Wilk
Cc: linux-kernel, Dingweiping, Yanqiangjun, jinjian
On 2015.5.5 17:09, David Vrabel wrote:
> On 04/05/15 03:56, Ouyang Zhaowei (Charles) wrote:
>> The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
>> while on canceled suspend, the call to xen_hvm_init_shared_info() will now make the guest think that vcpu_info
>> is in the shared page, so we do not call xen_hvm_init_shared_info() on suspend canceled.
>
> I don't understand what the impact of this fix is. Can you include this
> in the commit message?
>
> David
Sure, please let me know if there is anything not clear enough.
vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
while on canceled suspend, the call to xen_hvm_init_shared_info() will make the guest think that vcpu_info
is in the shared page. This will cause non-boot cpus keep checking a wrong vcpu_info address and can not handle
irqs any more after suspend canceled, for example after we do "xl save -c/restore", PVOPS VM can not be resumed
as the cpus can not finish their irqs.
>
>> Signed-off-by: Charles Ouyang <ouyangzhaowei@huawei.com>
>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> ---
>> arch/x86/xen/suspend.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
>> index d949769..b2bed45 100644
>> --- a/arch/x86/xen/suspend.c
>> +++ b/arch/x86/xen/suspend.c
>> @@ -32,7 +32,8 @@ static void xen_hvm_post_suspend(int suspend_cancelled)
>> {
>> #ifdef CONFIG_XEN_PVHVM
>> int cpu;
>> - xen_hvm_init_shared_info();
>> + if (!suspend_cancelled)
>> + xen_hvm_init_shared_info();
>> xen_callback_vector();
>> xen_unplug_emulated_devices();
>> if (xen_feature(XENFEAT_hvm_safe_pvclock)) {
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
2015-05-04 2:56 Ouyang Zhaowei (Charles)
@ 2015-05-05 9:09 ` David Vrabel
2015-05-05 13:19 ` Ouyang Zhaowei (Charles)
0 siblings, 1 reply; 9+ messages in thread
From: David Vrabel @ 2015-05-05 9:09 UTC (permalink / raw)
To: Ouyang Zhaowei (Charles), Boris Ostrovsky, Konrad Rzeszutek Wilk
Cc: linux-kernel, Dingweiping, Yanqiangjun, jinjian
On 04/05/15 03:56, Ouyang Zhaowei (Charles) wrote:
> The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
> while on canceled suspend, the call to xen_hvm_init_shared_info() will now make the guest think that vcpu_info
> is in the shared page, so we do not call xen_hvm_init_shared_info() on suspend canceled.
I don't understand what the impact of this fix is. Can you include this
in the commit message?
David
> Signed-off-by: Charles Ouyang <ouyangzhaowei@huawei.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> arch/x86/xen/suspend.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
> index d949769..b2bed45 100644
> --- a/arch/x86/xen/suspend.c
> +++ b/arch/x86/xen/suspend.c
> @@ -32,7 +32,8 @@ static void xen_hvm_post_suspend(int suspend_cancelled)
> {
> #ifdef CONFIG_XEN_PVHVM
> int cpu;
> - xen_hvm_init_shared_info();
> + if (!suspend_cancelled)
> + xen_hvm_init_shared_info();
> xen_callback_vector();
> xen_unplug_emulated_devices();
> if (xen_feature(XENFEAT_hvm_safe_pvclock)) {
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu
@ 2015-05-04 2:56 Ouyang Zhaowei (Charles)
2015-05-05 9:09 ` David Vrabel
0 siblings, 1 reply; 9+ messages in thread
From: Ouyang Zhaowei (Charles) @ 2015-05-04 2:56 UTC (permalink / raw)
To: Boris Ostrovsky, Konrad Rzeszutek Wilk, David Vrabel
Cc: linux-kernel, Dingweiping, Yanqiangjun, jinjian
The hypervisor continues assuming that vcpu_info is stored in per-cpu data which was set up by xen_vcpu_setup(),
while on canceled suspend, the call to xen_hvm_init_shared_info() will now make the guest think that vcpu_info
is in the shared page, so we do not call xen_hvm_init_shared_info() on suspend canceled.
Signed-off-by: Charles Ouyang <ouyangzhaowei@huawei.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
arch/x86/xen/suspend.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
index d949769..b2bed45 100644
--- a/arch/x86/xen/suspend.c
+++ b/arch/x86/xen/suspend.c
@@ -32,7 +32,8 @@ static void xen_hvm_post_suspend(int suspend_cancelled)
{
#ifdef CONFIG_XEN_PVHVM
int cpu;
- xen_hvm_init_shared_info();
+ if (!suspend_cancelled)
+ xen_hvm_init_shared_info();
xen_callback_vector();
xen_unplug_emulated_devices();
if (xen_feature(XENFEAT_hvm_safe_pvclock)) {
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-05-28 11:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-06 1:47 [RESEND PATCH] xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu Ouyang Zhaowei (Charles)
2015-05-28 8:34 ` Ouyang Zhaowei (Charles)
2015-05-28 11:07 ` David Vrabel
2015-05-28 11:38 ` Ouyang Zhaowei (Charles)
-- strict thread matches above, loose matches on Subject: below --
2015-05-04 2:56 Ouyang Zhaowei (Charles)
2015-05-05 9:09 ` David Vrabel
2015-05-05 13:19 ` Ouyang Zhaowei (Charles)
2015-05-05 17:21 ` David Vrabel
2015-05-06 1:41 ` Ouyang Zhaowei (Charles)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.