Skip to content

x86/hyperv: Fix wrong argument to hv_vtl_bringup_vcpu#126

Open
hargar19 wants to merge 4 commits intoproject/hcl-dev/6.18from
user/hargar/6.18-tdx-fix-dev-branch
Open

x86/hyperv: Fix wrong argument to hv_vtl_bringup_vcpu#126
hargar19 wants to merge 4 commits intoproject/hcl-dev/6.18from
user/hargar/6.18-tdx-fix-dev-branch

Conversation

@hargar19
Copy link
Copy Markdown
Collaborator

The revert of the TDX reset page commit (3fca701) left apicid instead of cpu as the second argument to hv_vtl_bringup_vcpu(). This causes secondary CPUs to boot with the wrong idle task stack, leading to stack corruption and triple faults during sidecar servicing.

Fixes: 3fca701 ("Revert "x86/hyperv: Use Hyper-V reset page to boot tdx APs"")

Signed-off-by: Hardik Garg hargar@microsoft.com

The revert of the TDX reset page commit (3fca701) left
`apicid` instead of `cpu` as the second argument to
hv_vtl_bringup_vcpu(). This causes secondary CPUs to boot with
the wrong idle task stack, leading to stack corruption and
triple faults during sidecar servicing.

Fixes: 3fca701 ("Revert "x86/hyperv: Use Hyper-V reset page to boot tdx APs"")

Signed-off-by: Hardik Garg <hargar@microsoft.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a regression in the x86 Hyper-V VTL2 AP bringup path where the wrong identifier was passed to hv_vtl_bringup_vcpu(), which could lead to secondary CPUs using an incorrect idle task stack and crashing during servicing.

Changes:

  • Pass cpu (logical CPU number) instead of apicid as the second argument to hv_vtl_bringup_vcpu().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Brian Perkins and others added 3 commits April 21, 2026 16:45
…uests running in Hyper-V.

For backward compatibility, turn it off only when built with CONFIG_HYPERV_VTL_MODE.
This addresses an issue where the TSC_ADJUST runtime checks would conflict with
restore_partition_time handling that would adjust the global TSC value using a Hyper-V
hypercall.

Signed-off-by: Brian Perkins <brian.perkins@microsoft.com>
…ALLOC

Revert the hypercall page allocation range from MODULES_VADDR..MODULES_END
back to VMALLOC_START..VMALLOC_END to test if this upstream change (commit
c8ed081) causes VBS triple faults in VTL2.

The upstream commit moved the allocation to allow direct calls via
static_call, but VBS isolation may depend on the hypercall page being
in the vmalloc range.

This is a test commit - to be reverted if not the root cause.
Revert the VTL return hypercall mechanism from the new static call
trampoline assembly (__mshv_vtl_return_call in mshv_vtl_asm.S) back to
the 6.12-style inline asm using CALL_NOSPEC.

This tests whether the refactored VTL return path (which uses
call STATIC_CALL_TRAMP_STR(__mshv_vtl_return_hypercall) in a separate
.S file) is the cause of VBS triple faults in VTL2.

The key differences:
- 6.12: inline asm in mshv_vtl_return_call() with CALL_NOSPEC
- 6.18: separate __mshv_vtl_return_call() in mshv_vtl_asm.S with
  call to static_call trampoline

This is a test commit - to be reverted if not the root cause.
Copilot AI review requested due to automatic review settings April 21, 2026 21:13
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread arch/x86/hyperv/hv_vtl.c
Comment on lines +377 to +381
: "=a"(vtl0->rax), "=c"(vtl0->rcx),
"+d"(vtl0->rdx), "+b"(vtl0->rbx), "+S"(vtl0->rsi), "+D"(vtl0->rdi),
"+r"(r8), "+r"(r9), "+r"(r10), "+r"(r11),
"+r"(r12), "+r"(r13), "+r"(r14), "+r"(r15)
: THUNK_TARGET(hypercall_addr), "c"(&vtl0->rbp)
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This inline asm uses RCX both as an output (=c for vtl0->rcx) and as a separate fixed-reg input ("c"(&vtl0->rbp)). GCC/Clang do not allow an untied input operand to overlap a write-only output operand in the same register, so this is likely to fail to compile with an “impossible constraints” style error. Use a single in/out operand for RCX (e.g. +c` with a temporary that starts as &vtl0->rbp and ends as the returned RCX), or pass the rbp pointer in a different register/class and reference it via a named operand in the asm template.

Copilot uses AI. Check for mistakes.
Comment thread arch/x86/hyperv/hv_vtl.c
Comment on lines +85 to +89
/*
* Disable TSC_ADJUST so that the periodic TSC sync check timer
* (start_sync_check_timer) and related TSC adjustment logic are
* skipped. The TSC exposed via Hyper-V will be consistent across
* processors.
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description/title only mention fixing the wrong argument passed to hv_vtl_bringup_vcpu(), but this hunk also introduces a new CPU feature override (clearing X86_FEATURE_TSC_ADJUST). Please either update the PR description to cover the additional behavioral change (and rationale/impact), or split it into a separate patch to keep the fix focused.

Copilot uses AI. Check for mistakes.
Comment thread arch/x86/hyperv/hv_vtl.c
r14 = vtl0->r14;
r15 = vtl0->r15;

asm __volatile__ ( \
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an unnecessary trailing '' after asm __volatile__ (. That backslash escapes the newline in the preprocessor stage and is easy to miss/review; please drop it to avoid confusing formatting and potential style/tooling issues.

Suggested change
asm __volatile__ ( \
asm __volatile__ (

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants