Jason Gunthorpe
2023-Sep-22 12:41 UTC
[PATCH v2 1/2] iommu/virtio: Make use of ops->iotlb_sync_map
On Fri, Sep 22, 2023 at 08:57:19AM +0100, Jean-Philippe Brucker wrote:> > > They're not strictly equivalent: this check works around a temporary issue > > > with the IOMMU core, which calls map/unmap before the domain is > > > finalized. > > > > Where? The above points to iommu_create_device_direct_mappings() but > > it doesn't because the pgsize_bitmap == 0: > > __iommu_domain_alloc() sets pgsize_bitmap in this case: > > /* > * If not already set, assume all sizes by default; the driver > * may override this later > */ > if (!domain->pgsize_bitmap) > domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap;Dirver's shouldn't do that. The core code was fixed to try again with mapping reserved regions to support these kinds of drivers. Jason
Robin Murphy
2023-Sep-22 13:13 UTC
[PATCH v2 1/2] iommu/virtio: Make use of ops->iotlb_sync_map
On 22/09/2023 1:41 pm, Jason Gunthorpe wrote:> On Fri, Sep 22, 2023 at 08:57:19AM +0100, Jean-Philippe Brucker wrote: >>>> They're not strictly equivalent: this check works around a temporary issue >>>> with the IOMMU core, which calls map/unmap before the domain is >>>> finalized. >>> >>> Where? The above points to iommu_create_device_direct_mappings() but >>> it doesn't because the pgsize_bitmap == 0: >> >> __iommu_domain_alloc() sets pgsize_bitmap in this case: >> >> /* >> * If not already set, assume all sizes by default; the driver >> * may override this later >> */ >> if (!domain->pgsize_bitmap) >> domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap; > > Dirver's shouldn't do that. > > The core code was fixed to try again with mapping reserved regions to > support these kinds of drivers.This is still the "normal" code path, really; I think it's only AMD that started initialising the domain bitmap "early" and warranted making it conditional. However we *do* ultimately want all the drivers to do the same, so we can get rid of ops->pgsize_bitmap, because it's already pretty redundant and meaningless in the face of per-domain pagetable formats. Thanks, Robin.