On 04.12.2023 10:43, Roger Pau Monne wrote:
> @@ -476,58 +406,55 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain
> *d)
> if ( !map )
> panic("IOMMU init: unable to allocate rangeset\n");
>
> - max_pfn = (GB(4) >> PAGE_SHIFT) - 1;
> - top = max(max_pdx, pfn_to_pdx(max_pfn) + 1);
> + if ( iommu_hwdom_inclusive )
> + {
> + /* Add the whole range below 4GB, UNUSABLE regions will be removed.
> */
> + rc = rangeset_add_range(map, 0, max_pfn);
> + if ( rc )
> + panic("IOMMU inclusive mappings can't be added: %d\n",
> + rc);
> + }
>
> - for ( i = 0, start = 0, count = 0; i < top; )
> + for ( i = 0; i < e820.nr_map; i++ )
> {
> - unsigned long pfn = pdx_to_pfn(i);
> - unsigned int perms = hwdom_iommu_map(d, pfn, max_pfn);
> + struct e820entry entry = e820.map[i];
>
> - if ( !perms )
> - /* nothing */;
> - else if ( paging_mode_translate(d) )
> + switch ( entry.type )
> {
> - int rc;
> + case E820_UNUSABLE:
> + if ( !iommu_hwdom_inclusive || PFN_DOWN(entry.addr) > max_pfn )
> + continue;
The !iommu_hwdom_inclusive part isn't really needed here, is it? The ...
> - rc = p2m_add_identity_entry(d, pfn,
> - perms & IOMMUF_writable ?
> p2m_access_rw
> - :
> p2m_access_r,
> - 0);
> + rc = rangeset_remove_range(map, PFN_DOWN(entry.addr),
> + PFN_DOWN(entry.addr + entry.size -
> 1));
... call here would then simply be a no-op, as it looks. And things would
overall look more safe if the removal was skipped for fewer reasons.
> @@ -605,7 +532,7 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> rangeset_destroy(map);
>
> /* Use if to avoid compiler warning */
> - if ( iommu_iotlb_flush_all(d, flush_flags) )
> + if ( iommu_iotlb_flush_all(d, map_data.flush_flags) )
> return;
> }
Ah yes, here is said change. But I think for correctness this wants
moving to the earlier patch.
Jan
