x86/hyperv: Fix wrong argument to hv_vtl_bringup_vcpu#125
x86/hyperv: Fix wrong argument to hv_vtl_bringup_vcpu#125hargar19 wants to merge 3 commits intoproduct/hcl-main/6.18from
Conversation
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>
hargar19
requested review from
Copilot and
dcui
and removed request for
Copilot
Match VMSA page with vcpu index instead of APIC id. APIC id maybe identity map with vcpu index. Fixes: f313db8 ("x86/sev: Use hvcall to convert apic id to vpid") Signed-off-by: Tianyu Lan <tiala@microsoft.com>
The ioread32() in print_s5_reset_status_mmio() causes a triple fault in isolated VMs because the MMIO access to FCH_PM_S5_RESET_STATUS is blocked by the hypervisor. The existing workaround only checked for SNP isolation (hv_isolation_type_snp()), but VBS (software isolation) has the same restriction. Broaden the check to hv_is_isolation_supported() to cover all isolation types (SNP, TDX, and VBS), preventing the triple fault during early kernel init on VBS VMs.
There was a problem hiding this comment.
Pull request overview
Fixes a Hyper-V x86 bring-up regression where the wrong identifier was used when starting secondary vCPUs, which can lead to using an incorrect idle task stack and subsequent stack corruption/triple faults in isolated VM scenarios.
Changes:
- Pass the Linux
cpuindex (notapicid) intohv_vtl_bringup_vcpu()so the correct idle thread stack is selected. - Use the provided
cpuargument for per-CPU VMSA bookkeeping inhv_snp_boot_ap()and simplify related mapping code. - Adjust the AMD S5 reset status MMIO workaround gating to use
hv_is_isolation_supported().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| arch/x86/kernel/cpu/amd.c | Changes the condition that skips an MMIO read under Hyper-V isolation. |
| arch/x86/hyperv/ivm.c | Updates SNP AP bring-up bookkeeping to index per-CPU state by cpu and refactors mapping logic. |
| arch/x86/hyperv/hv_vtl.c | Fixes the argument passed to hv_vtl_bringup_vcpu() to use cpu instead of apicid. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * bug. Work it around for now. | ||
| */ | ||
| if (!ms_hyperv.paravisor_present && hv_isolation_type_snp()) | ||
| if (!ms_hyperv.paravisor_present && hv_is_isolation_supported()) |
There was a problem hiding this comment.
The comment says the ioread32() triple-fault issue is SNP-specific, but the new condition skips the read for any Hyper-V isolation type (hv_is_isolation_supported()), which includes non-SNP isolation modes. Either narrow the condition back to hv_isolation_type_snp() or update the comment to match the broadened behavior (and justify why all isolation types need the workaround).
| if (!ms_hyperv.paravisor_present && hv_is_isolation_supported()) | |
| if (!ms_hyperv.paravisor_present && hv_isolation_type_snp()) |
| if (vp_index < 0 || vp_index > ms_hyperv.max_vp_index) | ||
| return -EINVAL; |
There was a problem hiding this comment.
On the vp_index validation failure path, the function returns -EINVAL without freeing the newly allocated VMSA page (vmsa), leaking a page. Free vmsa (free_page()) before returning, e.g., via a common error path/goto.
| if (vp_index < 0 || vp_index > ms_hyperv.max_vp_index) | |
| return -EINVAL; | |
| if (vp_index < 0 || vp_index > ms_hyperv.max_vp_index) { | |
| free_page((u64)vmsa); | |
| return -EINVAL; | |
| } |
| } | ||
|
|
||
| return hv_vtl_bringup_vcpu(vp_index, apicid, start_eip); | ||
| return hv_vtl_bringup_vcpu(vp_index, cpu, start_eip); |
There was a problem hiding this comment.
The PR description focuses on fixing the wrong argument passed to hv_vtl_bringup_vcpu(), but this PR also changes Hyper-V isolation gating in arch/x86/kernel/cpu/amd.c and per-CPU VMSA bookkeeping in arch/x86/hyperv/ivm.c. Please either mention these additional changes in the PR description (with rationale) or split them into separate commits/PRs so the fix can be reviewed/merged independently.
The revert of the TDX reset page commit (3fca701) left
apicidinstead ofcpuas 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