drivers: hv: mshv_vtl: Support for Secure AVIC #111

tiala · 2025-12-12T15:00:29Z

Secure AVIC for VTL0

Signed-off-by: Roman Kisel <[email protected]>

Copilot

Pull request overview

This PR adds support for Secure AVIC (Advanced Virtual Interrupt Controller) for VTL0 on AMD SNP (Secure Nested Paging) platforms. The implementation extends the existing TDX interrupt handling infrastructure to support SNP-based hardware-isolated VMs.

Key changes:

Refactored TDX-specific interrupt handling code to be shared between TDX and SNP platforms
Added SNP-specific APIC page management with proper RMP (Reverse Map Table) adjustments
Implemented SNP exit handling for incomplete IPI, HLT, and idle operations

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
include/uapi/linux/mshv.h	Adds new IOCTL for retrieving Secure AVIC VTL0 page frame number
arch/x86/include/uapi/asm/svm.h	Adds new SVM exit codes (BUSLOCK, IDLE_HLT) and fixes formatting
arch/x86/include/asm/svm.h	Adds VINTR control masks and works around build issues by commenting out hyperv-tlfs.h dependencies
arch/x86/include/asm/sev.h	Adds RMPADJUST permission flags for RMP configuration
arch/x86/include/asm/apic.h	Exports x2apic_savic_init_backing_page function for SAVIC initialization
arch/x86/kernel/apic/x2apic_savic.c	Exports previously internal APIC page initialization function
drivers/hv/mshv_vtl_main.c	Main implementation adding SNP Secure AVIC support, refactoring shared TDX/SNP code, and new IOCTL handlers

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

Copilot · 2025-12-12T15:09:04Z

drivers/hv/mshv_vtl_main.c

+/*
+ * Attempts to handle an ICR write. Returns 0 if successful, other values
+ * indicate user-space should be invoked to gracefully handle the error.
+ * Secure AVIC accelerates self-IPI only.


The function name is misleading. The function mshv_snp_handle_simple_icr_write has a comment stating "Secure AVIC accelerates self-IPI only" (line 498), but the implementation doesn't actually restrict to self-IPI. It processes all target CPUs via mshv_cpu_mask_for_icr_write. Either the implementation should be restricted to self-IPI only, or the comment should be updated to reflect what the function actually does.

Suggested change

* Secure AVIC accelerates self-IPI only.

* Handles ICR writes for all target CPUs as determined by mshv_cpu_mask_for_icr_write.

Copilot · 2025-12-12T15:09:04Z

drivers/hv/mshv_vtl_main.c

+	// for_each_cpu(cpu, &local_mask) {
+	// 	u64 irr_reg_off;
+	// 	unsigned long *irr_reg;
+	// 	void* irr;
+
+	// 	/*
+	// 	 * IRRs are banked into eight 32-bit registers each starting on the
+	// 	 * 16-byte boundary (4 byte of an IRR + 12 byte stride).
+	// 	 */
+	// 	irr_reg_off = (vector >> 5) << 4;
+	// 	irr = mshv_snp_secure_avic_irr(cpu);
+	// 	irr_reg = (unsigned long*)((u8*)irr + irr_reg_off);
+
+	// 	/* Inject the interrupt. */
+	// 	test_and_set_bit(vector & 0x1f, irr_reg);
+	// }
+


Commented-out code block should be removed. Large blocks of commented code reduce readability and should not be committed. If this code is needed for reference, consider moving it to commit messages or documentation.

Suggested change

// for_each_cpu(cpu, &local_mask) {

// u64 irr_reg_off;

// unsigned long *irr_reg;

// void* irr;

// /*

// * IRRs are banked into eight 32-bit registers each starting on the

// * 16-byte boundary (4 byte of an IRR + 12 byte stride).

// */

// irr_reg_off = (vector >> 5) << 4;

// irr = mshv_snp_secure_avic_irr(cpu);

// irr_reg = (unsigned long*)((u8*)irr + irr_reg_off);

// /* Inject the interrupt. */

// test_and_set_bit(vector & 0x1f, irr_reg);

// }

Copilot · 2025-12-12T15:09:04Z

drivers/hv/mshv_vtl_main.c

