xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu

PATCH RESEND In review Ouyang Zhaowei (Charles) 2 participants 4 messages May 6, 2015 – May 28, 2015

Subsystems (2)

x86 Xen hypervisor x86

lore

Share & tools

lore

Ouyang Zhaowei (Charles)

author

2015-05-06 01:49 UTC #0 lore raw

PATCH RESEND 1/1 xen: vcpu_info would be reset to wrong place on canceled suspend on PVOPS VM which has multi-cpu

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 <[email protected]>

Reviewed-by Boris Ostrovsky <[email protected]>

Reviewed-by David Vrabel <[email protected]>

---
 arch/x86/xen/suspend.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

arch/x86/xen/suspend.c +2 -1

index d949769..b2bed45 100644
@@ -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)) {

Ouyang Zhaowei (Charles)

author

2015-05-28 08:35 UTC #1 lore raw

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

Quoted 39 lines · Ouyang Zhaowei (Charles)

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 <[email protected]>
Reviewed-by Boris Ostrovsky <[email protected]>
Reviewed-by David Vrabel <[email protected]>
---
 arch/x86/xen/suspend.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/x86/xen/suspend.c +2 -2
index d949769..b2bed45 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
.

David Vrabel

2015-05-28 11:08 UTC #2 lore raw

In reply to Ouyang Zhaowei (Charles)

Quoted 9 lines · Ouyang Zhaowei (Charles)

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.

Quoted 3 lines

Signed-off-by Charles Ouyang <[email protected]>
Reviewed-by Boris Ostrovsky <[email protected]>
Reviewed-by David Vrabel <[email protected]>

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

Ouyang Zhaowei (Charles)

author

2015-05-28 11:39 UTC #3 lore raw

Quoted 10 lines · David Vrabel

Quoted 9 lines · Ouyang Zhaowei (Charles)
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.

Quoted 8 lines

Quoted 3 lines
Signed-off-by Charles Ouyang <[email protected]>
Reviewed-by Boris Ostrovsky <[email protected]>
Reviewed-by David Vrabel <[email protected]>
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

Quoted 8 lines

But I've applied this to for-linus-4.1b anyway, thanks.
David
.