Re: [PATCH v7 02/24] KVM: arm64: Donate MMIO to the hypervisor
Sebastian Ene <[email protected]> Wed, 5 Aug 2026 14:57:02 +0000
| Newsgroups | dev.linux.lists.kvmarm,dev.linux.lists.iommu,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Wed, Aug 05, 2026 at 01:42:10PM +0000, Mostafa Saleh wrote: > On Wed, Aug 05, 2026 at 12:47:03PM +0000, Sebastian Ene wrote: > > On Wed, Jul 15, 2026 at 11:58:43AM +0000, Mostafa Saleh wrote: > > > Add a function to donate MMIO to the hypervisor so IOMMU hypervisor > > > drivers can protect and access the MMIO of IOMMUs. > > > > > > As donating MMIO is very rare, and we don’t need to encode the full > > > state, it’s reasonable to have a separate function to do this. > > > It will init the host s2 page table with an invalid leaf with the owner ID > > > to prevent the host from mapping the page on faults. > > > > > > Also, prevent kvm_pgtable_stage2_unmap() from removing owner ID from > > > stage-2 PTEs, as this can be triggered from recycle logic under memory > > > pressure. There is no code relying on this, as all ownership changes is > > > done via kvm_pgtable_stage2_set_owner() > > > > > > For the error path in IOMMU drivers, add a function to donate MMIO > > > back from hyp to host. However, that leaks the hypervisor virtual > > > address range which should be acceptable as this is quite rare and > > > it matches the behaviour of fix_map/block. > > > > > > Signed-off-by: Mostafa Saleh <[email protected]> > > > --- > > > arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 7 ++ > > > arch/arm64/kvm/hyp/nvhe/mem_protect.c | 91 ++++++++++++++++++- > > > arch/arm64/kvm/hyp/pgtable.c | 11 +-- > > > 3 files changed, 102 insertions(+), 7 deletions(-) > > > > > > diff --git a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > > > index 29935c7da1de..51b0eb3844a9 100644 > > > --- a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > > > +++ b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > > > @@ -36,6 +36,13 @@ int __pkvm_guest_share_host(struct pkvm_hyp_vcpu *vcpu, u64 gfn); > > > int __pkvm_guest_unshare_host(struct pkvm_hyp_vcpu *vcpu, u64 gfn); > > > int __pkvm_host_unshare_hyp(u64 pfn); > > > int __pkvm_host_donate_hyp(u64 pfn, u64 nr_pages); > > > +/* > > > + * Donate MMIO range to the hypervisor, it will be mapped in the hypervisor's > > > + * private range and unmapped from the host stage-2. > > > + */ > > > +int __pkvm_host_donate_hyp_mmio(phys_addr_t addr, size_t size, unsigned long *haddr); > > > +/* Remaps MMIO range in the host, typically used in error path. */ > > > +int __pkvm_hyp_donate_host_mmio(phys_addr_t addr, size_t size); > > > int __pkvm_hyp_donate_host(u64 pfn, u64 nr_pages); > > > int __pkvm_host_share_ffa(u64 pfn, u64 nr_pages); > > > int __pkvm_host_unshare_ffa(u64 pfn, u64 nr_pages); > > > diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c > > > index 4e329e39a695..d803b3dd4cb4 100644 > > > --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c > > > +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c > > > @@ -378,7 +378,11 @@ static int host_stage2_unmap_dev_all(void) > > > u64 addr = 0; > > > int i, ret; > > > > > > - /* Unmap all non-memory regions to recycle the pages */ > > > + /* > > > + * Unmap all non-memory regions to recycle the pages. > > > + * That relies on kvm_pgtable_stage2_unmap() not clearing > > > + * counted PTEs which include hypervisor MMIO. > > > + */ > > > for (i = 0; i < hyp_memblock_nr; i++, addr = reg->base + reg->size) { > > > reg = &hyp_memory[i]; > > > ret = kvm_pgtable_stage2_unmap(pgt, addr, reg->base - addr); > > > @@ -1119,6 +1123,91 @@ int __pkvm_host_donate_hyp(u64 pfn, u64 nr_pages) > > > return ret; > > > } > > > > > > +int __pkvm_host_donate_hyp_mmio(phys_addr_t addr, size_t size, unsigned long *haddr) > > > +{ > > > + kvm_pte_t pte; > > > + u64 offset; > > > + int ret; > > > + > > > + /* Only before de-privilege. */ > > > + if (static_branch_unlikely(&kvm_protected_mode_initialized)) > > > + return -EPERM; > > > + > > > + if (!PAGE_ALIGNED(addr | size) || > > > + !pfn_range_is_valid(hyp_phys_to_pfn(addr), size >> PAGE_SHIFT)) > > > + return -EINVAL; > > > + > > > + ret = __pkvm_create_private_mapping(addr, size, PAGE_HYP_DEVICE, haddr); > > > > > + if (ret) > > > + return ret; > > > + > > > + host_lock_component(); > > > + for (offset = 0; offset < size; offset += PAGE_SIZE) { > > > + if (addr_is_memory(addr + offset)) { > > > + ret = -EINVAL; > > > + goto unlock; > > > > If this fails we are left with the mapping inside the hyp because the unlock > > doesn't destroy the private mapping. Is this intended ? > > > > Yes, there is no way to remove a private mapping at the moment, all > the callers to __pkvm_create_private_mapping() will leak it on failure. > > > > + } > > > + ret = kvm_pgtable_get_leaf(&host_mmu.pgt, addr + offset, &pte, NULL); > > > + if (ret) > > > + goto unlock; > > > + if (pte && !kvm_pte_valid(pte)) { > > > + ret = -EPERM; > > > + goto unlock; > > > + } > > > + } > > > + /* > > > + * We set HYP as the owner of the MMIO pages in the host stage-2, for: > > > + * - host aborts: host_stage2_adjust_range() would fail for invalid non zero PTEs. > > > + * - recycle under memory pressure: host_stage2_unmap_dev_all() would call > > > + * kvm_pgtable_stage2_unmap() which will not clear non zero invalid ptes (counted). > > > + * - other MMIO donation: Would fail as we check that the PTE is valid or empty. > > > + */ > > > + ret = host_stage2_try(kvm_pgtable_stage2_annotate, &host_mmu.pgt, > > > + addr, size, &host_s2_pool, > > > + KVM_HOST_INVALID_PTE_TYPE_DONATION, > > > + FIELD_PREP(KVM_HOST_DONATION_PTE_OWNER_MASK, PKVM_ID_HYP)); > > > +unlock: > > > + host_unlock_component(); > > > + return ret; > > > +} > > > + > > > +int __pkvm_hyp_donate_host_mmio(phys_addr_t addr, size_t size) > > > > This function seems to only update the host stage-2 annotation but it > > doesn't destroy the hyp mapping. > > Yes, as mentioned above there is no way to destroy it, and this was > not desgined for frequent use. Typicaly, __pkvm_host_donate_hyp_mmio() > is called at boot per area/device. And __pkvm_hyp_donate_host_mmio() > is only used for failures. > Yes, I saw that you only allow the call during the init pKVM calls, however it seems a bit fragile. > > > > I was looking to make use of this patch in an upcoming posting for the > > v2 ITS hardening but in my case I don't need the private VA range > > creation. > > I believe if you need to map MMIO, private range is the right way to > do it as the linear map was mainly designed around system memory. Is there a reason behind not having MMIO as part of the linear map in the hypervisor ? I would like to avoid holding the hva around and just do a simple addition to resolve the hyp_va. > > Thanks, > Mostafa > Thanks, Sebastian