lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 16 Jan 2023 17:24:56 +0100
From:   Marek Szyprowski <m.szyprowski@...sung.com>
To:     Lu Baolu <baolu.lu@...ux.intel.com>,
        Joerg Roedel <joro@...tes.org>,
        Jason Gunthorpe <jgg@...dia.com>,
        Christoph Hellwig <hch@...radead.org>,
        Kevin Tian <kevin.tian@...el.com>,
        Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>
Cc:     Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
        Vasant Hegde <vasant.hegde@....com>,
        Hector Martin <marcan@...can.st>,
        Sven Peter <sven@...npeter.dev>,
        Rob Clark <robdclark@...il.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Yong Wu <yong.wu@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Heiko Stuebner <heiko@...ech.de>,
        Matthew Rosato <mjrosato@...ux.ibm.com>,
        Orson Zhai <orsonzhai@...il.com>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        Chunyan Zhang <zhang.lyra@...il.com>,
        Chen-Yu Tsai <wens@...e.org>,
        Thierry Reding <thierry.reding@...il.com>,
        iommu@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 0/5] iommu: Retire detach_dev callback

Hi All,


On 10.01.2023 03:54, Lu Baolu wrote:
> Hi folks,
>
> The iommu core calls the driver's detach_dev domain op callback only when
> a device is finished assigning to user space and
> iommu_group_release_dma_owner() is called to return the device to the
> kernel, where iommu core wants to set the default domain to the device but
> the driver didn't provide one. The code looks like:
>
>          /*
>           * New drivers should support default domains and so the detach_dev() op
>           * will never be called. Otherwise the NULL domain represents some
>           * platform specific behavior.
>           */
>          if (!new_domain) {
>                  if (WARN_ON(!group->domain->ops->detach_dev))
>                          return -EINVAL;
>                  __iommu_group_for_each_dev(group, group->domain,
>                                             iommu_group_do_detach_device);
>                  group->domain = NULL;
>                  return 0;
>          }
>
> In other words, if the iommu driver provides default domains, the
> .detach_dev callback will never be called; Otherwise, the .detach_dev
> callback is actually called to return control back to the platform DMA
> ops, other than detaching the domain from device.
>
> This series cleanups this by:
>
> - If the IOMMU driver provides default domains, remove .detach_dev
>    callback.
> - Adds a new set_platform_dma iommu op. Any IOMMU driver that doesn't
>    provide default domain should implement set_platform_dma callback
>    instead.
> - Retire .detach_dev callback.
>
> This series originates from various discussion in the community. Thanks
> to Jason, Robin and all others for their ideas.

I wonder how to handle the ARM 32bit case, which doesn't use the default 
domain solution. Today, once this patchset has been merged to 
linux-next, I've noticed that it broke booting of ARM 32bit Exynos based 
boards.

The final solution would be to switch ARM 32bit to generic DMA-IOMMU 
glue, but I'm not sure it this will happen soon. I will try to check if 
any kind of quick workaround can be applied to get it working again.


> The whole series is available on github:
> https://protect2.fireeye.com/v1/url?k=ef2183b6-b0bababa-ef2008f9-000babff3563-336828b3433153d2&q=1&e=4ae5dae4-383f-4a82-9449-3f08f0422cb1&u=https%3A%2F%2Fgithub.com%2FLuBaolu%2Fintel-iommu%2Fcommits%2Fiommu-retire-detach_dev-v5
>
> Change log:
> v5:
>   - Merge some patches to make the series cute. No functionality changes.
>   - Check the op directly and WARN_ON the lack of any necessary
>     callbacks. Get rid of the ret and EINVAL.
>
> v4:
>   - https://lore.kernel.org/linux-iommu/20230104125725.271850-1-baolu.lu@linux.intel.com/
>   - Drop the patch which renamed .attach_dev to .set_dev. As Robin said,
>     "... I also wouldn't say that "attach" loses its meaning in a context
>     where an equivalent "detach" operation is only ever implicit in
>     reattaching to something else...". If we have a better name in the
>     future, we can do it in other series.
>   - Adjust the patch of "iommu: Add set_platform_dma_ops iommu ops"
>     according to Jason's following suggestion " ... This is a bit ugly,
>     it would be better to make the previous patch call set_platform_dma
>     if it is set instead of detach_dev and then these patches should just
>     rename the driver's fsl_pamu_detach_device to
>     fsl_pamu_set_platform_dma ..."
>   - Add a new patch to remove deferred attach check from
>     __iommu_detach_domain() path. Make it a separate patch as the
>     prerequisite to remove __iommu_detach_device() helper.
>   - Rename set_platform_dma to set_platform_dma_ops to make it more
>     meaningful.
>
> v3:
>   - https://lore.kernel.org/linux-iommu/20221128064648.1934720-1-baolu.lu@linux.intel.com/
>   - Setting blocking domain is not conceptually equal to detach_dev.
>     Dropped all blocking domain related staffs in the previous version.
>
> v2:
>   - https://lore.kernel.org/linux-iommu/20220826123014.52709-1-baolu.lu@linux.intel.com/
>   - Replace .detach_dev callback with static block domain ops;
>   - Rename .attach_dev to .set_dev.
>
> v1:
>   - https://lore.kernel.org/linux-iommu/20220516015759.2952771-1-baolu.lu@linux.intel.com/
>
> Jason Gunthorpe (1):
>    iommu: Remove deferred attach check from __iommu_detach_device()
>
> Lu Baolu (4):
>    iommu: Remove detach_dev callbacks
>    iommu: Add set_platform_dma_ops iommu ops
>    iommu: Add set_platform_dma_ops callbacks
>    iommu: Remove detach_dev callback
>
>   include/linux/iommu.h                   |  8 ++-
>   include/trace/events/iommu.h            |  7 --
>   drivers/iommu/amd/iommu.c               | 26 -------
>   drivers/iommu/apple-dart.c              | 24 -------
>   drivers/iommu/arm/arm-smmu/qcom_iommu.c | 23 ------
>   drivers/iommu/exynos-iommu.c            |  1 -
>   drivers/iommu/fsl_pamu_domain.c         |  6 +-
>   drivers/iommu/iommu-traces.c            |  1 -
>   drivers/iommu/iommu.c                   | 94 ++++++++++++-------------
>   drivers/iommu/ipmmu-vmsa.c              | 16 -----
>   drivers/iommu/msm_iommu.c               |  6 +-
>   drivers/iommu/mtk_iommu.c               |  9 ---
>   drivers/iommu/mtk_iommu_v1.c            |  4 +-
>   drivers/iommu/omap-iommu.c              |  6 +-
>   drivers/iommu/rockchip-iommu.c          |  1 -
>   drivers/iommu/s390-iommu.c              |  7 +-
>   drivers/iommu/sprd-iommu.c              | 16 -----
>   drivers/iommu/sun50i-iommu.c            |  1 -
>   drivers/iommu/tegra-gart.c              |  6 +-
>   drivers/iommu/tegra-smmu.c              |  5 +-
>   20 files changed, 69 insertions(+), 198 deletions(-)
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