From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Jason Gunthorpe <jgg@nvidia.com>
CC:     Lu Baolu <baolu.lu@linux.intel.com>,
        Christoph Hellwig <hch@infradead.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Joerg Roedel" <joro@8bytes.org>,
        Alex Williamson <alex.williamson@redhat.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        "Raj, Ashok" <ashok.raj@intel.com>,
        Chaitanya Kulkarni <kch@nvidia.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "rafael@kernel.org" <rafael@kernel.org>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        Cornelia Huck <cohuck@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
        "Pan, Jacob jun" <jacob.jun.pan@intel.com>,
        Diana Craciun <diana.craciun@oss.nxp.com>,
        "Will Deacon" <will@kernel.org>
Subject: RE: [PATCH 01/11] iommu: Add device dma ownership set/release
 interfaces
Thread-Topic: [PATCH 01/11] iommu: Add device dma ownership set/release
 interfaces
Thread-Index: AQHX2cX72SW3lNH8302ZqE4v8PTHlawEkdAAgADVRACAAMX3gIACZvYQgAC6LICAAQu4sA==
Date:   Fri, 19 Nov 2021 05:44:35 +0000
Message-ID: <BN9PR11MB5433E5B63E575E2232DFBBE48C9C9@BN9PR11MB5433.namprd11.prod.outlook.com>
References: <20211115020552.2378167-1-baolu.lu@linux.intel.com>
 <20211115020552.2378167-2-baolu.lu@linux.intel.com>
 <YZJdJH4AS+vm0j06@infradead.org>
 <cc7ce6f4-b1ec-49ef-e245-ab6c330154c2@linux.intel.com>
 <20211116134603.GA2105516@nvidia.com>
 <BN9PR11MB5433639E43C37C5D2462BD718C9B9@BN9PR11MB5433.namprd11.prod.outlook.com>
 <20211118133325.GO2105516@nvidia.com>
