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

[Mostafa Saleh <smostafa@google.com>]

On Tue, Sep 22, 2026 at 01:25:30PM +0000, sashiko-bot@kernel.org wrote:
> 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?


Yes, as mentioned in patch-7, this is by design, the IOMMU pool should
never get -ENOMEM similar to the CPU pool.

Otherwise, recovery from this would be too complicated as the hypervisor
already poked the HW.

> 
> > 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?

Not artificially, that is added due to the hypervisor buddy allocator
limits, I sent a separate patch for that:
https://lore.kernel.org/all/20260715150125.2798116-1-smostafa@google.com/

> 
> > +		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