[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>, Andrew Cooper <amc96@xxxxxxxx>
- From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
- Date: Tue, 19 Jul 2022 11:22:18 +0000
- Accept-language: en-GB, en-US
- 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=jpE5EaLymmS3gJJbX75WqtTv82C9G5itlG5AWkllfU8=; b=UmsFF2zeXlyXuUNnsO+hP0dJEjkn30bnph0U8om+pR6JwcMI1EZmGEXb8SaBar2N8l2hNOabK/R8PJnzTCnSunl2viJNIwQJrcr1aioXU8fUkpCqjzeLFiaDbjetA49YYswxZxp8FCDSKaLZscAKPJ3/xahjohJdqEQ87XJa0YT8CV/+UECqon8ptN+N1cYvIIzCKuKmmvwq5pPcc7tctUtPWuXF+vsBQ1Ps22Uaf/oiuUbw9K2+bgQi58evU40IabVuAWl8RTglA4jbjbV1eGdW8epIgIU0rkVc3NQfZbmGRVF3Kh32Fdm3IcwHsmY3KMDXfcL7Avkc0K9obI/NvQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=W9N3K9TaklxVcgXaab/UvgJIQybb/tiqs0wH5RU6MxbaDzQ7dKHYxJx+e6UAEIUAV9MjFzFYOJwdRUaDBUbuJNMCFIlYzd25Ek/H+TyhyQx/ZcK8at/Ga3TtVWSLmdn0K+TZB5BzKmSM0G+d4tXoz6jzDhrKqfGRjiZdd1HlhQGzqvAmS9fxhO19D636lLC695t0+kJbBIESYvdPeGREPpLoKC4wbxtxICGaKnWAeMm6weZcXthoYJiRLc1OvGTApddD/RyYPKuWhol3+LoY848gmyyZ7kc4Sn591FizLqic5kEANcpQMZy2hgHJInRR9oSb0/YDT8smLEtLH+obSg==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Tue, 19 Jul 2022 11:22:39 +0000
- Ironport-data: A9a23:lDH27ql6kBjYob9RoDzH9GDo5gyvJ0RdPkR7XQ2eYbSJt1+Wr1Gzt xJMWDuCP62IZ2L3e4t1Po20oxwAvZSHyNFlTABuqiAyHyMWpZLJC+rCIxarNUt+DCFioGGLT Sk6QoOdRCzhZiaE/n9BCpC48T8mk/ngqoPUUIbsIjp2SRJvVBAvgBdin/9RqoNziJ2yDhjlV ena+qUzA3f4nW8uWo4ow/jb8kk3462j4GpwUmEWPpingnePzxH5M7pHTU2BByOQapVZGOe8W 9HCwNmRlo8O105wYj8Nuu+TnnwiGtY+DyDX4pZlc/HKbix5jj4zys4G2M80Mi+7vdkrc+dZk 72hvbToIesg0zaldO41C3G0GAkmVUFKFSOuzdFSfqV/wmWfG0YAzcmCA2kNJdE2w+hlAVtL+ KYKBh5WZT7SusS5lefTpulE3qzPLeHNFaZH5jRM6G+cCvwrB5feX6/N+NlUmi8qgdxDFurfY MxfbidzaBPHYFtEPVJ/5JAWxb/0wCWgNWID7g7JzUY0yzG7IAhZ+b7hKtfKPPeNQt1YhB2wr WPa5WXpRBodMbRzzBLaoiz81rKexksXXqobDYKl0OFKx2eh4WsdGhRGWGD45uWm3xvWt9V3b hZ8FjAVhao4+VGvT9L9dwalu3PCtRkZM/JZFuZrtimW0KHapQCUGgAsUTppeNEg8sgsSlQCx lKP2t/kGzFrmLmUUm6GsKeZqyuoPioYJnNEYjULJTbp+PHmqYA3yx7KENBqFfbpisWvQW2sh TeXsCI5mrMfy9YR0Lm29kzGhDTqoYXVSgky5UPcWWfNAh5FWbNJrreAsTDzhcus5q7DJrVdl BDoQ/Sj0d0=
- Ironport-hdrordr: A9a23:s9LCHaNJ31eW6cBcT2L155DYdb4zR+YMi2TDiHoddfUFSKalfp 6V98jzjSWE8wr4WBkb6LO90DHpewKQyXcH2/hqAV7EZnirhILIFvAp0WKG+VHd8kLFh4lgPM tbEpSWTeeAdWSS7vyKrzVQcexQpuVvmZrA7Yix854ud3ASV0gK1XYaNu/vKDwTeOAwP+tdKH Pz3Kp6jgvlXU5SQtWwB3EDUeSGjcbMjojabRkPAANiwBWSjBuzgYSKUiSw71M7aXdi0L0i+W /Kn0jS/aO4qcy2zRfayiv684lWot380dFObfb8yvT9aw+cyTpAVr4RHoFqjwpF5N1HL2xa1+ Ukli1QffibLUmhOF1d7yGdgjUImwxelkMKgWXo/UcL5/aJCg7SQvAx+76wOHHimjUdlcA536 RR022DsZ1LSRvGgSTm/tDNEwpnj0yuvBMZ4KcuZlFkIPwjgYVq3Poi1VIQFI1FEDPx6YghHu UrBMbA5OxOeVffa3zCpGFgzNGlQ3x2R369MwM/k93Q1yITkGFyzkMeysBalnAc9IglQ50B4+ jfKKxnmLxHU8dTZ6NgA+UKR9exFwX2MFrxGXPXJU6iGLAMOnrLpZKy6LIp5PuycJhN15c2kI SpaiItiYfzQTOaNSSj5uw6zvmWehTNYd3E8LAs27Fp/rvhWbHsLSqPDFgzjsrImYRsPvHm
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Thread-index: AQHX8DQPNo4qXehMEESfj1fdbKCDRqw1BvMAgAAbnwCBUcCggA==
- Thread-topic: [PATCH] x86: enable interrupts around dump_execstate()
On 16/12/2021 13:33, Jan Beulich wrote:
> On 16.12.2021 12:54, Andrew Cooper wrote:
>> On 13/12/2021 15:12, 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.
>>>
>>> Fixes: adb715db698b ("x86/HVM: also dump stacks from
>>> show_execution_state()")
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> ---
>>> The obvious (but imo undesirable) alternative is to suppress the call to
>>> show_hvm_stack() when interrupts are disabled.
>> show_execution_state() need to work in any context including the #DF
>> handler,
> Why? There's no show_execution_state() on that path.
Yes there is - it's reachable from any BUG().
It's also reachable on the NMI path via fatal_trap().
Talking of, didn't you say you'd found an unexplained deadlock with NMI
handling... ?
>
>> and
>>
>> /*
>> * Stop interleaving prevention: The necessary P2M lookups
>> * involve locking, which has to occur with IRQs enabled.
>> */
>> console_unlock_recursive_irqrestore(flags);
>>
>> show_hvm_stack(curr, regs);
>>
>> is looking distinctly dodgy...
> Well, yes, it does.
Because it is.
You cannot enable interrupts here, because you have no clue if it safe
to do so.
What you are doing is creating yet another instance of the broken
pattern we already have with shutdown trying to move itself to CPU0,
that occasionally explodes in the middle of a context switch.
Furthermore show_execution_state() it is already broken for any path
where interrupts are already disabled, including but not limited to the
one you've found here.
adb715db698bc8ec3b88c24eb88b21e9da5b6c07 is broken and needs reverting.
No amount of playing games with irqs here is going to improve things.
~Andrew
|
