drivers: hv: mshv_vtl: Support for Secure AVIC#121
drivers: hv: mshv_vtl: Support for Secure AVIC#121Brian-Perkins wants to merge 7 commits intomicrosoft:product/hcl-main/6.18from
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Secure AVIC (SEV-SNP) support to the Hyper-V VTL (mshv_vtl) path, primarily to accelerate interrupt/IPI handling for hardware-isolated guests and expose required metadata to userspace.
Changes:
- Introduces a new MSHV VTL ioctl to retrieve the Secure AVIC backing page PFN for VTL0, and allocates/configures the backing page on SNP.
- Refactors/extends in-kernel interrupt offload plumbing (APIC page selection, proxy IRR handling, APICID↔CPUID mapping) to support both TDX and SNP paths.
- Updates x86 Hyper-V / SVM related UAPI and headers (new SVM exit codes/flags, VMCB enlightenments structure, Secure AVIC backing page initializer).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| include/uapi/linux/mshv.h | Adds a new VTL ioctl definition for Secure AVIC PFN retrieval. |
| include/uapi/hyperv/hvgdk_mini.h | Adds a UAPI definition for Hyper-V VMCB enlightenments. |
| drivers/hv/mshv_vtl_main.c | Implements Secure AVIC backing page allocation/config and new ioctl; generalizes APIC/proxy-IRR handling for isolated guests. |
| arch/x86/kernel/apic/x2apic_savic.c | Exposes backing-page initialization helper for Secure AVIC. |
| arch/x86/include/uapi/asm/svm.h | Adds/aligns SVM exit reason constants used by the new SNP exit handling. |
| arch/x86/include/asm/svm.h | Adds VMCB flag bits used by SNP exit handling; adjusts Hyper-V header include. |
| arch/x86/include/asm/sev.h | Adds RMPADJUST permission bit definitions used when configuring Secure AVIC pages. |
| arch/x86/include/asm/apic.h | Declares the new Secure AVIC backing-page init helper (with stub when unsupported). |
| arch/x86/hyperv/hv_vtl.c | Disables TSC_ADJUST to skip periodic TSC sync logic under Hyper-V VTL. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
579837a to
c323ace
Compare
c323ace to
3331547
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
arch/x86/kernel/apic/x2apic_savic.c:347
x2apic_savic_init_backing_page()is now a non-static symbol used bydrivers/hv/mshv_vtl_main.c, but it is not exported fromx2apic_savic.c. SinceCONFIG_MSHV_VTLis a tristate (module-capable), buildingmshv_vtlas a module will fail to link with an undefined symbol unless this function is exported (e.g.,EXPORT_SYMBOL_GPL) or the call site is structured to avoid module->vmlinux linkage.
void x2apic_savic_init_backing_page(void *ap)
{
u32 apic_id;
/*
* Before Secure AVIC is enabled, APIC msr reads are intercepted.
* APIC_ID msr read returns the value from the Hypervisor.
*/
apic_id = native_apic_msr_read(APIC_ID);
apic_set_reg(ap, APIC_ID, apic_id);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3331547 to
78fa247
Compare
78fa247 to
44270f4
Compare
When Secure AVIC is enabled, call Secure AVIC function to allow Hyper-V to inject STIMER0 interrupt. Reviewed-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com> Reviewed-by: Michael Kelley <mhklinux@outlook.com> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
When Secure AVIC is enabled, VMBus driver should call x2apic Secure AVIC interface to allow Hyper-V to inject VMBus message interrupt. Reviewed-by: Michael Kelley <mhklinux@outlook.com> Reviewed-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
When Secure AVIC is available, the AMD x2apic Secure AVIC driver will be selected. In that case, have hv_apic_init() return immediately without doing anything. Reviewed-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com> Reviewed-by: Michael Kelley <mhklinux@outlook.com> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
Hyper-V doesn't support auto-eoi with Secure AVIC. So set the HV_DEPRECATING_AEOI_RECOMMENDED flag to force writing the EOI register after handling an interrupt. Reviewed-by: Michael Kelley <mhklinux@outlook.com> Reviewed-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
Secure AVIC provides backing page to aid the guest in limiting which interrupt vectors can be injected into the guest. Hyper-V has specific hvcall to set backing page and call it in Secure AVIC driver. Signed-off-by: Tianyu Lan <tiala@microsoft.com>
Signed-off-by: Roman Kisel <romank@linux.microsoft.com> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
44270f4 to
0955f99
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -361,7 +376,11 @@ static void savic_setup(void) | |||
| * VMRUN, the hypervisor makes use of this information to make sure | |||
| * the APIC backing page is mapped in NPT. | |||
| */ | |||
| res = savic_register_gpa(gpa); | |||
| if (hv_isolation_type_snp()) | |||
| ret = hv_set_savic_backing_page(gfn); | |||
| else | |||
| ret = savic_register_gpa(gpa); | |||
|
|
|||
| if (res != ES_OK) | |||
| sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SAVIC_FAIL); | |||
There was a problem hiding this comment.
In savic_setup(), enum es_result res is never assigned after switching to ret = hv_set_savic_backing_page() / ret = savic_register_gpa(), but the code still checks if (res != ES_OK). This leaves res uninitialized and will trigger incorrect termination; also the return types now mismatch (hv_set_savic_backing_page returns enum es_result). Use a single enum es_result res result variable and assign/check it consistently (or update the check to use ret and convert types appropriately).
| if (cc_platform_has(CC_ATTR_SNP_SECURE_AVIC)) { | ||
| hv_vp_early_input_arg = kcalloc(num_possible_cpus(), | ||
| PAGE_SIZE, | ||
| GFP_KERNEL); | ||
| if (hv_vp_early_input_arg) { | ||
| ret = set_memory_decrypted((u64)hv_vp_early_input_arg, | ||
| num_possible_cpus()); | ||
| if (ret) { | ||
| kfree(hv_vp_early_input_arg); | ||
| goto common_free; | ||
| } | ||
| } else { | ||
| goto common_free; | ||
| } |
There was a problem hiding this comment.
hv_vp_early_input_arg is allocated with kcalloc() and then passed to set_memory_decrypted(). set_memory_decrypted() warns and rounds down misaligned addresses, so this needs a PAGE_SIZE-aligned allocation to avoid converting the wrong pages. Consider allocating with a page-aligned API (e.g., __get_free_pages()/alloc_pages_exact()) rather than kcalloc().
| kfree(hv_vp_assist_page); | ||
| hv_vp_assist_page = NULL; | ||
| free_vp_early_input_arg: | ||
| set_memory_encrypted((u64)hv_vp_early_input_arg, num_possible_cpus()); |
There was a problem hiding this comment.
The free_vp_early_input_arg: error path unconditionally calls set_memory_encrypted() on hv_vp_early_input_arg. If Secure AVIC isn’t enabled (or allocation failed before setting the pointer), this can end up converting address 0 / a NULL pointer. Guard the conversion with if (hv_vp_early_input_arg) (and only attempt set_memory_encrypted() when it was successfully decrypted earlier).
| set_memory_encrypted((u64)hv_vp_early_input_arg, num_possible_cpus()); | |
| if (hv_vp_early_input_arg) | |
| set_memory_encrypted((u64)hv_vp_early_input_arg, | |
| num_possible_cpus()); |
| struct hv_set_vp_registers_input *input | ||
| = hv_vp_early_input_arg + smp_processor_id() * PAGE_SIZE; |
There was a problem hiding this comment.
hv_set_savic_backing_page() does pointer arithmetic on hv_vp_early_input_arg even though it’s declared as void * (hv_vp_early_input_arg + smp_processor_id() * PAGE_SIZE). This trips -Wpointer-arith on Clang/GCC and is non-standard C. Cast to a byte pointer for the arithmetic (or change the global’s type to u8 *) before adding the per-CPU offset.
| struct hv_set_vp_registers_input *input | |
| = hv_vp_early_input_arg + smp_processor_id() * PAGE_SIZE; | |
| struct hv_set_vp_registers_input *input = | |
| (struct hv_set_vp_registers_input *) | |
| ((u8 *)hv_vp_early_input_arg + smp_processor_id() * PAGE_SIZE); |
| */ | ||
| #define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003 | ||
| #define HV_X64_VTL_MASK GENMASK(3, 0) | ||
| #define HV_X64_REGISTER_SEV_AVIC_GPA 0x00090043 |
There was a problem hiding this comment.
These new #define constants are duplicates of ones already provided via <hyperv/hvhdk.h> → hvgdk_mini.h (e.g., HV_X64_REGISTER_VSM_VP_STATUS / HV_X64_VTL_MASK). Because this header includes hvhdk.h before these #defines, this can trigger macro redefinition warnings (notably with Clang). Prefer to define them in only one place, or wrap with #ifndef guards here.
| */ | |
| #define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003 | |
| #define HV_X64_VTL_MASK GENMASK(3, 0) | |
| #define HV_X64_REGISTER_SEV_AVIC_GPA 0x00090043 | |
| */ | |
| #ifndef HV_X64_REGISTER_VSM_VP_STATUS | |
| #define HV_X64_REGISTER_VSM_VP_STATUS 0x000D0003 | |
| #endif | |
| #ifndef HV_X64_VTL_MASK | |
| #define HV_X64_VTL_MASK GENMASK(3, 0) | |
| #endif | |
| #ifndef HV_X64_REGISTER_SEV_AVIC_GPA | |
| #define HV_X64_REGISTER_SEV_AVIC_GPA 0x00090043 | |
| #endif |
No description provided.