From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: Xen crash: map_domain_page() on an NMI path Date: Thu, 19 Dec 2013 12:11:24 +0000 Message-ID: <52B2E26C.2000402@citrix.com> References: <52B1F978.4090803@citrix.com> <20131219110036.GB20571@deinos.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20131219110036.GB20571@deinos.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan Cc: Keir Fraser , Jan Beulich , Xen-devel List List-Id: xen-devel@lists.xenproject.org On 19/12/13 11:00, Tim Deegan wrote: > At 19:37 +0000 on 18 Dec (1387391848), Andrew Cooper wrote: >> Hello, >> >> This is a stack trace caught by automated testing. The server BMC has >> indicated that it has genuinely injected an IOCK NMI (which is believed >> to be caused by a system erratum we are aware of and trying to work around) >> >> However, the interesting point is the nested crash. This is a failed >> assertion while attempting to execute the kexec crash path. Xen is >> 4.3.1 based, and built with debug, so the stack trace below is generated >> with frame pointers, and is correct. >> >> (XEN) Xen call trace: >> (XEN) [] __context_switch+0xb0/0x41e >> (XEN) [] __sync_local_execstate+0x73/0x83 >> (XEN) [] sync_local_execstate+0x9/0xb >> (XEN) [] map_domain_page+0x98/0x5c4 >> (XEN) [] map_vtd_domain_page+0xd/0x1d >> (XEN) [] queue_invalidate_context+0x94/0x141 >> (XEN) [] flush_context_qi+0x55/0x66 >> (XEN) [] iommu_flush_all+0x68/0x12f >> (XEN) [] vtd_crash_shutdown+0x15/0x64 >> (XEN) [] iommu_crash_shutdown+0x3f/0x4f >> (XEN) [] machine_crash_shutdown+0x273/0x2eb >> (XEN) [] kexec_crash+0x4c/0x70 >> (XEN) [] panic+0x12c/0x15b >> (XEN) [] fatal_trap+0xb8/0xc6 >> (XEN) [] do_nmi+0xf9/0x180 >> (XEN) [] handle_ist_exception+0x92/0xf6 >> (XEN) [] write_cr3+0x6a/0x83 >> (XEN) [] write_ptbase+0x10/0x12 >> (XEN) [] __context_switch+0x34f/0x41e >> (XEN) [] __sync_local_execstate+0x73/0x83 >> (XEN) [] sync_local_execstate+0x9/0xb >> (XEN) [] do_tasklet_work+0x9d/0xeb >> (XEN) [] tasklet_softirq_action+0x44/0x92 >> (XEN) [] __do_softirq+0x9f/0xb0 >> (XEN) [] do_softirq+0x13/0x15 >> (XEN) [] idle_loop+0x66/0x6c >> (XEN) >> (XEN) >> (XEN) **************************************** >> (XEN) Panic on CPU 0: >> (XEN) Assertion 'cpumask_empty(n->vcpu_dirty_cpumask)' failed at >> domain.c:1321 >> (XEN) **************************************** >> (XEN) >> >> Here, we have managed to re-enter the __context_switch() path because of >> an NMI interrupting it. The sync_local_execstate() in map_domain_page() >> is by way of mapcache_current_vcpu(). >> >> I am struggling to work out how best to fix this. Would it be best for >> the crash path to unconditionally change to the idle_pagetables and use >> mapcache_override_current(NULL)? > I think it would be best for the iommu_crash_shutdown() path to be > made crash-safe -- after all, that code takes spinlocks too. > Presumably we can do something a bit ruder in crash code, like just > turn the IOMMUs off entirely? > > Or are there other map_domain_page() ops on the crash path? Does > kexec need it? > > Tim. I don't believe we can safely just disable the IOMMU without tearing it down in a sensible fashion. Having said that, we certainly should try and make the crash path as "crash safe" as possible. I don't think it is reasonable to prevent the use of map_domain_page() on codepaths in the crash path (as being too invasive), but the mapcache_override_current(NULL) is an override which prevents any playing with the pagetables, with the caveat that mfn_to_virt(mfn) needs to work for all mfn's in the current set of pagetables. ~Andrew