Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Jason Gunthorpe <jgg@nvidia.com>,
        "robin.murphy@arm.com" <robin.murphy@arm.com>
CC:     Jean-Philippe Brucker <jean-philippe@linaro.org>,
        Alex Williamson <alex.williamson@redhat.com>,
        "Liu, Yi L" <yi.l.liu@intel.com>, "hch@lst.de" <hch@lst.de>,
        "jasowang@redhat.com" <jasowang@redhat.com>,
        "joro@8bytes.org" <joro@8bytes.org>,
        "parav@mellanox.com" <parav@mellanox.com>,
        "lkml@metux.net" <lkml@metux.net>,
        "pbonzini@redhat.com" <pbonzini@redhat.com>,
        "lushenming@huawei.com" <lushenming@huawei.com>,
        "eric.auger@redhat.com" <eric.auger@redhat.com>,
        "corbet@lwn.net" <corbet@lwn.net>,
        "Raj, Ashok" <ashok.raj@intel.com>,
        "yi.l.liu@linux.intel.com" <yi.l.liu@linux.intel.com>,
        "Tian, Jun J" <jun.j.tian@intel.com>, "Wu, Hao" <hao.wu@intel.com>,
        "Jiang, Dave" <dave.jiang@intel.com>,
        "jacob.jun.pan@linux.intel.com" <jacob.jun.pan@linux.intel.com>,
        "kwankhede@nvidia.com" <kwankhede@nvidia.com>,
        "robin.murphy@arm.com" <robin.murphy@arm.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
        "dwmw2@infradead.org" <dwmw2@infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "baolu.lu@linux.intel.com" <baolu.lu@linux.intel.com>,
        "david@gibson.dropbear.id.au" <david@gibson.dropbear.id.au>,
        "nicolinc@nvidia.com" <nicolinc@nvidia.com>
Subject: RE: [RFC 10/20] iommu/iommufd: Add IOMMU_DEVICE_GET_INFO
Thread-Topic: [RFC 10/20] iommu/iommufd: Add IOMMU_DEVICE_GET_INFO
Thread-Index: AQHXrSGNbNtRgavabUSKJjvt8l12BauwlhaAgAAouwCAACufAIAAgyUAgAAUFACAAAdkgIAAB/8AgAkqbYA=
Date:   Wed, 29 Sep 2021 08:48:28 +0000
Message-ID: <BN9PR11MB5433D75C09C6FDA01C2B7CF48CA99@BN9PR11MB5433.namprd11.prod.outlook.com>
References: <20210919063848.1476776-1-yi.l.liu@intel.com>
 <20210919063848.1476776-11-yi.l.liu@intel.com>
 <20210922152407.1bfa6ff7.alex.williamson@redhat.com>
 <20210922234954.GB964074@nvidia.com>
 <BN9PR11MB543333AD3C81312115686AAA8CA39@BN9PR11MB5433.namprd11.prod.outlook.com>
 <YUxTvCt1mYDntO8z@myrica> <20210923112716.GE964074@nvidia.com>
 <BN9PR11MB5433BCFCF3B0CB657E9BFE898CA39@BN9PR11MB5433.namprd11.prod.outlook.com>
 <20210923122220.GL964074@nvidia.com>
In-Reply-To: <20210923122220.GL964074@nvidia.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?oTUszFfXuzYcI8Fpmrarkb0ifhipruO7YE70A9eKwg1CkQ4gEFbMQaY3pJGM?=
 =?us-ascii?Q?HB73VNnoMnQfjHZJDsCaXZjcpkKzc9vzszjKy6O0Ww/o44dK8ld/rkQU0BOq?=
 =?us-ascii?Q?gLZIyLyTkzXOOftLGJiFMEDpNV9bAIrVt2emrZvTe9Fw8wptkxtbfF5L+vI1?=
 =?us-ascii?Q?g6ycF6MrZjo7qsbeT3LI7xKuvjq63VHOmtfRslv7OnvBGgP5KEk6qvmKsgpH?=
 =?us-ascii?Q?H3ngTlurInu1+28wiNiYf3Dnuyi36XaHqzvrKZ9/TCV2BAJFC8tCdlGWU6Ul?=
 =?us-ascii?Q?fejUFzAD+qykqatY8IerMfohnximC4G2HQtOEYKjEETvejS0yF7PuS/62Nba?=
 =?us-ascii?Q?XqkWVDg+YZPZCnLXsmciYEhHD1+yeq0+rYH2XnaCsJJE8eW7oqYQDzZDCBhS?=
 =?us-ascii?Q?1mkJV78mrfRa6Y/DYtrra/XgH+3PqsvL/MAGbgal92xPJsNS9bjenZZQl296?=
 =?us-ascii?Q?33+tBIrgj4O9r7xa4sqklnGkxaq+YuKLfJ6v9XG4ykfPRGEZZtqydx65twpm?=
 =?us-ascii?Q?CxhqlaM5/HgsY7oedNZ9qsWmFtgzPHMGE77dLGiYjJDTUgYNPKSG6I3QhaMv?=
 =?us-ascii?Q?3v0J0GJaaqef0pZcknjwxs6s2EgbV1l5hlGpDrLgj2cLxKa25bs8T78AHi0x?=
 =?us-ascii?Q?r1gynX8f32SRTEd3TeSPT0MMi7C4gacKyudwf6oNY/UZ0DuTGUDR8iuwpsru?=
 =?us-ascii?Q?mPdr8my1nh+txxgocw19A8tSwJwdibo+VAh348XRzgnlRZx/kjQ9UAwJhQoj?=
 =?us-ascii?Q?cQqlwmYwz7an1jI0ptqtH6bgmnQomMCkKdxMS6JBjPBDURksmC8UlTh+9IOU?=
 =?us-ascii?Q?VB37syNNSfdhjaqT7Xv7DT2D2dycWmbFxlC6q+znMl25yta9BN9qZDoJd84N?=
 =?us-ascii?Q?Dic0at9I/zoy/N/vATdVMcGQ+wsgDmWTLFfRp3jEITdhVk4TEK5/JIkzoukG?=
 =?us-ascii?Q?5mhm2dQ0T76DTy7awsl+xrMsumaC0EAKDBO+KOqd+5rllSW1kf0nEFnK7l5I?=
 =?us-ascii?Q?eB2ECKcXSzlK72NSaQOy7iQah1wwaMQxHw5K6PxkMPMtJt2cKn6uCNhKdgD9?=
 =?us-ascii?Q?WXGNCphvMDXxAqLRmKHjIYIppJImSo+iER+5Qxm8OypRqzmlH/x3dY5AqMnp?=
 =?us-ascii?Q?0nI/1PYGdqJf5/35OvUjrCEiUacrJUY7Q8XLcP/Y345T65C6JN8IgcS5/0SR?=
 =?us-ascii?Q?p1zrU7Is7AKN71FOZ8KYMLKcMU0YVrDThXXSsAC8jgXaWbsgpgq7Liqht0GY?=
 =?us-ascii?Q?n8d5txvkw+fRLrD8VXL4B4ems4EENJOPmb7kBpM0PdlK2VskFHVmmfA+fIQN?=
 =?us-ascii?Q?Qv1f0jw6xxpH8omgDtBeyRPH?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5433.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4785c995-aeb6-4cf3-e78b-08d98325e83e
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Sep 2021 08:48:28.1428
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: qBIIu2XHN3COhkmo28i25aC+prjyjnmnGN08ecdKUc7XR2Krodp3AFj0pBr+iXTBdlmqKlTMX9Dm7yynXy5WVw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR11MB2594
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

