On 20/11/2025 11:01 am, Andrew Cooper wrote: > On 19/11/2025 10:50 am, Jan Beulich wrote: >> There's no need to do this every time init_evtchn() is called. Just do it >> once when setting up CPU0. Drop the assertion as well, as >> alloc_hipriority_vector() (called by alloc_direct_apic_vector()) uses more >> restrictive BUG_ON() anyway. Then evtchn_upcall_vector can also validly >> become ro-after-init, just that it needs to move out of init_evtchn(). >> >> Signed-off-by: Jan Beulich <[email protected]> >> >> --- a/xen/arch/x86/guest/xen/xen.c >> +++ b/xen/arch/x86/guest/xen/xen.c >> @@ -233,16 +233,12 @@ static void cf_check xen_evtchn_upcall(v >> ack_APIC_irq(); >> } >> >> +static uint8_t __ro_after_init evtchn_upcall_vector; >> + >> static int init_evtchn(void) >> { >> - static uint8_t evtchn_upcall_vector; >> int rc; >> >> - if ( !evtchn_upcall_vector ) >> - alloc_direct_apic_vector(&evtchn_upcall_vector, xen_evtchn_upcall); >> - >> - ASSERT(evtchn_upcall_vector); >> - >> rc = xen_hypercall_set_evtchn_upcall_vector(this_cpu(vcpu_id), >> evtchn_upcall_vector); >> if ( rc ) >> @@ -293,6 +289,8 @@ static void __init cf_check setup(void) >> XEN_LEGACY_MAX_VCPUS); >> } >> >> + alloc_direct_apic_vector(&evtchn_upcall_vector, xen_evtchn_upcall); >> + >> BUG_ON(init_evtchn()); >> } >> >> > This patch is fine, but it would be nicer to split init_evtchn() into > bsp_init_evtchn() and percpu_init_evtchn(). > > Just out of context in init_evtchn(), there's a check for CPU0 that also > ought to move into bsp_init_evtchn() (and therefore into __init), at > which point the percpu simplifies to a single hypercall, and we keep > subsystem specifics out of setup().
No, scratch that. HVM_PARAM_CALLBACK_IRQ is not in the list of HVM Params that migration moves on migrate (see write_hvm_params() in xg_sr_save_x86_hvm.c). Everything is awful. Could you include a comment such as /* HVM_PARAM_CALLBACK_IRQ is not moved on migrate, so has to be set up again on resume. */ to make it clear why that piece of logic needs to stay in a non-init function? ~Andrew
