Skip to contentSkip to conversation

33 VCPUs in HVM guests with live migration with Linux hangs

PATCH New Konrad Rzeszutek Wilk 6 participants 36 messages Apr 4, 2014 – Apr 23, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

When live migrating I found out that if you try with more than 32 VCPUs the guest is stuck. It works OK when booting - all 33 VCPUs show up.

I use this small config:

kernel = "hvmloader"
device_model_version = 'qemu-xen-traditional'
vcpus = 33
builder='hvm'
memory=1024
serial='file:/var/log/xen/console-bootstrap-x86_64-pvhvm'
name="m"
disk = [ 'file:/mnt/lab/bootstrap-x86_64/root_image.iso,hdc:cdrom,r','phy:/dev/guests/bootstrap-x86_64-pvhvm,xvda,w'] boot="dn"
vif = [ 'mac=00:0F:4B:00:00:68, bridge=switch' ]
vnc=1
vnclisten="0.0.0.0"
usb=1
usbdevice="tablet"

And do a migration:

m 33 1023 33 -b---- 14.3
-bash-4.1# xl migrate m localhost
root@localhost's password:
migration target: Ready to receive domain.
Saving to migration stream new xl format (info 0x0/0x0/418)
Loading new save file <incoming migration stream> (new xl fmt info 0x0/0x0/418)
Savefile contains xl domain config
WARNING: ignoring "kernel" directive for HVM guest. Use "firmware_override" instead if you really want a non-default firmware
libxl: notice: libxl_numa.c:494:libxl__get_numa_candidate: NUMA placement failed, performance might be affected
xc: Reloading memory pages: 262144/1045504 25%migration target: Transfer complete, requesting permission to start domain.
migration sender: Target has acknowledged transfer.
migration sender: Giving target permission to start.
migration target: Got permission, starting domain.
migration target: Domain started successsfully.
migration sender: Target reports successful startup.
Migration successful.

Which completes succesfully.

xl vpcu-list tells me that all 32 VCPUs are blocked except one - 33rd. Which seems to alternate between:

Call Trace:
                    [<ffffffff81128a5a>] multi_cpu_stop+0x9a <--
ffff8800365b5da0:   [<ffffffff811289c0>] multi_cpu_stop
ffff8800365b5dc0:   [<ffffffff811290aa>] cpu_stopper_thread+0x4a
ffff8800365b5de0:   [<ffffffff816f7081>] __schedule+0x381
ffff8800365b5e38:   [<ffffffff810cbf90>] smpboot_thread_fn
ffff8800365b5e80:   [<ffffffff810cc0d8>] smpboot_thread_fn+0x148
ffff8800365b5eb0:   [<ffffffff810cbf90>] smpboot_thread_fn
ffff8800365b5ec0:   [<ffffffff810c498e>] kthread+0xce
ffff8800365b5f28:   [<ffffffff810c48c0>] kthread
ffff8800365b5f50:   [<ffffffff81703e0c>] ret_from_fork+0x7c
ffff8800365b5f80:   [<ffffffff810c48c0>] kthread

and

                   [<ffffffff8108f3c8>] pvclock_clocksource_read+0x18 <--
ffff880038603ef0:   [<ffffffff81045698>] xen_clocksource_read+0x28
ffff880038603f00:   [<ffffffff81057909>] sched_clock+0x9
ffff880038603f10:   [<ffffffff810d7b85>] sched_clock_local+0x25
ffff880038603f40:   [<ffffffff810d7ca8>] sched_clock_cpu+0xb8
ffff880038603f60:   [<ffffffff810d840e>] irqtime_account_irq+0x4e
ffff880038603f80:   [<ffffffff810a5279>] irq_enter+0x39
ffff880038603f90:   [<ffffffff813f8480>] xen_evtchn_do_upcall+0x20
ffff880038603fb0:   [<ffffffff817058ed>] xen_hvm_callback_vector+0x6d

which implies that the CPU is receiving interrupts, but somehow is in thread doing something.. probably waiting for an mutex.

When the CPU (33) started (before migration) this was its stack:

Call Trace:
                    [<ffffffff8108e846>] native_safe_halt+0x6 <--
ffff8800377d1e90:   [<ffffffff8105989a>] default_idle+0x1a
ffff8800377d1eb0:   [<ffffffff810591f6>] arch_cpu_idle+0x26
ffff8800377d1ec0:   [<ffffffff810f6d76>] cpu_startup_entry+0xa6
ffff8800377d1ef0:   [<ffffffff81109a55>] clockevents_register_device+0x105
ffff8800377d1f30:   [<ffffffff8108236e>] start_secondary+0x19e

The only culprit I could think of was commit d5b17dbff83d63fb6bf35daec21c8ebfb8d695b5
"xen/smp/pvhvm: Don't point per_cpu(xen_vpcu, 33 and larger) to shared_info"

which I had reverted - but that did not help.

So questions:

1) Had anybody else actually booted HVM guests with more than 32 VCPUs and tried to migrate?

2) If yes, had you seen this before?

#1 lore raw
Quoted 4 lines · Konrad Rzeszutek Wilk

The only culprit I could think of was commit d5b17dbff83d63fb6bf35daec21c8ebfb8d695b5
"xen/smp/pvhvm: Don't point per_cpu(xen_vpcu, 33 and larger) to shared_info"

which I had reverted - but that did not help.

Looking at the commit log that isn't surprising. That message hints that there is some later setup which calls VCPUOP_register_info and causes cpus >= 33 to work. (since as the message says shared info only has space for 32 cpus).

I think you want to be investigating the (lack of) calls to VCPUOP_register_info.

Quoted 4 lines

So questions:

1) Had anybody else actually booted HVM guests with more than 32 VCPUs and tried to migrate?

Not me.

Ian.

#2 lore raw

PATCH 1/1 Fixes for more than 32 VCPUs migration for HVM guests (v1).

These two patches (one for Linux, one for Xen) allow PVHVM guests to use the per-cpu VCPU mechanism after migration. Currently when an PVHVM guest migrates all the per-cpu information is lost and we fallback on the shared_info structure. This is regardless if the HVM guest has 2 or 128 CPUs. Since the structure has an array for only 32 CPUs that means if we are to migrate a PVHVM guest - we can only do it up to 32 CPUs.

These patches fix it and allow more than 32 VCPUs to be migrated with PVHVM Linux guests.

The Linux diff is:

 arch/x86/xen/enlighten.c |   21 ++++++++++++++++-----
 arch/x86/xen/suspend.c   |    6 +-----
 arch/x86/xen/time.c      |    3 +++
 3 files changed, 20 insertions(+), 10 deletions(-)

while the Xen one is:

 xen/arch/x86/hvm/hvm.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
#3 lore raw

PATCH 1/2 hvm: Support more than 32 VCPUS when migrating.

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

---
 xen/arch/x86/hvm/hvm.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
