Re: [PATCH v1 06/13] mshv: detect and report SEV-SNP support at init

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Failing to query optional SEV-SNP support aborts the entire `mshv` module initialization, preventing non-SNP virtual machines from running on unsupported platforms.
- [Low] The helper function `hv_snp_status_to_string` is defined unconditionally but only used inside an x86-specific block, causing a build failure on non-x86 architectures.
--

--- Patch [6]: [PATCH v1 06/13] mshv: detect and report SEV-SNP support at init ---

commit e3193a7357d34307ce8275f28a3eb193fd3183cf
Author: Wei Liu [off-list ref]

mshv: detect and report SEV-SNP support at init
    
Query Hyper-V dynamic processor features during mshv initialization to
determine SEV-SNP availability, and report the result.

[ ... ]

+	status = hv_do_hypercall(HVCALL_GET_SYSTEM_PROPERTY, input, output);
+	if (!hv_result_success(status)) {
+		local_irq_restore(flags);
+		dev_err(dev, "Failed to get max SNP partitions: %s\n",
+			hv_result_to_string(status));
+		return hv_result_to_errno(status);
+	}

[Severity: High]
Does propagating this error cause driver initialization to abort on platforms
that do not support SEV-SNP? 

If the hypervisor does not support this dynamic processor feature, returning
an error here will cause the entire mshv module to fail to load because it
is propagated up to mshv_parent_partition_init().

[ ... ]

+static const char *hv_snp_status_to_string(enum hv_snp_status status)
+{
+	switch (status) {
+	case HV_SNP_STATUS_NONE:
+		return "not available";

[Severity: Low]
This function is defined unconditionally, but it is only called from within
an `#if defined(__x86_64__)` block in mshv_check_sev_snp_support(). 

Does this cause an unused function warning on non-x86 architectures (such as
ARM64) that could break the build when CONFIG_WERROR is enabled?

[ ... ]

+static int __init mshv_check_sev_snp_support(struct device *dev)
+{
+#if defined(__x86_64__)
+	struct hv_input_get_system_property *input;
+	struct hv_output_get_system_property *output;
+	unsigned long flags;
+	u64 status;
+	enum hv_snp_status snp_status;
+
+	local_irq_save(flags);
+	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
+
+	memset(input, 0, sizeof(*input));
+	input->property_id = HV_DYNAMIC_PROCESSOR_FEATURE_PROPERTY;
+	input->hv_processor_feature = HV_X64_DYNAMIC_PROCESSOR_FEATURE_SNP_STATUS;
+
+	status = hv_do_hypercall(HVCALL_GET_SYSTEM_PROPERTY, input, output);
+	if (!hv_result_success(status)) {
+		local_irq_restore(flags);
+		dev_err(dev, "Failed to get SNP support: %s\n",
+			hv_result_to_string(status));
+		return hv_result_to_errno(status);
+	}

[Severity: High]
Similar to above, does returning an error here prevent the mshv module from
loading on systems without SEV-SNP support (like Intel CPUs) or on older
Hyper-V versions? 

It appears this will abort module initialization if the hypercall fails.

+
+	snp_status = output->hv_processor_feature_value;
+	local_irq_restore(flags);
+
+	dev_info(dev, "SEV-SNP support status: %s (%u)\n",
+		 hv_snp_status_to_string(snp_status), snp_status);
+
+	if (snp_status == HV_SNP_STATUS_AVAILABLE)
+		return mshv_print_max_sev_snp_partitions(dev);
+#endif
+
+	return 0;
+}
+
 static int
 root_scheduler_init(struct device *dev)
 {

[ ... ]

[Severity: High]
This is where the error from the informational SEV-SNP checks causes module
initialization to abort. Should these functions handle unsupported features
gracefully instead of failing the init?

 	ret = root_scheduler_init(dev);
 	if (ret)
 		goto synic_cleanup;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807135134.303943-1-weh@linux.microsoft.com?part=6