Re: [PATCH v2 01/13] KVM: arm64: Donate MMIO to the hypervisor

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] In `__pkvm_hyp_donate_host_mmio`, if `kvm_pgtable_hyp_unmap` fails, the function jumps to the error path but fails to set `ret` to an error code, silently returning success (0).
--

--- Patch [1]: [PATCH v2 01/13] KVM: arm64: Donate MMIO to the hypervisor ---
commit 91906bfffc8e911901f2cb86959abcf8b184a527
Author: Mostafa Saleh <[email protected]>

KVM: arm64: Donate MMIO to the hypervisor

This patch introduces the ability for the host to donate MMIO ranges to
the hypervisor's linear map in pKVM. It initializes the host stage-2 page
table with an invalid leaf containing the hypervisor's owner ID to prevent
host mapping faults.

> diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> index 4e329e39a695a..5cf7c4a0ed208 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c

[ ... ]

> +int __pkvm_hyp_donate_host_mmio(phys_addr_t addr, size_t size)
> +{
> +	kvm_pte_t pte;
> +	u64 offset;
> +	int ret = 0;
> +	void *virt;

[ ... ]

> +	for (offset = 0; offset < size; offset += PAGE_SIZE) {
> +		if (addr_is_memory(addr + offset)) {
> +			ret = -EINVAL;
> +			goto err_with_unmap;
> +		}
> +		ret = kvm_pgtable_get_leaf(&host_mmu.pgt, addr + offset, &pte, NULL);
> +		if (ret)
> +			goto err_with_unmap;

[ ... ]

> +		virt = __hyp_va(addr + offset);
> +		if (kvm_pgtable_hyp_unmap(&pkvm_pgtable, (u64)virt, PAGE_SIZE) != PAGE_SIZE)
> +			goto err_with_unmap;

[Severity: High]
When kvm_pgtable_hyp_unmap() fails here, does the code return success?

Looking at __pkvm_hyp_donate_host_mmio() in mem_protect.c, the ret variable
would be 0 from the earlier successful kvm_pgtable_get_leaf() call. By
jumping to err_with_unmap without updating ret, the function silently returns
0 to the caller even though the unmap operation failed.

> +	}
> +	WARN_ON(host_stage2_idmap_locked(addr, size, PKVM_HOST_MMIO_PROT));
> +unlock:
> +	hyp_unlock_component();
> +	host_unlock_component();
> +	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1