Re: [PATCH 04/14] iommu/pages: Add APIs to preserve/unpreserve/restore iommu pages

[Samiullah Khawaja <skhawaja@google.com>]

On Fri, Apr 10, 2026 at 11:13:55AM -0300, Jason Gunthorpe wrote:
>On Fri, Mar 20, 2026 at 06:12:28PM +0000, Samiullah Khawaja wrote:
>> > > > > +void iommu_restore_page(u64 phys)
>> > > > > +{
>> > > > > +	struct ioptdesc *iopt;
>> > > > > +	struct folio *folio;
>> > > > > +	unsigned long pgcnt;
>> > > > > +	unsigned int order;
>> > > > > +
>> > > > > +	folio = kho_restore_folio(phys);
>> > > > > +	BUG_ON(!folio);
>> > > > > +
>> > > > > +	iopt = folio_ioptdesc(folio);
>> > > >
>> > > > iopt->incoherent = false; should be here?
>> > > >
>> > >
>> > > Yes this should be set here. I will update this.
>> >
>> > I'm wondering if we are silently losing state here. What if the
>> > preserved page was actually incoherent in the previous kernel?
>> > I understand we likely need to initialize it to false here because we
>> > don't have a dev pointer for DMA sync operations at this low level (though
>> > x86 uses clflush).
>> >
>> > But when is it set back to "incoherent" again? I don't see that
>> > happening during the driver re-attach phase?
>>
>> This can be done during restore_domain as the domain has a reference to
>> the dev when it is recreated. I will updated the walker and add this in
>> the next revision.
>
>The only reason for this tracking is for DMA API debugging. It might
>be better to just remove the DMA API from this path and use a direct
>cache flush. Then you don't really need to care, the successor kernel
>never writes to this memory so it can just free it in a normal way.
>
>Otherwise you have to remap it on restore so you can unmap it on free.

Interesting.. That makes sense. Since these are not going to be written
by the successor kernel we can keep incoherent to be false.
>
>Jason