On Tue, May 03, 2022 at 04:44:45PM +0200, Jan Beulich wrote: > On 03.05.2022 14:37, Roger Pau Monné wrote: > > On Mon, Apr 25, 2022 at 10:33:32AM +0200, Jan Beulich wrote: > >> --- a/xen/drivers/passthrough/iommu.c > >> +++ b/xen/drivers/passthrough/iommu.c > >> @@ -307,11 +338,10 @@ int iommu_map(struct domain *d, dfn_t df > >> if ( !d->is_shutting_down && printk_ratelimit() ) > >> printk(XENLOG_ERR > >> "d%d: IOMMU mapping dfn %"PRI_dfn" to mfn %"PRI_mfn" > >> failed: %d\n", > >> - d->domain_id, dfn_x(dfn_add(dfn, i)), > >> - mfn_x(mfn_add(mfn, i)), rc); > >> + d->domain_id, dfn_x(dfn), mfn_x(mfn), rc); > > > > Since you are already adjusting the line, I wouldn't mind if you also > > switched to use %pd at once (and in the same adjustment done to > > iommu_unmap). > > I did consider doing so, but decided against since this would lead > to also touching the format string (which right now is unaltered). > > >> > >> /* while statement to satisfy __must_check */ > >> - while ( iommu_unmap(d, dfn, i, flush_flags) ) > >> + while ( iommu_unmap(d, dfn0, i, flush_flags) ) > > > > To match previous behavior you likely need to use i + (1UL << order), > > so pages covered by the map_page call above are also taken care in the > > unmap request? > > I'm afraid I don't follow: Prior behavior was to unmap only what > was mapped on earlier iterations. This continues to be that way.
My bad, I was wrong and somehow assumed that the previous behavior would also pass the failed map entry, but that's not the case. > > With that fixed: > > > > Reviewed-by: Roger Pau Monné <roger....@citrix.com> > > Thanks, but I'll wait with applying this. I withdraw my previous comment, feel free to apply this. Thanks, Roger.
