[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 3/4] xen/pvhvm: Unmap all PIRQs on startup and shutdown
David Vrabel <david.vrabel@xxxxxxxxxx> writes: > On 15/07/14 14:40, Vitaly Kuznetsov wrote: >> When kexec is being run PIRQs from Qemu-emulated devices are still >> mapped to old event channels and new kernel has no information about >> that. Trying to map them twice results in the following in Xen's dmesg: >> >> (XEN) irq.c:2278: dom7: pirq 24 or emuirq 8 already mapped >> (XEN) irq.c:2278: dom7: pirq 24 or emuirq 12 already mapped >> (XEN) irq.c:2278: dom7: pirq 24 or emuirq 1 already mapped >> ... >> >> and the following in new kernel's dmesg: >> >> [ 92.286796] xen:events: Failed to obtain physical IRQ 4 >> >> The result is that the new kernel doesn't recieve IRQs for Qemu-emulated >> devices. Address the issue by unmapping all mapped PIRQs on kernel shutdown >> when kexec was requested and on every kernel startup. We need to do this >> twice to deal with the following issues: >> - startup-time unmapping is required to make kdump work; >> - shutdown-time unmapping is required to support kexec-ing non-fixed kernels; >> - shutdown-time unmapping is required to make Qemu-emulated NICs work after >> kexec (event channel is being closed on shutdown but no >> PHYSDEVOP_unmap_pirq >> is being performed). > > I think this should be done only in one place -- just prior to exec'ing > the new kernel (including kdump kernels). > Thank you for your comments! The problem I'm fighting wiht atm is: with FIFO-based event channels we need to call evtchn_fifo_destroy() so next EVTCHNOP_init_control won't fail. I was intended to put evtchn_fifo_destroy() in EVTCHNOP_reset. That introduces a problem: we need to deal with store/console channels. It is possible to remap those from guest with EVTCHNOP_bind_interdomain (if we remember where they were mapped before) but we can't do it after we did evtchn_fifo_destroy() and we can't rebind them after kexec and performing EVTCHNOP_init_control as we can't remember where these channels were mapped to after kexec/kdump. I see the following possible solutions: 1) We put evtchn_fifo_destroy() in EVTCHNOP_init_control so EVTCHNOP_init_control can be called twice. No EVTCHNOP_reset is required in that case. 2) Introduce special (e.g. 'EVTCHNOP_fifo_destroy') hypercall to do evtchn_fifo_destroy() without closing all channels. Alternatively we can avoid closing all channels in EVTCHNOP_reset when called with DOMID_SELF (as this mode is not being used atm) -- but that would look unobvious. 3) Keep evtchn_fifo_destroy() in EVTCHNOP_reset but keep console/store channels -- I saw your concerns it is not safe, some sort of additional blocking will be required. 4) Do the remapping boot time (query for store/console channels -> perform EVTCHNOP_reset -> rebind with EVTCHNOP_bind_interdomain). There is an additional problem: EVTCHNOP_bind_interdomain operation has local port as OUT parameter so we can't guarantee that remapping store/console channels will remap them to the same local channel they were mapped before EVTCHNOP_reset (and we have this information in hvm info: HVM_PARAM_CONSOLE_EVTCHN/HVM_PARAM_STORE_EVTCHN, ...). Not sure how to deal with that in case we go with remapping. Your thoughts would be very appreciated. Thank you again, >> --- a/arch/x86/xen/smp.c >> +++ b/arch/x86/xen/smp.c >> @@ -768,6 +768,7 @@ void xen_kexec_shutdown(void) >> #ifdef CONFIG_KEXEC >> if (!kexec_in_progress) >> return; >> + xen_unmap_all_pirqs(); >> #endif >> } >> >> diff --git a/drivers/xen/events/events_base.c >> b/drivers/xen/events/events_base.c >> index c919d3d..7701c7f 100644 >> --- a/drivers/xen/events/events_base.c >> +++ b/drivers/xen/events/events_base.c >> @@ -1643,6 +1643,80 @@ void xen_callback_vector(void) {} >> static bool fifo_events = true; >> module_param(fifo_events, bool, 0); >> >> +void xen_unmap_all_pirqs(void) >> +{ >> + int pirq, rc, gsi, irq, evtchn; >> + struct physdev_unmap_pirq unmap_irq; >> + struct irq_info *info; >> + struct evtchn_close close; >> + >> + mutex_lock(&irq_mapping_update_lock); >> + >> + list_for_each_entry(info, &xen_irq_list_head, list) { >> + if (info->type != IRQT_PIRQ) >> + continue; > > I think you need to do this by querying Xen state rather than relying on > potentially bad guest state. Particularly since you may crash while > holding irq_mapping_update_lock. > > EVTCHNOP_status gets you the info you need I think. > > David -- Vitaly _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |