Re: [PATCH v5 02/15] IOMMU/x86: perform PV Dom0 mappings in batches

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jun 01, 2022 at 09:30:07AM +0200, Jan Beulich wrote:
> On 31.05.2022 18:01, Roger Pau Monné wrote:
> > On Fri, May 27, 2022 at 01:12:48PM +0200, Jan Beulich wrote:
> >> @@ -406,20 +406,41 @@ void __hwdom_init arch_iommu_hwdom_init(
> >>          if ( !perms )
> >>              rc = 0;
> >>          else if ( paging_mode_translate(d) )
> >> +        {
> >>              rc = p2m_add_identity_entry(d, pfn,
> >>                                          perms & IOMMUF_writable ? 
> >> p2m_access_rw
> >>                                                                  : 
> >> p2m_access_r,
> >>                                          0);
> >> +            if ( rc )
> >> +                printk(XENLOG_WARNING
> >> +                       "%pd: identity mapping of %lx failed: %d\n",
> >> +                       d, pfn, rc);
> >> +        }
> >> +        else if ( pfn != start + count || perms != start_perms )
> >> +        {
> >> +        commit:
> >> +            rc = iommu_map(d, _dfn(start), _mfn(start), count, 
> >> start_perms,
> >> +                           &flush_flags);
> >> +            if ( rc )
> >> +                printk(XENLOG_WARNING
> >> +                       "%pd: IOMMU identity mapping of [%lx,%lx) failed: 
> >> %d\n",
> >> +                       d, pfn, pfn + count, rc);
> >> +            SWAP(start, pfn);
> >> +            start_perms = perms;
> >> +            count = 1;
> >> +        }
> >>          else
> >> -            rc = iommu_map(d, _dfn(pfn), _mfn(pfn), 1ul << PAGE_ORDER_4K,
> >> -                           perms, &flush_flags);
> >> +        {
> >> +            ++count;
> >> +            rc = 0;
> >> +        }
> >>  
> >> -        if ( rc )
> >> -            printk(XENLOG_WARNING "%pd: identity %smapping of %lx failed: 
> >> %d\n",
> >> -                   d, !paging_mode_translate(d) ? "IOMMU " : "", pfn, rc);
> >>  
> >> -        if (!(i & 0xfffff))
> >> +        if ( !(++i & 0xfffff) )
> >>              process_pending_softirqs();
> >> +
> >> +        if ( i == top && count )
> > 
> > Nit: do you really need to check for count != 0? AFAICT this is only
> > possible in the first iteration.
> 
> Yes, to avoid taking the PV path for PVH on the last iteration (count
> remains zero for PVH throughout the entire loop).

Oh, I see, that chunk is shared by both PV and PVH.

Thanks, Roger.