Re: [Qemu-devel] [PATCH v2 1/2] exec: add page_mask for flatview_do_translate

[Maxime Coquelin]

On 10/06/2017 02:48 PM, Paolo Bonzini wrote:
> On 06/10/2017 14:46, Maxime Coquelin wrote:
> > > >            addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
> > > >                    | (addr & iotlb.addr_mask));
> > > > -        *plen = MIN(*plen, (addr | iotlb.addr_mask) - addr + 1);
> > > > +        page_mask = iotlb.addr_mask;
> > >
> > > Should this be "page_mask &= iotlb.addr_mask"?
> > >
> > > If you have multiple IOMMUs on top of each other (yeah, I know...) I
> > > think the smallest size should win.  This is also consistent with the
> > > MIN in the line below.
> >
> > I agree, but changin to "page_mask &= iotlb.addr_mask" will not be
> > enough, we also have to change the init value. Else we will always end
> > up with 0xfff.
> >
> > Maybe we could do as plen was handled before, i.e. setting page_mask
> > init value to (hwaddr)(-1), and after the loop set it to
> > ~TARGET_PAGE_MASK if it hasn't been changed.
> >
> > Does that sound reasonable?
>
> True that, in fact it makes sense for the "IOTLB entry" to represent all
> of memory if there's no IOMMU at all.

Indeed, that makes sense as no iommu means identity mapping. It would
moreover improve performance, as the vhost backend will only have a
single IOTLB entry in its cache.

Maybe it is better to wait for Peter to understand the reason he limited
it to the target page size?

Thanks,
Maxime

> Thanks,
>
> Paolo