Messages in this thread | | | Date | Wed, 13 Jul 2022 19:27:20 -0400 | Subject | Re: [PATCH] x86/xen: Add support for HVMOP_set_evtchn_upcall_vector | From | Boris Ostrovsky <> |
| |
On 7/11/22 11:22 AM, Jane Malalane wrote: > --- a/arch/x86/xen/enlighten_hvm.c > +++ b/arch/x86/xen/enlighten_hvm.c > @@ -7,6 +7,7 @@ > > #include <xen/features.h> > #include <xen/events.h> > +#include <xen/interface/hvm/hvm_op.h> > #include <xen/interface/memory.h> > > #include <asm/apic.h> > @@ -30,6 +31,9 @@ > > static unsigned long shared_info_pfn; > > +__ro_after_init bool xen_ack_upcall; > +EXPORT_SYMBOL_GPL(xen_ack_upcall);
Shouldn't this be called something like xen_percpu_upcall?
> + > void xen_hvm_init_shared_info(void) > { > struct xen_add_to_physmap xatp; > @@ -125,6 +129,9 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_xen_hvm_callback) > { > struct pt_regs *old_regs = set_irq_regs(regs); > > + if (xen_ack_upcall) > + ack_APIC_irq(); > + > inc_irq_stat(irq_hv_callback_count); > > xen_hvm_evtchn_do_upcall(); > @@ -168,6 +175,15 @@ static int xen_cpu_up_prepare_hvm(unsigned int cpu) > if (!xen_have_vector_callback) > return 0; > > + if (xen_ack_upcall) { > + xen_hvm_evtchn_upcall_vector_t op = { > + .vector = HYPERVISOR_CALLBACK_VECTOR, > + .vcpu = per_cpu(xen_vcpu_id, cpu), > + }; > + > + BUG_ON(HYPERVISOR_hvm_op(HVMOP_set_evtchn_upcall_vector, &op));
Does this have to be fatal? Can't we just fail bringing this vcpu up?
> + } > + > if (xen_feature(XENFEAT_hvm_safe_pvclock)) > xen_setup_timer(cpu); > > @@ -211,8 +227,7 @@ static void __init xen_hvm_guest_init(void) > > xen_panic_handler_init(); > > - if (!no_vector_callback && xen_feature(XENFEAT_hvm_callback_vector)) > - xen_have_vector_callback = 1; > + xen_have_vector_callback = !no_vector_callback;
Can we get rid of one of those two variables then?
> > xen_hvm_smp_init(); > WARN_ON(xen_cpuhp_setup(xen_cpu_up_prepare_hvm, xen_cpu_dead_hvm)); > diff --git a/arch/x86/xen/suspend_hvm.c b/arch/x86/xen/suspend_hvm.c > index 9d548b0c772f..be66e027ef28 100644 > --- a/arch/x86/xen/suspend_hvm.c > +++ b/arch/x86/xen/suspend_hvm.c > @@ -5,6 +5,7 @@ > #include <xen/hvm.h> > #include <xen/features.h> > #include <xen/interface/features.h> > +#include <xen/events.h> > > #include "xen-ops.h" > > @@ -14,6 +15,23 @@ void xen_hvm_post_suspend(int suspend_cancelled) > xen_hvm_init_shared_info(); > xen_vcpu_restore(); > } > - xen_setup_callback_vector(); > + if (xen_ack_upcall) { > + unsigned int cpu; > + > + for_each_online_cpu(cpu) { > + xen_hvm_evtchn_upcall_vector_t op = { > + .vector = HYPERVISOR_CALLBACK_VECTOR, > + .vcpu = per_cpu(xen_vcpu_id, cpu), > + }; > + > + BUG_ON(HYPERVISOR_hvm_op(HVMOP_set_evtchn_upcall_vector, > + &op)); > + /* Trick toolstack to think we are enlightened. */ > + if (!cpu) > + BUG_ON(xen_set_callback_via(1));
What are you trying to make the toolstack aware of? That we have *a* callback (either global or percpu)?
BTW, you can take it out the loop. And maybe @op definition too, except for .vcpu assignment.
> + } > + } else { > + xen_setup_callback_vector(); > + } > xen_unplug_emulated_devices(); > }
-boris
| |