Re: [PATCH v2 00/25] iommu: Make default_domain's mandatory

2023-05-18 Thread Steven Price
On 16/05/2023 01:00, Jason Gunthorpe wrote:
> This is on github: 
> https://github.com/jgunthorpe/linux/commits/iommu_all_defdom

Tested-by: Steven Price 

Works fine on my Firefly-RK3288.

Thanks,

Steve



Re: [PATCH v2 00/25] iommu: Make default_domain's mandatory

2023-05-17 Thread Nicolin Chen
On Mon, May 15, 2023 at 09:00:33PM -0300, Jason Gunthorpe wrote:
 
> This is on github: 
> https://github.com/jgunthorpe/linux/commits/iommu_all_defdom

Ran some VFIO-passthrough sanity on x86 and ARM64, using this
branch. It should cover partially this series. So, if I may:

Tested-by: Nicolin Chen 

Thanks
Nic

> v2:
>  - FSL is an IDENTITY domain
>  - Delete terga-gart instead of trying to carry it
>  - Use the policy determination from iommu_get_default_domain_type() to
>drive the arm_iommu mode
>  - Reorganize and introduce new patches to do the above:
> * Split the ops->identity_domain to an independent earlier patch
> * Remove the UNMANAGED return from def_domain_type in mtk_v1 earlier
>   so the new iommu_get_default_domain_type() can work
> * Make the driver's def_domain_type have higher policy priority than
>   untrusted
> * Merge the set_platfom_dma_ops hunk from mtk_v1 along with rockchip
>   into the patch that forced IDENTITY on ARM32
>  - Revise sun50i to be cleaner and have a non-NULL internal domain
>  - Reword logging in exynos
>  - Remove the gdev from the group alloc path, instead add a new
>function __iommu_group_domain_alloc() that takes in the group
>and uses the first device. Split this to its own patch
>  - New patch to make iommufd's mock selftest into a real driver
>  - New patch to fix power's partial iommu driver


Re: [PATCH v2 00/25] iommu: Make default_domain's mandatory

2023-05-17 Thread Marek Szyprowski
On 16.05.2023 02:00, Jason Gunthorpe wrote:
> [ There was alot of unexpected complication after rc1 with this series,
> several new patches were needed ]
>
> It has been a long time coming, this series completes the default_domain
> transition and makes it so that the core IOMMU code will always have a
> non-NULL default_domain for every driver on every
> platform. set_platform_dma_ops() turned out to be a bad idea, and so
> completely remove it.
>
> This is achieved by changing each driver to either:
>
> 1 - Convert the existing (or deleted) ops->detach_dev() into an
>  op->attach_dev() of an IDENTITY domain.
>
>  This is based on the theory that the ARM32 HW is able to function when
>  the iommu is turned off as so the turned off state is an IDENTITY
>  translation.
>
> 2 - Use a new PLATFORM domain type. This is a hack to accommodate drivers
>  that we don't really know WTF they do. S390 is legitimately using this
>  to switch to it's platform dma_ops implementation, which is where the
>  name comes from.
>
> 3 - Do #1 and force the default domain to be IDENTITY, this corrects
>  the tegra-smmu case where even an ARM64 system would have a NULL
>  default_domain.
>
> Using this we can apply the rules:
>
> a) ARM_DMA_USE_IOMMU mode always uses either the driver's
> ops->default_domain, ops->def_domain_type(), or an IDENTITY domain.
> All ARM32 drivers provide one of these three options.
>
> b) dma-iommu.c mode uses either the driver's ops->default_domain,
> ops->def_domain_type or the usual DMA API policy logic based on the
> command line/etc to pick IDENTITY/DMA domain types
>
> c) All other arch's (PPC/S390) use ops->default_domain always.
>
> See the patch "Require a default_domain for all iommu drivers" for a
> per-driver breakdown.
>
> The conversion broadly teaches a bunch of ARM32 drivers that they can do
> IDENTITY domains. There is some educated guessing involved that these are
> actual IDENTITY domains. If this turns out to be wrong the driver can be
> trivially changed to use a BLOCKING domain type instead. Further, the
> domain type only matters for drivers using ARM64's dma-iommu.c mode as it
> will select IDENTITY based on the command line and expect IDENTITY to
> work. For ARM32 and other arch cases it is purely documentation.
>
> Finally, based on all the analysis in this series, we can purge
> IOMMU_DOMAIN_UNMANAGED/DMA constants from most of the drivers. This
> greatly simplifies understanding the driver contract to the core
> code. IOMMU drivers should not be involved in policy for how the DMA API
> works, that should be a core core decision.
>
> The main gain from this work is to remove alot of ARM_DMA_USE_IOMMU
> specific code and behaviors from drivers. All that remains in iommu
> drivers after this series is the calls to arm_iommu_create_mapping().
>
> This is a step toward removing ARM_DMA_USE_IOMMU.
>
> The IDENTITY domains added to the ARM64 supporting drivers can be tested
> by booting in ARM64 mode and enabling CONFIG_IOMMU_DEFAULT_PASSTHROUGH. If
> the system still boots then most likely the implementation is an IDENTITY
> domain. If not we can trivially change it to BLOCKING or at worst PLATFORM
> if there is no detail what is going on in the HW.
>
> I think this is pretty safe for the ARM32 drivers as they don't really
> change, the code that was in detach_dev continues to be called in the same
> places it was called before.

Tested-by: Marek Szyprowski 

Works fine on ARM 32bit Exynos based boards.

> This follows the prior series:
>
> https://lore.kernel.org/r/0-v5-1b99ae392328+44574-iommu_err_unwind_...@nvidia.com
>
> This is on github: 
> https://protect2.fireeye.com/v1/url?k=773809ed-1645e36e-773982a2-74fe48600158-bd3a7b89de1f2061=1=7f176af7-9cf3-429b-a0ce-8812c59dfb5c=https%3A%2F%2Fgithub.com%2Fjgunthorpe%2Flinux%2Fcommits%2Fiommu_all_defdom
>
> v2:
>   - FSL is an IDENTITY domain
>   - Delete terga-gart instead of trying to carry it
>   - Use the policy determination from iommu_get_default_domain_type() to
> drive the arm_iommu mode
>   - Reorganize and introduce new patches to do the above:
>  * Split the ops->identity_domain to an independent earlier patch
>  * Remove the UNMANAGED return from def_domain_type in mtk_v1 earlier
>so the new iommu_get_default_domain_type() can work
>  * Make the driver's def_domain_type have higher policy priority than
>untrusted
>  * Merge the set_platfom_dma_ops hunk from mtk_v1 along with rockchip
>into the patch that forced IDENTITY on ARM32
>   - Revise sun50i to be cleaner and have a non-NULL internal domain
>   - Reword logging in exynos
>   - Remove the gdev from the group alloc path, instead add a new
> function __iommu_group_domain_alloc() that takes in the group
> and uses the first device. Split this to its own patch
>   - New patch to make iommufd's mock selftest into a real driver
>   - New patch to fix power's partial