* [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore
@ 2017-06-03 0:05 Ankur Arora
2017-06-03 0:05 ` [PATCH 1/5] xen/vcpu: Simplify xen_vcpu related code Ankur Arora
` (6 more replies)
0 siblings, 7 replies; 10+ messages in thread
From: Ankur Arora @ 2017-06-03 0:05 UTC (permalink / raw)
To: linux-kernel, xen-devel; +Cc: boris.ostrovsky, jgross, Ankur Arora
This patch series fixes a bunch of issues in the xen_vcpu setup
logic.
Simplify xen_vcpu related code: code refactoring in advance of the
rest of the patch series.
Support > 32 VCPUs at restore: unify all vcpu restore logic in
xen_vcpu_restore() and support > 32 VCPUs for PVH*.
Remove vcpu info placement from restore (!SMP): some pv_ops are
marked RO after init so lets not redo xen_setup_vcpu_info_placement
at restore.
Handle xen_vcpu_setup() failure in hotplug: handle vcpu_info
registration failures by propagating them from the cpuhp-prepare
callback back up to the cpuhp logic.
Handle xen_vcpu_setup() failure at boot: pull CPUs (> MAX_VIRT_CPUS)
down if we fall back to xen_have_vcpu_info_placement = 0.
Tested with various combinations of PV/PVHv2/PVHVM save/restore
and cpu-hotadd-hotremove. Also tested by simulating failure in
VCPUOP_register_vcpu_info.
Please review.
Ankur Arora (5):
xen/vcpu: Simplify xen_vcpu related code
xen/pvh*: Support > 32 VCPUs at domain restore
xen/pv: Fix OOPS on restore for a PV, !SMP domain
xen/vcpu: Handle xen_vcpu_setup() failure in hotplug
xen/vcpu: Handle xen_vcpu_setup() failure at boot
arch/x86/xen/enlighten.c | 154 +++++++++++++++++++++++++++++++------------
arch/x86/xen/enlighten_hvm.c | 33 ++++------
arch/x86/xen/enlighten_pv.c | 87 +++++++++++-------------
arch/x86/xen/smp.c | 31 +++++++++
arch/x86/xen/smp.h | 2 +
arch/x86/xen/smp_hvm.c | 14 +++-
arch/x86/xen/smp_pv.c | 6 +-
arch/x86/xen/suspend_hvm.c | 11 +---
arch/x86/xen/xen-ops.h | 3 +-
include/xen/xen-ops.h | 2 +
10 files changed, 218 insertions(+), 125 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/5] xen/vcpu: Simplify xen_vcpu related code
2017-06-03 0:05 [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Ankur Arora
@ 2017-06-03 0:05 ` Ankur Arora
2017-06-03 0:05 ` [PATCH 2/5] xen/pvh*: Support > 32 VCPUs at domain restore Ankur Arora
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ankur Arora @ 2017-06-03 0:05 UTC (permalink / raw)
To: linux-kernel, xen-devel; +Cc: boris.ostrovsky, jgross, Ankur Arora
Largely mechanical changes to aid unification of xen_vcpu_restore()
logic for PV, PVH and PVHVM.
xen_vcpu_setup(): the only change in logic is that clamp_max_cpus()
is now handled inside the "if (!xen_have_vcpu_info_placement)" block.
xen_vcpu_restore(): code movement from enlighten_pv.c to enlighten.c.
xen_vcpu_info_reset(): pulls together all the code where xen_vcpu
is set to default.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
arch/x86/xen/enlighten.c | 101 +++++++++++++++++++++++++++++++------------
arch/x86/xen/enlighten_hvm.c | 6 +--
arch/x86/xen/enlighten_pv.c | 47 +++++---------------
arch/x86/xen/smp_hvm.c | 3 +-
arch/x86/xen/xen-ops.h | 1 +
5 files changed, 89 insertions(+), 69 deletions(-)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index a5ffcbb20cc0..96b745e3f56c 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -106,6 +106,35 @@ int xen_cpuhp_setup(int (*cpu_up_prepare_cb)(unsigned int),
return rc >= 0 ? 0 : rc;
}
+/*
+ * On restore, set the vcpu placement up again.
+ * If it fails, then we're in a bad state, since
+ * we can't back out from using it...
+ */
+void xen_vcpu_restore(void)
+{
+ int cpu;
+
+ for_each_possible_cpu(cpu) {
+ bool other_cpu = (cpu != smp_processor_id());
+ bool is_up = HYPERVISOR_vcpu_op(VCPUOP_is_up, xen_vcpu_nr(cpu),
+ NULL);
+
+ if (other_cpu && is_up &&
+ HYPERVISOR_vcpu_op(VCPUOP_down, xen_vcpu_nr(cpu), NULL))
+ BUG();
+
+ xen_setup_runstate_info(cpu);
+
+ if (xen_have_vcpu_info_placement)
+ xen_vcpu_setup(cpu);
+
+ if (other_cpu && is_up &&
+ HYPERVISOR_vcpu_op(VCPUOP_up, xen_vcpu_nr(cpu), NULL))
+ BUG();
+ }
+}
+
static void clamp_max_cpus(void)
{
#ifdef CONFIG_SMP
@@ -114,6 +143,17 @@ static void clamp_max_cpus(void)
#endif
}
+void xen_vcpu_info_reset(int cpu)
+{
+ if (xen_vcpu_nr(cpu) < MAX_VIRT_CPUS) {
+ per_cpu(xen_vcpu, cpu) =
+ &HYPERVISOR_shared_info->vcpu_info[xen_vcpu_nr(cpu)];
+ } else {
+ /* Set to NULL so that if somebody accesses it we get an OOPS */
+ per_cpu(xen_vcpu, cpu) = NULL;
+ }
+}
+
void xen_vcpu_setup(int cpu)
{
struct vcpu_register_vcpu_info info;
@@ -137,40 +177,45 @@ void xen_vcpu_setup(int cpu)
if (per_cpu(xen_vcpu, cpu) == &per_cpu(xen_vcpu_info, cpu))
return;
}
- if (xen_vcpu_nr(cpu) < MAX_VIRT_CPUS)
- per_cpu(xen_vcpu, cpu) =
- &HYPERVISOR_shared_info->vcpu_info[xen_vcpu_nr(cpu)];
+
+ xen_vcpu_info_reset(cpu);
+
+ if (xen_have_vcpu_info_placement) {
+ vcpup = &per_cpu(xen_vcpu_info, cpu);
+ info.mfn = arbitrary_virt_to_mfn(vcpup);
+ info.offset = offset_in_page(vcpup);
+
+ /*
+ * Check to see if the hypervisor will put the vcpu_info
+ * structure where we want it, which allows direct access via
+ * a percpu-variable.
+ * N.B. This hypercall can _only_ be called once per CPU.
+ * Subsequent calls will error out with -EINVAL. This is due to
+ * the fact that hypervisor has no unregister variant and this
+ * hypercall does not allow to over-write info.mfn and
+ * info.offset.
+ */
+ err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info,
+ xen_vcpu_nr(cpu), &info);
+
+ if (err) {
+ pr_warn_once("register_vcpu_info failed: cpu=%d err=%d\n",
+ cpu, err);
+ xen_have_vcpu_info_placement = 0;
+ } else {
+ /*
+ * This cpu is using the registered vcpu info, even if
+ * later ones fail to.
+ */
+ per_cpu(xen_vcpu, cpu) = vcpup;
+ }
+ }
if (!xen_have_vcpu_info_placement) {
if (cpu >= MAX_VIRT_CPUS)
clamp_max_cpus();
return;
}
-
- vcpup = &per_cpu(xen_vcpu_info, cpu);
- info.mfn = arbitrary_virt_to_mfn(vcpup);
- info.offset = offset_in_page(vcpup);
-
- /* Check to see if the hypervisor will put the vcpu_info
- structure where we want it, which allows direct access via
- a percpu-variable.
- N.B. This hypercall can _only_ be called once per CPU. Subsequent
- calls will error out with -EINVAL. This is due to the fact that
- hypervisor has no unregister variant and this hypercall does not
- allow to over-write info.mfn and info.offset.
- */
- err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, xen_vcpu_nr(cpu),
- &info);
-
- if (err) {
- printk(KERN_DEBUG "register_vcpu_info failed: err=%d\n", err);
- xen_have_vcpu_info_placement = 0;
- clamp_max_cpus();
- } else {
- /* This cpu is using the registered vcpu info, even if
- later ones fail to. */
- per_cpu(xen_vcpu, cpu) = vcpup;
- }
}
void xen_reboot(int reason)
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index a6d014f47e52..eb53da6547ee 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -45,11 +45,7 @@ void __ref xen_hvm_init_shared_info(void)
* online but xen_hvm_init_shared_info is run at resume time too and
* in that case multiple vcpus might be online. */
for_each_online_cpu(cpu) {
- /* Leave it to be NULL. */
- if (xen_vcpu_nr(cpu) >= MAX_VIRT_CPUS)
- continue;
- per_cpu(xen_vcpu, cpu) =
- &HYPERVISOR_shared_info->vcpu_info[xen_vcpu_nr(cpu)];
+ xen_vcpu_info_reset(cpu);
}
}
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 7cd442690f9d..f51e48299692 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -107,35 +107,6 @@ struct tls_descs {
*/
static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
-/*
- * On restore, set the vcpu placement up again.
- * If it fails, then we're in a bad state, since
- * we can't back out from using it...
- */
-void xen_vcpu_restore(void)
-{
- int cpu;
-
- for_each_possible_cpu(cpu) {
- bool other_cpu = (cpu != smp_processor_id());
- bool is_up = HYPERVISOR_vcpu_op(VCPUOP_is_up, xen_vcpu_nr(cpu),
- NULL);
-
- if (other_cpu && is_up &&
- HYPERVISOR_vcpu_op(VCPUOP_down, xen_vcpu_nr(cpu), NULL))
- BUG();
-
- xen_setup_runstate_info(cpu);
-
- if (xen_have_vcpu_info_placement)
- xen_vcpu_setup(cpu);
-
- if (other_cpu && is_up &&
- HYPERVISOR_vcpu_op(VCPUOP_up, xen_vcpu_nr(cpu), NULL))
- BUG();
- }
-}
-
static void __init xen_banner(void)
{
unsigned version = HYPERVISOR_xen_version(XENVER_version, NULL);
@@ -1339,9 +1310,17 @@ asmlinkage __visible void __init xen_start_kernel(void)
*/
acpi_numa = -1;
#endif
- /* Don't do the full vcpu_info placement stuff until we have a
- possible map and a non-dummy shared_info. */
- per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
+ /* Let's presume PV guests always boot on vCPU with id 0. */
+ per_cpu(xen_vcpu_id, 0) = 0;
+
+ /*
+ * Setup xen_vcpu early because start_kernel needs it for
+ * local_irq_disable(), irqs_disabled().
+ *
+ * Don't do the full vcpu_info placement stuff until we have
+ * the cpu_possible_mask and a non-dummy shared_info.
+ */
+ xen_vcpu_info_reset(0);
WARN_ON(xen_cpuhp_setup(xen_cpu_up_prepare_pv, xen_cpu_dead_pv));
@@ -1438,9 +1417,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
#endif
xen_raw_console_write("about to get started...\n");
- /* Let's presume PV guests always boot on vCPU with id 0. */
- per_cpu(xen_vcpu_id, 0) = 0;
-
+ /* We need this for printk timestamps */
xen_setup_runstate_info(0);
xen_efi_init();
diff --git a/arch/x86/xen/smp_hvm.c b/arch/x86/xen/smp_hvm.c
index f18561bbf5c9..9e0fb9a015d4 100644
--- a/arch/x86/xen/smp_hvm.c
+++ b/arch/x86/xen/smp_hvm.c
@@ -12,7 +12,8 @@ static void __init xen_hvm_smp_prepare_boot_cpu(void)
native_smp_prepare_boot_cpu();
/*
- * Setup vcpu_info for boot CPU.
+ * Setup vcpu_info for boot CPU. Secondary CPUs get their vcpu_info
+ * in xen_cpu_up_prepare_hvm().
*/
xen_vcpu_setup(0);
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 9a440a42c618..90828256248b 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -79,6 +79,7 @@ bool xen_vcpu_stolen(int vcpu);
extern int xen_have_vcpu_info_placement;
void xen_vcpu_setup(int cpu);
+void xen_vcpu_info_reset(int cpu);
void xen_setup_vcpu_info_placement(void);
#ifdef CONFIG_SMP
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] xen/pvh*: Support > 32 VCPUs at domain restore
2017-06-03 0:05 [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Ankur Arora
2017-06-03 0:05 ` [PATCH 1/5] xen/vcpu: Simplify xen_vcpu related code Ankur Arora
@ 2017-06-03 0:05 ` Ankur Arora
2017-06-03 0:06 ` [PATCH 3/5] xen/pv: Fix OOPS on restore for a PV, !SMP domain Ankur Arora
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ankur Arora @ 2017-06-03 0:05 UTC (permalink / raw)
To: linux-kernel, xen-devel; +Cc: boris.ostrovsky, jgross, Ankur Arora
When Xen restores a PVHVM or PVH guest, its shared_info only holds
up to 32 CPUs. The hypercall VCPUOP_register_vcpu_info allows
us to setup per-page areas for VCPUs. This means we can boot
PVH* guests with more than 32 VCPUs. During restore the per-cpu
structure is allocated freshly by the hypervisor (vcpu_info_mfn is
set to INVALID_MFN) so that the newly restored guest can make a
VCPUOP_register_vcpu_info hypercall.
However, 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 makes this work by
iterating over cpu_possible in xen_vcpu_restore() 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
With Xen commit 192df6f9122d ("xen/x86: allow HVM guests to use
hypercalls to bring up vCPUs") this is available for non-PV guests.
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_vcpu_restore()
let's make it callable on PV, PVH and PVHVM.
Based-on-patch-by: Konrad Wilk <konrad.wilk@oracle.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
arch/x86/xen/enlighten.c | 45 +++++++++++++++++++++++++++++++-------------
arch/x86/xen/enlighten_hvm.c | 20 +++++++-------------
arch/x86/xen/smp_hvm.c | 10 ++++++++++
arch/x86/xen/suspend_hvm.c | 11 +++--------
include/xen/xen-ops.h | 2 ++
5 files changed, 54 insertions(+), 34 deletions(-)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 96b745e3f56c..276cc21619ec 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -106,6 +106,21 @@ int xen_cpuhp_setup(int (*cpu_up_prepare_cb)(unsigned int),
return rc >= 0 ? 0 : rc;
}
+static void xen_vcpu_setup_restore(int cpu)
+{
+ /* Any per_cpu(xen_vcpu) is stale, so reset it */
+ xen_vcpu_info_reset(cpu);
+
+ /*
+ * For PVH and PVHVM, setup online VCPUs only. The rest will
+ * be handled by hotplug.
+ */
+ if (xen_pv_domain() ||
+ (xen_hvm_domain() && cpu_online(cpu))) {
+ xen_vcpu_setup(cpu);
+ }
+}
+
/*
* On restore, set the vcpu placement up again.
* If it fails, then we're in a bad state, since
@@ -117,17 +132,23 @@ void xen_vcpu_restore(void)
for_each_possible_cpu(cpu) {
bool other_cpu = (cpu != smp_processor_id());
- bool is_up = HYPERVISOR_vcpu_op(VCPUOP_is_up, xen_vcpu_nr(cpu),
- NULL);
+ bool is_up;
+
+ if (xen_vcpu_nr(cpu) == XEN_VCPU_ID_INVALID)
+ continue;
+
+ /* Only Xen 4.5 and higher support this. */
+ is_up = HYPERVISOR_vcpu_op(VCPUOP_is_up,
+ xen_vcpu_nr(cpu), NULL) > 0;
if (other_cpu && is_up &&
HYPERVISOR_vcpu_op(VCPUOP_down, xen_vcpu_nr(cpu), NULL))
BUG();
- xen_setup_runstate_info(cpu);
+ if (xen_pv_domain() || xen_feature(XENFEAT_hvm_safe_pvclock))
+ xen_setup_runstate_info(cpu);
- if (xen_have_vcpu_info_placement)
- xen_vcpu_setup(cpu);
+ xen_vcpu_setup_restore(cpu);
if (other_cpu && is_up &&
HYPERVISOR_vcpu_op(VCPUOP_up, xen_vcpu_nr(cpu), NULL))
@@ -163,11 +184,11 @@ void xen_vcpu_setup(int cpu)
BUG_ON(HYPERVISOR_shared_info == &xen_dummy_shared_info);
/*
- * This path is called twice on PVHVM - first during bootup via
- * smp_init -> xen_hvm_cpu_notify, and then if the VCPU is being
- * hotplugged: cpu_up -> xen_hvm_cpu_notify.
- * As we can only do the VCPUOP_register_vcpu_info once lets
- * not over-write its result.
+ * This path is called on PVHVM at bootup (xen_hvm_smp_prepare_boot_cpu)
+ * and at restore (xen_vcpu_restore). Also called for hotplugged
+ * VCPUs (cpu_init -> xen_hvm_cpu_prepare_hvm).
+ * However, the hypercall can only be done once (see below) so if a VCPU
+ * is offlined and comes back online then let's not redo the hypercall.
*
* For PV it is called during restore (xen_vcpu_restore) and bootup
* (xen_setup_vcpu_info_placement). The hotplug mechanism does not
@@ -178,8 +199,6 @@ void xen_vcpu_setup(int cpu)
return;
}
- xen_vcpu_info_reset(cpu);
-
if (xen_have_vcpu_info_placement) {
vcpup = &per_cpu(xen_vcpu_info, cpu);
info.mfn = arbitrary_virt_to_mfn(vcpup);
@@ -214,7 +233,7 @@ void xen_vcpu_setup(int cpu)
if (!xen_have_vcpu_info_placement) {
if (cpu >= MAX_VIRT_CPUS)
clamp_max_cpus();
- return;
+ xen_vcpu_info_reset(cpu);
}
}
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index eb53da6547ee..ba1afadb2512 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -20,7 +20,6 @@
void __ref xen_hvm_init_shared_info(void)
{
- int cpu;
struct xen_add_to_physmap xatp;
static struct shared_info *shared_info_page;
@@ -35,18 +34,6 @@ void __ref xen_hvm_init_shared_info(void)
BUG();
HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
-
- /* xen_vcpu is a pointer to the vcpu_info struct in the shared_info
- * page, we use it in the event channel upcall and in some pvclock
- * related functions. We don't need the vcpu_info placement
- * optimizations because we don't use any pv_mmu or pv_irq op on
- * HVM.
- * When xen_hvm_init_shared_info is run at boot time only vcpu 0 is
- * online but xen_hvm_init_shared_info is run at resume time too and
- * in that case multiple vcpus might be online. */
- for_each_online_cpu(cpu) {
- xen_vcpu_info_reset(cpu);
- }
}
static void __init init_hvm_pv_info(void)
@@ -150,6 +137,13 @@ static void __init xen_hvm_guest_init(void)
xen_hvm_init_shared_info();
+ /*
+ * xen_vcpu is a pointer to the vcpu_info struct in the shared_info
+ * page, we use it in the event channel upcall and in some pvclock
+ * related functions.
+ */
+ xen_vcpu_info_reset(0);
+
xen_panic_handler_init();
if (xen_feature(XENFEAT_hvm_callback_vector))
diff --git a/arch/x86/xen/smp_hvm.c b/arch/x86/xen/smp_hvm.c
index 9e0fb9a015d4..6c8a805819ff 100644
--- a/arch/x86/xen/smp_hvm.c
+++ b/arch/x86/xen/smp_hvm.c
@@ -28,10 +28,20 @@ static void __init xen_hvm_smp_prepare_boot_cpu(void)
static void __init xen_hvm_smp_prepare_cpus(unsigned int max_cpus)
{
+ int cpu;
+
native_smp_prepare_cpus(max_cpus);
WARN_ON(xen_smp_intr_init(0));
xen_init_lock_cpu(0);
+
+ for_each_possible_cpu(cpu) {
+ if (cpu == 0)
+ continue;
+
+ /* Set default vcpu_id to make sure that we don't use cpu-0's */
+ per_cpu(xen_vcpu_id, cpu) = XEN_VCPU_ID_INVALID;
+ }
}
#ifdef CONFIG_HOTPLUG_CPU
diff --git a/arch/x86/xen/suspend_hvm.c b/arch/x86/xen/suspend_hvm.c
index 01afcadde50a..484999416d8b 100644
--- a/arch/x86/xen/suspend_hvm.c
+++ b/arch/x86/xen/suspend_hvm.c
@@ -8,15 +8,10 @@
void xen_hvm_post_suspend(int suspend_cancelled)
{
- int cpu;
-
- if (!suspend_cancelled)
+ if (!suspend_cancelled) {
xen_hvm_init_shared_info();
+ xen_vcpu_restore();
+ }
xen_callback_vector();
xen_unplug_emulated_devices();
- if (xen_feature(XENFEAT_hvm_safe_pvclock)) {
- for_each_online_cpu(cpu) {
- xen_setup_runstate_info(cpu);
- }
- }
}
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index c44a2ee8c8f8..218e6aae5433 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -15,6 +15,8 @@ static inline uint32_t xen_vcpu_nr(int cpu)
return per_cpu(xen_vcpu_id, cpu);
}
+#define XEN_VCPU_ID_INVALID U32_MAX
+
void xen_arch_pre_suspend(void);
void xen_arch_post_suspend(int suspend_cancelled);
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] xen/pv: Fix OOPS on restore for a PV, !SMP domain
2017-06-03 0:05 [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Ankur Arora
2017-06-03 0:05 ` [PATCH 1/5] xen/vcpu: Simplify xen_vcpu related code Ankur Arora
2017-06-03 0:05 ` [PATCH 2/5] xen/pvh*: Support > 32 VCPUs at domain restore Ankur Arora
@ 2017-06-03 0:06 ` Ankur Arora
2017-06-03 0:06 ` [PATCH 4/5] xen/vcpu: Handle xen_vcpu_setup() failure in hotplug Ankur Arora
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ankur Arora @ 2017-06-03 0:06 UTC (permalink / raw)
To: linux-kernel, xen-devel; +Cc: boris.ostrovsky, jgross, Ankur Arora
If CONFIG_SMP is disabled, xen_setup_vcpu_info_placement() is called from
xen_setup_shared_info(). This is fine as far as boot goes, but it means
that we also call it in the restore path. This results in an OOPS
because we assign to pv_mmu_ops.read_cr2 which is __ro_after_init.
Also, though less problematically, this means we call xen_vcpu_setup()
twice at restore -- once from the vcpu info placement call and the
second time from xen_vcpu_restore().
Fix by calling xen_setup_vcpu_info_placement() at boot only.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
arch/x86/xen/enlighten_pv.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index f51e48299692..29cad193db53 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -938,23 +938,27 @@ void xen_setup_shared_info(void)
HYPERVISOR_shared_info =
(struct shared_info *)__va(xen_start_info->shared_info);
-#ifndef CONFIG_SMP
- /* In UP this is as good a place as any to set up shared info */
- xen_setup_vcpu_info_placement();
-#endif
-
xen_setup_mfn_list_list();
- /*
- * Now that shared info is set up we can start using routines that
- * point to pvclock area.
- */
- if (system_state == SYSTEM_BOOTING)
+ if (system_state == SYSTEM_BOOTING) {
+#ifndef CONFIG_SMP
+ /*
+ * In UP this is as good a place as any to set up shared info.
+ * Limit this to boot only, at restore vcpu setup is done via
+ * xen_vcpu_restore().
+ */
+ xen_setup_vcpu_info_placement();
+#endif
+ /*
+ * Now that shared info is set up we can start using routines
+ * that point to pvclock area.
+ */
xen_init_time_ops();
+ }
}
/* This is called once we have the cpu_possible_mask */
-void xen_setup_vcpu_info_placement(void)
+void __ref xen_setup_vcpu_info_placement(void)
{
int cpu;
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] xen/vcpu: Handle xen_vcpu_setup() failure in hotplug
2017-06-03 0:05 [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Ankur Arora
` (2 preceding siblings ...)
2017-06-03 0:06 ` [PATCH 3/5] xen/pv: Fix OOPS on restore for a PV, !SMP domain Ankur Arora
@ 2017-06-03 0:06 ` Ankur Arora
2017-06-03 0:06 ` [PATCH 5/5] xen/vcpu: Handle xen_vcpu_setup() failure at boot Ankur Arora
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Ankur Arora @ 2017-06-03 0:06 UTC (permalink / raw)
To: linux-kernel, xen-devel; +Cc: boris.ostrovsky, jgross, Ankur Arora
The hypercall VCPUOP_register_vcpu_info can fail. This failure is
handled by making per_cpu(xen_vcpu, cpu) point to its shared_info
slot and those without one (cpu >= MAX_VIRT_CPUS) be NULL.
For PVH/PVHVM, this is not enough, because we also need to pull
these VCPUs out of circulation.
Fix for PVH/PVHVM: on registration failure in the cpuhp prepare
callback (xen_cpu_up_prepare_hvm()), return an error to the cpuhp
state-machine so it can fail the CPU init.
Fix for PV: the registration happens before smp_init(), so, in the
failure case we clamp setup_max_cpus and limit the number of VCPUs
that smp_init() will bring-up to MAX_VIRT_CPUS.
This is functionally correct but it makes the code a bit simpler
if we get rid of this explicit clamping: for VCPUs that don't have
valid xen_vcpu, fail the CPU init in the cpuhp prepare callback
(xen_cpu_up_prepare_pv()).
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
arch/x86/xen/enlighten.c | 46 +++++++++++++++++++++++++-------------------
arch/x86/xen/enlighten_hvm.c | 9 +++++----
arch/x86/xen/enlighten_pv.c | 14 +++++++++++++-
arch/x86/xen/xen-ops.h | 2 +-
4 files changed, 45 insertions(+), 26 deletions(-)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 276cc21619ec..0e7ef69e8531 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -106,8 +106,10 @@ int xen_cpuhp_setup(int (*cpu_up_prepare_cb)(unsigned int),
return rc >= 0 ? 0 : rc;
}
-static void xen_vcpu_setup_restore(int cpu)
+static int xen_vcpu_setup_restore(int cpu)
{
+ int rc = 0;
+
/* Any per_cpu(xen_vcpu) is stale, so reset it */
xen_vcpu_info_reset(cpu);
@@ -117,8 +119,10 @@ static void xen_vcpu_setup_restore(int cpu)
*/
if (xen_pv_domain() ||
(xen_hvm_domain() && cpu_online(cpu))) {
- xen_vcpu_setup(cpu);
+ rc = xen_vcpu_setup(cpu);
}
+
+ return rc;
}
/*
@@ -128,7 +132,7 @@ static void xen_vcpu_setup_restore(int cpu)
*/
void xen_vcpu_restore(void)
{
- int cpu;
+ int cpu, rc;
for_each_possible_cpu(cpu) {
bool other_cpu = (cpu != smp_processor_id());
@@ -148,22 +152,25 @@ void xen_vcpu_restore(void)
if (xen_pv_domain() || xen_feature(XENFEAT_hvm_safe_pvclock))
xen_setup_runstate_info(cpu);
- xen_vcpu_setup_restore(cpu);
-
- if (other_cpu && is_up &&
+ rc = xen_vcpu_setup_restore(cpu);
+ if (rc)
+ pr_emerg_once("vcpu restore failed for cpu=%d err=%d. "
+ "System will hang.\n", cpu, rc);
+ /*
+ * In case xen_vcpu_setup_restore() fails, do not bring up the
+ * VCPU. This helps us avoid the resulting OOPS when the VCPU
+ * accesses pvclock_vcpu_time via xen_vcpu (which is NULL.)
+ * Note that this does not improve the situation much -- now the
+ * VM hangs instead of OOPSing -- with the VCPUs that did not
+ * fail, spinning in stop_machine(), waiting for the failed
+ * VCPUs to come up.
+ */
+ if (other_cpu && is_up && (rc == 0) &&
HYPERVISOR_vcpu_op(VCPUOP_up, xen_vcpu_nr(cpu), NULL))
BUG();
}
}
-static void clamp_max_cpus(void)
-{
-#ifdef CONFIG_SMP
- if (setup_max_cpus > MAX_VIRT_CPUS)
- setup_max_cpus = MAX_VIRT_CPUS;
-#endif
-}
-
void xen_vcpu_info_reset(int cpu)
{
if (xen_vcpu_nr(cpu) < MAX_VIRT_CPUS) {
@@ -175,7 +182,7 @@ void xen_vcpu_info_reset(int cpu)
}
}
-void xen_vcpu_setup(int cpu)
+int xen_vcpu_setup(int cpu)
{
struct vcpu_register_vcpu_info info;
int err;
@@ -196,7 +203,7 @@ void xen_vcpu_setup(int cpu)
*/
if (xen_hvm_domain()) {
if (per_cpu(xen_vcpu, cpu) == &per_cpu(xen_vcpu_info, cpu))
- return;
+ return 0;
}
if (xen_have_vcpu_info_placement) {
@@ -230,11 +237,10 @@ void xen_vcpu_setup(int cpu)
}
}
- if (!xen_have_vcpu_info_placement) {
- if (cpu >= MAX_VIRT_CPUS)
- clamp_max_cpus();
+ if (!xen_have_vcpu_info_placement)
xen_vcpu_info_reset(cpu);
- }
+
+ return ((per_cpu(xen_vcpu, cpu) == NULL) ? -ENODEV : 0);
}
void xen_reboot(int reason)
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index ba1afadb2512..13b5fa1a211c 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -89,7 +89,7 @@ static void xen_hvm_crash_shutdown(struct pt_regs *regs)
static int xen_cpu_up_prepare_hvm(unsigned int cpu)
{
- int rc;
+ int rc = 0;
/*
* This can happen if CPU was offlined earlier and
@@ -104,7 +104,9 @@ static int xen_cpu_up_prepare_hvm(unsigned int cpu)
per_cpu(xen_vcpu_id, cpu) = cpu_acpi_id(cpu);
else
per_cpu(xen_vcpu_id, cpu) = cpu;
- xen_vcpu_setup(cpu);
+ rc = xen_vcpu_setup(cpu);
+ if (rc)
+ return rc;
if (xen_have_vector_callback && xen_feature(XENFEAT_hvm_safe_pvclock))
xen_setup_timer(cpu);
@@ -113,9 +115,8 @@ static int xen_cpu_up_prepare_hvm(unsigned int cpu)
if (rc) {
WARN(1, "xen_smp_intr_init() for CPU %d failed: %d\n",
cpu, rc);
- return rc;
}
- return 0;
+ return rc;
}
static int xen_cpu_dead_hvm(unsigned int cpu)
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 29cad193db53..e6639da11e0b 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -965,7 +965,16 @@ void __ref xen_setup_vcpu_info_placement(void)
for_each_possible_cpu(cpu) {
/* Set up direct vCPU id mapping for PV guests. */
per_cpu(xen_vcpu_id, cpu) = cpu;
- xen_vcpu_setup(cpu);
+
+ /*
+ * xen_vcpu_setup(cpu) can fail -- in which case it
+ * falls back to the shared_info version for cpus
+ * where xen_vcpu_nr(cpu) < MAX_VIRT_CPUS.
+ *
+ * xen_cpu_up_prepare_pv() handles the rest by failing
+ * them in hotplug.
+ */
+ (void) xen_vcpu_setup(cpu);
}
/*
@@ -1439,6 +1448,9 @@ static int xen_cpu_up_prepare_pv(unsigned int cpu)
{
int rc;
+ if (per_cpu(xen_vcpu, cpu) == NULL)
+ return -ENODEV;
+
xen_setup_timer(cpu);
rc = xen_smp_intr_init(cpu);
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 90828256248b..0d5004477db6 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -78,7 +78,7 @@ bool xen_vcpu_stolen(int vcpu);
extern int xen_have_vcpu_info_placement;
-void xen_vcpu_setup(int cpu);
+int xen_vcpu_setup(int cpu);
void xen_vcpu_info_reset(int cpu);
void xen_setup_vcpu_info_placement(void);
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] xen/vcpu: Handle xen_vcpu_setup() failure at boot
2017-06-03 0:05 [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Ankur Arora
` (3 preceding siblings ...)
2017-06-03 0:06 ` [PATCH 4/5] xen/vcpu: Handle xen_vcpu_setup() failure in hotplug Ankur Arora
@ 2017-06-03 0:06 ` Ankur Arora
2017-06-08 8:28 ` [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Juergen Gross
2017-06-13 14:12 ` Juergen Gross
6 siblings, 0 replies; 10+ messages in thread
From: Ankur Arora @ 2017-06-03 0:06 UTC (permalink / raw)
To: linux-kernel, xen-devel; +Cc: boris.ostrovsky, jgross, Ankur Arora
On PVH, PVHVM, at failure in the VCPUOP_register_vcpu_info hypercall
we limit the number of cpus to to MAX_VIRT_CPUS. However, if this
failure had occurred for a cpu beyond MAX_VIRT_CPUS, we continue
to function with > MAX_VIRT_CPUS.
This leads to problems at the next save/restore cycle when there
are > MAX_VIRT_CPUS threads going into stop_machine() but coming
back up there's valid state for only the first MAX_VIRT_CPUS.
This patch pulls the excess CPUs down via cpu_down().
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
arch/x86/xen/smp.c | 31 +++++++++++++++++++++++++++++++
arch/x86/xen/smp.h | 2 ++
arch/x86/xen/smp_hvm.c | 1 +
arch/x86/xen/smp_pv.c | 6 +-----
4 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 82ac611f2fc1..e7f02eb73727 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -1,4 +1,5 @@
#include <linux/smp.h>
+#include <linux/cpu.h>
#include <linux/slab.h>
#include <linux/cpumask.h>
#include <linux/percpu.h>
@@ -114,6 +115,36 @@ int xen_smp_intr_init(unsigned int cpu)
return rc;
}
+void __init xen_smp_cpus_done(unsigned int max_cpus)
+{
+ int cpu, rc, count = 0;
+
+ if (xen_hvm_domain())
+ native_smp_cpus_done(max_cpus);
+
+ if (xen_have_vcpu_info_placement)
+ return;
+
+ for_each_online_cpu(cpu) {
+ if (xen_vcpu_nr(cpu) < MAX_VIRT_CPUS)
+ continue;
+
+ rc = cpu_down(cpu);
+
+ if (rc == 0) {
+ /*
+ * Reset vcpu_info so this cpu cannot be onlined again.
+ */
+ xen_vcpu_info_reset(cpu);
+ count++;
+ } else {
+ pr_warn("%s: failed to bring CPU %d down, error %d\n",
+ __func__, cpu, rc);
+ }
+ }
+ WARN(count, "%s: brought %d CPUs offline\n", __func__, count);
+}
+
void xen_smp_send_reschedule(int cpu)
{
xen_send_IPI_one(cpu, XEN_RESCHEDULE_VECTOR);
diff --git a/arch/x86/xen/smp.h b/arch/x86/xen/smp.h
index 8ebb6acca64a..87d3c76cba37 100644
--- a/arch/x86/xen/smp.h
+++ b/arch/x86/xen/smp.h
@@ -14,6 +14,8 @@ extern void xen_smp_intr_free(unsigned int cpu);
int xen_smp_intr_init_pv(unsigned int cpu);
void xen_smp_intr_free_pv(unsigned int cpu);
+void xen_smp_cpus_done(unsigned int max_cpus);
+
void xen_smp_send_reschedule(int cpu);
void xen_smp_send_call_function_ipi(const struct cpumask *mask);
void xen_smp_send_call_function_single_ipi(int cpu);
diff --git a/arch/x86/xen/smp_hvm.c b/arch/x86/xen/smp_hvm.c
index 6c8a805819ff..fd60abedf658 100644
--- a/arch/x86/xen/smp_hvm.c
+++ b/arch/x86/xen/smp_hvm.c
@@ -71,4 +71,5 @@ void __init xen_hvm_smp_init(void)
smp_ops.send_call_func_ipi = xen_smp_send_call_function_ipi;
smp_ops.send_call_func_single_ipi = xen_smp_send_call_function_single_ipi;
smp_ops.smp_prepare_boot_cpu = xen_hvm_smp_prepare_boot_cpu;
+ smp_ops.smp_cpus_done = xen_smp_cpus_done;
}
diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c
index aae32535f4ec..1ea598e5f030 100644
--- a/arch/x86/xen/smp_pv.c
+++ b/arch/x86/xen/smp_pv.c
@@ -371,10 +371,6 @@ static int xen_pv_cpu_up(unsigned int cpu, struct task_struct *idle)
return 0;
}
-static void xen_pv_smp_cpus_done(unsigned int max_cpus)
-{
-}
-
#ifdef CONFIG_HOTPLUG_CPU
static int xen_pv_cpu_disable(void)
{
@@ -469,7 +465,7 @@ static irqreturn_t xen_irq_work_interrupt(int irq, void *dev_id)
static const struct smp_ops xen_smp_ops __initconst = {
.smp_prepare_boot_cpu = xen_pv_smp_prepare_boot_cpu,
.smp_prepare_cpus = xen_pv_smp_prepare_cpus,
- .smp_cpus_done = xen_pv_smp_cpus_done,
+ .smp_cpus_done = xen_smp_cpus_done,
.cpu_up = xen_pv_cpu_up,
.cpu_die = xen_pv_cpu_die,
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore
2017-06-03 0:05 [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Ankur Arora
` (4 preceding siblings ...)
2017-06-03 0:06 ` [PATCH 5/5] xen/vcpu: Handle xen_vcpu_setup() failure at boot Ankur Arora
@ 2017-06-08 8:28 ` Juergen Gross
2017-06-08 22:53 ` [Xen-devel] " Konrad Rzeszutek Wilk
2017-06-13 14:12 ` Juergen Gross
6 siblings, 1 reply; 10+ messages in thread
From: Juergen Gross @ 2017-06-08 8:28 UTC (permalink / raw)
To: Ankur Arora, linux-kernel, xen-devel; +Cc: boris.ostrovsky
On 03/06/17 02:05, Ankur Arora wrote:
> This patch series fixes a bunch of issues in the xen_vcpu setup
> logic.
>
> Simplify xen_vcpu related code: code refactoring in advance of the
> rest of the patch series.
>
> Support > 32 VCPUs at restore: unify all vcpu restore logic in
> xen_vcpu_restore() and support > 32 VCPUs for PVH*.
>
> Remove vcpu info placement from restore (!SMP): some pv_ops are
> marked RO after init so lets not redo xen_setup_vcpu_info_placement
> at restore.
>
> Handle xen_vcpu_setup() failure in hotplug: handle vcpu_info
> registration failures by propagating them from the cpuhp-prepare
> callback back up to the cpuhp logic.
>
> Handle xen_vcpu_setup() failure at boot: pull CPUs (> MAX_VIRT_CPUS)
> down if we fall back to xen_have_vcpu_info_placement = 0.
>
> Tested with various combinations of PV/PVHv2/PVHVM save/restore
> and cpu-hotadd-hotremove. Also tested by simulating failure in
> VCPUOP_register_vcpu_info.
>
> Please review.
Just a question regarding the sequence of tags (Reviewed-by: and
Signed-off-by:) in the patches:
It seems a little bit odd to have the Reviewed-by: tag before the
S-o-b: tag. This suggests the review was done before you wrote the
patches, which is hard to believe. :-)
So please reorder the tags in future patches to be in their logical
sequence.
I can fix this up in this series in case there is no need for V2.
Juergen
>
> Ankur Arora (5):
> xen/vcpu: Simplify xen_vcpu related code
> xen/pvh*: Support > 32 VCPUs at domain restore
> xen/pv: Fix OOPS on restore for a PV, !SMP domain
> xen/vcpu: Handle xen_vcpu_setup() failure in hotplug
> xen/vcpu: Handle xen_vcpu_setup() failure at boot
>
> arch/x86/xen/enlighten.c | 154 +++++++++++++++++++++++++++++++------------
> arch/x86/xen/enlighten_hvm.c | 33 ++++------
> arch/x86/xen/enlighten_pv.c | 87 +++++++++++-------------
> arch/x86/xen/smp.c | 31 +++++++++
> arch/x86/xen/smp.h | 2 +
> arch/x86/xen/smp_hvm.c | 14 +++-
> arch/x86/xen/smp_pv.c | 6 +-
> arch/x86/xen/suspend_hvm.c | 11 +---
> arch/x86/xen/xen-ops.h | 3 +-
> include/xen/xen-ops.h | 2 +
> 10 files changed, 218 insertions(+), 125 deletions(-)
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore
2017-06-08 8:28 ` [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Juergen Gross
@ 2017-06-08 22:53 ` Konrad Rzeszutek Wilk
2017-06-09 0:05 ` Ankur Arora
0 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-06-08 22:53 UTC (permalink / raw)
To: Juergen Gross; +Cc: Ankur Arora, linux-kernel, xen-devel, boris.ostrovsky
On Thu, Jun 08, 2017 at 10:28:15AM +0200, Juergen Gross wrote:
> On 03/06/17 02:05, Ankur Arora wrote:
> > This patch series fixes a bunch of issues in the xen_vcpu setup
> > logic.
> >
> > Simplify xen_vcpu related code: code refactoring in advance of the
> > rest of the patch series.
> >
> > Support > 32 VCPUs at restore: unify all vcpu restore logic in
> > xen_vcpu_restore() and support > 32 VCPUs for PVH*.
> >
> > Remove vcpu info placement from restore (!SMP): some pv_ops are
> > marked RO after init so lets not redo xen_setup_vcpu_info_placement
> > at restore.
> >
> > Handle xen_vcpu_setup() failure in hotplug: handle vcpu_info
> > registration failures by propagating them from the cpuhp-prepare
> > callback back up to the cpuhp logic.
> >
> > Handle xen_vcpu_setup() failure at boot: pull CPUs (> MAX_VIRT_CPUS)
> > down if we fall back to xen_have_vcpu_info_placement = 0.
> >
> > Tested with various combinations of PV/PVHv2/PVHVM save/restore
> > and cpu-hotadd-hotremove. Also tested by simulating failure in
> > VCPUOP_register_vcpu_info.
> >
> > Please review.
>
> Just a question regarding the sequence of tags (Reviewed-by: and
> Signed-off-by:) in the patches:
>
> It seems a little bit odd to have the Reviewed-by: tag before the
> S-o-b: tag. This suggests the review was done before you wrote the
> patches, which is hard to believe. :-)
That is how the Linux orders the tags, just do 'git log' and you
will see that pattern.
>
> So please reorder the tags in future patches to be in their logical
> sequence.
While Xen orders it in the other order (SoB first, then Reviewed-by).
>
> I can fix this up in this series in case there is no need for V2.
>
>
> Juergen
>
> >
> > Ankur Arora (5):
> > xen/vcpu: Simplify xen_vcpu related code
> > xen/pvh*: Support > 32 VCPUs at domain restore
> > xen/pv: Fix OOPS on restore for a PV, !SMP domain
> > xen/vcpu: Handle xen_vcpu_setup() failure in hotplug
> > xen/vcpu: Handle xen_vcpu_setup() failure at boot
> >
> > arch/x86/xen/enlighten.c | 154 +++++++++++++++++++++++++++++++------------
> > arch/x86/xen/enlighten_hvm.c | 33 ++++------
> > arch/x86/xen/enlighten_pv.c | 87 +++++++++++-------------
> > arch/x86/xen/smp.c | 31 +++++++++
> > arch/x86/xen/smp.h | 2 +
> > arch/x86/xen/smp_hvm.c | 14 +++-
> > arch/x86/xen/smp_pv.c | 6 +-
> > arch/x86/xen/suspend_hvm.c | 11 +---
> > arch/x86/xen/xen-ops.h | 3 +-
> > include/xen/xen-ops.h | 2 +
> > 10 files changed, 218 insertions(+), 125 deletions(-)
> >
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore
2017-06-08 22:53 ` [Xen-devel] " Konrad Rzeszutek Wilk
@ 2017-06-09 0:05 ` Ankur Arora
0 siblings, 0 replies; 10+ messages in thread
From: Ankur Arora @ 2017-06-09 0:05 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, Juergen Gross
Cc: linux-kernel, xen-devel, boris.ostrovsky
On 2017-06-08 03:53 PM, Konrad Rzeszutek Wilk wrote:
> On Thu, Jun 08, 2017 at 10:28:15AM +0200, Juergen Gross wrote:
>> On 03/06/17 02:05, Ankur Arora wrote:
>>> This patch series fixes a bunch of issues in the xen_vcpu setup
>>> logic.
>>>
>>> Simplify xen_vcpu related code: code refactoring in advance of the
>>> rest of the patch series.
>>>
>>> Support > 32 VCPUs at restore: unify all vcpu restore logic in
>>> xen_vcpu_restore() and support > 32 VCPUs for PVH*.
>>>
>>> Remove vcpu info placement from restore (!SMP): some pv_ops are
>>> marked RO after init so lets not redo xen_setup_vcpu_info_placement
>>> at restore.
>>>
>>> Handle xen_vcpu_setup() failure in hotplug: handle vcpu_info
>>> registration failures by propagating them from the cpuhp-prepare
>>> callback back up to the cpuhp logic.
>>>
>>> Handle xen_vcpu_setup() failure at boot: pull CPUs (> MAX_VIRT_CPUS)
>>> down if we fall back to xen_have_vcpu_info_placement = 0.
>>>
>>> Tested with various combinations of PV/PVHv2/PVHVM save/restore
>>> and cpu-hotadd-hotremove. Also tested by simulating failure in
>>> VCPUOP_register_vcpu_info.
>>>
>>> Please review.
>>
>> Just a question regarding the sequence of tags (Reviewed-by: and
>> Signed-off-by:) in the patches:
>>
>> It seems a little bit odd to have the Reviewed-by: tag before the
>> S-o-b: tag. This suggests the review was done before you wrote the
>> patches, which is hard to believe. :-)
Heh :). As Konrad surmises, I was unsure of the order and manually
ordered them to comport with Linux style. (Now that I see arch/x86/xen/,
I see that Xen puts them in time-order.)
Happy to reorder in case of V2.
Ankur
>
> That is how the Linux orders the tags, just do 'git log' and you
> will see that pattern >>
>> So please reorder the tags in future patches to be in their logical
>> sequence.
>
> While Xen orders it in the other order (SoB first, then Reviewed-by).
>
>>
>> I can fix this up in this series in case there is no need for V2.
>>
>>
>> Juergen
>>
>>>
>>> Ankur Arora (5):
>>> xen/vcpu: Simplify xen_vcpu related code
>>> xen/pvh*: Support > 32 VCPUs at domain restore
>>> xen/pv: Fix OOPS on restore for a PV, !SMP domain
>>> xen/vcpu: Handle xen_vcpu_setup() failure in hotplug
>>> xen/vcpu: Handle xen_vcpu_setup() failure at boot
>>>
>>> arch/x86/xen/enlighten.c | 154 +++++++++++++++++++++++++++++++------------
>>> arch/x86/xen/enlighten_hvm.c | 33 ++++------
>>> arch/x86/xen/enlighten_pv.c | 87 +++++++++++-------------
>>> arch/x86/xen/smp.c | 31 +++++++++
>>> arch/x86/xen/smp.h | 2 +
>>> arch/x86/xen/smp_hvm.c | 14 +++-
>>> arch/x86/xen/smp_pv.c | 6 +-
>>> arch/x86/xen/suspend_hvm.c | 11 +---
>>> arch/x86/xen/xen-ops.h | 3 +-
>>> include/xen/xen-ops.h | 2 +
>>> 10 files changed, 218 insertions(+), 125 deletions(-)
>>>
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore
2017-06-03 0:05 [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Ankur Arora
` (5 preceding siblings ...)
2017-06-08 8:28 ` [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Juergen Gross
@ 2017-06-13 14:12 ` Juergen Gross
6 siblings, 0 replies; 10+ messages in thread
From: Juergen Gross @ 2017-06-13 14:12 UTC (permalink / raw)
To: Ankur Arora, linux-kernel, xen-devel; +Cc: boris.ostrovsky
On 03/06/17 02:05, Ankur Arora wrote:
> This patch series fixes a bunch of issues in the xen_vcpu setup
> logic.
>
> Simplify xen_vcpu related code: code refactoring in advance of the
> rest of the patch series.
>
> Support > 32 VCPUs at restore: unify all vcpu restore logic in
> xen_vcpu_restore() and support > 32 VCPUs for PVH*.
>
> Remove vcpu info placement from restore (!SMP): some pv_ops are
> marked RO after init so lets not redo xen_setup_vcpu_info_placement
> at restore.
>
> Handle xen_vcpu_setup() failure in hotplug: handle vcpu_info
> registration failures by propagating them from the cpuhp-prepare
> callback back up to the cpuhp logic.
>
> Handle xen_vcpu_setup() failure at boot: pull CPUs (> MAX_VIRT_CPUS)
> down if we fall back to xen_have_vcpu_info_placement = 0.
>
> Tested with various combinations of PV/PVHv2/PVHVM save/restore
> and cpu-hotadd-hotremove. Also tested by simulating failure in
> VCPUOP_register_vcpu_info.
>
> Please review.
>
> Ankur Arora (5):
> xen/vcpu: Simplify xen_vcpu related code
> xen/pvh*: Support > 32 VCPUs at domain restore
> xen/pv: Fix OOPS on restore for a PV, !SMP domain
> xen/vcpu: Handle xen_vcpu_setup() failure in hotplug
> xen/vcpu: Handle xen_vcpu_setup() failure at boot
>
> arch/x86/xen/enlighten.c | 154 +++++++++++++++++++++++++++++++------------
> arch/x86/xen/enlighten_hvm.c | 33 ++++------
> arch/x86/xen/enlighten_pv.c | 87 +++++++++++-------------
> arch/x86/xen/smp.c | 31 +++++++++
> arch/x86/xen/smp.h | 2 +
> arch/x86/xen/smp_hvm.c | 14 +++-
> arch/x86/xen/smp_pv.c | 6 +-
> arch/x86/xen/suspend_hvm.c | 11 +---
> arch/x86/xen/xen-ops.h | 3 +-
> include/xen/xen-ops.h | 2 +
> 10 files changed, 218 insertions(+), 125 deletions(-)
>
Series committed to xen/tip.git for-linus-4.13
Thanks,
Juergen
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-06-13 14:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-03 0:05 [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Ankur Arora
2017-06-03 0:05 ` [PATCH 1/5] xen/vcpu: Simplify xen_vcpu related code Ankur Arora
2017-06-03 0:05 ` [PATCH 2/5] xen/pvh*: Support > 32 VCPUs at domain restore Ankur Arora
2017-06-03 0:06 ` [PATCH 3/5] xen/pv: Fix OOPS on restore for a PV, !SMP domain Ankur Arora
2017-06-03 0:06 ` [PATCH 4/5] xen/vcpu: Handle xen_vcpu_setup() failure in hotplug Ankur Arora
2017-06-03 0:06 ` [PATCH 5/5] xen/vcpu: Handle xen_vcpu_setup() failure at boot Ankur Arora
2017-06-08 8:28 ` [PATCH 0/5] xen/pvh*: Support > 32 VCPUs at restore Juergen Gross
2017-06-08 22:53 ` [Xen-devel] " Konrad Rzeszutek Wilk
2017-06-09 0:05 ` Ankur Arora
2017-06-13 14:12 ` Juergen Gross
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).