Re: [PATCH v8 08/25] KVM: arm64: iommu: Add memory pool

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] kvm: incorrect WARN_ON() on expected OOM during IOMMU shadow page table allocation
- [Low] kvm: math error in early_iommu_pgt_mem() bounds check artificially limits IOMMU pool to 4GB
--

commit c0283e6a54103e1cdad88307cf87917d222a0087
Author: Mostafa Saleh <smostafa@google.com>

KVM: arm64: iommu: Add memory pool

This commit introduces an IOMMU memory pool for shadow page tables, allocating
it early from the KVM carveout based on the kvm-arm.iommu_pgt_mem command
line parameter.

> diff --git a/arch/arm64/kvm/hyp/nvhe/iommu.c b/arch/arm64/kvm/hyp/nvhe/iommu.c
> --- a/arch/arm64/kvm/hyp/nvhe/iommu.c
> +++ b/arch/arm64/kvm/hyp/nvhe/iommu.c
[ ... ]
> @@ -122,6 +123,14 @@ int pkvm_iommu_init(void)
>  	    !pkvm_iommu_ops->host_stage2_idmap)
>  		return 0;
>  
> +	if (!nr_pages)
> +		return -ENOMEM;
> +
> +	ret = hyp_pool_init(&iommu_pages_pool, hyp_virt_to_pfn(pool_base),
> +			    nr_pages, 0);
> +	if (ret)
> +		return ret;
> +
>  	ret = pkvm_iommu_ops->init();
>  	if (ret)
>  		return ret;

[Severity: Medium]
Does wrapping pkvm_iommu_snapshot_host_stage2() in WARN_ON() safely
handle expected out-of-memory conditions?

Looking at the rest of pkvm_iommu_init(), it ends with:

	return WARN_ON(pkvm_iommu_snapshot_host_stage2());

The snapshot function walks the host page tables and allocates memory for
IOMMU shadow mappings from the fixed-size pool configured by the host via
kvm-arm.iommu_pgt_mem.

If the user-provided pool is too small, the allocation fails and returns
-ENOMEM. At EL2, WARN_ON() expands to a fatal BRK instruction, which panics
the hypervisor and crashes the host.

Could we return the error directly without panicking, since this turns a
recoverable initialization failure into a host-trusted denial of service
vector?

> diff --git a/arch/arm64/kvm/iommu.c b/arch/arm64/kvm/iommu.c
> --- a/arch/arm64/kvm/iommu.c
> +++ b/arch/arm64/kvm/iommu.c
[ ... ]
> +static int __init early_iommu_pgt_mem(char *arg)
> +{
> +	unsigned long long requested_size;
> +
> +	if (!arg)
> +		return -EINVAL;
> +
> +	requested_size = memparse(arg, NULL);
> +
> +	if (requested_size > UINT_MAX) {

[Severity: Low]
Is this math check bounding the IOMMU pool size to 4GB?

The early_iommu_pgt_mem() function parses the user-requested size into
bytes, and bounds requested_size against UINT_MAX.

Since the target variable hyp_kvm_iommu_pages stores a page count as an
unsigned int, the byte limit could safely be much larger, up to
(u64)UINT_MAX * PAGE_SIZE.

Does this artificially reject valid configurations on systems with massive
RAM that might require larger page table pools?

> +		kvm_err("kvm-arm.iommu_pgt_mem is too large\n");
> +		return -EINVAL;
> +	}
> +
> +	kvm_nvhe_sym(hyp_kvm_iommu_pages) = DIV_ROUND_UP(requested_size, PAGE_SIZE);
> +	return 0;
> +}
> +early_param("kvm-arm.iommu_pgt_mem", early_iommu_pgt_mem);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922131259.2975334-1-smostafa@google.com?part=8