xen/arch/x86/hvm/hvm.c +3 -0
index 38c491e..b5b92fe 100644
@@ -3470,6 +3470,9 @@ static long hvm_vcpu_op(
     case VCPUOP_stop_singleshot_timer:
     case VCPUOP_register_vcpu_info:
     case VCPUOP_register_vcpu_time_memory_area:
+    case VCPUOP_down:
+    case VCPUOP_up:
+    case VCPUOP_is_up:
         rc = do_vcpu_op(cmd, vcpuid, arg);
         break;
     default:
Signature · 1 line
1.7.7.6
#4 lore raw

Re: [Xen-devel] [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

Then on resume we unblock the "suspended" CPUs, and the first thing they do is call cpu_ops.cpu_resume which is basically going to setup the vcpu_info using VCPUOP_register_vcpu_info. Not sure if something similar is possible under Linux, but it seems easier and doesn't require any Xen-side changes.

Roger.

#5 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

Quoted 29 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Quoted 6 lines

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

Quoted 14 lines

Then on resume we unblock the "suspended" CPUs, and the first thing they do is call cpu_ops.cpu_resume which is basically going to setup the vcpu_info using VCPUOP_register_vcpu_info. Not sure if something similar is possible under Linux, but it seems easier and doesn't require any Xen-side changes.

Roger.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#6 lore raw

Re: [Xen-devel] [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to Roger Pau Monné
Quoted 29 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Quoted 6 lines

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

Quoted 14 lines

Then on resume we unblock the "suspended" CPUs, and the first thing they do is call cpu_ops.cpu_resume which is basically going to setup the vcpu_info using VCPUOP_register_vcpu_info. Not sure if something similar is possible under Linux, but it seems easier and doesn't require any Xen-side changes.

Roger.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#7 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

In order to suspend them an IPI_SUSPEND is sent to all vCPUs except vCPU#0:

http://fxr.watson.org/fxr/source/kern/subr_smp.c#L289

Which makes all APs call cpususpend_handler, so we know all APs are stuck in a while loop with interrupts disabled:

http://fxr.watson.org/fxr/source/amd64/amd64/mp_machdep.c#L1459

Then on resume the APs are taken out of the while loop and the first thing they do before returning from the IPI handler is registering the new per-cpu vcpu_info area. But I'm not sure this is something that can be accomplished easily on Linux.

I've tried to local-migrate a FreeBSD PVHVM guest with 33 vCPUs on my 8-way box, and it seems to be working fine :).

Roger.

#8 lore raw

Re: [Xen-devel] [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to Konrad Rzeszutek Wilk
Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

In order to suspend them an IPI_SUSPEND is sent to all vCPUs except vCPU#0:

http://fxr.watson.org/fxr/source/kern/subr_smp.c#L289

Which makes all APs call cpususpend_handler, so we know all APs are stuck in a while loop with interrupts disabled:

http://fxr.watson.org/fxr/source/amd64/amd64/mp_machdep.c#L1459

Then on resume the APs are taken out of the while loop and the first thing they do before returning from the IPI handler is registering the new per-cpu vcpu_info area. But I'm not sure this is something that can be accomplished easily on Linux.

I've tried to local-migrate a FreeBSD PVHVM guest with 33 vCPUs on my 8-way box, and it seems to be working fine :).

Roger.

#9 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

Quoted 49 lines · Roger Pau Monné
Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

In order to suspend them an IPI_SUSPEND is sent to all vCPUs except vCPU#0:

http://fxr.watson.org/fxr/source/kern/subr_smp.c#L289

Which makes all APs call cpususpend_handler, so we know all APs are stuck in a while loop with interrupts disabled:

Quoted 6 lines

http://fxr.watson.org/fxr/source/amd64/amd64/mp_machdep.c#L1459

Then on resume the APs are taken out of the while loop and the first thing they do before returning from the IPI handler is registering the new per-cpu vcpu_info area. But I'm not sure this is something that can be accomplished easily on Linux.

That is a bit of what the 'stop_machine' would do. It puts all of the CPUs in whatever function you want. But I am not sure of the latency impact - as in what if the migration takes longer and all of the CPUs sit there spinning. Another variant of that is the 'smp_call_function'.

Then when we resume - we need a mailbox that is shared (easily enough I think) to tell us that the migration has been done - and then need to call that VCPUOP_register_vcpu_info.

But if the migration has taken quite long - I fear that the watchdogs might kick in and start complaining about the CPUs stuck. Especially if we migrating on overcommitted guest.

With this the latency for them to be 'paused', 'initted', 'unpaused' I think is much much smaller.

Ugh, lets wait with this exercise of using the 'smp_call_function' sometime at the end of the summer - and see. That functionality should be shared with the PV code path IMHO.

Quoted 3 lines

I've tried to local-migrate a FreeBSD PVHVM guest with 33 vCPUs on my 8-way box, and it seems to be working fine :).

Awesome!

Quoted 3 lines

Roger.

#10 lore raw

Re: [Xen-devel] [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to Roger Pau Monné
Quoted 49 lines · Roger Pau Monné
Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

In order to suspend them an IPI_SUSPEND is sent to all vCPUs except vCPU#0:

http://fxr.watson.org/fxr/source/kern/subr_smp.c#L289

Which makes all APs call cpususpend_handler, so we know all APs are stuck in a while loop with interrupts disabled:

Quoted 6 lines

http://fxr.watson.org/fxr/source/amd64/amd64/mp_machdep.c#L1459

Then on resume the APs are taken out of the while loop and the first thing they do before returning from the IPI handler is registering the new per-cpu vcpu_info area. But I'm not sure this is something that can be accomplished easily on Linux.

That is a bit of what the 'stop_machine' would do. It puts all of the CPUs in whatever function you want. But I am not sure of the latency impact - as in what if the migration takes longer and all of the CPUs sit there spinning. Another variant of that is the 'smp_call_function'.

Then when we resume - we need a mailbox that is shared (easily enough I think) to tell us that the migration has been done - and then need to call that VCPUOP_register_vcpu_info.

But if the migration has taken quite long - I fear that the watchdogs might kick in and start complaining about the CPUs stuck. Especially if we migrating on overcommitted guest.

With this the latency for them to be 'paused', 'initted', 'unpaused' I think is much much smaller.

Ugh, lets wait with this exercise of using the 'smp_call_function' sometime at the end of the summer - and see. That functionality should be shared with the PV code path IMHO.

Quoted 3 lines

I've tried to local-migrate a FreeBSD PVHVM guest with 33 vCPUs on my 8-way box, and it seems to be working fine :).

Awesome!

Quoted 3 lines

Roger.

#11 lore raw

Re: [Xen-devel] [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

Quoted 57 lines · Konrad Rzeszutek Wilk
Quoted 49 lines · Roger Pau Monné
Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

In order to suspend them an IPI_SUSPEND is sent to all vCPUs except vCPU#0:

http://fxr.watson.org/fxr/source/kern/subr_smp.c#L289

Which makes all APs call cpususpend_handler, so we know all APs are stuck in a while loop with interrupts disabled:

Quoted 6 lines

http://fxr.watson.org/fxr/source/amd64/amd64/mp_machdep.c#L1459

Then on resume the APs are taken out of the while loop and the first thing they do before returning from the IPI handler is registering the new per-cpu vcpu_info area. But I'm not sure this is something that can be accomplished easily on Linux.

Quoted 4 lines

That is a bit of what the 'stop_machine' would do. It puts all of the CPUs in whatever function you want. But I am not sure of the latency impact - as in what if the migration takes longer and all of the CPUs sit there spinning. Another variant of that is the 'smp_call_function'.

I tested stop_machine() on all CPUs during suspend once and it was
awful: 100s of ms of additional downtime.

Perhaps a hand-rolled IPI-and-park-in-handler would be quicker the full stop_machine().

David

#12 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

Quoted 58 lines · David Vrabel
Quoted 57 lines · Konrad Rzeszutek Wilk
Quoted 55 lines · Roger Pau Monné
Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

In order to suspend them an IPI_SUSPEND is sent to all vCPUs except vCPU#0:

http://fxr.watson.org/fxr/source/kern/subr_smp.c#L289

Which makes all APs call cpususpend_handler, so we know all APs are stuck in a while loop with interrupts disabled:

http://fxr.watson.org/fxr/source/amd64/amd64/mp_machdep.c#L1459

Then on resume the APs are taken out of the while loop and the first thing they do before returning from the IPI handler is registering the new per-cpu vcpu_info area. But I'm not sure this is something that can be accomplished easily on Linux.

Quoted 7 lines
Quoted 4 lines

That is a bit of what the 'stop_machine' would do. It puts all of the CPUs in whatever function you want. But I am not sure of the latency impact - as in what if the migration takes longer and all of the CPUs sit there spinning. Another variant of that is the 'smp_call_function'.

I tested stop_machine() on all CPUs during suspend once and it was
awful: 100s of ms of additional downtime.

Yikes.

Quoted 3 lines

Perhaps a hand-rolled IPI-and-park-in-handler would be quicker the full stop_machine().

But that is clearly a bigger patch than this little bug-fix.

Do you want to just take this patch as is and then later on I can work on prototyping the 'IPI-and-park-in-handler'?

#13 lore raw

Re: [Xen-devel] [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to David Vrabel
Quoted 58 lines · David Vrabel
Quoted 57 lines · Konrad Rzeszutek Wilk
Quoted 55 lines · Roger Pau Monné
Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

In order to suspend them an IPI_SUSPEND is sent to all vCPUs except vCPU#0:

http://fxr.watson.org/fxr/source/kern/subr_smp.c#L289

Which makes all APs call cpususpend_handler, so we know all APs are stuck in a while loop with interrupts disabled:

http://fxr.watson.org/fxr/source/amd64/amd64/mp_machdep.c#L1459

Then on resume the APs are taken out of the while loop and the first thing they do before returning from the IPI handler is registering the new per-cpu vcpu_info area. But I'm not sure this is something that can be accomplished easily on Linux.

Quoted 7 lines
Quoted 4 lines

That is a bit of what the 'stop_machine' would do. It puts all of the CPUs in whatever function you want. But I am not sure of the latency impact - as in what if the migration takes longer and all of the CPUs sit there spinning. Another variant of that is the 'smp_call_function'.

I tested stop_machine() on all CPUs during suspend once and it was
awful: 100s of ms of additional downtime.

Yikes.

Quoted 3 lines

Perhaps a hand-rolled IPI-and-park-in-handler would be quicker the full stop_machine().

But that is clearly a bigger patch than this little bug-fix.

Do you want to just take this patch as is and then later on I can work on prototyping the 'IPI-and-park-in-handler'?

#14 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to Konrad Rzeszutek Wilk
Quoted 57 lines · Konrad Rzeszutek Wilk
Quoted 49 lines · Roger Pau Monné
Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

In order to suspend them an IPI_SUSPEND is sent to all vCPUs except vCPU#0:

http://fxr.watson.org/fxr/source/kern/subr_smp.c#L289

Which makes all APs call cpususpend_handler, so we know all APs are stuck in a while loop with interrupts disabled:

Quoted 6 lines

http://fxr.watson.org/fxr/source/amd64/amd64/mp_machdep.c#L1459

Then on resume the APs are taken out of the while loop and the first thing they do before returning from the IPI handler is registering the new per-cpu vcpu_info area. But I'm not sure this is something that can be accomplished easily on Linux.

Quoted 4 lines

That is a bit of what the 'stop_machine' would do. It puts all of the CPUs in whatever function you want. But I am not sure of the latency impact - as in what if the migration takes longer and all of the CPUs sit there spinning. Another variant of that is the 'smp_call_function'.

I tested stop_machine() on all CPUs during suspend once and it was
awful: 100s of ms of additional downtime.

Perhaps a hand-rolled IPI-and-park-in-handler would be quicker the full stop_machine().

David

#15 lore raw

Re: [Xen-devel] [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to Konrad Rzeszutek Wilk
Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

Yes, this is why the ability to have the toolstack save/restore the secondary vcpu state was added. It's especially important for checkpointing, but it's relevant to regular migrate as a performance improvement too.

It's not just stop-machine, IIRC it's a tonne of udev events relating to cpus off/onlinign etc too and all the userspace activity which that implies.

Ian.

#16 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

Quoted 42 lines · Ian Campbell
Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

Quoted 8 lines

Yes, this is why the ability to have the toolstack save/restore the secondary vcpu state was added. It's especially important for checkpointing, but it's relevant to regular migrate as a performance improvement too.

It's not just stop-machine, IIRC it's a tonne of udev events relating to cpus off/onlinign etc too and all the userspace activity which that implies.

Well, what it's done on FreeBSD is nothing like that, it's called the cpususpend handler, but it's not off-lining CPUs or anything like that, it just places the CPU in a while loop inside of an IPI handler, so we can do something like this will all APs:

while (suspended)
pause();

register_vcpu_info();

So the registration of the vcpu_info area happens just after the CPU is waken from suspension and before it leaves the IPI handler, and it's the CPU itself the one that calls VCPUOP_register_vcpu_info (so we can avoid the gate in Xen that prevents registering the vcpu_info area for CPUs different that ourself).

Roger.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#17 lore raw

Re: [Xen-devel] [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to Ian Campbell
Quoted 42 lines · Ian Campbell
Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

Quoted 8 lines

Yes, this is why the ability to have the toolstack save/restore the secondary vcpu state was added. It's especially important for checkpointing, but it's relevant to regular migrate as a performance improvement too.

It's not just stop-machine, IIRC it's a tonne of udev events relating to cpus off/onlinign etc too and all the userspace activity which that implies.

Well, what it's done on FreeBSD is nothing like that, it's called the cpususpend handler, but it's not off-lining CPUs or anything like that, it just places the CPU in a while loop inside of an IPI handler, so we can do something like this will all APs:

while (suspended)
pause();

register_vcpu_info();

So the registration of the vcpu_info area happens just after the CPU is waken from suspension and before it leaves the IPI handler, and it's the CPU itself the one that calls VCPUOP_register_vcpu_info (so we can avoid the gate in Xen that prevents registering the vcpu_info area for CPUs different that ourself).

Roger.

#18 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to Konrad Rzeszutek Wilk
Quoted 35 lines · Konrad Rzeszutek Wilk
Quoted 34 lines · Roger Pau Monné
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

Quoted 5 lines
Quoted 1 line

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

How do you 'suspend' them? If I remember there is a disadvantage of doing this as you have to bring all the CPUs "offline". That in Linux means using the stop_machine which is pretty big hammer and increases the latency for migration.

Yes, this is why the ability to have the toolstack save/restore the secondary vcpu state was added. It's especially important for checkpointing, but it's relevant to regular migrate as a performance improvement too.

It's not just stop-machine, IIRC it's a tonne of udev events relating to cpus off/onlinign etc too and all the userspace activity which that implies.

Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#19 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to [email protected]
Quoted 19 lines · On 08/04/14 19:25, [email protected] wrote:

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

Quoted 8 lines

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

Hmmm, this looks like a very convoluted approach to something that could be solved more easily IMHO. What we do on FreeBSD is put all vCPUs into suspension, which means that all vCPUs except vCPU#0 will be in the cpususpend_handler, see:

http://svnweb.freebsd.org/base/head/sys/amd64/amd64/mp_machdep.c?revision=263878&view=markup#l1460

Then on resume we unblock the "suspended" CPUs, and the first thing they do is call cpu_ops.cpu_resume which is basically going to setup the vcpu_info using VCPUOP_register_vcpu_info. Not sure if something similar is possible under Linux, but it seems easier and doesn't require any Xen-side changes.

Roger.

#20 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to [email protected]

xen/arch/x86/hvm/hvm.c static long hvm_vcpu_op( · line 3475

Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 08.04.14 at 19:25, <[email protected]> wrote:
xen/arch/x86/hvm/hvm.c +3 -0
@@ -3470,6 +3470,9 @@ static long hvm_vcpu_op(
     case VCPUOP_stop_singleshot_timer:
     case VCPUOP_register_vcpu_info:
     case VCPUOP_register_vcpu_time_memory_area:
+    case VCPUOP_down:
+    case VCPUOP_up:
+    case VCPUOP_is_up:

This, if I checked it properly, leaves only VCPUOP_initialise, VCPUOP_send_nmi, and VCPUOP_get_physid disallowed for HVM. None of which look inherently bad to be used by HVM (but VCPUOP_initialise certainly would need closer checking), so I wonder whether either the wrapper shouldn't be dropped altogether or at least be converted from a white list approach to a black list one.

Jan

#21 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to [email protected]

xen/arch/x86/hvm/hvm.c static long hvm_vcpu_op( · line 3475

Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 08.04.14 at 19:25, <[email protected]> wrote:
xen/arch/x86/hvm/hvm.c +3 -0
@@ -3470,6 +3470,9 @@ static long hvm_vcpu_op(
     case VCPUOP_stop_singleshot_timer:
     case VCPUOP_register_vcpu_info:
     case VCPUOP_register_vcpu_time_memory_area:
+    case VCPUOP_down:
+    case VCPUOP_up:
+    case VCPUOP_is_up:

This, if I checked it properly, leaves only VCPUOP_initialise, VCPUOP_send_nmi, and VCPUOP_get_physid disallowed for HVM. None of which look inherently bad to be used by HVM (but VCPUOP_initialise certainly would need closer checking), so I wonder whether either the wrapper shouldn't be dropped altogether or at least be converted from a white list approach to a black list one.

Jan

#22 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

Quoted 17 lines · Jan Beulich

xen/arch/x86/hvm/hvm.c static long hvm_vcpu_op( · line 3475

Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 08.04.14 at 19:25, <[email protected]> wrote:
xen/arch/x86/hvm/hvm.c +3 -0
@@ -3470,6 +3470,9 @@ static long hvm_vcpu_op(
     case VCPUOP_stop_singleshot_timer:
     case VCPUOP_register_vcpu_info:
     case VCPUOP_register_vcpu_time_memory_area:
+    case VCPUOP_down:
+    case VCPUOP_up:
+    case VCPUOP_is_up:

This, if I checked it properly, leaves only VCPUOP_initialise, VCPUOP_send_nmi, and VCPUOP_get_physid disallowed for HVM. None of which look inherently bad to be used by HVM (but VCPUOP_initialise certainly would need closer checking), so I wonder whether either the wrapper shouldn't be dropped altogether or at least be converted from a white list approach to a black list one.

I was being conservative here because I did not want to allow the other ones without at least testing it.

Perhaps that can be done as a seperate patch and this just as a bug-fix?

Quoted 3 lines

Jan

#23 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to Jan Beulich
Quoted 17 lines · Jan Beulich

xen/arch/x86/hvm/hvm.c static long hvm_vcpu_op( · line 3475

Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 08.04.14 at 19:25, <[email protected]> wrote:
xen/arch/x86/hvm/hvm.c +3 -0
@@ -3470,6 +3470,9 @@ static long hvm_vcpu_op(
     case VCPUOP_stop_singleshot_timer:
     case VCPUOP_register_vcpu_info:
     case VCPUOP_register_vcpu_time_memory_area:
+    case VCPUOP_down:
+    case VCPUOP_up:
+    case VCPUOP_is_up:

This, if I checked it properly, leaves only VCPUOP_initialise, VCPUOP_send_nmi, and VCPUOP_get_physid disallowed for HVM. None of which look inherently bad to be used by HVM (but VCPUOP_initialise certainly would need closer checking), so I wonder whether either the wrapper shouldn't be dropped altogether or at least be converted from a white list approach to a black list one.

I was being conservative here because I did not want to allow the other ones without at least testing it.

Perhaps that can be done as a seperate patch and this just as a bug-fix?

Quoted 3 lines

Jan

#24 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

Quoted 20 lines
Quoted 1 line
Quoted 1 line · On 09.04.14 at 17:27, <[email protected]> wrote:
Quoted 17 lines · Jan Beulich

xen/arch/x86/hvm/hvm.c static long hvm_vcpu_op( · line 3475

Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 08.04.14 at 19:25, <[email protected]> wrote:
xen/arch/x86/hvm/hvm.c +3 -0
@@ -3470,6 +3470,9 @@ static long hvm_vcpu_op(
     case VCPUOP_stop_singleshot_timer:
     case VCPUOP_register_vcpu_info:
     case VCPUOP_register_vcpu_time_memory_area:
+    case VCPUOP_down:
+    case VCPUOP_up:
+    case VCPUOP_is_up:

This, if I checked it properly, leaves only VCPUOP_initialise, VCPUOP_send_nmi, and VCPUOP_get_physid disallowed for HVM. None of which look inherently bad to be used by HVM (but VCPUOP_initialise certainly would need closer checking), so I wonder whether either the wrapper shouldn't be dropped altogether or at least be converted from a white list approach to a black list one.

Quoted 5 lines

I was being conservative here because I did not want to allow the other ones without at least testing it.

Perhaps that can be done as a seperate patch and this just as a bug-fix?

I'm clearly not in favor of the patch as is - minimally I'd want it to convert the white list to a black list. And once you do this it would seem rather natural to not pointlessly add entries.

Jan

#25 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to Konrad Rzeszutek Wilk
Quoted 20 lines
Quoted 1 line
Quoted 1 line · On 09.04.14 at 17:27, <[email protected]> wrote:
Quoted 17 lines · Jan Beulich

xen/arch/x86/hvm/hvm.c static long hvm_vcpu_op( · line 3475

Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 08.04.14 at 19:25, <[email protected]> wrote:
xen/arch/x86/hvm/hvm.c +3 -0
@@ -3470,6 +3470,9 @@ static long hvm_vcpu_op(
     case VCPUOP_stop_singleshot_timer:
     case VCPUOP_register_vcpu_info:
     case VCPUOP_register_vcpu_time_memory_area:
+    case VCPUOP_down:
+    case VCPUOP_up:
+    case VCPUOP_is_up:

This, if I checked it properly, leaves only VCPUOP_initialise, VCPUOP_send_nmi, and VCPUOP_get_physid disallowed for HVM. None of which look inherently bad to be used by HVM (but VCPUOP_initialise certainly would need closer checking), so I wonder whether either the wrapper shouldn't be dropped altogether or at least be converted from a white list approach to a black list one.

Quoted 5 lines

I was being conservative here because I did not want to allow the other ones without at least testing it.

Perhaps that can be done as a seperate patch and this just as a bug-fix?

I'm clearly not in favor of the patch as is - minimally I'd want it to convert the white list to a black list. And once you do this it would seem rather natural to not pointlessly add entries.

Jan

#26 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

Quoted 23 lines · Jan Beulich
Quoted 23 lines
Quoted 1 line
Quoted 1 line · On 09.04.14 at 17:27, <[email protected]> wrote:
Quoted 17 lines · Jan Beulich

xen/arch/x86/hvm/hvm.c static long hvm_vcpu_op( · line 3475

Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 08.04.14 at 19:25, <[email protected]> wrote:
xen/arch/x86/hvm/hvm.c +3 -0
@@ -3470,6 +3470,9 @@ static long hvm_vcpu_op(
     case VCPUOP_stop_singleshot_timer:
     case VCPUOP_register_vcpu_info:
     case VCPUOP_register_vcpu_time_memory_area:
+    case VCPUOP_down:
+    case VCPUOP_up:
+    case VCPUOP_is_up:

This, if I checked it properly, leaves only VCPUOP_initialise, VCPUOP_send_nmi, and VCPUOP_get_physid disallowed for HVM. None of which look inherently bad to be used by HVM (but VCPUOP_initialise certainly would need closer checking), so I wonder whether either the wrapper shouldn't be dropped altogether or at least be converted from a white list approach to a black list one.

I was being conservative here because I did not want to allow the other ones without at least testing it.

Quoted 6 lines
Quoted 2 lines

Perhaps that can be done as a seperate patch and this just as a bug-fix?

I'm clearly not in favor of the patch as is - minimally I'd want it to convert the white list to a black list. And once you do this it would seem rather natural to not pointlessly add entries.

With this patch:

xen/arch/x86/hvm/hvm.c +2 -54
index b5b92fe..5eee790 100644
@@ -3455,34 +3455,6 @@ static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     }
 }
 
-static long hvm_vcpu_op(
-    int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
-{
-    long rc;
-
-    switch ( cmd )
-    {
-    case VCPUOP_register_runstate_memory_area:
-    case VCPUOP_get_runstate_info:
-    case VCPUOP_set_periodic_timer:
-    case VCPUOP_stop_periodic_timer:
-    case VCPUOP_set_singleshot_timer:
-    case VCPUOP_stop_singleshot_timer:
-    case VCPUOP_register_vcpu_info:
-    case VCPUOP_register_vcpu_time_memory_area:
-    case VCPUOP_down:
-    case VCPUOP_up:
-    case VCPUOP_is_up:
-        rc = do_vcpu_op(cmd, vcpuid, arg);
-        break;
-    default:
-        rc = -ENOSYS;
-        break;
-    }
-
-    return rc;
-}
-
 typedef unsigned long hvm_hypercall_t(
     unsigned long, unsigned long, unsigned long, unsigned long, unsigned long,
     unsigned long);
@@ -3517,30 +3489,6 @@ static long hvm_memory_op_compat32(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     return compat_memory_op(cmd, arg);
 }
 
-static long hvm_vcpu_op_compat32(
-    int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
-{
-    long rc;
-
-    switch ( cmd )
-    {
-    case VCPUOP_register_runstate_memory_area:
-    case VCPUOP_get_runstate_info:
-    case VCPUOP_set_periodic_timer:
-    case VCPUOP_stop_periodic_timer:
-    case VCPUOP_set_singleshot_timer:
-    case VCPUOP_stop_singleshot_timer:
-    case VCPUOP_register_vcpu_info:
-    case VCPUOP_register_vcpu_time_memory_area:
-        rc = compat_vcpu_op(cmd, vcpuid, arg);
-        break;
-    default:
-        rc = -ENOSYS;
-        break;
-    }
-
-    return rc;
-}
 
 static long hvm_physdev_op_compat32(
     int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
@@ -3563,7 +3511,7 @@ static long hvm_physdev_op_compat32(
 static hvm_hypercall_t *const hvm_hypercall64_table[NR_hypercalls] = {
     [ __HYPERVISOR_memory_op ] = (hvm_hypercall_t *)hvm_memory_op,
     [ __HYPERVISOR_grant_table_op ] = (hvm_hypercall_t *)hvm_grant_table_op,
-    [ __HYPERVISOR_vcpu_op ] = (hvm_hypercall_t *)hvm_vcpu_op,
+    HYPERCALL(vcpu_op),
     [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op,
     HYPERCALL(xen_version),
     HYPERCALL(console_io),
@@ -3583,7 +3531,7 @@ static hvm_hypercall_t *const hvm_hypercall64_table[NR_hypercalls] = {
 static hvm_hypercall_t *const hvm_hypercall32_table[NR_hypercalls] = {
     [ __HYPERVISOR_memory_op ] = (hvm_hypercall_t *)hvm_memory_op_compat32,
     [ __HYPERVISOR_grant_table_op ] = (hvm_hypercall_t *)hvm_grant_table_op_compat32,
-    [ __HYPERVISOR_vcpu_op ] = (hvm_hypercall_t *)hvm_vcpu_op_compat32,
+    COMPAT_CALL(vcpu_op),
     [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op_compat32,
     COMPAT_CALL(xen_version),
     HYPERCALL(console_io),

And with an HVM guest poking at the rest of VCPUOPs: VCPUOP_get_physid, VCPUOP_initialise, and VCPUOP_send_nmi - either before the CPU is up or when it is up - work.

That is: the VCPUOP_get_physid would return -EINVAL; VCPUOP_initialise would return either -EEXIST or 0, and in either case
the content of the ctxt was full of zeros.

The VCPUOP_send_nmi did cause the HVM to get an NMI and it spitted out 'Dazed and confused'. It also noticed corruption:

[    3.611742] Corrupted low memory at c000fffc (fffc phys) = 00029b00
[    2.386785] Corrupted low memory at ffff88000000fff8 (fff8 phys) = 2990000000000

Which is odd because there does not seem to be anything in the path of hypervisor that would cause this.

Quoted 3 lines

Jan

#27 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to Jan Beulich
Quoted 23 lines · Jan Beulich
Quoted 23 lines
Quoted 1 line
Quoted 1 line · On 09.04.14 at 17:27, <[email protected]> wrote:
Quoted 17 lines · Jan Beulich

xen/arch/x86/hvm/hvm.c static long hvm_vcpu_op( · line 3475

Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 08.04.14 at 19:25, <[email protected]> wrote:
xen/arch/x86/hvm/hvm.c +3 -0
@@ -3470,6 +3470,9 @@ static long hvm_vcpu_op(
     case VCPUOP_stop_singleshot_timer:
     case VCPUOP_register_vcpu_info:
     case VCPUOP_register_vcpu_time_memory_area:
+    case VCPUOP_down:
+    case VCPUOP_up:
+    case VCPUOP_is_up:

This, if I checked it properly, leaves only VCPUOP_initialise, VCPUOP_send_nmi, and VCPUOP_get_physid disallowed for HVM. None of which look inherently bad to be used by HVM (but VCPUOP_initialise certainly would need closer checking), so I wonder whether either the wrapper shouldn't be dropped altogether or at least be converted from a white list approach to a black list one.

I was being conservative here because I did not want to allow the other ones without at least testing it.

Quoted 6 lines
Quoted 2 lines

Perhaps that can be done as a seperate patch and this just as a bug-fix?

I'm clearly not in favor of the patch as is - minimally I'd want it to convert the white list to a black list. And once you do this it would seem rather natural to not pointlessly add entries.

With this patch:

xen/arch/x86/hvm/hvm.c +2 -54
index b5b92fe..5eee790 100644
@@ -3455,34 +3455,6 @@ static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     }
 }
 
-static long hvm_vcpu_op(
-    int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
-{
-    long rc;
-
-    switch ( cmd )
-    {
-    case VCPUOP_register_runstate_memory_area:
-    case VCPUOP_get_runstate_info:
-    case VCPUOP_set_periodic_timer:
-    case VCPUOP_stop_periodic_timer:
-    case VCPUOP_set_singleshot_timer:
-    case VCPUOP_stop_singleshot_timer:
-    case VCPUOP_register_vcpu_info:
-    case VCPUOP_register_vcpu_time_memory_area:
-    case VCPUOP_down:
-    case VCPUOP_up:
-    case VCPUOP_is_up:
-        rc = do_vcpu_op(cmd, vcpuid, arg);
-        break;
-    default:
-        rc = -ENOSYS;
-        break;
-    }
-
-    return rc;
-}
-
 typedef unsigned long hvm_hypercall_t(
     unsigned long, unsigned long, unsigned long, unsigned long, unsigned long,
     unsigned long);
@@ -3517,30 +3489,6 @@ static long hvm_memory_op_compat32(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     return compat_memory_op(cmd, arg);
 }
 
-static long hvm_vcpu_op_compat32(
-    int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
-{
-    long rc;
-
-    switch ( cmd )
-    {
-    case VCPUOP_register_runstate_memory_area:
-    case VCPUOP_get_runstate_info:
-    case VCPUOP_set_periodic_timer:
-    case VCPUOP_stop_periodic_timer:
-    case VCPUOP_set_singleshot_timer:
-    case VCPUOP_stop_singleshot_timer:
-    case VCPUOP_register_vcpu_info:
-    case VCPUOP_register_vcpu_time_memory_area:
-        rc = compat_vcpu_op(cmd, vcpuid, arg);
-        break;
-    default:
-        rc = -ENOSYS;
-        break;
-    }
-
-    return rc;
-}
 
 static long hvm_physdev_op_compat32(
     int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
@@ -3563,7 +3511,7 @@ static long hvm_physdev_op_compat32(
 static hvm_hypercall_t *const hvm_hypercall64_table[NR_hypercalls] = {
     [ __HYPERVISOR_memory_op ] = (hvm_hypercall_t *)hvm_memory_op,
     [ __HYPERVISOR_grant_table_op ] = (hvm_hypercall_t *)hvm_grant_table_op,
-    [ __HYPERVISOR_vcpu_op ] = (hvm_hypercall_t *)hvm_vcpu_op,
+    HYPERCALL(vcpu_op),
     [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op,
     HYPERCALL(xen_version),
     HYPERCALL(console_io),
@@ -3583,7 +3531,7 @@ static hvm_hypercall_t *const hvm_hypercall64_table[NR_hypercalls] = {
 static hvm_hypercall_t *const hvm_hypercall32_table[NR_hypercalls] = {
     [ __HYPERVISOR_memory_op ] = (hvm_hypercall_t *)hvm_memory_op_compat32,
     [ __HYPERVISOR_grant_table_op ] = (hvm_hypercall_t *)hvm_grant_table_op_compat32,
-    [ __HYPERVISOR_vcpu_op ] = (hvm_hypercall_t *)hvm_vcpu_op_compat32,
+    COMPAT_CALL(vcpu_op),
     [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op_compat32,
     COMPAT_CALL(xen_version),
     HYPERCALL(console_io),

And with an HVM guest poking at the rest of VCPUOPs: VCPUOP_get_physid, VCPUOP_initialise, and VCPUOP_send_nmi - either before the CPU is up or when it is up - work.

That is: the VCPUOP_get_physid would return -EINVAL; VCPUOP_initialise would return either -EEXIST or 0, and in either case
the content of the ctxt was full of zeros.

The VCPUOP_send_nmi did cause the HVM to get an NMI and it spitted out 'Dazed and confused'. It also noticed corruption:

[    3.611742] Corrupted low memory at c000fffc (fffc phys) = 00029b00
[    2.386785] Corrupted low memory at ffff88000000fff8 (fff8 phys) = 2990000000000

Which is odd because there does not seem to be anything in the path of hypervisor that would cause this.

Quoted 3 lines

Jan

#28 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 22.04.14 at 20:34, <[email protected]> wrote:

With this patch:
[...]
And with an HVM guest poking at the rest of VCPUOPs: VCPUOP_get_physid, VCPUOP_initialise, and VCPUOP_send_nmi - either before the CPU is up or when it is up - work.

That is: the VCPUOP_get_physid would return -EINVAL; VCPUOP_initialise would return either -EEXIST or 0, and in either case
the content of the ctxt was full of zeros.

Good.

Quoted 9 lines

The VCPUOP_send_nmi did cause the HVM to get an NMI and it spitted out 'Dazed and confused'. It also noticed corruption:

[    3.611742] Corrupted low memory at c000fffc (fffc phys) = 00029b00
[    2.386785] Corrupted low memory at ffff88000000fff8 (fff8 phys) = 

2990000000000

Which is odd because there does not seem to be anything in the path of hypervisor that would cause this.

Indeed. This looks a little like a segment descriptor got modified here with a descriptor table base of zero and a selector of 0xfff8. That corruption needs to be hunted down in any case before enabling VCPUOP_send_nmi for HVM.

Jan

#29 lore raw

Re: [XEN PATCH 1/2] hvm: Support more than 32 VCPUS when migrating.

In reply to Konrad Rzeszutek Wilk
Quoted 10 lines
Quoted 1 line
Quoted 1 line · On 22.04.14 at 20:34, <[email protected]> wrote:

With this patch:
[...]
And with an HVM guest poking at the rest of VCPUOPs: VCPUOP_get_physid, VCPUOP_initialise, and VCPUOP_send_nmi - either before the CPU is up or when it is up - work.

That is: the VCPUOP_get_physid would return -EINVAL; VCPUOP_initialise would return either -EEXIST or 0, and in either case
the content of the ctxt was full of zeros.

Good.

Quoted 9 lines

The VCPUOP_send_nmi did cause the HVM to get an NMI and it spitted out 'Dazed and confused'. It also noticed corruption:

[    3.611742] Corrupted low memory at c000fffc (fffc phys) = 00029b00
[    2.386785] Corrupted low memory at ffff88000000fff8 (fff8 phys) = 

2990000000000

Which is odd because there does not seem to be anything in the path of hypervisor that would cause this.

Indeed. This looks a little like a segment descriptor got modified here with a descriptor table base of zero and a selector of 0xfff8. That corruption needs to be hunted down in any case before enabling VCPUOP_send_nmi for HVM.

Jan

#30 lore raw

PATCH 1/2 hvm: Support more than 32 VCPUS when migrating.

In reply to [email protected]

From: Konrad Rzeszutek Wilk <[email protected]>

When we migrate an HVM guest, by default our shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated fresh by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can do make the VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. This patch enables this.

---
 xen/arch/x86/hvm/hvm.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
xen/arch/x86/hvm/hvm.c +3 -0
index 38c491e..b5b92fe 100644
@@ -3470,6 +3470,9 @@ static long hvm_vcpu_op(
     case VCPUOP_stop_singleshot_timer:
     case VCPUOP_register_vcpu_info:
     case VCPUOP_register_vcpu_time_memory_area:
+    case VCPUOP_down:
+    case VCPUOP_up:
+    case VCPUOP_is_up:
         rc = do_vcpu_op(cmd, vcpuid, arg);
         break;
     default:
Signature · 1 line
1.7.7.6
#31 lore raw

PATCH 2/2 xen/pvhvm: Support more than 32 VCPUs when migrating.

In reply to [email protected]

From: Konrad Rzeszutek Wilk <[email protected]>

When Xen migrates an HVM guest, by default its shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated freshly by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can make an VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition in Xen:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. However with the Xen commit "hvm: Support more than 32 VCPUS when migrating." we can do this. As such first check if VCPUOP_is_up is actually possible before trying this dance.

As most of this dance code is done already in 'xen_setup_vcpu' lets make it callable on both PV and HVM. This means moving one of the checks out to 'xen_setup_runstate_info'.

---
 arch/x86/xen/enlighten.c |   21 ++++++++++++++++-----
 arch/x86/xen/suspend.c   |    6 +-----
 arch/x86/xen/time.c      |    3 +++
 3 files changed, 20 insertions(+), 10 deletions(-)
arch/x86/xen/enlighten.c +16 -5
index 201d09a..af8be96 100644
@@ -236,12 +236,23 @@ static void xen_vcpu_setup(int cpu)
 void xen_vcpu_restore(void)
 {
 	int cpu;
+	bool vcpuops = true;
+	const struct cpumask *mask;
 
-	for_each_possible_cpu(cpu) {
+	mask = xen_pv_domain() ? cpu_possible_mask : cpu_online_mask;
+
+	/* Only Xen 4.5 and higher supports this. */
+	if (HYPERVISOR_vcpu_op(VCPUOP_is_up, smp_processor_id(), NULL) == -ENOSYS)
+		vcpuops = false;
+
+	for_each_cpu(cpu, mask) {
 		bool other_cpu = (cpu != smp_processor_id());
-		bool is_up = HYPERVISOR_vcpu_op(VCPUOP_is_up, cpu, NULL);
+		bool is_up = false;
+
+		if (vcpuops)
+			is_up = HYPERVISOR_vcpu_op(VCPUOP_is_up, cpu, NULL);
 
-		if (other_cpu && is_up &&
+		if (vcpuops && other_cpu && is_up &&
 		    HYPERVISOR_vcpu_op(VCPUOP_down, cpu, NULL))
 			BUG();
 
@@ -250,7 +261,7 @@ void xen_vcpu_restore(void)
 		if (have_vcpu_info_placement)
 			xen_vcpu_setup(cpu);
 
-		if (other_cpu && is_up &&
+		if (vcpuops && other_cpu && is_up &&
 		    HYPERVISOR_vcpu_op(VCPUOP_up, cpu, NULL))
 			BUG();
 	}
@@ -1751,7 +1762,7 @@ void __ref xen_hvm_init_shared_info(void)
 	for_each_online_cpu(cpu) {
 		/* Leave it to be NULL. */
 		if (cpu >= MAX_VIRT_CPUS)
-			continue;
+			per_cpu(xen_vcpu, cpu) = NULL; /* Triggers xen_vcpu_setup.*/
 		per_cpu(xen_vcpu, cpu) = &HYPERVISOR_shared_info->vcpu_info[cpu];
 	}
 }
arch/x86/xen/suspend.c +1 -5
index 45329c8..6fb3298 100644
@@ -33,11 +33,7 @@ void xen_arch_hvm_post_suspend(int suspend_cancelled)
 	xen_hvm_init_shared_info();
 	xen_callback_vector();
 	xen_unplug_emulated_devices();
-	if (xen_feature(XENFEAT_hvm_safe_pvclock)) {
-		for_each_online_cpu(cpu) {
-			xen_setup_runstate_info(cpu);
-		}
-	}
+	xen_vcpu_restore();
 #endif
 }
 
arch/x86/xen/time.c +3 -0
index 7b78f88..d4feb2e 100644
@@ -105,6 +105,9 @@ void xen_setup_runstate_info(int cpu)
 {
 	struct vcpu_register_runstate_memory_area area;
 
+	if (xen_hvm_domain() && !(xen_feature(XENFEAT_hvm_safe_pvclock)))
+		return;
+
 	area.addr.v = &per_cpu(xen_runstate, cpu);
 
 	if (HYPERVISOR_vcpu_op(VCPUOP_register_runstate_memory_area,
Signature · 1 line
1.7.7.6
#32 lore raw

PATCH 2/2 xen/pvhvm: Support more than 32 VCPUs when migrating.

In reply to [email protected]

From: Konrad Rzeszutek Wilk <[email protected]>

When Xen migrates an HVM guest, by default its shared_info can only hold up to 32 CPUs. As such the hypercall VCPUOP_register_vcpu_info was introduced which allowed us to setup per-page areas for VCPUs. This means we can boot PVHVM guest with more than 32 VCPUs. During migration the per-cpu structure is allocated freshly by the hypervisor (vcpu_info_mfn is set to INVALID_MFN) so that the newly migrated guest can make an VCPUOP_register_vcpu_info hypercall.

Unfortunatly we end up triggering this condition in Xen:
/* Run this command on yourself or on other offline VCPUS. */
if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )

which means we are unable to setup the per-cpu VCPU structures for running vCPUS. The Linux PV code paths make this work by iterating over every vCPU with:

1) is target CPU up (VCPUOP_is_up hypercall?)
2) if yes, then VCPUOP_down to pause it.
3) VCPUOP_register_vcpu_info
4) if it was down, then VCPUOP_up to bring it back up

But since VCPUOP_down, VCPUOP_is_up, and VCPUOP_up are not allowed on HVM guests we can't do this. However with the Xen commit "hvm: Support more than 32 VCPUS when migrating." we can do this. As such first check if VCPUOP_is_up is actually possible before trying this dance.

As most of this dance code is done already in 'xen_setup_vcpu' lets make it callable on both PV and HVM. This means moving one of the checks out to 'xen_setup_runstate_info'.

---
 arch/x86/xen/enlighten.c |   21 ++++++++++++++++-----
 arch/x86/xen/suspend.c   |    6 +-----
 arch/x86/xen/time.c      |    3 +++
 3 files changed, 20 insertions(+), 10 deletions(-)
arch/x86/xen/enlighten.c +16 -5
index 201d09a..af8be96 100644
@@ -236,12 +236,23 @@ static void xen_vcpu_setup(int cpu)
 void xen_vcpu_restore(void)
 {
 	int cpu;
+	bool vcpuops = true;
+	const struct cpumask *mask;
 
-	for_each_possible_cpu(cpu) {
+	mask = xen_pv_domain() ? cpu_possible_mask : cpu_online_mask;
+
+	/* Only Xen 4.5 and higher supports this. */
+	if (HYPERVISOR_vcpu_op(VCPUOP_is_up, smp_processor_id(), NULL) == -ENOSYS)
+		vcpuops = false;
+
+	for_each_cpu(cpu, mask) {
 		bool other_cpu = (cpu != smp_processor_id());
-		bool is_up = HYPERVISOR_vcpu_op(VCPUOP_is_up, cpu, NULL);
+		bool is_up = false;
+
+		if (vcpuops)
+			is_up = HYPERVISOR_vcpu_op(VCPUOP_is_up, cpu, NULL);
 
-		if (other_cpu && is_up &&
+		if (vcpuops && other_cpu && is_up &&
 		    HYPERVISOR_vcpu_op(VCPUOP_down, cpu, NULL))
 			BUG();
 
@@ -250,7 +261,7 @@ void xen_vcpu_restore(void)
 		if (have_vcpu_info_placement)
 			xen_vcpu_setup(cpu);
 
-		if (other_cpu && is_up &&
+		if (vcpuops && other_cpu && is_up &&
 		    HYPERVISOR_vcpu_op(VCPUOP_up, cpu, NULL))
 			BUG();
 	}
@@ -1751,7 +1762,7 @@ void __ref xen_hvm_init_shared_info(void)
 	for_each_online_cpu(cpu) {
 		/* Leave it to be NULL. */
 		if (cpu >= MAX_VIRT_CPUS)
-			continue;
+			per_cpu(xen_vcpu, cpu) = NULL; /* Triggers xen_vcpu_setup.*/
 		per_cpu(xen_vcpu, cpu) = &HYPERVISOR_shared_info->vcpu_info[cpu];
 	}
 }
arch/x86/xen/suspend.c +1 -5
index 45329c8..6fb3298 100644
@@ -33,11 +33,7 @@ void xen_arch_hvm_post_suspend(int suspend_cancelled)
 	xen_hvm_init_shared_info();
 	xen_callback_vector();
 	xen_unplug_emulated_devices();
-	if (xen_feature(XENFEAT_hvm_safe_pvclock)) {
-		for_each_online_cpu(cpu) {
-			xen_setup_runstate_info(cpu);
-		}
-	}
+	xen_vcpu_restore();
 #endif
 }
 
arch/x86/xen/time.c +3 -0
index 7b78f88..d4feb2e 100644
@@ -105,6 +105,9 @@ void xen_setup_runstate_info(int cpu)
 {
 	struct vcpu_register_runstate_memory_area area;
 
+	if (xen_hvm_domain() && !(xen_feature(XENFEAT_hvm_safe_pvclock)))
+		return;
+
 	area.addr.v = &per_cpu(xen_runstate, cpu);
 
 	if (HYPERVISOR_vcpu_op(VCPUOP_register_runstate_memory_area,
Signature · 1 line
1.7.7.6
#33 lore raw

Re: [LINUX PATCH 2/2] xen/pvhvm: Support more than 32 VCPUs when migrating.

arch/x86/xen/enlighten.c static void xen_vcpu_setup(int cpu) · line 246

Quoted 4 lines
Quoted 1 line
Quoted 1 line · On 08.04.14 at 19:25, <[email protected]> wrote:
arch/x86/xen/enlighten.c +3 -0
+	/* Only Xen 4.5 and higher supports this. */
+	if (HYPERVISOR_vcpu_op(VCPUOP_is_up, smp_processor_id(), NULL) == -ENOSYS)
+		vcpuops = false;

Did you mean to say "for HVM guests" in the comment? And of course the comment could quickly become stale if we backported the Xen side change to e.g. 4.4.1.

Jan

#34 lore raw

Re: [LINUX PATCH 2/2] xen/pvhvm: Support more than 32 VCPUs when migrating.

In reply to [email protected]

arch/x86/xen/enlighten.c static void xen_vcpu_setup(int cpu) · line 246

Quoted 4 lines
Quoted 1 line
Quoted 1 line · On 08.04.14 at 19:25, <[email protected]> wrote:
arch/x86/xen/enlighten.c +3 -0
+	/* Only Xen 4.5 and higher supports this. */
+	if (HYPERVISOR_vcpu_op(VCPUOP_is_up, smp_processor_id(), NULL) == -ENOSYS)
+		vcpuops = false;

Did you mean to say "for HVM guests" in the comment? And of course the comment could quickly become stale if we backported the Xen side change to e.g. 4.4.1.

Jan

#35 lore raw

PATCH 1/1 Fixes for more than 32 VCPUs migration for HVM guests (v1).

In reply to Ian Campbell

These two patches (one for Linux, one for Xen) allow PVHVM guests to use the per-cpu VCPU mechanism after migration. Currently when an PVHVM guest migrates all the per-cpu information is lost and we fallback on the shared_info structure. This is regardless if the HVM guest has 2 or 128 CPUs. Since the structure has an array for only 32 CPUs that means if we are to migrate a PVHVM guest - we can only do it up to 32 CPUs.

These patches fix it and allow more than 32 VCPUs to be migrated with PVHVM Linux guests.

The Linux diff is:

 arch/x86/xen/enlighten.c |   21 ++++++++++++++++-----
 arch/x86/xen/suspend.c   |    6 +-----
 arch/x86/xen/time.c      |    3 +++
 3 files changed, 20 insertions(+), 10 deletions(-)

while the Xen one is:

 xen/arch/x86/hvm/hvm.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)