+Robin.

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, September 23, 2021 8:22 PM
>=20
> On Thu, Sep 23, 2021 at 12:05:29PM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > Sent: Thursday, September 23, 2021 7:27 PM
> > >
> > > On Thu, Sep 23, 2021 at 11:15:24AM +0100, Jean-Philippe Brucker wrote=
:
> > >
> > > > So we can only tell userspace "No_snoop is not supported" (provided
> we
> > > > even want to allow them to enable No_snoop). Users in control of
> stage-1
> > > > tables can create non-cacheable mappings through MAIR attributes.
> > >
> > > My point is that ARM is using IOMMU_CACHE to control the overall
> > > cachability of the DMA
> > >
> > > ie not specifying IOMMU_CACHE requires using the arch specific DMA
> > > cache flushers.
> > >
> > > Intel never uses arch specifc DMA cache flushers, and instead is
> > > abusing IOMMU_CACHE to mean IOMMU_BLOCK_NO_SNOOP on DMA
> that
> > > is always
> > > cachable.
> >
> > it uses IOMMU_CACHE to force all DMAs to snoop, including those which
> > has non_snoop flag and wouldn't snoop cache if iommu is disabled.
> Nothing
> > is blocked.
>=20
> I see it differently, on Intel the only way to bypass the cache with
> DMA is to specify the no-snoop bit in the TLP. The IOMMU PTE flag we
> are talking about tells the IOMMU to ignore the no snoop bit.
>=20
> Again, Intel arch in the kernel does not support the DMA cache flush
> arch API and *DOES NOT* support incoherent DMA at all.
>=20
> ARM *does* implement the DMA cache flush arch API and is using
> IOMMU_CACHE to control if the caller will, or will not call the cache
> flushes.

I still didn't fully understand this point after reading the code. Looking
at dma-iommu its cache flush functions are all coded with below as
the first check:

        if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
                return;

dev->dma_coherent is initialized upon firmware info, not decided by=20
IOMMU_CACHE.

i.e. it's not IOMMU_CACHE to decide whether cache flushes should
be called.

Probably the confusion comes from __iommu_dma_alloc_noncontiguous:

        if (!(ioprot & IOMMU_CACHE)) {
                struct scatterlist *sg;
                int i;

                for_each_sg(sgt->sgl, sg, sgt->orig_nents, i)
                        arch_dma_prep_coherent(sg_page(sg), sg->length);
        }

Here it makes more sense to be if (!coherent) {}.

with above being corrected, I think all iommu drivers do associate=20
IOMMU_CACHE to the snoop aspect:

Intel:
    - either force snooping by ignoring snoop bit in TLP (IOMMU_CACHE)
    - or has snoop decided by TLP (!IOMMU_CACHE)

ARM:
    - set to snoop format if IOMMU_CACHE
    - set to nonsnoop format if !IOMMU_CACHE
(in both cases TLP snoop bit is ignored?)

Other archs
    - ignore IOMMU_CACHE as cache is always snooped via their IOMMUs

>=20
> This is fundamentally different from what Intel is using it for.
>=20
> > but why do you call it abuse? IOMMU_CACHE was first introduced for
> > Intel platform:
>=20
> IMHO ARM changed the meaning when Robin linked IOMMU_CACHE to
> dma_is_coherent stuff. At that point it became linked to 'do I need to
> call arch cache flushers or not'.
>=20

I didn't identify the exact commit for above meaning change.

Robin, could you help share some thoughts here?

Thanks
Kevin