+	switch (vmsa->guest_exit_code)
+	{


Inconsistent brace style: The switch statement uses opening brace on the same line (line 1932), which is inconsistent with standard Linux kernel coding style where the opening brace should be on the next line for switch statements.

Copilot · 2025-12-12T15:09:04Z

drivers/hv/mshv_vtl_main.c

+		{
+			struct hv_x64_exception_intercept_message *expt_msg = 
+				(struct hv_x64_exception_intercept_message*)msg->u.payload;
+			if (expt_msg->exception_vector != X86_TRAP_VC)
+				return false;
+		}


Missing space before opening brace on line 2011, and inconsistent brace placement. The opening brace should be on the next line for consistency with kernel coding style.

Suggested change

{

struct hv_x64_exception_intercept_message *expt_msg =

(struct hv_x64_exception_intercept_message*)msg->u.payload;

if (expt_msg->exception_vector != X86_TRAP_VC)

return false;

}

{

struct hv_x64_exception_intercept_message *expt_msg =

(struct hv_x64_exception_intercept_message*)msg->u.payload;

if (expt_msg->exception_vector != X86_TRAP_VC)

return false;

}

Copilot · 2025-12-12T15:09:05Z

drivers/hv/mshv_vtl_main.c

+			void *synic_message_page = mshv_cpu->synic_message_page;
+
+			if (likely(synic_message_page))
+				msg = (struct hv_message *)synic_message_page + HV_SYNIC_INTERCEPTION_SINT_INDEX;			


Trailing whitespace on line 1990 and inconsistent indentation. The line should end without spaces, and indentation should use tabs consistently with the rest of the file.

Suggested change

msg = (struct hv_message *)synic_message_page + HV_SYNIC_INTERCEPTION_SINT_INDEX;

msg = (struct hv_message *)synic_message_page + HV_SYNIC_INTERCEPTION_SINT_INDEX;

Copilot · 2025-12-12T15:09:07Z

drivers/hv/mshv_vtl_main.c

+		// ret = sev_ghcb_msr_read(APIC_BASE_MSR + (APIC_ID >> 4), &apic_id);
+		// BUG_ON(ret != ES_OK);
+		// WRITE_ONCE(*((u32*)page_address(snp_secure_avic_page) + APIC_ID), lower_32_bits(apic_id));
+		x2apic_savic_init_backing_page(page_address(snp_secure_avic_page)); // ???


The function x2apic_savic_init_backing_page is being exported and called but there's no CONFIG_AMD_SECURE_AVIC check at the call site (line 1056). If CONFIG_AMD_SECURE_AVIC is not defined, this will result in a link error. Add a compile-time check or provide a stub implementation.

Suggested change

x2apic_savic_init_backing_page(page_address(snp_secure_avic_page)); // ???

#ifdef CONFIG_AMD_SECURE_AVIC

x2apic_savic_init_backing_page(page_address(snp_secure_avic_page));

#endif

Copilot · 2025-12-12T15:09:07Z

drivers/hv/mshv_vtl_main.c

+#endif
+}
+
+static struct page* mshv_apic_page(int cpu)


Missing space after asterisk in function declaration. Should be "struct page* mshv_apic_page" -> "struct page *mshv_apic_page" to follow kernel coding style where the asterisk is adjacent to the variable name.

Suggested change

static struct page* mshv_apic_page(int cpu)

static struct page *mshv_apic_page(int cpu)

Copilot · 2025-12-12T15:09:08Z

drivers/hv/mshv_vtl_main.c

+	ret = copy_from_user(&cpu_id, user_arg, sizeof(cpu_id)) ? -EFAULT : 0;
+	if (ret)
+		return ret;
+
+	ret = smp_call_function_single(cpu_id, secure_avic_vtl0_this_cpu, &pfn, true);


Missing validation: The cpu_id parameter is read from user space but not validated before being passed to smp_call_function_single. If cpu_id is larger than or equal to NR_CPUS or refers to an offline CPU, this could cause issues. Add validation to ensure cpu_id is valid and the CPU is online.

Copilot · 2025-12-12T15:09:08Z

drivers/hv/mshv_vtl_main.c

+					RMP_PG_SIZE_4K, 2 | RMPADJUST_ENABLE_READ | RMPADJUST_ENABLE_WRITE);
+		if (ret) {
+			pr_err("failed to adjust RMP for the secure AVIC page: %d\n", ret);
+			free_page((u64)snp_secure_avic_page);


Potential resource leak: If rmpadjust fails, the allocated page is freed with an incorrect cast. The function alloc_page returns a struct page pointer, but free_page expects an unsigned long virtual address. Use __free_page(snp_secure_avic_page) instead of free_page((u64)snp_secure_avic_page).

Suggested change

free_page((u64)snp_secure_avic_page);

__free_page(snp_secure_avic_page);

Copilot · 2025-12-12T15:09:08Z

drivers/hv/mshv_vtl_main.c

+		// ret = sev_ghcb_msr_read(APIC_BASE_MSR + (APIC_ID >> 4), &apic_id);
+		// BUG_ON(ret != ES_OK);
+		// WRITE_ONCE(*((u32*)page_address(snp_secure_avic_page) + APIC_ID), lower_32_bits(apic_id));


Commented-out code should be removed. These debug/initialization lines should either be implemented properly or removed entirely before merging.

Suggested change

// ret = sev_ghcb_msr_read(APIC_BASE_MSR + (APIC_ID >> 4), &apic_id);

// BUG_ON(ret != ES_OK);

// WRITE_ONCE(*((u32*)page_address(snp_secure_avic_page) + APIC_ID), lower_32_bits(apic_id));

drivers: hv: mshv_vtl: Support for Secure AVIC

056f157

Signed-off-by: Roman Kisel <[email protected]>

Copilot AI review requested due to automatic review settings December 12, 2025 15:00

Copilot started reviewing on behalf of tiala December 12, 2025 15:02 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

drivers: hv: mshv_vtl: Support for Secure AVIC #111

drivers: hv: mshv_vtl: Support for Secure AVIC #111

Uh oh!

tiala commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	* Secure AVIC accelerates self-IPI only.
	* Handles ICR writes for all target CPUs as determined by mshv_cpu_mask_for_icr_write.

	msg = (struct hv_message *)synic_message_page + HV_SYNIC_INTERCEPTION_SINT_INDEX;
	msg = (struct hv_message *)synic_message_page + HV_SYNIC_INTERCEPTION_SINT_INDEX;

	static struct page* mshv_apic_page(int cpu)
	static struct page *mshv_apic_page(int cpu)

	free_page((u64)snp_secure_avic_page);
	__free_page(snp_secure_avic_page);

	// ret = sev_ghcb_msr_read(APIC_BASE_MSR + (APIC_ID >> 4), &apic_id);
	// BUG_ON(ret != ES_OK);
	// WRITE_ONCE(((u32)page_address(snp_secure_avic_page) + APIC_ID), lower_32_bits(apic_id));

drivers: hv: mshv_vtl: Support for Secure AVIC #111

Are you sure you want to change the base?

drivers: hv: mshv_vtl: Support for Secure AVIC #111

Uh oh!

Conversation

tiala commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants