HAOC: IEE_SIP: Fix kdump failure in native_idt_invalidate()#1523
Open
amjac27 wants to merge 1 commit intodeepin-community:linux-6.6.yfrom
Open
HAOC: IEE_SIP: Fix kdump failure in native_idt_invalidate()#1523amjac27 wants to merge 1 commit intodeepin-community:linux-6.6.yfrom
amjac27 wants to merge 1 commit intodeepin-community:linux-6.6.yfrom
Conversation
The native_idt_invalidate() function is called via machine_kexec() during the kdump vmcore generation path. When IEE_SIP is enabled, the invalidation eventually triggers iee_rwx_gate, which fails while attempting to switch the function stack. This patch modifies native_idt_invalidate(), which is exclusively called by machine_kexec(), to always invoke the 'lidt' instruction directly. This avoids calling into the complex iee_rwx_gate.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts native_idt_invalidate() so that, when CONFIG_IEE_SIP is enabled, the kdump path uses iee_load_idt_early() instead of going through the IEE_SIP indirection, preventing a stack-switch error during kexec/kdump while still loading an invalid IDT descriptor. Sequence diagram for kdump machine_kexec IDT invalidation with CONFIG_IEE_SIPsequenceDiagram
actor KdumpPath
participant machine_kexec
participant native_idt_invalidate
participant iee_load_idt_early
participant native_load_idt
KdumpPath ->> machine_kexec: initiate kdump
machine_kexec ->> native_idt_invalidate: native_idt_invalidate()
alt CONFIG_IEE_SIP enabled
native_idt_invalidate ->> iee_load_idt_early: iee_load_idt_early(&invalid_idt)
iee_load_idt_early --> native_idt_invalidate: invalid IDT loaded
else CONFIG_IEE_SIP disabled
native_idt_invalidate ->> native_load_idt: native_load_idt(&invalid_idt)
native_load_idt --> native_idt_invalidate: invalid IDT loaded
end
native_idt_invalidate --> machine_kexec: return
machine_kexec --> KdumpPath: continue kdump without IEE_SIP stack switch
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new comments in native_idt_invalidate() use
//style, which is non-idiomatic for kernel code; please convert them to/* ... */block comments to match existing style. - Instead of adding a CONFIG_IEE_SIP-specific call to iee_load_idt_early() in native_idt_invalidate(), consider moving this conditional logic into native_load_idt() (or a helper it uses) so callers don't need to know about IEE_SIP-specific variants and you avoid sprinkling CONFIG_IEE_SIP ifdefs at call sites.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new comments in native_idt_invalidate() use `//` style, which is non-idiomatic for kernel code; please convert them to `/* ... */` block comments to match existing style.
- Instead of adding a CONFIG_IEE_SIP-specific call to iee_load_idt_early() in native_idt_invalidate(), consider moving this conditional logic into native_load_idt() (or a helper it uses) so callers don't need to know about IEE_SIP-specific variants and you avoid sprinkling CONFIG_IEE_SIP ifdefs at call sites.
## Individual Comments
### Comment 1
<location path="arch/x86/include/asm/desc.h" line_range="266-269" />
<code_context>
+#ifdef CONFIG_IEE_SIP
+ // The native_idt_invalidate() is only called by machine_kexec().
+ // In the kdump path, IEE_SIP should not be used, so we directly
+ // call iee_load_idt_early(), which is the original version of
+ // native_load_idt() to load an invalid IDT.
+ iee_load_idt_early(&invalid_idt);
+#else
native_load_idt(&invalid_idt);
</code_context>
<issue_to_address>
**question:** Comment about kdump usage may be misleading given `native_idt_invalidate()` is used for all `machine_kexec()` paths.
Since `native_idt_invalidate()` is used for all `machine_kexec()` paths (not just kdump), this wording could suggest a behavior that only applies to kdump. If you intend to describe a kdump-specific constraint, please adjust the condition or comment to clearly distinguish kdump from other kexec paths so the documentation matches the actual semantics.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+266
to
+269
| // In the kdump path, IEE_SIP should not be used, so we directly | ||
| // call iee_load_idt_early(), which is the original version of | ||
| // native_load_idt() to load an invalid IDT. | ||
| iee_load_idt_early(&invalid_idt); |
There was a problem hiding this comment.
question: Comment about kdump usage may be misleading given native_idt_invalidate() is used for all machine_kexec() paths.
Since native_idt_invalidate() is used for all machine_kexec() paths (not just kdump), this wording could suggest a behavior that only applies to kdump. If you intend to describe a kdump-specific constraint, please adjust the condition or comment to clearly distinguish kdump from other kexec paths so the documentation matches the actual semantics.
There was a problem hiding this comment.
Pull request overview
This PR fixes a kdump vmcore generation failure when machine_kexec() triggers native_idt_invalidate() on kernels built with CONFIG_IEE_SIP, by ensuring the invalid IDT load bypasses the IEE-enabled IDT loading path.
Changes:
- Update
native_idt_invalidate()to load an invalid IDT viaiee_load_idt_early()whenCONFIG_IEE_SIPis enabled. - Avoid the IEE gate/stack-switching path (
iee_rwx_gate/iee_load_idt) during kexec/kdump IDT invalidation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+265
to
+268
| // The native_idt_invalidate() is only called by machine_kexec(). | ||
| // In the kdump path, IEE_SIP should not be used, so we directly | ||
| // call iee_load_idt_early(), which is the original version of | ||
| // native_load_idt() to load an invalid IDT. |
There was a problem hiding this comment.
The new inline comment uses '//' style and includes trailing whitespace after machine_kexec(). (checkpatch will flag this). Please switch to kernel-standard /* ... */ comments and remove the trailing space to keep the header consistent with the surrounding comment style in this file.
Suggested change
| // The native_idt_invalidate() is only called by machine_kexec(). | |
| // In the kdump path, IEE_SIP should not be used, so we directly | |
| // call iee_load_idt_early(), which is the original version of | |
| // native_load_idt() to load an invalid IDT. | |
| /* | |
| * The native_idt_invalidate() is only called by machine_kexec(). | |
| * In the kdump path, IEE_SIP should not be used, so we directly | |
| * call iee_load_idt_early(), which is the original version of | |
| * native_load_idt() to load an invalid IDT. | |
| */ |
Comment on lines
+265
to
+268
| // The native_idt_invalidate() is only called by machine_kexec(). | ||
| // In the kdump path, IEE_SIP should not be used, so we directly | ||
| // call iee_load_idt_early(), which is the original version of | ||
| // native_load_idt() to load an invalid IDT. |
There was a problem hiding this comment.
The comment says "In the kdump path" but native_idt_invalidate() is invoked from machine_kexec() for both regular kexec and kdump flows. Consider rewording to describe the broader "machine_kexec"/kexec context (and the reason for bypassing the IEE-enabled path) to avoid misleading future readers.
Suggested change
| // The native_idt_invalidate() is only called by machine_kexec(). | |
| // In the kdump path, IEE_SIP should not be used, so we directly | |
| // call iee_load_idt_early(), which is the original version of | |
| // native_load_idt() to load an invalid IDT. | |
| // native_idt_invalidate() is only called from machine_kexec(), which | |
| // covers both regular kexec and kdump flows. In this context IEE_SIP | |
| // must be bypassed, so we directly call iee_load_idt_early(), the | |
| // non-IEE variant of native_load_idt(), to load an invalid IDT. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
kdump 生成 vmcore 的路径中会通过 machine_kexec() 调用 native_idt_invalidate() 。当启用 IEE_SIP 时，native_idt_invalidate() 最终会调用 iee_rwx_gate，其试图切换函数栈时发生错误。
因此，我们修改了 native_idt_invalidate() 函数，它只被 machine_kexec() 调用，使其始终直接调用 lidt，避免访问 iee_rwx_gate。
Summary by Sourcery
Bug Fixes: