Re: [PATCH V4 5/5] x86/hyperv: Implement root partition IOMMU kernel only driver

[Mukesh R <mrathor@linux.microsoft.com>]

On 11/4/26 18:38, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] iommu/hyperv: missing !hv_root_partition() check in hv_root_iommu_detect()
> --
> 
> commit c89a0e9d0fe34e767bd4527e0d99839504bcbea1
> Author: Mukesh R <mrathor@linux.microsoft.com>
> 
> x86/hyperv: Implement root partition IOMMU kernel only driver
> 
> This commit implements a kernel-only virtual IOMMU driver for baremetal
> Microsoft Hyper-V root partitions, handling paging domain management, device
> attachment, and IOVA page mapping via hypercalls.
> 
>> diff --git a/drivers/iommu/hyperv/hv-iommu-root.c b/drivers/iommu/hyperv/hv-iommu-root.c
>> new file mode 100644
>> index 0000000000000..e0e3e9d4385e2
>> --- /dev/null
>> +++ b/drivers/iommu/hyperv/hv-iommu-root.c
> 
> [ ... ]
> 
>> +static int hv_iommu_get_caps(struct hv_output_get_iommu_capabilities *caps)
>> +{
>> +	u64 status;
>> +	unsigned long flags;
>> +	struct hv_input_get_iommu_capabilities *input;
>> +	struct hv_output_get_iommu_capabilities *output;
>> +
>> +	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->partition_id = HV_PARTITION_ID_SELF;
>> +	status = hv_do_hypercall(HVCALL_GET_IOMMU_CAPABILITIES, input, output);
>> +	*caps = *output;
>> +
>> +	local_irq_restore(flags);
>> +
>> +	if (!hv_result_success(status))
>> +		hv_status_err(status, "\n");
>> +
>> +	return hv_result_to_errno(status);
>> +}
>> +
>> +static int __init hv_root_iommu_init(void)
>> +{
>> +	int rc;
>> +	struct iommu_device *iommup = &hv_virt_iommu;
>> +	struct hv_output_get_iommu_capabilities caps;
>> +
>> +	if (!hv_is_hyperv_initialized())
>> +		return -ENODEV;
>> +
>> +	rc = hv_iommu_get_caps(&caps);
> 
> [ ... ]
> 
>> +void __init hv_root_iommu_detect(void)
>> +{
>> +	if (no_iommu || iommu_detected || hv_l1vh_partition())
>> +		return;
>> +
>> +	/* This is set for root partitions only, it is never set for non-root */
>> +	if (!(ms_hyperv.misc_features & HV_DEVICE_DOMAIN_AVAILABLE))
>> +		return;
>> +
>> +	iommu_detected = 1;
>> +	x86_init.iommu.iommu_init = hv_root_iommu_init;
>> +
>> +	pci_request_acs();
>> +}
> 
> [Severity: High]
> As reported by Sashiko on v3 [1], should hv_root_iommu_detect() check
> !hv_root_partition() instead of hv_l1vh_partition()?
> 
> During x86 boot, ms_hyperv_init_platform() calls hv_identify_partition_type()
> in drivers/hv/hv_common.c, which forces hv_curr_partition_type to
> HV_PARTITION_TYPE_GUEST whenever HV_ISOLATION is set so that Confidential VMs
> do not execute root partition paths:
> 
> drivers/hv/hv_common.c:hv_identify_partition_type() {
> 	hv_curr_partition_type = HV_PARTITION_TYPE_GUEST;
> 	...
> 	if ((ms_hyperv.priv_high & HV_CREATE_PARTITIONS) &&
> 	    !(ms_hyperv.priv_high & HV_ISOLATION)) {
> 		...
> 	}
> }
> 
> Later in boot, pci_iommu_alloc() calls hv_root_iommu_detect(). Because
> hv_root_iommu_detect() only checks hv_l1vh_partition(), a guest partition
> where HV_DEVICE_DOMAIN_AVAILABLE is set in ms_hyperv.misc_features will set
> iommu_detected = 1 and x86_init.iommu.iommu_init = hv_root_iommu_init.
> 
> When hyperv_init() then calls hv_common_init() in drivers/hv/hv_common.c,
> hyperv_pcpu_output_arg is only allocated if hv_output_page_exists() is true:
> 
> drivers/hv/hv_common.c:hv_common_init() {
> 	...
> 	if (hv_output_page_exists()) {
> 		hyperv_pcpu_output_arg = alloc_percpu(void *);
> 		BUG_ON(!hyperv_pcpu_output_arg);
> 	}
> }
> 
> For HV_PARTITION_TYPE_GUEST (with CONFIG_MSHV_ROOT depending on
> !HYPERV_VTL_MODE), hv_output_page_exists() is false and hyperv_pcpu_output_arg
> remains NULL.
> 
> If CONFIG_HYPERV_PVIOMMU is disabled, pci_iommu_init() invokes
> hv_root_iommu_init() -> hv_iommu_get_caps(), which dereferences
> *this_cpu_ptr(hyperv_pcpu_output_arg) while hyperv_pcpu_output_arg is NULL. If
> CONFIG_HYPERV_PVIOMMU is enabled, hyperv_init() overwrites
> x86_init.iommu.iommu_init with hv_pviommu_init, which then fails with -ENODEV
> because hv_root_iommu_detect() already set iommu_detected = 1.
> 
> Would checking !hv_root_partition() in hv_root_iommu_detect() avoid this by
> excluding both L1VH and guest partitions?
> 
> [1] https://lore.kernel.org/r/20261103025150.3192119-6-mrathor@linux.microsoft.com
> 

No. HV_DEVICE_DOMAIN_AVAILABLE is never set for non-root, ie, guest
partitions. So, a guest partition will never set iommu_detected here.