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(). ~Andrew
