Messages in this thread | | | From | Jane Malalane <> | Subject | Re: [PATCH] x86/xen: Add support for HVMOP_set_evtchn_upcall_vector | Date | Fri, 15 Jul 2022 08:18:27 +0000 |
| |
On 14/07/2022 00:27, Boris Ostrovsky wrote: > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open > attachments unless you have verified the sender and know the content is > safe. > > 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? Sure. > > >> + >> 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? Yes, will amend. > > >> + } >> + >> 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? I'll remove no_vector_callback for less code changes. > > >> 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)? Yes, specifically for the check in libxl__domain_pvcontrol_available. > > > 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 > >
Thank you,
Jane. | |