[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] x86: enable interrupts around dump_execstate()
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Wed, 14 Sep 2022 11:13:48 +0200
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=HyIRimXdcvsWwDAu0Fp3XuJ3HBSlF0vaJM+TCBloFwI=; b=PHYgag/xJQz2kHvvNz+kIWFdRWujChp/nUuH7YpC5qgoBh5sKC/K+/QUuHYyTydmMRIJIPoZdJ8kyS1dG/i5jUMDpSo8cXsSfn0Dv41rcmhB52DotMcr3qy8IBbj6yQeP/zd658pry19bSW3r+SZBB2TOXaBQimLp7v+EVt9uaJZP9ePik9YknYj+3TSfjCfw0DNQDRiryfgxM+AE0p6K6S26xGO49dtTkOX0qqUJhnKXvm8rh48reexr586pL1mwrtg53GAbAT/tX2fMOfLLnJEaQUNv4TLSFIUmlfLzuEO5frN5cRAfwikioTBt1HjkoVdrw7ZTKv7dDV2mBOneg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fhGOeAlvrTdFoK4yycEoavBZ0yiO8wgOUo0dqh2dVBMBuZbsVOwp1Y2BM18tOpU2i7ZSbNQGUVUabfoaHPmwu2MVqbKz5aFjUPGtIqS9if4/xC69nt27Ye/467a0TKzs6FvP6y471Zj9V+usMOGV0uFDC6Xq+Y3SgQPsXv7mygFTCQz42l0ayLYPLdV1QPxhFZUVVYvPJqqum3tjB2isGI3zkojHQiNB+RMyDsIGSonUf8yaAhQENDYGkSB0sRDOZdihB6dmkaTr+Br0MGrJghoFwXjKSvUHuDYfQEOJ9TRa6v8i6anasXJ5BsoJ6cZwhKeIOZBAG4+78f70xookwQ==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
- Delivery-date: Wed, 14 Sep 2022 09:14:10 +0000
- Ironport-data: A9a23:9iENyaylLx0mg0oZbJN6t+clxyrEfRIJ4+MujC+fZmUNrF6WrkUGn 2MXXz/VOP+LZDenf9wiO97j8kJTvJOAyNc2GwdpqyAxQypGp/SeCIXCJC8cHc8zwu4v7q5Dx 59DAjUVBJlsFhcwnj/0bv676yEUOZigHtLUEPTDNj16WThqQSIgjQMLs+Mii8tjjMPR7zml4 LsemOWCfg7+s9JIGjhMsfjb+Uoz5K2aVA4w5TTSW9ga5DcyqFFNZH4vDfnZB2f1RIBSAtm7S 47rpF1u1jqEl/uFIorNfofTKiXmcJaLVeS9oiM+t5yZqgpDvkQPPpMTb5LwX6v1ZwKhxLidw P0V3XC5pJxA0qfkwIzxWDEAe81y0DEvFBYq7hFTvOTKp3AqfUcAzN1yMWJxNpASyNxlEEh8q qMaEDomVD660rfeLLKTEoGAh+wFBeyyZcY0nSglyjvUS/E7XZrEXqPGo8dC2ys9jdxPGvCYY NcFbT1ob1LLZBgn1lU/Ucpi2rv3wCShNWQH9Dp5poJui4TX5BZ237X3dsLcZ/SBRNlPn1ber WXDl4j8KkFCbYDAmWHdmp6qrunAky3JZaUyLayfqK473BqqwHQuJzRDADNXptH80CZSQel3K UYZ5y4vpqga71GwQ5/2WBjQiG6JuFsQVsRdF8U+6RqR0ezE7gCBHG8GQzVdLts8u6ceWjgCx lKP2dTzClRSXKa9THuc8vKfqmq0MC1MdGsaP3ZbEU0C/sXpp5w1glTXVNF/HaWpj9rzXzbt3 zSNqyt4jLIW5SIW65iGEZn8q2rEjvD0osQdv207gkrNAttFWbOY
- Ironport-hdrordr: A9a23:G8RRran4QMfXAzzVDwezKGnjT1XpDfO3imdD5ihNYBxZY6Wkfp +V8cjzhCWftN9OYhodcLC7V5Voj0mskKKdxbNhRYtKOzOWw1dATbsSlLcKpgeNJ8SQzI5gPM tbAstD4ZjLfCJHZKXBkXaF+rQbsb66GcmT7I+xrkuFDzsaDZ2Ihz0JdjpzeXcGIDWua6BJdq Z1saF81kedkDksH7KGL0hAe9KGi8zAlZrgbxJDLxk76DOWhTftzLLhCRCX0joXTjsKmN4ZgC D4uj28wp/mn+Cwyxfa2WOWx5NKmOH5wt8GIMCXkMAaJhjllw7tToV8XL+puiwzvYiUmR8Xue iJhy1lE9V46nvXcG3wiRzx2zP42DJr0HPmwU/wuwqXneXJABYBT+ZRj4NQdRXUr2A6ustn7a 5N12WF87JKEBLphk3GlpT1fiAvsnDxjWspkOYVgXAae5AZcqVtoYsW+14QOIscHRj99JssHI BVfYzhDc5tAB2nhk3izyhSKITGZAVyIv7GeDlJhiWt6UkYoJgjpHFoh/D2nR87heAAotd/lq b5259T5cFzp/8tHNxA7dg6MLqK40z2MGbx2TGpUCPaPZBCHU7xgLjKx5hwzN2WWfUzvegPcd L6IRhliVI=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Wed, Sep 14, 2022 at 10:31:34AM +0200, Jan Beulich wrote:
> On 14.09.2022 10:14, Jan Beulich wrote:
> > On 13.09.2022 16:50, Roger Pau Monné wrote:
> >> On Mon, Dec 13, 2021 at 04:12:55PM +0100, Jan Beulich wrote:
> >>> show_hvm_stack() requires interrupts to be enabled to avoids triggering
> >>> the consistency check in check_lock() for the p2m lock. To do so in
> >>> spurious_interrupt() requires adding reentrancy protection / handling
> >>> there.
> >>
> >> There's also an ASSERT(!in_irq()) in _percpu_write_lock() that will
> >> trigger when trying to acquire the p2m lock from spurious_interrupt()
> >> context, as p2m_lock() -> mm_write_lock() -> _mm_write_lock ->
> >> percpu_write_lock().
> >
> > s/will/may/ since spurious_interrupt() doesn't itself use irq_enter(),
do_IRQ() does call irq_enter(), and that's the caller of
spurious_interrupt() AFAICT.
> > but yes - we could nest inside a lower priority interrupt. I'll make
> > local_irq_enable() depend on !in_irq().
>
> Upon further thought I guess more precautions are necessary: We might
> have interrupted code holding the P2M lock already, and we might also
> have interrupted code holding another MM lock precluding acquiring of
> the P2M lock. All of this probably plays into Andrew's concerns, yet
> still I don't view it as a viable route to omit the stack dump for HVM
> domains, and in particular for PVH Dom0. Sadly I can't think of any
> better approach ...
Yes, I also had those concerns. The mm locks are recursive, but
spurious_interrupt() hitting in the middle of code already holding any
mm lock is likely to end up triggering the mm lock order checker.
One (likely very risky option ATM) is to introduce a per pCPU flag
that when set will turn all mm locks into noops, and use it here in
order to avoid any locking issues. This could introduce two issues at
least: first one is how resilient page walking routines are against
page tables changing under their feet. The second one is that any
page table walker p2m helper should avoid doing modifications to the
p2m, so no P2M_ALLOC or P2M_UNSHARE flags could be used.
Thanks, Roger.
|
