Re: [PATCH v6 08/25] KVM: arm64: iommu: Shadow host stage-2 page table

[Vincent Donnefort]

On Mon, Jul 13, 2026 at 03:51:07PM +0000, Mostafa Saleh wrote:
> On Mon, Jul 13, 2026 at 04:14:55PM +0100, Vincent Donnefort wrote:
> > On Mon, Jul 13, 2026 at 02:00:31PM +0000, Mostafa Saleh wrote:
> > > On Mon, Jul 13, 2026 at 02:24:19PM +0100, Vincent Donnefort wrote:
> > > > On Fri, May 01, 2026 at 11:19:10AM +0000, Mostafa Saleh wrote:
> > > > > Create a page-table for the IOMMU that shadows the host CPU stage-2
> > > > > to establish DMA isolation.
> > > > > 
> > > > > An initial snapshot is created after the driver init, then
> > > > > on every permission change a callback would be called for
> > > > > the IOMMU driver to update the page table.
> > > > > 
> > > 
> > > [...]
> > > 
> > > > > +	 */
> > > > > +	if (pte && !kvm_pte_valid(pte))
> > > > > +		return 0;
> > > > > +
> > > > > +	if (kvm_pte_valid(pte)) {
> > > > > +		prot = pkvm_to_iommu_prot(kvm_pgtable_stage2_pte_prot(pte));
> > > > > +		/* If the range is mapped in a single PTE, it must be the same type.*/
> > > > > +		if (!addr_is_memory(start))
> > > > > +			prot |= IOMMU_MMIO;
> > > > > +
> > > > > +		return kvm_iommu_ops->host_stage2_idmap(start, end, prot);
> > > > 
> > > > Do we really need to do that when is_memory()? 
> > > > 
> > > > fix_host_ownership_walker() by calling host_stage2_idmap_locked() and
> > > > host_stage2_set_owner_locked() should already handle the memory region. That
> > > > would also get rid of kvm_idmap_initialized.
> > > > 
> > > > So this one here could only take care of the MMIO?
> > > > 
> > > > Overall we would have a common point of synchro which is
> > > > fix_host_ownership_walker() after which the host ownership is ready for both
> > > > CPU stage-2 and the IOMMU?
> > > > 
> > > 
> > > I am not sure I understand, this is another empty page table, so we
> > > have to walk all of the host CPU stage-2 page table to shadow it in the
> > > IOMMU. if you are refering to the case where it handle zero ptes for
> > > memory, I can drop that but it will not change much in this logic.
> > 
> > In fixup_host_ownership() we already walk the hyp pgtable to know what needs to be
> > map/unmapped from the host stage-2. Can't we rely on that for the IOMMU
> > page-table as well?
> > 
> > As of, fix_host_ownership() could handle the host stage-2 __and__ the iommu? 
> 
> fix_host_ownership() only walks the hypervisor page table, and fixes the
> hypervisor pages in the host.

Yeah, Looking closer I don't think my proposal simplify things so much in the
end.

> That means that the IOMMU page table has
> to  be fully populated first, then we unmap the donated pages.
> That seems more complicated and it feels that decoupling the IOMMU
> logic outside of this would be better.

Why more complicated? That's actually another solution I was thinking about, to
just map everything as soon as possible and let host_stage2_idmap_locked() and
host_stage2_set_owner_locked() unmap what is needed here. (and also in
__pkvm_host_donate_hyp_mmio())

We wouldn't need any specific setup walker for the iommu at all.

> 
> Also, I rely on the IOMMU init to be done at the end of setup, so we
> do not have to clean the IOMMU init if any other part of KVM fails.

We could just leak those pages if it fails?

> 
> > 
> > > 
> > > > > +	}
> > > > > +
> > > > > +	/* In case of invalid PTE, we need to figure out which part of it is MMIO */
> > > 
> > > [...]
> > > 

[...]