In-Reply-To: <20211118133325.GO2105516@nvidia.com>
Accept-Language: en-US
Content-Language: en-US
dlp-version: 11.6.200.16
dlp-product: dlpe-windows
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?4XhPW5y7zKvWtaaM4WzsExRHz3Iq3b2DPpDNqEWG650nAisLywqa3Oflwo9N?=
 =?us-ascii?Q?TLJx9+9HLLE9LjxZNjybZUZ0cPh2m3EHtxqjOu3XaisQaOxFaOpZQ6Rz3abc?=
 =?us-ascii?Q?lJ4AF5SYHcQKFATStkwaBcmhWs3xn1Flgi7yiMVpTrH/f58GFtmSMRo1h6Nw?=
 =?us-ascii?Q?T0P7wBYbdnxg9bD/Eo1RRw7blzNQ6cxZKgZaXvUI2HQgyf9qYDF6U5BAfOwY?=
 =?us-ascii?Q?3RlGFVh1w5PmK/9do9eScow8jgdNBjBMNeMLVdUqC0/g0y6MiAihvxwaalbV?=
 =?us-ascii?Q?tu9VaR48HIsyR8BhMHvep4igJban7+5NAAnJHM/VACrbmnS04bcqvw+y61xp?=
 =?us-ascii?Q?KQj06YK6uooCCjvmiWQsJWhjMF1LSokqxN2M3G3fy3G9SCfytYl3feRQMNTk?=
 =?us-ascii?Q?RAddH8aCmLdcRs3bidnwvEmzj2b1/xFgG6fCHIzxtX1eMJFvzUy9imEmvwFS?=
 =?us-ascii?Q?Ws0TlokRzKt3kdbJdkk5IqzQmVPFxc+pwbW5WtzNI6nlB7xIb+TPxpjzVIiv?=
 =?us-ascii?Q?QI7x5EoDqasokPJeH76aFmTKgIr1xpvvyyDnuB4ls6R+RZXwN+S7wuDp7IOF?=
 =?us-ascii?Q?aOCrzlnWqdB7GY0mqHqcwbfzYDkHnCsbkRTQsjHATqBwqoHLcCO8gwAWWb1l?=
 =?us-ascii?Q?DS36sAUqDUDSvdb4qjCUWkKFcGgPh++6ZOXpqx4pYqVDo5RtYk+nWkZtHpV7?=
 =?us-ascii?Q?HCJ3a5+2KeRJI0z8s8FcUPOiaAMT/vq5xZ/VH2rbd/YR3ymE3CNpdd0et/6/?=
 =?us-ascii?Q?6x/OKE+wqySm81SLVvaP5PbPxWUKHD5RTKNEBXRRvgGho1SwWs2N7hDNF1vO?=
 =?us-ascii?Q?ide52cq7PLxAkHQUEYdTymV2s8RYt9I5c+9ELu6CxivNhdMeA8HttEnZk5Eq?=
 =?us-ascii?Q?ADq4d0NL/kPUF96D58qG3Uq3iTG+P0NL/W/snYdTn0ixuKHEVlhrTzWot1qh?=
 =?us-ascii?Q?0BE/4RSR8xOOKvmwf+UmI/B2P+hiSNLXxH85O6OIKzwWIgl/efnbI6kiWo8y?=
 =?us-ascii?Q?P9I8qRwhLlZ9qIvzzGtIGOUkcdYeRi6Uo2WfV7lSkln8dYTshV1guL7Y3ZJu?=
 =?us-ascii?Q?6yD4LhXYtLie+AV/URfRFDzFNxU1aUWInKHbQU0CWeBhUN9dBX7UGNbp22I0?=
 =?us-ascii?Q?awHcDApBF8FU80pEPK63P/AAochzDIviZVHgF0F3086GzNmjXPfEYTunHaLI?=
 =?us-ascii?Q?Pg5/tVTCw9JJ7VsvRVUT3hxg/962e9OCbW00yW9f503xJCLkf52OEuYuD0CW?=
 =?us-ascii?Q?0zImzR9hRPvZH3exVvHhrQ3vC2UJoOuIcmEbmz8Z7gRPyYOXq5jaUY8HOVI3?=
 =?us-ascii?Q?pd30mcC9ZJ7r5ORJz9dED0t5aG80EbDrbH1zDpA8TvRcFzvJ1Q3c4huTs593?=
 =?us-ascii?Q?lpb6KTJGxa6Y6JTuFOEOT9DkH5s8droSci9mz1UQk5zimRr9iltBIJj8FmYM?=
 =?us-ascii?Q?pNd98mGo4Em8JUf4HrzuWjoTKLoP6hITN8suYmya/X2f/JTLuyarZqLxVrT6?=
 =?us-ascii?Q?CmLb7P1e9TrT1fq/ZDVnS/VBrMEN9j8LadDrjmqswKbLd6f6orUx/+ivMaSh?=
 =?us-ascii?Q?dgGLNpKdhYuLRXOgaIDRrKAabi2LoXpWekyYmpIZQIbzielhnh6zSSbX00Qf?=
 =?us-ascii?Q?HA=3D=3D?=
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: 138a110b-8d55-494e-daf1-08d9ab1fab0a
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Nov 2021 05:44:35.0920
 (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: U0IZm3EJ6GW/suwo3q626mdBHLM/ErBD+6Z3Mmn2Zz+CrRMluN1toK+jIcZKwgdHvgKzVh/WSYCA2QW9RLLlJQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5338
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, November 18, 2021 9:33 PM
>=20
> > In concept a singleton group is different from a
> > multi-devices group which has only one device bound to driver...
>=20
> Really? Why? I don't see it that way..
>=20
> A singleton group is just a multi-device group that hasn't been
> hotplugged yet.
>=20
> We don't seem to have the concept of a "true" singleton group which is
> permanently single due to HW features.
>=20
> > This series aims to avoid conflict having both user and kernel drivers
> > mixed in a multi-devices group.

Well, the difference is just in literal. I don't know the background
why the existing iommu_attach_device() users want to do it this
way. But given the condition in iommu_attach_device() it could
in theory imply some unknown hardware-level side effect which=20
may break the desired functionality once the group size grows=20
beyond singleton. Is it a real case? I don't know...

You are now redefining that condition from singleton group to
multi-devices group with single driver bound. As long as no object
from existing driver users, I'm fine with it. But still want to raise
awareness as it does change the existing semantics (though might
be considered as an imperfect way).

Thanks
Kevin