On Fri, Jan 24, 2025 at 03:17:17PM -0500, Matthew Rosato wrote:
> Enabled via the kernel command-line 'iommu.passthrough=1' option.
>
> Introduce the concept of identity domains to s390-iommu, which relies on
> the bus_dma_region to offset identity mappings to the start of the DMA
> aperture advertized by CLP.
>
> Signed-off-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
> ---
> arch/s390/pci/pci.c | 6 ++-
> drivers/iommu/s390-iommu.c | 95 +++++++++++++++++++++++++++++---------
> 2 files changed, 76 insertions(+), 25 deletions(-)
Seems Ok
Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> -static const struct iommu_ops s390_iommu_ops = {
> - .blocked_domain = &blocking_domain,
> - .release_domain = &blocking_domain,
> - .capable = s390_iommu_capable,
> - .domain_alloc_paging = s390_domain_alloc_paging,
> - .probe_device = s390_iommu_probe_device,
> - .device_group = generic_device_group,
> - .pgsize_bitmap = SZ_4K,
> - .get_resv_regions = s390_iommu_get_resv_regions,
> - .default_domain_ops = &(const struct iommu_domain_ops) {
> - .attach_dev = s390_iommu_attach_device,
> - .map_pages = s390_iommu_map_pages,
> - .unmap_pages = s390_iommu_unmap_pages,
> - .flush_iotlb_all = s390_iommu_flush_iotlb_all,
> - .iotlb_sync = s390_iommu_iotlb_sync,
> - .iotlb_sync_map = s390_iommu_iotlb_sync_map,
> - .iova_to_phys = s390_iommu_iova_to_phys,
> - .free = s390_domain_free,
> +#define S390_IOMMU_COMMON_OPS() \
> + .blocked_domain = &blocking_domain, \
> + .release_domain = &blocking_domain, \
> + .capable = s390_iommu_capable, \
> + .domain_alloc_paging = s390_domain_alloc_paging, \
> + .probe_device = s390_iommu_probe_device, \
> + .device_group = generic_device_group, \
> + .pgsize_bitmap = SZ_4K, \
> + .get_resv_regions = s390_iommu_get_resv_regions, \
> + .default_domain_ops = &(const struct iommu_domain_ops) { \
> + .attach_dev = s390_iommu_attach_device, \
> + .map_pages = s390_iommu_map_pages, \
> + .unmap_pages = s390_iommu_unmap_pages, \
> + .flush_iotlb_all = s390_iommu_flush_iotlb_all, \
> + .iotlb_sync = s390_iommu_iotlb_sync, \
> + .iotlb_sync_map = s390_iommu_iotlb_sync_map, \
> + .iova_to_phys = s390_iommu_iova_to_phys, \
> + .free = s390_domain_free, \
> }
> +
> +static const struct iommu_ops s390_iommu_ops = {
> + S390_IOMMU_COMMON_OPS()
> +};
> +
> +static const struct iommu_ops s390_iommu_rtr_ops = {
> + .identity_domain = &s390_identity_domain,
> + S390_IOMMU_COMMON_OPS()
> };
Though it is a pattern in iommu drivers to use a non-cost ops and
mutate them during probe. For instance you could NULL
s390_iommu_rtr_ops->identity_domain if the platform does not support
it.
On the other hand your version with a const ops is security nicer.
Alternatively, I have a patch series that adds a
domain_alloc_identity() function to get virtio-iommu moved to the new
APIs, that could work well here too and keep the const ops.
Jason
