The pvops function for Xen PV guests handling the interrupt flag are
much more complex than needed.
With the supported Xen hypervisor versions they can be simplified a
lot, especially by removing the need for disabling preemption.
Juergen Gross (2):
x86/xen: remove xen_have_vcpu_info_placement flag
x86/xen: switch initial pvops IRQ functions to dummy ones
arch/x86/include/asm/paravirt_types.h | 2 +
arch/x86/kernel/paravirt.c | 13 ++-
arch/x86/xen/enlighten.c | 116 ++++++--------------------
arch/x86/xen/enlighten_hvm.c | 6 +-
arch/x86/xen/enlighten_pv.c | 28 ++-----
arch/x86/xen/irq.c | 61 +-------------
arch/x86/xen/smp.c | 24 ------
arch/x86/xen/xen-ops.h | 4 +-
8 files changed, 53 insertions(+), 201 deletions(-)
--
2.26.2
The initial pvops functions handling irq flags will only ever be called
before interrupts are being enabled.
So make the __init and switch them to be dummy functions:
- xen_save_fl() can always return 0
- xen_irq_disable() is a nop
- xen_irq_enable() can BUG()
Add some generic paravirt functions for that purpose.
Signed-off-by: Juergen Gross <[email protected]>
---
arch/x86/include/asm/paravirt_types.h | 2 +
arch/x86/kernel/paravirt.c | 13 +++++-
arch/x86/xen/enlighten.c | 19 +--------
arch/x86/xen/irq.c | 61 ++-------------------------
4 files changed, 20 insertions(+), 75 deletions(-)
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index d9d6b0203ec4..fc1151e77569 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -577,7 +577,9 @@ void paravirt_leave_lazy_mmu(void);
void paravirt_flush_lazy_mmu(void);
void _paravirt_nop(void);
+void paravirt_BUG(void);
u64 _paravirt_ident_64(u64);
+unsigned long paravirt_ret0(void);
#define paravirt_nop ((void *)_paravirt_nop)
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 04cafc057bed..4f0ebc46a15d 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -46,6 +46,17 @@ asm (".pushsection .entry.text, \"ax\"\n"
".type _paravirt_nop, @function\n\t"
".popsection");
+/* stub always return ing 0. */
+asm (".pushsection .entry.text, \"ax\"\n"
+ ".global paravirt_ret0\n"
+ "paravirt_ret0:\n\t"
+ "xor %" _ASM_AX ", %" _ASM_AX ";\n\t"
+ "ret\n\t"
+ ".size paravirt_ret0, . - paravirt_ret0\n\t"
+ ".type paravirt_ret0, @function\n\t"
+ ".popsection");
+
+
void __init default_banner(void)
{
printk(KERN_INFO "Booting paravirtualized kernel on %s\n",
@@ -53,7 +64,7 @@ void __init default_banner(void)
}
/* Undefined instruction for dealing with missing ops pointers. */
-static void paravirt_BUG(void)
+void paravirt_BUG(void)
{
BUG();
}
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 8a7bd3f4591e..d96cabe34a01 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -27,25 +27,10 @@ EXPORT_SYMBOL_GPL(hypercall_page);
* Pointer to the xen_vcpu_info structure or
* &HYPERVISOR_shared_info->vcpu_info[cpu]. See xen_hvm_init_shared_info
* and xen_vcpu_setup for details. By default it points to share_info->vcpu_info
- * but if the hypervisor supports VCPUOP_register_vcpu_info then it can point
- * to xen_vcpu_info. The pointer is used in __xen_evtchn_do_upcall to
- * acknowledge pending events.
- * Also more subtly it is used by the patched version of irq enable/disable
- * e.g. xen_irq_enable_direct and xen_iret in PV mode.
- *
- * The desire to be able to do those mask/unmask operations as a single
- * instruction by using the per-cpu offset held in %gs is the real reason
- * vcpu info is in a per-cpu pointer and the original reason for this
- * hypercall.
- *
+ * but during boot it is switched to point to xen_vcpu_info.
+ * The pointer is used in __xen_evtchn_do_upcall to acknowledge pending events.
*/
DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
-
-/*
- * Per CPU pages used if hypervisor supports VCPUOP_register_vcpu_info
- * hypercall. This can be used both in PV and PVHVM mode. The structure
- * overrides the default per_cpu(xen_vcpu, cpu) value.
- */
DEFINE_PER_CPU(struct vcpu_info, xen_vcpu_info);
/* Linux <-> Xen vCPU id mapping */
diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
index dfa091d79c2e..ae8537583102 100644
--- a/arch/x86/xen/irq.c
+++ b/arch/x86/xen/irq.c
@@ -24,60 +24,6 @@ void xen_force_evtchn_callback(void)
(void)HYPERVISOR_xen_version(0, NULL);
}
-asmlinkage __visible unsigned long xen_save_fl(void)
-{
- struct vcpu_info *vcpu;
- unsigned long flags;
-
- vcpu = this_cpu_read(xen_vcpu);
-
- /* flag has opposite sense of mask */
- flags = !vcpu->evtchn_upcall_mask;
-
- /* convert to IF type flag
- -0 -> 0x00000000
- -1 -> 0xffffffff
- */
- return (-flags) & X86_EFLAGS_IF;
-}
-PV_CALLEE_SAVE_REGS_THUNK(xen_save_fl);
-
-asmlinkage __visible void xen_irq_disable(void)
-{
- /* There's a one instruction preempt window here. We need to
- make sure we're don't switch CPUs between getting the vcpu
- pointer and updating the mask. */
- preempt_disable();
- this_cpu_read(xen_vcpu)->evtchn_upcall_mask = 1;
- preempt_enable_no_resched();
-}
-PV_CALLEE_SAVE_REGS_THUNK(xen_irq_disable);
-
-asmlinkage __visible void xen_irq_enable(void)
-{
- struct vcpu_info *vcpu;
-
- /*
- * We may be preempted as soon as vcpu->evtchn_upcall_mask is
- * cleared, so disable preemption to ensure we check for
- * events on the VCPU we are still running on.
- */
- preempt_disable();
-
- vcpu = this_cpu_read(xen_vcpu);
- vcpu->evtchn_upcall_mask = 0;
-
- /* Doesn't matter if we get preempted here, because any
- pending event will get dealt with anyway. */
-
- barrier(); /* unmask then check (avoid races) */
- if (unlikely(vcpu->evtchn_upcall_pending))
- xen_force_evtchn_callback();
-
- preempt_enable();
-}
-PV_CALLEE_SAVE_REGS_THUNK(xen_irq_enable);
-
static void xen_safe_halt(void)
{
/* Blocking includes an implicit local_irq_enable(). */
@@ -95,9 +41,10 @@ static void xen_halt(void)
}
static const struct pv_irq_ops xen_irq_ops __initconst = {
- .save_fl = PV_CALLEE_SAVE(xen_save_fl),
- .irq_disable = PV_CALLEE_SAVE(xen_irq_disable),
- .irq_enable = PV_CALLEE_SAVE(xen_irq_enable),
+ /* Initial interrupt flag handling only called while interrupts off. */
+ .save_fl = __PV_IS_CALLEE_SAVE(paravirt_ret0),
+ .irq_disable = __PV_IS_CALLEE_SAVE(paravirt_nop),
+ .irq_enable = __PV_IS_CALLEE_SAVE(paravirt_BUG),
.safe_halt = xen_safe_halt,
.halt = xen_halt,
--
2.26.2
The flag xen_have_vcpu_info_placement was needed to support Xen
hypervisors older than version 3.4. Today the Linux kernel requires
at least Xen 4.0 to be able to run, so xen_have_vcpu_info_placement
can be dropped.
This allows to let some functions return void now, as they can never
fail.
Signed-off-by: Juergen Gross <[email protected]>
---
arch/x86/xen/enlighten.c | 97 +++++++++---------------------------
arch/x86/xen/enlighten_hvm.c | 6 +--
arch/x86/xen/enlighten_pv.c | 28 ++---------
arch/x86/xen/smp.c | 24 ---------
arch/x86/xen/xen-ops.h | 4 +-
5 files changed, 33 insertions(+), 126 deletions(-)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index c79bd0af2e8c..8a7bd3f4591e 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -81,21 +81,6 @@ EXPORT_SYMBOL(xen_start_flags);
*/
struct shared_info *HYPERVISOR_shared_info = &xen_dummy_shared_info;
-/*
- * Flag to determine whether vcpu info placement is available on all
- * VCPUs. We assume it is to start with, and then set it to zero on
- * the first failure. This is because it can succeed on some VCPUs
- * and not others, since it can involve hypervisor memory allocation,
- * or because the guest failed to guarantee all the appropriate
- * constraints on all VCPUs (ie buffer can't cross a page boundary).
- *
- * Note that any particular CPU may be using a placed vcpu structure,
- * but we can only optimise if the all are.
- *
- * 0: not available, 1: available
- */
-int xen_have_vcpu_info_placement = 1;
-
static int xen_cpu_up_online(unsigned int cpu)
{
xen_init_lock_cpu(cpu);
@@ -121,10 +106,8 @@ int xen_cpuhp_setup(int (*cpu_up_prepare_cb)(unsigned int),
return rc >= 0 ? 0 : rc;
}
-static int xen_vcpu_setup_restore(int cpu)
+static void xen_vcpu_setup_restore(int cpu)
{
- int rc = 0;
-
/* Any per_cpu(xen_vcpu) is stale, so reset it */
xen_vcpu_info_reset(cpu);
@@ -133,11 +116,8 @@ static int xen_vcpu_setup_restore(int cpu)
* be handled by hotplug.
*/
if (xen_pv_domain() ||
- (xen_hvm_domain() && cpu_online(cpu))) {
- rc = xen_vcpu_setup(cpu);
- }
-
- return rc;
+ (xen_hvm_domain() && cpu_online(cpu)))
+ xen_vcpu_setup(cpu);
}
/*
@@ -147,7 +127,7 @@ static int xen_vcpu_setup_restore(int cpu)
*/
void xen_vcpu_restore(void)
{
- int cpu, rc;
+ int cpu;
for_each_possible_cpu(cpu) {
bool other_cpu = (cpu != smp_processor_id());
@@ -167,20 +147,9 @@ void xen_vcpu_restore(void)
if (xen_pv_domain() || xen_feature(XENFEAT_hvm_safe_pvclock))
xen_setup_runstate_info(cpu);
- 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) &&
+ xen_vcpu_setup_restore(cpu);
+
+ if (other_cpu && is_up &&
HYPERVISOR_vcpu_op(VCPUOP_up, xen_vcpu_nr(cpu), NULL))
BUG();
}
@@ -197,7 +166,7 @@ void xen_vcpu_info_reset(int cpu)
}
}
-int xen_vcpu_setup(int cpu)
+void xen_vcpu_setup(int cpu)
{
struct vcpu_register_vcpu_info info;
int err;
@@ -218,44 +187,26 @@ int xen_vcpu_setup(int cpu)
*/
if (xen_hvm_domain()) {
if (per_cpu(xen_vcpu, cpu) == &per_cpu(xen_vcpu_info, cpu))
- return 0;
+ return;
}
- 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);
+ 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)
- xen_vcpu_info_reset(cpu);
+ /*
+ * 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)
+ panic("register_vcpu_info failed: cpu=%d err=%d\n", cpu, err);
- return ((per_cpu(xen_vcpu, cpu) == NULL) ? -ENODEV : 0);
+ 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 e68ea5f4ad1c..42300941ec29 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -163,9 +163,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;
- rc = xen_vcpu_setup(cpu);
- if (rc || !xen_have_vector_callback)
- return rc;
+ xen_vcpu_setup(cpu);
+ if (!xen_have_vector_callback)
+ return 0;
if (xen_feature(XENFEAT_hvm_safe_pvclock))
xen_setup_timer(cpu);
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 349f780a1567..6719963901e5 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1018,31 +1018,13 @@ void __init 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) 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);
+ xen_vcpu_setup(cpu);
}
- /*
- * xen_vcpu_setup managed to place the vcpu_info within the
- * percpu area for all cpus, so make use of it.
- */
- if (xen_have_vcpu_info_placement) {
- pv_ops.irq.save_fl = __PV_IS_CALLEE_SAVE(xen_save_fl_direct);
- pv_ops.irq.irq_disable =
- __PV_IS_CALLEE_SAVE(xen_irq_disable_direct);
- pv_ops.irq.irq_enable =
- __PV_IS_CALLEE_SAVE(xen_irq_enable_direct);
- pv_ops.mmu.read_cr2 =
- __PV_IS_CALLEE_SAVE(xen_read_cr2_direct);
- }
+ pv_ops.irq.save_fl = __PV_IS_CALLEE_SAVE(xen_save_fl_direct);
+ pv_ops.irq.irq_disable = __PV_IS_CALLEE_SAVE(xen_irq_disable_direct);
+ pv_ops.irq.irq_enable = __PV_IS_CALLEE_SAVE(xen_irq_enable_direct);
+ pv_ops.mmu.read_cr2 = __PV_IS_CALLEE_SAVE(xen_read_cr2_direct);
}
static const struct pv_info xen_info __initconst = {
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index c1b2f764b29a..bafa61b1482f 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -121,34 +121,10 @@ int xen_smp_intr_init(unsigned int cpu)
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);
else
calculate_max_logical_packages();
-
- if (xen_have_vcpu_info_placement)
- return;
-
- for_each_online_cpu(cpu) {
- if (xen_vcpu_nr(cpu) < MAX_VIRT_CPUS)
- continue;
-
- rc = remove_cpu(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)
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 8d7ec49a35fb..c2da84484b8d 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -75,9 +75,7 @@ irqreturn_t xen_debug_interrupt(int irq, void *dev_id);
bool xen_vcpu_stolen(int vcpu);
-extern int xen_have_vcpu_info_placement;
-
-int xen_vcpu_setup(int cpu);
+void xen_vcpu_setup(int cpu);
void xen_vcpu_info_reset(int cpu);
void xen_setup_vcpu_info_placement(void);
--
2.26.2
On Wed, Sep 22, 2021 at 12:31:00PM +0200, Juergen Gross wrote:
> The pvops function for Xen PV guests handling the interrupt flag are
> much more complex than needed.
>
> With the supported Xen hypervisor versions they can be simplified a
> lot, especially by removing the need for disabling preemption.
>
> Juergen Gross (2):
> x86/xen: remove xen_have_vcpu_info_placement flag
> x86/xen: switch initial pvops IRQ functions to dummy ones
>
> arch/x86/include/asm/paravirt_types.h | 2 +
> arch/x86/kernel/paravirt.c | 13 ++-
> arch/x86/xen/enlighten.c | 116 ++++++--------------------
> arch/x86/xen/enlighten_hvm.c | 6 +-
> arch/x86/xen/enlighten_pv.c | 28 ++-----
> arch/x86/xen/irq.c | 61 +-------------
> arch/x86/xen/smp.c | 24 ------
> arch/x86/xen/xen-ops.h | 4 +-
> 8 files changed, 53 insertions(+), 201 deletions(-)
That looks awesome, I'm totally in favour of deleting code :-)
Acked-by: Peter Zijlstra (Intel) <[email protected]>
On 9/22/21 6:31 AM, Juergen Gross wrote:
>
> - 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);
> + 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)
> - xen_vcpu_info_reset(cpu);
> + /*
> + * 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)
> + panic("register_vcpu_info failed: cpu=%d err=%d\n", cpu, err);
>
This is change in behavior. Before if the hypercall failed we still try to boot. I am not sure we need to worry about this (since it's not clear it actually works) but I'd at least mention this in the commit message.
-boris
On 9/22/21 6:31 AM, Juergen Gross wrote:
> The initial pvops functions handling irq flags will only ever be called
> before interrupts are being enabled.
>
> So make the __init and switch them to be dummy functions:
What are you making __init?
>
> +/* stub always return ing 0. */
"always returning"
-boris
On 22.09.21 23:43, Boris Ostrovsky wrote:
>
> On 9/22/21 6:31 AM, Juergen Gross wrote:
>>
>> - 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);
>> + 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)
>> - xen_vcpu_info_reset(cpu);
>> + /*
>> + * 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)
>> + panic("register_vcpu_info failed: cpu=%d err=%d\n", cpu, err);
>>
>
>
> This is change in behavior. Before if the hypercall failed we still try to boot. I am not sure we need to worry about this (since it's not clear it actually works) but I'd at least mention this in the commit message.
Hmm, maybe I should have been more explicit saying that the hypercall
was introduced in Xen 3.4, and only reason of failure is either an
illegal vcpu, an invalid mapping specification, or a try to reissue the
hypercall for a vcpu. None of those should ever happen.
Juergen
On 22.09.21 23:49, Boris Ostrovsky wrote:
>
> On 9/22/21 6:31 AM, Juergen Gross wrote:
>> The initial pvops functions handling irq flags will only ever be called
>> before interrupts are being enabled.
>>
>> So make the __init and switch them to be dummy functions:
>
>
> What are you making __init?
Oh, sorry, that's a leftover from a previous version.
>> +/* stub always return ing 0. */
>
>
> "always returning"
Yes.
Juergen
On 9/23/21 12:44 AM, Juergen Gross wrote:
>
> Hmm, maybe I should have been more explicit saying that the hypercall
> was introduced in Xen 3.4, and only reason of failure is either an
> illegal vcpu, an invalid mapping specification, or a try to reissue the
> hypercall for a vcpu. None of those should ever happen.
>
That last sentence -- famous last words ;-) But yes, sure.
Assuming both patches adjust their commit messages and the typo
Reviewed-by: Boris Ostrovsky <[email protected]>